[Bug fortran/41936] Memory leakage with allocatables and user-defined operators

[Bug fortran/41936] Memory leakage with allocatables and user-defined operators

[mikael at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=41936

Mikael Morin <mikael at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |patch
                 CC|                            |mikael at gcc dot gnu.org

--- Comment #2 from Mikael Morin <mikael at gcc dot gnu.org> 2011-02-28 21:22:39 UTC ---
Fixed if one adds the code below (copied from gfc_conv_array_parameter).
I'm afraid this could change a memory leak into a double free (see PR 40850).
Also, not quite right (even if the middle-end optimizers are likely to fix it)
because it adds cleanup code to both foo and bar.

diff --git a/trans-expr.c b/trans-expr.c
index d6c1f9f..3919870 100644
--- a/trans-expr.c
+++ b/trans-expr.c
@@ -4900,6 +4900,19 @@ 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->ts.type == BT_CLASS)
+      && 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);
+    }
 }

[Bug fortran/41936] Memory leakage with allocatables and user-defined operators

[dominiq at lps dot ens.fr]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=41936

--- Comment #3 from Dominique d'Humieres <dominiq at lps dot ens.fr> ---
> Fixed if one adds the code below (copied from gfc_conv_array_parameter).
> I'm afraid this could change a memory leak into a double free (see PR 40850).
> Also, not quite right (even if the middle-end optimizers are likely to fix it) 
> because it adds cleanup code to both foo and bar.

With the patch in comment 2, the tests gfortran.dg/alloc_comp_basics_1.f90 and
gfortran.dg/alloc_comp_constructor_1.f90 fail because there are 21 occurrences
of "builtin_free". This is fixed by the following updated patch (against
r209838)

--- ../_clean/gcc/fortran/trans-expr.c    2014-04-25 14:30:23.000000000 +0200
+++ gcc/fortran/trans-expr.c    2014-04-27 16:03:28.000000000 +0200
@@ -6474,6 +6474,19 @@ gfc_conv_expr_reference (gfc_se * se, gf

   /* Take the address of that value.  */
   se->expr = gfc_build_addr_expr (NULL_TREE, var);
+  if ((expr->ts.type == BT_DERIVED || expr->ts.type == BT_CLASS)
+      && expr->ts.u.derived->attr.alloc_comp && expr->rank
+      && 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);
+    }
 }

[Bug fortran/41936] Memory leakage with allocatables and user-defined operators

