Bug 48699

Summary: [4.6/4.7 Regression] [OOP] MOVE_ALLOC inside SELECT TYPE
Product: gcc Reporter: Salvatore Filippone <sfilippone>
Component: fortranAssignee: janus
Status: RESOLVED FIXED    
Severity: normal CC: burnus, janus
Priority: P4    
Version: 4.7.0   
Target Milestone: 4.6.1   
Host: Target:
Build: Known to work:
Known to fail: Last reconfirmed: 2011-05-15 11:47:17
Attachments: test-case
test-case

Description Salvatore Filippone 2011-04-20 12:06:39 UTC
Created attachment 24056 [details]
test-case

Hello,
The attached examples fail in rather strange ways, on both current trunk and 4.6.0.

----------------------------------------------
[sfilippo@donald bug31]$ gfortran -v
Using built-in specs.
COLLECT_GCC=gfortran
COLLECT_LTO_WRAPPER=/usr/local/gnu47/libexec/gcc/x86_64-unknown-linux-gnu/4.7.0/lto-wrapper
Target: x86_64-unknown-linux-gnu
Configured with: ../gcc/configure --prefix=/usr/local/gnu47 --enable-languages=c,c++,fortran --with-gmp=/home/travel/GNUBUILD/gmp --with-mpfr=/home/travel/GNUBUILD/mpfr --with-mpc=/home/travel/GNUBUILD/mpc : (reconfigured) ../gcc/configure --prefix=/usr/local/gnu47 --enable-languages=c,c++,fortran --with-gmp=/home/travel/GNUBUILD/gmp --with-mpfr=/home/travel/GNUBUILD/mpfr --with-mpc=/home/travel/GNUBUILD/mpc : (reconfigured) ../gcc/configure --prefix=/usr/local/gnu47 --enable-languages=c,c++,fortran --with-gmp=/home/travel/GNUBUILD/gmp --with-mpfr=/home/travel/GNUBUILD/mpfr --with-mpc=/home/travel/GNUBUILD/mpc
Thread model: posix
gcc version 4.7.0 20110418 (experimental) (GCC) 

