[Patch, fortran] PR66079 - [6 Regression] memory leak with source allocation in internal subprogram

[Patch, fortran] PR66079 - [6 Regression] memory leak with source allocation in internal subprogram

[Paul Richard Thomas]

[-- Attachment #1: Type: text/plain, Size: 1126 bytes --]

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  <pault@gcc.gnu.org>

    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  <pault@gcc.gnu.org>

    PR fortran/66079
    * gfortran.dg/allocatable_scalar_13.f90: New test

[-- Attachment #2: check2305.diff --]
[-- Type: text/plain, Size: 2781 bytes --]

Index: gcc/fortran/trans-expr.c
===================================================================
*** gcc/fortran/trans-expr.c	(revision 223234)
--- gcc/fortran/trans-expr.c	(working copy)
*************** gfc_conv_procedure_call (gfc_se * se, gf
*** 5877,5882 ****
--- 5877,5896 ----
    fntype = TREE_TYPE (TREE_TYPE (se->expr));
    se->expr = build_call_vec (TREE_TYPE (fntype), se->expr, arglist);
  
+   /* Allocatable scalar function results must be freed and nullified
+      after use. This necessitates the creation of a temporary to
+      hold the result to prevent duplicate calls.  */
+   if (!byref && sym->ts.type != BT_CHARACTER
+       && sym->attr.allocatable && !sym->attr.dimension)
+     {
+       tmp = gfc_create_var (TREE_TYPE (se->expr), NULL);
+       gfc_add_modify (&se->pre, tmp, se->expr);
+       se->expr = tmp;
+       tmp = gfc_call_free (tmp);
+       gfc_add_expr_to_block (&post, tmp);
+       gfc_add_modify (&post, se->expr, build_int_cst (TREE_TYPE (se->expr), 0));
+     }
+ 
    /* If we have a pointer function, but we don't want a pointer, e.g.
       something like
          x = f()
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.  */
*************** gfc_trans_allocate (gfc_code * code)
*** 5626,5631 ****
--- 5627,5633 ----
  			    fold_convert (TREE_TYPE (al_len),
  					  integer_zero_node));
  	}
+ 
        if (code->expr3 && !code->expr3->mold)
  	{
  	  /* Initialization via SOURCE block
*************** gfc_trans_allocate (gfc_code * code)
*** 5650,5655 ****
--- 5652,5669 ----
  			se.expr :
  			build_fold_indirect_ref_loc (input_location,
  						     se.expr);
+ 
+ 	      /* Fixed length allocatable results and dummies need further
+ 		 dereferencing.  */
+ 	      if (!expr->ts.deferred
+ 		  && TREE_CODE (se.expr) == PARM_DECL)
+ 		tmp = build_fold_indirect_ref_loc (input_location, tmp);
+ 
+ 	      /* Fixed length allocations need this because al_len is
+ 		 never set.  */
+ 	      if (al_len == NULL_TREE)
+ 		al_len = memsz;
+ 
  	      gfc_trans_string_copy (&block, al_len, tmp,
  				     code->expr3->ts.kind,
  				     expr3_len, expr3,

Re: [Patch, fortran] PR66079 - [6 Regression] memory leak with source allocation in internal subprogram

[Andre Vehreschild]

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 <paul.richard.thomas@gmail.com> 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  <pault@gcc.gnu.org>
> 
>     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  <pault@gcc.gnu.org>
> 
>     PR fortran/66079
>     * gfortran.dg/allocatable_scalar_13.f90: New test


-- 
Andre Vehreschild * Email: vehre ad gmx dot de 

Re: [Patch, fortran] PR66079 - [6 Regression] memory leak with source allocation in internal subprogram

[Paul Richard Thomas]

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 <vehre@gmx.de> 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 <paul.richard.thomas@gmail.com> 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  <pault@gcc.gnu.org>
>>
>>     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  <pault@gcc.gnu.org>
>>
>>     PR fortran/66079
>>     * gfortran.dg/allocatable_scalar_13.f90: New test
>
>
> --
> Andre Vehreschild * Email: vehre ad gmx dot de



-- 
Outside of a dog, a book is a man's best friend. Inside of a dog it's
too dark to read.

Groucho Marx

Re: [Patch, fortran] PR66079 - [6 Regression] memory leak with source allocation in internal subprogram

[Andre Vehreschild]

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 <paul.richard.thomas@gmail.com> 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 <vehre@gmx.de> 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 <paul.richard.thomas@gmail.com> 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  <pault@gcc.gnu.org>
> >>
> >>     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  <pault@gcc.gnu.org>
> >>
> >>     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 

Re: [Patch, fortran] PR66079 - [6 Regression] memory leak with source allocation in internal subprogram

[Paul Richard Thomas]

Dear Andre,

I'll put both points right. Thanks for pointing them out.

Cheers

Paul

On 23 May 2015 at 19:52, Andre Vehreschild <vehre@gmx.de> 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 <paul.richard.thomas@gmail.com> 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  <pault@gcc.gnu.org>
>>
>>     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  <pault@gcc.gnu.org>
>>
>>     PR fortran/66079
>>     * gfortran.dg/allocatable_scalar_13.f90: New test
>
>
> --
> Andre Vehreschild * Email: vehre ad gmx dot de



-- 
Outside of a dog, a book is a man's best friend. Inside of a dog it's
too dark to read.

Groucho Marx

Re: [Patch, fortran] PR66079 - [6 Regression] memory leak with source allocation in internal subprogram

[Paul Richard Thomas]

[-- Attachment #1: Type: text/plain, Size: 4663 bytes --]

Dear All,

Lets see if I can get it right this time :-)

Note that I have changed the name of the temporary variable in
trans_allocate from 'atmp' to 'expr3' so that it is not confused with
array temporaries. I am not suree how much of the testcase is
pertinent after the reform of the evaluation of expr3 performed by
Andre. However, there were still memory leaks that are fixed by the
attached patch.

Bootstrapped and regtested on a current trunk - OK for trunk?

Paul

2015-05-23  Paul Thomas  <pault@gcc.gnu.org>

    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): Rename temporary variable
    as 'expr3'. Deallocate allocatable components of non-variable
    expr3s.

2015-05-23  Paul Thomas  <pault@gcc.gnu.org>

    PR fortran/66079
    * gfortran.dg/allocatable_scalar_13.f90: New test


On 24 May 2015 at 09:51, Paul Richard Thomas
<paul.richard.thomas@gmail.com> wrote:
> Dear Andre,
>
> I'll put both points right. Thanks for pointing them out.
>
> Cheers
>
> Paul
>
> On 23 May 2015 at 19:52, Andre Vehreschild <vehre@gmx.de> 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 <paul.richard.thomas@gmail.com> 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  <pault@gcc.gnu.org>
>>>
>>>     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  <pault@gcc.gnu.org>
>>>
>>>     PR fortran/66079
>>>     * gfortran.dg/allocatable_scalar_13.f90: New test
>>
>>
>> --
>> Andre Vehreschild * Email: vehre ad gmx dot de
>
>
>
> --
> Outside of a dog, a book is a man's best friend. Inside of a dog it's
> too dark to read.
>
> Groucho Marx



-- 
Outside of a dog, a book is a man's best friend. Inside of a dog it's
too dark to read.

Groucho Marx

[-- Attachment #2: submit.diff --]
[-- Type: text/plain, Size: 5355 bytes --]

Index: gcc/fortran/trans-expr.c
===================================================================
*** gcc/fortran/trans-expr.c	(revision 223641)
--- gcc/fortran/trans-expr.c	(working copy)
*************** gfc_conv_procedure_call (gfc_se * se, gf
*** 5877,5882 ****
--- 5877,5896 ----
    fntype = TREE_TYPE (TREE_TYPE (se->expr));
    se->expr = build_call_vec (TREE_TYPE (fntype), se->expr, arglist);
  
+   /* Allocatable scalar function results must be freed and nullified
+      after use. This necessitates the creation of a temporary to
+      hold the result to prevent duplicate calls.  */
+   if (!byref && sym->ts.type != BT_CHARACTER
+       && sym->attr.allocatable && !sym->attr.dimension)
+     {
+       tmp = gfc_create_var (TREE_TYPE (se->expr), NULL);
+       gfc_add_modify (&se->pre, tmp, se->expr);
+       se->expr = tmp;
+       tmp = gfc_call_free (tmp);
+       gfc_add_expr_to_block (&post, tmp);
+       gfc_add_modify (&post, se->expr, build_int_cst (TREE_TYPE (se->expr), 0));
+     }
+ 
    /* If we have a pointer function, but we don't want a pointer, e.g.
       something like
          x = f()
Index: gcc/fortran/trans-stmt.c
===================================================================
*** gcc/fortran/trans-stmt.c	(revision 223641)
--- gcc/fortran/trans-stmt.c	(working copy)
*************** gfc_trans_allocate (gfc_code * code)
*** 5214,5219 ****
--- 5214,5220 ----
  				     false, false);
  	  gfc_add_block_to_block (&block, &se.pre);
  	  gfc_add_block_to_block (&post, &se.post);
+ 
  	  /* Prevent aliasing, i.e., se.expr may be already a
  		 variable declaration.  */
  	  if (!VAR_P (se.expr))
*************** gfc_trans_allocate (gfc_code * code)
*** 5223,5230 ****
  						 se.expr);
  	      /* We need a regular (non-UID) symbol here, therefore give a
  		 prefix.  */
! 	      var = gfc_create_var (TREE_TYPE (tmp), "atmp");
  	      gfc_add_modify_loc (input_location, &block, var, tmp);
  	      tmp = var;
  	    }
  	  else
--- 5224,5243 ----
  						 se.expr);
  	      /* We need a regular (non-UID) symbol here, therefore give a
  		 prefix.  */
! 	      var = gfc_create_var (TREE_TYPE (tmp), "expr3");
  	      gfc_add_modify_loc (input_location, &block, var, tmp);
+ 
+ 	      /* Deallocate any allocatable components after all the allocations
+ 		 and assignments of expr3 have been completed.  */
+ 	      if (code->expr3->ts.type == BT_DERIVED
+ 		  && code->expr3->rank == 0
+ 		  && code->expr3->ts.u.derived->attr.alloc_comp)
+ 		{
+ 		  tmp = gfc_deallocate_alloc_comp (code->expr3->ts.u.derived,
+ 						   var, 0);
+ 		  gfc_add_expr_to_block (&post, tmp);
+ 		}
+ 
  	      tmp = var;
  	    }
  	  else
