Bug 48462

Summary: [4.6/4.7 Regression] realloc on assignment: matmul Segmentation Fault with Allocatable Array
Product: gcc Reporter: ortp21
Component: fortranAssignee: Paul Thomas <pault>
Status: RESOLVED FIXED    
Severity: normal CC: burnus, jakub, ortp21, pault, pcpa
Priority: P4 Keywords: wrong-code
Version: 4.6.0   
Target Milestone: 4.6.1   
Host: Target:
Build: Known to work: 4.5.0
Known to fail: 4.6.1, 4.7.0 Last reconfirmed: 2011-04-05 15:43:17
Bug Depends on: 48360    
Bug Blocks:    
Attachments: A fix for the PR
A more elegant fix

Description ortp21 2011-04-05 15:21:49 UTC
The segmentation fault occurs in the current stable release version of gcc (4.6.0). The release version source code was downloaded from a gcc mirror and configured in the following way:

Target: x86_64-apple-darwin10.6.0
Configured with: ../gcc-4.6.0/configure --prefix=$HOME/gcc/gcc-release --enable-languages=fortran --enable-checking=release --disable-bootstrap
Thread model: posix
gcc version 4.6.0 (GCC) 

Small example:


! matmulTester.f90
program main
implicit none
	integer, parameter :: dp = kind(0.0d0)
	real(kind=dp), allocatable :: a(:,:)
	real(kind=dp), allocatable :: b(:,:)

	allocate(a(3,3))
	allocate(b(3,3))

	a(1,:) = [ 3.11_dp, 3.12_dp, 3.13_dp ]
	a(2,:) = [ 3.21_dp, 3.22_dp, 3.23_dp ]
	a(3,:) = [ 3.31_dp, 3.32_dp, 3.33_dp ]

	b = 6.0_dp * a

	a = matmul( matmul( a, b ), b ) ! Segmentation Fault
end program main


The segmentation fault occurs on the last line of the program above. It can be fixed by separating the lines:

a = matmul(a,b)
a = matmul(a,b)

The segmentation fault also disappears, and the program runs fine, if non-allocatable arrays are used for a and b. In addition, replacing the last line of the program with the following removes the segmentation fault:

b = matmul( matmul( a, b ), b ) ! NO Segmentation Fault

Also, the segmentation fault disappears when another allocatable array is introduced to the code above:

real(kind=dp), allocatable :: c(:,:)
allocate(3,3)
...
c = matmul( matmul( a, b ), b ) ! NO Segmentation Fault

I used the following command:

gfortran -o matmulTester matmulTester.f90

Finally, building in debug mode and running yields the following:

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_INVALID_ADDRESS at address: 0x0000000000000000
0x000000010008b3ab in __gfortran_matmul_r8 (retarray=<value temporarily unavailable, due to optimizations>, a=<value temporarily unavailable, due to optimizations>, b=<value temporarily unavailable, due to optimizations>, try_blas=<value temporarily unavailable, due to optimizations>, blas_limit=24, gemm=0x18) at ../../../gcc-4.6.0/libgfortran/generated/matmul_r8.c:284
284			  dest_y[x] += abase_n[x] * bbase_yn;


I hope this is not a duplicate and was a worthwhile reporting. If not, I apologize.

Thanks,
John N.
Comment 1 Joost VandeVondele 2011-04-05 15:38:21 UTC
so, the segfault is at run time. 4.5 is doing fine.

==14586== Invalid read of size 8
==14586==    at 0x4EC9AD1: _gfortran_matmul_r8 (matmul_r8.c:284)
==14586==    by 0x400877: main (in /data03/vondele/bugs/a.out)
==14586==  Address 0x0 is not stack'd, malloc'd or (recently) free'd

looks like an important issue to me.
Comment 2 Joost VandeVondele 2011-04-05 15:40:41 UTC
somewhat reduced:

program main
implicit none
    integer, parameter :: dp = kind(0.0d0)
    real(kind=dp), allocatable :: a(:,:)
    real(kind=dp), allocatable :: b(:,:)
    allocate(a(3,3))
    allocate(b(3,3))
    a = matmul( matmul( a, b ), b ) ! Segmentation Fault
end program main
Comment 3 Tobias Burnus 2011-04-05 15:57:52 UTC
Work around: -fno-realloc-lhs
 
See also other (re)allocation on assignment related bugs: PR 48360 and 48456.
Comment 4 Paul Thomas 2011-04-08 04:23:05 UTC
This should be easy. The only difference between default (failing) and -fno-realloc-lhs is a chunk at the beginning of the assignment:

          {
            void * D.1571;

            D.1571 = (void *) a.data;
            if (D.1571 != 0B)
              {
                __builtin_free (D.1571);
              }
          }
          a.data = 0B;
          a.dtype = 538;

which sort of does a job on 'a'.  I say that it is easy because there are not many sections in trans-xxx that hide behind the -frealloc-lhs condition.

It's obviously mine.

