GDB crash due to infinite recursion in typedef substitution (Re: [PATCHv2 3/3] gdb: Remove C++ symbol aliases from completion list)

[Pedro Alves]

On 5/26/20 10:17 PM, Keith Seitz wrote:
> On 5/25/20 7:34 AM, Pedro Alves wrote:
>>
>> To recap, that happens when we're processing a DEMANGLE_COMPONENT_QUAL_NAME
>> subtree.  We've processed "gdb", and then "option", and now we're going to
>> process the "boolean_option_def" component:
>>
>>  typed name
>>    qualified name
>>      name 'gdb'
>>      qualified name
>>        name 'option'
>>        qualified name
>>          template
>>            name 'boolean_option_def'      <<< 
>>            template argument list
>>              name 'value_print_options'
>>          name 'boolean_option_def'
>>
>> To me, it looks quite obvious that the problem is
>> that we're looking up that boolean_option_def symbol in the
>> global name namespace.  I mean, this is a qualified symbol
>> name, like:
>>
>>    gdb::option::boolean_option_def
>>
> 
> Hmm. I'm not sure that's how the code was originally written to work...
> replace_typedefs_qualified_name is supposed to "skip over" *_NAME
> components until it finds something that looks like it could be a type name.
> All these name components should simply be copied to the buffer. There's
> no reason to look them up as symbols.
> 
> That would imply that the code simply needs to be modified to look
> at DEMANGLE_COMPONENT_TEMPLATE properly.
> 
> Something like:
> 
> diff --git a/gdb/cp-support.c b/gdb/cp-support.c
> index 1e54aaea3b..7761470b2d 100644
> --- a/gdb/cp-support.c
> +++ b/gdb/cp-support.c
> @@ -314,6 +314,9 @@ replace_typedefs_qualified_name (struct demangle_parse_info *info,
>       substituted name.  */
>    while (comp->type == DEMANGLE_COMPONENT_QUAL_NAME)
>      {
> +      if (d_left (comp)->type == DEMANGLE_COMPONENT_TEMPLATE)
> +       comp = d_left (comp);
> +
>        if (d_left (comp)->type == DEMANGLE_COMPONENT_NAME)
>         {
>           struct demangle_component newobj;
> 
> NOTE: I haven't extensively tested this patch. Fedora 31 system GDB
> crashes or fails to set a breakpoint on one of the completed values -- it
> does complete it okay. On master the above tentative patch seems to work.
> 
> I have much more investigation and testing to do for this before submitting
> anything official, though.

Today I experimented with different kinds of symbols to get a better sense
of what the demangled tree node looks like in different scenarios.
I understand this better now.

I now agree that this is looking like the direction that we need to
take (#2 in my original email).

However, as you have it I don't think is fully correct.  Because, say,
you have a symbol like:

  A::B<int>::C<int>::function()

It seems like that change will make it so that we won't ever
try to substitute a typedef past the first template?

so with e.g., 

 typedef int MyInt;

setting a breakpoint like this works:

 "(gdb) b A::B<MyInt>::C<int>::function()"

but like this doesn't:

 "(gdb) b A::B<int>::C<MyInt>::function()"

I started writing a testcase to try some of these different scenarios, based
on gdb.linespec/cp-completion-aliases-2.exp.  See patches attached.
One of the patches includes the recursion detection I'm using to avoid
crashing GDB.  That patch also includes some hacks to print some debugging
stuff.

I also ran into another case of GDB recursing forever due to a template.
This is when the tree has a qualified name node with the template node on
the right child node instead of on the left.  Seems like we'll need a similar
treatment for templates at the end of replace_typedefs_qualified_name.

Here's how to trigger it with the testcase attached:

(gdb) b NS1::NS2::grab_it(NS1::NS2::magic<int>*) 
warning: inspect_type: max recursion limit reached for NS1
warning: inspect_type: max recursion limit reached for NS1::NS2
warning: inspect_type: max recursion limit reached for magic
warning: inspect_type: max recursion limit reached for NS1
warning: inspect_type: max recursion limit reached for NS1::NS2
warning: inspect_type: max recursion limit reached for magic
Function "NS1::NS2::grab_it(NS1::NS2::magic<int>*)" not defined.
Make breakpoint pending on future shared library load? (y or [n]) 

The tree looks like:

d_dump tree for NS1::NS2::grab_it(NS1::NS2::magic<int>*):
typed name
  qualified name
    name 'NS1'
    qualified name
      name 'NS2'
      name 'grab_it'
  function type
    argument list
      pointer
        qualified name
          name 'NS1'
          qualified name
            name 'NS2'
            template
              name 'magic'
              template argument list
                builtin type int

Seems like this will be more work that I hoped...

Maybe we should start by putting in the recursion limit, to
avoid the crash, and then we can fix the issues more like
other bugs than fatal crashes that make it impossible to debug
gdb itself...

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-recursion-limit-debug-stuff.patch
Type: text/x-patch
Size: 6056 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/gdb-patches/attachments/20200527/80ae23ba/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-rough-testcase.patch
Type: text/x-patch
Size: 6094 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/gdb-patches/attachments/20200527/80ae23ba/attachment-0003.bin>