Index: gcc/testsuite/gfortran.dg/allocatable_scalar_13.f90
===================================================================
*** gcc/testsuite/gfortran.dg/allocatable_scalar_13.f90	(revision 0)
--- gcc/testsuite/gfortran.dg/allocatable_scalar_13.f90	(working copy)
***************
*** 0 ****
--- 1,70 ----
+ ! { dg-do run }
+ ! { dg-options "-fdump-tree-original" }
+ !
+ ! Test the fix for PR66079. The original problem was with the first
+ ! allocate statement. The rest of this testcase fixes problems found
+ ! whilst working on it!
+ !
+ ! Reported by Damian Rouson  <damian@sourceryinstitute.org>
+ !
+   type subdata
+     integer, allocatable :: b
+   endtype
+ !  block
+     call newRealVec
+ !  end block
+ contains
+   subroutine newRealVec
+     type(subdata), allocatable :: d, e, f
+     character(:), allocatable :: g, h, i
+     character(8), allocatable :: j
+     allocate(d,source=subdata(1)) ! memory was lost, now OK
+     allocate(e,source=d) ! OK
+     allocate(f,source=create (99)) ! memory was lost, now OK
+     if (d%b .ne. 1) call abort
+     if (e%b .ne. 1) call abort
+     if (f%b .ne. 99) call abort
+     allocate (g, source = greeting1("good day"))
+     if (g .ne. "good day") call abort
+     allocate (h, source = greeting2("hello"))
+     if (h .ne. "hello") call abort
+     allocate (i, source = greeting3("hiya!"))
+     if (i .ne. "hiya!") call abort
+     call greeting4 (j, "Goodbye ") ! Test that dummy arguments are OK
+     if (j .ne. "Goodbye ") call abort
+   end subroutine
+ 
+   function create (arg) result(res)
+     integer :: arg
+     type(subdata), allocatable :: res, res1
+     allocate(res, res1, source = subdata(arg))
+   end function
+ 
+   function greeting1 (arg) result(res) ! memory was lost, now OK
+     character(*) :: arg
+     Character(:), allocatable :: res
+     allocate(res, source = arg)
+   end function
+ 
+   function greeting2 (arg) result(res)
+     character(5) :: arg
+     Character(:), allocatable :: res
+     allocate(res, source = arg)
+   end function
+ 
+   function greeting3 (arg) result(res)
+     character(5) :: arg
+     Character(5), allocatable :: res, res1
+     allocate(res, res1, source = arg) ! Caused an ICE
+     if (res1 .ne. res) call abort
+   end function
+ 
+   subroutine greeting4 (res, arg)
+     character(8), intent(in) :: arg
+     Character(8), allocatable, intent(out) :: res
+     allocate(res, source = arg) ! Caused an ICE
+   end subroutine
+ end
+ ! { dg-final { scan-tree-dump-times "builtin_malloc" 20 "original" } }
+ ! { dg-final { scan-tree-dump-times "builtin_free" 21 "original" } }
+ ! { dg-final { cleanup-tree-dump "original" } }