Paul
Comment 5 Paul Thomas 2011-04-10 18:48:40 UTC
(In reply to comment #4)
> This should be easy. The only difference between default (failing) and

....snip....

> which sort of does a job on 'a'.  I say that it is easy because there are not
> many sections in trans-xxx that hide behind the -frealloc-lhs condition.

The problem is with trans-expr.c (realloc_lhs_bounds_for_intrinsic_call), where the result data is freed, in order to signal to the library that an allocation is needed.

The easiest solution would be to use arrayfunc_assign_needs_temporary to force temporary creation, if the function is intrinsic and the lhs variable appears anywhere in the rhs.  I'll put thinking hat on to see if something more elegant comes to mind.

Paul
Comment 6 Paul Thomas 2011-04-15 20:40:58 UTC
Created attachment 24008 [details]
A fix for the PR

This fixes the PR and bootstraps/regtests on FC9/x86_64.

I will continue to look for something more elegant but I think that is is possible the best that can be done whilst using the library to do the allocation.

Paul
Comment 7 Paul Thomas 2011-04-16 21:25:55 UTC
Created attachment 24014 [details]
A more elegant fix

This bootstraps and regtests on FC9/x86_64.

A testcase has been developed.  Will submit tomorrow.

Paul
Comment 8 Paulo C├ęsar Pereira de Andrade 2011-04-17 17:54:26 UTC
I am using the suggested solution in the Mandriva
gcc-4.6.0 package, to correct a bug report from
an user at

https://qa.mandriva.com/show_bug.cgi?id=63047

with the test case:

-%<-
program segmatmul
  
  implicit none

  integer :: i,j
  integer :: nsize
  real, dimension(:,:), allocatable :: matrixA, matrixB
  
  nsize=10

  allocate(matrixA(nsize,nsize),matrixB(nsize,nsize))

  do i=1,nsize
     do j=1,nsize
        matrixA = real(i+j)
        matrixB = real(i-j)
     enddo
  enddo

!segfaults
  matrixA = matmul(matmul(transpose(matrixB),matrixA),matrixB)

 

  deallocate(matrixA, matrixB)

end program segmatmul
-%<-
and compiled as:

$ gfortran -g segmatmul.f90 -o segmatmul.bin

that would cause a segfault due to matrixA being an
argument and also output value of the call.

Adding -std=f95 appeared to correct the issue in
the above sample.
Comment 9 Paul Thomas 2011-04-18 05:07:44 UTC
Author: pault
Date: Mon Apr 18 05:07:38 2011
New Revision: 172636

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=172636
Log:
2011-04-18  Paul Thomas  <pault@gcc.gnu.org>

	PR fortran/48462
	* trans-expr.c (fcncall_realloc_result): Renamed version of
	realloc_lhs_bounds_for_intrinsic_call that does not touch the
	descriptor bounds anymore but makes a temporary descriptor to
	hold the result.
	(gfc_trans_arrayfunc_assign): Modify the reference to above
	renamed function.

2011-04-18  Paul Thomas  <pault@gcc.gnu.org>

	PR fortran/48462
	* gfortran.dg/realloc_on_assign_7.f03: New test.

Added:
    trunk/gcc/testsuite/gfortran.dg/realloc_on_assign_7.f03
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/trans-expr.c
    trunk/gcc/testsuite/ChangeLog
Comment 10 Tobias Burnus 2011-04-19 16:32:41 UTC
Note: The commit from comment 9 fixes only the issue for non-TARGET LHS.

If the LHS has the TARGET attribute (and only* then) a version with an additional temporary has to be used to make sure that associated pointers still work; the standard mandates this for the case that the shape of the RHS is the same as the one on the LHS.

(* the reason of the only is performance not semantics)
Comment 11 Paul Thomas 2011-04-29 20:26:59 UTC
Author: pault
Date: Fri Apr 29 20:26:56 2011
New Revision: 173185

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=173185
Log:
2011-04-29  Paul Thomas  <pault@gcc.gnu.org>

	PR fortran/48462
	* trans-expr.c (arrayfunc_assign_needs_temporary): Deal with
	automatic reallocation when the lhs is a target.

	PR fortran/48746
	* trans-expr.c (fcncall_realloc_result): Make sure that the
	result dtype field is set before the function call.

2011-04-29  Paul Thomas  <pault@gcc.gnu.org>

	PR fortran/48462
	* gfortran.dg/realloc_on_assign_7.f03: Modify to test for lhs
	being a target.

	PR fortran/48746
	* gfortran.dg/realloc_on_assign_7.f03: Add subroutine pr48746.

Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/trans-expr.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gfortran.dg/realloc_on_assign_7.f03
Comment 12 Paul Thomas 2011-04-30 12:00:53 UTC
Author: pault
Date: Sat Apr 30 12:00:50 2011
New Revision: 173214

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=173214
Log:
2011-04-30  Paul Thomas  <pault@gcc.gnu.org>

	PR fortran/48462
	PR fortran/48746
	* trans-expr.c ( arrayfunc_assign_needs_temporary): Need a temp
	if automatic reallocation on assignement is active, the lhs is a
	target and the rhs an intrinsic function.
	(realloc_lhs_bounds_for_intrinsic_call): Rename as next.
	(fcncall_realloc_result): Renamed version of above function.
	Free the original descriptor data after the function call.Set the bounds and the
	offset so that the lbounds are one.
	(gfc_trans_arrayfunc_assign): Call renamed function.

2011-04-30  Paul Thomas  <pault@gcc.gnu.org>

	PR fortran/48462
	PR fortran/48746
	* gfortran.dg/realloc_on_assign_7.f03: New test.


Added:
    branches/gcc-4_6-branch/gcc/testsuite/gfortran.dg/realloc_on_assign_7.f03
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
Comment 13 Paul Thomas 2011-04-30 12:05:23 UTC
Fixed on trunk at 4.6

Thanks for the report.

Paul