Get rid of infinite recursion for 'typedef' used with GTY-marked 'gcc/diagnostic-spec.h:nowarn_map' [PR101204] (was: [PATCH 1/13] v2 [PATCH 1/13] Add support for per-location warning groups (PR 74765))
Commit Message
Hi!
On 2021-09-01T18:14:46-0600, Martin Sebor <msebor@gmail.com> wrote:
> On 9/1/21 1:35 PM, Thomas Schwinge wrote:
>> On 2021-06-23T13:47:08-0600, Martin Sebor via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>>> --- /dev/null
>>> +++ b/gcc/diagnostic-spec.h
>>
>>> +typedef location_t key_type_t;
>>> +typedef int_hash <key_type_t, 0, UINT_MAX> xint_hash_t;
>>> +typedef hash_map<xint_hash_t, nowarn_spec_t> xint_hash_map_t;
>>> +
>>> +/* A mapping from the location of an expression to the warning spec
>>> + set for it. */
>>> +extern GTY(()) xint_hash_map_t *nowarn_map;
>> Oh, and one of [my pending changes] actually (unintentially so) happens to resolve
>> <https://gcc.gnu.org/PR101204> "[12 Regression] infinite recursion in
>> gtype-desc.c since r12-1801-g7036e9ef462fde8181bece4ac4e03f3aa27204dc",
>> so unless you've done any work on that, may I take over that PR?
>
> I haven't. Thanks for offering to take it on! I'm curious to
> hear how your change fixes that problem.
So, instead of my earlier drive-by fix, I've since distilled what it
actually is that is causing/fixing this (strange...) problem. OK to push
the attached "Get rid of infinite recursion for 'typedef' used with
GTY-marked 'gcc/diagnostic-spec.h:nowarn_map' [PR101204]"? (This, of
course, only fixes the symptom but not the actual underlying problem.
But I'm not going to dig deep into 'gengtype' at this time.) ;-)
Grüße
Thomas
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955
Comments
On Tue, Nov 9, 2021 at 11:28 AM Thomas Schwinge <thomas@codesourcery.com> wrote:
>
> Hi!
>
> On 2021-09-01T18:14:46-0600, Martin Sebor <msebor@gmail.com> wrote:
> > On 9/1/21 1:35 PM, Thomas Schwinge wrote:
> >> On 2021-06-23T13:47:08-0600, Martin Sebor via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> >>> --- /dev/null
> >>> +++ b/gcc/diagnostic-spec.h
> >>
> >>> +typedef location_t key_type_t;
> >>> +typedef int_hash <key_type_t, 0, UINT_MAX> xint_hash_t;
> >>> +typedef hash_map<xint_hash_t, nowarn_spec_t> xint_hash_map_t;
> >>> +
> >>> +/* A mapping from the location of an expression to the warning spec
> >>> + set for it. */
> >>> +extern GTY(()) xint_hash_map_t *nowarn_map;
>
> >> Oh, and one of [my pending changes] actually (unintentially so) happens to resolve
> >> <https://gcc.gnu.org/PR101204> "[12 Regression] infinite recursion in
> >> gtype-desc.c since r12-1801-g7036e9ef462fde8181bece4ac4e03f3aa27204dc",
> >> so unless you've done any work on that, may I take over that PR?
> >
> > I haven't. Thanks for offering to take it on! I'm curious to
> > hear how your change fixes that problem.
>
> So, instead of my earlier drive-by fix, I've since distilled what it
> actually is that is causing/fixing this (strange...) problem. OK to push
> the attached "Get rid of infinite recursion for 'typedef' used with
> GTY-marked 'gcc/diagnostic-spec.h:nowarn_map' [PR101204]"? (This, of
> course, only fixes the symptom but not the actual underlying problem.
> But I'm not going to dig deep into 'gengtype' at this time.) ;-)
Did you try if
typedef hash_map<int_hash<location_t, 0, UINT_MAX>, nowarn_spec_t>
xint_hash_map_t;
works? Usually a typedef is needed so that gengtype can grok struct
members but it
might very well be confused about two levels of typedefs here?
That said ... I guess this kind of fix is OK - but can you please put
a big fat comment
about this in the actual code and open a bugreport tracking this
gengtype defect?
Thanks,
Richard.
>
> Grüße
> Thomas
>
>
> -----------------
> Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955
On 11/9/21 3:28 AM, Thomas Schwinge wrote:
> Hi!
>
> On 2021-09-01T18:14:46-0600, Martin Sebor <msebor@gmail.com> wrote:
>> On 9/1/21 1:35 PM, Thomas Schwinge wrote:
>>> On 2021-06-23T13:47:08-0600, Martin Sebor via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>>>> --- /dev/null
>>>> +++ b/gcc/diagnostic-spec.h
>>>
>>>> +typedef location_t key_type_t;
>>>> +typedef int_hash <key_type_t, 0, UINT_MAX> xint_hash_t;
>>>> +typedef hash_map<xint_hash_t, nowarn_spec_t> xint_hash_map_t;
>>>> +
>>>> +/* A mapping from the location of an expression to the warning spec
>>>> + set for it. */
>>>> +extern GTY(()) xint_hash_map_t *nowarn_map;
>
>>> Oh, and one of [my pending changes] actually (unintentially so) happens to resolve
>>> <https://gcc.gnu.org/PR101204> "[12 Regression] infinite recursion in
>>> gtype-desc.c since r12-1801-g7036e9ef462fde8181bece4ac4e03f3aa27204dc",
>>> so unless you've done any work on that, may I take over that PR?
>>
>> I haven't. Thanks for offering to take it on! I'm curious to
>> hear how your change fixes that problem.
>
> So, instead of my earlier drive-by fix, I've since distilled what it
> actually is that is causing/fixing this (strange...) problem. OK to push
> the attached "Get rid of infinite recursion for 'typedef' used with
> GTY-marked 'gcc/diagnostic-spec.h:nowarn_map' [PR101204]"? (This, of
> course, only fixes the symptom but not the actual underlying problem.
> But I'm not going to dig deep into 'gengtype' at this time.) ;-)
Great work figuring it out! Amazing how riddled with bugs this
gengtype stuff is. Thanks a lot for the patch!
Martin
>
>
> Grüße
> Thomas
>
>
> -----------------
> Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955
>
Hi!
On 2021-11-09T21:52:50-0700, Martin Sebor <msebor@gmail.com> wrote:
> Amazing how riddled with bugs this
> gengtype stuff is.
Relevant Mike Stump quote from long ago, that I ran into while
researching 'gengtype' vs. 'typedef',
<https://gcc.gnu.org/legacy-ml/gcc/2003-03/msg01259.html>:
| Think of it this way, imagine you had a really good C parser, but that
| you couldn't reuse any of it to write complex transformational style
| code, but rather, had to re-implement a new C parser from scratch and
| you did it, uhm, what's the right word, expediently.
;-\
Alternative approaches to 'gengtype' have been discussed more than once,
but...
Grüße
Thomas
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955
From 4d691426a602f3179ef2847903e417fe473955c5 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <thomas@codesourcery.com>
Date: Tue, 9 Nov 2021 10:55:15 +0100
Subject: [PATCH] Get rid of infinite recursion for 'typedef' used with
GTY-marked 'gcc/diagnostic-spec.h:nowarn_map' [PR101204]
Reproduced with clang version 10.0.0-4ubuntu1:
gtype-desc.c:11333:1: warning: all paths through this function will call itself [-Winfinite-recursion]
... as well as some GCC's '-O2 -fdump-tree-optimized':
void gt_pch_nx(int_hash<unsigned int, 0u, 4294967295u>*, gt_pointer_operator, void*) ([...])
{
<bb 2>:
<bb 3>:
goto <bb 3>;
}
That three-arguments 'gt_pch_nx' function as well as two one-argument
'gt_ggc_mx', 'gt_pch_nx' functions now turn empty:
[...]
void
-gt_ggc_mx (int_hash<location_t,0,UINT_MAX>& x_r ATTRIBUTE_UNUSED)
+gt_ggc_mx (struct xint_hash_t& x_r ATTRIBUTE_UNUSED)
{
- int_hash<location_t,0,UINT_MAX> * ATTRIBUTE_UNUSED x = &x_r;
- gt_ggc_mx (&((*x)));
+ struct xint_hash_t * ATTRIBUTE_UNUSED x = &x_r;
}
[...]
void
-gt_pch_nx (int_hash<location_t,0,UINT_MAX>& x_r ATTRIBUTE_UNUSED)
+gt_pch_nx (struct xint_hash_t& x_r ATTRIBUTE_UNUSED)
{
- int_hash<location_t,0,UINT_MAX> * ATTRIBUTE_UNUSED x = &x_r;
- gt_pch_nx (&((*x)));
+ struct xint_hash_t * ATTRIBUTE_UNUSED x = &x_r;
}
[...]
void
-gt_pch_nx (int_hash<location_t,0,UINT_MAX>* x ATTRIBUTE_UNUSED,
+gt_pch_nx (struct xint_hash_t* x ATTRIBUTE_UNUSED,
ATTRIBUTE_UNUSED gt_pointer_operator op,
ATTRIBUTE_UNUSED void *cookie)
{
- gt_pch_nx (&((*x)), op, cookie);
}
[...]
gcc/
PR middle-end/101204
* diagnostic-spec.h (typedef xint_hash_t): Turn into...
(struct xint_hash_t): ... this.
* doc/gty.texi: Update.
---
gcc/diagnostic-spec.h | 2 +-
gcc/doc/gty.texi | 8 ++++++++
2 files changed, 9 insertions(+), 1 deletion(-)
@@ -130,7 +130,7 @@ operator!= (const nowarn_spec_t &lhs, const nowarn_spec_t &rhs)
return !(lhs == rhs);
}
-typedef int_hash <location_t, 0, UINT_MAX> xint_hash_t;
+struct xint_hash_t : int_hash<location_t, 0, UINT_MAX> {};
typedef hash_map<xint_hash_t, nowarn_spec_t> xint_hash_map_t;
/* A mapping from a 'location_t' to the warning spec set for it. */
@@ -64,6 +64,14 @@ The parser understands simple typedefs such as
@code{typedef int @var{name};}.
These don't need to be marked.
+However, in combination with GTY, avoid using typedefs such as
+@code{typedef int_hash<@dots{}> @var{name};}
+for these generate infinite-recursion code.
+See @uref{https://gcc.gnu.org/PR101204,PR101204}.
+Instead, you may use
+@code{struct @var{name} : int_hash<@dots{}> @{@};},
+for example.
+
Since @code{gengtype}'s understanding of C++ is limited, there are
several constructs and declarations that are not supported inside
classes/structures marked for automatic GC code generation. The
--
2.33.0