Bug 44660 - ICE in resolve_equivalence()
Summary: ICE in resolve_equivalence()
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: fortran (show other bugs)
Version: 4.6.0
: P3 normal
Target Milestone: 4.4.5
Assignee: Mikael Morin
URL:
Keywords: ice-on-invalid-code, patch
Depends on:
Blocks:
 
Reported: 2010-06-25 02:36 UTC by Sebastian Pop
Modified: 2010-08-06 17:19 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work: 4.3.6 4.4.5 4.5.1 4.6.0
Known to fail:
Last reconfirmed: 2010-07-24 19:52:15


Attachments
patch against my (diry) tree (853 bytes, patch)
2010-06-25 10:33 UTC, Mikael Morin
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sebastian Pop 2010-06-25 02:36:47 UTC
GCC trunk is ICEing in the fortran front-end while compiling this:

$ cat ./bug.f
  165        CONTINUE
      EQUIVALENCE (CHECK, CHECK_STR)
      DATA CHECK_STR/"CHECK   "/
      END

$ ./f951 bug.f
bug.f:2.72:

      EQUIVALENCE (CHECK, CHECK_STR)                                    
                                                                        1
Error: Unexpected EQUIVALENCE statement at (1)
bug.f:3.10:

      DATA CHECK_STR/"CHECK   "/                                        
          1
Error: Incompatible types in DATA statement at (1); attempted conversion of CHARACTER(1) to REAL(4)
f951: internal compiler error: Segmentation fault
Please submit a full bug report,
with preprocessed source if appropriate.
See <http://gcc.gnu.org/bugs.html> for instructions.

The ICE happens in:
(gdb) bt
#0  0x000000000053dcfe in resolve_equivalence (eq=0x17e6d60) at ../../gcc/fortran/resolve.c:12363
#1  0x000000000053ec22 in resolve_types (ns=0x1835640) at ../../gcc/fortran/resolve.c:12788
#2  0x000000000053ee9d in gfc_resolve (ns=0x1835640) at ../../gcc/fortran/resolve.c:12849
#3  0x000000000051dcff in gfc_parse_file () at ../../gcc/fortran/parse.c:4275
#4  0x000000000055e86f in gfc_be_parse_file (set_yydebug=0) at ../../gcc/fortran/f95-lang.c:234
#5  0x0000000000a1c7b0 in compile_file () at ../../gcc/toplev.c:1044
#6  0x0000000000a1ec33 in do_compile () at ../../gcc/toplev.c:2441
#7  0x0000000000a1ed09 in toplev_main (argc=5, argv=0x7fffffffe868) at ../../gcc/toplev.c:2483
#8  0x00000000005e69b0 in main (argc=5, argv=0x7fffffffe868) at ../../gcc/main.c:35
Comment 1 kargls 2010-06-25 04:02:15 UTC
Index: resolve.c
===================================================================
--- resolve.c   (revision 161047)
+++ resolve.c   (working copy)
@@ -12506,6 +12506,9 @@ resolve_equivalence (gfc_equiv *eq)
   int object, cnt_protected;
   const char *msg;
 
+  if (eq->expr->symtree->n.sym == NULL)
+    return;
+
   last_ts = &eq->expr->symtree->n.sym->ts;
 
   first_sym = eq->expr->symtree->n.sym;
Comment 2 Jerry DeLisle 2010-06-25 04:35:14 UTC
Confirmed. I came up with the exact same patch and it does pass regression testing, of course. Collided when I tried to post this. :)
Comment 3 kargls 2010-06-25 04:42:46 UTC
(In reply to comment #2)
> Confirmed. I came up with the exact same patch and it does pass regression
> testing, of course. Collided when I tried to post this. :)
> 

:)

The mangled Fortran code caught my eye.  I'm actually wondering
where Sebastian found this gem.
Comment 4 sebpop@gmail.com 2010-06-25 05:32:52 UTC
Subject: Re:  [regression 4.4/4.5/4.6] ICE in 
	resolve_equivalence()

On Thu, Jun 24, 2010 at 23:42, kargl at gcc dot gnu dot org
<gcc-bugzilla@gcc.gnu.org> wrote:
> The mangled Fortran code caught my eye.  I'm actually wondering
> where Sebastian found this gem.

I was reducing a graphite ICE in gamess that turned out into
a fortran front-end ICE...  With your fix I will start reducing
again my ICE ;-)

Thanks,
Sebastian
Comment 5 sebpop@gmail.com 2010-06-25 05:49:23 UTC
Subject: Re:  [regression 4.4/4.5/4.6] ICE in 
	resolve_equivalence()

