[Bug fortran/52151] New: Segfault with realloc on assignment and RESHAPE to unallocated LHS

[Bug fortran/52151] New: Segfault with realloc on assignment and RESHAPE to unallocated LHS

[burnus at gcc dot gnu.org]

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

             Bug #: 52151
           Summary: Segfault with realloc on assignment and RESHAPE to
                    unallocated LHS
    Classification: Unclassified
           Product: gcc
           Version: 4.7.0
            Status: UNCONFIRMED
          Keywords: wrong-code
          Severity: normal
          Priority: P3
         Component: fortran
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: burnus@gcc.gnu.org
                CC: pault@gcc.gnu.org


Follow up to PR 52012 and 52117.

* B is allocated with the shape (n1,n2,n3):
  -> Works since the patch for PR 52012
  (Before, it had the wrong result for the last two elements)

* B is not allocated
  -> Segfault since the patch for PR 52012
  -> Before: no segfault but wrong result in for the last two elements

* B allocated with shape (1,1,1)
  -> Since patch for PR 52012: No crash, but wrong result ("2.0" and 3.0 appear
     multiple times, 5., 6., 7., 8. are missing and 4. is at the wrong
position)
  -> Before: no segfault but wrong result in for the last two elements

The test case has been taken from PR 52117 with adaption to show this bug.
chapter08/puppeteer_f2003 of Damian's/Xia's/Xu's book is also affected.


I think there might be an ordering issue with regards to the zero condition and
the bounds condition. Something like the following might work (completely
untested; note also the "ORIF"):

