From: "Andrew Burgess (Code Review)" <gerrit@gnutoolchain-gerrit.osci.io>
To: Tom Tromey <tromey@sourceware.org>, gdb-patches@sourceware.org
Cc: Christian Biesinger <cbiesinger@google.com>,
Simon Marchi <simon.marchi@polymtl.ca>
Subject: [review v3] gdb: Introduce global_symbol_searcher
Date: Fri, 08 Nov 2019 00:50:00 -0000 [thread overview]
Message-ID: <20191108005036.92D6C25B28@gnutoolchain-gerrit.osci.io> (raw)
In-Reply-To: <gerrit.1571909344000.I488ab292a892d9e9e84775c632c5f198b6ad3710@gnutoolchain-gerrit.osci.io>
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/264
......................................................................
gdb: Introduce global_symbol_searcher
Introduce a new class to wrap up the parameters needed for the
function search_symbols, which has now become a member function of
this new class.
The motivation is that search_symbols already takes a lot of
parameters, and a future commit is going to add even more. This
commit hopefully makes collecting the state required for a search
easier.
As part of this conversion the list of filenames in which to search
has been converted to a std::vector.
There should be no user visible changes after this commit.
gdb/ChangeLog:
* python/python.c (gdbpy_rbreak): Convert to using
global_symbol_searcher.
* symtab.c (file_matches): Convert return type to bool, change
file list to std::vector.
(search_symbols): Rename to...
(global_symbol_searcher::search): ...this and update now its
a member function of global_symbol_searcher. Take account of the
changes to file_matches.
(symtab_symbol_info): Convert to using global_symbol_searcher.
(rbreak_command): Likewise.
(search_module_symbols): Likewise.
* symtab.h (enum symbol_search): Update comment.
(search_symbols): Remove declaration.
(class global_symbol_searcher): New class.
Change-Id: I488ab292a892d9e9e84775c632c5f198b6ad3710
---
M gdb/ChangeLog
M gdb/python/python.c
M gdb/symtab.c
M gdb/symtab.h
4 files changed, 124 insertions(+), 100 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 41daef6..55f8d1e 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,20 @@
+2019-11-01 Andrew Burgess <andrew.burgess@embecosm.com>
+
+ * python/python.c (gdbpy_rbreak): Convert to using
+ global_symbol_searcher.
+ * symtab.c (file_matches): Convert return type to bool, change
+ file list to std::vector.
+ (search_symbols): Rename to...
+ (global_symbol_searcher::search): ...this and update now its
+ a member function of global_symbol_searcher. Take account of the
+ changes to file_matches.
+ (symtab_symbol_info): Convert to using global_symbol_searcher.
+ (rbreak_command): Likewise.
+ (search_module_symbols): Likewise.
+ * symtab.h (enum symbol_search): Update comment.
+ (search_symbols): Remove declaration.
+ (class global_symbol_searcher): New class.
+
2019-11-04 Christian Biesinger <cbiesinger@google.com>
* psympriv.h: Add static_asserts for sizeof (general_symbol_info)
diff --git a/gdb/python/python.c b/gdb/python/python.c
index ddf0e72..7345770 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -645,19 +645,6 @@
static PyObject *
gdbpy_rbreak (PyObject *self, PyObject *args, PyObject *kw)
{
- /* A simple type to ensure clean up of a vector of allocated strings
- when a C interface demands a const char *array[] type
- interface. */
- struct symtab_list_type
- {
- ~symtab_list_type ()
- {
- for (const char *elem: vec)
- xfree ((void *) elem);
- }
- std::vector<const char *> vec;
- };
-
char *regex = NULL;
std::vector<symbol_search> symbols;
unsigned long count = 0;
@@ -667,7 +654,6 @@
unsigned int throttle = 0;
static const char *keywords[] = {"regex","minsyms", "throttle",
"symtabs", NULL};
- symtab_list_type symtab_paths;
if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "s|O!IO", keywords,
®ex, &PyBool_Type,
@@ -684,6 +670,12 @@
minsyms_p = cmp;
}
+ global_symbol_searcher spec (FUNCTIONS_DOMAIN, regex);
+ SCOPE_EXIT {
+ for (const char *elem : spec.filenames)
+ xfree ((void *) elem);
+ };
+
/* The "symtabs" keyword is any Python iterable object that returns
a gdb.Symtab on each iteration. If specified, iterate through
the provided gdb.Symtabs and extract their full path. As
@@ -729,20 +721,13 @@
/* Make sure there is a definite place to store the value of
filename before it is released. */
- symtab_paths.vec.push_back (nullptr);
- symtab_paths.vec.back () = filename.release ();
+ spec.filenames.push_back (nullptr);
+ spec.filenames.back () = filename.release ();
}
}
- if (symtab_list)
- {
- const char **files = symtab_paths.vec.data ();
-
- symbols = search_symbols (regex, FUNCTIONS_DOMAIN, NULL,
- symtab_paths.vec.size (), files, false);
- }
- else
- symbols = search_symbols (regex, FUNCTIONS_DOMAIN, NULL, 0, NULL, false);
+ /* The search spec. */
+ symbols = spec.search ();
/* Count the number of symbols (both symbols and optionally minimal
symbols) so we can correctly check the throttle limit. */
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 2c934b9..092e9ff 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -4339,27 +4339,24 @@
printf_filtered ("\n");
}
-/* Compare FILE against all the NFILES entries of FILES. If BASENAMES is
- non-zero compare only lbasename of FILES. */
+/* Compare FILE against all the entries of FILENAMES. If BASENAMES is
+ non-zero compare only lbasename of FILENAMES. */
-static int
-file_matches (const char *file, const char *files[], int nfiles, int basenames)
+static bool
+file_matches (const char *file, const std::vector<const char *> &filenames,
+ bool basenames)
{
- int i;
+ if (filenames.empty ())
+ return true;
- if (file != NULL && nfiles != 0)
+ for (const char *name : filenames)
{
- for (i = 0; i < nfiles; i++)
- {
- if (compare_filenames_for_search (file, (basenames
- ? lbasename (files[i])
- : files[i])))
- return 1;
- }
+ name = (basenames ? lbasename (name) : name);
+ if (compare_filenames_for_search (file, name))
+ return true;
}
- else if (nfiles == 0)
- return 1;
- return 0;
+
+ return false;
}
/* Helper function for sort_search_symbols_remove_dups and qsort. Can only
@@ -4436,31 +4433,16 @@
result->end ());
}
-/* Search the symbol table for matches to the regular expression REGEXP,
- returning the results.
-
- Only symbols of KIND are searched:
- VARIABLES_DOMAIN - search all symbols, excluding functions, type names,
- and constants (enums).
- if T_REGEXP is not NULL, only returns var that have
- a type matching regular expression T_REGEXP.
- FUNCTIONS_DOMAIN - search all functions
- TYPES_DOMAIN - search all type names
- ALL_DOMAIN - an internal error for this function
-
- Within each file the results are sorted locally; each symtab's global and
- static blocks are separately alphabetized.
- Duplicate entries are removed.
-
- When EXCLUDE_MINSYMS is false then matching minsyms are also returned,
- otherwise they are excluded. */
+/* See symtab.h. */
std::vector<symbol_search>
-search_symbols (const char *regexp, enum search_domain kind,
- const char *t_regexp,
- int nfiles, const char *files[],
- bool exclude_minsyms)
+global_symbol_searcher::search () const
{
+ const char *regexp = m_symbol_regexp;
+ const char *t_regexp = m_type_regexp;
+ enum search_domain kind = m_kind;
+ bool exclude_minsyms = m_exclude_minsyms;
+ int nfiles = filenames.size ();
const struct blockvector *bv;
const struct block *b;
int i = 0;
@@ -4543,8 +4525,11 @@
the machinery below. */
expand_symtabs_matching ([&] (const char *filename, bool basenames)
{
- return file_matches (filename, files, nfiles,
- basenames);
+ /* EXPAND_SYMTABS_MATCHING expects a callback
+ that returns an integer, not a boolean as
+ FILE_MATCHES does. */
+ return file_matches (filename, filenames,
+ basenames) ? 1 : 0;
},
lookup_name_info::match_any (),
[&] (const char *symname)
@@ -4628,12 +4613,12 @@
/* 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, files, nfiles, 0)
+ if ((file_matches (real_symtab->filename, filenames, false)
|| ((basenames_may_differ
|| file_matches (lbasename (real_symtab->filename),
- files, nfiles, 1))
+ filenames, true))
&& file_matches (symtab_to_fullname (real_symtab),
- files, nfiles, 0)))
+ filenames, false)))
&& ((!preg.has_value ()
|| preg->exec (SYMBOL_NATURAL_NAME (sym), 0,
NULL, 0) == 0)
@@ -4844,9 +4829,8 @@
regexp = nullptr;
/* Must make sure that if we're interrupted, symbols gets freed. */
- std::vector<symbol_search> symbols = search_symbols (regexp, kind,
- t_regexp, 0, NULL,
- exclude_minsyms);
+ global_symbol_searcher spec (kind, regexp, t_regexp, exclude_minsyms);
+ std::vector<symbol_search> symbols = spec.search ();
if (!quiet)
{
@@ -5085,11 +5069,9 @@
rbreak_command (const char *regexp, int from_tty)
{
std::string string;
- const char **files = NULL;
- const char *file_name;
- int nfiles = 0;
+ const char *file_name = nullptr;
- if (regexp)
+ if (regexp != nullptr)
{
const char *colon = strchr (regexp, ':');
@@ -5105,17 +5087,14 @@
while (isspace (local_name[colon_index]))
local_name[colon_index--] = 0;
file_name = local_name;
- files = &file_name;
- nfiles = 1;
regexp = skip_spaces (colon + 1);
}
}
- std::vector<symbol_search> symbols = search_symbols (regexp,
- FUNCTIONS_DOMAIN,
- NULL,
- nfiles, files,
- false);
+ global_symbol_searcher spec (FUNCTIONS_DOMAIN, regexp);
+ if (file_name != nullptr)
+ spec.filenames.push_back (file_name);
+ std::vector<symbol_search> symbols = spec.search ();
scoped_rbreak_breakpoints finalize;
for (const symbol_search &p : symbols)
@@ -6360,17 +6339,14 @@
std::vector<module_symbol_search> results;
/* Search for all modules matching MODULE_REGEXP. */
- std::vector<symbol_search> modules = search_symbols (module_regexp,
- MODULES_DOMAIN,
- NULL, 0, NULL,
- true);
+ global_symbol_searcher spec1 (MODULES_DOMAIN, module_regexp, nullptr, true);
+ std::vector<symbol_search> modules = spec1.search ();
/* Now search for all symbols of the required KIND matching the required
regular expressions. We figure out which ones are in which modules
below. */
- std::vector<symbol_search> symbols = search_symbols (regexp, kind,
- type_regexp, 0,
- NULL, true);
+ global_symbol_searcher spec2 (kind, regexp, type_regexp, true);
+ std::vector<symbol_search> symbols = spec2.search ();
/* Now iterate over all MODULES, checking to see which items from
SYMBOLS are in each module. */
diff --git a/gdb/symtab.h b/gdb/symtab.h
index 83b75d1..6cda5df 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -823,7 +823,7 @@
extern const char *domain_name (domain_enum);
-/* Searching domains, used for `search_symbols'. Element numbers are
+/* Searching domains, used when searching for symbols. Element numbers are
hardcoded in GDB, check all enum uses before changing it. */
enum search_domain
@@ -2026,11 +2026,9 @@
extern symbol *find_function_alias_target (bound_minimal_symbol msymbol);
/* Symbol searching */
-/* Note: struct symbol_search, search_symbols, et.al. are declared here,
- instead of making them local to symtab.c, for gdbtk's sake. */
-/* When using search_symbols, a vector of the following structs is
- returned. */
+/* When using the symbol_searcher struct to search for symbols, a vector of
+ the following structs is returned. */
struct symbol_search
{
symbol_search (int block_, struct symbol *symbol_)
@@ -2079,12 +2077,60 @@
const symbol_search &sym_b);
};
-extern std::vector<symbol_search> search_symbols (const char *,
- enum search_domain,
- const char *,
- int,
- const char **,
- bool);
+/* In order to search for global symbols of a particular kind matching
+ particular regular expressions, create an instance of this structure and
+ call the SEARCH member function. */
+class global_symbol_searcher
+{
+public:
+
+ /* Constructor. */
+ global_symbol_searcher (enum search_domain kind,
+ const char *symbol_regexp = nullptr,
+ const char *type_regexp = nullptr,
+ bool exclude_minsyms = false,
+ std::vector<const char *> filename = {})
+ : m_kind (kind),
+ m_symbol_regexp (symbol_regexp),
+ m_type_regexp (type_regexp),
+ m_exclude_minsyms (exclude_minsyms)
+ {
+ /* The symbol searching is designed to only find one kind of thing. */
+ gdb_assert (m_kind != ALL_DOMAIN);
+ }
+
+ /* Search the symbol table for matches as defined by SEARCH_SPEC.
+
+ Within each file the results are sorted locally; each symtab's global
+ and static blocks are separately alphabetized. Duplicate entries are
+ removed. */
+ std::vector<symbol_search> search () const;
+
+ /* The set of source files to search in for matching symbols. This is
+ currently public so that it can be populated after this object has
+ been constructed. */
+ std::vector<const char *> filenames;
+
+private:
+ /* The kind of symbols are we searching for.
+ VARIABLES_DOMAIN - Search all symbols, excluding functions, type
+ names, and constants (enums).
+ FUNCTIONS_DOMAIN - Search all functions..
+ TYPES_DOMAIN - Search all type names.
+ MODULES_DOMAIN - Search all Fortran modules.
+ ALL_DOMAIN - Not valid for this function. */
+ enum search_domain m_kind;
+
+ /* Regular expression to match against the symbol name. */
+ const char *m_symbol_regexp = nullptr;
+
+ /* Regular expression to match against the type of the symbol. */
+ const char *m_type_regexp = nullptr;
+
+ /* When this flag is false then minsyms that match M_SYMBOL_REGEXP will
+ be included in the results, otherwise they are excluded. */
+ bool m_exclude_minsyms = false;
+};
/* When searching for Fortran symbols within modules (functions/variables)
we return a vector of this type. The first item in the pair is the
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I488ab292a892d9e9e84775c632c5f198b6ad3710
Gerrit-Change-Number: 264
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-CC: Christian Biesinger <cbiesinger@google.com>
Gerrit-CC: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-MessageType: newpatchset
next prev parent reply other threads:[~2019-11-08 0:50 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <gerrit.1571909344000.I488ab292a892d9e9e84775c632c5f198b6ad3710@gnutoolchain-gerrit.osci.io>
2019-10-30 14:19 ` [review] gdb: Introduce symbol_search_spec Tom Tromey (Code Review)
2019-10-30 14:31 ` Christian Biesinger (Code Review)
2019-11-01 1:28 ` [review v2] " Andrew Burgess (Code Review)
2019-11-01 1:30 ` Andrew Burgess (Code Review)
2019-11-01 13:58 ` Simon Marchi (Code Review)
2019-11-08 0:50 ` Andrew Burgess (Code Review) [this message]
2019-11-21 3:32 ` [review v3] gdb: Introduce global_symbol_searcher Simon Marchi (Code Review)
2019-11-21 4:02 ` Simon Marchi (Code Review)
2019-11-22 16:42 ` [review v4] " Andrew Burgess (Code Review)
2019-11-22 16:52 ` Andrew Burgess (Code Review)
2019-11-22 17:32 ` [review v5] " Andrew Burgess (Code Review)
2019-11-22 17:33 ` Andrew Burgess (Code Review)
2019-11-23 13:17 ` Simon Marchi (Code Review)
2019-11-26 23:26 ` [review v6] " Andrew Burgess (Code Review)
2019-11-26 23:40 ` [review v7] " Andrew Burgess (Code Review)
2019-11-26 23:43 ` Andrew Burgess (Code Review)
2019-11-27 3:59 ` Simon Marchi (Code Review)
2019-11-27 13:03 ` [pushed] " Sourceware to Gerrit sync (Code Review)
2019-11-27 13:03 ` Sourceware to Gerrit sync (Code Review)
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=20191108005036.92D6C25B28@gnutoolchain-gerrit.osci.io \
--to=gerrit@gnutoolchain-gerrit.osci.io \
--cc=andrew.burgess@embecosm.com \
--cc=cbiesinger@google.com \
--cc=gdb-patches@sourceware.org \
--cc=simon.marchi@polymtl.ca \
--cc=tromey@sourceware.org \
/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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).