GCC Bugzilla has been upgraded from version 4.4.9 to 5.0rc3. If you see any problem, please report it to bug 64968.

Bug 50981

Summary: [4.6 Regression] Wrong-code for scalarizing ELEMENTAL call with absent OPTIONAL argument
Product: gcc Reporter: Tobias Burnus <burnus>
Component: fortranAssignee: Mikael Morin <mikael>
Status: RESOLVED FIXED    
Severity: normal CC: anlauf, burnus, mhp77, mikael
Priority: P4 Keywords: wrong-code
Version: 4.7.0   
Target Milestone: 4.6.4   
Host: Target:
Build: Known to work:
Known to fail: Last reconfirmed: 2012-01-04 00:00:00
Bug Depends on: 53692    
Bug Blocks:    
Attachments: test case
Another failing variant, with allocatables
Another failing variant, with pointers
Another extensive test case
Patch for the scalar cases, including the OOP ones (with CLASS).
Updated patch
Small test case for polymorphic optional dummies
better patch

Description Tobias Burnus 2011-11-03 14:19:14 UTC
Created attachment 25705 [details]
test case

As reported by Andriy Kostyuk,
http://gcc.gnu.org/ml/fortran/2011-11/msg00035.html

There is a wrong-code issue with absent OPTIONAL arguments and ELEMENTAL.

It works for me with 4.1, 4.3, and
  4.4.0 20090206 (experimental) [trunk revision 143983] (SUSE Linux)
It fails for me with 4.5, 4.6 and 4.7
  4.4.6 20110219 (prerelease) [gcc-4_4-branch revision 170290]
and was reported to be broken in
  GNU Fortran (Ubuntu/Linaro 4.4.4-14ubuntu5) 4.4.5