Re: [Patch, fortran] PR66079 - [6 Regression] memory leak with source allocation in internal subprogram

[Andre Vehreschild]

[-- Attachment #1: Type: text/plain, Size: 7054 bytes --]

Hi Paul,

I am not quite happy with the naming of the temporary variable. When I
initially set the prefix to "atmp" this was because the variable would be an
array most of the time and because of the number appended to it should be unique
anyway. However I would like to point out that disclosing an internal
implementation detail of the compiler to a future developer looking at the
pseudo-code dump will not help (I mean "expr3", here). I would rather use
"source" as the prefix now that I think of it with some distance to the
original naming. What do you think? 

Now that the deallocate for source's components is in the patch, I understand
why initially the source= preevaluation for derived types with allocatable
components was disallowed. Thanks for clarifying that. 

I wonder though, if we can't do better...

Please have a look at the attached patch. It not only renames the temporary
variable from "expr3" to "source" (couldn't help, but do it. Please don't be
angry :-)), but also adds move semantics to source= expressions for the last
object to allocate. I.e., when a scalar source= expression with allocatable
components is detected, then its content is "moved" (memcpy'ed) to the last
object to allocate instead of being assigned. All former objects to allocate
are of course handled like before, i.e., components are allocated and the
contents of the source= expression is copied using the assign. But when a move
could be done the alloc/dealloc of the components is skipped. With this I hope
to safe a lot of mallocs and frees, which are not that cheap. In the most common
case where only one object is allocated, there now is only one alloc for the
components to get expr3 up and one for the object to allocate. We safe the
allocate of the allocatable components in the object to allocate and the free
of the source= components. I hope I could make clear what I desire? If not
maybe a look into the patch might help. What do you think?

The patch of course is only a quick implementation of the idea. Please
comment, everyone!

Regards,
	Andre


On Mon, 25 May 2015 09:30:34 +0200
Paul Richard Thomas <paul.richard.thomas@gmail.com> wrote:

> Dear All,
> 
> Lets see if I can get it right this time :-)
> 
> Note that I have changed the name of the temporary variable in
> trans_allocate from 'atmp' to 'expr3' so that it is not confused with
> array temporaries. I am not suree how much of the testcase is
> pertinent after the reform of the evaluation of expr3 performed by
> Andre. However, there were still memory leaks that are fixed by the
> attached patch.
> 
> Bootstrapped and regtested on a current trunk - OK for trunk?
> 
> Paul
> 
> 2015-05-23  Paul Thomas  <pault@gcc.gnu.org>
> 
>     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): Rename temporary variable
>     as 'expr3'. Deallocate allocatable components of non-variable
>     expr3s.
> 
> 2015-05-23  Paul Thomas  <pault@gcc.gnu.org>
> 
>     PR fortran/66079
>     * gfortran.dg/allocatable_scalar_13.f90: New test
> 
> 
> On 24 May 2015 at 09:51, Paul Richard Thomas
> <paul.richard.thomas@gmail.com> wrote:
> > Dear Andre,
> >
> > I'll put both points right. Thanks for pointing them out.
> >
> > Cheers
> >
> > Paul
> >
> > On 23 May 2015 at 19:52, Andre Vehreschild <vehre@gmx.de> 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 <paul.richard.thomas@gmail.com> 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  <pault@gcc.gnu.org>
> >>>
> >>>     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  <pault@gcc.gnu.org>
> >>>
> >>>     PR fortran/66079
> >>>     * gfortran.dg/allocatable_scalar_13.f90: New test
> >>
> >>
> >> --
> >> Andre Vehreschild * Email: vehre ad gmx dot de
> >
> >
> >
> > --
> > Outside of a dog, a book is a man's best friend. Inside of a dog it's
> > too dark to read.
> >
> > Groucho Marx
> 
> 
> 


