This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [patch,fortran] Bug 69497 - ICE in gfc_free_namespace
Le 26/03/2018 à 03:53, Jerry DeLisle a écrit :
On 03/25/2018 02:11 PM, Mikael Morin wrote:
Le 25/03/2018 à 21:27, Jerry DeLisle a écrit :
On 03/25/2018 10:49 AM, Mikael Morin wrote:
Le 25/03/2018 à 00:25, Jerry DeLisle a écrit :
On 03/24/2018 02:56 PM, Steve Kargl wrote:
On Sat, Mar 24, 2018 at 02:25:36PM -0700, Jerry DeLisle wrote:
diff --git a/gcc/fortran/symbol.c b/gcc/fortran/symbol.c
index ce6b1e93644..997d90b00fd 100644
--- a/gcc/fortran/symbol.c
+++ b/gcc/fortran/symbol.c
@@ -4037,10 +4037,9 @@ gfc_free_namespace (gfc_namespace *ns)
return;
ns->refs--;
- if (ns->refs > 0)
- return;
- gcc_assert (ns->refs == 0);
+ if (ns->refs != 0)
+ return;
gfc_free_statements (ns->code);
The ChangeLog doesn't seem to match the patch.
If ns->refs==0, you free the namespace.
If ns->refs!=0, you return.
So, if ns->refs<0, the namespace is not freed.
That is what I get when I am in hurry. Try this:
PR fortran/84506
* symbol.c (gfc_free_namespace): Delete the assert and only if
refs count equals zero, free the namespece. Otherwise,
something is halfway and other errors will resound.
Hello,
The assert was put in place to exhibit memory management issues, and
that’s what it does.
If ns->refs < 0, then it was 0 on the previous call, and ns should
have been freed at that time.
So if you read ns->refs you are reading garbage, and if you decrease
it you are writing to memory that you don’t own any more.
I think ICEing at this point is good enough, instead of going
further down the road.
The problem with ICEing is that it tells the users to report it as a
bug in the compiler.
It is a bug in the compiler, albeit one of little concern to us (at
least when dealing with invalid code): the memory is incorrectly managed.
No argument there, just saying in the cases of the PR, it is not useful
to the user.
This is a lot more useful then a fatal error. All of the 30 cases I
tested gave similar reasonable errors.
A fatal error doesn’t actually remove previously emitted (reasonable)
errors, it just doesn’t let the compiler continue. I can propose the
attached patch to convince you.
No need to convince. If you prefer your patch, its OK with me.
I have tried to restore the assert instead.
With the attached patch, freshly regression tested.
I have also checked the 29 cases from the PR.
OK?
Mikael
2018-03-27 Mikael Morin <mikael@gcc.gnu.org>
PR fortran/69497
* symbol.c (gfc_symbol_done_2): Start freeing namespaces
from the root.
(gfc_free_namespace): Restore assert (revert r258839).
diff --git a/gcc/fortran/symbol.c b/gcc/fortran/symbol.c
index 997d90b00fd..546a4fae0a8 100644
--- a/gcc/fortran/symbol.c
+++ b/gcc/fortran/symbol.c
@@ -4037,10 +4037,11 @@ gfc_free_namespace (gfc_namespace *ns)
return;
ns->refs--;
-
- if (ns->refs != 0)
+ if (ns->refs > 0)
return;
+ gcc_assert (ns->refs == 0);
+
gfc_free_statements (ns->code);
free_sym_tree (ns->sym_root);
@@ -4087,8 +4088,14 @@ gfc_symbol_init_2 (void)
void
gfc_symbol_done_2 (void)
{
- gfc_free_namespace (gfc_current_ns);
- gfc_current_ns = NULL;
+ if (gfc_current_ns != NULL)
+ {
+ /* free everything from the root. */
+ while (gfc_current_ns->parent != NULL)
+ gfc_current_ns = gfc_current_ns->parent;
+ gfc_free_namespace (gfc_current_ns);
+ gfc_current_ns = NULL;
+ }
gfc_free_dt_list ();
enforce_single_undo_checkpoint ();