From: Tom Tromey Date: Tue, 13 May 2025 23:53:27 +0000 (-0600) Subject: Remove best_symbol and better_symbol X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=03c9c4c8d0ab4b8fc082afb8a0bdd69a7b846a37;p=thirdparty%2Fbinutils-gdb.git Remove best_symbol and better_symbol This patch removes best_symbol and better_symbol. Before some c-exp.y changes this year, best_symbol was the source of excess CU expansion, because searches that include SEARCH_VAR_DOMAIN will keep going until a symbol is found in that domain, or until all matches have been exhausted. So, for instance a "SEARCH_VFT" search for "int" will, most likely, expand all CUs -- even though the argument implies that a type would be suitable. Nowadays, best_symbol is more of an aesthetic offense and abstraction violation, encoding a workaround for a C++ expression lookup bug into a low-level lookup function. This patch changes the the C parser to simply do two lookups, prioritizing SEARCH_VAR_DOMAIN. This way, it gets the result it is presumably looking for, without causing excessive expansion (if that's currently possible) and without pushing its requirements into generic code. Note that the special case for "stub" symbols remains. This one is less problematic, at least for the time being, as such symbols aren't common. (Kevin does have a patch pending in this area but I don't believe this change should affect it, as the stub symbol case remains unchanged.) New in v2: addressed review comments and reworded the commit text. v1 is here: https://inbox.sourceware.org/gdb-patches/87345e71b1.fsf@tromey.com/T/#meb7622f6d4d5bb0c1cd6170d875b37a0ceb9a1ed Regression tested on x86-64 Fedora 40. --- diff --git a/gdb/block.c b/gdb/block.c index 730e4580a5b..2860e2cb146 100644 --- a/gdb/block.c +++ b/gdb/block.c @@ -583,46 +583,6 @@ block_iterator_next (struct block_iterator *iterator) return block_iter_match_step (iterator, false); } -/* See block.h. */ - -bool -best_symbol (struct symbol *a, const domain_search_flags domain) -{ - if (a->loc_class () == LOC_UNRESOLVED) - return false; - - if ((domain & SEARCH_VAR_DOMAIN) != 0) - return a->domain () == VAR_DOMAIN; - - return a->matches (domain); -} - -/* See block.h. */ - -struct symbol * -better_symbol (struct symbol *a, struct symbol *b, - const domain_search_flags domain) -{ - if (a == NULL) - return b; - if (b == NULL) - return a; - - if (a->matches (domain) && !b->matches (domain)) - return a; - - if (b->matches (domain) && !a->matches (domain)) - return b; - - if (a->loc_class () != LOC_UNRESOLVED && b->loc_class () == LOC_UNRESOLVED) - return a; - - if (b->loc_class () != LOC_UNRESOLVED && a->loc_class () == LOC_UNRESOLVED) - return b; - - return a; -} - /* See block.h. Note that if NAME is the demangled form of a C++ symbol, we will fail @@ -682,6 +642,9 @@ best_symbol_tracker::search (compunit_symtab *symtab, { for (symbol *sym : block_iterator_range (block, &name)) { + if (!sym->matches (domain)) + continue; + /* With the fix for PR gcc/debug/91507, we get for: ... extern char *zzz[]; @@ -703,31 +666,22 @@ best_symbol_tracker::search (compunit_symtab *symtab, doesn't work either because the type of the decl does not specify a size. - To fix this, we prefer def over decl in best_symbol and - better_symbol. + To fix this, we prefer definitions over declarations. In absence of the gcc fix, both def and decl have type char *[], so the only option to make this work is improve the fallback to use the size of the minimal symbol. Filed as PR exp/24989. */ - if (best_symbol (sym, domain)) + if (sym->is_definition ()) { best_symtab = symtab; currently_best = { sym, block }; return true; } - /* This is a bit of a hack, but 'matches' might ignore - STRUCT vs VAR domain symbols. So if a matching symbol is found, - make sure there is no "better" matching symbol, i.e., one with - exactly the same domain. PR 16253. */ - if (sym->matches (domain)) + if (currently_best.symbol == nullptr) { - symbol *better = better_symbol (sym, currently_best.symbol, domain); - if (better != currently_best.symbol) - { - best_symtab = symtab; - currently_best = { better, block }; - } + best_symtab = symtab; + currently_best = { sym, block }; } } diff --git a/gdb/block.h b/gdb/block.h index b84ca12c35a..a859d610d0e 100644 --- a/gdb/block.h +++ b/gdb/block.h @@ -659,16 +659,6 @@ block_iterator_range (const block *block, return iterator_range (std::move (begin)); } -/* Return true if symbol A is the best match possible for DOMAIN. */ - -extern bool best_symbol (struct symbol *a, const domain_search_flags domain); - -/* Return symbol B if it is a better match than symbol A for DOMAIN. - Otherwise return A. */ - -extern struct symbol *better_symbol (struct symbol *a, struct symbol *b, - const domain_search_flags domain); - /* Search BLOCK for symbol NAME in DOMAIN. */ extern struct symbol *block_lookup_symbol (const struct block *block, @@ -676,8 +666,8 @@ extern struct symbol *block_lookup_symbol (const struct block *block, const domain_search_flags domain); /* When searching for a symbol, the "best" symbol is preferred over - one that is merely acceptable. See 'best_symbol'. This class - keeps track of this distinction while searching. */ + one that is merely acceptable. This class keeps track of this + distinction while searching. */ struct best_symbol_tracker { diff --git a/gdb/c-exp.y b/gdb/c-exp.y index 9a37eb5fb69..33e1a3847c8 100644 --- a/gdb/c-exp.y +++ b/gdb/c-exp.y @@ -3130,8 +3130,14 @@ classify_name (struct parser_state *par_state, const struct block *block, std::string copy = copy_name (yylval.sval); - bsym = lookup_symbol (copy.c_str (), block, SEARCH_VFT, + /* Prefer variables over functions or types. This preserves some + historical parser behavior. */ + bsym = lookup_symbol (copy.c_str (), block, SEARCH_VAR_DOMAIN, &is_a_field_of_this); + if (bsym.symbol == nullptr) + bsym = lookup_symbol (copy.c_str (), block, + SEARCH_FUNCTION_DOMAIN | SEARCH_TYPE_DOMAIN, + &is_a_field_of_this); if (bsym.symbol && bsym.symbol->loc_class () == LOC_BLOCK) { diff --git a/gdb/symtab.c b/gdb/symtab.c index 35e380980b5..7f9f0fb24a9 100644 --- a/gdb/symtab.c +++ b/gdb/symtab.c @@ -2589,7 +2589,7 @@ lookup_global_symbol (const char *name, sym = lookup_symbol_in_block (name, symbol_name_match_type::FULL, global_block, domain); - if (sym != NULL && best_symbol (sym, domain)) + if (sym != nullptr && sym->is_definition ()) return { sym, global_block }; } @@ -2603,10 +2603,16 @@ lookup_global_symbol (const char *name, block_symbol bs = lookup_global_or_static_symbol (name, GLOBAL_BLOCK, objfile, domain); - if (better_symbol (sym, bs.symbol, domain) == sym) - return { sym, global_block }; - else + /* Prefer a definition over a declaration. (At this point, SYM is + either nullptr or a declaration.) */ + if (bs.symbol != nullptr && bs.symbol->is_definition ()) + return bs; + /* Prefer a declaration if that's all we have. */ + if (bs.symbol != nullptr && sym == nullptr) return bs; + /* Return either an empty result or the declaration we found in the + "local" global block. */ + return { sym, global_block }; } /* See symtab.h. */ diff --git a/gdb/symtab.h b/gdb/symtab.h index 8f0cc728410..63741fb7fa7 100644 --- a/gdb/symtab.h +++ b/gdb/symtab.h @@ -1288,6 +1288,13 @@ struct symbol : public general_symbol_info, public allocate_on_obstack return this->impl ().loc_class; } + /* Return true if the symbol is not LOC_UNRESOLVED -- that is, if it + is a definition and not just a declaration. */ + bool is_definition () const + { + return loc_class () != LOC_UNRESOLVED; + } + /* Return true if this symbol's domain matches FLAGS. */ bool matches (domain_search_flags flags) const;