--- trans-expr.c        (revision 183971)
+++ trans-expr.c        (working copy)
@@ -6296 +6296 @@ fcncall_realloc_result (gfc_se *se, int
-  tree zero_cond;
+  tree zero_cond, cond;
@@ -6324,0 +6325 @@ fcncall_realloc_result (gfc_se *se, int
+  cond = boolean_true_node;
@@ -6344 +6345 @@ fcncall_realloc_result (gfc_se *se, int
-                                  zero_cond);
+                                  cond);
@@ -6345,0 +6347,4 @@ fcncall_realloc_result (gfc_se *se, int
+
+   zero_cond = fold_build2_loc (input_location, TRUTH_ORIF_EXPR,
+                               boolean_type_node, zero_cond,
+                               cond);



! Based on the example of PR 52117 by Steven Hirshman
      PROGRAM RESHAPEIT
      INTEGER, PARAMETER :: n1=2, n2=2, n3=2
      INTEGER            :: m1, m2, m3, lc
      REAL, ALLOCATABLE  :: A(:,:), B(:,:,:)
      REAL               :: val

      ALLOCATE (A(n1,n2*n3))
! <<< No allocation: Segfault
!      ALLOCATE (B(1,1,1))    ! << Does not segfault, but result is wrong
!      ALLOCATE (B(n1,n2,n3)) ! << WORKS

      val = 0
      lc = 0
      DO m3=1,n3
         DO m2=1,n2
            lc = lc+1
            DO m1=1,n1
               val = val+1
               A(m1, lc) = val
            END DO
         END DO
      END DO

      B = RESHAPE(A, [n1,n2,n3])

      lc = 0
      DO m3=1,n3
         DO m2=1,n2
            lc = lc+1
            DO m1=1,n1
               PRINT *,'A(',m1,',',lc,') = ',A(m1,lc),' B = ',B(m1,m2,m3)
               if (A(m1,lc) /= B(m1,m2,m3)) call abort ()
            END DO
         END DO
      END DO
      DEALLOCATE(A, B)
      END PROGRAM RESHAPEIT

[Bug fortran/52151] Segfault with realloc on assignment and RESHAPE to unallocated LHS

[burnus at gcc dot gnu.org]

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

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

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

--- Comment #1 from Tobias Burnus <burnus at gcc dot gnu.org> 2012-02-07 18:03:13 UTC ---
(In reply to comment #0)
> I think there might be an ordering issue with regards to the zero condition
> and the bounds condition. Something like the following might work.

Of course, it doesn't. Additionally: Don't forget about the bounds issue, which
is unrelated to the segfault.

[Bug fortran/52151] Segfault with realloc on assignment and RESHAPE to unallocated LHS

[dominiq at lps dot ens.fr]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2012-02-07
     Ever Confirmed|0                           |1

--- Comment #2 from Dominique d'Humieres <dominiq at lps dot ens.fr> 2012-02-07 21:28:15 UTC ---
> * B is not allocated
>   -> Segfault since the patch for PR 52012

With -O, I don't get the segfault, but the following wrong result:

 A(           1 ,           1 ) =    1.00000000      B =    1.00000000    
 A(           2 ,           1 ) =    2.00000000      B =    2.00000000    
 A(           1 ,           2 ) =    3.00000000      B =    1.00000000    
 A(           2 ,           2 ) =    4.00000000      B =    2.00000000    
 A(           1 ,           3 ) =    5.00000000      B =    1.00000000    
 A(           2 ,           3 ) =    6.00000000      B =    2.00000000    
 A(           1 ,           4 ) =    7.00000000      B =    1.00000000    
 A(           2 ,           4 ) =    8.00000000      B =    2.00000000

[Bug fortran/52151] Segfault with realloc on assignment and RESHAPE to unallocated LHS

[burnus at gcc dot gnu.org]

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

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

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

--- Comment #3 from Tobias Burnus <burnus at gcc dot gnu.org> 2012-02-08 16:48:42 UTC ---
Created attachment 26617
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=26617
Draft patch

Regarding my suggested patch from comment 0: The patch is not needed as one
has:

D.1897 = ((b.dim[0].lbound - D.1894.dim[0].lbound) - b.dim[0].ubound)
                           + D.1894.dim[0].ubound != 0;

If "b" is not allocated, doing this arithmetic does not make sense, but it is
nevertheless valid. The only issue in which one can run is one overflows (for
random/uninitialized data). While that's not a good thing to happen, it should
not matter. (The only potential issue I could see is -ftrapv or something like
that. However, my impression is that integer overflow trapping doesn't work at
all.)

 * * *

Better patch: Taking the stride from the LHS is not helpful, if it is not
allocated or has the wrong bounds. Take it from the result variable instead. We
do not have to take care of any stride in the Fortran sense as the LHS is
allocatable (i.e. contiguous); hence, the only stride is due to the internal
implementation and due to the shape. But that stride is already correct in the
result variable.
Additionally, it is useful to not only read the correct stride but also to set
the stride ...

[Bug fortran/52151] Segfault with realloc on assignment and RESHAPE to unallocated LHS

[burnus at gcc dot gnu.org]

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

--- Comment #4 from Tobias Burnus <burnus at gcc dot gnu.org> 2012-02-08 19:40:31 UTC ---
Author: burnus
Date: Wed Feb  8 19:40:23 2012
New Revision: 184016

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

        PR fortran/52151
        * trans-expr.c (fcncall_realloc_result): Set also the stride.

2012-02-08  Tobias Burnus  <burnus@net-b.de>

        PR fortran/52151
        * gfortran.dg/realloc_on_assign_12.f90: New.


Added:
    trunk/gcc/testsuite/gfortran.dg/realloc_on_assign_12.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/trans-expr.c
    trunk/gcc/testsuite/ChangeLog

[Bug fortran/52151] Segfault with realloc on assignment and RESHAPE to unallocated LHS

[burnus at gcc dot gnu.org]

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

--- Comment #5 from Tobias Burnus <burnus at gcc dot gnu.org> 2012-02-08 21:29:52 UTC ---
Author: burnus
Date: Wed Feb  8 21:29:45 2012
New Revision: 184020

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

        PR fortran/52151
        * trans-expr.c (fcncall_realloc_result): Set also the stride.

2012-02-08  Tobias Burnus  <burnus@net-b.de>

        PR fortran/52151
        * gfortran.dg/realloc_on_assign_12.f90: New.


Added:
    branches/gcc-4_6-branch/gcc/testsuite/gfortran.dg/realloc_on_assign_12.f90
Modified:
    branches/gcc-4_6-branch/gcc/fortran/ChangeLog
    branches/gcc-4_6-branch/gcc/fortran/trans-expr.c
    branches/gcc-4_6-branch/gcc/testsuite/ChangeLog

[Bug fortran/52151] Segfault with realloc on assignment and RESHAPE to unallocated LHS

[burnus at gcc dot gnu.org]

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

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

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

--- Comment #6 from Tobias Burnus <burnus at gcc dot gnu.org> 2012-02-08 21:31:18 UTC ---
FIXED on the trunk (4.7) and on the 4.6 branch.

[Bug fortran/52151] Segfault with realloc on assignment and RESHAPE to unallocated LHS

[burnus at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |therobbot at gmail dot com

--- Comment #7 from Tobias Burnus <burnus at gcc dot gnu.org> 2012-02-28 16:25:26 UTC ---
*** Bug 52418 has been marked as a duplicate of this bug. ***