Bug 27470 - [4.1 regression] wrong memory allocator for derived types
Summary: [4.1 regression] wrong memory allocator for derived types
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: fortran (show other bugs)
Version: 4.2.0
: P5 normal
Target Milestone: 4.1.2
Assignee: Thomas Koenig
URL:
Keywords: wrong-code
Depends on: 27411
Blocks:
  Show dependency treegraph
 
Reported: 2006-05-07 09:41 UTC by Thomas Koenig
Modified: 2006-06-28 18:02 UTC (History)
5 users (show)

See Also:
Host:
Target:
Build:
Known to work: 4.2.0
Known to fail: 4.1.1
Last reconfirmed:


Attachments
patch (602 bytes, patch)
2006-05-08 21:59 UTC, Thomas Koenig
Details | Diff
better patch (622 bytes, patch)
2006-05-08 22:11 UTC, Thomas Koenig
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Thomas Koenig 2006-05-07 09:41:02 UTC
The following code

 TYPE foo
   INTEGER, DIMENSION(:), POINTER :: array
 END TYPE foo

 type(foo),allocatable,dimension(:) :: mol
 integer :: i, n

 n = 10
 ALLOCATE (mol(n))

 
 ALLOCATE (mol(1)%array(5))

 END

calls _gfortran_allocate_array for the second allocation, which
is wrong and can lead to segfault.

The problem is in gfc_array_allocate, where the check for

   expr->symtree->n.sym->attr.allocatable

refers to mol, not to mol(1)%array.

This one is tricky.
Comment 1 Andrew Pinski 2006-05-07 17:31:19 UTC
I wonder how related this is to PR 27411.
Comment 2 Thomas Koenig 2006-05-07 19:36:42 UTC
(In reply to comment #1)
> I wonder how related this is to PR 27411.

As far as I can see, there's no relationship.  To my eye, this looks
like a stand-alone bug :-)

Summary of what goes wrong:

gfc_array_allocate needs to differentiate between pointer and
allocatable arrays because it needs to call _gfortran_allocate_array for
allocatable arrays and _gfortran_allocate for pointers.  The difference
is that, for an allocatable array, it is an error if the allocatable
array is already allocated.  For pointers, it's ok (it potentially
leaks memory, though).

The test wether this is an allocatable array is done via

expr->symtree->n.sym->attr.allocatable, which is fine for non-derived
types.  The test case exposes a problem, that "mol" is allocatable,
mol%array is a pointer, and what we get via expr->symtree->... is
the array and not the pointer.  Apparently, we need to look somewhere
else for that information, but I haven't found out where yet.

Here is a test case which actually provokes a runtime error:


  TYPE foo
    INTEGER, DIMENSION(:), POINTER :: array
  END TYPE foo

  type(foo),allocatable,dimension(:) :: mol

  ALLOCATE (mol(1))
  mol = transfer(-1, mol)  ! Initialize the array with garbage
  ALLOCATE (mol(1)%array(5)) ! Fails with "Attempting to allocate already allocated array"

  END
Comment 3 Thomas Koenig 2006-05-08 21:06:20 UTC
Here's a test case that does not require transfer:


  TYPE foo
    INTEGER, DIMENSION(:), POINTER :: array
  END TYPE foo

  type(foo),allocatable,dimension(:) :: mol

  ALLOCATE (mol(1))
  ALLOCATE (mol(1)%array(5))
  ALLOCATE (mol(1)%array(5))

  END

This shouldn't error out with "Attempting to allocate already allocated array".
Comment 4 Thomas Koenig 2006-05-08 21:59:55 UTC
Created attachment 11415 [details]
patch

This fixes the regression.  It isn't pretty, because it would need
to be changed as part of an implementation of allocatable components.
Comment 5 Thomas Koenig 2006-05-08 22:11:14 UTC
Created attachment 11416 [details]
better patch

Forgot to initialize a variable in the earlier attempt.

This one looks OK.
Comment 6 Thomas Koenig 2006-05-10 18:27:00 UTC
Subject: Bug 27470

Author: tkoenig
Date: Wed May 10 18:26:51 2006
New Revision: 113680

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=113680
Log:
2005-05-10  Thomas Koenig  <Thomas.Koenig@online.de>

	PR fortran/27470
	* trans-array.c(gfc_array_allocate):  If ref->next exists
	that is if there is a statement like ALLOCATE(foo%bar(2)),
	F95 rules require that bar should be a pointer.

2005-05-10  Thomas Koenig  <Thomas.Koenig@online.de>

	PR fortran/27470
	* gfortran.dg/multiple_allocation_2.f90:  New test case.


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

Comment 7 Mark Mitchell 2006-05-14 22:34:21 UTC
P5: F95 is not release-critical.
Comment 8 patchapp@dberlin.org 2006-05-15 19:38:52 UTC
Subject: Bug number PR 27470

A patch for this bug has been added to the patch tracker.
The mailing list url for the patch is http://gcc.gnu.org/ml/gcc-patches/2006-05/msg00365.html
Comment 9 Mark Mitchell 2006-05-25 02:36:48 UTC
Will not be fixed in 4.1.1; adjust target milestone to 4.1.2.
Comment 10 Thomas Koenig 2006-05-27 21:25:35 UTC
I applied the patch to the 4.1 branch, and I have a strange "regression":