-- 
Andre Vehreschild * Email: vehre ad gmx dot de 

[-- Attachment #2: pr66079.patch --]
[-- Type: text/x-patch, Size: 4081 bytes --]

diff --git a/gcc/fortran/trans-stmt.c b/gcc/fortran/trans-stmt.c
index 2eb297a..2ef9f15 100644
--- a/gcc/fortran/trans-stmt.c
+++ b/gcc/fortran/trans-stmt.c
@@ -5104,11 +5104,12 @@ gfc_trans_allocate (gfc_code * code)
      element size, i.e. _vptr%size, is stored in expr3_esize.  Any of
      the trees may be the NULL_TREE indicating that this is not
      available for expr3's type.  */
-  tree expr3, expr3_vptr, expr3_len, expr3_esize;
+  tree expr3, expr3_vptr, expr3_len, expr3_esize, expr3_tmp_var;
   stmtblock_t block;
   stmtblock_t post;
   tree nelems;
   bool upoly_expr, tmp_expr3_len_flag = false, al_len_needs_set;
+  bool needs_comp_dealloc = false;
   gfc_symtree *newsym = NULL;
 
   if (!code->ext.alloc.list)
@@ -5117,6 +5118,7 @@ gfc_trans_allocate (gfc_code * code)
   stat = tmp = memsz = al_vptr = al_len = NULL_TREE;
   expr3 = expr3_vptr = expr3_len = expr3_esize = NULL_TREE;
   label_errmsg = label_finish = errmsg = errlen = NULL_TREE;
+  expr3_tmp_var = NULL_TREE;
 
   gfc_init_block (&block);
   gfc_init_block (&post);
@@ -5219,26 +5221,21 @@ gfc_trans_allocate (gfc_code * code)
 		 variable declaration.  */
 	  if (!VAR_P (se.expr))
 	    {
-	      tree var;
 	      tmp = build_fold_indirect_ref_loc (input_location,
 						 se.expr);
 	      /* We need a regular (non-UID) symbol here, therefore give a
 		 prefix.  */
-	      var = gfc_create_var (TREE_TYPE (tmp), "expr3");
-	      gfc_add_modify_loc (input_location, &block, var, tmp);
+	      expr3_tmp_var = gfc_create_var (TREE_TYPE (tmp), "source");
+	      gfc_add_modify_loc (input_location, &block, expr3_tmp_var, tmp);
 
-	      /* Deallocate any allocatable components after all the allocations
-		 and assignments of expr3 have been completed.  */
-	      if (code->expr3->ts.type == BT_DERIVED
+	      /* Check whether deallocate any allocatable components after
+		 all the allocations and assignments of expr3 have been
+		 completed are needed, in case we can't do move semantics.  */
+	      needs_comp_dealloc = code->expr3->ts.type == BT_DERIVED
 		  && code->expr3->rank == 0
-		  && code->expr3->ts.u.derived->attr.alloc_comp)
-		{
-		  tmp = gfc_deallocate_alloc_comp (code->expr3->ts.u.derived,
-						   var, 0);
-		  gfc_add_expr_to_block (&post, tmp);
-		}
+		  && code->expr3->ts.u.derived->attr.alloc_comp;
 
-	      tmp = var;
+	      tmp = expr3_tmp_var;
 	    }
 	  else
 	    tmp = se.expr;
@@ -5840,15 +5837,40 @@ gfc_trans_allocate (gfc_code * code)
 	    }
 	  else
 	    {
-	      /* Switch off automatic reallocation since we have just
+	      if (al->next == NULL && needs_comp_dealloc && expr->rank == 0)
+		{
+		  /* Move the contents of expr3 into the last object to
+		     allocate instead of allocating its allocatable components
+		     anew.  */
+		  tmp = gfc_build_memcpy_call (se.expr, expr3,
+					       TYPE_SIZE_UNIT (
+						 TREE_TYPE (expr3)));
+		  /* Because the allocatable components allocated for source=
+		     are reused, do not add code to free them.  */
+		  needs_comp_dealloc = false;
+		}
+	      else
+		{
+		  /* Switch off automatic reallocation since we have just
 		 done the ALLOCATE.  */
-	      int realloc_lhs = flag_realloc_lhs;
-	      flag_realloc_lhs = 0;
-	      tmp = gfc_trans_assignment (gfc_expr_to_initialize (expr),
-					  e3rhs, false, false);
-	      flag_realloc_lhs = realloc_lhs;
+		  int realloc_lhs = flag_realloc_lhs;
+		  flag_realloc_lhs = 0;
+		  tmp = gfc_trans_assignment (gfc_expr_to_initialize (expr),
+					      e3rhs, false, false);
+		  flag_realloc_lhs = realloc_lhs;
+		}
 	    }
 	  gfc_add_expr_to_block (&block, tmp);
+
+	  if (al->next == NULL && needs_comp_dealloc)
+	    {
+	      /* Allocatable components of expr3 are not "moved" into the
+		 last object to allocate, but have to be freeed to prevent
+		 memory loss.  */
+	      tmp = gfc_deallocate_alloc_comp (code->expr3->ts.u.derived,
+					       expr3_tmp_var, 0);
+	      gfc_add_expr_to_block (&post, tmp);
+	    }
 	}
      else if (code->expr3 && code->expr3->mold
 	    && code->expr3->ts.type == BT_CLASS)

