Message ID | 87k0hh7hdn.fsf@euler.schwinge.homeip.net |
---|---|
State | New |
Headers | show |
Series | 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)) | expand |
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(-) diff --git a/gcc/diagnostic-spec.h b/gcc/diagnostic-spec.h index 9b3aaaa3ce6..d29cdd46290 100644 --- a/gcc/diagnostic-spec.h +++ b/gcc/diagnostic-spec.h @@ -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. */ diff --git a/gcc/doc/gty.texi b/gcc/doc/gty.texi index 2ad7793191b..8900e082225 100644 --- a/gcc/doc/gty.texi +++ b/gcc/doc/gty.texi @@ -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