diff mbox

[RFC] partial symbol name matching vs regexp

Message ID 87k1gtp9nj.fsf@tromey.com
State New
Headers show

Commit Message

Tom Tromey March 20, 2019, 8:34 p.m. UTC
>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> One of our users reported that info types was sometimes not working
Joel> for Ada types. Consider for instance the following code:
[...]

I finally got to reading this message.  Thanks for the thorough
write-up; and for your patience waiting for a response.

Joel> In our case, that symbol_matchers doesn't match alpha.beta,
Joel> because SYMNAME is the symbol's search name, and in Ada,
Joel> we search by encoded name. Thus, for the symbol we were
Joel> hoping to match, it gets called with "alpha__beta".

The crux of the problem.

Joel> With that assumption in mind, I think the way to make it work
Joel> is to change the symbol_matcher's signature to receive more
Joel> information - so that each caller of expand_symtabs_matching can
Joel> then decide which symbol name it needs to look at

This makes sense to me.

Joel> Following this, I thought the simplest would probably to have
Joel> the symbol_matcher receive a general_symbol_info instead of
Joel> just the name.

I am not so sure about this part.  See below.

[ gdb index ]
Joel> Trying to resolve that hitch with the gdb_index handling, and
Joel> after having read the description of what's in the gdb_index
Joel> section, I think there should be a way to link a symbol in
Joel> the gdb_index back to the DWARF CU, and therefore get the symbol's
Joel> language.

This can be done, maybe with a small bit of difficulty.  In the index,
each symbol references its CU.  However, to get the language, you'd have
to instantiate a reader and read the outermost DIE -- not too difficult
but does require some new code, I think.

Joel> Thus, a second option would be to keep the name parameter
Joel> in expand_symtabs_symbol_matcher_ftype, but then add the symbol
Joel> language as a second parameter; we would then modify the symbol_matcher
Joel> in search_symbols to check the language, and if the language is Ada,
Joel> then decode the name before evaluating the regexp.
...
Joel> But I'm not really satisfied with the second option (two parameters).

What about passing a flag to expand_symtabs_matching that says whether
the particular site wants the natural name or the search name?  I've
appended a symfile.h patch that shows the core change.

Joel> What if a search routine wanted to expand based on the linkage name,
Joel> one day, even for C++ symbols. I don't know if mangling a symbol back
Joel> would be possible or not, but regardless of that, it would feel wrong
Joel> to compute a mangled name when the caller had it.

Maybe re-mangling a symbol could be done, but I don't think we'd want to
attempt it.  This whole area has been a bit broken for years now,
because you still can't look up symbols by their mangled name, and the
"demangle" setting basically doesn't impact C++.  Oops on all that!
At one point I had a complicated plan to fix this, but maybe something
less complicated could be done... not sure.

Another thing I wanted to mention here is that IMO we should start
considering .gdb_index as deprecated, and should move to the DWARF 5
indices as much as possible.  (I don't know the full state of this
support in the toolchain though.  So maybe I'm super mistaken about the
viability of this.)

Joel> For that reason, I tend to still think that changing
Joel> expand_symtabs_symbol_matcher_ftype to receive a general_symbol_info
Joel> is better. It's not perfect in the case of gdb_index handling,
Joel> but I think that the consequences of that peculiarity would be
Joel> contained within the gdb_index handling in dwarf2read.c. So,
Joel> at least, it wouldn't be a caller in dwarf2read.c passing an
Joel> unsuspecting symbol_matcher function defined elsewhere an incomplete
Joel> general_symbol_info.

This is the part of the patch that is a bit uncomfortable for me.

I mean, on the one hand, maybe it isn't so bad: presumably if some
callback mistreats the general_symbol_info, it will simply not work.
But... I feel like we've had situations in the past with symbol tables
where things didn't work properly and simply went undiagnosed for years
(perhaps sometimes working via a minsym fallback).

And, I think the peculiarity isn't actually isolated.
dw2_expand_symtabs_matching_symbol calls symbol_matcher, which ultimately
is a callback function passed in by the caller of expand_symtabs_matching.
So, every caller of that will have to know to only look at certain fields
of the symbol.

Tom
diff mbox

Patch

diff --git a/gdb/symfile.h b/gdb/symfile.h
index 64d5a23f9b2..bb7a340132b 100644
--- a/gdb/symfile.h
+++ b/gdb/symfile.h
@@ -245,8 +245,9 @@  struct quick_symbol_functions
      Otherwise, if KIND does not match, this symbol is skipped.
 
      If even KIND matches, SYMBOL_MATCHER is called for each symbol
-     defined in the file.  The symbol "search" name is passed to
-     SYMBOL_MATCHER.
+     defined in the file.  If SEARCH_NAME is true, then the symbol
+     "search" name is passed to SYMBOL_MATCHER; otherwise the symbol
+     "natural" name is passed in.
 
      If SYMBOL_MATCHER returns false, then the symbol is skipped.
 
@@ -257,7 +258,8 @@  struct quick_symbol_functions
      const lookup_name_info &lookup_name,
      gdb::function_view<expand_symtabs_symbol_matcher_ftype> symbol_matcher,
      gdb::function_view<expand_symtabs_exp_notify_ftype> expansion_notify,
-     enum search_domain kind);
+     enum search_domain kind,
+     bool search_name);
 
   /* Return the comp unit from OBJFILE that contains PC and
      SECTION.  Return NULL if there is no such compunit.  This
@@ -526,7 +528,8 @@  void expand_symtabs_matching
    const lookup_name_info &lookup_name,
    gdb::function_view<expand_symtabs_symbol_matcher_ftype> symbol_matcher,
    gdb::function_view<expand_symtabs_exp_notify_ftype> expansion_notify,
-   enum search_domain kind);
+   enum search_domain kind,
+   bools search_name);
 
 void map_symbol_filenames (symbol_filename_ftype *fun, void *data,
 			   int need_fullname);