[Bug fortran/103506] [10/11/12/13 Regression] ICE in gfc_free_namespace, at fortran/symbol.c:4039 since r10-2798-ge68a35ae4a65d2b3
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103506 --- Comment #15 from CVS Commits --- The master branch has been updated by Jerry DeLisle : https://gcc.gnu.org/g:8011fbba7baa46947341ca8069b5a327163a68d5 commit r13-5485-g8011fbba7baa46947341ca8069b5a327163a68d5 Author: Jerry DeLisle Date: Sat Jan 28 20:00:34 2023 -0800 ICE in gfc_free_namespace. ice-on-invalid. PR fortran/103506 gcc/fortran/ChangeLog: * parse.cc (parse_module): Remove use of a bool error value that prevented proper setting of the namespace pointer. gcc/testsuite/ChangeLog: * gfortran.dg/pr103506_1.f90: New test.
[Bug fortran/103506] [10/11/12/13 Regression] ICE in gfc_free_namespace, at fortran/symbol.c:4039 since r10-2798-ge68a35ae4a65d2b3
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103506
--- Comment #14 from Jerry DeLisle ---
This is interesting. Simply doing the following eliminates the ice.
diff --git a/gcc/fortran/parse.cc b/gcc/fortran/parse.cc
index 0fb19cc9f0f..a9e538cc2a1 100644
--- a/gcc/fortran/parse.cc
+++ b/gcc/fortran/parse.cc
@@ -6544,8 +6544,6 @@ loop:
default:
gfc_error ("Unexpected %s statement in MODULE at %C",
gfc_ascii_statement (st));
-
- error = true;
reject_statement ();
st = next_statement ();
goto loop;
I discovered this studying other code that was rejecting statements and found
the following existing function:
/* Generic complaint about an out of order statement. We also do
whatever is necessary to clean up. */
static void
unexpected_statement (gfc_statement st)
{
gfc_error ("Unexpected %s statement at %C", gfc_ascii_statement (st));
reject_statement ();
}
I tried it at the location of the patch just above and it worked. The error
message is generic but no ice. In fact the only difference is setting
error=true.
With the generic error message there are a couple of existing test cases that
don't pass but these would be minor edits.
All this begs the question about why set error=true. Right after this the code
on parse.cc has:
/* Make sure not to free the namespace twice on error. */
if (!error)
s->ns = gfc_current_ns;
Well, setting it true s->ns is not getting set at all. Meaning it is left
uninitialized. The comment about freeing the namespace is also bogus here.
There is no freeing of anything in this chunk of code.
[Bug fortran/103506] [10/11/12/13 Regression] ICE in gfc_free_namespace, at fortran/symbol.c:4039 since r10-2798-ge68a35ae4a65d2b3
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103506 --- Comment #13 from Jerry DeLisle --- (In reply to anlauf from comment #11) > (In reply to Jerry DeLisle from comment #8) > > Doing the search in bugzilla, 137 bugs are marked as ic-on-invalid-code. I > > suggest we make all of these P5 or Wont fix. > > Please don't make them wont-fix. I would not do anything like that unilaterally. I am exploring how we can sort these things out by priority. I am also aware that different contributors may have different priorities. Regarding ice-on-invalid being possible indicators of a problem elsewhere. This very bug is one of those. I have a variation on the patch I posted in comment #5 where the initial error occurs. There is a loop in the parsing there that bothers me. I am not done yet.
[Bug fortran/103506] [10/11/12/13 Regression] ICE in gfc_free_namespace, at fortran/symbol.c:4039 since r10-2798-ge68a35ae4a65d2b3
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103506 --- Comment #12 from Thomas Koenig --- (In reply to anlauf from comment #11) > (In reply to Jerry DeLisle from comment #8) > > Doing the search in bugzilla, 137 bugs are marked as ic-on-invalid-code. I > > suggest we make all of these P5 or Wont fix. > > Please don't make them wont-fix. I concur. While it is annoying to have that many bugs, everybody has their own priorities, and if somebody wants to spend their time fixing them, there is no reason not to, and certainly no reason to remove them from the search (which is what marking them as wont-fix would do). > An ICE is always a bug, as we used to explain everybody who asked, > and I still consider it that way. Agreed. > If you must, make them P5, but didn't we have P5 for enhancements, > or (missing/wrong) diagnostics? Not sure about that. I hardly look at priorites, anyway, except for the really high ones. > Also, an ICE-on-invalid input that confuses the parser points at a > different part of the compiler than an ICE that happens during resolution, > simplification, frontend-optimization, translation. > > In several cases, an ICE in one of the trans*.cc was caused by an issue > much earlier in the process. > > One problem is that there are lots of PRs that are - either seemingly or > likely - related. A good (better?) classification of bugs to find those > that might be connected or near-duplicates would be helpful. > > I once tried to edit the summary of some bugs that were e.g. coarray-related, > or OOP; not sure if that was appreciated. We have some meta-bugs for coarray-related stuff. Coarry-related bugs can be set as blocking Coarray (aka PR83700). > (We could more aggressively mark PRs as F2018 or F2023.) There is the F2018 meta-bug, aka 85836, and I have just created F2023, aka PR108577. This is probably the best way to track these PRs - just mark them as blocking the relevant standard PR. People who are interested in following those can just put themselves on the CC list. > Also, there are several bugs pertaining only to CLASS. Some of those > would be addressed along with the fix for PR106856. Tobias' patch plus > some minor fixup to it seems to solve many of them. I don't think we have a class meta-bug, but I'm not sure.
[Bug fortran/103506] [10/11/12/13 Regression] ICE in gfc_free_namespace, at fortran/symbol.c:4039 since r10-2798-ge68a35ae4a65d2b3
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103506
anlauf at gcc dot gnu.org changed:
What|Removed |Added
CC||anlauf at gcc dot gnu.org
--- Comment #11 from anlauf at gcc dot gnu.org ---
(In reply to Jerry DeLisle from comment #8)
> Doing the search in bugzilla, 137 bugs are marked as ic-on-invalid-code. I
> suggest we make all of these P5 or Wont fix.
Please don't make them wont-fix.
An ICE is always a bug, as we used to explain everybody who asked,
and I still consider it that way.
If you must, make them P5, but didn't we have P5 for enhancements,
or (missing/wrong) diagnostics?
Also, an ICE-on-invalid input that confuses the parser points at a
different part of the compiler than an ICE that happens during resolution,
simplification, frontend-optimization, translation.
In several cases, an ICE in one of the trans*.cc was caused by an issue
much earlier in the process.
One problem is that there are lots of PRs that are - either seemingly or
likely - related. A good (better?) classification of bugs to find those
that might be connected or near-duplicates would be helpful.
I once tried to edit the summary of some bugs that were e.g. coarray-related,
or OOP; not sure if that was appreciated.
(We could more aggressively mark PRs as F2018 or F2023.)
Also, there are several bugs pertaining only to CLASS. Some of those
would be addressed along with the fix for PR106856. Tobias' patch plus
some minor fixup to it seems to solve many of them.
However, that patch creates a regression in one ("invalid") testcase because
of a corruption of one list, leading to a segfault in restore_old_symbol.
I think it is not the right way to try to make the cleanup more robust; this
would only sweep the actual issues under the rug. It's the cause of
corruption of lists etc. that should matter.
Recovering memory or garbage collection should really be lower on our list.
[Bug fortran/103506] [10/11/12/13 Regression] ICE in gfc_free_namespace, at fortran/symbol.c:4039 since r10-2798-ge68a35ae4a65d2b3
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103506 --- Comment #10 from Steve Kargl --- On Fri, Jan 27, 2023 at 02:06:12AM +, jvdelisle at gcc dot gnu.org wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103506 > > --- Comment #7 from Jerry DeLisle --- > > Well we sure are not going to completely rewrite gfortran. Sometimes I think > we just ought to accept ice-on-invalid and just toss out all those bug reports > as the effort to fix them is not worth the benefit from doing so. > A number of these would go away if gfortran set -fmax-error=1 as the default.
[Bug fortran/103506] [10/11/12/13 Regression] ICE in gfc_free_namespace, at fortran/symbol.c:4039 since r10-2798-ge68a35ae4a65d2b3
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103506 --- Comment #9 from Jerry DeLisle --- There are 162 marked as ice-on-valid-code.
[Bug fortran/103506] [10/11/12/13 Regression] ICE in gfc_free_namespace, at fortran/symbol.c:4039 since r10-2798-ge68a35ae4a65d2b3
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103506 --- Comment #8 from Jerry DeLisle --- Doing the search in bugzilla, 137 bugs are marked as ic-on-invalid-code. I suggest we make all of these P5 or Wont fix. As my time and others is scarce, I plan to focus on the valid-code bugs. This one was a good one for me to study, but that is the only value I got out of it.
[Bug fortran/103506] [10/11/12/13 Regression] ICE in gfc_free_namespace, at fortran/symbol.c:4039 since r10-2798-ge68a35ae4a65d2b3
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103506 --- Comment #7 from Jerry DeLisle --- The only other way would be some sort of built in memory management scheme that would guarantee all "objects" are freed implicitly. Of course gfortran itself implements this type of thing as does I think C++ and Rust. Well we sure are not going to completely rewrite gfortran. Sometimes I think we just ought to accept ice-on-invalid and just toss out all those bug reports as the effort to fix them is not worth the benefit from doing so. I would like to see if we can get a list out of Bugzilla for at least the ones we have marked as ice-on-invalid and just push those priorities to the back of the line.
[Bug fortran/103506] [10/11/12/13 Regression] ICE in gfc_free_namespace, at fortran/symbol.c:4039 since r10-2798-ge68a35ae4a65d2b3
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103506 --- Comment #6 from Steve Kargl --- On Thu, Jan 26, 2023 at 02:56:02AM +, jvdelisle at gcc dot gnu.org wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103506 > > --- Comment #5 from Jerry DeLisle --- > I found that the attached patch does not work. At the point of assertion many > of the other functions to free memory have null pointers which leads to > segfaults all along the way. > > The following approach appears to work, however, obviously there is a lot of > memory left not freed, so we would rely on the OS to clean it up. Maybe this > is > ok, not sure. > I think that it is ok. A lot of the gfortran FE code assumes that it is dealing with a conforming Fortran program. Many failing test cases are constructed from invalid Fortran code (see any of a number of Gerhard's PR), which takes torturous paths through the Fortran FE. The only other option is to check the pointers, but this is going to get ugly with some of the structs we have. Something like if (a->b->c->d) free(...) becomes if (a && a->b && a->b->c && a->b->c->d) free(...)
[Bug fortran/103506] [10/11/12/13 Regression] ICE in gfc_free_namespace, at fortran/symbol.c:4039 since r10-2798-ge68a35ae4a65d2b3
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103506
--- Comment #5 from Jerry DeLisle ---
I found that the attached patch does not work. At the point of assertion many
of the other functions to free memory have null pointers which leads to
segfaults all along the way.
The following approach appears to work, however, obviously there is a lot of
memory left not freed, so we would rely on the OS to clean it up. Maybe this is
ok, not sure.
+++ b/gcc/fortran/symbol.cc
@@ -4032,7 +4032,7 @@ void
gfc_free_namespace (gfc_namespace *)
{
gfc_namespace *p, *q;
- int i;
+ int i, ecnt;
gfc_was_finalized *f;
if (ns == NULL)
@@ -4042,6 +4042,12 @@ gfc_free_namespace (gfc_namespace *)
if (ns->refs > 0)
return;
+ /* In the presence of errors, the namespace may be
+ corrupted or non-sense. Don't assert() and bail out. */
+ gfc_get_errors (NULL, );
+ if (ecnt > 0)
+return;
+
gcc_assert (ns->refs == 0);
gfc_free_statements (ns->code);
[Bug fortran/103506] [10/11/12/13 Regression] ICE in gfc_free_namespace, at fortran/symbol.c:4039 since r10-2798-ge68a35ae4a65d2b3
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103506 --- Comment #4 from kargl at gcc dot gnu.org --- A patch to address this issue was submitted 7 months ago.
[Bug fortran/103506] [10/11/12/13 Regression] ICE in gfc_free_namespace, at fortran/symbol.c:4039 since r10-2798-ge68a35ae4a65d2b3
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103506 Jakub Jelinek changed: What|Removed |Added Target Milestone|10.4|10.5 --- Comment #3 from Jakub Jelinek --- GCC 10.4 is being released, retargeting bugs to GCC 10.5.
