On 03/27/2018 01:53 PM, Mikael Morin wrote:
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


Good to go Mikael, thanks.

Jerry

Reply via email to