diff mbox series

Get rid of infinite recursion for 'typedef' used with GTY-marked 'gcc/diagnostic-spec.h:nowarn_map' [PR101204, PR103157]

Message ID 87ee7p7byl.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, PR103157] | expand

Commit Message

Thomas Schwinge Nov. 9, 2021, 12:25 p.m. UTC 
Hi!

On 2021-11-09T11:54:04+0100, Richard Biener <richard.guenther@gmail.com> wrote:
> On Tue, Nov 9, 2021 at 11:28 AM Thomas Schwinge <thomas@codesourcery.com> wrote:
>> 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?

First, that would be in conflict with my goal of settling on a *single*
'location_hash' type, but also: no, 'gengtype' doesn't grok that either:

    In file included from config.h:8:0,
                     from gtype-desc.c:21:
    [...]/source-gcc/gcc/../include/ansidecl.h:148:55: error: wrong number of template arguments (1, should be at least 2)
     #  define ATTRIBUTE_UNUSED __attribute__ ((__unused__))
                                                           ^
    gtype-desc.c:3014:37: note: in expansion of macro 'ATTRIBUTE_UNUSED'
     gt_ggc_mx (int_hash<location_t& x_r ATTRIBUTE_UNUSED)
                                         ^
    [...]

Note bogus formal parameter 'int_hash<location_t& x_r'.

That probably falls into category "understanding of C++ is limited", as
documented in 'gcc/doc/gty.texi'.  :-|


> 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

I had thought about that, but then decided not to do that, as the problem
is general, not specific to this particular code.  Instead, I had put
something into 'gcc/doc/gty.texi'.  Anyway, I've now put something into
that place, too.

> and open a bugreport tracking this
> gengtype defect?

<https://gcc.gnu.org/PR103157> "'gengtype': 'typedef' causing
infinite-recursion code to be generated".

Pushed to master branch commit c71cb26a9e841888f52e4bfcaad94c8f8ecb4fdb
"Get rid of infinite recursion for 'typedef' used with GTY-marked
'gcc/diagnostic-spec.h:nowarn_map' [PR101204, PR103157]", see attached.


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
diff mbox series

Patch

From c71cb26a9e841888f52e4bfcaad94c8f8ecb4fdb 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, PR103157]

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
	PR other/103157
	* diagnostic-spec.h (typedef xint_hash_t): Turn into...
	(struct xint_hash_t): ... this.
	* doc/gty.texi: Update.
---
 gcc/diagnostic-spec.h | 6 +++++-
 gcc/doc/gty.texi      | 8 ++++++++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/gcc/diagnostic-spec.h b/gcc/diagnostic-spec.h
index 9b3aaaa3ce6..e54e9e3ddbe 100644
--- a/gcc/diagnostic-spec.h
+++ b/gcc/diagnostic-spec.h
@@ -130,7 +130,11 @@  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;
+/* Per PR103157 "'gengtype': 'typedef' causing infinite-recursion code to be
+   generated", don't use
+       typedef int_hash<location_t, 0, UINT_MAX> xint_hash_t;
+   here.  */
+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..b996ff2c44e 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/PR103157,PR103157}.
+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