[sfilippo@donald bug31]$ gfortran -o testmv1 testmv1.f90
/tmp/ccIl5I0L.o:(.data+0x58): undefined reference to `__copy_foo2_Bar2.1685'
collect2: ld returned 1 exit status

[sfilippo@donald bug31]$ gfortran -c testmv2.f90
testmv2.f90:38.20:

    call move_alloc(sm,dat%sm)
                    1
Error: 'from' argument of 'move_alloc' intrinsic at (1) must be ALLOCATABLE
--------------------------------------------------------------------
The symptoms seem to be about two different things, though.
Comment 1 Salvatore Filippone 2011-04-20 12:07:04 UTC
Created attachment 24057 [details]
test-case
Comment 2 Tobias Burnus 2011-04-20 13:08:39 UTC
See also PR 48700 (memleak with polymorphic vars in MOVE_ALLOC), which might be a duplicate.
Comment 3 Salvatore Filippone 2011-04-20 13:20:48 UTC
(In reply to comment #2)
> See also PR 48700 (memleak with polymorphic vars in MOVE_ALLOC), which might be
> a duplicate.

They are related in the sense that the test cases for this one were obtained while searching for a memleak test case. Whether the root cause is the same is beyond me..
Comment 4 janus 2011-05-15 11:47:17 UTC
(In reply to comment #0)
> [sfilippo@donald bug31]$ gfortran -c testmv2.f90
> testmv2.f90:38.20:
> 
>     call move_alloc(sm,dat%sm)
>                     1
> Error: 'from' argument of 'move_alloc' intrinsic at (1) must be ALLOCATABLE

Here is a reduced test case for this one:


program testmv2

  type bar
    integer, allocatable  :: ia(:), ja(:)
  end type bar

  class(bar), allocatable :: sm,sm2

  allocate(sm2)

  select type(sm2) 
  type is (bar)
    call move_alloc(sm2,sm)
  end select

end program testmv2


In each TYPE IS/CLASS IS block we generate a temporary for the selector. Apparently we need to set the correct attributes for the temporary.
Comment 5 janus 2011-05-15 11:53:00 UTC
(In reply to comment #4)
> In each TYPE IS/CLASS IS block we generate a temporary for the selector.
> Apparently we need to set the correct attributes for the temporary.

cf. select_type_set_tmp (match.c)
Comment 6 janus 2011-05-20 20:17:21 UTC
Patch:

Index: gcc/fortran/match.c
===================================================================
--- gcc/fortran/match.c	(Revision 173965)
+++ gcc/fortran/match.c	(Arbeitskopie)
@@ -4533,7 +4533,11 @@ select_type_set_tmp (gfc_typespec *ts)
   gfc_get_sym_tree (name, gfc_current_ns, &tmp, false);
   gfc_add_type (tmp->n.sym, ts, NULL);
   gfc_set_sym_referenced (tmp->n.sym);
-  gfc_add_pointer (&tmp->n.sym->attr, NULL);
+  if (select_type_stack->selector->ts.type == BT_CLASS &&
+      CLASS_DATA (select_type_stack->selector)->attr.allocatable)
+    gfc_add_allocatable (&tmp->n.sym->attr, NULL);
+  else
+    gfc_add_pointer (&tmp->n.sym->attr, NULL);
   gfc_add_flavor (&tmp->n.sym->attr, FL_VARIABLE, name, NULL);
   if (ts->type == BT_CLASS)
     gfc_build_class_symbol (&tmp->n.sym->ts, &tmp->n.sym->attr,
Comment 7 janus 2011-05-20 21:31:42 UTC
(In reply to comment #6)
> Patch:

... regtests cleanly.
Comment 8 janus 2011-05-21 19:12:56 UTC
Author: janus
Date: Sat May 21 19:12:51 2011
New Revision: 174001

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=174001
Log:
2011-05-21  Janus Weil  <janus@gcc.gnu.org>

	PR fortran/48699
	* match.c (select_type_set_tmp): Make the temporary ALLOCATABLE if the
	selector is ALLOCATABLE.

2011-05-21  Janus Weil  <janus@gcc.gnu.org>

	PR fortran/48699
	* gfortran.dg/select_type_23.f03: New.

Added:
    trunk/gcc/testsuite/gfortran.dg/select_type_23.f03
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/match.c
    trunk/gcc/testsuite/ChangeLog
Comment 9 janus 2011-05-21 19:33:32 UTC
r174001 fixes the test cases in comment #1 and #4.

The test case in comment #0 still fails. Reduced version:


program testmv1

  type bar
  end type

  type, extends(bar) ::  bar2
  end type

  class(bar), allocatable :: sm
  type(bar2), allocatable :: sm2

  allocate(sm2)
  call move_alloc(sm2,sm)

end program


/tmp/ccSfRlZ5.o:(.data+0x38): undefined reference to `__copy_testmv1_Bar2.1582'
Comment 10 janus 2011-05-21 19:37:16 UTC
(In reply to comment #9)
> program testmv1
> 
>   type bar
>   end type
> 
>   type, extends(bar) ::  bar2
>   end type
> 
>   class(bar), allocatable :: sm
>   type(bar2), allocatable :: sm2
> 
>   allocate(sm2)
>   call move_alloc(sm2,sm)
> 
> end program
> 
> 
> /tmp/ccSfRlZ5.o:(.data+0x38): undefined reference to `__copy_testmv1_Bar2.1582'


Btw, this one works with 4.5, but fails with 4.6 and 4.7, which makes it a regresssion.
Comment 11 janus 2011-05-21 20:09:07 UTC
(In reply to comment #9)
> program testmv1
> 
>   type bar
>   end type
> 
>   type, extends(bar) ::  bar2
>   end type
> 
>   class(bar), allocatable :: sm
>   type(bar2), allocatable :: sm2
> 
>   allocate(sm2)
>   call move_alloc(sm2,sm)
> 
> end program
> 
> 
> /tmp/ccSfRlZ5.o:(.data+0x38): undefined reference to `__copy_testmv1_Bar2.1582'

The problem here simply is that the vtab is constructed too late (only at translation stage), so that the __copy routine is never generated. The following obvious patch fixes it:

Index: gcc/fortran/check.c
===================================================================
--- gcc/fortran/check.c	(Revision 174000)
+++ gcc/fortran/check.c	(Arbeitskopie)
@@ -2588,6 +2588,10 @@ gfc_check_move_alloc (gfc_expr *from, gfc_expr *to
       return FAILURE;
     }
 
+  /* Make sure the vtab is present.  */
+  if (to->ts.type == BT_CLASS)
+    gfc_find_derived_vtab (from->ts.u.derived);
+
   return SUCCESS;
 }
Comment 12 Dominique d'Humieres 2011-05-21 23:32:58 UTC
The patch in comment #11 fixes the runtime for the tests in comments #0 and #9. However the other tests give a backtrace on x86_64-apple-darwin10. and valgrind reports errors of the kind

