On Thu, May 31, 2012 at 12:28 PM, Tobias Burnus <bur...@net-b.de> wrote:
> Dear all,
>
> gfortran was producing the following code:
>
> res = realloc (mem, size)
> if (size == 0)
> res = NULL;
> ...
> if (res != 0)
> free (res)
>
> The problem is that "realloc (..., 0)" does not have to produce a NULL
> pointer as result.
>
>
> While in "valgrind" one only gets the warning that "0 bytes in <n> blocks"
> where not freed, looking at "top", one sees an excessive use of memory.
>
> That seems to cause crashes in CP2K. At least the valgrind issue is a GCC
> 4.3 to 4.8 regression.
>
>
> Without the patch, gfortran produces:
> --------------------------------------------------------------------
> 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;
> }
> atmp.0.data = (void * restrict) D.1888;
> --------------------------------------------------------------------
>
>
> I see two possibilities:
>
>
> 1) FIRST approach: manual freeing/NULL setting; that's the closed what is
> currently done. (Note: It calls free unconditionally; "free(0)" is optimized
> away by the middle end.
> (Note: That requires an update of the "free" count in
> gfortran.dg/allocatable_function_4.f90)
> --------------------------------------------------------------------
> if (D.1887 == 0)
> {
> __builtin_free ((void *) atmp.0.data);
> D.1888 = 0B;
> }
> else
> {
> 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});
> }
> }
> atmp.0.data = (void * restrict) D.1888;
> --------------------------------------------------------------------
>
>
> 2) SECOND approach: Simply removing the NULL setting and just using the
> result which realloc returns; if not NULL, the free() will properly handle
> it.
> --------------------------------------------------------------------
> 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});
> }
> atmp.0.data = (void * restrict) D.1888;
> --------------------------------------------------------------------
>
>
> Both patches have been tested with Joost's example and the test suite.
> Which version do you prefer? OK for the trunk?
>
> I like the second version more. (And I would even consider to get rid of the
> os_error.)
I prefer the second approach as well, Ok for trunk.
--
Janne Blomqvist
