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

[Keith Seitz]

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.

> One difficulty I have with this, is that I couldn't find
> any case of this code in replace_typedefs_qualified_name:
> 
> 	  /* The current node is not a name, so simply replace any
> 	     typedefs in it.  Then print it to the stream to continue
> 	     checking for more typedefs in the tree.  */
> 	  replace_typedefs (info, d_left (comp), finder, data);
> 
> actually ever replacing any typedef in the testsuite.
> I mean, I put an abort() in place if the returned component
> as a string is different from the name before the 
> replace_typedefs call, and then ran the gdb.cp/ testcases,
> and the abort never triggered...

Huh. I'm not sure what to say. Normally, I'm pretty thorough with
coverage testing. Either I missed something during test writing,
or perhaps this has code has since been rendered unnecessary?

Keith