Re: [Patch, fortran] PR66079 - [6 Regression] memory leak with source allocation in internal subprogram

[Mikael Morin]

hello Andre,

Le 25/05/2015 12:24, Andre Vehreschild a écrit :
> Hi Paul,
> 
> I am not quite happy with the naming of the temporary variable. When I
> initially set the prefix to "atmp" this was because the variable would be an
> array most of the time and because of the number appended to it should be unique
> anyway. However I would like to point out that disclosing an internal
> implementation detail of the compiler to a future developer looking at the
> pseudo-code dump will not help (I mean "expr3", here). I would rather use
> "source" as the prefix now that I think of it with some distance to the
> original naming. What do you think? 
> 
> Now that the deallocate for source's components is in the patch, I understand
> why initially the source= preevaluation for derived types with allocatable
> components was disallowed. Thanks for clarifying that. 
> 
> I wonder though, if we can't do better...
> 
> Please have a look at the attached patch. It not only renames the temporary
> variable from "expr3" to "source" (couldn't help, but do it. Please don't be
> angry :-)), but also adds move semantics to source= expressions for the last
> object to allocate. I.e., when a scalar source= expression with allocatable
> components is detected, then its content is "moved" (memcpy'ed) to the last
> object to allocate instead of being assigned. All former objects to allocate
> are of course handled like before, i.e., components are allocated and the
> contents of the source= expression is copied using the assign. But when a move
> could be done the alloc/dealloc of the components is skipped. With this I hope
> to safe a lot of mallocs and frees, which are not that cheap. In the most common
> case where only one object is allocated, there now is only one alloc for the
> components to get expr3 up and one for the object to allocate. We safe the
> allocate of the allocatable components in the object to allocate and the free
> of the source= components. I hope I could make clear what I desire? If not
> maybe a look into the patch might help. What do you think?
> 
> The patch of course is only a quick implementation of the idea. Please
> comment, everyone!
> 
This looks good to me as well with...

*************** gfc_trans_allocate (gfc_code * code)
*** 5840,5845 ****
--- 5837,5856 ----
  	    }
  	  else
  	    {
+ 	      if (al->next == NULL && needs_comp_dealloc && expr->rank == 0)
+ 		{

...please use "else if" to avoid changing indentation unnecessarily.
Then OK if it passes the testsuite.
Give Paul a couple of days to comment, before committing.

Mikael

Re: [Patch, fortran] PR66079 - [6 Regression] memory leak with source allocation in internal subprogram

[Paul Richard Thomas]

Dear Andre,

I am perfectly happy with renaming the rename to "source". I was
attempting to distinguish "atmp" coming from trans-array.c from this
temporary; just as an aid to any possible future debugging.

The rework of the patch looks fine to me as well. Do you want to
commit or should I do so?

Cheers

Paul

On 25 May 2015 at 12:24, Andre Vehreschild <vehre@gmx.de> wrote:
> Hi Paul,
>
> I am not quite happy with the naming of the temporary variable. When I
> initially set the prefix to "atmp" this was because the variable would be an
> array most of the time and because of the number appended to it should be unique
> anyway. However I would like to point out that disclosing an internal
> implementation detail of the compiler to a future developer looking at the
> pseudo-code dump will not help (I mean "expr3", here). I would rather use
> "source" as the prefix now that I think of it with some distance to the
> original naming. What do you think?
>
> Now that the deallocate for source's components is in the patch, I understand
> why initially the source= preevaluation for derived types with allocatable
> components was disallowed. Thanks for clarifying that.
>
> I wonder though, if we can't do better...
>
> Please have a look at the attached patch. It not only renames the temporary
> variable from "expr3" to "source" (couldn't help, but do it. Please don't be
> angry :-)), but also adds move semantics to source= expressions for the last
> object to allocate. I.e., when a scalar source= expression with allocatable
> components is detected, then its content is "moved" (memcpy'ed) to the last
> object to allocate instead of being assigned. All former objects to allocate
> are of course handled like before, i.e., components are allocated and the
> contents of the source= expression is copied using the assign. But when a move
> could be done the alloc/dealloc of the components is skipped. With this I hope
> to safe a lot of mallocs and frees, which are not that cheap. In the most common
> case where only one object is allocated, there now is only one alloc for the
> components to get expr3 up and one for the object to allocate. We safe the
> allocate of the allocatable components in the object to allocate and the free
> of the source= components. I hope I could make clear what I desire? If not
> maybe a look into the patch might help. What do you think?
>
> The patch of course is only a quick implementation of the idea. Please
> comment, everyone!
>
> Regards,
>         Andre
>
>
> On Mon, 25 May 2015 09:30:34 +0200
> Paul Richard Thomas <paul.richard.thomas@gmail.com> wrote:
>
>> Dear All,
>>
>> Lets see if I can get it right this time :-)
>>
>> Note that I have changed the name of the temporary variable in
>> trans_allocate from 'atmp' to 'expr3' so that it is not confused with
>> array temporaries. I am not suree how much of the testcase is
>> pertinent after the reform of the evaluation of expr3 performed by
>> Andre. However, there were still memory leaks that are fixed by the
>> attached patch.
>>
>> Bootstrapped and regtested on a current trunk - OK for trunk?
>>
>> Paul
>>
>> 2015-05-23  Paul Thomas  <pault@gcc.gnu.org>
>>
>>     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): Rename temporary variable
>>     as 'expr3'. Deallocate allocatable components of non-variable
>>     expr3s.
>>
>> 2015-05-23  Paul Thomas  <pault@gcc.gnu.org>
>>
>>     PR fortran/66079
>>     * gfortran.dg/allocatable_scalar_13.f90: New test
>>
>>
>> On 24 May 2015 at 09:51, Paul Richard Thomas
>> <paul.richard.thomas@gmail.com> wrote:
>> > Dear Andre,
>> >
>> > I'll put both points right. Thanks for pointing them out.
>> >
>> > Cheers
>> >
>> > Paul
>> >
>> > On 23 May 2015 at 19:52, Andre Vehreschild <vehre@gmx.de> 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 <paul.richard.thomas@gmail.com> 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  <pault@gcc.gnu.org>
>> >>>
>> >>>     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  <pault@gcc.gnu.org>
>> >>>
>> >>>     PR fortran/66079
>> >>>     * gfortran.dg/allocatable_scalar_13.f90: New test
>> >>
>> >>
>> >> --
>> >> Andre Vehreschild * Email: vehre ad gmx dot de
>> >
>> >
>> >
>> > --
>> > Outside of a dog, a book is a man's best friend. Inside of a dog it's
>> > too dark to read.
>> >
>> > Groucho Marx
>>
>>
>>
>
>
> --
> Andre Vehreschild * Email: vehre ad gmx dot de