==27268== Invalid read of size 8
==27268==    at 0x100000D48: MAIN__ (pr48699_1.f90:16)
==27268==    by 0x100000DE8: main (pr48699_1.f90:16)
==27268==  Address 0x100442470 is 0 bytes inside a block of size 96 free'd
==27268==    at 0x10001146F: free (vg_replace_malloc.c:366)
==27268==    by 0x100000D34: MAIN__ (pr48699_1.f90:16)
==27268==    by 0x100000DE8: main (pr48699_1.f90:16)
==27268== 
==27268== Invalid write of size 8
==27268==    at 0x100000D69: MAIN__ (pr48699_1.f90:16)
==27268==    by 0x100000DE8: main (pr48699_1.f90:16)
==27268==  Address 0x100442470 is 0 bytes inside a block of size 96 free'd
==27268==    at 0x10001146F: free (vg_replace_malloc.c:366)
==27268==    by 0x100000D34: MAIN__ (pr48699_1.f90:16)
==27268==    by 0x100000DE8: main (pr48699_1.f90:16)
==27268== 
==27268== Invalid read of size 8
==27268==    at 0x100000D77: MAIN__ (pr48699_1.f90:16)
==27268==    by 0x100000DE8: main (pr48699_1.f90:16)
==27268==  Address 0x1004424a0 is 48 bytes inside a block of size 96 free'd
==27268==    at 0x10001146F: free (vg_replace_malloc.c:366)
==27268==    by 0x100000D34: MAIN__ (pr48699_1.f90:16)
==27268==    by 0x100000DE8: main (pr48699_1.f90:16)
==27268== 
==27268== Invalid write of size 8
==27268==    at 0x100000D9A: MAIN__ (pr48699_1.f90:16)
==27268==    by 0x100000DE8: main (pr48699_1.f90:16)
==27268==  Address 0x1004424a0 is 48 bytes inside a block of size 96 free'd
==27268==    at 0x10001146F: free (vg_replace_malloc.c:366)
==27268==    by 0x100000D34: MAIN__ (pr48699_1.f90:16)
==27268==    by 0x100000DE8: main (pr48699_1.f90:16)
==27268== 
==27268== Invalid free() / delete / delete[]
==27268==    at 0x10001146F: free (vg_replace_malloc.c:366)
==27268==    by 0x100000DB0: MAIN__ (pr48699_1.f90:16)
==27268==    by 0x100000DE8: main (pr48699_1.f90:16)
==27268==  Address 0x100442470 is 0 bytes inside a block of size 96 free'd
==27268==    at 0x10001146F: free (vg_replace_malloc.c:366)
==27268==    by 0x100000D34: MAIN__ (pr48699_1.f90:16)
==27268==    by 0x100000DE8: main (pr48699_1.f90:16)