[burnus at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=41936

--- Comment #4 from Tobias Burnus <burnus at gcc dot gnu.org> ---
(In reply to Dominique d'Humieres from comment #3)
> +  if ((expr->ts.type == BT_DERIVED || expr->ts.type == BT_CLASS)
> +      && expr->ts.u.derived->attr.alloc_comp && expr->rank
> +      && 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);

Calling gfc_deallocate_alloc_comp for BT_CLASS looks wrong. You have to call
the finalization wrapper - to ensure that not only the allocatable components
of the declared type but also the ones of the effective/actual type are
deallocated. Additionally, that ensure that user's finalizer is called when it
exists.

(For BT_DERIVED, you may also have to call the finalization wrapper - but only
if the type has finalizers.)

[Bug fortran/41936] Memory leakage with allocatables and user-defined operators

[dominiq at lps dot ens.fr]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=41936

--- Comment #5 from Dominique d'Humieres <dominiq at lps dot ens.fr> ---
> Calling gfc_deallocate_alloc_comp for BT_CLASS looks wrong. ...

I have only added " && expr->rank" to the three year old Mikael's patch.

> You have to call the finalization wrapper - to ensure that not only the 
> allocatable components of the declared type but also the ones of the
> effective/actual type are deallocated. Additionally, that ensure that
> user's finalizer is called when it exists.
>
> (For BT_DERIVED, you may also have to call the finalization wrapper - but only
> if the type has finalizers.)

Could you please provide a more explicit pointer?

[Bug fortran/41936] Memory leakage with allocatables and user-defined operators

[dominiq at lps dot ens.fr]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=41936

--- Comment #6 from Dominique d'Humieres <dominiq at lps dot ens.fr> ---
Is the following patch OK?

--- ../_clean/gcc/fortran/trans-expr.c    2014-04-30 21:41:33.000000000 +0200
+++ gcc/fortran/trans-expr.c    2014-05-04 00:42:50.000000000 +0200
@@ -6504,6 +6504,20 @@ gfc_conv_expr_reference (gfc_se * se, gf

   /* 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);
+    }
 }


It fixes the memory leaks for the test in comment 0 and for
gfortran.dg/class_array_15.f03, but not pr55603 nor pr60913.

Is the following change for gfortran.dg/class_array_15.f03

--- ../_clean/gcc/testsuite/gfortran.dg/class_array_15.f03    2013-01-06
22:34:50.000000000 +0100
+++ gcc/testsuite/gfortran.dg/class_array_15.f03    2014-05-04
10:24:06.000000000 +0200
@@ -1,4 +1,5 @@
 ! { dg-do run }
+! { dg-options "-fdump-tree-original" }
 !
 ! Tests the fixes for three bugs with the same underlying cause.  All are
regressions
 ! that come about because class array elements end up with a different tree
type
@@ -114,3 +115,5 @@ subroutine pr54992  ! This test remains 
   bh => bhGet(b,instance=2)
   if (loc (b) .ne. loc(bh%hostNode)) call abort
 end
+! { dg-final { scan-tree-dump-times "builtin_free" 12 "original" } }
+! { dg-final { cleanup-tree-dump "original" } }

enough (there are only 11 builtin_free without the patch)? or should I add the
test in comment 0 (15 builtin_free with the patch, 12 without)?

[Bug fortran/41936] Memory leakage with allocatables and user-defined operators

[dominiq at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=41936

--- Comment #7 from dominiq at gcc dot gnu.org ---
Author: dominiq
Date: Tue Jun 10 11:42:38 2014
New Revision: 211405

URL: http://gcc.gnu.org/viewcvs?rev=211405&root=gcc&view=rev
Log:
2014-06-10  Dominique d'Humieres <dominiq@lps.ens.fr>
        Mikael Morin <mikael@gcc.gnu.org>

    PR fortran/41936
gcc/fortran/
    * trans-expr.c (gfc_conv_expr_reference): Deallocate array
    components.

gcc/testsuite/
    * gfortran.dg/class_array_15.f03: Check memory leaks.


Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/trans-expr.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gfortran.dg/class_array_15.f03

[Bug fortran/41936] Memory leakage with allocatables and user-defined operators

[dominiq at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=41936

--- Comment #8 from dominiq at gcc dot gnu.org ---
Author: dominiq
Date: Mon Jul  7 12:32:37 2014
New Revision: 212329

URL: https://gcc.gnu.org/viewcvs?rev=212329&root=gcc&view=rev
Log:
2014-07-07  Dominique d'Humieres <dominiq@lps.ens.fr>
        Mikael Morin <mikael@gcc.gnu.org>

    PR fortran/41936
gcc/fortran/
    * trans-expr.c (gfc_conv_expr_reference): Deallocate array
    components.

gcc/testsuite/
    * gfortran.dg/class_array_15.f03: Check memory leaks.


Modified:
    branches/gcc-4_9-branch/gcc/fortran/ChangeLog
    branches/gcc-4_9-branch/gcc/fortran/trans-expr.c
    branches/gcc-4_9-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_9-branch/gcc/testsuite/gfortran.dg/class_array_15.f03

[Bug fortran/41936] Memory leakage with allocatables and user-defined operators

[dominiq at lps dot ens.fr]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=41936

Dominique d'Humieres <dominiq at lps dot ens.fr> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|pault at gcc dot gnu.org           |dominiq at lps dot ens.fr

--- Comment #9 from Dominique d'Humieres <dominiq at lps dot ens.fr> ---
Fixed on 4.9.1 and trunk (4.10.0).

[Bug fortran/41936] Memory leakage with allocatables and user-defined operators

[dominiq at lps dot ens.fr]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=41936

Dominique d'Humieres <dominiq at lps dot ens.fr> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #10 from Dominique d'Humieres <dominiq at lps dot ens.fr> ---
Closing.

[Bug fortran/41936] Memory leakage with allocatables and user-defined operators

[pault at gcc dot gnu dot org]

------- Comment #1 from pault at gcc dot gnu dot org  2009-11-12 13:50 -------
The testcase segfaults on the a = a + b and the c = c + d on Cygwin.  On FC8
i386, valgrind does not show any problem with either and the testprogramme runs
to completion!

However, moving a = a + b to subroutine foo and c = c+ d to subroutine bar
isolates the source of the problem.

bar ()
{
  {
    integer(kind=4) S.23;

    S.23 = 1;
    while (1)
      {
        if (S.23 > 3) goto L.11;
        {
          struct my_type D.1542;

          D.1542 = my_add (&c[S.23 + -1], &d[S.23 + -1]);
          my_assign (&c[S.23 + -1], &D.1542);
        }
        S.23 = S.23 + 1;
      }
    L.11:;
  }
}


foo ()
{
  {
    struct my_type D.1547;

    D.1547 = my_add (&a, &b);
    my_assign (&a, &D.1547);
    if (D.1547.x.data != 0B)
      {
        __builtin_free ((void *) D.1547.x.data);
      }
    D.1547.x.data = 0B;
    if (D.1547.y.data != 0B)
      {
        __builtin_free ((void *) D.1547.y.data);
      }
    D.1547.y.data = 0B;
    if (D.1547.z.data != 0B)
      {
        __builtin_free ((void *) D.1547.z.data);
      }
    D.1547.z.data = 0B;
  }
}

Most of the right allocating and freeing goes on in 'my_add' and 'my_assign'. 
However, whilst 'foo' (scalar) frees the allocatable components of the
temporary D.1547, the same is not true for D.1542 in 'bar' (array).

This loses (n = 3) x (4 bytes) x (3 components) x (dim = 10) = 360 bytes.

The problem, therefore, lies in the scalarization of the elemental call to
'my_add', where the deallocation of the temporary goes walkabout.  If I would
hazard a guess, without looking, I would say that the post-block for the scalar
assignment has not been incorporated in the loop body.

Paul


-- 

pault at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |pault at gcc dot gnu dot org
                   |dot org                     |
             Status|UNCONFIRMED                 |ASSIGNED
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2009-11-12 13:50:59
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=41936