[review,v3] gdb: Introduce global_symbol_searcher

Message ID 20191108005036.92D6C25B28@gnutoolchain-gerrit.osci.io
State New
Headers show
Series
  • [review,v3] gdb: Introduce global_symbol_searcher
Related show

Commit Message

Simon Marchi (Code Review) Nov. 8, 2019, 12:50 a.m.
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(-)




-- 
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

Patch

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,
 					&regex, &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