[Bug fortran/48700] New: [OOP] memory leak with MOVE_ALLOC of polymorphic variables

[Bug fortran/48700] New: [OOP] memory leak with MOVE_ALLOC of polymorphic variables

[sfilippone at uniroma2 dot it]

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

           Summary: [OOP] memory leak with  MOVE_ALLOC  of polymorphic
                    variables
           Product: gcc
           Version: 4.7.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: fortran
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: sfilippone@uniroma2.it


Created attachment 24058
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=24058
test-case

Running the attached test case through valgrind gives memory leak warnings,
both with current trunk and 4.6.0

----------------------------------------------------------------------------
[sfilippo@donald bug31]$ gfortran -v
Using built-in specs.
COLLECT_GCC=gfortran
COLLECT_LTO_WRAPPER=/usr/local/gnu47/libexec/gcc/x86_64-unknown-linux-gnu/4.7.0/lto-wrapper
Target: x86_64-unknown-linux-gnu
Configured with: ../gcc/configure --prefix=/usr/local/gnu47
--enable-languages=c,c++,fortran --with-gmp=/home/travel/GNUBUILD/gmp
--with-mpfr=/home/travel/GNUBUILD/mpfr --with-mpc=/home/travel/GNUBUILD/mpc :
(reconfigured) ../gcc/configure --prefix=/usr/local/gnu47
--enable-languages=c,c++,fortran --with-gmp=/home/travel/GNUBUILD/gmp
--with-mpfr=/home/travel/GNUBUILD/mpfr --with-mpc=/home/travel/GNUBUILD/mpc :
(reconfigured) ../gcc/configure --prefix=/usr/local/gnu47
--enable-languages=c,c++,fortran --with-gmp=/home/travel/GNUBUILD/gmp
--with-mpfr=/home/travel/GNUBUILD/mpfr --with-mpc=/home/travel/GNUBUILD/mpc
Thread model: posix
gcc version 4.7.0 20110418 (experimental) (GCC) 
[sfilippo@donald bug31]$ gfortran -o testmv3 testmv3.f90 -ggdb 
[sfilippo@donald bug31]$ valgrind --leak-check=full ./testmv3
==25909== Memcheck, a memory error detector
==25909== Copyright (C) 2002-2009, and GNU GPL'd, by Julian Seward et al.
==25909== Using Valgrind-3.5.0 and LibVEX; rerun with -h for copyright info
==25909== Command: ./testmv3
==25909== 
==25909== 
==25909== HEAP SUMMARY:
==25909==     in use at exit: 216 bytes in 4 blocks
==25909==   total heap usage: 26 allocs, 22 frees, 12,368 bytes allocated
==25909== 
==25909== 40 bytes in 1 blocks are definitely lost in loss record 3 of 4
==25909==    at 0x4A05E46: malloc (vg_replace_malloc.c:195)
==25909==    by 0x401425: MAIN__ (testmv3.f90:37)
==25909==    by 0x401729: main (testmv3.f90:22)
==25909== 
==25909== 176 (96 direct, 80 indirect) bytes in 1 blocks are definitely lost in
loss record 4 of 4
==25909==    at 0x4A05E46: malloc (vg_replace_malloc.c:195)
==25909==    by 0x400DCF: MAIN__ (testmv3.f90:30)
==25909==    by 0x401729: main (testmv3.f90:22)
==25909== 
==25909== LEAK SUMMARY:
==25909==    definitely lost: 136 bytes in 2 blocks
==25909==    indirectly lost: 80 bytes in 2 blocks
==25909==      possibly lost: 0 bytes in 0 blocks
==25909==    still reachable: 0 bytes in 0 blocks
==25909==         suppressed: 0 bytes in 0 blocks
==25909== 
==25909== For counts of detected and suppressed errors, rerun with: -v
==25909== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 6 from 6)

[Bug fortran/48700] [OOP] memory leak with MOVE_ALLOC of polymorphic variables

[janus at gcc dot gnu.org]

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

janus at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2011.05.15 10:30:25
                 CC|                            |janus at gcc dot gnu.org
     Ever Confirmed|0                           |1

