Bug 50659 - [4.4/4.5/4.6/4.7 Regression] [F03] ICE with PROCEDURE statement
Summary: [4.4/4.5/4.6/4.7 Regression] [F03] ICE with PROCEDURE statement
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: fortran (show other bugs)
Version: 4.7.0
: P4 normal
Target Milestone: 4.4.7
Assignee: janus
URL:
Keywords: ice-on-valid-code
Depends on:
Blocks:
 
Reported: 2011-10-07 17:20 UTC by Andrew Benson
Modified: 2011-10-20 12:37 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2011-10-07 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Benson 2011-10-07 17:20:14 UTC
:: arrSize
end module Size_Mod

module Test_Mod
!  use Size_Mod !! Works if module used here.
  implicit none
  private
  public :: Init, Proc1
  
contains

  subroutine Init(Proc_Get)
    implicit none
    procedure(Proc1), pointer, intent(inout) :: Proc_Get

    return
  end subroutine Init

  function Proc1(arg1) result (res)
    use Size_Mod !! Fails if module used here.
    implicit none
    double precision, dimension(arrSize) :: res
    double precision, intent(in) :: arg1

    return
  end function Proc1
  
end module Test_Mod

$ gfortran -v
Using built-in specs.
COLLECT_GCC=/usr/local/gcc-4.7/bin/gfortran
COLLECT_LTO_WRAPPER=/usr/local/gcc-4.7/libexec/gcc/i686-pc-linux-
gnu/4.7.0/lto-wrapper
Target: i686-pc-linux-gnu
Configured with: ../gcc-4.7/configure --prefix=/usr/local/gcc-4.7 --enable-
languages=c,c++,fortran --disable-multilib
Thread model: posix
gcc version 4.7.0 20111007 (experimental) (GCC)

$ gfortran -c test.F90 -o test.o
f951: internal compiler error: in replace_symbol, at fortran/expr.c:4155
Please submit a full bug report,
with preprocessed source if appropriate.
See <http://gcc.gnu.org/bugs.html> for instructions.

The code is presumably invalid, because the "arrSize" variable (defined in 
Size_Mod) isn't in scope where Proc1 is used as a procedure interface - moving 
the "use Size_Mod" so that "arrSize" is in scope throughout the "Test_Mod" 
module makes the ICE go away.

The ICE also seems to occur in 4.6.1.

Interestingly, 4.4.5 compiles without complaint, unless I remove "Proc1" from 
the "public" statement in which case it instead complains "Error: Interface 
'proc1' of procedure 'proc_get' at (1) must be explicit". My understanding is 
that Proc1 should have an explicit interface as it's in a module.
Comment 1 janus 2011-10-07 18:22:47 UTC
(In reply to comment #0)
> :: arrSize
> end module Size_Mod

Apparently the first line of the module is missing:

module Size_Mod
Comment 2 janus 2011-10-07 18:45:43 UTC
Slight reduction of the original test case:


module m
 integer :: arrSize
end module


program p

  implicit none
  procedure(Proc) :: Proc_Get

contains

 function Proc (arg)
   use m
   double precision, dimension(arrSize) :: proc
   double precision :: arg
 end function

end


Fails with 4.5, 4.6 and trunk.
Comment 3 janus 2011-10-07 19:32:25 UTC
Here is a variant which gives the same ICE, but after a regular error message:


program p

  implicit none
  procedure(Proc) :: Proc_Get

contains

  function Proc (arg)
    integer :: arrSize
    double precision, dimension(arrSize) :: proc
    double precision :: arg
  end function

end



    double precision, dimension(arrSize) :: proc
                                1
Error: Variable 'arrsize' cannot appear in the expression at (1)
f951: internal compiler error: in replace_symbol, at fortran/expr.c:4155
Comment 4 janus 2011-10-07 20:36:34 UTC
I'm still not completely sure if the test case is valid or not. I could not find anything in the standard which forbids it, so I'm assuming it is valid for now.

In any case, here is a draft patch which fixes the ICE:

Index: gcc/fortran/expr.c
===================================================================
--- gcc/fortran/expr.c	(revision 179659)
+++ gcc/fortran/expr.c	(working copy)
@@ -4146,15 +4147,14 @@ replace_symbol (gfc_expr *expr, gfc_symbol *sym, i
 	   && !gfc_is_intrinsic (expr->symtree->n.sym, 0, expr->where)))
       && expr->symtree->n.sym->ns == sym->ts.interface->formal_ns)
     {
-      gfc_symtree *stree;
-      gfc_namespace *ns = sym->formal_ns;
-      /* Don't use gfc_get_symtree as we prefer to fail badly if we don't find
-	 the symtree rather than create a new one (and probably fail later).  */
-      stree = gfc_find_symtree (ns ? ns->sym_root : gfc_current_ns->sym_root,
-		      		expr->symtree->n.sym->name);
-      gcc_assert (stree);
-      stree->n.sym->attr = expr->symtree->n.sym->attr;
-      expr->symtree = stree;
+      gfc_symtree *root = sym->formal_ns ? sym->formal_ns->sym_root
+					 : gfc_current_ns->sym_root;
+      gfc_symtree *stree = gfc_find_symtree (root, expr->symtree->n.sym->name);
+      if (stree)
+	{
+	  stree->n.sym->attr = expr->symtree->n.sym->attr;
+	  expr->symtree = stree;
+	}
     }
   return false;
 }