When running "make check-fortran", I get

Running target unix
Using /usr/share/dejagnu/baseboards/unix.exp as board description file for target.
Using /usr/share/dejagnu/config/unix.exp as generic interface file for target.
Using /home/ig25/gcc/branches/gcc-4_1-branch/gcc/testsuite/config/default.exp as tool-and-target-specific interface file.
Running /home/ig25/gcc/branches/gcc-4_1-branch/gcc/testsuite/gfortran.dg/dg.exp ...
FAIL: gfortran.dg/io_constraints_2.f90  -O   (test for errors, line 61)
FAIL: gfortran.dg/io_constraints_2.f90  -O  (test for excess errors)
got a INT signal, interrupted by user

                === gfortran Summary ===

# of expected passes            6807
# of unexpected failures        2
# of expected failures          11
# of unsupported tests          16

(although it is hard to see how that came about).

When I run just that test case, I get

$ make -k check-gfortran RUNTESTFLAGS="dg.exp=gfortran.dg/io_constraints_2.f90"
test -d testsuite || mkdir testsuite
test -d testsuite/gfortran || mkdir testsuite/gfortran
(rootme=`${PWDCMD-pwd}`; export rootme; \
        srcdir=`cd ../../../../gcc/branches/gcc-4_1-branch/gcc; ${PWDCMD-pwd}` ; export srcdir ; \
        cd testsuite/gfortran; \
        rm -f tmp-site.exp; \
        sed '/set tmpdir/ s|testsuite|testsuite/gfortran|' \
                < ../../site.exp > tmp-site.exp; \
        /bin/sh ${srcdir}/../move-if-change tmp-site.exp site.exp; \
        EXPECT=`if [ -f ${rootme}/../expect/expect ] ; then echo ${rootme}/../expect/expect ; else echo expect ; fi` ; export EXPECT ; \
        if [ -f ${rootme}/../expect/expect ] ; then  \
           TCL_LIBRARY=`cd .. ; cd ${srcdir}/../tcl/library ; ${PWDCMD-pwd}` ; \
            export TCL_LIBRARY ; fi ; \
        `if [ -f ${srcdir}/../dejagnu/runtest ] ; then echo ${srcdir}/../dejagnu/runtest ; else echo runtest; fi` --tool gfortran dg.exp=gfortran.dg/io_constraints_2.f90)
site.exp is unchanged
Test Run By ig25 on Sat May 27 23:22:53 2006
Native configuration is i686-pc-linux-gnu

                === gfortran tests ===

Schedule of variations:
    unix

Running target unix
Using /usr/share/dejagnu/baseboards/unix.exp as board description file for target.
Using /usr/share/dejagnu/config/unix.exp as generic interface file for target.
Using /home/ig25/gcc/branches/gcc-4_1-branch/gcc/testsuite/config/default.exp as tool-and-target-specific interface file.
Running /home/ig25/gcc/branches/gcc-4_1-branch/gcc/testsuite/gfortran.dg/dg.exp ...

                === gfortran Summary ===

# of expected passes            18
/home/ig25/gcc-bin/branches/gcc-4_1-branch/gcc/testsuite/gfortran/../../gfortran  version 4.1.2 20060527 (prerelease)

Not committing right now.
Comment 11 Thomas Koenig 2006-05-28 12:33:11 UTC
(In reply to comment #10)
>
> Not committing right now.

Downloading a fresh version via svn cured that problem.

I'd have commited the patch by now, but for some strange reason,
I get "Authentication failed" errors with svn, and my E-Mails to
the gcc and fortran mailing lists also don't go through.

Comment 12 Thomas Koenig 2006-05-28 20:36:00 UTC
Subject: Bug 27470

Author: tkoenig
Date: Sun May 28 20:35:53 2006
New Revision: 114176

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=114176
Log:
2005-05-28  Thomas Koenig  <Thomas.Koenig@online.de>

	PR fortran/27470
	Backport from trunk.
	* trans-array.c(gfc_array_allocate):  If ref->next exists
	that is if there is a statement like ALLOCATE(foo%bar(2)),
	F95 rules require that bar should be a pointer.

2005-05-28  Thomas Koenig  <Thomas.Koenig@online.de>

	PR fortran/27470
	Backport from trunk.
	* gfortran.dg/multiple_allocation_2.f90:  New test case.


Added:
    branches/gcc-4_1-branch/gcc/testsuite/gfortran.dg/multiple_allocation_2.f90
Modified:
    branches/gcc-4_1-branch/gcc/fortran/ChangeLog
    branches/gcc-4_1-branch/gcc/fortran/trans-array.c
    branches/gcc-4_1-branch/gcc/testsuite/ChangeLog

Comment 13 Thomas Koenig 2006-05-28 20:37:16 UTC
Fixed on trunk and 4.1.

Closing.
Comment 14 Joshua Cogliati 2006-06-28 18:02:40 UTC
This works in 4.1.0, so only 4.1.1 has this bug so far as I can tell.