--- Comment #1 from janus at gcc dot gnu.org 2011-05-15 10:30:25 UTC ---
(In reply to comment #0)
> ==25909== 40 bytes in 1 blocks are definitely lost in loss record 3 of 4
> ==25909==    at 0x4A05E46: malloc (vg_replace_malloc.c:195)
> ==25909==    by 0x401425: MAIN__ (testmv3.f90:37)
> ==25909==    by 0x401729: main (testmv3.f90:22)

This guy is due to polymorphic deallocation not working properly yet (cf.
PR46321), it is unrelated to MOVE_ALLOC. The component "ka", which is allocated
by "sm%ka=dat%sm%ja", is never freed. We presently only free the components of
the declared type, not the dynamic type.

[Bug fortran/48700] memory leak with MOVE_ALLOC

[janus at gcc dot gnu.org]

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

janus at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[OOP] memory leak with      |memory leak with MOVE_ALLOC
                   |MOVE_ALLOC  of polymorphic  |
                   |variables                   |

--- Comment #2 from janus at gcc dot gnu.org 2011-05-15 11:05:24 UTC ---
(In reply to comment #0)
> ==25909== 176 (96 direct, 80 indirect) bytes in 1 blocks are definitely lost in
> loss record 4 of 4
> ==25909==    at 0x4A05E46: malloc (vg_replace_malloc.c:195)
> ==25909==    by 0x400DCF: MAIN__ (testmv3.f90:30)
> ==25909==    by 0x401729: main (testmv3.f90:22)

This one indeed seems to be a problem with MOVE_ALLOC, but apparently unrelated
to OOP/polymorphism. Reduced test case:


program testmv3
  type bar
    integer, allocatable  :: ia(:), ja(:)
  end type

  type(bar), allocatable :: sm,sm2

  allocate(sm)
  allocate(sm%ia(10),sm%ja(10))

  call move_alloc(sm2,sm)

end program testmv3 


I think the 80 indirectly lost bytes should be the allocatable components
(40+40), while the 96 are probably their array descriptors (48+48).

The MOVE_ALLOC statement is simply translated to:

        sm = sm2;
        sm2 = 0B;

We miss to deallocate "sm", before it gets overridden.

The standard definitely requires this, because
1) it says that the second argument ('TO') of MOVE_ALLOC is INTENT(OUT), cf.
F08:13.7.118,
2) allocatable INTENT(OUT) arguments must be deallocated upon procedure call,
cf. F08:6.7.3.2.

[Bug fortran/48700] memory leak with MOVE_ALLOC

[janus at gcc dot gnu.org]

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

janus at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|unassigned at gcc dot       |janus at gcc dot gnu.org
                   |gnu.org                     |