-- 
Outside of a dog, a book is a man's best friend. Inside of a dog it's
too dark to read.

Groucho Marx

Re: [Patch, fortran] PR66079 - [6 Regression] memory leak with source allocation in internal subprogram

[Andre Vehreschild]

Hi Paul, hi Mikael,

about renaming the identifier emitted: I would like to keep it short. Remember,
there is always a number attached to it, which makes it unique. Furthermore
does "alloc_source_tmp" sound unnecessarily long to me. It tastes like we do
not trust the unique identifier mechanism established in gfortran. But that is
just my personal taste.

about missing expr->rank == 0) in the extended patch: I just wanted to present
an idea here. The patch was not meant to be commited yet. I think it
furthermore is just half of the rent (like we say in Germany). I think we can
do better, when we also think about the preceeding two if-blocks (the ones
taking care about derived and class types). It should be possible to do
something similar there. Furthermore could one think about moving e3rhs for
array valued objects, too. But then we should not move to the last element, but
instead to the first element. Nevertheless in the array valued case one might
end up still having to deallocate the components or e3rhs, when the object
allocated is zero sized. I wonder whether the bother really pays.

What do you think about it?

Paul: I would recommend you commit with symbol rename, but without the move
optimization. We can do that later.

Mikael: I usually do favor else if, too. Because of quick and dirty nature of
the patch, I omitted to stick to the standard code convention.

Regards,
	Andre

On Wed, 27 May 2015 09:59:20 +0200
Paul Richard Thomas <paul.richard.thomas@gmail.com> wrote:

> Dear Andre,
> 
> I am perfectly happy with renaming the rename to "source". I was
> attempting to distinguish "atmp" coming from trans-array.c from this
> temporary; just as an aid to any possible future debugging.
> 
> The rework of the patch looks fine to me as well. Do you want to
> commit or should I do so?
> 
> Cheers
> 
> Paul
> 
> On 25 May 2015 at 12:24, Andre Vehreschild <vehre@gmx.de> wrote:
> > Hi Paul,
> >
> > I am not quite happy with the naming of the temporary variable. When I
> > initially set the prefix to "atmp" this was because the variable would be an
> > array most of the time and because of the number appended to it should be
> > unique anyway. However I would like to point out that disclosing an internal
> > implementation detail of the compiler to a future developer looking at the
> > pseudo-code dump will not help (I mean "expr3", here). I would rather use
> > "source" as the prefix now that I think of it with some distance to the
> > original naming. What do you think?
> >
> > Now that the deallocate for source's components is in the patch, I
> > understand why initially the source= preevaluation for derived types with
> > allocatable components was disallowed. Thanks for clarifying that.
> >
> > I wonder though, if we can't do better...
> >
> > Please have a look at the attached patch. It not only renames the temporary
> > variable from "expr3" to "source" (couldn't help, but do it. Please don't be
> > angry :-)), but also adds move semantics to source= expressions for the last
> > object to allocate. I.e., when a scalar source= expression with allocatable
> > components is detected, then its content is "moved" (memcpy'ed) to the last
> > object to allocate instead of being assigned. All former objects to allocate
> > are of course handled like before, i.e., components are allocated and the
> > contents of the source= expression is copied using the assign. But when a
> > move could be done the alloc/dealloc of the components is skipped. With
> > this I hope to safe a lot of mallocs and frees, which are not that cheap.
> > In the most common case where only one object is allocated, there now is
> > only one alloc for the components to get expr3 up and one for the object to
> > allocate. We safe the allocate of the allocatable components in the object
> > to allocate and the free of the source= components. I hope I could make
> > clear what I desire? If not maybe a look into the patch might help. What do
> > you think?
> >
> > The patch of course is only a quick implementation of the idea. Please
> > comment, everyone!
> >
> > Regards,
> >         Andre
> >
> >
> > On Mon, 25 May 2015 09:30:34 +0200
> > Paul Richard Thomas <paul.richard.thomas@gmail.com> wrote:
> >
> >> Dear All,
> >>
> >> Lets see if I can get it right this time :-)
> >>
> >> Note that I have changed the name of the temporary variable in
> >> trans_allocate from 'atmp' to 'expr3' so that it is not confused with
> >> array temporaries. I am not suree how much of the testcase is
> >> pertinent after the reform of the evaluation of expr3 performed by
> >> Andre. However, there were still memory leaks that are fixed by the
> >> attached patch.
> >>
> >> Bootstrapped and regtested on a current trunk - OK for trunk?
> >>
> >> Paul
> >>
> >> 2015-05-23  Paul Thomas  <pault@gcc.gnu.org>
> >>
> >>     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): Rename temporary variable
> >>     as 'expr3'. Deallocate allocatable components of non-variable
> >>     expr3s.
> >>
> >> 2015-05-23  Paul Thomas  <pault@gcc.gnu.org>
> >>
> >>     PR fortran/66079
> >>     * gfortran.dg/allocatable_scalar_13.f90: New test
> >>
> >>
> >> On 24 May 2015 at 09:51, Paul Richard Thomas
> >> <paul.richard.thomas@gmail.com> wrote:
> >> > Dear Andre,
> >> >
> >> > I'll put both points right. Thanks for pointing them out.
> >> >
> >> > Cheers
> >> >
> >> > Paul
> >> >
> >> > On 23 May 2015 at 19:52, Andre Vehreschild <vehre@gmx.de> 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 <paul.richard.thomas@gmail.com> 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  <pault@gcc.gnu.org>
> >> >>>
> >> >>>     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  <pault@gcc.gnu.org>
> >> >>>
> >> >>>     PR fortran/66079
> >> >>>     * gfortran.dg/allocatable_scalar_13.f90: New test
> >> >>
> >> >>
> >> >> --
> >> >> Andre Vehreschild * Email: vehre ad gmx dot de
> >> >
> >> >
> >> >
> >> > --
> >> > Outside of a dog, a book is a man's best friend. Inside of a dog it's
> >> > too dark to read.
> >> >
> >> > Groucho Marx
> >>
> >>
> >>
> >
> >
> > --
> > Andre Vehreschild * Email: vehre ad gmx dot de
> 
> 
> 


