From: "Rohr, Stephan" <stephan.rohr@intel.com>
To: Simon Marchi <simon.marchi@efficios.com>,
"gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
Subject: RE: [PATCH 7/9] gdb: add matching helpers to global_symbol_searcher
Date: Wed, 30 Sep 2026 13:51:03 +0000 [thread overview]
Message-ID: <DS7PR11MB6247BB6E26265A540B9CC816938B2@DS7PR11MB6247.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20260929194119.155169-8-simon.marchi@efficios.com>
> -----Original Message-----
> From: Simon Marchi <simon.marchi@efficios.com>
> Sent: Tuesday, 29 September 2026 21:39
> To: gdb-patches@sourceware.org
> Cc: Simon Marchi <simon.marchi@efficios.com>
> Subject: [PATCH 7/9] gdb: add matching helpers to global_symbol_searcher
>
> The methods of global_symbol_searcher repeat the same checks in several
> places. Add some helpers to make the code easier to follow:
>
> - name_matches: check a name against the (optional) name regexp
>
> - symtab_matches_filenames: check a symtab against the searched file
> names
>
> - symbol_matches: all the filtering done on full symbols in
> add_matching_symbols
>
> - msymbol_matches: the filtering done on minimal symbols, shared by
> expand_symtabs and add_matching_msymbols
>
> Switch expand_symtabs, add_matching_symbols and
> add_matching_msymbols to
> use early continues to avoid deeply nested ifs.
>
> Change-Id: I553f70fb8ea9c7e55b89bb1204a402938c116c3b
> ---
> gdb/symtab.c | 205 ++++++++++++++++++++++++++++-----------------------
> gdb/symtab.h | 13 ++++
> 2 files changed, 127 insertions(+), 91 deletions(-)
>
> diff --git a/gdb/symtab.c b/gdb/symtab.c
> index 9ce6911666c6..79e43eafde18 100644
> --- a/gdb/symtab.c
> +++ b/gdb/symtab.c
> @@ -4741,6 +4741,93 @@ global_symbol_searcher::is_suitable_msymbol
> }
> }
>
> +/* Return true if NAME matches NAME_REGEX, or if NAME_REGEX is empty.
> */
> +
> +static bool
> +name_matches (const std::optional<compiled_regex> &name_regex,
> + const char *name)
> +{
> + return (!name_regex.has_value ()
> + || name_regex->exec (name, 0, nullptr, 0) == 0);
> +}
> +
> +/* See symtab.h. */
> +
> +bool
> +global_symbol_searcher::symtab_matches_filenames (symtab *symtab)
> const
> +{
> + /* Check first sole SYMTAB->FILENAME. It does not need to be a substring
> + of symtab_to_fullname as it may contain "./" etc. */
> + if (file_matches (symtab->filename (), m_filenames, false))
> + return true;
> +
> + if (!basenames_may_differ
> + && !file_matches (lbasename (symtab->filename ()), m_filenames, true))
> + return false;
> +
> + return file_matches (symtab_to_fullname (symtab), m_filenames, false);
> +}
> +
> +/* See symtab.h. */
> +
> +bool
> +global_symbol_searcher::symbol_matches
> + (const symbol *sym,
> + const std::optional<compiled_regex> &name_regex,
> + const std::optional<compiled_regex> &type_regex) const
> +{
> + if (!symtab_matches_filenames (sym->symtab ()))
> + return false;
> +
> + if (!sym->matches (to_search_flags (m_kind)))
> + return false;
> +
> + if (!name_matches (name_regex, sym->natural_name ()))
> + return false;
> +
> + switch (m_kind)
> + {
> + case symbol_search_kind::VARIABLE:
> + /* LOC_CONST can be used for more than just enums, e.g., C++ static
> + const members. We only want to skip enums here. */
> + if (sym->loc_class () == LOC_UNRESOLVED
> + || (sym->loc_class () == LOC_CONST
> + && sym->type ()->code () == TYPE_CODE_ENUM))
> + return false;
> + [[fallthrough]];
> +
> + case symbol_search_kind::FUNCTION:
> + if (type_regex.has_value ()
> + && !treg_matches_sym_type_name (*type_regex, sym))
> + return false;
> + break;
> +
> + case symbol_search_kind::MODULE:
> + /* Skip module declarations, keep only definitions. */
> + if (sym->line () == 0)
> + return false;
> + break;
> + }
> +
> + return true;
> +}
> +
> +/* See symtab.h. */
> +
> +bool
> +global_symbol_searcher::msymbol_matches
> + (const minimal_symbol *msymbol,
> + const std::optional<compiled_regex> &name_regex) const
> +{
> + if (msymbol->created_by_gdb)
> + return false;
> +
> + if (!is_suitable_msymbol (msymbol))
> + return false;
> +
> + return name_matches (name_regex, msymbol->natural_name ());
> +}
> +
> /* See symtab.h. */
>
> bool
> @@ -4763,8 +4850,7 @@ global_symbol_searcher::expand_symtabs
> &lookup_name_info::match_any (),
> [&] (const char *symname)
> {
> - return (!name_regex.has_value ()
> - || name_regex->exec (symname, 0, NULL, 0) == 0);
> + return name_matches (name_regex, symname);
> },
> NULL,
> SEARCH_GLOBAL_BLOCK | SEARCH_STATIC_BLOCK,
> @@ -4790,25 +4876,16 @@ global_symbol_searcher::expand_symtabs
> {
> QUIT;
>
> - if (msymbol->created_by_gdb)
> + if (!msymbol_matches (msymbol, name_regex))
> continue;
>
> - if (is_suitable_msymbol (msymbol))
> - {
> - if (!name_regex.has_value ()
> - || name_regex->exec (msymbol->natural_name (), 0,
> - NULL, 0) == 0)
> - {
> - /* 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 (find_compunit_symtab_for_pc
> - (msymbol->value_address (objfile)) == nullptr)
> - found_func_msymbol_without_debug_info = true;
> - }
> - }
> + /* 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 (find_compunit_symtab_for_pc
> + (msymbol->value_address (objfile)) == nullptr)
> + found_func_msymbol_without_debug_info = true;
> }
> }
>
> @@ -4824,8 +4901,6 @@ global_symbol_searcher::add_matching_symbols
> const std::optional<compiled_regex> &type_regex,
> std::set<symbol_search> *result_set) const
> {
> - domain_search_flags domain = to_search_flags (m_kind);
IIUC this is moved to "symbol_matches"? This implies "to_search_flags" is run
for every symbol. This looks unnecessary. We could add an additional argument to
"symbol_matches".
> -
> /* Add matching symbols (if not already present). */
> for (compunit_symtab &cust : objfile->compunits ())
> {
> @@ -4837,54 +4912,11 @@
> global_symbol_searcher::add_matching_symbols
>
> for (struct symbol *sym : block_iterator_range (b))
> {
> - struct symtab *real_symtab = sym->symtab ();
> -
> QUIT;
>
> - /* Check first sole REAL_SYMTAB->FILENAME. It does
> - not need to be a substring of symtab_to_fullname as
> - it may contain "./" etc. */
> - if (!(file_matches (real_symtab->filename (), m_filenames, false)
> - || ((basenames_may_differ
> - || file_matches (lbasename (real_symtab->filename
> ()),
> - m_filenames, true))
> - && file_matches (symtab_to_fullname (real_symtab),
> - m_filenames, false))))
> + if (!symbol_matches (sym, name_regex, type_regex))
> continue;
>
> - if (!sym->matches (domain))
> - continue;
> -
> - if (name_regex.has_value ()
> - && name_regex->exec (sym->natural_name (), 0,
> - nullptr, 0) != 0)
> - continue;
> -
> - switch (m_kind)
> - {
> - case symbol_search_kind::VARIABLE:
> - /* LOC_CONST can be used for more than just enums, e.g.,
> - C++ static const members. We only want to skip enums
> - here. */
> - if (sym->loc_class () == LOC_UNRESOLVED
> - || (sym->loc_class () == LOC_CONST
> - && sym->type ()->code () == TYPE_CODE_ENUM))
> - continue;
> - [[fallthrough]];
> -
> - case symbol_search_kind::FUNCTION:
> - if (type_regex.has_value ()
> - && !treg_matches_sym_type_name (*type_regex, sym))
> - continue;
> - break;
> -
> - case symbol_search_kind::MODULE:
> - /* Skip module declarations, keep only definitions. */
> - if (sym->line () == 0)
> - continue;
> - break;
> - }
> -
> /* Match, insert if not already in the results. */
> if (result_set->size () < m_max_search_results)
> result_set->emplace (block, sym);
> @@ -4908,34 +4940,25 @@
> global_symbol_searcher::add_matching_msymbols
> {
> QUIT;
>
> - if (msymbol->created_by_gdb)
> + if (!msymbol_matches (msymbol, name_regex))
> continue;
>
> - if (is_suitable_msymbol (msymbol))
> - {
> - if (!name_regex.has_value ()
> - || name_regex->exec (msymbol->natural_name (), 0,
> - NULL, 0) == 0)
> - {
> - /* For functions we can do a quick check of whether the
> - symbol might be found via find_pc_symtab. */
> - if (m_kind != symbol_search_kind::FUNCTION
> - || (find_compunit_symtab_for_pc
> - (msymbol->value_address (objfile)) == NULL))
> - {
> - if (lookup_symbol_in_objfile_from_linkage_name
> - (objfile, msymbol->linkage_name (),
> - SEARCH_VFT).symbol == NULL)
> - {
> - /* Matching msymbol, add it to the results list. */
> - if (results->size () < m_max_search_results)
> - results->emplace_back (GLOBAL_BLOCK, msymbol,
> objfile);
> - else
> - return false;
> - }
> - }
> - }
> - }
> + /* For functions we can do a quick check of whether the
> + symbol might be found via find_pc_symtab. */
> + if (m_kind == symbol_search_kind::FUNCTION
> + && (find_compunit_symtab_for_pc
> + (msymbol->value_address (objfile)) != NULL))
> + continue;
> +
> + if (lookup_symbol_in_objfile_from_linkage_name
> + (objfile, msymbol->linkage_name (), SEARCH_VFT).symbol != NULL)
> + continue;
> +
> + /* Matching msymbol, add it to the results list. */
> + if (results->size () < m_max_search_results)
> + results->emplace_back (GLOBAL_BLOCK, msymbol, objfile);
> + else
> + return false;
> }
>
> return true;
> diff --git a/gdb/symtab.h b/gdb/symtab.h
> index acbe41d131b7..5dd0475e4491 100644
> --- a/gdb/symtab.h
> +++ b/gdb/symtab.h
> @@ -2675,6 +2675,19 @@ class global_symbol_searcher
> /* Compile M_SYMBOL_TYPE_REGEXP, if set. */
> std::optional<compiled_regex> compile_type_regex () const;
>
> + /* Return true if SYMTAB matches one of M_FILENAMES. */
> + bool symtab_matches_filenames (symtab *symtab) const;
> +
> + /* Return true if SYM is of type M_KIND, is in a file matching
Rephrase to "is of kind M_KIND"?
> + M_FILENAMES, and matches NAME_REGEX and TYPE_REGEX. */
> + bool symbol_matches (const symbol *sym,
> + const std::optional<compiled_regex> &name_regex,
> + const std::optional<compiled_regex> &type_regex) const;
> +
> + /* Return true if MSYMBOL is of type M_KIND and matches NAME_REGEX.
> */
Same here.
> + bool msymbol_matches (const minimal_symbol *msymbol,
> + const std::optional<compiled_regex> &name_regex)
> const;
The function also checks the "created_by_gdb" flag and "is_suitable_msymbol".
Should this be mentioned in the comment, too?
> +
> /* Expand symtabs in OBJFILE that match NAME_REGEX, are of type
> M_KIND.
> Return true if a matching function msymbol without debug info was
> seen, meaning that msymbols should later be considered for the results
> --
> 2.55.0
________________________________________
Intel Deutschland GmbH
Registered Address: Dornacher Strasse 1, 85622 Feldkirchen, Germany
Tel: +49 (89) 99143-0
www.intel.de
Managing Directors: Candice Moore, Jeffrey Schneiderman, Ramachandran Sitaraman
Chairperson of the Supervisory Board: Sonja Pierer
Registered Seat: Munich Commercial Register B: Amtsgericht Munich HRB 186928
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
next prev parent reply other threads:[~2026-09-30 13:52 UTC|newest]
Thread overview: 59+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-29 19:39 [PATCH 0/9] Cleanups in global_symbol_searcher Simon Marchi
2026-09-29 19:39 ` [PATCH 1/9] gdb: add symbol_search_kind enum for global_symbol_searcher Simon Marchi
2026-09-30 9:31 ` Rohr, Stephan
2026-09-30 14:50 ` Simon Marchi
2026-09-29 19:39 ` [PATCH 2/9] gdb: factor out regexp compilation from global_symbol_searcher::search Simon Marchi
2026-09-30 12:10 ` Rohr, Stephan
2026-09-30 16:21 ` Simon Marchi
2026-09-29 19:39 ` [PATCH 3/9] gdb: simplify insertion in global_symbol_searcher::add_matching_symbols Simon Marchi
2026-09-30 8:20 ` Rohr, Stephan
2026-09-30 16:36 ` Simon Marchi
2026-09-29 19:39 ` [PATCH 4/9] gdb: use a switch on m_kind in add_matching_symbols Simon Marchi
[not found] ` <DS7PR11MB624725508FC878F3C263C2B3938B2@DS7PR11MB6247.namprd11.prod.outlook.com>
2026-09-30 19:41 ` Simon Marchi
2026-09-29 19:39 ` [PATCH 5/9] gdb: make global_symbol_searcher::is_suitable_msymbol non-static Simon Marchi
2026-09-30 12:39 ` Rohr, Stephan
2026-09-29 19:39 ` [PATCH 6/9] gdb: update comment of global_symbol_searcher::expand_symtabs Simon Marchi
2026-09-30 13:08 ` Rohr, Stephan
2026-09-30 20:01 ` Simon Marchi
2026-09-29 19:39 ` [PATCH 7/9] gdb: add matching helpers to global_symbol_searcher Simon Marchi
2026-09-30 13:51 ` Rohr, Stephan [this message]
2026-09-30 20:27 ` Simon Marchi
2026-09-29 19:39 ` [PATCH 8/9] gdb: pass result containers by reference in global_symbol_searcher Simon Marchi
2026-09-30 13:58 ` Rohr, Stephan
2026-10-01 1:33 ` Simon Marchi
2026-09-29 19:39 ` [PATCH 9/9] gdb: fix some comments " Simon Marchi
2026-09-30 14:01 ` Rohr, Stephan
2026-10-01 1:38 ` Simon Marchi
2026-10-01 2:09 ` [PATCH v2 00/11] Cleanups " simon.marchi
2026-10-01 2:09 ` [PATCH v2 01/11] gdb: add symbol_search_kind enum for global_symbol_searcher simon.marchi
2026-10-01 10:30 ` Andrew Burgess
2026-10-01 13:22 ` Simon Marchi
2026-10-01 15:35 ` Tom Tromey
2026-10-01 2:09 ` [PATCH v2 02/11] gdb: rename some preg/treg variables simon.marchi
2026-10-01 12:59 ` Andrew Burgess
2026-10-01 13:52 ` Simon Marchi
2026-10-01 2:09 ` [PATCH v2 03/11] gdb: factor out regexp compilation from global_symbol_searcher::search simon.marchi
2026-10-01 13:19 ` Andrew Burgess
2026-10-01 13:54 ` Simon Marchi
2026-10-01 2:09 ` [PATCH v2 04/11] gdb: use compile_symbol_search_regex in stack.c simon.marchi
2026-10-01 13:43 ` Andrew Burgess
2026-10-01 15:36 ` Tom Tromey
2026-10-01 15:52 ` Simon Marchi
2026-10-01 2:09 ` [PATCH v2 05/11] gdb: simplify insertion in global_symbol_searcher::add_matching_symbols simon.marchi
2026-10-01 13:46 ` Andrew Burgess
2026-10-01 2:09 ` [PATCH v2 06/11] gdb: use a switch on m_kind in add_matching_symbols simon.marchi
2026-10-01 15:01 ` Andrew Burgess
2026-10-01 15:07 ` Simon Marchi
2026-10-01 2:09 ` [PATCH v2 07/11] gdb: make global_symbol_searcher::is_suitable_msymbol non-static simon.marchi
2026-10-01 15:09 ` Andrew Burgess
2026-10-01 2:09 ` [PATCH v2 08/11] gdb: update comments of global_symbol_searcher::expand_symtabs simon.marchi
2026-10-01 15:13 ` Andrew Burgess
2026-10-01 2:09 ` [PATCH v2 09/11] gdb: add matching helpers to global_symbol_searcher simon.marchi
2026-10-01 15:39 ` Andrew Burgess
2026-10-01 15:48 ` Simon Marchi
2026-10-01 2:09 ` [PATCH v2 10/11] gdb: change some pointers to references in global_symbol_searcher simon.marchi
2026-10-01 15:53 ` Andrew Burgess
2026-10-01 15:55 ` Simon Marchi
2026-10-01 2:09 ` [PATCH v2 11/11] gdb: fix some comments " simon.marchi
2026-10-01 15:56 ` Andrew Burgess
2026-10-01 17:47 ` Simon Marchi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DS7PR11MB6247BB6E26265A540B9CC816938B2@DS7PR11MB6247.namprd11.prod.outlook.com \
--to=stephan.rohr@intel.com \
--cc=gdb-patches@sourceware.org \
--cc=simon.marchi@efficios.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox