This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [Patch, Fortran, PR58586, v1] ICE with derived type with allocatable component passed by value
- From: Mikael Morin <mikael dot morin at sfr dot fr>
- To: Andre Vehreschild <vehre at gmx dot de>, GCC-Patches-ML <gcc-patches at gcc dot gnu dot org>, GCC-Fortran-ML <fortran at gcc dot gnu dot org>
- Date: Sun, 19 Apr 2015 12:01:23 +0200
- Subject: Re: [Patch, Fortran, PR58586, v1] ICE with derived type with allocatable component passed by value
- Authentication-results: sourceware.org; auth=none
- Authentication-results: sfrmc.priv.atos.fr; dkim=none (no signature); dkim-adsp=none (no policy) header dot from=mikael dot morin at sfr dot fr
- References: <20150415200304 dot 7101aca9 at gmx dot de>
Hello,
Le 15/04/2015 20:03, Andre Vehreschild a écrit :
> by accident I patched this pr. For short, when a structure constructor for a
> structure with an allocatable component or a function returning a type with an
> allocatable component is passed as actual argument to a function, then gfortran
> ICEs. This patch fixes, both the ICE and a segfault at runtime.
>
> I was pointed to the patch in comment #44 of pr61831 which seemingly fixes the
> #3 comment of pr58586, too, but causes a memory leak. I therefore like to point
> out, that adding the a->expr.expr_type != EXPR_STRUCTURE of Mikael's patch in
> pr61831 should not be added to trans-expr.c::gfc_conv_procedure_call (), when
> this patch for 58586 is applied.
Note that I plan to submit the pr61831 patch soon, and that the comment
#44 patch doesn't have the a->expr.expr_type != EXPR_STRUCTURE (in
opposition to precedent patches).
I hope that means the patches are compatible. ;-)
>
> diff --git a/gcc/fortran/trans-expr.c b/gcc/fortran/trans-expr.c
> index 9e6432f..80dfed1 100644
> --- a/gcc/fortran/trans-expr.c
> +++ b/gcc/fortran/trans-expr.c
> @@ -5344,8 +5344,19 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym,
> && (e->expr_type != EXPR_VARIABLE && !e->rank))
> {
> int parm_rank;
> - tmp = build_fold_indirect_ref_loc (input_location,
> - parmse.expr);
> + /* It is known the e returns a structure type with at least one
> + allocatable component. When e is a function, ensure that the
> + function is called once only by using a temporary variable. */
> + if (e->expr_type == EXPR_FUNCTION)
> + parmse.expr = gfc_evaluate_now_loc (input_location,
> + parmse.expr, &se->pre);
You need not limit this to functions only.
I think you can even do this without condition.
> +
> + if (POINTER_TYPE_P (TREE_TYPE (parmse.expr)))
This distinguishes arguments with/without value attribute, right?
I think it's better to use the frontend information here (fsym->attr.value).
> + tmp = build_fold_indirect_ref_loc (input_location,
> + parmse.expr);
> + else
> + tmp = parmse.expr;
> +
> parm_rank = e->rank;
> switch (parm_kind)
> {
Otherwise, this looks good. Can you post an updated patch taking the
above comments into account?
Ah, and don't forget to provide a ChangeLog entry with it.
Mikael