From patchwork Sat May 16 16:43:14 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mikael Morin X-Patchwork-Id: 473058 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org 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 B0B5714027F for ; Sun, 17 May 2015 02:43:49 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=a4Jv1J8w; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:subject:content-type; q= dns; s=default; b=uU1PbVGJ9bEbxRBYdnssb79qDWcH/G2vfRqz2oS5nq8pLk 1ch2BrdnjiqXT5bQ4YIofYwCLkWSpSC/gFld4UF3c3ddJlhbkeTFPeHa+FJcp2tH tVzzE666uUEzsExYllVbqQdvrPbiE1fyo0TBGcS0qZPVDvqAWzhcYKJ9vZwP4= 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 :message-id:date:from:mime-version:to:subject:content-type; s= default; bh=Wlj/hiKfR611k5Zoqy30RjwQcE4=; b=a4Jv1J8w1wtMcK4MH8FL IBhd+6XVEKVVvxDEQWBmcH5ev6tAsmebWG/Nxe6WmtEayxYH4nUoBRHmPHiTUmX2 vF99vSnSzMEgPkyvpkS8e54IopSG8gQ3/JWIkm5NXZKYZ2jh87cTdFXD1onxMZWm MJg7Oj9Stc1E9szIkA59Zpc= Received: (qmail 61245 invoked by alias); 16 May 2015 16:43:35 -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 61223 invoked by uid 89); 16 May 2015 16:43:35 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_NONE, SPF_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-Spam-User: qpsmtpd, 2 recipients X-HELO: smtp26.services.sfr.fr Received: from smtp26.services.sfr.fr (HELO smtp26.services.sfr.fr) (93.17.128.163) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Sat, 16 May 2015 16:43:33 +0000 Received: from filter.sfr.fr (localhost [86.72.15.254]) by msfrf2604.sfr.fr (SMTP Server) with ESMTP id 76FB51C000C1A; Sat, 16 May 2015 18:43:30 +0200 (CEST) Authentication-Results: sfrmc.priv.atos.fr; dkim=none (no signature); dkim-adsp=none (no policy) header.from=mikael.morin@sfr.fr Received: from tolstoi.localhost (254.15.72.86.rev.sfr.net [86.72.15.254]) (using TLSv1.2 with cipher DHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) by msfrf2604.sfr.fr (SMTP Server) with ESMTP id E321F1C000409; Sat, 16 May 2015 18:43:29 +0200 (CEST) X-SFR-UUID: 20150516164329930.E321F1C000409@msfrf2604.sfr.fr Message-ID: <555773A2.9070400@sfr.fr> Date: Sat, 16 May 2015 18:43:14 +0200 From: Mikael Morin User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-Version: 1.0 To: gcc-patches , gfortran Subject: [Patch, fortran] PR61831 side-effect deallocation of variable components X-IsSubscribed: yes Hello, this is about PR61831 where in code like: type :: string_t character(LEN=1), dimension(:), allocatable :: chars end type string_t type(string_t) :: prt_in (...) tmp = new_prt_spec ([prt_in]) the deallocation of the argument's allocatable components after the procedure call (to new_prt_spec) has the side effect of freeing prt_in's allocatable components, as the array constructor temporary for [prt_in] is a shallow copy of prt_in. This bug is a regression caused by the Dominique's PR41936 memory leak fix, itself based on a patch originally from me. The attached patch is basically a revert of that fix. It avoids the problem by not deallocating allocatable components in the problematic case, at the price of a (possible) memory leak. A new function is introduced telling whether there is aliasing, so that we don't regress on PR41936's memory leak when there is no aliasing, and we don't free components when there is aliasing. The possible remaining memory leak case is the case of a "mixed" array constructor with some parts aliasing variables, and some non-aliasing parts. The patch takes also the opportunity to reassemble the scattered procedure argument deallocation code into a single place. The test needs pr65792's fix (thanks Paul), so for the 4.9 branch I propose commenting the parts that depend on PR65792 in the test. Regression tested on x86_64-linux. OK for 6/5/4.9 ? Mikael 2015-05-16 Mikael Morin PR fortran/61831 * trans-array.c (gfc_conv_array_parameter): Remove allocatable component deallocation code generation. * trans-expr.c (gfc_conv_expr_reference): Ditto. (expr_may_alias_variables): New function. (gfc_conv_procedure_call): Use it to decide whether generate allocatable component deallocation code. (gfc_trans_subarray_assign): Set deep copy flag. 2015-05-16 Mikael Morin PR fortran/61831 * gfortran.dg/derived_constructor_components_6.f90: New. diff --git a/gcc/fortran/trans-array.c b/gcc/fortran/trans-array.c index 8267f6a..210b2ec 100644 --- a/gcc/fortran/trans-array.c +++ b/gcc/fortran/trans-array.c @@ -7305,19 +7305,6 @@ gfc_conv_array_parameter (gfc_se * se, gfc_expr * expr, bool g77, expr, size); } - /* Deallocate the allocatable components of structures that are - not variable. */ - if ((expr->ts.type == BT_DERIVED || expr->ts.type == BT_CLASS) - && expr->ts.u.derived->attr.alloc_comp - && expr->expr_type != EXPR_VARIABLE) - { - tmp = build_fold_indirect_ref_loc (input_location, se->expr); - tmp = gfc_deallocate_alloc_comp (expr->ts.u.derived, tmp, expr->rank); - - /* The components shall be deallocated before their containing entity. */ - gfc_prepend_expr_to_block (&se->post, tmp); - } - if (g77 || (fsym && fsym->attr.contiguous && !gfc_is_simply_contiguous (expr, false))) { diff --git a/gcc/fortran/trans-expr.c b/gcc/fortran/trans-expr.c index 9be8a42..e375453 100644 --- a/gcc/fortran/trans-expr.c +++ b/gcc/fortran/trans-expr.c @@ -4536,6 +4536,36 @@ conv_arglist_function (gfc_se *se, gfc_expr *expr, const char *name) } +/* Temporary arrays generated to represent array constructors are made + using simple copies, so that their elements may alias some variable + they were copied from. + This function tells whether the expression given as input may alias + some other variable, under the assumption that only variables and array + constructor may alias (in particular structure constructors don't alias), + and array constructor elements alias iff they are copied from a variable. + This function is used to decide whether freeing an expression's allocatable + components is safe or should be avoided. */ + +static bool +expr_may_alias_variables (gfc_expr *e) +{ + gfc_constructor *c; + + if (e->expr_type == EXPR_VARIABLE) + return true; + else if (e->expr_type != EXPR_ARRAY) + return false; + + for (c = gfc_constructor_first (e->value.constructor); + c; c = gfc_constructor_next (c)) + if (c->expr + && expr_may_alias_variables (c->expr)) + return true; + + return false; +} + + /* Generate code for a procedure call. Note can return se->post != NULL. If se->direct_byref is set then se->expr contains the return parameter. Return nonzero, if the call has alternate specifiers. @@ -5328,7 +5358,7 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, if (e && (e->ts.type == BT_DERIVED || e->ts.type == BT_CLASS) && e->ts.u.derived->attr.alloc_comp && !(e->symtree && e->symtree->n.sym->attr.pointer) - && (e->expr_type != EXPR_VARIABLE && !e->rank)) + && !expr_may_alias_variables (e)) { int parm_rank; tmp = build_fold_indirect_ref_loc (input_location, @@ -6642,7 +6672,7 @@ gfc_trans_subarray_assign (tree dest, gfc_component * cm, gfc_expr * expr) gfc_conv_expr (&rse, expr); - tmp = gfc_trans_scalar_assign (&lse, &rse, cm->ts, true, false, true); + tmp = gfc_trans_scalar_assign (&lse, &rse, cm->ts, true, true, true); gfc_add_expr_to_block (&body, tmp); gcc_assert (rse.ss == gfc_ss_terminator); @@ -7513,20 +7543,6 @@ gfc_conv_expr_reference (gfc_se * se, gfc_expr * expr) /* Take the address of that value. */ se->expr = gfc_build_addr_expr (NULL_TREE, var); - if (expr->ts.type == BT_DERIVED && expr->rank - && !gfc_is_finalizable (expr->ts.u.derived, NULL) - && expr->ts.u.derived->attr.alloc_comp - && expr->expr_type != EXPR_VARIABLE) - { - tree tmp; - - tmp = build_fold_indirect_ref_loc (input_location, se->expr); - tmp = gfc_deallocate_alloc_comp (expr->ts.u.derived, tmp, expr->rank); - - /* The components shall be deallocated before - their containing entity. */ - gfc_prepend_expr_to_block (&se->post, tmp); - } }