Re: [RFC] Revisit PR 16253 ("Attempt to use a type name...")

Doug Evans
2015-06-16T17:54:28Z

Keith Seitz writes:
> Last year a patch was submitted/approved/commited to eliminate
> symbol_matches_domain which was causing this problem. It was later
reverted
> because it introduced a (severe) performance regression.
>
> Recap:
>
> (gdb) list
> 1 enum e {A,B,C} e;
> 2 int main (void) { return 0; }
> 3
> (gdb) p e
> Attempt to use a type name as an expression
>
> The parser attempts to find a symbol named "e" of VAR_DOMAIN.
> This gets passed down through lookup_symbol and (eventually) into
> block_lookup_symbol_primary, which iterates over the block's dictionary
> of symbols:
>
> for (sym = dict_iter_name_first (block->dict, name, &dict_iter);
> sym != NULL;
> sym = dict_iter_name_next (name, &dict_iter))
> {
> if (symbol_matches_domain (SYMBOL_LANGUAGE (sym),
> SYMBOL_DOMAIN (sym), domain))
> return sym;
> }
>
> The problem here is that we have a symbol named "e" in both STRUCT_DOMAIN
> and VAR_DOMAIN, and for languages like C++, Java, and Ada, where a tag
name
> may be used as an implicit typedef of the type, symbol_matches_domain
ignores
> the difference between VAR_DOMAIN and STRUCT_DOMAIN. As it happens, the
> STRUCT_DOMAIN symbol is found first, considered a match, and that symbol
is
> returned to the parser, eliciting the (now dreaded) error message.
>
> Since this bug exists specifically because we have both STRUCT and
VAR_DOMAIN
> symbols in a given block/CU, this patch rather simply/naively changes
> block_lookup_symbol_primary so that it continues to search for an exact
> domain match on the symbol if symbol_matches_domain returns a symbol
> which does not exactly match the requested domain.
>
> This "fixes" the immediate problem, but admittedly might uncover other,
> related bugs. [Paranoia?] However, it causes no regressions (functional
> or performance) in the test suite.
>
> I have also resurrected the tests from the previous submission. However
> since we can still be given a matching symbol with a different domain
than
> requested, we cannot say that a symbol "was not found." The error
messages
> today will still be the (dreaded) "Attempt to use a type name..." I've
> updated the tests to reflect this.
>
> ChangeLog
>
> PR 16253
> * block.c (block_lookup_symbol_primary): If a symbol is found
> which does not exactly match the requested domain, keep searching
> for an exact match. Otherwise, return the previously found "best"
> symbol.

Hi.

This approach is an improvement with no ill effects (that I can see),
so I'm ok with it.
Please add a reference to PR 16253 in the "hack" comment
in the code.

Do other callers of symbol_matches_domain need similar treatment?
I was wondering about block_lookup_symbol.

btw, here's some perf data using the gmonster1-pervasive-typedef.exp
test from my monster testcase generator.
[basically, every CU has "typedef int my_int;"
and the test does "ptype my_func" where my_func
takes a my_int as an argument]

trunk:

gmonster1:gmonster-pervasive-typedef cpu_time 10-cus 0.00091
gmonster1:gmonster-pervasive-typedef cpu_time 100-cus 0.000966
gmonster1:gmonster-pervasive-typedef cpu_time 1000-cus 0.001145
gmonster1:gmonster-pervasive-typedef cpu_time 10000-cus 0.011014
gmonster1:gmonster-pervasive-typedef wall_time 10-cus 0.000936985015869
gmonster1:gmonster-pervasive-typedef wall_time 100-cus 0.000993967056274
gmonster1:gmonster-pervasive-typedef wall_time 1000-cus 0.00116896629333
gmonster1:gmonster-pervasive-typedef wall_time 10000-cus 0.0110721588135
gmonster1:gmonster-pervasive-typedef vmsize 10-cus 31480
gmonster1:gmonster-pervasive-typedef vmsize 100-cus 67152
gmonster1:gmonster-pervasive-typedef vmsize 1000-cus 429264
gmonster1:gmonster-pervasive-typedef vmsize 10000-cus 3976272

with orig 16253 patch:

gmonster1:gmonster-pervasive-typedef cpu_time 10-cus 0.060466
gmonster1:gmonster-pervasive-typedef cpu_time 100-cus 0.549413
gmonster1:gmonster-pervasive-typedef cpu_time 1000-cus 8.515956
gmonster1:gmonster-pervasive-typedef cpu_time 10000-cus 492.20361
gmonster1:gmonster-pervasive-typedef wall_time 10-cus 0.0605120658875
gmonster1:gmonster-pervasive-typedef wall_time 100-cus 0.549317836761
gmonster1:gmonster-pervasive-typedef wall_time 1000-cus 8.5144739151
gmonster1:gmonster-pervasive-typedef wall_time 10000-cus 492.134341955
gmonster1:gmonster-pervasive-typedef vmsize 10-cus 43176
gmonster1:gmonster-pervasive-typedef vmsize 100-cus 174356
gmonster1:gmonster-pervasive-typedef vmsize 1000-cus 1501776
gmonster1:gmonster-pervasive-typedef vmsize 10000-cus 14895344

Ok, it's not in the 1000s :-),
but it is an additional 490+ seconds and 10+ GB.

with this 16253 patch:

gmonster1:gmonster-pervasive-typedef cpu_time 10-cus 0.0009
gmonster1:gmonster-pervasive-typedef cpu_time 100-cus 0.000964
gmonster1:gmonster-pervasive-typedef cpu_time 1000-cus 0.001097
gmonster1:gmonster-pervasive-typedef cpu_time 10000-cus 0.010179
gmonster1:gmonster-pervasive-typedef wall_time 10-cus 0.000927209854126
gmonster1:gmonster-pervasive-typedef wall_time 100-cus 0.000990152359009
gmonster1:gmonster-pervasive-typedef wall_time 1000-cus 0.00112009048462
gmonster1:gmonster-pervasive-typedef wall_time 10000-cus 0.0102391242981
gmonster1:gmonster-pervasive-typedef vmsize 10-cus 31484
gmonster1:gmonster-pervasive-typedef vmsize 100-cus 67148
gmonster1:gmonster-pervasive-typedef vmsize 1000-cus 429248
gmonster1:gmonster-pervasive-typedef vmsize 10000-cus 3976256

btw,

I think I have a simple way to get the perf back with the original
patch, but it involves (I think, TBD) "breaking"
psyms/gold-generated-gdb-index the same way gdb-generated-gdb-index
is "broken": PR 17387.
Namely, only record one static psym, the theory being
if one is not in a context where static symbol my_foo is defined,
gdb is going to (essentially) pick a random one so why record them all?
The catch is that, e.g., "info types foo" uses psyms/gdb-index too
so if we went this route we'd either have to accept the breakage
that .gdb_index introduced (PR 17387) or rewrite "info types, etc.,
to work differently: it'd have to scan the debug info, but how important
is a fast "info types"? One could employ various kinds of caching
to speed things up a bit.

The other way is recording the symbol domain in the index.

Neither of these is proposed for 7.10 though.