[Bug fortran/37992] New: [4.4 Regression] ICE segfault for "character(len=len(x)) :: foo,x"

[Bug fortran/37992] New: [4.4 Regression] ICE segfault for "character(len=len(x)) :: foo,x"

[burnus at gcc dot gnu dot org]

Split off from PR 35040 comment 5. The following program gives now an ICE:

 function foo(x)
   character(len=len(x)) :: foo,x
 end function foo

With 4.3 there is no ICE, but with 4.4 there is an ICE after the (new!) error
detection. ("len(x)... :: x" is invalid.)

Valgrind shows a huge number of invalid reads.


-- 
           Summary: [4.4 Regression] ICE segfault for "character(len=len(x))
                    :: foo,x"
           Product: gcc
           Version: 4.4.0
            Status: UNCONFIRMED
          Keywords: ice-on-invalid-code
          Severity: normal
          Priority: P3
         Component: fortran
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: burnus at gcc dot gnu dot org


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37992

[Bug fortran/37992] [4.4 Regression] ICE segfault for "character(len=len(x)) :: foo,x"

[burnus at gcc dot gnu dot org]

------- Comment #1 from burnus at gcc dot gnu dot org  2008-11-01 22:38 -------
First valgrind error:

==21621== Invalid read of size 8
==21621==    at 0x46AAEA: gfc_resolve_expr (resolve.c:4248)
That is:
  if (e->symtree == NULL || e->symtree->n.sym == NULL)

I think e->expr_type == EXPR_FUNCTION, i.e. the access should be OK?!?


Second valgrind error:

==21621==  Address 0x65b2ef8 is 40 bytes inside a block of size 56 free'd
==21621==    at 0x4C243AF: free (in
/usr/lib64/valgrind/amd64-linux/vgpreload_memcheck.so)
==21621==    by 0x482C6A: gfc_delete_symtree (symbol.c:2269)

That's
  st0 = gfc_find_symtree (*root, name);
  st.name = gfc_get_string (name);
  gfc_delete_bbt (root, &st, compare_symtree);
  gfc_free (st0);  // <<<< Line 2269


Seemingly, the recovery after detecting the error half deletes a symbol. I
think the problem is the "foo" which exists not only as local symbol.

Note: If one adds "IMPLICIT NONE" the ICE disappears.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37992

[Bug fortran/37992] [4.4 Regression] ICE segfault for "character(len=len(x)) :: foo,x"

[janus at gcc dot gnu dot org]

------- Comment #2 from janus at gcc dot gnu dot org  2008-11-01 22:42 -------
This is really funny. If I compile a file which contains exactly the three
lines given in comment #0 (and nothing else), I get some errors, but no ICE.

However, if I add at least two blank lines (no matter where), the ICE appears.

Someone with a clear mind should check this, I feel a little paranoid ;)


-- 

janus at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2008-11-01 22:42:53
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37992

[Bug fortran/37992] [4.4 Regression] ICE segfault for "character(len=len(x)) :: foo,x"

[mikael dot morin at tele2 dot fr]

------- Comment #3 from mikael dot morin at tele2 dot fr  2008-11-02 17:50 -------
(In reply to comment #1)
> First valgrind error:
> 
> ==21621== Invalid read of size 8
> ==21621==    at 0x46AAEA: gfc_resolve_expr (resolve.c:4248)

> Second valgrind error:
> 
> ==21621==  Address 0x65b2ef8 is 40 bytes inside a block of size 56 free'd
> ==21621==    at 0x4C243AF: free (in
> /usr/lib64/valgrind/amd64-linux/vgpreload_memcheck.so)
> ==21621==    by 0x482C6A: gfc_delete_symtree (symbol.c:2269)


Those are the same error (look carefully, the second one is indented).
The first part indicates where the error appears. 
The second one precises the error (here access to a freed block) and where (in
this case) the free was. 


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37992

[Bug fortran/37992] [4.4 Regression] ICE segfault for "character(len=len(x)) :: foo,x"

[mikael dot morin at tele2 dot fr]

------- Comment #4 from mikael dot morin at tele2 dot fr  2008-11-02 18:03 -------
Created an attachment (id=16612)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=16612&action=view)
hackish patch

I think I got it. 
When the statement is rejected, all changes are reverted. 
However, the namespace is still holding the expression for len(x) in cl_list. 
So, when resolving len(x) we have a pointer to len's freed symtree. 

This patch solves the problem by adding a old_cl_list field which is copied
back to cl_list if the statement is rejected. 

I'm not sure it is the right way to do it though. 
As an inquiry function, len should always be reachable, whatever happens. 
What's your opinion?


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37992

[Bug fortran/37992] [4.4 Regression] ICE segfault for "character(len=len(x)) :: foo,x"

[dominiq at lps dot ens dot fr]

------- Comment #5 from dominiq at lps dot ens dot fr  2008-11-03 11:02 -------
The patch in comment #4 generate a lot of bus errors in my tests. Looking at
it, I think there is something missing: gfc_current_ns->old_cl_list is only set
to NULL, it should likely be set to gfc_current_ns->cl_list somewhere.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37992

