Bug 47674 - gfortran.dg/realloc_on_assign_5.f03: Segfault at run time for deferred (allocatable) string length
Summary: gfortran.dg/realloc_on_assign_5.f03: Segfault at run time for deferred (alloc...
Alias: None
Product: gcc
Classification: Unclassified
Component: fortran (show other bugs)
Version: 4.6.0
: P3 normal
Target Milestone: ---
Assignee: Thomas Koenig
Keywords: wrong-code
: 55484 56594 60542 (view as bug list)
Depends on:
Blocks: 45170
  Show dependency treegraph
Reported: 2011-02-10 07:03 UTC by Tobias Burnus
Modified: 2015-06-04 09:28 UTC (History)
5 users (show)

See Also:
Known to work:
Known to fail:
Last reconfirmed: 2013-06-16 00:00:00


Note You need to log in before you can comment on or make changes to this bug.
Description Tobias Burnus 2011-02-10 07:03:03 UTC
gfortran.dg/realloc_on_assign_5.f03 segfaults here; it works if I unset the environment variable MALLOC_CHECK_.

Valgrind shows:

Invalid read of size 1
   at 0x4C285C8: memmove (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
   by 0x400B41: MAIN__ (realloc_on_assign_5.f03:15)
   by 0x400BF7: main (realloc_on_assign_5.f03:18)
Address 0x5b524c1 is 0 bytes after a block of size 1 alloc'd
   at 0x4C26682: realloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
   by 0x400B0C: MAIN__ (realloc_on_assign_5.f03:15)
   by 0x400BF7: main (realloc_on_assign_5.f03:18)

Excerpt from the test:
  a = 'ab'   ! OK
  a = (a(2:2)) ! seems to fail
  print '(">",a,"<")', a
prints ">", STX (start of text character), "<" and a new line. Without MALLOC_CHECK_ the desired ">b<" and a new line is printed.

DUMP: The first "if" does not make sense - at least in this special case - and there is the issue that "a" is also used on the RHS without using a temporary.

        D.1531 = .a;
        if (D.1531 != 0)
            if ((<unnamed-unsigned:64>) D.1531 <= 1)
                __builtin_memmove (a, &(*a)[2]{lb: 1 sz: 1}, D.1531);
                __builtin_memcpy (a, &(*a)[2]{lb: 1 sz: 1}, 1);
                __builtin_memset (a + 1, 32, D.1531 + 0x0ffffffffffffffff);
Comment 1 Tobias Burnus 2011-02-12 19:24:21 UTC
The file was added for PR 47523.

The issue is that there is no temporary generated - but the LHS is dependent on the LHS. This should only occur for "LHS = LHS(substring)" where no temporary is needed for the RHS expression but where a reallocation happens. If no reallocation happens, memmove takes care of possibly overlapping memory. And if there is a more complicated expression on the RHS, a temporary should always be present. Thus, the issue should only occur for
  a = a(1:2)
  a = (((( a(1:2) ))))
Comment 2 Tobias Burnus 2011-08-03 07:23:18 UTC
The problem seems to be that for strings, the dependency resolver does not trigger: as there are no strides, certain dependencies are already handled, but it fails if the LHS/RHS variables are the same and LHS is reallocated because of a different (shorter) string length.

dependency.c's gfc_dep_resolver has:

        case REF_SUBSTRING:
          /* Substring overlaps are handled by the string assignment code
             if there is not an underlying dependency.  */
          return (fin_dep == GFC_DEP_OVERLAP) ? 1 : 0;

which returns 0 for:

  string = string(1:2) ! Issue: Realloc without temporary

trans-expr.c's alloc_scalar_allocatable_for_assignment handles scalars, where the value might bet wrong.

A similar issue exists for arrays, though here there are provisions for adding temporaries. (Cf. also trans-array.c's gfc_alloc_allocatable_for_assignment.)

I am not quite sure whether which if any part should be handled in the depenedency analysis and which in the assignment code. At least the scalar assignment code does not seem provide for temporaries at all.

A related issue is PR 49954: ICE with concatenated array strings.
Comment 3 Thomas Koenig 2013-04-08 20:25:49 UTC
*** Bug 56594 has been marked as a duplicate of this bug. ***
Comment 4 Dominique d'Humieres 2013-06-16 20:39:35 UTC
Comment 5 Dominique d'Humieres 2013-09-02 11:17:56 UTC
*** Bug 55484 has been marked as a duplicate of this bug. ***
Comment 6 Dominique d'Humieres 2013-11-23 14:04:13 UTC
gfortran.dg/realloc_on_assign_5.f03 also fails at run time when compiled with  -fsanitize=address.
Comment 7 Thomas Koenig 2014-03-16 10:09:28 UTC
*** Bug 60542 has been marked as a duplicate of this bug. ***
Comment 8 Bernd Edlinger 2014-09-28 21:48:10 UTC

I just noticed, that it also fails if MALLOC_CHECK_ is defined,
although MALLOC_PERTURB_=237 without MALLOC_CHECK_ does nothing.

$ gfortran realloc_on_assign_5.f03
$ MALLOC_CHECK_=3 ./a.out 

Program aborted. Backtrace:
#0  0x7F46086BB307
#1  0x7F46086BC9E2
#2  0x7F460878E6D8
#3  0x400C0B in MAIN_
Comment 9 Thomas Koenig 2014-12-29 17:17:34 UTC
I have a patch (not a pretty one, though).
Comment 10 Thomas Koenig 2015-01-05 17:15:49 UTC
Author: tkoenig
Date: Mon Jan  5 17:15:17 2015
New Revision: 219193

URL: https://gcc.gnu.org/viewcvs?rev=219193&root=gcc&view=rev
2015-01-05  Thomas Koenig  <tkoenig@gcc.gnu.org>

	PR fortran/47674
	* dependency.c:  Update copyright years.
	(gfc_discard_nops):  Add prototype.
	* dependency.c (discard_nops):  Rename to gfc_discard_nops,
	make non-static.
	(gfc_discard_nops):  Use gfc_discard_nops.
	(gfc_dep_difference):  Likewise.
	* frontend-passes.c  Update copyright years.
	(realloc_strings):  New function.  Add prototype.
	(gfc_run_passes):  Call realloc_strings.
	(realloc_string_callback):  New function.
	(create_var):  Add prototype.  Handle case of a
	scalar character variable.
	(optimize_trim):  Do not handle allocatable variables.

2015-01-05  Thomas Koenig  <tkoenig@gcc.gnu.org>

	PR fortran/47674
	* gfortran.dg/realloc_on_assign_25.f90:  New test.

Comment 11 Thomas Koenig 2015-01-05 19:21:45 UTC
Author: tkoenig
Date: Mon Jan  5 19:21:12 2015
New Revision: 219195

URL: https://gcc.gnu.org/viewcvs?rev=219195&root=gcc&view=rev
2015-01-05  Thomas Koenig  <tkoenig@gcc.gnu.org>

	PR fortran/47674
	* dependency.h:  Actually commit changes.

Comment 12 Thomas Koenig 2015-05-16 06:04:36 UTC
Is there interesting in further backporting?

If not, I would close this as fixed.
Comment 13 Thomas Koenig 2015-06-04 09:28:41 UTC
Fixed, as nobody expressed interest in further backporting.