--- Comment #3 from janus at gcc dot gnu.org 2011-05-15 12:18:57 UTC ---
(In reply to comment #2)
> We miss to deallocate "sm", before it gets overridden.

Simple patch which does just that (not regtested):


Index: gcc/fortran/trans-intrinsic.c
===================================================================
--- gcc/fortran/trans-intrinsic.c    (revision 173579)
+++ gcc/fortran/trans-intrinsic.c    (working copy)
@@ -6961,12 +6961,20 @@ gfc_conv_intrinsic_move_alloc (gfc_code *code)
       gfc_expr *from, *to;
       stmtblock_t block;
       tree tmp;
+      gfc_se se;

       from = code->ext.actual->expr;
       to = code->ext.actual->next->expr;

       gfc_start_block (&block);

+      /* Deallocate 'TO' argument.  */
+      gfc_init_se (&se, NULL);
+      se.want_pointer = 1;
+      gfc_conv_expr (&se, to);
+      tmp = gfc_deallocate_scalar_with_status (se.expr, NULL, true, to,
to->ts);
+      gfc_add_expr_to_block (&block, tmp);
+
       if (to->ts.type == BT_CLASS)
     tmp = gfc_trans_class_assign (to, from, EXEC_POINTER_ASSIGN);
       else

[Bug fortran/48700] memory leak with MOVE_ALLOC

[janus at gcc dot gnu.org]

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

--- Comment #4 from janus at gcc dot gnu.org 2011-05-15 13:15:23 UTC ---
(In reply to comment #3)
> > We miss to deallocate "sm", before it gets overridden.
> 
> Simple patch which does just that (not regtested):

Fails at least on move_alloc_2.f90.

[Bug fortran/48700] memory leak with MOVE_ALLOC

[janus at gcc dot gnu.org]

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

--- Comment #5 from janus at gcc dot gnu.org 2011-05-15 17:23:11 UTC ---
Updated patch:

Index: gcc/fortran/trans-intrinsic.c
===================================================================
--- gcc/fortran/trans-intrinsic.c    (revision 173770)
+++ gcc/fortran/trans-intrinsic.c    (working copy)
@@ -6958,15 +6958,27 @@ gfc_conv_intrinsic_move_alloc (gfc_code *code)
   if (code->ext.actual->expr->rank == 0)
     {
       /* Scalar arguments: Generate pointer assignments.  */
-      gfc_expr *from, *to;
+      gfc_expr *from, *to, *deal;
       stmtblock_t block;
       tree tmp;
+      gfc_se se;

       from = code->ext.actual->expr;
       to = code->ext.actual->next->expr;

       gfc_start_block (&block);

+      /* Deallocate 'TO' argument.  */
+      gfc_init_se (&se, NULL);
+      se.want_pointer = 1;
+      deal = gfc_copy_expr (to);
+      if (deal->ts.type == BT_CLASS)
+    gfc_add_data_component (deal);
+      gfc_conv_expr (&se, deal);
+      tmp = gfc_deallocate_scalar_with_status (se.expr, NULL, true,
+                           deal, deal->ts);
+      gfc_add_expr_to_block (&block, tmp);
+
       if (to->ts.type == BT_CLASS)
     tmp = gfc_trans_class_assign (to, from, EXEC_POINTER_ASSIGN);
       else

[Bug fortran/48700] memory leak with MOVE_ALLOC

[janus at gcc dot gnu.org]

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

--- Comment #6 from janus at gcc dot gnu.org 2011-05-15 22:05:00 UTC ---
The patch in comment #5 regtests cleanly.


But apparently there is also a problem with MOVE_ALLOC and allocatable arrays:


program testmv3
  type bar
    integer, allocatable  :: ia(:), ja(:)
  end type

  type(bar), allocatable :: sm(:),sm2(:)

  allocate(sm(1))
  allocate(sm(1)%ia(10),sm(1)%ja(10))

  call move_alloc(sm2,sm)

end program testmv3


valgrind shows that the allocatable components are not being freed:


==21249== 40 bytes in 1 blocks are definitely lost in loss record 1 of 2
==21249==    at 0x4C2683D: malloc (in
/usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==21249==    by 0x400B8F: MAIN__ (arr.f90:10)
==21249==    by 0x400FE6: main (arr.f90:14)
==21249== 
==21249== 40 bytes in 1 blocks are definitely lost in loss record 2 of 2
==21249==    at 0x4C2683D: malloc (in
/usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==21249==    by 0x400CF4: MAIN__ (arr.f90:10)
==21249==    by 0x400FE6: main (arr.f90:14)

[Bug fortran/48700] memory leak with MOVE_ALLOC

[janus at gcc dot gnu.org]

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

--- Comment #7 from janus at gcc dot gnu.org 2011-05-18 18:51:11 UTC ---
Author: janus
Date: Wed May 18 18:51:08 2011
New Revision: 173874

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=173874
Log:
2011-05-18  Janus Weil  <janus@gcc.gnu.org>

    PR fortran/48700
    * trans-intrinsic.c (gfc_conv_intrinsic_move_alloc): Deallocate 'TO'
    argument to avoid memory leaks.


2011-05-18  Janus Weil  <janus@gcc.gnu.org>

    PR fortran/48700
    * gfortran.dg/move_alloc_4.f90: New.

Added:
    trunk/gcc/testsuite/gfortran.dg/move_alloc_4.f90
Modified:
    trunk/gcc/fortran/trans-intrinsic.c

[Bug fortran/48700] memory leak with MOVE_ALLOC

[janus at gcc dot gnu.org]

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

--- Comment #8 from janus at gcc dot gnu.org 2011-05-18 19:23:01 UTC ---
r173874 should fix the issue mentioned in comment #2, but the problem with
allocatable arrays in comment #5 is still present.

[Bug fortran/48700] memory leak with MOVE_ALLOC

[burnus at gcc dot gnu.org]

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

--- Comment #9 from Tobias Burnus <burnus at gcc dot gnu.org> 2011-11-29 09:57:45 UTC ---
Author: burnus
Date: Tue Nov 29 09:57:40 2011
New Revision: 181801

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=181801
Log:
2011-11-29  Tobias Burnus  <burnus@net-b.de>

        PR fortran/51306
        PR fortran/48700
        * check.c (gfc_check_move_alloc): Make sure that from/to
        are both polymorphic or neither.
        * trans-intrinsic.c (conv_intrinsic_move_alloc): Cleanup,
        generate inline code.

2011-11-29  Tobias Burnus  <burnus@net-b.de>

        PR fortran/51306
        PR fortran/48700
        * gfortran.dg/move_alloc_5.f90: Add dg-error.
        * gfortran.dg/select_type_23.f03: Add dg-error.
        * gfortran.dg/move_alloc_6.f90: New.
        * gfortran.dg/move_alloc_7.f90: New.


Added:
    trunk/gcc/testsuite/gfortran.dg/move_alloc_6.f90
    trunk/gcc/testsuite/gfortran.dg/move_alloc_7.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/check.c
    trunk/gcc/fortran/trans-intrinsic.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gfortran.dg/move_alloc_5.f90
    trunk/gcc/testsuite/gfortran.dg/select_type_23.f03

[Bug fortran/48700] memory leak with MOVE_ALLOC

[burnus at gcc dot gnu.org]

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

Tobias Burnus <burnus at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
                 CC|                            |burnus at gcc dot gnu.org
         Resolution|                            |FIXED

--- Comment #10 from Tobias Burnus <burnus at gcc dot gnu.org> 2011-11-29 10:01:39 UTC ---
FIXED on the trunk for 4.7.

The just committed patch fixes memory leaking issues for arrays by calling the
deallocation function within the compiler. With the current implementation, for
polymorphic scalars and arrays only the allocatable components of the declared
type are freed. That issue remains and is tracked in PR 46321.