[patch,fortran] Bug 69497 - ICE in gfc_free_namespace

[Mikael Morin]

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: pr69497.CL
Type: text/x-opencl-src
Size: 198 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/fortran/attachments/20180327/3482dbc7/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pr69497.diff
Type: text/x-patch
Size: 871 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/fortran/attachments/20180327/3482dbc7/attachment-0001.bin>