From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 85384 invoked by alias); 24 May 2015 15:54:35 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 85363 invoked by uid 89); 24 May 2015 15:54:35 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.6 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,LIKELY_SPAM_BODY,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=no version=3.3.2 X-Spam-User: qpsmtpd, 2 recipients X-HELO: mout.gmx.net Received: from mout.gmx.net (HELO mout.gmx.net) (212.227.15.18) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Sun, 24 May 2015 15:54:24 +0000 Received: from vepi2 ([88.75.104.20]) by mail.gmx.com (mrgmx001) with ESMTPSA (Nemesis) id 0LwaQZ-1ZD4jt1EBb-018GjU; Sun, 24 May 2015 17:54:18 +0200 Date: Sun, 24 May 2015 18:55:00 -0000 From: Andre Vehreschild To: Paul Richard Thomas Cc: "fortran@gcc.gnu.org" , gcc-patches , Damian Rouson Subject: Re: [Patch, fortran] PR66079 - [6 Regression] memory leak with source allocation in internal subprogram Message-ID: <20150524175417.7c4434f5@vepi2> In-Reply-To: References: <20150523195252.22647b46@vepi2> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-UI-Out-Filterresults: notjunk:1; X-SW-Source: 2015-05/txt/msg02220.txt.bz2 Hi Paul, thanks for the answers. I did not want to be nasty, but was just wondering, if I only I had those problems. Given furthermore, that reviews are sparse lately, I thought it might help, when a glitch was removed before a reviewer put his head into it :-) I am curious though, why we loose memory with the second point I mentioned. Do you have an easy example for that? Regards, Andre PS: No hurry needed. I just started with investigation about what is needed for submodules and am already encountering first questions marks with the example in the F2008 standard (section C.8 I think). Just to let you know. On Sat, 23 May 2015 21:40:08 +0200 Paul Richard Thomas wrote: > Dear Andre, > > To answer your fist question - no, it doesn't. I was working with my > laptop, which is over slow when it comes to > updating. I should have realised that since you are working in this > area that there might be a problem. I discovered it when I did an > update on my workstation this afternoon. Of course the fixes are > trivial. > > As to your second question about the chunk that avoids creating an > expr3. Yes, I am aware of the consequences. I am perfectly happy to > insert a TODO that saying that we should effect all the good things > that happen when the standard assignment is used in another block of > code. Alternatively, we could make a temporary variable to which the > expr3 is assigned that is used latter in the assignment to the > allocated variables. For the moment though, repeating evaluation of > expressions seems like a small price to pay for avoiding memory > leakage. > > Cheers > > Paul > > > > On 23 May 2015 at 19:52, Andre Vehreschild wrote: > > Hi Paul, > > > > does this patch apply to current trunk cleanly? I get an issue with the last > > hunk, because all of the prerequisites are gone since r223445. The string > > copy is completely handled by the trans_assignment at the bottom of the if > > (code->expr3) block. Therefore I doubt the patches last hunk is needed any > > longer. > > > > Do you have an example why this hunk is needed? > > > > Index: gcc/fortran/trans-stmt.c > > =================================================================== > > *** gcc/fortran/trans-stmt.c (revision 223233) > > --- gcc/fortran/trans-stmt.c (working copy) > > *************** gfc_trans_allocate (gfc_code * code) > > *** 5200,5206 **** > > } > > /* else expr3 = NULL_TREE set above. */ > > } > > ! else > > { > > /* In all other cases evaluate the expr3 and create a > > temporary. */ > > --- 5200,5207 ---- > > } > > /* else expr3 = NULL_TREE set above. */ > > } > > ! else if (!(code->expr3->ts.type == BT_DERIVED > > ! && code->expr3->ts.u.derived->attr.alloc_comp)) > > { > > /* In all other cases evaluate the expr3 and create a > > temporary. */ > > > > When I get the code right, than all derived-typed source= expressions that > > have an allocatable component will not be prepared for copy to the > > allocated object. This also means, that functions returning an object of > > such a type are called multiple times. Once for each object to allocate. Is > > this really desired? > > > > I am sorry, that I have to say that, but the check2305.diff file does not > > bring the testcase with it. > > > > Regards, > > Andre > > > > > > On Sat, 23 May 2015 14:48:53 +0200 > > Paul Richard Thomas wrote: > > > >> Dear All, > >> > >> This patch started out fixing a single source of memory leak and then > >> went on to fix various other issues that I found upon investigation. > >> > >> The fortran ChangeLog entry is sufficiently descripive that I do not > >> think that there is a need to say more. > >> > >> Bootstrapped and regtested on x86_64/FC21 - OK for trunk? > >> > >> I am rather sure that some of the issues go further back than 6.0. I > >> will investigate what should be fixed for 5.2. > >> > >> Cheers > >> > >> Paul > >> > >> 2015-05-23 Paul Thomas > >> > >> PR fortran/66079 > >> * trans-expr.c (gfc_conv_procedure_call): Allocatable scalar > >> function results must be freed and nullified after use. Create > >> a temporary to hold the result to prevent duplicate calls. > >> * trans-stmt.c (gfc_trans_allocate): Prevent memory leaks by > >> not evaluating expr3 for scalar derived types with allocatable > >> components. Fixed character length allocatable results and > >> dummies need to be dereferenced. Also, if al_len is NULL use > >> memsz for the string copy. > >> > >> 2015-05-23 Paul Thomas > >> > >> PR fortran/66079 > >> * gfortran.dg/allocatable_scalar_13.f90: New test > > > > > > -- > > Andre Vehreschild * Email: vehre ad gmx dot de > > > -- Andre Vehreschild * Email: vehre ad gmx dot de