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

Andrew Burgess andrew.burgess@embecosm.com
Tue May 26 17:02:45 GMT 2020


* Pedro Alves <palves@redhat.com> [2020-05-25 15:34:27 +0100]:

> Adding Keith, who I believe is the original author of this
> whole replace-typedefs machinery.  [ Help? :-) ]
> 
> On 5/24/20 12:35 PM, Pedro Alves via Gdb-patches wrote:
> > Hi,
> > 
> >> gdb/ChangeLog:
> >>
> >> 	* completer.c (completion_tracker::remove_completion): Define new
> >> 	function.
> >> 	* completer.h (completion_tracker::remove_completion): Declare new
> >> 	function.
> >> 	* symtab.c (completion_list_add_symbol): Remove aliasing msymbols
> >> 	when adding a C++ function symbol.
> >>
> >> gdb/testsuite/ChangeLog:
> >>
> >> 	* gdb.linespec/cp-completion-aliases.cc: New file.
> >> 	* gdb.linespec/cp-completion-aliases.exp: New file.
> > 
> > This causes GDB to crash for me, when debugging GDB itself, and then
> > doing "b main<TAB>".
> > 
> > $ ./gdb -data-directory=data-directory ./gdb -ex "complete b main"
> > GNU gdb (GDB) 10.0.50.20200319-git
> > ...
> > Reading symbols from ./gdb...
> > Setting up the environment for debugging gdb.
> > During symbol reading: unsupported tag: 'DW_TAG_unspecified_type'
> > During symbol reading: debug info gives source 55 included from file at zero line 0
> > During symbol reading: debug info gives command-line macro definition with non-zero line 19: _STDC_PREDEF_H 1
> > Breakpoint 1 at 0xaf48c3: file /home/pedro/gdb/mygit/src/gdbsupport/errors.cc, line 54.
> > During symbol reading: Member function "~_Sp_counted_base" (offset 0x683070) is virtual but the vtable offset is not specified
> > During symbol reading: const value length mismatch for 'std::ratio<1, 1000000000>::num', got 8, expected 0
> > During symbol reading: cannot get low and high bounds for subprogram DIE at 0x694d73
> > During symbol reading: Child DIE 0x6afc31 and its abstract origin 0x6afbcd have different parents
> > During symbol reading: Multiple children of DIE 0x6b2668 refer to DIE 0x6b2657 as their abstract origin
> > Breakpoint 2 at 0x4f6775: file /home/pedro/gdb/mygit/src/gdb/cli/cli-cmds.c, line 201.
> > Aborted (core dumped)
> > 
> > GDB seemingly crashes due to infinite recursion.  
> > 
> > Here's the top of the stack, showing the recursion starting:
> > 
> > $ gdb ./gdb -c core.4577
> > ...
> > Program terminated with signal SIGABRT, Aborted.
> > #0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
> > 51      }
> > [Current thread is 1 (Thread 0x7f9d6f25ecc0 (LWP 4577))]
> > (gdb) bt -44
> > #51229 0x000000000055533c in inspect_type (info=0x38ff720, ret_comp=0x3ba6b10, finder=0x0, data=0x0) at /home/pedro/gdb/mygit/src/gdb/cp-support.c:267
> > #51230 0x0000000000555a6f in replace_typedefs (info=0x38ff720, ret_comp=0x3ba6b10, finder=0x0, data=0x0) at /home/pedro/gdb/mygit/src/gdb/cp-support.c:475
> > #51231 0x0000000000555a36 in replace_typedefs (info=0x38ff720, ret_comp=0x3ba6b70, finder=0x0, data=0x0) at /home/pedro/gdb/mygit/src/gdb/cp-support.c:470
> > #51232 0x0000000000555800 in replace_typedefs_qualified_name (info=0x38ff720, ret_comp=0x37c7e20, finder=0x0, data=0x0) at /home/pedro/gdb/mygit/src/gdb/cp-support.c:389
> > #51233 0x0000000000555a8c in replace_typedefs (info=0x38ff720, ret_comp=0x37c7e20, finder=0x0, data=0x0) at /home/pedro/gdb/mygit/src/gdb/cp-support.c:479
> > #51234 0x000000000055533c in inspect_type (info=0x38ff720, ret_comp=0x37c7e20, finder=0x0, data=0x0) at /home/pedro/gdb/mygit/src/gdb/cp-support.c:267
> > #51235 0x0000000000555a6f in replace_typedefs (info=0x38ff720, ret_comp=0x37c7e20, finder=0x0, data=0x0) at /home/pedro/gdb/mygit/src/gdb/cp-support.c:475
> > #51236 0x0000000000555a36 in replace_typedefs (info=0x38ff720, ret_comp=0x37c7e80, finder=0x0, data=0x0) at /home/pedro/gdb/mygit/src/gdb/cp-support.c:470
> > #51237 0x0000000000555696 in replace_typedefs_qualified_name (info=0x38ff720, ret_comp=0x37c7dc0, finder=0x0, data=0x0) at /home/pedro/gdb/mygit/src/gdb/cp-support.c:357
> > #51238 0x0000000000555a8c in replace_typedefs (info=0x38ff720, ret_comp=0x37c7dc0, finder=0x0, data=0x0) at /home/pedro/gdb/mygit/src/gdb/cp-support.c:479
> > #51239 0x0000000000555a36 in replace_typedefs (info=0x38ff720, ret_comp=0x37c8400, finder=0x0, data=0x0) at /home/pedro/gdb/mygit/src/gdb/cp-support.c:470
> > #51240 0x0000000000555bd0 in cp_canonicalize_string_full[abi:cxx11](char const*, char const* (*)(type*, void*), void*) (string=0x980b2c0 "gdb::option::boolean_option_def<filename_partial_match_opts>::boolean_option_def(char const*, bool* (*)(filename_partial_match_opts*), void (*)(ui_file*, int, cmd_list_element*, char const*), char con"..., finder=0x0, data=0x0) at /home/pedro/gdb/mygit/src/gdb/cp-support.c:527
> > #51241 0x0000000000555d28 in cp_canonicalize_string_no_typedefs[abi:cxx11](char const*) (string=0x980b2c0 "gdb::option::boolean_option_def<filename_partial_match_opts>::boolean_option_def(char const*, bool* (*)(filename_partial_match_opts*), void (*)(ui_file*, int, cmd_list_element*, char const*), char con"...) at /home/pedro/gdb/mygit/src/gdb/cp-support.c:550
> > #51242 0x00000000008c4959 in completion_list_add_symbol (tracker=..., sym=0x980fde0, lookup_name=..., text=0x7fff6159e7f2 "main", word=0x7fff6159e7f2 "main") at /home/pedro/gdb/mygit/src/gdb/symtab.c:5316
> > #51243 0x00000000008c541b in add_symtab_completions (cust=0x673ba40, tracker=..., mode=complete_symbol_mode::LINESPEC, lookup_name=..., text=0x7fff6159e7f2 "main", word=0x7fff6159e7f2 "main", code=TYPE_CODE_UNDEF) at /home/pedro/gdb/mygit/src/gdb/symtab.c:5576
> > #51244 0x00000000008c5496 in <lambda(compunit_symtab*)>::operator()(compunit_symtab *) const (__closure=0x7fff6159e230, symtab=0x673ba40) at /home/pedro/gdb/mygit/src/gdb/symtab.c:5697
> > #51245 0x00000000008c8f14 in gdb::function_view<void(compunit_symtab*)>::<lambda(gdb::fv_detail::erased_callable, compunit_symtab*)>::operator()(gdb::fv_detail::erased_callable, compunit_symtab *) const (__closure=0x0, ecall=..., args#0=0x673ba40) at /home/pedro/gdb/mygit/src/gdb/../gdbsupport/function-view.h:263
> > #51246 0x00000000008c8f3c in gdb::function_view<void(compunit_symtab*)>::<lambda(gdb::fv_detail::erased_callable, compunit_symtab*)>::_FUN(gdb::fv_detail::erased_callable, compunit_symtab *) () at /home/pedro/gdb/mygit/src/gdb/../gdbsupport/function-view.h:257
> > #51247 0x0000000000603da8 in gdb::function_view<void (compunit_symtab*)>::operator()(compunit_symtab*) const (this=0x7fff6159df00, args#0=0x673ba40) at /home/pedro/gdb/mygit/src/gdb/../gdbsupport/function-view.h:247
> > #51248 0x00000000007c8483 in psym_expand_symtabs_matching(objfile *, gdb::function_view<bool(char const*, bool)>, const lookup_name_info &, gdb::function_view<bool(char const*)>, gdb::function_view<void(compunit_symtab*)>, search_domain) (objfile=0x232c6a0, file_matcher=..., lookup_name_in=..., symbol_matcher=..., expansion_notify=..., domain=ALL_DOMAIN) at /home/pedro/gdb/mygit/src/gdb/psymtab.c:1354
> > #51249 0x00000000008ab6a7 in expand_symtabs_matching(gdb::function_view<bool (char const*, bool)>, lookup_name_info const&, gdb::function_view<bool (char const*)>, gdb::function_view<void (compunit_symtab*)>, search_domain) (file_matcher=..., lookup_name=..., symbol_matcher=..., expansion_notify=..., kind=ALL_DOMAIN) at /home/pedro/gdb/mygit/src/gdb/symfile.c:3788
> > #51250 0x00000000008c5ab6 in default_collect_symbol_completion_matches_break_on (tracker=..., mode=complete_symbol_mode::LINESPEC, name_match_type=symbol_name_match_type::WILD, text=0x7fff6159e7f2 "main", word=0x7fff6159e7f2 "main", break_on=0xc3468e "", code=TYPE_CODE_UNDEF) at /home/pedro/gdb/mygit/src/gdb/symtab.c:5692
> > #51251 0x00000000008c5f79 in default_collect_symbol_completion_matches (tracker=..., mode=complete_symbol_mode::LINESPEC, name_match_type=symbol_name_match_type::WILD, text=0x7fff6159e7f2 "main", word=0x7fff6159e7f2 "main", code=TYPE_CODE_UNDEF) at /home/pedro/gdb/mygit/src/gdb/symtab.c:5795
> > #51252 0x00000000008c5fc2 in collect_symbol_completion_matches (tracker=..., mode=complete_symbol_mode::LINESPEC, name_match_type=symbol_name_match_type::WILD, text=0x7fff6159e7f2 "main", word=0x7fff6159e7f2 "main") at /home/pedro/gdb/mygit/src/gdb/symtab.c:5810
> > #51253 0x00000000006fcdf2 in linespec_complete_function (tracker=..., function=0x7fff6159e7f2 "main", func_match_type=symbol_name_match_type::WILD, source_filename=0x0) at /home/pedro/gdb/mygit/src/gdb/linespec.c:2816
> > #51254 0x00000000006fcecf in complete_linespec_component (parser=0x7fff6159e500, tracker=..., text=0x7fff6159e7f2 "main", component=linespec_complete_what::FUNCTION, source_filename=0x0) at /home/pedro/gdb/mygit/src/gdb/linespec.c:2848
> > #51255 0x00000000006fd63b in linespec_complete (tracker=..., text=0x7fff6159e7f2 "main", match_type=symbol_name_match_type::WILD) at /home/pedro/gdb/mygit/src/gdb/linespec.c:3063
> > #51256 0x00000000005405b1 in complete_address_and_linespec_locations (tracker=..., text=0x7fff6159e7f2 "main", match_type=symbol_name_match_type::WILD) at /home/pedro/gdb/mygit/src/gdb/completer.c:690
> > #51257 0x0000000000540eee in location_completer (ignore=0x1cb8320, tracker=..., text=0x7fff6159e7f2 "main") at /home/pedro/gdb/mygit/src/gdb/completer.c:1034
> > #51258 0x0000000000541024 in location_completer_handle_brkchars (ignore=0x1cb8320, tracker=..., text=0x7fff6159e7f2 "main", word_ignored=0x0) at /home/pedro/gdb/mygit/src/gdb/completer.c:1071
> > #51259 0x00000000005416e4 in complete_line_internal_normal_command (tracker=..., command=0x7fff6159e7f0 "b main", word=0x0, cmd_args=0x7fff6159e7f2 "main", reason=handle_brkchars, c=0x1cb8320) at /home/pedro/gdb/mygit/src/gdb/completer.c:1306
> > #51260 0x0000000000541b72 in complete_line_internal_1 (tracker=..., text=0x0, line_buffer=0x7fff615a0905 "b main", point=6, reason=handle_brkchars) at /home/pedro/gdb/mygit/src/gdb/completer.c:1531
> > #51261 0x0000000000541bb5 in complete_line_internal (tracker=..., text=0x0, line_buffer=0x7fff615a0905 "b main", point=6, reason=handle_brkchars) at /home/pedro/gdb/mygit/src/gdb/completer.c:1550
> > #51262 0x0000000000542cee in completion_find_completion_word (tracker=..., text=0x7fff615a0905 "b main", quote_char=0x7fff6159eb10) at /home/pedro/gdb/mygit/src/gdb/completer.c:2054
> > #51263 0x000000000054247a in complete (line=0x7fff615a0905 "b main", word=0x7fff6159eb08, quote_char=0x7fff6159eb10) at /home/pedro/gdb/mygit/src/gdb/completer.c:1770
> > #51264 0x00000000004f6ed0 in complete_command (arg=0x7fff615a0905 "b main", from_tty=1) at /home/pedro/gdb/mygit/src/gdb/cli/cli-cmds.c:364
> > #51265 0x00000000004fee70 in do_const_cfunc (c=0x2385760, args=0x7fff615a0905 "b main", from_tty=1) at /home/pedro/gdb/mygit/src/gdb/cli/cli-decode.c:107
> > #51266 0x0000000000501e28 in cmd_func (cmd=0x2385760, args=0x7fff615a0905 "b main", from_tty=1) at /home/pedro/gdb/mygit/src/gdb/cli/cli-decode.c:1952
> > #51267 0x000000000090d07d in execute_command (p=0x7fff615a0905 "b main", from_tty=1) at /home/pedro/gdb/mygit/src/gdb/top.c:655
> > #51268 0x000000000073b9c2 in catch_command_errors (command=0x90cc11 <execute_command(char const*, int)>, arg=0x7fff615a08fc "complete b main", from_tty=1) at /home/pedro/gdb/mygit/src/gdb/main.c:401
> > #51269 0x000000000073cdaa in captured_main_1 (context=0x7fff6159f070) at /home/pedro/gdb/mygit/src/gdb/main.c:1163
> > #51270 0x000000000073cf9f in captured_main (data=0x7fff6159f070) at /home/pedro/gdb/mygit/src/gdb/main.c:1188
> > #51271 0x000000000073d00a in gdb_main (args=0x7fff6159f070) at /home/pedro/gdb/mygit/src/gdb/main.c:1213
> > #51272 0x000000000041563e in main (argc=6, argv=0x7fff6159f178) at /home/pedro/gdb/mygit/src/gdb/gdb.c:32
> > 
> > This cycle keeps repeating:
> > #1709 0x000000000055533c in inspect_type (info=0x38ff720, ret_comp=0xd83be10, finder=0x0, data=0x0) at /home/pedro/gdb/mygit/src/gdb/cp-support.c:267
> > #1710 0x0000000000555a6f in replace_typedefs (info=0x38ff720, ret_comp=0xd83be10, finder=0x0, data=0x0) at /home/pedro/gdb/mygit/src/gdb/cp-support.c:475
> > #1711 0x0000000000555a36 in replace_typedefs (info=0x38ff720, ret_comp=0xd83be70, finder=0x0, data=0x0) at /home/pedro/gdb/mygit/src/gdb/cp-support.c:470
> > #1712 0x0000000000555800 in replace_typedefs_qualified_name (info=0x38ff720, ret_comp=0xd839470, finder=0x0, data=0x0) at /home/pedro/gdb/mygit/src/gdb/cp-support.c:389
> > #1713 0x0000000000555a8c in replace_typedefs (info=0x38ff720, ret_comp=0xd839470, finder=0x0, data=0x0) at /home/pedro/gdb/mygit/src/gdb/cp-support.c:479
> > 
> > 
> > The symbol on which we call cp_canonicalize_string_no_typedefs is:
> > 
> > (top-gdb) p *sym
> > $4 = {<general_symbol_info> = {m_name = 0x980b2c0 "gdb::option::boolean_option_def<filename_partial_match_opts>::boolean_option_def(char const*, bool* (*)(filename_partial_
> > match_opts*), void (*)(ui_file*, int, cmd_list_element*, char const*), char con"..., value = {ivalue = 159449440, block = 0x9810160, bytes = 0x9810160 "\212\271\214", addre
> > ss = 0x9810160, common_block = 0x9810160, chain = 0x9810160}, language_specific = {obstack = 0x0, demangled_name = 0x0}, m_language = language_cplus, ada_mangled = 0, secti
> > on = -1}, <allocate_on_obstack> = {<No data fields>}, type = 0x980fc20, owner = {symtab = 0x673bdf0, arch = 0x673bdf0}, domain = VAR_DOMAIN, aclass_index = 19, is_objfile_o
> > wned = 1, is_argument = 0, is_inlined = 0, maybe_copied = 0, subclass = SYMBOL_NONE, line = 157, aux_value = 0x980fe30, hash_next = 0x9821bd0}
> > 
> > (top-gdb) p sym->m_name 
> > $6 = 0x980b2c0 "gdb::option::boolean_option_def<filename_partial_match_opts>::boolean_option_def(char const*, bool* (*)(filename_partial_match_opts*), void (*)(ui_file*, in t, cmd_list_element*, char const*), char const*, char const*, char const*)"
> > 
> 
> I figured out what is special about this symbol.  The issue is that
> we have a boolean_option_def typedef in the global namespace in the
> program.  Here, in stack.c:
> 
>  using boolean_option_def
>    = gdb::option::boolean_option_def<frame_print_options>;
> 
> actually we have two.  There's another one in valprint.c:
> 
>  using boolean_option_def
>    = gdb::option::boolean_option_def<value_print_options>;
> 
> The recursion happens because we start by breaking the
> 
>    gdb::option::boolean_option_def<filename_partial_match_opts>::boolean_option_def(....)
> 
> symbol name into demanged parse info components.
> This happens in cp_canonicalize_string_no_typedefs -> replace_typedefs.
> 
> The tree for that symbol looks like this:
> 
> (top-gdb) p d_dump (ret_comp, 0)
> 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'
>   function type
>     argument list
>       pointer
>         const
>           builtin type char
>       argument list
>         pointer
>           function type
>             pointer
>               builtin type bool
>             argument list
>               pointer
>                 name 'value_print_options'
>         argument list
>           pointer
>             function type
>               builtin type void
>               argument list
>                 pointer
>                   name 'ui_file'
>                 argument list
>                   builtin type int
>                   argument list
>                     pointer
>                       name 'cmd_list_element'
>                     argument list
>                       pointer
>                         const
>                           builtin type char
>           argument list
>             pointer
>               const
>                 builtin type char
>             argument list
>               pointer
>                 const
>                   builtin type char
>               argument list
>                 pointer
>                   const
>                     builtin type char
> $5 = void
> 
> Let's just focus on the top part, because that's where
> the recursion happens:
> 
>  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'
> 
> As we walk the tree of components, we get to replace_typedefs_qualified_name,
> and enter the loop:
> 
>   while (comp->type == DEMANGLE_COMPONENT_QUAL_NAME)
>     {
>       if (d_left (comp)->type == DEMANGLE_COMPONENT_NAME)
> 	{
> 
> The DEMANGLE_COMPONENT_NAME case is hit twice, once for
> "gdb", and another for "option".  After those two,
> the next d_left (comp)->type is DEMANGLE_COMPONENT_TEMPLATE.
> 
> This now gets us to the else branch within that
> DEMANGLE_COMPONENT_QUAL_NAME loop:
> 
>       else
> 	{
> 	  /* 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);
> 
> which, after some more replace_typedefs recursion, gets us to
> 
> 	case DEMANGLE_COMPONENT_NAME:
> 	  inspect_type (info, ret_comp, finder, data);
> 	  break;
> 
> This is for this part of the tree:
> 
>          template
>            name 'boolean_option_def'
> 
> and it's this inspect_type call that's problematic.
> 
> [come back here later]
> 
> inspect_type does a symbol lookup for "boolean_option_def",
> which finds the unrelated typedef at the global namespace.
> In this session I'm debugging, it finds the one in valprint.c:
> 
> (top-gdb) p *sym
> $10 = {<general_symbol_info> = {m_name = 0x3a5dc90 "boolean_option_def", value = {ivalue = 0, block = 0x0, bytes = 0x0, address = 0x0, common_block = 0x0, chain = 0x0}, language_specific = {obstack = 0x0, demangled_name = 0x0}, m_language = language_cplus, ada_mangled = 0, section = -1}, <allocate_on_obstack> = {<No data fields>}, type = 0x3a613f0, owner = {symtab = 0x34ab930, arch = 0x34ab930}, domain = VAR_DOMAIN, aclass_index = 8, is_objfile_owned = 1, is_argument = 0, is_inlined = 0, maybe_copied = 0, subclass = SYMBOL_NONE, line = 2987, aux_value = 0x0, hash_next = 0x3a694e0}
> (top-gdb) p *sym.owner.symtab 
> $11 = {next = 0x34ab750, compunit_symtab = 0x34ab6d0, linetable = 0x3b83e60, filename = 0x2df4b20 "/home/pedro/gdb/mygit/src/gdb/valprint.c", language = language_cplus, fullname = 0x0}
> 
> This symbol's type is of course a typedef, so we try to resolve it.
> check_typedef returns this type:
> 
>  (top-gdb) p *type->main_type
>  $14 = 
>  {name = 0x2de5a40 "gdb::option::boolean_option_def<value_print_options>",
>   code = TYPE_CODE_STRUCT,
>  ...
> 
> So, since a typedef was resolved, we get here:
> 
> 	  /* Turn the result into a new tree.  Note that this
> 	     tree will contain pointers into NAME, so NAME cannot
> 	     be free'd until all typedef conversion is done and
> 	     the final result is converted into a string.  */
> 	  i = cp_demangled_name_to_comp (name, NULL);
> 	  if (i != NULL)
> 	    {
> 	      /* Merge the two trees.  */
> 	      cp_merge_demangle_parse_infos (info, ret_comp, i.get ());
> 
> 	      /* Replace any newly introduced typedefs -- but not
> 		 if the type is anonymous (that would lead to infinite
> 		 looping).  */
> 	      if (!is_anon)
> 		{
> 		  fprintf_unfiltered (gdb_stdlog, "replace_typedefs: %s\n",
> 				      name);
> 
> 		  replace_typedefs (info, ret_comp, finder, data);
> 
> And it's that last replace_typedefs call that triggers the recursion,
> since we're now going to process a tree
> 
>  gdb::option::boolean_option_def<value_print_options>
>               ^^^^^^^^^^^^^^^^^^
> 
> that will again end up doing a lookup for that global scope
> boolean_option_def typedef, rinse repeat.
> 
> Locally, I put a recursion limit in inspect_type, and the resulting 
> symbol name that cp_canonicalize_string_no_typedefs returns is:
> 
> $18 = std::unique_ptr<char> containing 0x32f81a0 "gdb::option::gdb::option::gdb::option::gdb::option::gdb::option::gdb::option::gdb::option::gdb::option::gdb::option::gdb::
> option::gdb::option::gdb::option::gdb::option::gdb::option::gdb::option::gdb::option::gdb::option::gdb::option::gdb::option::gdb::option::gdb::option::boolean_option_def<va
> lue_print_options><value_print_options><value_print_options><value_print_options><value_print_options><value_print_options><value_print_options><value_print_options><value_
> print_options><value_print_options><value_print_options><value_print_options><value_print_options><value_print_options><value_print_options><value_print_options><value_prin
> t_options><value_print_options><value_print_options><value_print_options><value_print_options>::boolean_option_def(char const*, bool* (*)(value_print_options*), void (*)(ui
> _file*, int, cmd_list_element*, char const*), char const*, char const*, char const*)"
> 
> Obviously broken due to recursion around boolean_option_def.
> 
> 
> Now, let's get back to that inspect_type call that I mentioned
> was problematic.  See [come back here later] above.
> 
> 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
> 
> so we should be looking for a "boolean_option_def" typedef in
> the gdb::option namespace or struct/class.  Right?
> 
> The question to me is then, how to do that.
> In replace_typedefs_qualified_name, after we've processed
> "gdb" and "option", we have "gdb::option::" stored in
> the "buf" string_file, which seems perfect.  However, how do we
> get that scope down to inspect_type, since we have layers of
> replace_typedefs calls before we get there?
> 
> I can think of two ways:
> 
> #1 - store the current scope in the demangle_parse_info
> context object, and have inspect_type consult that.
> 
> #2 - Make replace_typedefs_qualified_name
> look ahead in the tree, inlining the
> DEMANGLE_COMPONENT_TEMPLATE case here, basically
> inlining enough forward peeking in order to do a directly
> inspect_type call from within replace_typedefs_qualified_name.
> Of course, we would likely have to do the same for other
> node types, not just DEMANGLE_COMPONENT_TEMPLATE.
> 
> And then, I'm not sure we should pass the scope as extra
> parameter to inspect_type, or, whether we should replace
> the "boolean_option_def" node in the tree with one that has
> a full-scoped name "gdb::option::boolean_option_def" name,
> similarly to how replace_typedefs_qualified_name
> handles the DEMANGLE_COMPONENT_NAME case.
> 
> 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...
> 
> So, in order to get the discussion going, I wrote a patch
> for solution #1.  This fixes the issue for me, and causes no
> testsuite regressions.  Find it below.
> 
> WDYT?
> 
> > 
> > I'm seeing this on a GDB built with:
> >  gcc version 7.3.1 20180712 (Red Hat 7.3.1-6) (GCC) 
> > on Fedora 27.
> > 
> > Anyone else seeing this?
> 
> I'm also seeing this when I build GDB with GCC 10 (and debug
> that GDB with itself), so it must be others see this too?

Yes I can reproduce the original issue you are seeing trying to
complete on main.

You also mentioned a couple of times seeing problems with the test
gdb.linespec/cp-completion-aliases.exp, this I don't see - you said
even with the patch you already pushed you were still seeing issues
with this test, and I can't reproduce this.

Still I think your first patch looks like a nice improvement, so
thanks for that.  The patch below looks like a good solution to me, I
only had one tiny bit of feedback...

> 
> From af60bb6c6d7c66307db49e524409bc81f1322907 Mon Sep 17 00:00:00 2001
> From: Pedro Alves <palves@redhat.com>
> Date: Mon, 25 May 2020 13:14:10 +0100
> Subject: [PATCH] Fix gdb::option::boolean_option_def recursion
> 
> ---
>  gdb/cp-support.c | 15 +++++++++++----
>  gdb/cp-support.h |  5 ++++-
>  2 files changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/gdb/cp-support.c b/gdb/cp-support.c
> index 1e54aaea3b1..2e4db8a7007 100644
> --- a/gdb/cp-support.c
> +++ b/gdb/cp-support.c
> @@ -137,14 +137,15 @@ inspect_type (struct demangle_parse_info *info,
>  	      canonicalization_ftype *finder,
>  	      void *data)
>  {
> -  char *name;
>    struct symbol *sym;
>  
>    /* Copy the symbol's name from RET_COMP and look it up
>       in the symbol table.  */
> -  name = (char *) alloca (ret_comp->u.s_name.len + 1);
> -  memcpy (name, ret_comp->u.s_name.s, ret_comp->u.s_name.len);
> -  name[ret_comp->u.s_name.len] = '\0';
> +  std::string name_str;
> +  if (info->current_scope != nullptr)
> +    name_str = info->current_scope;
> +  name_str.append (ret_comp->u.s_name.s, ret_comp->u.s_name.len);
> +  const char *name = name_str.c_str ();
>  
>    /* Ignore any typedefs that should not be substituted.  */
>    for (int i = 0; i < ARRAY_SIZE (ignore_typedefs); ++i)
> @@ -355,7 +356,13 @@ replace_typedefs_qualified_name (struct demangle_parse_info *info,
>  	  /* 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.  */
> +
> +	  /* The struct/template/function/field/type name should be
> +	     qualified in the current scope though.  */
> +	  info->current_scope = buf.string ().c_str ();
>  	  replace_typedefs (info, d_left (comp), finder, data);
> +	  info->current_scope = nullptr;

I don't know if you should be using make_scoped_restore to protect
info->current_scope?

Thanks,
Andrew

> +
>  	  gdb::unique_xmalloc_ptr<char> name
>  	    = cp_comp_to_string (d_left (comp), 100);
>  	  if (name == NULL)
> diff --git a/gdb/cp-support.h b/gdb/cp-support.h
> index 6ca898315bb..561de5a94df 100644
> --- a/gdb/cp-support.h
> +++ b/gdb/cp-support.h
> @@ -72,8 +72,11 @@ struct demangle_parse_info
>  
>    /* Any temporary memory used during typedef replacement.  */
>    struct obstack obstack;
> -};
>  
> +  /* The current qualified scope context, during typedef
> +     replacement.  */
> +  const char *current_scope = nullptr;
> +};
>  
>  /* Functions from cp-support.c.  */
>  
> 
> base-commit: af2c48d8545cbc24aa6caf9b9f2a49ab6c0aaa7b
> -- 
> 2.14.5
> 


More information about the Gdb-patches mailing list