On Thu, Jun 24, 2010 at 23:02, kargl at gcc dot gnu dot org
<gcc-bugzilla@gcc.gnu.org> wrote:
>
>
> ------- Comment #1 from kargl at gcc dot gnu dot org  2010-06-25 04:02 -------
> Index: resolve.c
> ===================================================================
> --- resolve.c   (revision 161047)
> +++ resolve.c   (working copy)
> @@ -12506,6 +12506,9 @@ resolve_equivalence (gfc_equiv *eq)
>   int object, cnt_protected;
>   const char *msg;
>
> +  if (eq->expr->symtree->n.sym == NULL)
> +    return;
> +
>   last_ts = &eq->expr->symtree->n.sym->ts;
>
>   first_sym = eq->expr->symtree->n.sym;
>
>

This patch doesn't fix the problem I am seeing.  If I'm testing this
in the loop before taking the value of e->symtree->n.sym->ts, then it
passes without ICE:

diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c
index 48bb618..7f66be4 100644
--- a/gcc/fortran/resolve.c
+++ b/gcc/fortran/resolve.c
@@ -12360,6 +12360,9 @@ resolve_equivalence (gfc_equiv *eq)
     {
       e = eq->expr;

+      if (eq->expr->symtree->n.sym == NULL)
+	return;
+
       e->ts = e->symtree->n.sym->ts;
       /* match_varspec might not know yet if it is seeing
 	 array reference or substring reference, as it doesn't
Comment 6 sebpop@gmail.com 2010-06-25 06:07:49 UTC
Subject: Re:  [regression 4.4/4.5/4.6] ICE in 
	resolve_equivalence()

These previous patches don't seem to solve the problem:
here is another reduced case that still fails in resolve_equivalence
at a different place than before.

$ cat bug.f
         CALL TRFWTM(JKT,XX,NX,Y,NIX,NORB2,1,TOL)
         IF(DBUG.AND.NX.GT.0) THEN
      EQUIVALENCE (DBUGME, DBUGME_STR)
      END IF
      END
Comment 7 kargls 2010-06-25 06:14:30 UTC
(In reply to comment #5)
> Subject: Re:  [regression 4.4/4.5/4.6] ICE in 
>         resolve_equivalence()
> 
> On Thu, Jun 24, 2010 at 23:02, kargl at gcc dot gnu dot org
> <gcc-bugzilla@gcc.gnu.org> wrote:
> >
> >
> > Comment #1 from kargl at gcc dot gnu dot org  2010-06-25 04:02 -------
> > Index: resolve.c
> > ===================================================================
> > --- resolve.c   (revision 161047)
> > +++ resolve.c   (working copy)
> > @@ -12506,6 +12506,9 @@ resolve_equivalence (gfc_equiv *eq)


> This patch doesn't fix the problem I am seeing.  If I'm testing this
> in the loop before taking the value of e->symtree->n.sym->ts, then it
> passes without ICE:

We'll need another test case then.   However, I note that...

> diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c
> index 48bb618..7f66be4 100644
> --- a/gcc/fortran/resolve.c
> +++ b/gcc/fortran/resolve.c
> @@ -12360,6 +12360,9 @@ resolve_equivalence (gfc_equiv *eq)

... there is a 200 line difference in the location of your
diff and my clean trunk.  Do you have other changes
in your source code?

Comment 8 Tobias Burnus 2010-06-25 06:20:09 UTC
Comment 6 prints correctly the error that EQUIVALENCE is unexpected, but then segfaults. Valgrind shows:

==11477== Invalid read of size 8
==11477==    at 0x52B7BB: resolve_types (resolve.c:12544)
==11477==    by 0x52504C: gfc_resolve (resolve.c:13040)

==11477== Invalid read of size 8
==11477==    at 0x52B84E: resolve_types (resolve.c:12554)
==11477==    by 0x52504C: gfc_resolve (resolve.c:13040)

That's in resolve_equivalence. Line 12544 is:
  last_ts = &eq->expr->symtree->n.sym->ts;
and line 12554 is
      e->ts = e->symtree->n.sym->ts;
Comment 9 sebpop@gmail.com 2010-06-25 06:24:42 UTC
Subject: Re:  [regression 4.4/4.5/4.6] ICE in 
	resolve_equivalence()

On Fri, Jun 25, 2010 at 01:14, kargl at gcc dot gnu dot org
<gcc-bugzilla@gcc.gnu.org> wrote:
> ... there is a 200 line difference in the location of your
> diff and my clean trunk.  Do you have other changes
> in your source code?

Sorry, my patch was against the graphite branch, last merged on
2010-06-07, r160224 from trunk.

Sebastian
Comment 10 kargls 2010-06-25 06:29:50 UTC
(In reply to comment #6)
> Subject: Re:  [regression 4.4/4.5/4.6] ICE in 
>         resolve_equivalence()
> 
> These previous patches don't seem to solve the problem:
> here is another reduced case that still fails in resolve_equivalence
> at a different place than before.
> 
> $ cat bug.f
>          CALL TRFWTM(JKT,XX,NX,Y,NIX,NORB2,1,TOL)
>          IF(DBUG.AND.NX.GT.0) THEN
>       EQUIVALENCE (DBUGME, DBUGME_STR)
>       END IF
>       END

What language is GAMESS written in?  The above simply is not
Fortran.  EQUIVALENCE is a specification-statement.  It cannot
appear after an executable-statement.

Comment 11 Tobias Burnus 2010-06-25 09:17:28 UTC
(In reply to comment #10)
> What language is GAMESS written in? 

Fortran, of course. See: http://www.spec.org/cpu2006/Docs/416.gamess.html
and http://www.msg.ameslab.gov/GAMESS/GAMESS.html

> The above simply is not Fortran.  EQUIVALENCE is a specification-statement.
> It cannot appear after an executable-statement.

Well, it is invalid code - based on a valid Fortran code. If you use Delta to reduce a test case (cf. http://gcc.gnu.org/wiki/A_guide_to_testcase_reduction), it simply removes lines of the source code using a constraint. I think Sebastian's constraint was that it segfaults. This seemingly happens for the unmodified gamess due to a Graphite bug - but also due to a buggy error recovery in gfortran.

Thus, this PR is about an ice-on-INVALID-code and error-recovery bug, thus, it has a lower priority than wrong-code bugs; nevertheless, it is a regression.

 * * *

Regarding the PR: For the test case in comment 0, one finds in gdb:

(gdb) p eq->expr->symtree->n.sym
$1 = (gfc_symbol *) 0x13fae6d
(gdb) p eq->expr->symtree->n.sym->name
$2 = 0x0
(gdb) p eq->expr->symtree->n.sym->ts
$3 = {type = 0, kind = 0, u = {derived = 0x0, cl = 0x0}, interface = 0x0, is_c_interop = 0, is_iso_c = 0, f90_type = 0}

That means that the symbol is only half present, i.e. it exists in the symtree, but seemingly, the values have not been set.


The next item in the list is eq->eq, for which
  (gdb) p eq->eq->expr
  $8 = (gfc_expr *) 0x1395700
  (gdb) p eq->eq->expr->symtree
  $9 = (gfc_symtree *) 0x13a3110
but
  (gdb) p eq->eq->expr->symtree->n.sym
  $10 = (gfc_symbol *) 0x0

Thus, for the case, adding a patch like in comment 1 and in comment 5 works. Even with both checks added, it will fail later (for comment 0's test case) because of
  12642         if (sym->ns->proc_name
as sym->ns == NULL.

One can, of course, put more checks in, e.g., whether eq->expr->symtree->n.sym->name == NULL or whether sym->ns == NULL, but the question is whether one should not fix it earlier.

Currently, the program is walked like this:

  parse_progunit -> parse_executable -> next_statement

if the next_statement is not an executable statement (which ST_EQUIVALENCE is not), parse_executable returns - and parse_progunit calls:
      unexpected_statement (st);
      reject_statement ();

Thus, seemingly, reject_statement leaves the symtree in a half-existing state. It calls:  gfc_undo_symbols () and undo_new_statement (). I think the former one only does a partial cleanup.
Comment 12 Mikael Morin 2010-06-25 10:32:01 UTC
(In reply to comment #11)
> Thus, seemingly, reject_statement leaves the symtree in a half-existing state.
> It calls:  gfc_undo_symbols () and undo_new_statement (). I think the former
> one only does a partial cleanup.
> 

My understanding is that gfc_undo_symbols indeed cleans the symbol, but the old symbol pointer escapes through the equivalence list.
Comment 13 Mikael Morin 2010-06-25 10:33:41 UTC
Created attachment 21003 [details]
patch against my (diry) tree

patch restoring the old equivalence list on reject_statement ()
Comment 14 kargls 2010-06-25 17:14:40 UTC
(In reply to comment #11)

> 
> Well, it is invalid code - based on a valid Fortran code. If you use Delta to
> reduce a test case (cf. http://gcc.gnu.org/wiki/A_guide_to_testcase_reduction),
> it simply removes lines of the source code using a constraint. I think
> Sebastian's constraint was that it segfaults. This seemingly happens for the
> unmodified gamess due to a Graphite bug - but also due to a buggy error
> recovery in gfortran.

Well, then Delta is broken for fortran.  Reducing valid fortran that is
causing a compiler issue to some invalid fortran fragment that may or 
(more than likely) may not have anything to do with the original issue
is a waste of time. 
Comment 15 Jakub Jelinek 2010-06-25 18:11:36 UTC
Well, the compiler shouldn't ICE on invalid input.

And, delta is just a simple text tool, it depends on the user how he writes the test script.  Ideally when reducing a testcase to find an ICE the delta test script should check just for the ICE that was reported originally, and if the original source didn't report any errors other than the ICE the script shouldn't allow any errors in the source either.  I'm usually using something like:
#!/bin/sh
/usr/src/gcc/obj/gcc/cc1 -quiet -g -O2 -m32 -fPIC $1 2>&1 \
  | awk \
    '/output_operand/{seen+=32;next}/error:/{exit 1}END{if (seen!=32)exit 1}'
if ! test $? == 0; then
  exit 1
fi
exit 0

so it doesn't match unrelated ICEs and doesn't match any error: lines (except for the ICE it is looking for).
Comment 16 Tobias Burnus 2010-07-22 09:43:49 UTC
(In reply to comment #13)
> Created an attachment (id=21003) [edit]
> patch against my (diry) tree
> patch restoring the old equivalence list on reject_statement ()

Mikael, what's the status of this patch?
Comment 17 Jerry DeLisle 2010-07-23 03:05:44 UTC
With 4.5 about to pop a release it would be good to get this fixed for it, with release manager approval
Comment 18 Mikael Morin 2010-07-24 19:52:15 UTC
(In reply to comment #16)
> Mikael, what's the status of this patch?
> 
Forgotten :-/

(In reply to comment #17)
> With 4.5 about to pop a release it would be good to get this fixed for it, with
> release manager approval
> 
I test tonight and I propose the patch tomorrow.

Comment 19 Mikael Morin 2010-07-25 17:01:36 UTC
Subject: Bug 44660

Author: mikael
Date: Sun Jul 25 17:01:15 2010
New Revision: 162516

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=162516
Log:
2010-07-25  Mikael Morin  <mikael@gcc.gnu.org>

	PR fortran/44660
	* gfortran.h (gfc_namespace): New field old_equiv.
	(gfc_free_equiv_until): New prototype.
	* match.c (gfc_free_equiv_until): New, renamed from gfc_free_equiv with
	a parameterized stop condition.
	(gfc_free_equiv): Use gfc_free_equiv_until.
	* parse.c (next_statement): Save equivalence list.
	(reject_statement): Restore equivalence list. 


Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/gfortran.h
    trunk/gcc/fortran/match.c
    trunk/gcc/fortran/parse.c

Comment 20 Mikael Morin 2010-07-27 12:07:04 UTC
4.6 done, backports pending.
Comment 21 Mikael Morin 2010-08-05 21:38:55 UTC
Subject: Bug 44660

Author: mikael
Date: Thu Aug  5 21:38:36 2010
New Revision: 162922

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=162922
Log:
2010-08-05  Mikael Morin  <mikael@gcc.gnu.org>

	PR fortran/44660
	* gfortran.h (gfc_namespace): New field old_equiv.
	(gfc_free_equiv_until): New prototype.
	* match.c (gfc_free_equiv_until): New, renamed from gfc_free_equiv with
	a parameterized stop condition.
	(gfc_free_equiv): Use gfc_free_equiv_until.
	* parse.c (next_statement): Save equivalence list.
	(reject_statement): Restore equivalence list. 


Modified:
    branches/gcc-4_5-branch/gcc/fortran/ChangeLog
    branches/gcc-4_5-branch/gcc/fortran/gfortran.h
    branches/gcc-4_5-branch/gcc/fortran/match.c
    branches/gcc-4_5-branch/gcc/fortran/parse.c

Comment 22 Mikael Morin 2010-08-05 21:40:38 UTC
Only 4.4 left. 
Let's not weaken now.
Comment 23 Mikael Morin 2010-08-06 17:17:54 UTC
Subject: Bug 44660

Author: mikael
Date: Fri Aug  6 17:17:37 2010
New Revision: 162949

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=162949
Log:
2010-08-06  Mikael Morin  <mikael@gcc.gnu.org>

	PR fortran/44660
	* gfortran.h (gfc_namespace): New field old_equiv.
	(gfc_free_equiv_until): New prototype.
	* match.c (gfc_free_equiv_until): New, renamed from gfc_free_equiv with
	a parameterized stop condition.
	(gfc_free_equiv): Use gfc_free_equiv_until.
	* parse.c (next_statement): Save equivalence list.
	(reject_statement): Restore equivalence list. 


Modified:
    branches/gcc-4_4-branch/gcc/fortran/ChangeLog
    branches/gcc-4_4-branch/gcc/fortran/gfortran.h
    branches/gcc-4_4-branch/gcc/fortran/match.c
    branches/gcc-4_4-branch/gcc/fortran/parse.c

Comment 24 Mikael Morin 2010-08-06 17:19:05 UTC
One more down