[PATCH v2] Remove best_symbol and better_symbol

Tom Tromey
2025-12-13T19:15:18Z

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.
---
gdb/block.c | 62 +++++++---------------------------------------------
gdb/block.h | 14 ++----------
gdb/c-exp.y | 8 ++++++-
gdb/symtab.c | 14 ++++++++----
gdb/symtab.h | 7 ++++++
5 files changed, 34 insertions(+), 71 deletions(-)

diff --git a/gdb/block.c b/gdb/block.c
index 3d2c51cc554..bcc35cc8b9e 100644
--- a/gdb/block.c
+++ b/gdb/block.c
@@ -578,46 +578,6 @@ block_iterator_next (struct block_iterator *iterator)
return block_iter_match_step (iterator, 0);
}

-/* 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
@@ -677,6 +637,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[];
@@ -698,31 +661,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 65e2e20877e..a9012c7207f 100644
--- a/gdb/block.h
+++ b/gdb/block.h
@@ -656,16 +656,6 @@ block_iterator_range (const block *block,
return iterator_range<block_iterator_wrapper> (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,
@@ -673,8 +663,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 d321e815ca4..bb0ec3cbe52 100644
--- a/gdb/c-exp.y
+++ b/gdb/c-exp.y
@@ -3133,8 +3133,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 5754d944b6c..47412516d11 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 9336f5d35a7..ab1a3d15950 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -1292,6 +1292,13 @@ struct symbol : public general_symbol_info, public allocate_on_obstack<symbol>
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;

--
2.49.0