From patchwork Mon Oct 15 08:23:06 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bernhard Reutner-Fischer X-Patchwork-Id: 983957 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-487530-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="QRn6BbYz"; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="NsT+6goA"; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 42YWkj1bwJz9s47 for ; Mon, 15 Oct 2018 19:26:51 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; q=dns; s=default; b=GFZ MtKsO0KXtEoS8f29sw6HpH+Gqb1ylMEiMSudFk9hL/eUYDnl5L09WzJ6DsI4qRod krmvEe5QKku11WbSkWuGgMVSzY1mRmS+3ASk3y58bGFWnMUl3jjyQMwLfNxTiDjD zRodcuyJqC0y12oPdNyVJqK8rT6HPsoqU97W7rS8= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; s=default; bh=tsf4wX2QS wO7nST657hnQNGvFsI=; b=QRn6BbYzlYazxdsr4FMJIryHjfnOAAj0p9ub2ck24 Q5PjM9PEX12U5m+GFLaTKkwSZNqejM7j4VPb0hq0yyDtoQBGTaa1swqZb2A8STup I1a8TRI9Llc5cOFfIK1E587lXd9F5KJrEYeTAiiYp90Ru84/ck7odUgtmu5YmwWc y0= Received: (qmail 34576 invoked by alias); 15 Oct 2018 08:26:10 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 78632 invoked by uid 89); 15 Oct 2018 08:23:28 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, FREEMAIL_FROM, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=FINAL, sibling, switching, 11976 X-HELO: mail-wm1-f45.google.com Received: from mail-wm1-f45.google.com (HELO mail-wm1-f45.google.com) (209.85.128.45) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 15 Oct 2018 08:23:25 +0000 Received: by mail-wm1-f45.google.com with SMTP id r63-v6so17923727wma.4; Mon, 15 Oct 2018 01:23:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=dcBM3dWle31yTordBseXlaWMpQwOIeUcD3rzkUC19hY=; b=NsT+6goAmvZQfDT1kAcBZGUm76qEGj3yGP7DhnZ3eGNiTSDVeKodvB3wl4RcUB79ln v3AypiVVyrgfpiRJMk5NFey4QPc4A53Ld6iuISgWhQt0frzvGTTU/QDiPRn32CSob52h 5/8eDfUU+zq7L/M3IRqxTsQ0oXYIWQGxGYZrrDuzo+F6l8AIGVLBDcA6pvbmGLix3Ysz 3cp8u5RqbgSpSvugBBK14u3nIvQ8aFaXmGAWyoDXmkevKHmxYfSrXuTTiNQcDRXz3xT6 ho+pXev4snXMro0M3TIf3BeFb+V9+H3bxS5eP49l/0kl3Hljp6l6mtiJBTYdnYjvhX0e J0pw== Received: from nbbrfq.loc (91-119-96-225.dsl.dynamic.surfer.at. [91.119.96.225]) by smtp.gmail.com with ESMTPSA id j6-v6sm8412523wmd.29.2018.10.15.01.23.22 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 15 Oct 2018 01:23:22 -0700 (PDT) Received: from b by nbbrfq.loc with local (Exim 4.91) (envelope-from ) id 1gBy9o-00061T-MG; Mon, 15 Oct 2018 10:23:20 +0200 From: Bernhard Reutner-Fischer To: fortran@gcc.gnu.org Cc: Bernhard Reutner-Fischer , gcc-patches@gcc.gnu.org, Tobias Burnus Subject: [PATCH,FORTRAN] Fix memory leak in finalization wrappers Date: Mon, 15 Oct 2018 10:23:06 +0200 Message-Id: <20181015082306.23083-1-rep.dot.nop@gmail.com> In-Reply-To: References: MIME-Version: 1.0 X-IsSubscribed: yes If a finalization is not required we created a namespace containing formal arguments for an internal interface definition but never used any of these. So the whole sub_ns namespace was not wired up to the program and consequently was never freed. The fix is to simply not generate any finalization wrappers if we know that it will be unused. Note that this reverts back to the original r190869 (8a96d64282ac534cb597f446f02ac5d0b13249cc) handling for this case by reverting this specific part of r194075 (f1ee56b4be7cc3892e6ccc75d73033c129098e87) for PR fortran/37336. Regtests cleanly, installed to the fortran-fe-stringpool branch, sent here for reference and later inclusion. I might plug a few more leaks in preparation of switching to hash-maps. I fear that the leaks around interfaces are another candidate ;) Should probably add a tag for the compile-time leak PR68800 shouldn't i. valgrind summary for e.g. gfortran.dg/abstract_type_3.f03 and gfortran.dg/abstract_type_4.f03 where ".orig" is pristine trunk and ".mine" contains this fix: at3.orig.vg:LEAK SUMMARY: at3.orig.vg- definitely lost: 8,460 bytes in 11 blocks at3.orig.vg- indirectly lost: 13,288 bytes in 55 blocks at3.orig.vg- possibly lost: 0 bytes in 0 blocks at3.orig.vg- still reachable: 572,278 bytes in 2,142 blocks at3.orig.vg- suppressed: 0 bytes in 0 blocks at3.orig.vg- at3.orig.vg-Use --track-origins=yes to see where uninitialised values come from at3.orig.vg-ERROR SUMMARY: 38 errors from 33 contexts (suppressed: 0 from 0) --- at3.mine.vg:LEAK SUMMARY: at3.mine.vg- definitely lost: 344 bytes in 1 blocks at3.mine.vg- indirectly lost: 7,192 bytes in 18 blocks at3.mine.vg- possibly lost: 0 bytes in 0 blocks at3.mine.vg- still reachable: 572,278 bytes in 2,142 blocks at3.mine.vg- suppressed: 0 bytes in 0 blocks at3.mine.vg- at3.mine.vg-ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0) at3.mine.vg-ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0) at4.orig.vg:LEAK SUMMARY: at4.orig.vg- definitely lost: 13,751 bytes in 12 blocks at4.orig.vg- indirectly lost: 11,976 bytes in 60 blocks at4.orig.vg- possibly lost: 0 bytes in 0 blocks at4.orig.vg- still reachable: 572,278 bytes in 2,142 blocks at4.orig.vg- suppressed: 0 bytes in 0 blocks at4.orig.vg- at4.orig.vg-Use --track-origins=yes to see where uninitialised values come from at4.orig.vg-ERROR SUMMARY: 18 errors from 16 contexts (suppressed: 0 from 0) -- at4.mine.vg:LEAK SUMMARY: at4.mine.vg- definitely lost: 3,008 bytes in 3 blocks at4.mine.vg- indirectly lost: 4,056 bytes in 11 blocks at4.mine.vg- possibly lost: 0 bytes in 0 blocks at4.mine.vg- still reachable: 572,278 bytes in 2,142 blocks at4.mine.vg- suppressed: 0 bytes in 0 blocks at4.mine.vg- at4.mine.vg-ERROR SUMMARY: 3 errors from 3 contexts (suppressed: 0 from 0) at4.mine.vg-ERROR SUMMARY: 3 errors from 3 contexts (suppressed: 0 from 0) gcc/fortran/ChangeLog: 2018-10-12 Bernhard Reutner-Fischer * class.c (generate_finalization_wrapper): Do leak finalization wrappers if they will not be used. * expr.c (gfc_free_actual_arglist): Formatting fix. * gfortran.h (gfc_free_symbol): Pass argument by reference. (gfc_release_symbol): Likewise. (gfc_free_namespace): Likewise. * symbol.c (gfc_release_symbol): Adjust acordingly. (free_components): Set procedure pointer components of derived types to NULL after freeing. (free_tb_tree): Likewise. (gfc_free_symbol): Set sym to NULL after freeing. (gfc_free_namespace): Set namespace to NULL after freeing. --- gcc/fortran/class.c | 25 +++++++++---------------- gcc/fortran/expr.c | 2 +- gcc/fortran/gfortran.h | 6 +++--- gcc/fortran/symbol.c | 19 ++++++++++--------- 4 files changed, 23 insertions(+), 29 deletions(-) diff --git a/gcc/fortran/class.c b/gcc/fortran/class.c index 69c95fc5dfa..e0bb381a55f 100644 --- a/gcc/fortran/class.c +++ b/gcc/fortran/class.c @@ -1533,7 +1533,6 @@ generate_finalization_wrapper (gfc_symbol *derived, gfc_namespace *ns, gfc_code *last_code, *block; const char *name; bool finalizable_comp = false; - bool expr_null_wrapper = false; gfc_expr *ancestor_wrapper = NULL, *rank; gfc_iterator *iter; @@ -1561,13 +1560,17 @@ generate_finalization_wrapper (gfc_symbol *derived, gfc_namespace *ns, } /* No wrapper of the ancestor and no own FINAL subroutines and allocatable - components: Return a NULL() expression; we defer this a bit to have have + components: Return a NULL() expression; we defer this a bit to have an interface declaration. */ if ((!ancestor_wrapper || ancestor_wrapper->expr_type == EXPR_NULL) && !derived->attr.alloc_comp && (!derived->f2k_derived || !derived->f2k_derived->finalizers) && !has_finalizer_component (derived)) - expr_null_wrapper = true; + { + vtab_final->initializer = gfc_get_null_expr (NULL); + gcc_assert (vtab_final->ts.interface == NULL); + return; + } else /* Check whether there are new allocatable components. */ for (comp = derived->components; comp; comp = comp->next) @@ -1581,7 +1584,7 @@ generate_finalization_wrapper (gfc_symbol *derived, gfc_namespace *ns, /* If there is no new finalizer and no new allocatable, return with an expr to the ancestor's one. */ - if (!expr_null_wrapper && !finalizable_comp + if (!finalizable_comp && (!derived->f2k_derived || !derived->f2k_derived->finalizers)) { gcc_assert (ancestor_wrapper && ancestor_wrapper->ref == NULL @@ -1605,8 +1608,7 @@ generate_finalization_wrapper (gfc_symbol *derived, gfc_namespace *ns, /* Set up the namespace. */ sub_ns = gfc_get_namespace (ns, 0); sub_ns->sibling = ns->contained; - if (!expr_null_wrapper) - ns->contained = sub_ns; + ns->contained = sub_ns; sub_ns->resolved = 1; /* Set up the procedure symbol. */ @@ -1622,7 +1624,7 @@ generate_finalization_wrapper (gfc_symbol *derived, gfc_namespace *ns, final->ts.kind = 4; final->attr.artificial = 1; final->attr.always_explicit = 1; - final->attr.if_source = expr_null_wrapper ? IFSRC_IFBODY : IFSRC_DECL; + final->attr.if_source = IFSRC_DECL; if (ns->proc_name->attr.flavor == FL_MODULE) final->module = ns->proc_name->name; gfc_set_sym_referenced (final); @@ -1672,15 +1674,6 @@ generate_finalization_wrapper (gfc_symbol *derived, gfc_namespace *ns, final->formal->next->next->sym = fini_coarray; gfc_commit_symbol (fini_coarray); - /* Return with a NULL() expression but with an interface which has - the formal arguments. */ - if (expr_null_wrapper) - { - vtab_final->initializer = gfc_get_null_expr (NULL); - vtab_final->ts.interface = final; - return; - } - /* Local variables. */ gfc_get_symbol (gfc_get_string ("%s", "idx"), sub_ns, &idx); diff --git a/gcc/fortran/expr.c b/gcc/fortran/expr.c index cc12e0a8402..3d744ec9641 100644 --- a/gcc/fortran/expr.c +++ b/gcc/fortran/expr.c @@ -533,7 +533,7 @@ gfc_free_actual_arglist (gfc_actual_arglist *a1) { a2 = a1->next; if (a1->expr) - gfc_free_expr (a1->expr); + gfc_free_expr (a1->expr); free (a1); a1 = a2; } diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h index 4612835706b..3466c42132f 100644 --- a/gcc/fortran/gfortran.h +++ b/gcc/fortran/gfortran.h @@ -3032,8 +3032,8 @@ gfc_user_op *gfc_get_uop (const char *); gfc_user_op *gfc_find_uop (const char *, gfc_namespace *); const char *gfc_get_uop_from_name (const char*); const char *gfc_get_name_from_uop (const char*); -void gfc_free_symbol (gfc_symbol *); -void gfc_release_symbol (gfc_symbol *); +void gfc_free_symbol (gfc_symbol *&); +void gfc_release_symbol (gfc_symbol *&); gfc_symbol *gfc_new_symbol (const char *, gfc_namespace *); gfc_symtree* gfc_find_symtree_in_proc (const char *, gfc_namespace *); int gfc_find_symbol (const char *, gfc_namespace *, int, gfc_symbol **); @@ -3058,7 +3058,7 @@ void gfc_commit_symbols (void); void gfc_commit_symbol (gfc_symbol *); gfc_charlen *gfc_new_charlen (gfc_namespace *, gfc_charlen *); void gfc_free_charlen (gfc_charlen *, gfc_charlen *); -void gfc_free_namespace (gfc_namespace *); +void gfc_free_namespace (gfc_namespace *&); void gfc_symbol_init_2 (void); void gfc_symbol_done_2 (void); diff --git a/gcc/fortran/symbol.c b/gcc/fortran/symbol.c index 09ad2bbf0cd..c99c106a0c0 100644 --- a/gcc/fortran/symbol.c +++ b/gcc/fortran/symbol.c @@ -2590,8 +2590,9 @@ free_components (gfc_component *p) gfc_free_expr (p->kind_expr); if (p->param_list) gfc_free_actual_arglist (p->param_list); - free (p->tb); + free (p->tb); + p->tb = NULL; free (p); } } @@ -3070,7 +3071,7 @@ set_symbol_common_block (gfc_symbol *sym, gfc_common_head *common_block) /* Remove a gfc_symbol structure and everything it points to. */ void -gfc_free_symbol (gfc_symbol *sym) +gfc_free_symbol (gfc_symbol *&sym) { if (sym == NULL) @@ -3078,8 +3079,6 @@ gfc_free_symbol (gfc_symbol *sym) gfc_free_array_spec (sym->as); - free_components (sym->components); - gfc_free_expr (sym->value); gfc_free_namelist (sym->namelist); @@ -3094,19 +3093,22 @@ gfc_free_symbol (gfc_symbol *sym) gfc_free_namespace (sym->f2k_derived); + free_components (sym->components); + set_symbol_common_block (sym, NULL); if (sym->param_list) gfc_free_actual_arglist (sym->param_list); free (sym); + sym = NULL; } /* Decrease the reference counter and free memory when we reach zero. */ void -gfc_release_symbol (gfc_symbol *sym) +gfc_release_symbol (gfc_symbol *&sym) { if (sym == NULL) return; @@ -3826,10 +3828,8 @@ free_tb_tree (gfc_symtree *t) free_tb_tree (t->left); free_tb_tree (t->right); - - /* TODO: Free type-bound procedure structs themselves; probably needs some - sort of ref-counting mechanism. */ free (t->n.tb); + t->n.tb = NULL; free (t); } @@ -4019,7 +4019,7 @@ free_entry_list (gfc_entry_list *el) taken care of when a specific name is freed. */ void -gfc_free_namespace (gfc_namespace *ns) +gfc_free_namespace (gfc_namespace *&ns) { gfc_namespace *p, *q; int i; @@ -4057,6 +4057,7 @@ gfc_free_namespace (gfc_namespace *ns) gfc_free_data (ns->data); p = ns->contained; free (ns); + ns = NULL; /* Recursively free any contained namespaces. */ while (p != NULL)