Comment 5 janus 2011-10-07 20:43:20 UTC
(In reply to comment #2)
> Fails with 4.5, 4.6 and trunk.

Since it is accepted with 4.4, the ICE is clearly a regression.
Comment 6 janus 2011-10-07 23:05:59 UTC
(In reply to comment #4)
> In any case, here is a draft patch which fixes the ICE:

The patch in comment #4 regtests cleanly.

Of course it simply avoids the ICE by removing the assert, but I think that's really the correct thing to do here, since the assert is apparently based on false assumptions.
Comment 7 janus 2011-10-08 10:49:38 UTC
(In reply to comment #4)
> I'm still not completely sure if the test case is valid or not. I could not
> find anything in the standard which forbids it, so I'm assuming it is valid for
> now.

For what it's worth, the test case in comment #2 is accepted by g95, PGI and PathScale.

It is rejected by ifort with:

error #8169: The specified interface is not declared.   [PROC]
  procedure(Proc) :: Proc_Get
------------^

To my understanding, this is a bug in ifort. However, ifort does accept the following variant (which also ICEs with gfortran):


module m
  integer :: arrSize
end module

module m2
contains
  function Proc (arg)
    use m
    double precision, dimension(arrSize) :: proc
    double precision :: arg
  end function
end

program p
  use m2
  implicit none
  procedure(Proc) :: Proc_Get
end


In summary, this test case is accepted by ifort, g95, PGI and PathScale, which I think is strong empirical evidence that it is valid (though it's not a proof, of course). I don't have access to any other compiler which supports PROCEDURE statements.

Unless anyone can show me a restriction in the standard which makes it illegal, I'll continue to assume that this is an ICE-on-valid bug (which makes it even more severe).
Comment 8 Dominique d'Humieres 2011-10-08 10:55:10 UTC
Compiling the code in comment #7 with 4.4.6 gives an ICE:

[macbook] f90/bug% gfortran-fsf-4.4 pr50659_3.f90
pr50659_3.f90:8: internal compiler error: in replace_symbol, at fortran/expr.c:3630

while 4.4.6 does not give an ICE on the other tests.
Comment 9 janus 2011-10-08 11:11:32 UTC
(In reply to comment #8)
> Compiling the code in comment #7 with 4.4.6 gives an ICE:

... while it is accepted by 4.3.

Oh boy. So this thing is even a 4.4 regression!
Comment 10 janus 2011-10-08 11:19:07 UTC
One more side note: The following variant segfaults with 4.3:


module Test_Mod
 implicit none

contains

 subroutine Init
   procedure(Proc1) :: Proc_Get
 end subroutine

 function Proc1() result (res)
   double precision, dimension(3) :: res
 end function

end module


while 4.4 rejects it with:

   procedure(Proc1) :: Proc_Get
                               1
Error: Interface 'proc1' of procedure 'proc_get' at (1) must be explicit


Since this is not a regression (and 4.3 is not being maintained any more, anyway), it probably won't get fixed. From 4.5 on it is accepted.
Comment 11 janus 2011-10-08 15:08:25 UTC
Here is an alternative patch to the one in comment #4:

Index: gcc/fortran/expr.c
===================================================================
--- gcc/fortran/expr.c	(revision 179710)
+++ gcc/fortran/expr.c	(working copy)
@@ -4144,7 +4144,8 @@ replace_symbol (gfc_expr *expr, gfc_symbol *sym, i
   if ((expr->expr_type == EXPR_VARIABLE 
        || (expr->expr_type == EXPR_FUNCTION
 	   && !gfc_is_intrinsic (expr->symtree->n.sym, 0, expr->where)))
-      && expr->symtree->n.sym->ns == sym->ts.interface->formal_ns)
+      && expr->symtree->n.sym->ns == sym->ts.interface->formal_ns
+      && expr->symtree->n.sym->attr.dummy)
     {
       gfc_symtree *stree;
       gfc_namespace *ns = sym->formal_ns;


We simply forgot to check if the symbol we're replacing is actually a dummy argument! This even qualifies as obvious, I think ...
Comment 12 janus 2011-10-09 11:34:25 UTC
Author: janus
Date: Sun Oct  9 11:34:21 2011
New Revision: 179723

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

	PR fortran/50659
	* expr.c (replace_symbol): Only do replacement if the symbol is a dummy.

2011-10-09  Janus Weil  <janus@gcc.gnu.org>

	PR fortran/50659
	* gfortran.dg/proc_decl_27.f90: New.

Added:
    trunk/gcc/testsuite/gfortran.dg/proc_decl_27.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/expr.c
    trunk/gcc/testsuite/ChangeLog
Comment 13 janus 2011-10-12 18:53:59 UTC
Author: janus
Date: Wed Oct 12 18:53:55 2011
New Revision: 179864

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

	PR fortran/50659
	* expr.c (replace_symbol): Only do replacement if the symbol is a dummy.

2011-10-12  Janus Weil  <janus@gcc.gnu.org>

	PR fortran/50659
	* gfortran.dg/proc_decl_27.f90: New.

Added:
    branches/gcc-4_6-branch/gcc/testsuite/gfortran.dg/proc_decl_27.f90
Modified:
    branches/gcc-4_6-branch/gcc/fortran/ChangeLog
    branches/gcc-4_6-branch/gcc/fortran/expr.c
    branches/gcc-4_6-branch/gcc/testsuite/ChangeLog
Comment 14 janus 2011-10-13 15:04:05 UTC
Author: janus
Date: Thu Oct 13 15:03:58 2011
New Revision: 179923

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

	PR fortran/50659
	* expr.c (replace_symbol): Only do replacement if the symbol is a dummy.

2011-10-13  Janus Weil  <janus@gcc.gnu.org>

	PR fortran/50659
	* gfortran.dg/proc_decl_27.f90: New.

Added:
    branches/gcc-4_5-branch/gcc/testsuite/gfortran.dg/proc_decl_27.f90
Modified:
    branches/gcc-4_5-branch/gcc/fortran/ChangeLog
    branches/gcc-4_5-branch/gcc/fortran/expr.c
    branches/gcc-4_5-branch/gcc/testsuite/ChangeLog
Comment 15 janus 2011-10-15 12:16:22 UTC
Author: janus
Date: Sat Oct 15 12:16:13 2011
New Revision: 180032

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

	PR fortran/50659
	* expr.c (replace_symbol): Only do replacement if the symbol is a dummy.

2011-10-15  Janus Weil  <janus@gcc.gnu.org>

	PR fortran/50659
	* gfortran.dg/proc_decl_27.f90: New.

Added:
    branches/gcc-4_4-branch/gcc/testsuite/gfortran.dg/proc_decl_27.f90
Modified:
    branches/gcc-4_4-branch/gcc/fortran/ChangeLog
    branches/gcc-4_4-branch/gcc/fortran/expr.c
    branches/gcc-4_4-branch/gcc/testsuite/ChangeLog
Comment 16 janus 2011-10-15 12:18:17 UTC
The fix has landed on all branches from 4.4 on upwards, so we can finally close this one.
Comment 17 Dodji Seketeli 2011-10-20 12:37:02 UTC
Author: dodji
Date: Thu Oct 20 12:36:55 2011
New Revision: 180250

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=180250
Log:
Use @smallexample instead of @quotation in cppopts.texi

gcc/
	PR other/50659
	* doc/cppopts.texi: Use @smallexample/@end smallexample in
	documentation for -fdebug-cpp instead of @quotation/@end quotation
	that is not supported by contrib/texi2pod.pl.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/doc/cppopts.texi