Re: [RFC] 12266 Fallout

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

On Fri, 26 Aug 2011 01:02:59 +0200, Keith Seitz wrote:
> The reporter's test case (C++):
> 
> typedef std::string foo;
> void calltest (foo) {}
> 
> Setting a break at "calltest(foo)" does not work because
> inspect_type (part of 12266 patchset) uses check_typedef, which
> resolves foo -> std::string -> std::basic_string<char,
> std::char_traits<char>, std::alloactor<char> >. But because we are
> no longer using DMGL_VERBOSE in dwarf2_physname (and other places)
> to do demangling, the symbol table actually stores
> "calltest(std::string)".

check_typedef always unwinds only the toplevel typedefs, not any inner ones.

I think you should move the comparison block

  /* Ignore any typedefs that should not be substituted.  */
  for (i = 0; i < ARRAY_SIZE (ignore_typedefs); ++i)
    {
      if (strcmp (name, ignore_typedefs[i]) == 0)
	return 0;
    }

into the later comparisons:

              /* Find the last typedef for the type.  */
              while (TYPE_TARGET_TYPE (last) != NULL
                     && (TYPE_CODE (TYPE_TARGET_TYPE (last))
                         == TYPE_CODE_TYPEDEF))
                last = TYPE_TARGET_TYPE (last);

and it would get properly caught, wouldn't be?


> There are two ways I can see to fix this.
> 1) Add DMGL_VERBOSE where it is needed so that NO typedefs ever
> appear in a (non-TYPE_CODE_TYPEDEF) symbol's name, i.e, store
> "calltest(std::basic_string<...>)" in the symbol table instead of
> "calltest(std::string)".
> Actually in this case, the "right" output would be "calltest(foo)",
> but that is a different, but related, problem, for which I developed
> a patch a long time ago, part of the first 12266 patch submissions
> (dwarf2_print_name).

As I stated off-list before I agree the primary goal is to make working
`break func(any_form_of_types)'.

And as also Tom said when there is the possibility to print the DWARF type
- incl. typedefs - it should be also printed as was implemented in:

But if you just lookup the minimal (ELF) symbol - where the dwarf2_print_name
form cannot apply - by:
(gdb) info sym 0x0000003f1b25ed70

I guess it should say:
std::hash<std::string>::operator()(std::string) const in section .text of /usr/lib64/libstdc++.so.6

and not:
std::hash<std::basic_string<char, std::char_traits<char>, std::allocator<char> > >::operator()(std::basic_string<char, std::char_traits<char>, std::allocator<char> >) const
in section .text of /usr/lib64/libstdc++.so.6

while nm -C says:
0000003f1b25ed70 T std::hash<std::string>::operator()(std::string) const


IMO if GDB starts to use DMGL_VERBOSE we should ask binutils to use
DMGL_VERBOSE for `nm -C' and other tools there.


> So, I humbly ask maintainers:
> 
> - Shall I continue #1 and start submitting patches?
> - Shall I start teaching gdb about std::string et al and how to deal
> with them in the environment we have today?
> - Have I missed something that would be preferable to anything I've
> mentioned?

I would prefer to keep `std::string' to be `std::string' as possible, which
I naively believe could be done without modifying/duplicating check_typedef by
the change suggested above.


Thanks,
Jan