for the test in comment #4.
Comment 13 janus 2011-05-29 21:51:55 UTC
(In reply to comment #12)
> The patch in comment #11 fixes the runtime for the tests in comments #0 and #9.
> However the other tests give a backtrace on x86_64-apple-darwin10. and valgrind
> reports errors of the kind

Apparently that already happens without the patch, with a clean trunk (note that comment #4 is select_type_23.f03 in the test suite, but only "do-compile").

Moreover, I think that this test case is in fact invalid, cf. PR 48887.
Comment 14 Dominique d'Humieres 2011-05-29 22:19:52 UTC
(In reply to comment #13)
> Moreover, I think that this test case is in fact invalid, cf. PR 48887.

I have only reported what I saw. My understanding of the standard past f77 is too shallow to allow me to be confident about the validity or not of these tests.
Comment 15 Salvatore Filippone 2011-05-30 09:11:08 UTC
(In reply to comment #13)
> 
> Moreover, I think that this test case is in fact invalid, cf. PR 48887.

Hm. 
I see; I wonder, is this something that was added in F2008 or was it present in F2003?
Comment 16 Tobias Burnus 2011-05-30 09:22:25 UTC
(In reply to comment #13)
> Moreover, I think that this test case is in fact invalid, cf. PR 48887.

We are talking about comment 4, aren't we?

(That test case fails with ifort 12 as "sm2" is not allocatable. It (and the one in PR 48887) compiles with crayftn, but I have not checked whether the resulting code makes sense.)
Comment 17 Tobias Burnus 2011-05-30 09:35:44 UTC
(In reply to comment #15)
> I see; I wonder, is this something that was added in F2008 or was it present in
> F2003?

If you talk about that associate-names don't inherit the allocate attribute from the selector: That's not new. Cf. F2003, "16.4.1.5 Construct association" and F2008, "16.5.1.6". A quote from the latter can be found in bug 48887 comment 0.
Comment 18 janus 2011-06-17 20:03:07 UTC
Author: janus
Date: Fri Jun 17 20:03:04 2011
New Revision: 175151

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=175151
Log:
2011-06-17  Janus Weil  <janus@gcc.gnu.org>

	PR fortran/48699
	* check.c (gfc_check_move_alloc): If 'TO' argument is polymorphic,
	make sure the vtab is present.

2011-06-17  Janus Weil  <janus@gcc.gnu.org>

	PR fortran/48699
	* gfortran.dg/move_alloc_5.f90: New.

Added:
    trunk/gcc/testsuite/gfortran.dg/move_alloc_5.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/check.c
    trunk/gcc/testsuite/ChangeLog
Comment 19 janus 2011-06-19 21:05:22 UTC
Author: janus
Date: Sun Jun 19 21:05:18 2011
New Revision: 175194

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=175194
Log:
2011-06-19  Janus Weil  <janus@gcc.gnu.org>

	PR fortran/47601
	* module.c (mio_component_ref): Handle components of extended types.
	* symbol.c (gfc_find_component): Return if sym is NULL.

	PR fortran/48699
	* check.c (gfc_check_move_alloc): If 'TO' argument is polymorphic,
	make sure the vtab is present.

	PR fortran/49074
	* interface.c (gfc_extend_assign): Propagate the locus from the
	assignment to the type-bound procedure call.

	PR fortran/49417
	* module.c (mio_component): Make sure the 'class_ok' attribute is set
	for use-associated CLASS components.
	* parse.c (parse_derived): Check for 'class_ok' attribute.
	* resolve.c (resolve_fl_derived): Ditto.


2011-06-19  Janus Weil  <janus@gcc.gnu.org>

	PR fortran/47601
	* gfortran.dg/extends_13.f03: New.

	PR fortran/48699
	* gfortran.dg/move_alloc_5.f90: New.

	PR fortran/49074
	* gfortran.dg/typebound_assignment_3.f03: New.

	PR fortran/49417
	* gfortran.dg/class_43.f03: New.

Added:
    branches/gcc-4_6-branch/gcc/testsuite/gfortran.dg/class_43.f03
    branches/gcc-4_6-branch/gcc/testsuite/gfortran.dg/extends_13.f03
    branches/gcc-4_6-branch/gcc/testsuite/gfortran.dg/move_alloc_5.f90
    branches/gcc-4_6-branch/gcc/testsuite/gfortran.dg/typebound_assignment_3.f03
Modified:
    branches/gcc-4_6-branch/gcc/fortran/ChangeLog
    branches/gcc-4_6-branch/gcc/fortran/check.c
    branches/gcc-4_6-branch/gcc/fortran/interface.c
    branches/gcc-4_6-branch/gcc/fortran/module.c
    branches/gcc-4_6-branch/gcc/fortran/parse.c
    branches/gcc-4_6-branch/gcc/fortran/resolve.c
    branches/gcc-4_6-branch/gcc/fortran/symbol.c
    branches/gcc-4_6-branch/gcc/testsuite/ChangeLog
Comment 20 janus 2011-06-19 21:20:14 UTC
The regression has been fixed on trunk and 4.6. The remaining issue will be tracked by PR 48887. Closing.
Comment 21 Tobias Burnus 2011-11-26 14:32:45 UTC
This comment is (just) for cross reference. The patch at
  http://gcc.gnu.org/ml/fortran/2011-11/msg00217.html
fixes some MOVE_ALLOC issues, including the following one.

Most of the examples in this PR are actually invalid as in MOVE_ALLOC either both arguments shall be polymorphic or both nonpolymorphic.

For instance (comment 9):
>   class(bar), allocatable :: sm
>   type(bar2), allocatable :: sm2
>   call move_alloc(sm2,sm)
wrongly mixes them.

But also (comment 4):
  class(bar), allocatable :: sm,sm2
  select type(sm2) 
  type is (bar)
    call move_alloc(sm2,sm)
  end select
where "sm2" is nonpolymorphic due to the TYPE IS: Fortran 2008, 8.1.9.2, states that "within the block following a TYPE IS type guard statement, the associating entity (16.5.5) is not polymorphic"
Comment 22 Tobias Burnus 2011-12-03 11:33:42 UTC
Actually, my comment 21 was a bit premature: FROM type TO class is valid, only FROM class TO type is invalid. Corrected at:
   http://gcc.gnu.org/ml/fortran/2011-12/msg00006.html

However, the test case of comment 8 (select_type_23.f03) is invalid - but for other reasons. Cf. PR 48887.
Comment 23 Salvatore Filippone 2011-12-03 12:00:43 UTC
Yes, TYPE FROM and polymorphic TO is exactly the typical usage I have (indeed, it also was the original test case)
Thanks