If one looks at the dump, one finds:
  ff (real(kind=8) & restrict a, integer(kind=4) * b)
  {
  ...
      integer(kind=4) D.1747;
    D.1747 = *b;
  ...
      while (1)
        {
          if (S.1 > 2) goto L.2;
          val.0 = gg (&ac[S.1 + -1], &D.1747) + val.0;
          S.1 = S.1 + 1;
        }

The "D.1747 = *b;" is a rather bad idea if "b == NULL".
Comment 1 Mikael Morin 2011-11-03 16:39:10 UTC
No bootstraped compiler at hand; does this work?

diff --git a/trans-array.c b/trans-array.c
index 3472804..c48f718 100644
--- a/trans-array.c
+++ b/trans-array.c
@@ -2180,6 +2180,7 @@ gfc_add_loop_ss_code (gfc_loopinfo * loop, gfc_ss * ss, bool subscript,
 	  /* Scalar argument to elemental procedure.  Evaluate this
 	     now.  */
 	  gfc_init_se (&se, NULL);
+	  se->want_pointer = 1;
 	  gfc_conv_expr (&se, ss->expr);
 	  gfc_add_block_to_block (&loop->pre, &se.pre);
 	  gfc_add_block_to_block (&loop->post, &se.post);
diff --git a/trans-expr.c b/trans-expr.c
index 09b98d0..9e9ceea 100644
--- a/trans-expr.c
+++ b/trans-expr.c
@@ -4823,8 +4823,6 @@ gfc_conv_expr (gfc_se * se, gfc_expr * expr)
       /* Substitute a scalar expression evaluated outside the scalarization
          loop.  */
       se->expr = se->ss->data.scalar.expr;
-      if (se->ss->type == GFC_SS_REFERENCE)
-	se->expr = gfc_build_addr_expr (NULL_TREE, se->expr);
       se->string_length = se->ss->string_length;
       gfc_advance_se_ss_chain (se);
       return;
Comment 2 Tobias Burnus 2011-11-03 17:12:15 UTC
(In reply to comment #1)
> No bootstraped compiler at hand; does this work?

Does not help. (Nit: in trans-array.c "se.want..." instead of "se->".)

Now the segfaults already for "ff" where the "b" argument is present. The segfault occurs at
    ELEMENTAL REAL(KIND=8) FUNCTION gg(a,b)
      ...
      IF(PRESENT(b)) THEN
        b1=b  ! <<< here

While "ff" now looks reasonable:

ff (real(kind=8) & restrict a, integer(kind=4) * b)
    integer(kind=4) * D.1747;
    D.1747 = b;
          val.0 = gg (&ac[S.1 + -1], D.1747) + val.0;

one has an issue with the call:
        rr[S.5 + -1] = ff (&aa[S.5 + -1], 1);

as it is probably not a good idea to dereference the literal "1", unless you really want to access the memory address 0x1 ...

 * * *

Works:
   4.4.5 20100627 (prerelease) [gcc-4_4-branch revision 161471]
Fails:
   4.4.5 20100627 (prerelease) [gcc-4_4-branch revision 161472]

Unless I made a mistake with building those versions, the regression is caused by:

Rev. 161472 (PR fortran/43841 and PR 43843).
http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=161472

        * trans-expr.c (gfc_conv_expr): Supply an address expression for
        GFC_SS_REFERENCE.
        (gfc_conv_expr_reference): Call gfc_conv_expr and return for
        GFC_SS_REFERENCE.
        * trans-array.c (gfc_add_loop_ss_code): Store the value rather
        than the address of a GFC_SS_REFERENCE.
        * trans.h : Change comment on GFC_SS_REFERENCE.
Comment 3 Mikael Morin 2011-11-03 19:19:51 UTC
(In reply to comment #2)
> Unless I made a mistake with building those versions, the regression is caused
> by:
> 
> Rev. 161472 (PR fortran/43841 and PR 43843).
> http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=161472
> 
Makes sense.
The patch I have proposed is more or less a revert of that change.
Comment 4 Mikael Morin 2011-11-22 18:57:52 UTC
I will take care of this one after mid-december.
Comment 5 Mikael Morin 2012-01-04 14:04:29 UTC
Author: mikael
Date: Wed Jan  4 14:04:24 2012
New Revision: 182874

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=182874
Log:
	PR fortran/50981
	* trans.h (struct gfc_ss_info): New field data::scalar::can_be_null_ref
	* trans-array.c: If the reference can be NULL, save the reference
	instead of the value.
	* trans-expr.c (gfc_conv_expr): If we have saved a reference,
	dereference it.


Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/trans-array.c
    trunk/gcc/fortran/trans-expr.c
    trunk/gcc/fortran/trans.h
Comment 6 Mikael Morin 2012-01-04 14:20:24 UTC
Author: mikael
Date: Wed Jan  4 14:20:17 2012
New Revision: 182875

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=182875
Log:
	PR fortran/50981
	* trans-array.h (gfc_walk_elemental_function_args): New argument.
	* trans-intrinsic.c (gfc_walk_intrinsic_function): Update call.
	* trans-stmt.c (gfc_trans_call): Ditto.
	* trans-array.c (gfc_walk_function_expr): Ditto.
	(gfc_walk_elemental_function_args): Get the dummy argument list
	if possible.  Check that the dummy and the actual argument are both
	optional, and set can_be_null_ref accordingly.


Added:
    trunk/gcc/testsuite/gfortran.dg/elemental_optional_args_2.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/trans-array.c
    trunk/gcc/fortran/trans-array.h
    trunk/gcc/fortran/trans-intrinsic.c
    trunk/gcc/fortran/trans-stmt.c
    trunk/gcc/testsuite/ChangeLog
Comment 7 Mikael Morin 2012-01-04 14:36:38 UTC
Author: mikael
Revision: 182875
Modified property: svn:log

Modified: svn:log at Wed Jan  4 14:36:34 2012
------------------------------------------------------------------------------
--- svn:log (original)
+++ svn:log Wed Jan  4 14:36:34 2012
@@ -1,3 +1,4 @@
+	
 	PR fortran/50981
 	* trans-array.h (gfc_walk_elemental_function_args): New argument.
 	* trans-intrinsic.c (gfc_walk_intrinsic_function): Update call.
@@ -7,3 +8,5 @@
 	if possible.  Check that the dummy and the actual argument are both
 	optional, and set can_be_null_ref accordingly.
 
+	* gfortran.dg/elemental_optional_args_2.f90: New test.
+
Comment 8 Mikael Morin 2012-01-04 15:05:47 UTC
Fixed on trunk (4.7), backport bending.
Comment 9 Mikael Morin 2012-01-04 15:06:32 UTC
(In reply to comment #8)
> Fixed on trunk (4.7), backport bending.

s/bending/pending/
Comment 10 Mikael Morin 2012-01-05 14:50:38 UTC
Created attachment 26250 [details]
Another failing variant, with allocatables

This is not fixed by the patch above.
Comment 11 Mikael Morin 2012-01-05 14:51:26 UTC
Created attachment 26252 [details]
Another failing variant, with pointers

This is not fixed by the patch above.
Comment 12 Tobias Burnus 2012-01-06 11:06:59 UTC
Draft patch. (It uses gfc_expr_attr to allow for pointer/allocatable components. I also tried to take BT_CLASS into account.)

(This patch is a side effect of unsuccessful debugging of the related PR 51758.)

--- trans-array.c       (revision 182944)
+++ trans-array.c       (working copy)
@@ -8362,9 +8384,17 @@ gfc_walk_elemental_function_args (gfc_ss * ss, gfc
 
 	  if (dummy_arg != NULL
 	      && dummy_arg->sym->attr.optional
-	      && arg->expr->symtree
-	      && arg->expr->symtree->n.sym->attr.optional
-	      && arg->expr->ref == NULL)
+	      && arg->expr->expr_type == EXPR_VARIABLE
+	      && ((arg->expr->symtree->n.sym->attr.optional
+		   && arg->expr->ref == NULL)
+		  || ((gfc_expr_attr (arg->expr).allocatable
+		       || gfc_expr_attr (arg->expr).pointer)
+		      && ((dummy_arg->sym->ts.type != BT_CLASS
+			   && !dummy_arg->sym->attr.allocatable
+			   && !dummy_arg->sym->attr.pointer)
+			 || (dummy_arg->sym->ts.type == BT_CLASS
+			     && !CLASS_DATA (dummy_arg->sym)->attr.allocatable
+			     && !CLASS_DATA (dummy_arg->sym)->attr.pointer)))))
 	    newss->info->data.scalar.can_be_null_ref = true;
 	}
       else
Comment 13 Tobias Burnus 2012-01-10 14:20:13 UTC
Created attachment 26291 [details]
Another extensive test case

(In reply to comment #12)
> Draft patch.

That patch unfortunately does not work for all of the tests in the attached file. (Assuming that the test cases are valid.)

TODO:
* Add also tests for polymorphic components as actual argument
* Fix issues found by the test case (and/or fix the test cases)
Comment 14 Tobias Burnus 2012-01-10 14:29:39 UTC
(In reply to comment #13)
> TODO:
> * Add also tests for polymorphic components as actual argument

And a test case where the dummy argument is BT_CLASS.
Comment 15 Mikael Morin 2012-01-10 15:15:47 UTC
I'm having a look.
Comment 16 Mikael Morin 2012-01-10 17:30:20 UTC
Created attachment 26296 [details]
Patch for the scalar cases, including the OOP ones (with CLASS).

This patch passes gfortran.dg/*elemental*, but fails on gfortran.dg/class_array_{1,2,3,7,9}.f03.

I'm inclined to defer the handling of class cases to 4.8.

Regarding the patch itself:
 - there were some cases where code->expr1 didn't have the function reference, and one had to look at code->resolved_sym to have the procedure's (or its interface's) symbol.
 - the big condition in comment #12 has been trimmed, assuming that: 
      * dummy arguments of elemental procedures don't have the pointer/allocatable attribute
      * gfc_expr_attr does the right thing w.r.t. class expressions
      * gfc_expr_attr does not propagate the pointer/allocatable/optional attribute to subcomponents.
Comment 17 Tobias Burnus 2012-01-10 23:54:50 UTC
(In reply to comment #16)
> This patch passes gfortran.dg/*elemental*, but fails on
> gfortran.dg/class_array_{1,2,3,7,9}.f03.

The problem is the way _copy is translated. One currently has in trans-stmt.c's gfc_trans_allocate

              /* Do a polymorphic deep copy.  */
              actual->next->expr->ts.type = BT_CLASS;
              gfc_add_data_component (actual->next->expr);

This adds "class._data". With the patch of comment 16 one has now additionally in trans-expr.c's gfc_conv_procedure_call:

+         if (fsym && fsym->ts.type == BT_DERIVED && e->ts.type == BT_CLASS)
+           parmse.expr = gfc_class_data_get (parmse.expr);

If one removes the latter it works.

I think the proper solution is to remove the trans-stmt.c manual scalarization and handle somewhere in trans-{array,expr}.c the array bounds.

Stupid attempts fail for various reasons: Changing the BT_CLASS to BT_DERIVED in gfc_trans_allocate does not work as one the offsets will no longer be class._size (why?). Keeping BT_CLASS but removing the gfc_add_data_component in trans-stmt.c will ICE for dataref - and removing that as well will lead to valid code with class._size, but without scalarization loop ...
Comment 18 Mikael Morin 2012-01-11 23:31:32 UTC
(In reply to comment #17)
> I think the proper solution is to remove the trans-stmt.c manual scalarization
> and handle somewhere in trans-{array,expr}.c the array bounds.
> 
Some of it might actually be dead code. I don't see where the `temp' variable is used.

> Stupid attempts fail for various reasons: Changing the BT_CLASS to BT_DERIVED
> in gfc_trans_allocate does not work as one the offsets will no longer be
> class._size (why?). Keeping BT_CLASS but removing the gfc_add_data_component > in trans-stmt.c will ICE for dataref - and removing that as well will lead to
> valid code with class._size, but without scalarization loop ...

Below is my `stupid' attempt at the not-so-proper solution: it does an `intelligent' e->ts.type == BT_CLASS in the new conditions in gfc_conv_procedure_call.  It passes gfortran.dg/*elemental* and gfortran.dg/*class*.  But it is admittedly somewhat hackish.



diff --git a/trans-expr.c b/trans-expr.c
index a2268a1..1a24d4b 100644
--- a/trans-expr.c
+++ b/trans-expr.c
@@ -3259,6 +3259,35 @@ conv_isocbinding_procedure (gfc_se * se, gfc_symbol * sym,
 }
 
 
+static bool
+needs_class_data_ref (gfc_expr *e)
+{
+  gfc_ref *ref;
+  bool result;
+
+  if (e->expr_type != EXPR_VARIABLE)
+    return false;
+
+  if (e->symtree->n.sym->ts.type == BT_CLASS)
+    result = true;
+
+  for (ref = e->ref; ref; ref = ref->next)
+    {
+      if (ref->type != REF_COMPONENT)
+	{
+	  result = false;
+	  continue;
+	}
+
+      if (ref->u.c.component->ts.type == BT_CLASS)
+	result = true; 
+      else if (!strcmp (ref->u.c.component->name, "_data"))
+	result = false;
+    }
+
+  return result;
+}
+
 /* Generate code for a procedure call.  Note can return se->post != NULL.
    If se->direct_byref is set then se->expr contains the return parameter.
    Return nonzero, if the call has alternate specifiers.
@@ -3435,7 +3464,7 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym,
 	  else
 	    gfc_conv_expr_reference (&parmse, e);
 
-	  if (fsym && fsym->ts.type == BT_DERIVED && e->ts.type == BT_CLASS)
+	  if (fsym && fsym->ts.type == BT_DERIVED && needs_class_data_ref (e))
 	    parmse.expr = gfc_class_data_get (parmse.expr);
 
 	  /* The scalarizer does not repackage the reference to a class
@@ -3514,7 +3543,7 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym,
 
 
 		  if (fsym && fsym->ts.type == BT_DERIVED
-		      && e->ts.type == BT_CLASS)
+		      && needs_class_data_ref (e))
 		    parmse.expr = gfc_class_data_get (parmse.expr);
 
 		  /* A class array element needs converting back to be a
Comment 19 Tobias Burnus 2012-01-16 19:51:48 UTC
Author: burnus
Date: Mon Jan 16 19:51:44 2012
New Revision: 183220

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=183220
Log:
2012-01-16  Mikael Morin  <mikael@gcc.gnu.org>
            Tobias Burnus  <burnus@net-b.de>

        PR fortran/50981
        * trans-array.c (gfc_walk_elemental_function_args): Fix
        passing of deallocated allocatables/pointers as absent argument. 

2012-01-16  Mikael Morin  <mikael@gcc.gnu.org>
            Tobias Burnus  <burnus@net-b.de>

        PR fortran/50981
        * gfortran.dg/elemental_optional_args_3.f90: New
        * gfortran.dg/elemental_optional_args_4.f90: New


Added:
    trunk/gcc/testsuite/gfortran.dg/elemental_optional_args_3.f90
    trunk/gcc/testsuite/gfortran.dg/elemental_optional_args_4.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/trans-array.c
    trunk/gcc/testsuite/ChangeLog
Comment 20 Tobias Burnus 2012-01-18 10:16:10 UTC
Status:

a) Passing absent optional as actual argument to elemental procedures.
   4.4-4.7 Regression. Fixed on the 4.7 trunk, needs backporting.
   See comment 5 / http://gcc.gnu.org/ml/gcc-patches/2012-01/msg00206.html

b) Passing unallocated allocatable / unassociated pointer:
   Affects 4.6/4.7; fixed on the 4.7 trunk with comment 19
   http://gcc.gnu.org/ml/gcc-patches/2012-01/msg00793.html
   Consider backporting to 4.6.

c) Issues with polymorphism and elemental
   See testcase of comment 13 and draft patch of comment 16 and comment 18.
   (A tiny part of the patch has been committed in comment 19.)
   That's a trunk-only item.
Comment 21 Mikael Morin 2012-01-18 13:05:01 UTC
Thanks Tobias.

(In reply to comment #20)
> Status:
> 
> a) Passing absent optional as actual argument to elemental procedures.
>    4.4-4.7 Regression. Fixed on the 4.7 trunk, needs backporting.
>    See comment 5 / http://gcc.gnu.org/ml/gcc-patches/2012-01/msg00206.html
> 
> b) Passing unallocated allocatable / unassociated pointer:
>    Affects 4.6/4.7; fixed on the 4.7 trunk with comment 19
>    http://gcc.gnu.org/ml/gcc-patches/2012-01/msg00793.html
>    Consider backporting to 4.6.
> 
> c) Issues with polymorphism and elemental
>    See testcase of comment 13 and draft patch of comment 16 and comment 18.
>    (A tiny part of the patch has been committed in comment 19.)
>    That's a trunk-only item.

I should be able to produce a patch for c) before the end of the week.
I'll defer backporting after that because it's possible that the final patch affects the way a) and b) are done.  Let's wait until the code has settled a bit on the trunk before applying (part of) it to the branche(s).
Comment 22 Mikael Morin 2012-01-20 00:21:44 UTC
Created attachment 26386 [details]
Updated patch

Updated patch.
This passes the full comment #13 test fixed as follows:

--- comment_13.f90	2012-01-20 01:04:29.000000000 +0100
+++ comment_13_fixed2.f90	2012-01-20 01:13:28.000000000 +0100
@@ -148,7 +148,7 @@
 call sub_t (s, cp, .true.)
 call sub_t (v, cp, .true.)
 !print *, s, v
-if (s /= 5*2) call abort() ! << FAIL
+if (s /= 7*2) call abort() ! << FAIL
 if (any (v /= [7*2, 7*2])) call abort() ! << FAIL
 
 ! ARRAY COMPONENTS: Non alloc/assoc
@@ -196,7 +196,7 @@
 
 call sub_t (v, tpa, .true.)
 !print *, v
-if (any (v /= [55, 555*2])) call abort() ! <<< FAIL
+if (any (v /= [55*2, 555*2])) call abort() ! <<< FAIL
 
 
 call sub_t (v, caa, .true.)
@@ -205,7 +205,7 @@
 
 call sub_t (v, cpa, .true.)
 !print *, v
-if (any (v /= [77, 777*2])) call abort() ! <<< FAIL
+if (any (v /= [77*2, 777*2])) call abort() ! <<< FAIL
 
 contains
Comment 23 Tobias Burnus 2012-01-20 11:28:23 UTC
Created attachment 26392 [details]
Small test case for polymorphic optional dummies

(In reply to comment #22)
> Updated patch.
> This passes the full comment #13 test fixed as follows:

Truly awesome! Will you submit the patch?

 * * *

Related, but a bit separate issue: OPTIONAL CLASS dummies. See attached test case. It seems to mostly work with the patch below, except for one ICE and one SEGV at run time. (And - out-commented part - I'm hitting an ICE which might be the same as PR 46356 comment 2.)

TODO: In the fixed-up test case of comment 13, copy sub_t and change "type" to "class" - copy all calls to sub_t - and modify those to call the new function. See what will break ...

TODO2: Call both the original sub_t and the new function with polymorphic components of derived types.

--- trans-expr.c        (revision 183328)
+++ trans-expr.c        (working copy)
@@ -576,8 +576,16 @@ gfc_conv_expr_present (gfc_symbol * sym)
   gcc_assert (sym->attr.dummy);

   decl = gfc_get_symbol_decl (sym);
-  if (TREE_CODE (decl) != PARM_DECL)
+
+  if (sym->ts.type == BT_CLASS)
     {
+      decl = gfc_class_data_get (decl);
+      if (CLASS_DATA (sym)->attr.dimension
+         || CLASS_DATA (sym)->attr.codimension)
+       decl = gfc_build_addr_expr (NULL_TREE, decl);
+    }
+  else if (TREE_CODE (decl) != PARM_DECL)
+    {
       /* Array parameters use a temporary descriptor, we want the real
          parameter.  */
       gcc_assert (GFC_DESCRIPTOR_TYPE_P (TREE_TYPE (decl))
Comment 24 Mikael Morin 2012-01-20 15:31:58 UTC
(In reply to comment #23)
> Created attachment 26392 [details]
> Small test case for polymorphic optional dummies
> 
> (In reply to comment #22)
> > Updated patch.
> > This passes the full comment #13 test fixed as follows:
> 
> Truly awesome! Will you submit the patch?
> 
Yes, I plan to polish it a bit, test, and submit.
Comment 25 Dominique d'Humieres 2012-01-20 15:54:14 UTC
On x86_64-apple-darwin10, the patch in comments #22 breaks most of the tests I have for extends_type_of (3 out of 5) and in the test suite (only gfortran.dg/extends_type_of_2.f03 passes the two other tests gfortran.dg/extends_type_of_(1.f03|3.f90) fails):

Program received signal SIGSEGV: Segmentation fault - invalid memory reference.

I have also another test without extends_type_of which fails:

[macbook] f90/bug% cat oop_14.f90
module realloc
  implicit none

  type :: base_type
     integer :: i =2
  contains
    procedure :: assign
    generic :: assignment(=) => assign   ! define generic assignment
  end type base_type

  type, extends(base_type) :: extended_type
     integer :: j =3
  end type extended_type

contains

  elemental subroutine assign (a, b)
    class(base_type), intent(out) :: a
    class(base_type), intent(in) :: b
    a%i = b%i
  end subroutine assign

  subroutine reallocate (a)
    class(base_type), dimension(:), allocatable, intent(inout) :: a
    class(base_type), dimension(:), allocatable :: tmp

    allocate (tmp (2 * size (a))) ! how to alloc b with same type as a ?

! This is one way how!
!    select type (a)
!      type is (base_type);      allocate (base_type :: tmp (2 * size (a)))
!      type is (extended_type);  allocate (extended_type :: tmp (2 * size (a)))
!    end select

    call print_type ("tmp", tmp)
    tmp(:size(a)) = a             ! polymorphic l.h.s.
!    call move_alloc (from=tmp, to=a) ! remains to be sorted out
  end subroutine reallocate

  subroutine print_type (name, a)
    character(*), intent(in) :: name
    class(base_type), dimension(:), intent(in) :: a
    select type (a)
    type is (base_type);      print *, name // " is base_type", a
    type is (extended_type);  print *, name // " is extended_type", a
    end select
  end subroutine print_type

end module realloc

program main
  use realloc
  implicit none
  class(base_type), dimension(:), allocatable :: a

!  allocate (a(10), source = extended_type(1,2)) ! this works
  allocate (extended_type::a(10))
  call print_type ("a", a)
  call reallocate (a)
  call print_type ("a", a)
end program main
[macbook] f90/bug% gfc oop_14.f90
[macbook] f90/bug% a.out 
 a is extended_type           2           3           2           3           2           3           2           3           2           3           2           3           2           3           2           3           2           3           2           3

Program received signal SIGSEGV: Segmentation fault - invalid memory reference.
Comment 26 Dominique d'Humieres 2012-01-21 11:41:15 UTC
Running the regression test suite gives:

FAIL: gfortran.dg/coarray_poly_1.f90  -O  (internal compiler error)
FAIL: gfortran.dg/coarray_poly_1.f90  -O   (test for errors, line 11)
FAIL: gfortran.dg/coarray_poly_1.f90  -O  (test for excess errors)
FAIL: gfortran.dg/extends_type_of_1.f03  -O0  execution test
FAIL: gfortran.dg/extends_type_of_1.f03  -O1  execution test
FAIL: gfortran.dg/extends_type_of_1.f03  -O2  execution test
FAIL: gfortran.dg/extends_type_of_1.f03  -O3 -fomit-frame-pointer  execution test
FAIL: gfortran.dg/extends_type_of_1.f03  -O3 -fomit-frame-pointer -funroll-loops  execution test
FAIL: gfortran.dg/extends_type_of_1.f03  -O3 -fomit-frame-pointer -funroll-all-loops -finline-functions  execution test
FAIL: gfortran.dg/extends_type_of_1.f03  -O3 -g  execution test
FAIL: gfortran.dg/extends_type_of_1.f03  -Os  execution test

The failure for gfortran.dg/coarray_poly_1.f90 is

[macbook] f90/bug% gfc /opt/gcc/work/gcc/testsuite/gfortran.dg/coarray_poly_1.f90 -fcoarray=single -O/opt/gcc/work/gcc/testsuite/gfortran.dg/coarray_poly_1.f90:9.25:

  class(t) :: A(:)[4,2:*] ! { dg-error "is not ALLOCATABLE, SAVE nor a dummy ar
                         1
Error: Variable 'a' at (1) is a coarray and is not ALLOCATABLE, SAVE nor a dummy argument
f951: internal compiler error: Segmentation fault

Note that gfortran.dg/extends_type_of_3.f90 does not appear because it is not run (dg-do compile). However it runs without the patch and gives a segmentation fault with it.
Comment 27 Tobias Burnus 2012-01-25 22:29:53 UTC
(In reply to comment #22)
> Created attachment 26386 [details]
> Updated patch

+gfc_array_spec *
+symbol_as (gfc_symbol *sym)
+{
+  if (sym->ts.type == BT_CLASS)
+    return CLASS_DATA (sym)->as;

I think the function rather belongs to symbol.c than to resolve.c, unless it is "static". But the real issue is the "if" condition. It should be:
  if (sym->ts.type == BT_CLASS && sym->attr.class_ok)
otherwise, one might get a segfault with gfortran.dg/coarray_poly_1.f90
Comment 28 Mikael Morin 2012-01-25 22:57:07 UTC
Created attachment 26468 [details]
better patch

This one should work.
Comment 29 Mikael Morin 2012-01-25 23:38:28 UTC
(In reply to comment #27)
> (In reply to comment #22)
> > Created attachment 26386 [details]
> > Updated patch
> 
> +gfc_array_spec *
> +symbol_as (gfc_symbol *sym)
> +{
> +  if (sym->ts.type == BT_CLASS)
> +    return CLASS_DATA (sym)->as;
> 
> I think the function rather belongs to symbol.c than to resolve.c, unless it is
> "static". But the real issue is the "if" condition. It should be:
>   if (sym->ts.type == BT_CLASS && sym->attr.class_ok)
> otherwise, one might get a segfault with gfortran.dg/coarray_poly_1.f90

Sorry, I didn't see your comment.
I solved the issue differently, but I'll try with your suggestion.

The test in comment #23 should be rejected, as sub_ctae is elemental with a non-scalar dummy. Otherwise, it should work with the just posted patch.
Comment 30 Dominique d'Humieres 2012-01-26 16:12:13 UTC
(In reply to comment #28)
> Created attachment 26468 [details]
> better patch
> 
> This one should work.

It does;-) 

I have applied the patch on revision 183541 on top of the Paul's patch at http://gcc.gnu.org/ml/fortran/2012-01/msg00214.html (pr51870) and Tobias' ones for pr51970 and pr51434 comment #12. 

It regtested without regression and all the failures reported in comments #25 and #26 are gone. It  passes the full comment #13 test fixed as in comment #22 (I blindly trust that it is the "right thing to do") and it fixes pr51514 and the test in pr41587 comment #1 (at least part of it: it now prints "          0           0          42" while I'ld expect "         42          42          42"(?); without the patch I get an ICE
pr41587_db.f90:10:0: internal compiler error: in gfc_conv_descriptor_data_get, at fortran/trans-array.c:147).

The test in comment #23 gives an ICE with/without the patch:

pr50981_4.f90: In function 'MAIN__':
pr50981_4.f90:16:0: internal compiler error: in fold_convert_loc, at fold-const.c:2016

i.e., the same ICE as for pr51977 (I did not tested the patchlet in comment #27).

Thanks for the work!
Comment 31 Dominique d'Humieres 2012-01-28 11:40:17 UTC
> The test in comment #23 gives an ICE with/without the patch:
>
> pr50981_4.f90: In function 'MAIN__':
> pr50981_4.f90:16:0: internal compiler error: in fold_convert_loc, at
> fold-const.c:2016
>
> i.e., the same ICE as for pr51977 (I did not tested the patchlet in comment
> #27).

This was pr52016 fixed by revision 183625. Looking further to this test, I have found the following reduced test which gives a segmentation fault at run time:

    type t
      integer :: a
    end type t
  type(t), allocatable :: var1, var2(:)
  call sub_cta(var2)

contains
  subroutine sub_cta(y)
    class(t), intent(inout), optional :: y(:)
! FIXME: PR 46356 ?
    print *, present(y)
    if (present(y)) y%a = 7
    if (present(y)) i = 7
  end subroutine sub_cta
end

[macbook] f90/bug% gfc pr50981_4_red_1.f90 
[macbook] f90/bug% a.out
 T

Program received signal SIGSEGV: Segmentation fault - invalid memory reference.
...

i.e., the unallocated 'var2' is viewed as present.
Comment 32 Dominique d'Humieres 2012-01-29 12:09:57 UTC
Using 'symbol_as' without prototypes, as in comment #28, breaks bootstrap when configured with '--disable-build-poststage1-with-cxx' (see pr52038).
Comment 33 Tobias Burnus 2012-01-29 12:17:25 UTC
(In reply to comment #32)
> Using 'symbol_as' without prototypes, as in comment #28, breaks bootstrap when
> configured with '--disable-build-poststage1-with-cxx' (see pr52038).

As written in comment 27, if it is only used in resolve.c, not a prototype but "static" should be used.
Comment 34 Dominique d'Humieres 2012-01-29 12:20:56 UTC
> As written in comment 27, if it is only used in resolve.c, not a prototype but
> "static" should be used.

AFAICT it could also be used in gcc/fortran/expr.c (as changed in r 83622).
Comment 35 Mikael Morin 2012-02-02 23:11:01 UTC
Author: mikael
Date: Thu Feb  2 23:10:55 2012
New Revision: 183853

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=183853
Log:
2012-02-02  Mikael Morin  <mikael@gcc.gnu.org>

	PR fortran/41587
	PR fortran/46356
	PR fortran/51754
	PR fortran/50981
	* class.c (insert_component_ref, class_data_ref_missing,
	gfc_fix_class_refs): New functions.
	* gfortran.h (gfc_fix_class_refs): New prototype.
	* trans-expr.c (gfc_conv_expr): Remove special case handling and call
	gfc_fix_class_refs instead.

2012-02-02  Mikael Morin  <mikael@gcc.gnu.org>

	PR fortran/41587
	* gfortran.dg/class_array_10.f03: New test.

	PR fortran/46356
	* gfortran.dg/class_array_11.f03: New test.

	PR fortran/51754
	* gfortran.dg/class_array_12.f03: New test.


Added:
    trunk/gcc/testsuite/gfortran.dg/class_array_10.f03
    trunk/gcc/testsuite/gfortran.dg/class_array_11.f03
    trunk/gcc/testsuite/gfortran.dg/class_array_12.f03
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/class.c
    trunk/gcc/fortran/gfortran.h
    trunk/gcc/fortran/trans-expr.c
    trunk/gcc/testsuite/ChangeLog
Comment 36 Tobias Burnus 2012-02-03 15:05:55 UTC
I lost a bit the overview, but I think the following still needs to be done:

- 4.4/4.5/4.6: Backporting the fix for nonpresent actuals to elemental procs,
  cf. commit in comment 5 / comment 6. That is a true [4.4-4.6] regression

- 4.6: Backporting for the fix for unallocated/unassociated variables to
  elemental procedures. That's not a true regression.


- The test case of comment 13  / attachment 26291 [details]
  with the test-case corrections of comment 22
  This test case still fails. There are some draft patches for that in this PR.

- The test case of comment 23 / attachment 26392 [details]
  "sub_ctae" is invalid in needs to be removed. The test case compiles without
  any problems, but the program segfaults at run time for:
    call sub_ct(var1)
    call sub_ct(var2)
  if the variables aren't allocated. That seems to be the same issue as
  comment 31. Namely: The present() check does not work for CLASS.


- For the following, a test case should be written, tested, and included
  in the test suite (partially covered by the tests above):

  OPTIONAL dummy argument:
    - actual argument not present (i.e. compiler added null), "NULL()",
      nonallocated actual, nonassociated actual. But also: Actual present.
    - Actual argument a scalar or an array
    - Actual argument CLASS or TYPE
    - Actual argument a variable or a derived-type component

  Dummy argument: CLASS or TYPE
  ELEMENTAL procedure with scalar
     or nonelemental with (scalar or) array dummy argument.
  
  For passing a CLASS actual to a TYPE dummy, see also PR 51514.
Comment 37 Mikael Morin 2012-02-12 15:46:18 UTC
Author: mikael
Date: Sun Feb 12 15:46:14 2012
New Revision: 184142

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=184142
Log:
gcc/fortran/
	PR fortran/50981
	* trans-stmt.c (gfc_get_proc_ifc_for_call): New function.
	(gfc_trans_call): Use gfc_get_proc_ifc_for_call.

gcc/testsuite/
	PR fortran/50981
	* gfortran.dg/elemental_optional_args_5.f03: New test.


Added:
    trunk/gcc/testsuite/gfortran.dg/elemental_optional_args_5.f03
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/trans-stmt.c
    trunk/gcc/testsuite/ChangeLog
Comment 38 Tobias Burnus 2012-02-21 13:32:38 UTC
Pending trunk patches (approved, but not committed; 4.8?):
- http://gcc.gnu.org/ml/fortran/2012-02/msg00061.html
- http://gcc.gnu.org/ml/fortran/2012-02/msg00062.html
Comment 39 Mikael Morin 2012-03-04 21:05:36 UTC
Author: mikael
Date: Sun Mar  4 21:05:32 2012
New Revision: 184896

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=184896
Log:
fortran/
	PR fortran/50981
	* trans-expr.c (gfc_conv_procedure_call): Save se->ss's value. 
	Handle the case of unallocated arrays passed to elemental procedures.

testsuite/
	PR fortran/50981
	* gfortran.dg/elemental_optional_args_5.f03: Add array checks.


Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/trans-expr.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gfortran.dg/elemental_optional_args_5.f03
Comment 40 Mikael Morin 2012-03-04 21:50:14 UTC
Author: mikael
Date: Sun Mar  4 21:50:08 2012
New Revision: 184904

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=184904
Log:
fortran/
	PR fortran/50981
	* gfortran.h (gfc_is_class_container_ref): New prototype.
	* class.c (gfc_is_class_container_ref): New function.
	* trans-expr.c (gfc_conv_procedure_call): Add a "_data" component
	reference to polymorphic actual arguments.

testsuite/
	PR fortran/50981
	* gfortran.dg/elemental_optional_args_5.f03: Add subcomponent actual
	argument checks.


Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/class.c
    trunk/gcc/fortran/gfortran.h
    trunk/gcc/fortran/trans-expr.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gfortran.dg/elemental_optional_args_5.f03
Comment 41 Jakub Jelinek 2012-03-13 12:45:35 UTC
4.4 branch is being closed, moving to 4.5.4 target.
Comment 42 Tobias Burnus 2012-05-07 14:34:16 UTC
(In reply to comment #36)
> I lost a bit the overview, but I think the following still needs to be done:
> 
> - 4.4/4.5/4.6: Backporting the fix for nonpresent actuals to elemental procs,
>   cf. commit in comment 5 / comment 6. That is a true [4.4-4.6] regression
> - 4.6: Backporting for the fix for unallocated/unassociated variables to
>   elemental procedures. That's not a true regression.

Not yet backported. The first issue (cf. comment 0) occurred with user code. It is probably sufficient to only backport that one to only 4.6.


> - The test case of comment 13  / attachment 26291 [details]
Now fixed (4.7/4.8).


> - The test case of comment 23 / attachment 26392 [details]
>   "sub_ctae" is invalid in needs to be removed. The test case compiles without
>   any problems, but the program segfaults at run time for:
>     call sub_ct(var1)
>     call sub_ct(var2)
>   if the variables aren't allocated. That seems to be the same issue as
>   comment 31. Namely: The present() check does not work for CLASS.

Seems still to be the case.  "print *, present(y)" should print false for comment 31 as "y" is an unallocated actual to an nonalloctable dummy. It shows "T".


> - For the following, a test case should be written, tested, and included
>   in the test suite (partially covered by the tests above):

Still valid, cf. comment 36
Comment 43 Tobias Burnus 2012-09-25 14:22:29 UTC
(In reply to comment #42)
> > - The test case of comment 23 / attachment 26392 [details]
> >   "sub_ctae" is invalid in needs to be removed.
> > [...] That seems to be the same issue as comment 31

Those issues are fixed by the patch attached to bug 54618 comment 8.

> > - For the following, a test case should be written, tested, and included
> >   in the test suite (partially covered by the tests above):
> Still valid, cf. comment 36

Well, more test cases never harm ...
Comment 44 Tobias Burnus 2012-10-16 13:02:09 UTC
Author: burnus
Date: Tue Oct 16 13:02:02 2012
New Revision: 192495

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

        PR fortran/50981
        PR fortran/54618
        * trans.h (gfc_conv_derived_to_class, gfc_conv_class_to_class):
        Update prototype.
        * trans-stmt.c (trans_associate_var,gfc_trans_allocate): Update
        calls to those functions.
        * trans-expr.c (gfc_conv_derived_to_class,
        * gfc_conv_class_to_class,
        gfc_conv_expr_present): Handle absent polymorphic arguments.
        (class_scalar_coarray_to_class): New function.
        (gfc_conv_procedure_call): Update calls.

2012-10-16  Tobias Burnus  <burnus@net-b.de>

        PR fortran/50981
        PR fortran/54618
        * gfortran.dg/class_optional_1.f90: New.
        * gfortran.dg/class_optional_2.f90: New.


Added:
    trunk/gcc/testsuite/gfortran.dg/class_optional_1.f90
    trunk/gcc/testsuite/gfortran.dg/class_optional_2.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/trans-expr.c
    trunk/gcc/fortran/trans-stmt.c
    trunk/gcc/fortran/trans.h
    trunk/gcc/testsuite/ChangeLog
Comment 45 Tobias Burnus 2012-10-16 13:08:39 UTC
I close this PR now as FIXED.

The initial problem has been fixed on 4.7 + trunk (= 4.8). Some issue exists in 4.4 to 4.6, another is a special case for a new 4.6 nonpolymorphism feature. I don't think it makes sense to backport those.

I think all test cases of this PR are effectively fixed on the trunk (= 4.8). Some issues - mainly with ELEMENTAL - remain, but those are tracked in PR54618.