[Bug fortran/37992] [4.4 Regression] ICE segfault for "character(len=len(x)) :: foo,x"

[pault at gcc dot gnu dot org]

------- Comment #6 from pault at gcc dot gnu dot org  2008-11-04 07:59 -------
(In reply to comment #5)
> The patch in comment #4 generate a lot of bus errors in my tests. Looking at
> it, I think there is something missing: gfc_current_ns->old_cl_list is only set
> to NULL, it should likely be set to gfc_current_ns->cl_list somewhere.
> 
It should not be necessary to do anything to the cl_list.  As long as nothing
points to a member, it can do nothing and gets cleaned up at the end of
compilation. The reason for this is that any number, including zero, of
typespecs can point to it, so it is very risky to delete a cl_list member.

Paul

Paul


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37992

[Bug fortran/37992] [4.4 Regression] ICE segfault for "character(len=len(x)) :: foo,x"

[mikael at gcc dot gnu dot org]

-- 

mikael at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |mikael at gcc dot gnu dot
                   |dot org                     |org
             Status|NEW                         |ASSIGNED
   Last reconfirmed|2008-11-01 22:42:53         |2008-11-05 14:02:04
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37992

[Bug fortran/37992] ICE while resolving charlen for rejected statements

[mikael at gcc dot gnu dot org]

------- Comment #7 from mikael at gcc dot gnu dot org  2008-11-05 17:02 -------
(In reply to comment #6)
> It should not be necessary to do anything to the cl_list.  As long as nothing
> points to a member, it can do nothing and gets cleaned up at the end of
> compilation. The reason for this is that any number, including zero, of
> typespecs can point to it, so it is very risky to delete a cl_list member.
Yes, but as we remove elements in the reject_statement function, any typespec
pointing to it should have been released already (as long as we remove new
elements only).
Patch coming. I don't know why it didn't work the first time. 
> 
> Paul
> 
> Paul
> 
No need to insist, we know it's you :p

This is not a regression, I have this failing as well with 4.3.2:
 function foo(x)
   integer :: x
   character(len=10) :: y
   character(len=len(y)) :: foo,x
 end function foo

Changing summary, from:
[4.4 Regression] ICE segfault for "character(len=len(x)) :: foo,x"
to:
ICE while resolving charlen for rejected statements


-- 

mikael at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[4.4 Regression] ICE        |ICE while resolving charlen
                   |segfault for                |for rejected statements
                   |"character(len=len(x)) ::   |
                   |foo,x"                      |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37992

[Bug fortran/37992] ICE while resolving charlen for rejected statements

[mikael at gcc dot gnu dot org]

------- Comment #8 from mikael at gcc dot gnu dot org  2008-11-05 18:47 -------
Patch here:
http://gcc.gnu.org/ml/fortran/2008-11/msg00032.html


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37992

[Bug fortran/37992] ICE while resolving charlen for rejected statements

[dominiq at lps dot ens dot fr]

------- Comment #9 from dominiq at lps dot ens dot fr  2008-11-05 22:29 -------
The patch in http://gcc.gnu.org/ml/fortran/2008-11/msg00032.html works as
advertised without regression on i686-apple-darwin9 (note that it is not a
review!-).

Note also that there are other similar instances for which gfortran gives an
ICE after error messages and that are not fixed by the patch, see:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36192#c6
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37744#c1


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37992

[Bug fortran/37992] ICE while resolving charlen for rejected statements

[mikael at gcc dot gnu dot org]

------- Comment #10 from mikael at gcc dot gnu dot org  2008-11-16 20:45 -------
Subject: Bug 37992

Author: mikael
Date: Sun Nov 16 20:44:33 2008
New Revision: 141927

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=141927
Log:
2008-11-16  Mikael Morin  <mikael.morin@tele2.fr>

        PR fortran/37992
        * gfortran.h (gfc_namespace): Added member old_cl_list, 
        backup of cl_list.
        (gfc_free_charlen): Added prototype.
        * symbol.c (gfc_free_charlen): New function.
        (gfc_free_namespace): Use gfc_free_charlen.
        * parse.c (next_statement): Backup gfc_current_ns->cl_list.
        (reject_statement): Restore gfc_current_ns->cl_list.
        Free cl_list's elements before dropping them.

2008-11-16  Mikael Morin  <mikael.morin@tele2.fr>

        PR fortran/37992
        * gfotran.dg/charlen_free_1.f90: New test.


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


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37992

[Bug fortran/37992] ICE while resolving charlen for rejected statements

[mikael at gcc dot gnu dot org]

------- Comment #11 from mikael at gcc dot gnu dot org  2008-11-16 21:05 -------
Fixed on trunk, closing


(In reply to comment #9)
> Note also that there are other similar instances for which gfortran gives an
> ICE after error messages and that are not fixed by the patch, see:

Those are ice-on-invalid-after-reporting-an-error. 
They are quite low in the priority list (after wrong-code, ice-on-valid,
ice-on-invalid) and not for trunk at this stage unless a trivial/obvious fix is
found for them.


-- 

mikael at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37992