* [patch,fortran] Bug 69497 - ICE in gfc_free_namespace
@ 2018-03-24 21:56 Jerry DeLisle
2018-03-24 23:25 ` Steve Kargl
0 siblings, 1 reply; 5+ messages in thread
From: Jerry DeLisle @ 2018-03-24 21:56 UTC (permalink / raw)
To: fortran; +Cc: GCC Patches
This one has been hanging around for a while. Dominique found this fix.
I retested with the 30 cases provided in the PR. These are all invalid
fortran. They give errors now and do not ICE.
Regression tested on trunk.
OK?
Jerry
PS I will use the first of the many test cases for the testsuite with
appropriate ChangeLog, etc.
2018-03-24 Jerry DeLisle <jvdelisle@gcc.gnu.org>
Dominique d'Humieres <dominiq@gcc.gnu.org>
PR fortran/84506
* symbol.c (gfc_free_namespace): Delete the assert and if refs
count is less than zero, free the namespece. Something is
halfway and other errors will resound.
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);
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [patch,fortran] Bug 69497 - ICE in gfc_free_namespace
2018-03-24 21:56 [patch,fortran] Bug 69497 - ICE in gfc_free_namespace Jerry DeLisle
@ 2018-03-24 23:25 ` Steve Kargl
2018-03-24 23:49 ` Jerry DeLisle
0 siblings, 1 reply; 5+ messages in thread
From: Steve Kargl @ 2018-03-24 23:25 UTC (permalink / raw)
To: Jerry DeLisle; +Cc: fortran, GCC Patches
On Sat, Mar 24, 2018 at 02:25:36PM -0700, Jerry DeLisle wrote:
>
> 2018-03-24 Jerry DeLisle <jvdelisle@gcc.gnu.org>
> Dominique d'Humieres <dominiq@gcc.gnu.org>
>
> PR fortran/84506
> * symbol.c (gfc_free_namespace): Delete the assert and if refs
> count is less than zero, free the namespece. Something is
> halfway and other errors will resound.
>
> 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.
--
Steve
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [patch,fortran] Bug 69497 - ICE in gfc_free_namespace
2018-03-24 23:25 ` Steve Kargl
@ 2018-03-24 23:49 ` Jerry DeLisle
[not found] ` <8ddae808-00c1-173c-9788-d34b134a823a@orange.fr>
0 siblings, 1 reply; 5+ messages in thread
From: Jerry DeLisle @ 2018-03-24 23:49 UTC (permalink / raw)
To: sgk; +Cc: fortran, GCC Patches
On 03/24/2018 02:56 PM, Steve Kargl wrote:
> On Sat, Mar 24, 2018 at 02:25:36PM -0700, Jerry DeLisle wrote:
>>
>> 2018-03-24 Jerry DeLisle <jvdelisle@gcc.gnu.org>
>> Dominique d'Humieres <dominiq@gcc.gnu.org>
>>
>> PR fortran/84506
>> * symbol.c (gfc_free_namespace): Delete the assert and if refs
>> count is less than zero, free the namespece. Something is
>> halfway and other errors will resound.
>>
>> 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.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [patch,fortran] Bug 69497 - ICE in gfc_free_namespace
[not found] ` <4ae5026c-d497-7edd-4c30-3536b6130aa4@charter.net>
@ 2018-03-27 21:07 ` Mikael Morin
2018-03-28 6:19 ` Jerry DeLisle
0 siblings, 1 reply; 5+ messages in thread
From: Mikael Morin @ 2018-03-27 21:07 UTC (permalink / raw)
To: Jerry DeLisle, fortran; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 2927 bytes --]
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
[-- Attachment #2: pr69497.CL --]
[-- Type: text/x-opencl-src, Size: 198 bytes --]
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).
[-- Attachment #3: pr69497.diff --]
[-- Type: text/x-patch, Size: 871 bytes --]
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 ();
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [patch,fortran] Bug 69497 - ICE in gfc_free_namespace
2018-03-27 21:07 ` Mikael Morin
@ 2018-03-28 6:19 ` Jerry DeLisle
0 siblings, 0 replies; 5+ messages in thread
From: Jerry DeLisle @ 2018-03-28 6:19 UTC (permalink / raw)
To: Mikael Morin, fortran; +Cc: gcc-patches
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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-03-28 1:03 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-24 21:56 [patch,fortran] Bug 69497 - ICE in gfc_free_namespace Jerry DeLisle
2018-03-24 23:25 ` Steve Kargl
2018-03-24 23:49 ` Jerry DeLisle
[not found] ` <8ddae808-00c1-173c-9788-d34b134a823a@orange.fr>
[not found] ` <e06e510c-64bb-121b-d8ae-3e3e6220924f@charter.net>
[not found] ` <9198ea4f-e040-9953-2899-a28820ae4be2@orange.fr>
[not found] ` <4ae5026c-d497-7edd-4c30-3536b6130aa4@charter.net>
2018-03-27 21:07 ` Mikael Morin
2018-03-28 6:19 ` Jerry DeLisle
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).