-- 
Andre Vehreschild * Email: vehre ad gmx dot de 

Re: [Patch, fortran] PR66079 - [6 Regression] memory leak with source allocation in internal subprogram

[Mikael Morin]

Le 27/05/2015 16:07, Andre Vehreschild a écrit :
> Hi Paul, hi Mikael,
> 
> about renaming the identifier emitted: I would like to keep it short. Remember,
> there is always a number attached to it, which makes it unique. Furthermore
> does "alloc_source_tmp" sound unnecessarily long to me. It tastes like we do
> not trust the unique identifier mechanism established in gfortran. But that is
> just my personal taste.
> 
Then let's go with "source", which seems to get the majority of the
votes.  It remains an improvement over "expr3" and "atmp".

> about missing expr->rank == 0) in the extended patch: I just wanted to present
> an idea here. The patch was not meant to be commited yet. I think it
> furthermore is just half of the rent (like we say in Germany). I think we can
> do better, when we also think about the preceeding two if-blocks (the ones
> taking care about derived and class types). It should be possible to do
> something similar there. Furthermore could one think about moving e3rhs for
> array valued objects, too. But then we should not move to the last element, but
> instead to the first element. Nevertheless in the array valued case one might
> end up still having to deallocate the components or e3rhs, when the object
> allocated is zero sized. I wonder whether the bother really pays.
> 
> What do you think about it?
> 
I don't want to review monster patches. ;-)
More seriously, I think there are more important things than this, but
the patch was there and seemed reasonable.
One can add support for the other if-blocks.  About the rest, I'm not
sure I understand.  Or rather, I'm sure I don't.
Does it make a difference first or last element?
What is so specific about "array valued case"?
We can try to add support for this in more and more cases, but please
let's not make the code impossible to understand.

> Paul: I would recommend you commit with symbol rename, but without the move
> optimization. We can do that later.
> 
Agreed.

Mikael

Re: [Patch, fortran] PR66079 - [6 Regression] memory leak with source allocation in internal subprogram

[Steve Kargl]

On Wed, May 27, 2015 at 06:24:25PM +0200, Mikael Morin wrote:
> Le 27/05/2015 16:07, Andre Vehreschild a ?crit :
> > Hi Paul, hi Mikael,
> > 
> > about renaming the identifier emitted: I would like to keep it short. Remember,
> > there is always a number attached to it, which makes it unique. Furthermore
> > does "alloc_source_tmp" sound unnecessarily long to me. It tastes like we do
> > not trust the unique identifier mechanism established in gfortran. But that is
> > just my personal taste.
> > 
> Then let's go with "source", which seems to get the majority of the
> votes.  It remains an improvement over "expr3" and "atmp".
> 

You do realize that expr3 holds things other than the 
expression in a source= in an allocate, right? 

-- 
Steve

Re: [Patch, fortran] PR66079 - [6 Regression] memory leak with source allocation in internal subprogram

[Mikael Morin]

Le 27/05/2015 23:09, Steve Kargl a écrit :
> On Wed, May 27, 2015 at 06:24:25PM +0200, Mikael Morin wrote:
>> Le 27/05/2015 16:07, Andre Vehreschild a ?crit :
>>> Hi Paul, hi Mikael,
>>>
>>> about renaming the identifier emitted: I would like to keep it short. Remember,
>>> there is always a number attached to it, which makes it unique. Furthermore
>>> does "alloc_source_tmp" sound unnecessarily long to me. It tastes like we do
>>> not trust the unique identifier mechanism established in gfortran. But that is
>>> just my personal taste.
>>>
>> Then let's go with "source", which seems to get the majority of the
>> votes.  It remains an improvement over "expr3" and "atmp".
>>
> 
> You do realize that expr3 holds things other than the 
> expression in a source= in an allocate, right? 
> 
I know there is mold.  I'm not aware of anything else.
Now that you tell about it, I realize that the code in that area doesn't
seem to  check for mold vs source.

Mikael

