[Bug fortran/106050] New: ICE in reject_statement, at fortran/parse.cc:2879

[Bug fortran/106050] New: ICE in reject_statement, at fortran/parse.cc:2879

[gscfq@t-online.de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106050

            Bug ID: 106050
           Summary: ICE in reject_statement, at fortran/parse.cc:2879
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: fortran
          Assignee: unassigned at gcc dot gnu.org
          Reporter: gscfq@t-online.de
  Target Milestone: ---

Affects versions down to r8 :
(out of order statement)


$ cat z1.f90
program p
   stop
   type t(k)
      integer, kind :: k = 4
   end type
end

$ cat z2.f90
program p
   continue
   type t(k)
      integer, kind :: k = 4
   end type
end

$ cat z3.f90
program p
   a = 1
   type t(k)
      integer, kind :: k = 4
   end type
end

$ cat z4.f90
program p
   print *
   type t(k)
      integer, kind :: k = 4
   end type
end


$ gfortran-13-20220619 -c z1.f90
z1.f90:3:12:

    3 |    type t(k)
      |            1
Error: Unexpected derived type declaration statement at (1)
f951: internal compiler error: Segmentation fault
0xe845df crash_signal
        ../../gcc/toplev.cc:322
0x70b53a delete_root
        ../../gcc/fortran/bbt.cc:150
0x70b6fe gfc_delete_bbt(void*, void*, int (*)(void*, void*))
        ../../gcc/fortran/bbt.cc:197
0x7deca8 gfc_delete_symtree(gfc_symtree**, char const*)
        ../../gcc/fortran/symbol.cc:2963
0x7e053d gfc_restore_last_undo_checkpoint()
        ../../gcc/fortran/symbol.cc:3708
0x7950f7 reject_statement
        ../../gcc/fortran/parse.cc:2879
0x79f8aa parse_progunit
        ../../gcc/fortran/parse.cc:6257
0x7a0ea1 gfc_parse_file()
        ../../gcc/fortran/parse.cc:6755
0x7ef83f gfc_be_parse_file
        ../../gcc/fortran/f95-lang.cc:229

[Bug fortran/106050] ICE in reject_statement, at fortran/parse.cc:2879

[kargl at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106050

kargl at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2022-06-21
                 CC|                            |kargl at gcc dot gnu.org

--- Comment #1 from kargl at gcc dot gnu.org ---
With torched input, I might expect the symbol table to get sideways.  Before
asserting the reference count is zero, check if the count is negative and an
error message has already been emitted.

diff --git a/gcc/fortran/symbol.cc b/gcc/fortran/symbol.cc
index 7a80dfd063b..3aa2b9bd962 100644
--- a/gcc/fortran/symbol.cc
+++ b/gcc/fortran/symbol.cc
@@ -3106,6 +3106,7 @@ gfc_free_symbol (gfc_symbol *&sym)
 void
 gfc_release_symbol (gfc_symbol *&sym)
 {
+
   if (sym == NULL)
     return;

@@ -3123,6 +3124,14 @@ gfc_release_symbol (gfc_symbol *&sym)
   if (sym->refs > 0)
     return;

+  if (sym->refs < 0)
+    {
+      int ecnt;
+      gfc_get_errors (NULL, &ecnt);
+      if (ecnt > 0)
+       return;
+    }
+
   gcc_assert (sym->refs == 0);
   gfc_free_symbol (sym);
 }

[Bug fortran/106050] ICE in reject_statement, at fortran/parse.cc:2879 since r8-3056-g5bab4c9631c478b7

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106050

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|ICE in reject_statement, at |ICE in reject_statement, at
                   |fortran/parse.cc:2879       |fortran/parse.cc:2879 since
                   |                            |r8-3056-g5bab4c9631c478b7
                 CC|                            |marxin at gcc dot gnu.org,
                   |                            |pault at gcc dot gnu.org

--- Comment #2 from Martin Liška <marxin at gcc dot gnu.org> ---
Likely started with r8-3056-g5bab4c9631c478b7, it was rejected before the
revision anyway.

[Bug fortran/106050] ICE in reject_statement, at fortran/parse.cc:2879 since r8-3056-g5bab4c9631c478b7

[kargl at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106050

kargl at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P4

[Bug fortran/106050] ICE in reject_statement, at fortran/parse.cc:2879 since r8-3056-g5bab4c9631c478b7

[pault at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106050

--- Comment #3 from Paul Thomas <pault at gcc dot gnu.org> ---
(In reply to Martin Liška from comment #2)
> Likely started with r8-3056-g5bab4c9631c478b7, it was rejected before the
> revision anyway.

With all branches up to 13-branch, I see:
../pr105594/pr106050.f90:3:12:

    3 |    type t(k)
      |            1
Error: Unexpected derived type declaration statement at (1)
(null):0: confused by earlier errors, bailing out

With trunk, I get:
../pr105594/pr106050.f90:3:12:

    3 |    type t(k)
      |            1
Error: Unexpected derived type declaration statement at (1)
f951: internal compiler error: Segmentation fault
0x101683f crash_signal
        ../../gcc/gcc/toplev.cc:314
0x9228ca delete_root
        ../../gcc/gcc/fortran/bbt.cc:150
0x922a8e gfc_delete_bbt(void*, void*, int (*)(void*, void*))
        ../../gcc/gcc/fortran/bbt.cc:197
0xa0923c gfc_delete_symtree(gfc_symtree**, char
......

I will be attaching a patch that works for me.

Paul

[Bug fortran/106050] ICE in reject_statement, at fortran/parse.cc:2879 since r8-3056-g5bab4c9631c478b7

[pault at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106050

--- Comment #4 from Paul Thomas <pault at gcc dot gnu.org> ---
Created attachment 55515
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=55515&action=edit
Patch that fixes this PR for me

I had to add to Steve's patch to get this PR sorted out.

Ideally of course, we would locate the culprit in the PDT instantiation and fix
that. However, I am disinclined to do that since PDTs require roots and
branches fixing. At least this patch is mostly harmless!

Paul

[Bug fortran/106050] ICE in reject_statement, at fortran/parse.cc:2879 since r8-3056-g5bab4c9631c478b7

[mikael at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106050

Mikael Morin <mikael at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mikael at gcc dot gnu.org

--- Comment #5 from Mikael Morin <mikael at gcc dot gnu.org> ---
When matching statement "type t(k)", two symbols are created, one for t and one
for k.
t is in gfc_current_ns and k is in t's f2k_derived namespace.
If the statement is rejected, both t and k need to be freed.
But one should care about ordering, as the release of t frees f2k_derived,
which is k's namespace, so k should be released before t.

I haven't checked that the above actually is the problem here, but it might be.
Possibly walking the symbols in reverse order to release them would fix this.

Regarding the patches posted, if sym->refs < 0 is true, then the memory for sym
has already been released and may be garbage (including sym->refs).  A crash is
as good as anything else at this point IMHO.

[Bug fortran/106050] ICE in reject_statement, at fortran/parse.cc:2879 since r8-3056-g5bab4c9631c478b7

[mikael at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106050

Mikael Morin <mikael at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |mikael at gcc dot gnu.org
             Status|NEW                         |ASSIGNED

--- Comment #6 from Mikael Morin <mikael at gcc dot gnu.org> ---
(In reply to Mikael Morin from comment #5)
> Possibly walking the symbols in reverse order to release them would fix this.
> 
It seems to work:

diff --git a/gcc/fortran/symbol.cc b/gcc/fortran/symbol.cc
index 37a9e8fa0ae..4a71d84b3fe 100644
--- a/gcc/fortran/symbol.cc
+++ b/gcc/fortran/symbol.cc
@@ -3661,7 +3661,7 @@ gfc_restore_last_undo_checkpoint (void)
   gfc_symbol *p;
   unsigned i;

-  FOR_EACH_VEC_ELT (latest_undo_chgset->syms, i, p)
+  FOR_EACH_VEC_ELT_REVERSE (latest_undo_chgset->syms, i, p)
     {
       /* Symbol in a common block was new. Or was old and just put in common
*/
       if (p->common_block

[Bug fortran/106050] ICE in reject_statement, at fortran/parse.cc:2879 since r8-3056-g5bab4c9631c478b7

[pault at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106050

--- Comment #7 from Paul Thomas <pault at gcc dot gnu.org> ---
(In reply to Mikael Morin from comment #6)
> (In reply to Mikael Morin from comment #5)
> > Possibly walking the symbols in reverse order to release them would fix this.
> > 
> It seems to work:
> 
> diff --git a/gcc/fortran/symbol.cc b/gcc/fortran/symbol.cc
> index 37a9e8fa0ae..4a71d84b3fe 100644
> --- a/gcc/fortran/symbol.cc
> +++ b/gcc/fortran/symbol.cc
> @@ -3661,7 +3661,7 @@ gfc_restore_last_undo_checkpoint (void)
>    gfc_symbol *p;
>    unsigned i;
>  
> -  FOR_EACH_VEC_ELT (latest_undo_chgset->syms, i, p)
> +  FOR_EACH_VEC_ELT_REVERSE (latest_undo_chgset->syms, i, p)
>      {
>        /* Symbol in a common block was new. Or was old and just put in
> common */
>        if (p->common_block

So does this:

diff --git a/gcc/fortran/symbol.cc b/gcc/fortran/symbol.cc
index 37a9e8fa0ae..ada8cb2ba83 100644
--- a/gcc/fortran/symbol.cc
+++ b/gcc/fortran/symbol.cc
@@ -3703,6 +3703,10 @@ gfc_restore_last_undo_checkpoint (void)
            }
          p->common_next = NULL;
        }
+
+      if (!strlen(p->name))
+       continue;
+
       if (p->gfc_new)
        {
          /* The derived type is saved in the symtree with the first

I think that yours is likely to be better though. Have you understood why the
symbol order matters in this case?

BTW pr99798 is another ref counting ICE; this time involving namespaces.

Paul

[Bug fortran/106050] ICE in reject_statement, at fortran/parse.cc:2879 since r8-3056-g5bab4c9631c478b7

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106050

--- Comment #8 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Mikael Morin <mikael@gcc.gnu.org>:

https://gcc.gnu.org/g:616a101848bfd5b61ffdd87ae9b1153139d916ca

commit r14-2507-g616a101848bfd5b61ffdd87ae9b1153139d916ca
Author: Mikael Morin <mikael@gcc.gnu.org>
Date:   Thu Jul 13 21:23:44 2023 +0200

    fortran: Release symbols in reversed order [PR106050]

    Release symbols in reversed order wrt the order they were allocated.
    This fixes an error recovery ICE in the case of a misplaced
    derived type declaration.  Such a declaration creates nested
    symbols, one for the derived type and one for each type parameter,
    which should be immediately released as the declaration is
    rejected.  This breaks if the derived type is released first.
    As the type parameter symbols are in the namespace of the derived
    type, releasing the derived type releases the type parameters, so
    one can't access them after that, even to release them.  Hence,
    the type parameters should be released first.

            PR fortran/106050

    gcc/fortran/ChangeLog:

            * symbol.cc (gfc_restore_last_undo_checkpoint): Release symbols
            in reverse order.

    gcc/testsuite/ChangeLog:

            * gfortran.dg/pdt_33.f90: New test.

[Bug fortran/106050] ICE in reject_statement, at fortran/parse.cc:2879 since r8-3056-g5bab4c9631c478b7

[mikael at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106050

Mikael Morin <mikael at gcc dot gnu.org> changed:

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

--- Comment #9 from Mikael Morin <mikael at gcc dot gnu.org> ---
Fixed for gfortran 14.  Closing.

[Bug fortran/106050] ICE in reject_statement, at fortran/parse.cc:2879 since r8-3056-g5bab4c9631c478b7

[mikael at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106050

Mikael Morin <mikael at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |14.0