From patchwork Sun May 24 18:55:48 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Thomas Koenig X-Patchwork-Id: 1296941 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=8.43.85.97; helo=sourceware.org; envelope-from=gcc-patches-bounces@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=gcc.gnu.org Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.a=rsa-sha256 header.s=default header.b=Pzxjc8Cq; dkim-atps=neutral Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 49VTvq3jjbz9sPK for ; Mon, 25 May 2020 04:56:05 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 7947A388B02A; Sun, 24 May 2020 18:55:59 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 7947A388B02A DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1590346559; bh=Ywz8aF2topPLBqcDHEU9Sq93vMyeb8SSkI/8SqRre4U=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=Pzxjc8Cq3re/SWPkJgdyzOeNqY+36jfxcL0XX+xCjeKALeZ13MemSiecHYqw10LjM Q6FkOqeNanugaClGIduT0HUhN4QvVjU2PHKwrVbWwapIeYmZ1nrVDSpk2+6PU+SwOk Upu+tFxG9AWYbKKVkAZTM7JwqAq8D6jvLhNs1aFg= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from cc-smtpout1.netcologne.de (cc-smtpout1.netcologne.de [IPv6:2001:4dd0:100:1062:25:2:0:1]) by sourceware.org (Postfix) with ESMTPS id B4748385BF81; Sun, 24 May 2020 18:55:56 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org B4748385BF81 Received: from cc-smtpin2.netcologne.de (cc-smtpin2.netcologne.de [89.1.8.202]) by cc-smtpout1.netcologne.de (Postfix) with ESMTP id EE73713669; Sun, 24 May 2020 20:55:54 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by cc-smtpin2.netcologne.de (Postfix) with ESMTP id E121011EF1; Sun, 24 May 2020 20:55:54 +0200 (CEST) Received: from [2001:4dd7:7821:0:f672:738:36da:e63a] (helo=cc-smtpin2.netcologne.de) by localhost with ESMTP (eXpurgate 4.11.6) (envelope-from ) id 5ecac33a-4f6f-7f0000012729-7f000001b0bc-1 for ; Sun, 24 May 2020 20:55:54 +0200 Received: from linux-p51k.fritz.box (2001-4dd7-7821-0-f672-738-36da-e63a.ipv6dyn.netcologne.de [IPv6:2001:4dd7:7821:0:f672:738:36da:e63a]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by cc-smtpin2.netcologne.de (Postfix) with ESMTPSA; Sun, 24 May 2020 20:55:53 +0200 (CEST) To: "fortran@gcc.gnu.org" , gcc-patches Subject: [patch, fortran] Fix memory leaks for finalized types Message-ID: <7829047f-d7f0-c549-ecaa-666547560f41@netcologne.de> Date: Sun, 24 May 2020 20:55:48 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.8.0 MIME-Version: 1.0 Content-Language: de-DE X-Spam-Status: No, score=-10.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Thomas Koenig via Gcc-patches From: Thomas Koenig Reply-To: Thomas Koenig Errors-To: gcc-patches-bounces@gcc.gnu.org Sender: "Gcc-patches" Hello world, this patch fixes a 8/9/10/11 regression, where finalized types were not finalized (and deallocated), which led to memory leaks. Once the offending commit was identified (thanks, Harald!) error analysis was rather straightforward. The central idea was that it is the expression that should not be finalized twice, not the component (which is shared). Less straightforward was writing a meaningful test case; why I could not get ! { dg-final { scan-tree-dump-times "__builtin_free.*dat" 2 "original" } } to work (dejagnu always complained about finding it once) I don't know. Anyway, here is the patch. I have regression-tested it and made sure that the size part of PR 87352 did not go up again through the roof. I have also tested all affected finalize test cases with valgrind and made sure they are still valid and do not leak. Once this is in, it will be interesting to see if any other finalizer bugs are affected. So, OK for trunk and for backporting to all affected branches? Regards Thomas Finalization depends on the expression, not on the component. gcc/fortran/ChangeLog: 2020-05-24 Thomas Koenig PR fortran/94361 * class.c (finalize_component): Use expr->finalized instead of comp->finalized. * gfortran.h (gfc_component): Remove finalized member. (gfc_expr): Add it here instead. gcc/testsuite/ChangeLog: 2020-05-24 Thomas Koenig PR fortran/94361 * gfortran.dg/finalize_28.f90: Adjusted free counts. * gfortran.dg/finalize_33.f90: Likewise. * gfortran.dg/finalize_34.f90: Likewise. * gfortran.dg/finalize_35.f90: New test.. diff --git a/gcc/fortran/class.c b/gcc/fortran/class.c index 9aa3eb7282c..b5a1edae27f 100644 --- a/gcc/fortran/class.c +++ b/gcc/fortran/class.c @@ -911,7 +911,7 @@ finalize_component (gfc_expr *expr, gfc_symbol *derived, gfc_component *comp, if (!comp_is_finalizable (comp)) return; - if (comp->finalized) + if (expr->finalized) return; e = gfc_copy_expr (expr); @@ -1002,6 +1002,7 @@ finalize_component (gfc_expr *expr, gfc_symbol *derived, gfc_component *comp, } else (*code) = cond; + } else if (comp->ts.type == BT_DERIVED && comp->ts.u.derived->f2k_derived @@ -1041,7 +1042,7 @@ finalize_component (gfc_expr *expr, gfc_symbol *derived, gfc_component *comp, sub_ns); gfc_free_expr (e); } - comp->finalized = true; + expr->finalized = 1; } diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h index 7094791e871..5af44847f9b 100644 --- a/gcc/fortran/gfortran.h +++ b/gcc/fortran/gfortran.h @@ -1107,7 +1107,6 @@ typedef struct gfc_component struct gfc_typebound_proc *tb; /* When allocatable/pointer and in a coarray the associated token. */ tree caf_token; - bool finalized; } gfc_component; @@ -2218,6 +2217,9 @@ typedef struct gfc_expr /* Set this if the expression came from expanding an array constructor. */ unsigned int from_constructor : 1; + /* Set this if the expression has already been finalized. */ + unsigned int finalized : 1; + /* If an expression comes from a Hollerith constant or compile-time evaluation of a transfer statement, it may have a prescribed target- memory representation, and these cannot always be backformed from diff --git a/gcc/testsuite/gfortran.dg/finalize_28.f90 b/gcc/testsuite/gfortran.dg/finalize_28.f90 index 597413b2dd3..f0c9665252f 100644 --- a/gcc/testsuite/gfortran.dg/finalize_28.f90 +++ b/gcc/testsuite/gfortran.dg/finalize_28.f90 @@ -21,4 +21,4 @@ contains integer, intent(out) :: edges(:,:) end subroutine coo_dump_edges end module coo_graphs -! { dg-final { scan-tree-dump-times "__builtin_free" 5 "original" } } +! { dg-final { scan-tree-dump-times "__builtin_free" 6 "original" } } diff --git a/gcc/testsuite/gfortran.dg/finalize_33.f90 b/gcc/testsuite/gfortran.dg/finalize_33.f90 index 2205f9eed7f..3857e4485ee 100644 --- a/gcc/testsuite/gfortran.dg/finalize_33.f90 +++ b/gcc/testsuite/gfortran.dg/finalize_33.f90 @@ -116,4 +116,4 @@ contains ! (iii) mci_template end program main_ut ! { dg-final { scan-tree-dump-times "__builtin_malloc" 17 "original" } } -! { dg-final { scan-tree-dump-times "__builtin_free" 19 "original" } } +! { dg-final { scan-tree-dump-times "__builtin_free" 20 "original" } } diff --git a/gcc/testsuite/gfortran.dg/finalize_34.f90 b/gcc/testsuite/gfortran.dg/finalize_34.f90 index e2f02a5c51c..fef7dac6d89 100644 --- a/gcc/testsuite/gfortran.dg/finalize_34.f90 +++ b/gcc/testsuite/gfortran.dg/finalize_34.f90 @@ -22,4 +22,4 @@ program main use testmodule type(evtlist_type), dimension(10) :: a end program main -! { dg-final { scan-tree-dump-times "__builtin_free" 8 "original" } } +! { dg-final { scan-tree-dump-times "__builtin_free" 12 "original" } } diff --git a/gcc/testsuite/gfortran.dg/finalize_35.f90 b/gcc/testsuite/gfortran.dg/finalize_35.f90 new file mode 100644 index 00000000000..66435c43ecc --- /dev/null +++ b/gcc/testsuite/gfortran.dg/finalize_35.f90 @@ -0,0 +1,48 @@ +! { dg-do compile } +! { dg-additional-options "-fdump-tree-original" } +! PR 94361 - this left open some memory leaks. Original test case by +! Antony Lewis. + +module debug + private + + Type TypeWithFinal + contains + FINAL :: finalizer !No leak if this line is commented + end type TypeWithFinal + + Type Tester + real, dimension(:), allocatable :: Dat + Type(TypeWithFinal) :: X + end Type Tester + + Type :: TestType2 + Type(Tester) :: T + end type TestType2 + public Leaker +contains + + subroutine Leaker + type(TestType2) :: Test + + allocate(Test%T%Dat(1000)) + end subroutine Leaker + + subroutine finalizer(this) + Type(TypeWithFinal) :: this + end subroutine finalizer + +end module debug + + +program run + use debug + implicit none + integer i + + do i=1, 1000 + call Leaker() + end do + +end program run +! { dg-final { scan-tree-dump-times "__builtin_free\\ \\(ptr2" 2 "original" } }