Re: [Patch, fortran] PR66079 - [6 Regression] memory leak with source allocation in internal subprogram

[Andre Vehreschild]

On Thu, 28 May 2015 16:58:44 +0200
Mikael Morin <mikael.morin@sfr.fr> wrote:

> Le 27/05/2015 23:09, Steve Kargl a écrit :
> > On Wed, May 27, 2015 at 06:24:25PM +0200, Mikael Morin wrote:
> >> Le 27/05/2015 16:07, Andre Vehreschild a ?crit :
> >>> Hi Paul, hi Mikael,
> >>>
> >>> about renaming the identifier emitted: I would like to keep it short.
> >>> Remember, there is always a number attached to it, which makes it unique.
> >>> Furthermore does "alloc_source_tmp" sound unnecessarily long to me. It
> >>> tastes like we do not trust the unique identifier mechanism established
> >>> in gfortran. But that is just my personal taste.
> >>>
> >> Then let's go with "source", which seems to get the majority of the
> >> votes.  It remains an improvement over "expr3" and "atmp".
> >>
> > 
> > You do realize that expr3 holds things other than the 
> > expression in a source= in an allocate, right? 
> > 
> I know there is mold.  I'm not aware of anything else.
> Now that you tell about it, I realize that the code in that area doesn't
> seem to  check for mold vs source.

Which is arbitrary.

- Andre
-- 
Andre Vehreschild * Email: vehre ad gmx dot de 

Re: [Patch, fortran] PR66079 - [6 Regression] memory leak with source allocation in internal subprogram

[Mikael Morin]

Le 25/05/2015 09:30, Paul Richard Thomas a écrit :
> Dear All,
> 
> Lets see if I can get it right this time :-)
> 
> Note that I have changed the name of the temporary variable in
> trans_allocate from 'atmp' to 'expr3' so that it is not confused with
> array temporaries.
I agree with Andre willing to use a more explicit name ("source"), and I
suggest pushing it yet further; something like "alloc_source_tmp".
We should never have named things "expr3" all over the place,  but
that's another matter.

> Bootstrapped and regtested on a current trunk - OK for trunk?
> 
Yes, looks good. Thanks.

Mikael

Re: [Patch, fortran] PR66079 - [6 Regression] memory leak with source allocation in internal subprogram

[Steve Kargl]

On Mon, May 25, 2015 at 07:33:12PM +0200, Mikael Morin wrote:
> Le 25/05/2015 09:30, Paul Richard Thomas a ?crit :
> > Dear All,
> > 
> > Lets see if I can get it right this time :-)
> > 
> > Note that I have changed the name of the temporary variable in
> > trans_allocate from 'atmp' to 'expr3' so that it is not confused with
> > array temporaries.
> I agree with Andre willing to use a more explicit name ("source"), and I
> suggest pushing it yet further; something like "alloc_source_tmp".
> We should never have named things "expr3" all over the place,  but
> that's another matter.
> 

I choose "expr3" because "expr1" and "expr2" were already 
present in gfc_code.  It's been awhile since I looked at
gfc_code.  Seems we now also have expr4.
 
-- 
Steve

Re: [Patch, fortran] PR66079 - [6 Regression] memory leak with source allocation in internal subprogram

[Paul Richard Thomas]

Dear Andre and Mikael,

Thanks for the reviews and the discussion about the name of the
temporary. I went for "source" in the end. The "expr3" was merely
meant to align with the expression....

Following Andre's suggestion, I left his additions to another time;
especially since the patch was heading towards bit-rot because I have
been so long in getting back to it.

Committed to trunk as revision 224383.


Cheers

Paul

On 25 May 2015 at 19:33, Mikael Morin <mikael.morin@sfr.fr> wrote:
> Le 25/05/2015 09:30, Paul Richard Thomas a écrit :
>> Dear All,
>>
>> Lets see if I can get it right this time :-)
>>
>> Note that I have changed the name of the temporary variable in
>> trans_allocate from 'atmp' to 'expr3' so that it is not confused with
>> array temporaries.
> I agree with Andre willing to use a more explicit name ("source"), and I
> suggest pushing it yet further; something like "alloc_source_tmp".
> We should never have named things "expr3" all over the place,  but
> that's another matter.
>
>> Bootstrapped and regtested on a current trunk - OK for trunk?
>>
> Yes, looks good. Thanks.
>
> Mikael



-- 
Outside of a dog, a book is a man's best friend. Inside of a dog it's
too dark to read.

Groucho Marx

Re: [Patch, fortran] PR66079 - [6 Regression] memory leak with source allocation in internal subprogram

[Paul Richard Thomas]

Dear All,

Somewhat belatedly, I have applied the patch to 5 branch and have
committed as revision 228952.

Closing the PR forthwith.

Paul

On 24 May 2015 at 09:51, Paul Richard Thomas
<paul.richard.thomas@gmail.com> wrote:
> Dear Andre,
>
> I'll put both points right. Thanks for pointing them out.
>
> Cheers
>
> Paul
>
> On 23 May 2015 at 19:52, Andre Vehreschild <vehre@gmx.de> 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 <paul.richard.thomas@gmail.com> 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  <pault@gcc.gnu.org>
>>>
>>>     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  <pault@gcc.gnu.org>
>>>
>>>     PR fortran/66079
>>>     * gfortran.dg/allocatable_scalar_13.f90: New test
>>
>>
>> --
>> Andre Vehreschild * Email: vehre ad gmx dot de
>
>
>
> --
> Outside of a dog, a book is a man's best friend. Inside of a dog it's
> too dark to read.
>
> Groucho Marx



-- 
Outside of a dog, a book is a man's best friend. Inside of a dog it's
too dark to read.

Groucho Marx