[Bug fortran/53521] New: Memory leak with zero sized array constructor

[Bug fortran/53521] New: Memory leak with zero sized array constructor

[Joost.VandeVondele at mat dot ethz.ch]

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

             Bug #: 53521
           Summary: Memory leak with zero sized array constructor
    Classification: Unclassified
           Product: gcc
           Version: 4.6.3
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: fortran
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: Joost.VandeVondele@mat.ethz.ch


The following testcase (as reduced from CP2K) leaks memory when compiled with
gfortran 4.6 - 4.8 :

MODULE TEST
 IMPLICIT NONE
CONTAINS
 SUBROUTINE T(n1)
  INTEGER :: n1(:)
 END SUBROUTINE T
 SUBROUTINE T2(n)
   INTEGER :: n
   INTEGER :: k
   CALL T((/(k,k=1,n-1)/))
 END SUBROUTINE
END MODULE
USE TEST
  CALL T2(1)
END 

as can be verified with valgrind or putting a loop around call t2. The issue
seems to be the zero-sized array constructor.

[Bug fortran/53521] [4.3/4.4/4.5/4.6/4.7/4.8 Regression] Zero-byte memory leak with zero-sized array constructor

[burnus at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P5
             Status|UNCONFIRMED                 |NEW
      Known to work|                            |4.1.2
           Keywords|                            |wrong-code
   Last reconfirmed|                            |2012-05-30
                 CC|                            |burnus at gcc dot gnu.org
     Ever Confirmed|0                           |1
            Summary|Memory leak with zero sized |[4.3/4.4/4.5/4.6/4.7/4.8
                   |array constructor           |Regression] Zero-byte
                   |                            |memory leak with zero-sized
                   |                            |array constructor
   Target Milestone|---                         |4.6.4
      Known to fail|                            |4.3.4, 4.4.0, 4.5.3, 4.6.3,
                   |                            |4.7.0, 4.8.0

--- Comment #1 from Tobias Burnus <burnus at gcc dot gnu.org> 2012-05-30 08:30:39 UTC ---
Confirmed. Valgrind shows with GCC 4.1 "no leaks are possible" while for GCC
4.3 to 4.8, it shows:


==25714== HEAP SUMMARY:
==25714==     in use at exit: 0 bytes in 1 blocks

That's not much memory, but I concur that it clutters the valgrind output. With
--leak-check=full:

 0 bytes in 1 blocks are definitely lost in loss record 1 of 1
    at 0x4C2ABED: malloc (in
/usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
    by 0x4C2AD6F: realloc (in
/usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
    by 0x400876: __test_MOD_t2 (test.f90:10)
    by 0x400999: MAIN__ (test.f90:14)
    by 0x4009CF: main (test.f90:13)


>From the dump:

    atmp.0.data = 0B;
      D.1888 = __builtin_realloc ((void *) atmp.0.data, D.1887);
      atmp.0.data = (void * restrict) D.1888;
      ...
      D.1883 = (void *) atmp.0.data;
      if (D.1883 != 0B)
        __builtin_free (D.1883);


One possibility would be to simply remove the "if": First, free also handles
the case of "freeing" NULL, secondly, for temporary variables "NULL" is a rare
(or nonexisting) special case (i.e. it hampers rather than helps optimization),
and, thirdly, this PR shows that also zero-sized NULL-pointers should be freed.

[Bug fortran/53521] [4.5/4.6/4.7/4.8 Regression] Zero-byte memory leak with zero-sized array constructor

[rguenth at gcc dot gnu.org]

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

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[4.3/4.4/4.5/4.6/4.7/4.8    |[4.5/4.6/4.7/4.8
                   |Regression] Zero-byte       |Regression] Zero-byte
                   |memory leak with zero-sized |memory leak with zero-sized
                   |array constructor           |array constructor

--- Comment #2 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-05-30 09:54:31 UTC ---
Well, I think this is a valgrind issue and not a real "leak".  Whether you
want to "optimize" code for the non-NULL case by omitting the NULL check is
another question of course.  It's definitely not wrong-code IMHO.

[Bug fortran/53521] [4.5/4.6/4.7/4.8 Regression] Zero-byte "memory leak" with zero-sized array constructor (valgrind warning)

[Joost.VandeVondele at mat dot ethz.ch]

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

--- Comment #3 from Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> 2012-05-30 12:31:18 UTC ---
(In reply to comment #2)
> Well, I think this is a valgrind issue and not a real "leak".  Whether you
> want to "optimize" code for the non-NULL case by omitting the NULL check is
> another question of course.  It's definitely not wrong-code IMHO.

No, definitely a real bug... not a valgrind issue. If you put a loop around
'CALL T2' the process memory usage is a >1Gb in a few seconds. This is a real
issue which causes our simulation code to crash after 24h of running.

[Bug fortran/53521] [4.5/4.6/4.7/4.8 Regression] Zero-byte "memory leak" with zero-sized array constructor (valgrind warning)

[rguenth at gcc dot gnu.org]

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

--- Comment #4 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-05-30 12:46:53 UTC ---
You say not doing free (0) leaks memory?  What OS is this on?  Note that
we fold such calls away:

    case BUILT_IN_FREE:
      if (integer_zerop (arg0))
        return build_empty_stmt (loc);

[Bug fortran/53521] [4.5/4.6/4.7/4.8 Regression] Zero-byte "memory leak" with zero-sized array constructor (valgrind warning)

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-05-30 13:49:51 UTC ---
That is clearly a bug in the Fortran FE.
      D.1888 = (integer(kind=4)[0] * restrict) __builtin_realloc ((void *)
atmp.0.data, D.1887);
      if (D.1888 == 0B && D.1887 != 0)
        {
          _gfortran_os_error (&"Allocation would exceed memory limit"[1]{lb: 1
sz: 1});
        }
      if (D.1887 == 0)
        {
          D.1888 = 0B;
        }
means that D.1888 isn't freed.  POSIX says that realloc (x, 0) acts as free
only if x is not NULL, realloc (NULL, 0) acts as malloc (0), which can either
return NULL, or a unique pointer that needs to be freed.

[Bug fortran/53521] [4.5/4.6/4.7/4.8 Regression] Zero-byte "memory leak" with zero-sized array constructor (valgrind warning)

[Joost.VandeVondele at mat dot ethz.ch]

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

--- Comment #6 from Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> 2012-05-30 14:37:09 UTC ---
(In reply to comment #4)
> You say not doing free (0) leaks memory?  What OS is this on?  

I'm observing on a Linux box that :

MODULE TEST
 IMPLICIT NONE
CONTAINS
 SUBROUTINE T(n1)
  INTEGER :: n1(:)
 END SUBROUTINE T
 SUBROUTINE T2(n)
   INTEGER :: n
   INTEGER :: k
   CALL T((/(k,k=1,n-1)/))
 END SUBROUTINE
END MODULE
USE TEST
  DO
    CALL T2(1)
  ENDDO
END 

needs 25Gb of memory after a while (notice the endless loop around CALL T2).

[Bug fortran/53521] [4.5/4.6/4.7/4.8 Regression] Memory leak with zero-sized array constructor

[burnus at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |wrong-code
             Status|NEW                         |ASSIGNED
            Summary|[4.5/4.6/4.7/4.8            |[4.5/4.6/4.7/4.8
                   |Regression] Zero-byte       |Regression] Memory leak
                   |"memory leak" with          |with zero-sized array
                   |zero-sized array            |constructor
                   |constructor (valgrind       |
                   |warning)                    |

--- Comment #7 from Tobias Burnus <burnus at gcc dot gnu.org> 2012-05-31 08:12:09 UTC ---
(In reply to comment #6)
> needs 25Gb of memory after a while (notice the endless loop around CALL T2).

I had tried it with a finite loop and valgrind, but the latter then only
showed:
==32236==    definitely lost: 0 bytes in 10,000 blocks

However, I could reproduce it, when looking at the memory consumption in "top".


(In reply to comment #5)
> That is clearly a bug in the Fortran FE.
>       D.1888 = __builtin_realloc (atmp.0.data, D.1887);
...
>       if (D.1887 == 0)
>         D.1888 = 0B;
> means that D.1888 isn't freed.

Well spotted! I got distracted by the valgrind result of 0 bytes - and by the
later FREE check, which should be okay.

The problem is in gcc/fortran/trans.c's gfc_call_realloc.

[Bug fortran/53521] [4.5/4.6/4.7/4.8 Regression] Memory leak with zero-sized array constructor

[jakub at gcc dot gnu.org]

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

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-05-31 08:19:34 UTC ---
(In reply to comment #7)
> (In reply to comment #5)
> > That is clearly a bug in the Fortran FE.
> >       D.1888 = __builtin_realloc (atmp.0.data, D.1887);
> ...
> >       if (D.1887 == 0)
> >         D.1888 = 0B;
> > means that D.1888 isn't freed.
> 
> Well spotted! I got distracted by the valgrind result of 0 bytes - and by the
> later FREE check, which should be okay.
> 
> The problem is in gcc/fortran/trans.c's gfc_call_realloc.

I guess passing MAX (D.1887, 1) as second argument to realloc above and
omitting that if (D.1887 == 0) D.1888 = 0B; stmt would be easiest and hopefully
not much slower (both sequences have one conditional).  Because, I believe you
still have free at the end anyway.

[Bug fortran/53521] [4.5/4.6/4.7/4.8 Regression] Memory leak with zero-sized array constructor

[burnus at gcc dot gnu.org]

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

--- Comment #9 from Tobias Burnus <burnus at gcc dot gnu.org> 2012-05-31 14:28:46 UTC ---
Author: burnus
Date: Thu May 31 14:28:41 2012
New Revision: 188062

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=188062
Log:
2012-05-31  Tobias Burnus  <burnus@net-b.de>

        PR fortran/53521
        * trans.c (gfc_deallocate_scalar_with_status): Properly
        handle the case size == 0.


Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/trans.c

[Bug fortran/53521] [4.5/4.6/4.7 Regression] Memory leak with zero-sized array constructor

[Joost.VandeVondele at mat dot ethz.ch]

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

Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[4.5/4.6/4.7/4.8            |[4.5/4.6/4.7 Regression]
                   |Regression] Memory leak     |Memory leak with zero-sized
                   |with zero-sized array       |array constructor
                   |constructor                 |
      Known to fail|4.8.0                       |

--- Comment #9 from Tobias Burnus <burnus at gcc dot gnu.org> 2012-05-31 14:28:46 UTC ---
Author: burnus
Date: Thu May 31 14:28:41 2012
New Revision: 188062

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=188062
Log:
2012-05-31  Tobias Burnus  <burnus@net-b.de>

        PR fortran/53521
        * trans.c (gfc_deallocate_scalar_with_status): Properly
        handle the case size == 0.


Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/trans.c

--- Comment #10 from Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> 2012-06-01 07:42:41 UTC ---
Thanks Tobias... this fixes the issue I saw for CP2K. From some further tests I
did, I couldn't see any negative side effects.

I had a look at other gcc branches, the patch applies flawlessly to 4.7 and 4.6
(I did not have a 4.5 branch around). I would be very happy to see it
integrated in 4.7.1 and 4.6.4, as it is nearly impossible to fully code around
this in CP2K. Array constructors are used much, and it is hard to guess which
ones could be zero-sized.

[Bug fortran/53521] [4.5/4.6/4.7 Regression] Memory leak with zero-sized array constructor

[burnus at gcc dot gnu.org]

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

--- Comment #11 from Tobias Burnus <burnus at gcc dot gnu.org> 2012-06-01 15:08:29 UTC ---
Author: burnus
Date: Fri Jun  1 15:08:24 2012
New Revision: 188111

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=188111
Log:
2012-06-01  Tobias Burnus  <burnus@net-b.de>

        PR fortran/53521
        * trans.c (gfc_deallocate_scalar_with_status): Properly
        handle the case size == 0.


Modified:
    branches/gcc-4_7-branch/gcc/fortran/ChangeLog
    branches/gcc-4_7-branch/gcc/fortran/trans.c

[Bug fortran/53521] [4.5/4.6/4.7 Regression] Memory leak with zero-sized array constructor

[burnus at gcc dot gnu.org]

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

--- Comment #12 from Tobias Burnus <burnus at gcc dot gnu.org> 2012-06-01 20:06:45 UTC ---
Author: burnus
Date: Fri Jun  1 20:06:39 2012
New Revision: 188124

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=188124
Log:
2012-06-01  Tobias Burnus  <burnus@net-b.de>

        PR fortran/53521
        * trans.c (gfc_deallocate_scalar_with_status): Properly
        handle the case size == 0.


Modified:
    branches/gcc-4_6-branch/gcc/fortran/ChangeLog
    branches/gcc-4_6-branch/gcc/fortran/trans.c

[Bug fortran/53521] [4.5/4.6/4.7 Regression] Memory leak with zero-sized array constructor

[burnus at gcc dot gnu.org]

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

--- Comment #13 from Tobias Burnus <burnus at gcc dot gnu.org> 2012-06-01 20:40:45 UTC ---
Author: burnus
Date: Fri Jun  1 20:40:36 2012
New Revision: 188125

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=188125
Log:
2012-06-01  Tobias Burnus  <burnus@net-b.de>

        PR fortran/53521
        * trans.c (gfc_deallocate_scalar_with_status): Properly
        handle the case size == 0.


Modified:
    branches/gcc-4_5-branch/gcc/fortran/ChangeLog
    branches/gcc-4_5-branch/gcc/fortran/trans.c

[Bug fortran/53521] [4.5/4.6/4.7/4.8 Regression] Memory leak with zero-sized array constructor

[burnus at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED
   Target Milestone|4.6.4                       |4.5.4
            Summary|[4.5/4.6/4.7 Regression]    |[4.5/4.6/4.7/4.8
                   |Memory leak with zero-sized |Regression] Memory leak
                   |array constructor           |with zero-sized array
                   |                            |constructor
      Known to fail|                            |4.8.0

--- Comment #14 from Tobias Burnus <burnus at gcc dot gnu.org> 2012-06-01 21:07:49 UTC ---
FIXED on the trunk and on all still maintained branches: 4.5, 4.6 and 4.7.


(In reply to comment #10)
> this fixes the issue I saw for CP2K. From some further tests I
> did, I couldn't see any negative side effects.

Good to hear.

> I had a look at other gcc branches, the patch applies flawlessly to 4.7 and 4.6
> (I did not have a 4.5 branch around).

The patch didn't apply directly to 4.5, but replicating the code manually was
very simple.

> I would be very happy to see it integrated in 4.7.1 and 4.6.4,

Note that 4.7.1 will be released soon; I think release-candidate build is due
next week.

[Bug fortran/53521] [4.5/4.6/4.7/4.8 Regression] Memory leak with zero-sized array constructor

[dominiq at lps dot ens.fr]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |xarthisius.kk at gmail dot
                   |                            |com

--- Comment #15 from Dominique d'Humieres <dominiq at lps dot ens.fr> 2013-02-27 12:44:31 UTC ---
*** Bug 49241 has been marked as a duplicate of this bug. ***