Re: [PATCH] gdb: Skip useless minsym scan in variable-only symbol searches

[Simon Marchi <simark@simark.ca>]

On 9/22/26 11:51 AM, Adhemerval Zanella wrote:
> The symbol-listing command -symbol-info-* family goes through
> global_symbol_searcher::search, which has 3 steps:
> 
> 1. expand_symtabs (objfile, preg): make sure every compunit that could
>    contain a match is expanded.  Returns found_msymbol = true if it
>    saw a matching msymbol that has no debug info.
> 
> 2. add_matching_symbols (...): walk the compunits and collect the real
>    matches.
> 
> 3. After the loop, a minsym fallback: if found_msymbol was set, or
>    unconditionally for any variable search with no file filter, re-scan
>    msymbols and append the ones without debug info (unless the searcher
>    set exclude_minsyms).
> 
> Step 1 had two phases, first objfile::search, which uses the
> quick-symbol index to expand candidate compunits; then the pre-pass
> for every msymbol in the objfile whose name matches the regexp:
> 
> 1.1. function search: call find_compunit_symtab_for_pc (address).  This
>      consults the quick functions and can expand the compunit containing
>      that address.
> 
> 1.2. variable search: call lookup_symbol_in_objfile_from_linkage_name,
>      which does a linear scan of objfile->compunits () per lookup.  It
>      can not expand anything; its only output is setting found_msymbol
>      when the lookup fails.
> 
> So for variables the pre-pass costs 'msymbols * expanded-compunits'
> block lookups and produces exactly one bit of information (found_msymbol)
> and this dominates the command execution time.
> 
> And in step 3. these MI3 commands (search_module_symbols) calls
> spec2.set_exclude_minsyms (true), which sets !m_exclude_minsyms to false.

The phrasing "sets !m_exclude_minsyms to false" really confused me,
could you rephrase this?

> The fallback never runs (found_msymbol is ignored), making the pre-pass
> calculation not required.
> 
> The fix is make SEARCH_VAR_DOMAIN skip the pre-pass.  For variable
> searches with no filenames the fallback condition already contains
> '(m_kind & SEARCH_VAR_DOMAIN) != 0' as an unconditional alternative.
> 
> Since the pre-pass now only runs for searches that include
> SEARCH_FUNCTION_DOMAIN, the lookup_symbol_in_objfile_from_linkage_name
> branch of the conditional inside the loop is unreachable.
> 
> Also update the comments, which still described the variable lookup and
> claimed it forces symtabs to be read.  That was true when
> lookup_symbol_in_objfile_from_linkage_name was added in commit
> 422d65e705c7, but it no longer expands any symtab.
> 
> -symbol-info-module-variables --module <M> on the 2000-module benchmark
> drops from 90 s to 0.56 s cold.

Awesome.  It took me a while to convince myself that the change is
correct, and how global_symbol_searcher works in the various cases.  One
thing that was not obvious to me was that it only supports one kind of
search at a time (you can't search for variables _and_ functions in one
search), despite the use of domain_search_flags that would suggest the
opposite.  In the process I made a few patches to tweak
global_symbol_searcher to make it more readable, I'll post them after
your patch it merged.

> 
> Change-Id: I6019bcf80629f21cf6dee74d0e96fa788a70e220
> ---
>  gdb/symtab.c | 35 ++++++++++++++---------------------
>  1 file changed, 14 insertions(+), 21 deletions(-)
> 
> diff --git a/gdb/symtab.c b/gdb/symtab.c
> index 93684c14777..13824587e75 100644
> --- a/gdb/symtab.c
> +++ b/gdb/symtab.c
> @@ -4750,22 +4750,20 @@ global_symbol_searcher::expand_symtabs
>       SEARCH_GLOBAL_BLOCK | SEARCH_STATIC_BLOCK,
>       kind);
>  
> -  /* Here, we search through the minimal symbol tables for functions and
> -     variables that match, and force their symbols to be read.  This is in
> -     particular necessary for demangled variable names, which are no longer
> -     put into the partial symbol tables.  The symbol will then be found
> +  /* Here, we search through the minimal symbol tables for functions that
> +     match, and force their symbols to be read.  The symbol will then be found
>       during the scan of symtabs later.
>  
> -     For functions, find_pc_symtab should succeed if we have debug info for
> -     the function, for variables we have to call
> -     lookup_symbol_in_objfile_from_linkage_name to determine if the
> -     variable has debug info.  If the lookup fails, set found_msymbol so
> -     that we will rescan to print any matching symbols without debug info.
> -     We only search the objfile the msymbol came from, we no longer search
> -     all objfiles.  In large programs (1000s of shared libs) searching all
> -     objfiles is not worth the pain.  */
> +     The find_compunit_symtab_for_pc should succeed if we have debug info for

I would remove the "The" in this last line.

> +     the function.  If it fails, set found_msymbol so that we will rescan to
> +     print any matching symbols without debug info.  We only search the
> +     objfile the msymbol came from, we no longer search all objfiles.
> +
> +     Variables are not handled here, and looking them up does not expand any
> +     symtab.  When no file names were given the caller unconditionally rescans
> +     the minimal symbols for SEARCH_VAR_DOMAIN.  */
>    if (m_filenames.empty ()
> -      && (kind & (SEARCH_VAR_DOMAIN | SEARCH_FUNCTION_DOMAIN)) != 0)
> +      && (kind & SEARCH_FUNCTION_DOMAIN) != 0)
>      {
>        for (minimal_symbol *msymbol : objfile->msymbols ())
>  	{
> @@ -4780,18 +4778,13 @@ global_symbol_searcher::expand_symtabs
>  		  || preg->exec (msymbol->natural_name (), 0,
>  				 NULL, 0) == 0)
>  		{
> -		  /* An important side-effect of these lookup functions is
> +		  /* An important side-effect of this lookup function is
>  		     to expand the symbol table if msymbol is found, later
>  		     in the process we will add matching symbols or
>  		     msymbols to the results list, and that requires that
>  		     the symbols tables are expanded.  */
> -		  if ((kind & SEARCH_FUNCTION_DOMAIN) != 0
> -		      ? (find_compunit_symtab_for_pc
> -			 (msymbol->value_address (objfile)) == NULL)
> -		      : (lookup_symbol_in_objfile_from_linkage_name
> -			 (objfile, msymbol->linkage_name (),
> -			  SEARCH_VFT)
> -			 .symbol == NULL))
> +		  if (find_compunit_symtab_for_pc
> +			(msymbol->value_address (objfile)) == nullptr)
>  		    found_msymbol = true;

One change I had locally to help me understand what this does was to
rename found_msymbol to found_func_msymbol_without_debug_info.  Could
you rename it as part of this patch, and rename the variable in
global_symbol_searcher::search too?

LGTM with that fixed.

Approved-By: Simon Marchi <simon.marchi@efficios.com>

And thanks Thiago for testing.

Simon