[v2,09/11] gdb: add matching helpers to global_symbol_searcher
Commit Message
From: Simon Marchi <simon.marchi@efficios.com>
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 | 16 ++++
2 files changed, 130 insertions(+), 91 deletions(-)
Comments
simon.marchi@polymtl.ca writes:
> From: Simon Marchi <simon.marchi@efficios.com>
>
> 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 | 16 ++++
> 2 files changed, 130 insertions(+), 91 deletions(-)
>
> diff --git a/gdb/symtab.c b/gdb/symtab.c
> index fc0ff22448ee..ef21f285abcd 100644
> --- a/gdb/symtab.c
> +++ b/gdb/symtab.c
> @@ -4906,34 +4938,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)
Two lines here where NULL can be updated to nullptr.
With that fix:
Approved-By: Andrew Burgess <aburgess@redhat.com>
Thanks,
Andrew
On 10/1/26 11:39 AM, Andrew Burgess wrote:
> simon.marchi@polymtl.ca writes:
>
>> From: Simon Marchi <simon.marchi@efficios.com>
>>
>> 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 | 16 ++++
>> 2 files changed, 130 insertions(+), 91 deletions(-)
>>
>> diff --git a/gdb/symtab.c b/gdb/symtab.c
>> index fc0ff22448ee..ef21f285abcd 100644
>> --- a/gdb/symtab.c
>> +++ b/gdb/symtab.c
>
>
>> @@ -4906,34 +4938,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)
>
> Two lines here where NULL can be updated to nullptr.
Yes, I had already fixed them following your previous review.
> With that fix:
>
> Approved-By: Andrew Burgess <aburgess@redhat.com>
Thanks,
Simon
@@ -4739,6 +4739,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 ()
+ && !regex_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
@@ -4761,8 +4848,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,
@@ -4788,25 +4874,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;
}
}
@@ -4822,8 +4899,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);
-
/* Add matching symbols (if not already present). */
for (compunit_symtab &cust : objfile->compunits ())
{
@@ -4835,54 +4910,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))))
- continue;
-
- if (!sym->matches (domain))
+ if (!symbol_matches (sym, name_regex, type_regex))
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 ()
- && !regex_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);
@@ -4906,34 +4938,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;
@@ -2687,6 +2687,22 @@ 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 kind M_KIND, is in a file matching
+ 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 kind M_KIND and matches NAME_REGEX.
+
+ This function also filters out the minimal symbols artificially created by
+ GDB. */
+ bool msymbol_matches (const minimal_symbol *msymbol,
+ const std::optional<compiled_regex> &name_regex) const;
+
/* Expand symtabs in OBJFILE that match NAME_REGEX and 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