[review] gdb: Introduce symbol_search_spec

[review] gdb: Introduce symbol_search_spec

[Tom Tromey (Code Review)]

Tom Tromey has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/264
......................................................................


Patch Set 1: Code-Review+2

(2 comments)

Thanks, this looks good.  I did have one question about whether search_symbols
should remain a function or should be a method of the new struct.

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/264/1/gdb/symtab.h 
File gdb/symtab.h:

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/264/1/gdb/symtab.h@2094 
PS1, Line 2094: 
2069 | struct search_symbols_spec
     | ...
2093 |   /* Constructor.  */
2094 |   search_symbols_spec (enum search_domain kind,
2095 | 		       const char *symbol_regexp = nullptr,
2096 | 		       const char *type_regexp = nullptr,
2097 | 		       bool exclude_minsyms = false)
2098 |     : kind (kind),
2099 |       symbol_regexp (symbol_regexp),
2100 |       type_regexp (type_regexp),
2101 |       exclude_minsyms (exclude_minsyms)
2102 |   { /* Nothing.  */ }

This could gdb_assert (kind != ALL_DOMAIN)


https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/264/1/gdb/symtab.c 
File gdb/symtab.c:

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/264/1/gdb/symtab.c@4449 
PS1, Line 4449: 
4447 | 
4448 | std::vector<symbol_search>
4449 | search_symbols (const search_symbols_spec &search_spec)
4450 | {
4451 |   /* Unpack the search spec.  */
4452 |   const char *regexp = search_spec.symbol_regexp;
4453 |   enum search_domain kind = search_spec.kind;
4454 |   const char *t_regexp = search_spec.type_regexp;
4455 |   int nfiles = search_spec.filenames.size ();
4456 |   bool exclude_minsyms = search_spec.exclude_minsyms;

Did you consider making search_symbols a method of
search_symbols_spec?  Then this unpacking wouldn't be
needed.



-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I488ab292a892d9e9e84775c632c5f198b6ad3710
Gerrit-Change-Number: 264
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-Comment-Date: Wed, 30 Oct 2019 14:19:19 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[review] gdb: Introduce symbol_search_spec

[Christian Biesinger (Code Review)]

Christian Biesinger has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/264
......................................................................


Patch Set 1:

(1 comment)

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/264/1/gdb/symtab.c 
File gdb/symtab.c:

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/264/1/gdb/symtab.c@4359 
PS1, Line 4359: 
4350 | }
4351 | 
4352 | /* Compare FILE against all the entries of FILENAMES.  If BASENAMES is
4353 |    non-zero compare only lbasename of FILENAMES.  */
4354 | 
4355 | static bool
4356 | file_matches (const char *file, const std::vector<const char *> &filenames,
4357 | 	      bool basenames)
4358 | {
4359 |   if (filenames.size () == 0)

filenames.empty()?



-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I488ab292a892d9e9e84775c632c5f198b6ad3710
Gerrit-Change-Number: 264
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-CC: Christian Biesinger <cbiesinger@google.com>
Gerrit-Comment-Date: Wed, 30 Oct 2019 14:30:59 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

[review v2] gdb: Introduce symbol_search_spec

[Andrew Burgess (Code Review)]

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/264
......................................................................

gdb: Introduce symbol_search_spec

TODO: Update changelogs and commit message.

Introduce a new structure to wrap up the parameters needed for
search_symbols.  We already pass a lot of parameters to
search_symbols and a later commit is going to add more.  As part of
this conversion the list of filenames 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
	search_symbols_spec.
	* symtab.c (file_matches): Convert return type to bool, change
	file list to std::vector.
	(search_symbols): Rename to...
	(search_symbols_spec::search_symbols): ...this and update now its
	a member function of search_symbols_spec.  Take account of the
	changes to file_matches.
	(symtab_symbol_info): Convert to using search_symbols_spec.
	(rbreak_command): Likewise.
	(search_module_symbols): Likewise.
	* symtab.h (struct search_symbols_spec): New struct.
	(search_symbols): Convert to be a member of search_symbols_spec.

Change-Id: I488ab292a892d9e9e84775c632c5f198b6ad3710
---
M gdb/ChangeLog
M gdb/python/python.c
M gdb/symtab.c
M gdb/symtab.h
4 files changed, 120 insertions(+), 95 deletions(-)



diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 381147b..980336e 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,19 @@
+2019-11-01  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* python/python.c (gdbpy_rbreak): Convert to using
+	search_symbols_spec.
+	* symtab.c (file_matches): Convert return type to bool, change
+	file list to std::vector.
+	(search_symbols): Rename to...
+	(search_symbols_spec::search_symbols): ...this and update now its
+	a member function of search_symbols_spec.  Take account of the
+	changes to file_matches.
+	(symtab_symbol_info): Convert to using search_symbols_spec.
+	(rbreak_command): Likewise.
+	(search_module_symbols): Likewise.
+	* symtab.h (struct search_symbols_spec): New struct.
+	(search_symbols): Convert to be a member of search_symbols_spec.
+
 2019-10-31  Andrew Burgess  <andrew.burgess@embecosm.com>
 
 	* ada-typeprint.c (ada_print_typedef): Don't print newline at the
diff --git a/gdb/python/python.c b/gdb/python/python.c
index ddf0e72..ae32f50 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;
     }
 
+  search_symbols_spec 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_symbols ();
 
   /* 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..6f7ada1 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)
+search_symbols_spec::search_symbols () 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);
+  search_symbols_spec spec (kind, regexp, t_regexp, exclude_minsyms);
+  std::vector<symbol_search> symbols = spec.search_symbols ();
 
   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);
+  search_symbols_spec spec (FUNCTIONS_DOMAIN, regexp);
+  if (file_name != nullptr)
+    spec.filenames.push_back (file_name);
+  std::vector<symbol_search> symbols = spec.search_symbols ();
 
   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);
+  search_symbols_spec spec1 (MODULES_DOMAIN, module_regexp, nullptr, true);
+  std::vector<symbol_search> modules = spec1.search_symbols ();
 
   /* 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);
+  search_symbols_spec spec2 (kind, regexp, type_regexp, true);
+  std::vector<symbol_search> symbols = spec2.search_symbols ();
 
   /* 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 eac44ae..bd665c9 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -2066,12 +2066,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_SYMBOLS member function.  */
+struct search_symbols_spec
+{
+public:
+
+  /* Constructor.  */
+  search_symbols_spec (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_symbols () 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: 2
Gerrit-Owner: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-CC: Christian Biesinger <cbiesinger@google.com>
Gerrit-MessageType: newpatchset

[review v2] gdb: Introduce symbol_search_spec

[Andrew Burgess (Code Review)]

Andrew Burgess has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/264
......................................................................


Patch Set 2:

(1 comment)

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/264/1/gdb/symtab.c 
File gdb/symtab.c:

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/264/1/gdb/symtab.c@4449 
PS1, Line 4449: 
4444 | }
4445 | 
4446 | /* See symtab.h.  */
4447 | 
4448 | std::vector<symbol_search>
4449 > search_symbols (const search_symbols_spec &search_spec)
4450 > {
4451 >   /* Unpack the search spec.  */
4452 >   const char *regexp = search_spec.symbol_regexp;
4453 >   enum search_domain kind = search_spec.kind;
4454 >   const char *t_regexp = search_spec.type_regexp;
4455 >   int nfiles = search_spec.filenames.size ();
4456 >   bool exclude_minsyms = search_spec.exclude_minsyms;
4457 | 
4458 |   /* The search.  */
4459 |   const struct blockvector *bv;
4460 |   const struct block *b;
4461 |   int i = 0;

> Did you consider making search_symbols a method of […]

I've done this in the new version of this patch.  This had more of an impact for later patches in the series.



-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I488ab292a892d9e9e84775c632c5f198b6ad3710
Gerrit-Change-Number: 264
Gerrit-PatchSet: 2
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-Comment-Date: Fri, 01 Nov 2019 01:30:55 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Tom Tromey <tromey@sourceware.org>
Gerrit-MessageType: comment

[review v2] gdb: Introduce symbol_search_spec

[Simon Marchi (Code Review)]

Simon Marchi has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/264
......................................................................


Patch Set 2:

(1 comment)

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/264/1/gdb/symtab.c 
File gdb/symtab.c:

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/264/1/gdb/symtab.c@4449 
PS1, Line 4449: 
4444 | }
4445 | 
4446 | /* See symtab.h.  */
4447 | 
4448 | std::vector<symbol_search>
4449 > search_symbols (const search_symbols_spec &search_spec)
4450 > {
4451 >   /* Unpack the search spec.  */
4452 >   const char *regexp = search_spec.symbol_regexp;
4453 >   enum search_domain kind = search_spec.kind;
4454 >   const char *t_regexp = search_spec.type_regexp;
4455 >   int nfiles = search_spec.filenames.size ();
4456 >   bool exclude_minsyms = search_spec.exclude_minsyms;
4457 | 
4458 |   /* The search.  */
4459 |   const struct blockvector *bv;
4460 |   const struct block *b;
4461 |   int i = 0;

> I've done this in the new version of this patch. […]

Then (name bikeshedding) it's not really a "search_symbols_spec", but more something like a "symbol_searcher".



-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I488ab292a892d9e9e84775c632c5f198b6ad3710
Gerrit-Change-Number: 264
Gerrit-PatchSet: 2
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-Comment-Date: Fri, 01 Nov 2019 13:58:20 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Andrew Burgess <andrew.burgess@embecosm.com>
Comment-In-Reply-To: Tom Tromey <tromey@sourceware.org>
Gerrit-MessageType: comment

[review v3] gdb: Introduce global_symbol_searcher

[Andrew Burgess (Code Review)]

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

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

[review v3] gdb: Introduce global_symbol_searcher

[Simon Marchi (Code Review)]

Simon Marchi has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/264
......................................................................


Patch Set 3:

(3 comments)

Just marking the comments as resolved, as they have been addressed in patchset 3.

| --- gdb/symtab.c
| +++ gdb/symtab.c
| @@ -4369,3 +4352,17 @@ file_matches (const char *file, const char *files[], int nfiles, int basenames)
| -    }
| -  else if (nfiles == 0)
| +/* Compare FILE against all the entries of FILENAMES.  If BASENAMES is
| +   non-zero compare only lbasename of FILENAMES.  */
| +
| +static bool
| +file_matches (const char *file, const std::vector<const char *> &filenames,
| +	      bool basenames)
| +{
| +  if (filenames.size () == 0)

PS1, Line 4359:

> filenames.empty()?

Done.

|      return 1;
| +
| +  for (const char *name : filenames)
| +    {
| +      name = (basenames ? lbasename (name) : name);
| +      if (compare_filenames_for_search (file, name))
| +	return 1;
| +    }
| +

 ...

| @@ -4466,15 +4446,20 @@ /* Search the symbol table for matches to the regular expression REGEXP,
| -   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)
| -{
| +search_symbols (const search_symbols_spec &search_spec)
| +{
| +  /* Unpack the search spec.  */
| +  const char *regexp = search_spec.symbol_regexp;
| +  enum search_domain kind = search_spec.kind;
| +  const char *t_regexp = search_spec.type_regexp;
| +  int nfiles = search_spec.filenames.size ();
| +  bool exclude_minsyms = search_spec.exclude_minsyms;

PS1, Line 4456:

> Did you consider making search_symbols a method of
search_symbols_spec?  Then this unpacking wouldn't be
needed.

Done.

| +
| +  /* The search.  */
|    const struct blockvector *bv;
|    const struct block *b;
|    int i = 0;
|    struct block_iterator iter;
|    struct symbol *sym;
|    int found_misc = 0;
|    static const enum minimal_symbol_type types[]
| --- gdb/symtab.h
| +++ gdb/symtab.h
| @@ -2073,0 +2085,27 @@ struct search_symbols_spec
| +
| +  /* When this flag is false then minsyms that match SYMBOL_REGEXP will be
| +     included in the results, otherwise they are excluded.  */
| +  bool exclude_minsyms = false;
| +
| +  /* The set of source files to search in for matching symbols.  */
| +  std::vector<const char *> filenames;
| +
| +  /* Constructor.  */
| +  search_symbols_spec (enum search_domain kind,
| +		       const char *symbol_regexp = nullptr,
| +		       const char *type_regexp = nullptr,
| +		       bool exclude_minsyms = false)
| +    : kind (kind),
| +      symbol_regexp (symbol_regexp),
| +      type_regexp (type_regexp),
| +      exclude_minsyms (exclude_minsyms)
| +  { /* Nothing.  */ }

PS1, Line 2102:

> This could gdb_assert (kind != ALL_DOMAIN)

Done.

| +};
| +
| +/* 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.  */
| +
| +extern std::vector<symbol_search> search_symbols

-- 
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: Joel Brobecker <brobecker@adacore.com>
Gerrit-CC: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Comment-Date: Thu, 21 Nov 2019 03:31:55 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Andrew Burgess <andrew.burgess@embecosm.com>
Comment-In-Reply-To: Christian Biesinger <cbiesinger@google.com>
Comment-In-Reply-To: Tom Tromey <tromey@sourceware.org>
Comment-In-Reply-To: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-MessageType: comment

[review v3] gdb: Introduce global_symbol_searcher

[Simon Marchi (Code Review)]

Simon Marchi has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/264
......................................................................


Patch Set 3: Code-Review-1

(6 comments)

| --- gdb/symtab.c
| +++ gdb/symtab.c
| @@ -4355,8 +4342,11 @@ file_matches (const char *file, const char *files[], int nfiles, int basenames)
| -						   ? lbasename (files[i])
| -						   : files[i])))
| -	    return 1;
| -	}
| -    }
| -  else if (nfiles == 0)
| -    return 1;
| -  return 0;
| +/* Compare FILE against all the entries of FILENAMES.  If BASENAMES is
| +   non-zero compare only lbasename of FILENAMES.  */

PS3, Line 4343:

true

| +
| +static bool
| +file_matches (const char *file, const std::vector<const char *> &filenames,
| +	      bool basenames)
| +{
| +  if (filenames.empty ())
| +    return true;
| +
| +  for (const char *name : filenames)

 ...

| @@ -4457,16 +4437,18 @@ /* 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 ();

PS3, Line 4445:

I think it would be clearer to just use the fields directly in this
function.

|    const struct blockvector *bv;
|    const struct block *b;
|    int i = 0;
|    struct block_iterator iter;
|    struct symbol *sym;
|    int found_misc = 0;
|    static const enum minimal_symbol_type types[]
|      = {mst_data, mst_text, mst_unknown};
|    static const enum minimal_symbol_type types2[]

 ...

| @@ -4539,16 +4521,19 @@ global_symbol_searcher::search () const
|      }
|  
|    /* Search through the partial symtabs *first* for all symbols
|       matching the regexp.  That way we don't have to reproduce all of
|       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.  */

PS3, Line 4530:

Does it?  Maybe this comes from an old patch, because the callback
type returns a bool, since:

 commit 14bc53a81471e0b550de1c24d4d5266f676aacc3
 Author:     Pedro Alves <palves@redhat.com>
 AuthorDate: Wed Feb 22 14:43:35 2017 +0000
 Commit:     Pedro Alves <palves@redhat.com>
 CommitDate: Thu Feb 23 16:16:06 2017 +0000

    Use gdb::function_view in iterate_over_symtabs & co

| +			     return file_matches (filename, filenames,
| +						  basenames) ? 1 : 0;
|  			   },
|  			   lookup_name_info::match_any (),
|  			   [&] (const char *symname)
|  			   {
|  			     return (!preg.has_value ()
|  				     || preg->exec (symname,
|  						    0, NULL, 0) == 0);

 ...

| @@ -4837,17 +4822,16 @@ symtab_symbol_info (bool quiet, bool exclude_minsyms,
|      {"variable", "function", "type", "module"};
|    const char *last_filename = "";
|    int first = 1;
|  
|    gdb_assert (kind != ALL_DOMAIN);
|  
|    if (regexp != nullptr && *regexp == '\0')
|      regexp = nullptr;
|  
|    /* Must make sure that if we're interrupted, symbols gets freed.  */

PS3, Line 4831:

This comment seems really outdated, maybe remove it?

| -  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)
|      {
|        if (regexp != NULL)
| --- gdb/symtab.h
| +++ gdb/symtab.h
| @@ -2088,0 +2080,32 @@ extern std::vector<symbol_search> search_symbols (const char *,
| +/* 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 = {})

PS3, Line 2092:

I don't know how big of a change it would be, but I think these
optional knobs would be better as methods on the class, rather than
parameters to the constructors.  The caller can just set those it
needs.  For example:

 global_symbol_searcher searcher (VARIABLES_DOMAIN);
 searcher.set_exclude_minsyms (true);
 searcher.set_name_regexp ("hello.*");

I think that's more readable than this, since the method names say
what they do:

 global_symbol_searcher searcher (VARIABLES_DOMAIN, "hello.*", NULL, false);

Also, as shown above, maybe "name_regexp", instead of "symbol_regexp",
since we're talking about the symbol name?

| +    : 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.

PS3, Line 2102:

I know that comes from the old comment, but I think it would be a good
time to improve it.

When I read "the symbol table", I wondered: which symbol table?  I
don't see any symbol table being passed.  So maybe it should say
"Search symbols of the objfiles in the current program space" or
something like that, it would be more accurate.

| +
| +     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.  */

-- 
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: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-CC: Christian Biesinger <cbiesinger@google.com>
Gerrit-CC: Joel Brobecker <brobecker@adacore.com>
Gerrit-Comment-Date: Thu, 21 Nov 2019 04:01:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[review v4] gdb: Introduce global_symbol_searcher

[Andrew Burgess (Code Review)]

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, update header comment.
	(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, 160 insertions(+), 123 deletions(-)



diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 93258c3..4a8bc0f 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,20 @@
+2019-11-22  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, update header comment.
+	(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-22  Tom de Vries  <tdevries@suse.de>
 
 	* contrib/words.sh: Improve words extraction.
diff --git a/gdb/python/python.c b/gdb/python/python.c
index f94214e..da68874 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 0064800..f9eddb0 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
+   true 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,30 +4433,10 @@
 		 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 struct blockvector *bv;
   const struct block *b;
@@ -4483,21 +4460,23 @@
   gdb::optional<compiled_regex> preg;
   gdb::optional<compiled_regex> treg;
 
-  gdb_assert (kind != ALL_DOMAIN);
+  gdb_assert (m_kind != ALL_DOMAIN);
 
-  ourtype = types[kind];
-  ourtype2 = types2[kind];
-  ourtype3 = types3[kind];
-  ourtype4 = types4[kind];
+  ourtype = types[m_kind];
+  ourtype2 = types2[m_kind];
+  ourtype3 = types3[m_kind];
+  ourtype4 = types4[m_kind];
 
-  if (regexp != NULL)
+  if (m_symbol_name_regexp != NULL)
     {
+      const char *symbol_name_regexp = m_symbol_name_regexp;
+
       /* Make sure spacing is right for C++ operators.
          This is just a courtesy to make the matching less sensitive
          to how many spaces the user leaves between 'operator'
          and <TYPENAME> or <OPERATOR>.  */
       const char *opend;
-      const char *opname = operator_chars (regexp, &opend);
+      const char *opname = operator_chars (symbol_name_regexp, &opend);
 
       if (*opname)
 	{
@@ -4522,29 +4501,34 @@
 	      char *tmp = (char *) alloca (8 + fix + strlen (opname) + 1);
 
 	      sprintf (tmp, "operator%.*s%s", fix, " ", opname);
-	      regexp = tmp;
+	      symbol_name_regexp = tmp;
 	    }
 	}
 
       int cflags = REG_NOSUB | (case_sensitivity == case_sensitive_off
 				? REG_ICASE : 0);
-      preg.emplace (regexp, cflags, _("Invalid regexp"));
+      preg.emplace (symbol_name_regexp, cflags,
+		    _("Invalid m_symbol_name_regexp"));
     }
 
-  if (t_regexp != NULL)
+  if (m_symbol_type_regexp != NULL)
     {
       int cflags = REG_NOSUB | (case_sensitivity == case_sensitive_off
 				? REG_ICASE : 0);
-      treg.emplace (t_regexp, cflags, _("Invalid regexp"));
+      treg.emplace (m_symbol_type_regexp, cflags,
+		    _("Invalid m_symbol_namregexp"));
     }
 
   /* Search through the partial symtabs *first* for all symbols
-     matching the regexp.  That way we don't have to reproduce all of
+     matching the m_symbol_namregexp.  That way we don't have to reproduce all of
      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)
@@ -4554,7 +4538,7 @@
 						    0, NULL, 0) == 0);
 			   },
 			   NULL,
-			   kind);
+			   m_kind);
 
   /* Here, we search through the minimal symbol tables for functions
      and variables that match, and force their symbols to be read.
@@ -4572,7 +4556,8 @@
      all objfiles.  In large programs (1000s of shared libs) searching all
      objfiles is not worth the pain.  */
 
-  if (nfiles == 0 && (kind == VARIABLES_DOMAIN || kind == FUNCTIONS_DOMAIN))
+  if (filenames.empty () && (m_kind == VARIABLES_DOMAIN
+			     || m_kind == FUNCTIONS_DOMAIN))
     {
       for (objfile *objfile : current_program_space->objfiles ())
 	{
@@ -4596,7 +4581,7 @@
 			 lookup functions is to expand the symbol
 			 table if msymbol is found, for the benefit of
 			 the next loop on compunits.  */
-		      if (kind == FUNCTIONS_DOMAIN
+		      if (m_kind == FUNCTIONS_DOMAIN
 			  ? (find_pc_compunit_symtab
 			     (MSYMBOL_VALUE_ADDRESS (objfile, msymbol))
 			     == NULL)
@@ -4628,16 +4613,16 @@
 		  /* 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)
-			  && ((kind == VARIABLES_DOMAIN
+			  && ((m_kind == VARIABLES_DOMAIN
 			       && SYMBOL_CLASS (sym) != LOC_TYPEDEF
 			       && SYMBOL_CLASS (sym) != LOC_UNRESOLVED
 			       && SYMBOL_CLASS (sym) != LOC_BLOCK
@@ -4650,15 +4635,15 @@
 					== TYPE_CODE_ENUM))
 			       && (!treg.has_value ()
 				   || treg_matches_sym_type_name (*treg, sym)))
-			      || (kind == FUNCTIONS_DOMAIN
+			      || (m_kind == FUNCTIONS_DOMAIN
 				  && SYMBOL_CLASS (sym) == LOC_BLOCK
 				  && (!treg.has_value ()
 				      || treg_matches_sym_type_name (*treg,
 								     sym)))
-			      || (kind == TYPES_DOMAIN
+			      || (m_kind == TYPES_DOMAIN
 				  && SYMBOL_CLASS (sym) == LOC_TYPEDEF
 				  && SYMBOL_DOMAIN (sym) != MODULE_DOMAIN)
-			      || (kind == MODULES_DOMAIN
+			      || (m_kind == MODULES_DOMAIN
 				  && SYMBOL_DOMAIN (sym) == MODULE_DOMAIN
 				  && SYMBOL_LINE (sym) != 0))))
 		    {
@@ -4675,11 +4660,11 @@
 
   /* If there are no eyes, avoid all contact.  I mean, if there are
      no debug symbols, then add matching minsyms.  But if the user wants
-     to see symbols matching a type regexp, then never give a minimal symbol,
+     to see symbols matching a type m_symbol_namregexp, then never give a minimal symbol,
      as we assume that a minimal symbol does not have a type.  */
 
-  if ((found_misc || (nfiles == 0 && kind != FUNCTIONS_DOMAIN))
-      && !exclude_minsyms
+  if ((found_misc || (filenames.empty () && m_kind != FUNCTIONS_DOMAIN))
+      && !m_exclude_minsyms
       && !treg.has_value ())
     {
       for (objfile *objfile : current_program_space->objfiles ())
@@ -4702,7 +4687,7 @@
 		    {
 		      /* For functions we can do a quick check of whether the
 			 symbol might be found via find_pc_symtab.  */
-		      if (kind != FUNCTIONS_DOMAIN
+		      if (m_kind != FUNCTIONS_DOMAIN
 			  || (find_pc_compunit_symtab
 			      (MSYMBOL_VALUE_ADDRESS (objfile, msymbol))
 			      == NULL))
@@ -4844,9 +4829,10 @@
     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);
+  spec.set_symbol_type_regexp (t_regexp);
+  spec.set_exclude_minsyms (exclude_minsyms);
+  std::vector<symbol_search> symbols = spec.search ();
 
   if (!quiet)
     {
@@ -5085,11 +5071,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 +5089,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)
@@ -6349,17 +6330,17 @@
   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);
+  spec1.set_exclude_minsyms (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);
+  spec2.set_symbol_type_regexp (type_regexp);
+  spec2.set_exclude_minsyms (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 8f95a3a..43aa9a9 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -811,7 +811,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,68 @@
 				  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_name_regexp)
+    : m_kind (kind),
+      m_symbol_name_regexp (symbol_name_regexp)
+  {
+    /* The symbol searching is designed to only find one kind of thing.  */
+    gdb_assert (m_kind != ALL_DOMAIN);
+  }
+
+  /* Set the optional regexp that matches against the symbol type.  */
+  void set_symbol_type_regexp (const char *regexp)
+  {
+    m_symbol_type_regexp = regexp;
+  }
+
+  /* Set the flag to exclude minsyms from the search results.  */
+  void set_exclude_minsyms (bool exclude_minsyms)
+  {
+    m_exclude_minsyms = exclude_minsyms;
+  }
+
+  /* Search the symbols from all objfiles in the current program space
+     looking for matches as defined by the current state of this object.
+
+     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_name_regexp = nullptr;
+
+  /* Regular expression to match against the symbol type.  */
+  const char *m_symbol_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: 4
Gerrit-Owner: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-CC: Christian Biesinger <cbiesinger@google.com>
Gerrit-CC: Joel Brobecker <brobecker@adacore.com>
Gerrit-MessageType: newpatchset

[review v4] gdb: Introduce global_symbol_searcher

[Andrew Burgess (Code Review)]

Andrew Burgess has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/264
......................................................................


Patch Set 4:

(4 comments)

Updated and addressed most of the comments, but I realise I missed one.

| --- gdb/symtab.c
| +++ gdb/symtab.c
| @@ -4355,8 +4342,11 @@ file_matches (const char *file, const char *files[], int nfiles, int basenames)
| -						   ? lbasename (files[i])
| -						   : files[i])))
| -	    return 1;
| -	}
| -    }
| -  else if (nfiles == 0)
| -    return 1;
| -  return 0;
| +/* Compare FILE against all the entries of FILENAMES.  If BASENAMES is
| +   non-zero compare only lbasename of FILENAMES.  */

PS3, Line 4343:

Done.

| +
| +static bool
| +file_matches (const char *file, const std::vector<const char *> &filenames,
| +	      bool basenames)
| +{
| +  if (filenames.empty ())
| +    return true;
| +
| +  for (const char *name : filenames)

 ...

| @@ -4837,17 +4822,16 @@ symtab_symbol_info (bool quiet, bool exclude_minsyms,
|      {"variable", "function", "type", "module"};
|    const char *last_filename = "";
|    int first = 1;
|  
|    gdb_assert (kind != ALL_DOMAIN);
|  
|    if (regexp != nullptr && *regexp == '\0')
|      regexp = nullptr;
|  
|    /* Must make sure that if we're interrupted, symbols gets freed.  */

PS3, Line 4831:

Done.

| -  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)
|      {
|        if (regexp != NULL)
| --- gdb/symtab.h
| +++ gdb/symtab.h
| @@ -2088,0 +2080,32 @@ extern std::vector<symbol_search> search_symbols (const char *,
| +/* 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 = {})

PS3, Line 2092:

Done.

| +    : 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.

PS3, Line 2102:

Updated.

| +
| +     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.  */

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I488ab292a892d9e9e84775c632c5f198b6ad3710
Gerrit-Change-Number: 264
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-CC: Christian Biesinger <cbiesinger@google.com>
Gerrit-CC: Joel Brobecker <brobecker@adacore.com>
Gerrit-Comment-Date: Fri, 22 Nov 2019 16:51:57 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-MessageType: comment

[review v5] gdb: Introduce global_symbol_searcher

[Andrew Burgess (Code Review)]

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, update header comment.
	(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, 156 insertions(+), 123 deletions(-)



diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 93258c3..4a8bc0f 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,20 @@
+2019-11-22  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, update header comment.
+	(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-22  Tom de Vries  <tdevries@suse.de>
 
 	* contrib/words.sh: Improve words extraction.
diff --git a/gdb/python/python.c b/gdb/python/python.c
index f94214e..da68874 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 0064800..5efb83b 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
+   true 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,30 +4433,10 @@
 		 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 struct blockvector *bv;
   const struct block *b;
@@ -4483,21 +4460,23 @@
   gdb::optional<compiled_regex> preg;
   gdb::optional<compiled_regex> treg;
 
-  gdb_assert (kind != ALL_DOMAIN);
+  gdb_assert (m_kind != ALL_DOMAIN);
 
-  ourtype = types[kind];
-  ourtype2 = types2[kind];
-  ourtype3 = types3[kind];
-  ourtype4 = types4[kind];
+  ourtype = types[m_kind];
+  ourtype2 = types2[m_kind];
+  ourtype3 = types3[m_kind];
+  ourtype4 = types4[m_kind];
 
-  if (regexp != NULL)
+  if (m_symbol_name_regexp != NULL)
     {
+      const char *symbol_name_regexp = m_symbol_name_regexp;
+
       /* Make sure spacing is right for C++ operators.
          This is just a courtesy to make the matching less sensitive
          to how many spaces the user leaves between 'operator'
          and <TYPENAME> or <OPERATOR>.  */
       const char *opend;
-      const char *opname = operator_chars (regexp, &opend);
+      const char *opname = operator_chars (symbol_name_regexp, &opend);
 
       if (*opname)
 	{
@@ -4522,28 +4501,30 @@
 	      char *tmp = (char *) alloca (8 + fix + strlen (opname) + 1);
 
 	      sprintf (tmp, "operator%.*s%s", fix, " ", opname);
-	      regexp = tmp;
+	      symbol_name_regexp = tmp;
 	    }
 	}
 
       int cflags = REG_NOSUB | (case_sensitivity == case_sensitive_off
 				? REG_ICASE : 0);
-      preg.emplace (regexp, cflags, _("Invalid regexp"));
+      preg.emplace (symbol_name_regexp, cflags,
+		    _("Invalid m_symbol_name_regexp"));
     }
 
-  if (t_regexp != NULL)
+  if (m_symbol_type_regexp != NULL)
     {
       int cflags = REG_NOSUB | (case_sensitivity == case_sensitive_off
 				? REG_ICASE : 0);
-      treg.emplace (t_regexp, cflags, _("Invalid regexp"));
+      treg.emplace (m_symbol_type_regexp, cflags,
+		    _("Invalid m_symbol_namregexp"));
     }
 
   /* Search through the partial symtabs *first* for all symbols
-     matching the regexp.  That way we don't have to reproduce all of
+     matching the m_symbol_namregexp.  That way we don't have to reproduce all of
      the machinery below.  */
   expand_symtabs_matching ([&] (const char *filename, bool basenames)
 			   {
-			     return file_matches (filename, files, nfiles,
+			     return file_matches (filename, filenames,
 						  basenames);
 			   },
 			   lookup_name_info::match_any (),
@@ -4554,7 +4535,7 @@
 						    0, NULL, 0) == 0);
 			   },
 			   NULL,
-			   kind);
+			   m_kind);
 
   /* Here, we search through the minimal symbol tables for functions
      and variables that match, and force their symbols to be read.
@@ -4572,7 +4553,8 @@
      all objfiles.  In large programs (1000s of shared libs) searching all
      objfiles is not worth the pain.  */
 
-  if (nfiles == 0 && (kind == VARIABLES_DOMAIN || kind == FUNCTIONS_DOMAIN))
+  if (filenames.empty () && (m_kind == VARIABLES_DOMAIN
+			     || m_kind == FUNCTIONS_DOMAIN))
     {
       for (objfile *objfile : current_program_space->objfiles ())
 	{
@@ -4596,7 +4578,7 @@
 			 lookup functions is to expand the symbol
 			 table if msymbol is found, for the benefit of
 			 the next loop on compunits.  */
-		      if (kind == FUNCTIONS_DOMAIN
+		      if (m_kind == FUNCTIONS_DOMAIN
 			  ? (find_pc_compunit_symtab
 			     (MSYMBOL_VALUE_ADDRESS (objfile, msymbol))
 			     == NULL)
@@ -4628,16 +4610,16 @@
 		  /* 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)
-			  && ((kind == VARIABLES_DOMAIN
+			  && ((m_kind == VARIABLES_DOMAIN
 			       && SYMBOL_CLASS (sym) != LOC_TYPEDEF
 			       && SYMBOL_CLASS (sym) != LOC_UNRESOLVED
 			       && SYMBOL_CLASS (sym) != LOC_BLOCK
@@ -4650,15 +4632,15 @@
 					== TYPE_CODE_ENUM))
 			       && (!treg.has_value ()
 				   || treg_matches_sym_type_name (*treg, sym)))
-			      || (kind == FUNCTIONS_DOMAIN
+			      || (m_kind == FUNCTIONS_DOMAIN
 				  && SYMBOL_CLASS (sym) == LOC_BLOCK
 				  && (!treg.has_value ()
 				      || treg_matches_sym_type_name (*treg,
 								     sym)))
-			      || (kind == TYPES_DOMAIN
+			      || (m_kind == TYPES_DOMAIN
 				  && SYMBOL_CLASS (sym) == LOC_TYPEDEF
 				  && SYMBOL_DOMAIN (sym) != MODULE_DOMAIN)
-			      || (kind == MODULES_DOMAIN
+			      || (m_kind == MODULES_DOMAIN
 				  && SYMBOL_DOMAIN (sym) == MODULE_DOMAIN
 				  && SYMBOL_LINE (sym) != 0))))
 		    {
@@ -4675,11 +4657,11 @@
 
   /* If there are no eyes, avoid all contact.  I mean, if there are
      no debug symbols, then add matching minsyms.  But if the user wants
-     to see symbols matching a type regexp, then never give a minimal symbol,
+     to see symbols matching a type m_symbol_namregexp, then never give a minimal symbol,
      as we assume that a minimal symbol does not have a type.  */
 
-  if ((found_misc || (nfiles == 0 && kind != FUNCTIONS_DOMAIN))
-      && !exclude_minsyms
+  if ((found_misc || (filenames.empty () && m_kind != FUNCTIONS_DOMAIN))
+      && !m_exclude_minsyms
       && !treg.has_value ())
     {
       for (objfile *objfile : current_program_space->objfiles ())
@@ -4702,7 +4684,7 @@
 		    {
 		      /* For functions we can do a quick check of whether the
 			 symbol might be found via find_pc_symtab.  */
-		      if (kind != FUNCTIONS_DOMAIN
+		      if (m_kind != FUNCTIONS_DOMAIN
 			  || (find_pc_compunit_symtab
 			      (MSYMBOL_VALUE_ADDRESS (objfile, msymbol))
 			      == NULL))
@@ -4843,10 +4825,10 @@
   if (regexp != nullptr && *regexp == '\0')
     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);
+  spec.set_symbol_type_regexp (t_regexp);
+  spec.set_exclude_minsyms (exclude_minsyms);
+  std::vector<symbol_search> symbols = spec.search ();
 
   if (!quiet)
     {
@@ -5085,11 +5067,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 +5085,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)
@@ -6349,17 +6326,17 @@
   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);
+  spec1.set_exclude_minsyms (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);
+  spec2.set_symbol_type_regexp (type_regexp);
+  spec2.set_exclude_minsyms (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 8f95a3a..43aa9a9 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -811,7 +811,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,68 @@
 				  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_name_regexp)
+    : m_kind (kind),
+      m_symbol_name_regexp (symbol_name_regexp)
+  {
+    /* The symbol searching is designed to only find one kind of thing.  */
+    gdb_assert (m_kind != ALL_DOMAIN);
+  }
+
+  /* Set the optional regexp that matches against the symbol type.  */
+  void set_symbol_type_regexp (const char *regexp)
+  {
+    m_symbol_type_regexp = regexp;
+  }
+
+  /* Set the flag to exclude minsyms from the search results.  */
+  void set_exclude_minsyms (bool exclude_minsyms)
+  {
+    m_exclude_minsyms = exclude_minsyms;
+  }
+
+  /* Search the symbols from all objfiles in the current program space
+     looking for matches as defined by the current state of this object.
+
+     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_name_regexp = nullptr;
+
+  /* Regular expression to match against the symbol type.  */
+  const char *m_symbol_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: 5
Gerrit-Owner: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-CC: Christian Biesinger <cbiesinger@google.com>
Gerrit-CC: Joel Brobecker <brobecker@adacore.com>
Gerrit-MessageType: newpatchset

[review v5] gdb: Introduce global_symbol_searcher

[Andrew Burgess (Code Review)]

Andrew Burgess has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/264
......................................................................


Patch Set 5:

All review issues now addressed.


-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I488ab292a892d9e9e84775c632c5f198b6ad3710
Gerrit-Change-Number: 264
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-CC: Christian Biesinger <cbiesinger@google.com>
Gerrit-CC: Joel Brobecker <brobecker@adacore.com>
Gerrit-Comment-Date: Fri, 22 Nov 2019 17:32:54 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

[review v5] gdb: Introduce global_symbol_searcher

[Simon Marchi (Code Review)]

Simon Marchi has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/264
......................................................................


Patch Set 5: Code-Review+2

(6 comments)

LGTM, I just noted a few small comments.

| --- gdb/symtab.c
| +++ gdb/symtab.c
| @@ -4457,16 +4437,18 @@ /* 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 ();

PS3, Line 4445:

Done

|    const struct blockvector *bv;
|    const struct block *b;
|    int i = 0;
|    struct block_iterator iter;
|    struct symbol *sym;
|    int found_misc = 0;
|    static const enum minimal_symbol_type types[]
|      = {mst_data, mst_text, mst_unknown};
|    static const enum minimal_symbol_type types2[]

 ...

| @@ -4539,16 +4521,19 @@ global_symbol_searcher::search () const
|      }
|  
|    /* Search through the partial symtabs *first* for all symbols
|       matching the regexp.  That way we don't have to reproduce all of
|       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.  */

PS3, Line 4530:

Done

| +			     return file_matches (filename, filenames,
| +						  basenames) ? 1 : 0;
|  			   },
|  			   lookup_name_info::match_any (),
|  			   [&] (const char *symname)
|  			   {
|  			     return (!preg.has_value ()
|  				     || preg->exec (symname,
|  						    0, NULL, 0) == 0);
| --- gdb/symtab.c
| +++ gdb/symtab.c
| @@ -4527,24 +4506,26 @@ global_symbol_searcher::search () const
|  	}
|  
|        int cflags = REG_NOSUB | (case_sensitivity == case_sensitive_off
|  				? REG_ICASE : 0);
| -      preg.emplace (regexp, cflags, _("Invalid regexp"));
| -    }
| -
| -  if (t_regexp != NULL)
| +      preg.emplace (symbol_name_regexp, cflags,
| +		    _("Invalid m_symbol_name_regexp"));

PS5, Line 4511:

These are user visible regexp, I think they should be user friendly
messages like

  "Invalid symbol name regexp"

| +    }
| +
| +  if (m_symbol_type_regexp != NULL)
|      {
|        int cflags = REG_NOSUB | (case_sensitivity == case_sensitive_off
|  				? REG_ICASE : 0);
| -      treg.emplace (t_regexp, cflags, _("Invalid regexp"));
| +      treg.emplace (m_symbol_type_regexp, cflags,
| +		    _("Invalid m_symbol_namregexp"));

PS5, Line 4519:

Same.

|      }
|  
|    /* Search through the partial symtabs *first* for all symbols
| -     matching the regexp.  That way we don't have to reproduce all of
| +     matching the m_symbol_namregexp.  That way we don't have to reproduce all of

PS5, Line 4523:

m_symbol_name_regexp

|       the machinery below.  */
|    expand_symtabs_matching ([&] (const char *filename, bool basenames)
|  			   {
| -			     return file_matches (filename, files, nfiles,
| +			     return file_matches (filename, filenames,
|  						  basenames);
|  			   },
|  			   lookup_name_info::match_any (),
|  			   [&] (const char *symname)

 ...

| @@ -4670,16 +4652,16 @@ global_symbol_searcher::search () const
|  	}
|      }
|  
|    if (!result.empty ())
|      sort_search_symbols_remove_dups (&result);
|  
|    /* If there are no eyes, avoid all contact.  I mean, if there are
|       no debug symbols, then add matching minsyms.  But if the user wants
| -     to see symbols matching a type regexp, then never give a minimal symbol,
| +     to see symbols matching a type m_symbol_namregexp, then never give a minimal symbol,

PS5, Line 4660:

That sounds not correct.

|       as we assume that a minimal symbol does not have a type.  */
|  
| -  if ((found_misc || (nfiles == 0 && kind != FUNCTIONS_DOMAIN))
| -      && !exclude_minsyms
| +  if ((found_misc || (filenames.empty () && m_kind != FUNCTIONS_DOMAIN))
| +      && !m_exclude_minsyms
|        && !treg.has_value ())
|      {
|        for (objfile *objfile : current_program_space->objfiles ())

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I488ab292a892d9e9e84775c632c5f198b6ad3710
Gerrit-Change-Number: 264
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-CC: Christian Biesinger <cbiesinger@google.com>
Gerrit-CC: Joel Brobecker <brobecker@adacore.com>
Gerrit-Comment-Date: Sat, 23 Nov 2019 13:17:16 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-MessageType: comment

[review v6] gdb: Introduce global_symbol_searcher

[Andrew Burgess (Code Review)]

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, update header comment.
	(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, 156 insertions(+), 123 deletions(-)



diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index fdba64e..b71f660 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,20 @@
+2019-11-26  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, update header comment.
+	(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-25  Tom de Vries  <tdevries@suse.de>
 
 	* contrib/words.sh: Add -c option.
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 7b561a1..d5b419b 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 2e8ae23..a758192 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -4344,27 +4344,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
+   true 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
@@ -4440,30 +4437,10 @@
 		 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 struct blockvector *bv;
   const struct block *b;
@@ -4487,21 +4464,23 @@
   gdb::optional<compiled_regex> preg;
   gdb::optional<compiled_regex> treg;
 
-  gdb_assert (kind != ALL_DOMAIN);
+  gdb_assert (m_kind != ALL_DOMAIN);
 
-  ourtype = types[kind];
-  ourtype2 = types2[kind];
-  ourtype3 = types3[kind];
-  ourtype4 = types4[kind];
+  ourtype = types[m_kind];
+  ourtype2 = types2[m_kind];
+  ourtype3 = types3[m_kind];
+  ourtype4 = types4[m_kind];
 
-  if (regexp != NULL)
+  if (m_symbol_name_regexp != NULL)
     {
+      const char *symbol_name_regexp = m_symbol_name_regexp;
+
       /* Make sure spacing is right for C++ operators.
          This is just a courtesy to make the matching less sensitive
          to how many spaces the user leaves between 'operator'
          and <TYPENAME> or <OPERATOR>.  */
       const char *opend;
-      const char *opname = operator_chars (regexp, &opend);
+      const char *opname = operator_chars (symbol_name_regexp, &opend);
 
       if (*opname)
 	{
@@ -4526,28 +4505,30 @@
 	      char *tmp = (char *) alloca (8 + fix + strlen (opname) + 1);
 
 	      sprintf (tmp, "operator%.*s%s", fix, " ", opname);
-	      regexp = tmp;
+	      symbol_name_regexp = tmp;
 	    }
 	}
 
       int cflags = REG_NOSUB | (case_sensitivity == case_sensitive_off
 				? REG_ICASE : 0);
-      preg.emplace (regexp, cflags, _("Invalid regexp"));
+      preg.emplace (symbol_name_regexp, cflags,
+		    _("Invalid m_symbol_name_regexp"));
     }
 
-  if (t_regexp != NULL)
+  if (m_symbol_type_regexp != NULL)
     {
       int cflags = REG_NOSUB | (case_sensitivity == case_sensitive_off
 				? REG_ICASE : 0);
-      treg.emplace (t_regexp, cflags, _("Invalid regexp"));
+      treg.emplace (m_symbol_type_regexp, cflags,
+		    _("Invalid m_symbol_namregexp"));
     }
 
   /* Search through the partial symtabs *first* for all symbols
-     matching the regexp.  That way we don't have to reproduce all of
+     matching the m_symbol_namregexp.  That way we don't have to reproduce all of
      the machinery below.  */
   expand_symtabs_matching ([&] (const char *filename, bool basenames)
 			   {
-			     return file_matches (filename, files, nfiles,
+			     return file_matches (filename, filenames,
 						  basenames);
 			   },
 			   lookup_name_info::match_any (),
@@ -4558,7 +4539,7 @@
 						    0, NULL, 0) == 0);
 			   },
 			   NULL,
-			   kind);
+			   m_kind);
 
   /* Here, we search through the minimal symbol tables for functions
      and variables that match, and force their symbols to be read.
@@ -4576,7 +4557,8 @@
      all objfiles.  In large programs (1000s of shared libs) searching all
      objfiles is not worth the pain.  */
 
-  if (nfiles == 0 && (kind == VARIABLES_DOMAIN || kind == FUNCTIONS_DOMAIN))
+  if (filenames.empty () && (m_kind == VARIABLES_DOMAIN
+			     || m_kind == FUNCTIONS_DOMAIN))
     {
       for (objfile *objfile : current_program_space->objfiles ())
 	{
@@ -4600,7 +4582,7 @@
 			 lookup functions is to expand the symbol
 			 table if msymbol is found, for the benefit of
 			 the next loop on compunits.  */
-		      if (kind == FUNCTIONS_DOMAIN
+		      if (m_kind == FUNCTIONS_DOMAIN
 			  ? (find_pc_compunit_symtab
 			     (MSYMBOL_VALUE_ADDRESS (objfile, msymbol))
 			     == NULL)
@@ -4631,16 +4613,16 @@
 		  /* 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 (sym->natural_name (), 0,
 					  NULL, 0) == 0)
-			  && ((kind == VARIABLES_DOMAIN
+			  && ((m_kind == VARIABLES_DOMAIN
 			       && SYMBOL_CLASS (sym) != LOC_TYPEDEF
 			       && SYMBOL_CLASS (sym) != LOC_UNRESOLVED
 			       && SYMBOL_CLASS (sym) != LOC_BLOCK
@@ -4653,15 +4635,15 @@
 					== TYPE_CODE_ENUM))
 			       && (!treg.has_value ()
 				   || treg_matches_sym_type_name (*treg, sym)))
-			      || (kind == FUNCTIONS_DOMAIN
+			      || (m_kind == FUNCTIONS_DOMAIN
 				  && SYMBOL_CLASS (sym) == LOC_BLOCK
 				  && (!treg.has_value ()
 				      || treg_matches_sym_type_name (*treg,
 								     sym)))
-			      || (kind == TYPES_DOMAIN
+			      || (m_kind == TYPES_DOMAIN
 				  && SYMBOL_CLASS (sym) == LOC_TYPEDEF
 				  && SYMBOL_DOMAIN (sym) != MODULE_DOMAIN)
-			      || (kind == MODULES_DOMAIN
+			      || (m_kind == MODULES_DOMAIN
 				  && SYMBOL_DOMAIN (sym) == MODULE_DOMAIN
 				  && SYMBOL_LINE (sym) != 0))))
 		    {
@@ -4678,11 +4660,11 @@
 
   /* If there are no eyes, avoid all contact.  I mean, if there are
      no debug symbols, then add matching minsyms.  But if the user wants
-     to see symbols matching a type regexp, then never give a minimal symbol,
+     to see symbols matching a type m_symbol_namregexp, then never give a minimal symbol,
      as we assume that a minimal symbol does not have a type.  */
 
-  if ((found_misc || (nfiles == 0 && kind != FUNCTIONS_DOMAIN))
-      && !exclude_minsyms
+  if ((found_misc || (filenames.empty () && m_kind != FUNCTIONS_DOMAIN))
+      && !m_exclude_minsyms
       && !treg.has_value ())
     {
       for (objfile *objfile : current_program_space->objfiles ())
@@ -4705,7 +4687,7 @@
 		    {
 		      /* For functions we can do a quick check of whether the
 			 symbol might be found via find_pc_symtab.  */
-		      if (kind != FUNCTIONS_DOMAIN
+		      if (m_kind != FUNCTIONS_DOMAIN
 			  || (find_pc_compunit_symtab
 			      (MSYMBOL_VALUE_ADDRESS (objfile, msymbol))
 			      == NULL))
@@ -4844,10 +4826,10 @@
   if (regexp != nullptr && *regexp == '\0')
     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);
+  spec.set_symbol_type_regexp (t_regexp);
+  spec.set_exclude_minsyms (exclude_minsyms);
+  std::vector<symbol_search> symbols = spec.search ();
 
   if (!quiet)
     {
@@ -5086,11 +5068,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, ':');
 
@@ -5106,17 +5086,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)
@@ -6350,17 +6327,17 @@
   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);
+  spec1.set_exclude_minsyms (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);
+  spec2.set_symbol_type_regexp (type_regexp);
+  spec2.set_exclude_minsyms (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 897ffda..105b93c 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -783,7 +783,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
@@ -1997,11 +1997,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_)
@@ -2050,12 +2048,68 @@
 				  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_name_regexp)
+    : m_kind (kind),
+      m_symbol_name_regexp (symbol_name_regexp)
+  {
+    /* The symbol searching is designed to only find one kind of thing.  */
+    gdb_assert (m_kind != ALL_DOMAIN);
+  }
+
+  /* Set the optional regexp that matches against the symbol type.  */
+  void set_symbol_type_regexp (const char *regexp)
+  {
+    m_symbol_type_regexp = regexp;
+  }
+
+  /* Set the flag to exclude minsyms from the search results.  */
+  void set_exclude_minsyms (bool exclude_minsyms)
+  {
+    m_exclude_minsyms = exclude_minsyms;
+  }
+
+  /* Search the symbols from all objfiles in the current program space
+     looking for matches as defined by the current state of this object.
+
+     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_name_regexp = nullptr;
+
+  /* Regular expression to match against the symbol type.  */
+  const char *m_symbol_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: 6
Gerrit-Owner: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-CC: Christian Biesinger <cbiesinger@google.com>
Gerrit-CC: Joel Brobecker <brobecker@adacore.com>
Gerrit-MessageType: newpatchset

[review v7] gdb: Introduce global_symbol_searcher

[Andrew Burgess (Code Review)]

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, update header comment.
	(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, 161 insertions(+), 128 deletions(-)



diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index fdba64e..b71f660 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,20 @@
+2019-11-26  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, update header comment.
+	(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-25  Tom de Vries  <tdevries@suse.de>
 
 	* contrib/words.sh: Add -c option.
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 7b561a1..d5b419b 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 2e8ae23..197128b 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -4344,27 +4344,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
+   true 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
@@ -4440,30 +4437,10 @@
 		 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 struct blockvector *bv;
   const struct block *b;
@@ -4487,21 +4464,23 @@
   gdb::optional<compiled_regex> preg;
   gdb::optional<compiled_regex> treg;
 
-  gdb_assert (kind != ALL_DOMAIN);
+  gdb_assert (m_kind != ALL_DOMAIN);
 
-  ourtype = types[kind];
-  ourtype2 = types2[kind];
-  ourtype3 = types3[kind];
-  ourtype4 = types4[kind];
+  ourtype = types[m_kind];
+  ourtype2 = types2[m_kind];
+  ourtype3 = types3[m_kind];
+  ourtype4 = types4[m_kind];
 
-  if (regexp != NULL)
+  if (m_symbol_name_regexp != NULL)
     {
+      const char *symbol_name_regexp = m_symbol_name_regexp;
+
       /* Make sure spacing is right for C++ operators.
          This is just a courtesy to make the matching less sensitive
          to how many spaces the user leaves between 'operator'
          and <TYPENAME> or <OPERATOR>.  */
       const char *opend;
-      const char *opname = operator_chars (regexp, &opend);
+      const char *opname = operator_chars (symbol_name_regexp, &opend);
 
       if (*opname)
 	{
@@ -4526,28 +4505,30 @@
 	      char *tmp = (char *) alloca (8 + fix + strlen (opname) + 1);
 
 	      sprintf (tmp, "operator%.*s%s", fix, " ", opname);
-	      regexp = tmp;
+	      symbol_name_regexp = tmp;
 	    }
 	}
 
       int cflags = REG_NOSUB | (case_sensitivity == case_sensitive_off
 				? REG_ICASE : 0);
-      preg.emplace (regexp, cflags, _("Invalid regexp"));
+      preg.emplace (symbol_name_regexp, cflags,
+		    _("Invalid regexp"));
     }
 
-  if (t_regexp != NULL)
+  if (m_symbol_type_regexp != NULL)
     {
       int cflags = REG_NOSUB | (case_sensitivity == case_sensitive_off
 				? REG_ICASE : 0);
-      treg.emplace (t_regexp, cflags, _("Invalid regexp"));
+      treg.emplace (m_symbol_type_regexp, cflags,
+		    _("Invalid regexp"));
     }
 
-  /* Search through the partial symtabs *first* for all symbols
-     matching the regexp.  That way we don't have to reproduce all of
-     the machinery below.  */
+  /* Search through the partial symtabs *first* for all symbols matching
+     the m_symbol_name_regexp (in preg).  That way we don't have to
+     reproduce all of the machinery below.  */
   expand_symtabs_matching ([&] (const char *filename, bool basenames)
 			   {
-			     return file_matches (filename, files, nfiles,
+			     return file_matches (filename, filenames,
 						  basenames);
 			   },
 			   lookup_name_info::match_any (),
@@ -4558,7 +4539,7 @@
 						    0, NULL, 0) == 0);
 			   },
 			   NULL,
-			   kind);
+			   m_kind);
 
   /* Here, we search through the minimal symbol tables for functions
      and variables that match, and force their symbols to be read.
@@ -4576,7 +4557,8 @@
      all objfiles.  In large programs (1000s of shared libs) searching all
      objfiles is not worth the pain.  */
 
-  if (nfiles == 0 && (kind == VARIABLES_DOMAIN || kind == FUNCTIONS_DOMAIN))
+  if (filenames.empty () && (m_kind == VARIABLES_DOMAIN
+			     || m_kind == FUNCTIONS_DOMAIN))
     {
       for (objfile *objfile : current_program_space->objfiles ())
 	{
@@ -4600,7 +4582,7 @@
 			 lookup functions is to expand the symbol
 			 table if msymbol is found, for the benefit of
 			 the next loop on compunits.  */
-		      if (kind == FUNCTIONS_DOMAIN
+		      if (m_kind == FUNCTIONS_DOMAIN
 			  ? (find_pc_compunit_symtab
 			     (MSYMBOL_VALUE_ADDRESS (objfile, msymbol))
 			     == NULL)
@@ -4631,16 +4613,16 @@
 		  /* 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 (sym->natural_name (), 0,
 					  NULL, 0) == 0)
-			  && ((kind == VARIABLES_DOMAIN
+			  && ((m_kind == VARIABLES_DOMAIN
 			       && SYMBOL_CLASS (sym) != LOC_TYPEDEF
 			       && SYMBOL_CLASS (sym) != LOC_UNRESOLVED
 			       && SYMBOL_CLASS (sym) != LOC_BLOCK
@@ -4653,15 +4635,15 @@
 					== TYPE_CODE_ENUM))
 			       && (!treg.has_value ()
 				   || treg_matches_sym_type_name (*treg, sym)))
-			      || (kind == FUNCTIONS_DOMAIN
+			      || (m_kind == FUNCTIONS_DOMAIN
 				  && SYMBOL_CLASS (sym) == LOC_BLOCK
 				  && (!treg.has_value ()
 				      || treg_matches_sym_type_name (*treg,
 								     sym)))
-			      || (kind == TYPES_DOMAIN
+			      || (m_kind == TYPES_DOMAIN
 				  && SYMBOL_CLASS (sym) == LOC_TYPEDEF
 				  && SYMBOL_DOMAIN (sym) != MODULE_DOMAIN)
-			      || (kind == MODULES_DOMAIN
+			      || (m_kind == MODULES_DOMAIN
 				  && SYMBOL_DOMAIN (sym) == MODULE_DOMAIN
 				  && SYMBOL_LINE (sym) != 0))))
 		    {
@@ -4676,13 +4658,13 @@
   if (!result.empty ())
     sort_search_symbols_remove_dups (&result);
 
-  /* If there are no eyes, avoid all contact.  I mean, if there are
-     no debug symbols, then add matching minsyms.  But if the user wants
-     to see symbols matching a type regexp, then never give a minimal symbol,
-     as we assume that a minimal symbol does not have a type.  */
+  /* If there are no debug symbols, then add matching minsyms.  But if the
+     user wants to see symbols matching a type m_symbol_type_regexp, then
+     never give a minimal symbol, as we assume that a minimal symbol does
+     not have a type.  */
 
-  if ((found_misc || (nfiles == 0 && kind != FUNCTIONS_DOMAIN))
-      && !exclude_minsyms
+  if ((found_misc || (filenames.empty () && m_kind != FUNCTIONS_DOMAIN))
+      && !m_exclude_minsyms
       && !treg.has_value ())
     {
       for (objfile *objfile : current_program_space->objfiles ())
@@ -4705,7 +4687,7 @@
 		    {
 		      /* For functions we can do a quick check of whether the
 			 symbol might be found via find_pc_symtab.  */
-		      if (kind != FUNCTIONS_DOMAIN
+		      if (m_kind != FUNCTIONS_DOMAIN
 			  || (find_pc_compunit_symtab
 			      (MSYMBOL_VALUE_ADDRESS (objfile, msymbol))
 			      == NULL))
@@ -4844,10 +4826,10 @@
   if (regexp != nullptr && *regexp == '\0')
     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);
+  spec.set_symbol_type_regexp (t_regexp);
+  spec.set_exclude_minsyms (exclude_minsyms);
+  std::vector<symbol_search> symbols = spec.search ();
 
   if (!quiet)
     {
@@ -5086,11 +5068,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, ':');
 
@@ -5106,17 +5086,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)
@@ -6350,17 +6327,17 @@
   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);
+  spec1.set_exclude_minsyms (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);
+  spec2.set_symbol_type_regexp (type_regexp);
+  spec2.set_exclude_minsyms (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 897ffda..105b93c 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -783,7 +783,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
@@ -1997,11 +1997,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_)
@@ -2050,12 +2048,68 @@
 				  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_name_regexp)
+    : m_kind (kind),
+      m_symbol_name_regexp (symbol_name_regexp)
+  {
+    /* The symbol searching is designed to only find one kind of thing.  */
+    gdb_assert (m_kind != ALL_DOMAIN);
+  }
+
+  /* Set the optional regexp that matches against the symbol type.  */
+  void set_symbol_type_regexp (const char *regexp)
+  {
+    m_symbol_type_regexp = regexp;
+  }
+
+  /* Set the flag to exclude minsyms from the search results.  */
+  void set_exclude_minsyms (bool exclude_minsyms)
+  {
+    m_exclude_minsyms = exclude_minsyms;
+  }
+
+  /* Search the symbols from all objfiles in the current program space
+     looking for matches as defined by the current state of this object.
+
+     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_name_regexp = nullptr;
+
+  /* Regular expression to match against the symbol type.  */
+  const char *m_symbol_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: 7
Gerrit-Owner: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-CC: Christian Biesinger <cbiesinger@google.com>
Gerrit-CC: Joel Brobecker <brobecker@adacore.com>
Gerrit-MessageType: newpatchset

[review v7] gdb: Introduce global_symbol_searcher

[Andrew Burgess (Code Review)]

Andrew Burgess has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/264
......................................................................


Patch Set 7:

I think all of the issues pointed out are fixed in the latest version.  I've not merged this yet as a later patch in the series still needs additional review first.  I don't believe I've made any significant changes between v5 and v7 other than a rebase and the minor fixes requested.


-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I488ab292a892d9e9e84775c632c5f198b6ad3710
Gerrit-Change-Number: 264
Gerrit-PatchSet: 7
Gerrit-Owner: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-CC: Christian Biesinger <cbiesinger@google.com>
Gerrit-CC: Joel Brobecker <brobecker@adacore.com>
Gerrit-Comment-Date: Tue, 26 Nov 2019 23:43:31 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

[review v7] gdb: Introduce global_symbol_searcher

[Simon Marchi (Code Review)]

Simon Marchi has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/264
......................................................................


Patch Set 7: Code-Review+2


-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I488ab292a892d9e9e84775c632c5f198b6ad3710
Gerrit-Change-Number: 264
Gerrit-PatchSet: 7
Gerrit-Owner: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-CC: Christian Biesinger <cbiesinger@google.com>
Gerrit-CC: Joel Brobecker <brobecker@adacore.com>
Gerrit-Comment-Date: Wed, 27 Nov 2019 03:58:52 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[pushed] gdb: Introduce global_symbol_searcher

[Sourceware to Gerrit sync (Code Review)]

Sourceware to Gerrit sync has submitted this change.

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, update header comment.
	(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, 161 insertions(+), 128 deletions(-)


diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 6c6c000..56536c4 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,20 @@
+2019-11-27  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, update header comment.
+	(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-26  Tom Tromey  <tromey@adacore.com>
 
 	* cp-support.c (_initialize_cp_support): Conditionally initialize
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 2f4fb0f..fad54e9 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -644,19 +644,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;
@@ -666,7 +653,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,
@@ -683,6 +669,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
@@ -728,20 +720,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 b5c8109..e5ed42a 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -4347,27 +4347,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
+   true 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
@@ -4443,30 +4440,10 @@
 		 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 struct blockvector *bv;
   const struct block *b;
@@ -4490,21 +4467,23 @@
   gdb::optional<compiled_regex> preg;
   gdb::optional<compiled_regex> treg;
 
-  gdb_assert (kind != ALL_DOMAIN);
+  gdb_assert (m_kind != ALL_DOMAIN);
 
-  ourtype = types[kind];
-  ourtype2 = types2[kind];
-  ourtype3 = types3[kind];
-  ourtype4 = types4[kind];
+  ourtype = types[m_kind];
+  ourtype2 = types2[m_kind];
+  ourtype3 = types3[m_kind];
+  ourtype4 = types4[m_kind];
 
-  if (regexp != NULL)
+  if (m_symbol_name_regexp != NULL)
     {
+      const char *symbol_name_regexp = m_symbol_name_regexp;
+
       /* Make sure spacing is right for C++ operators.
          This is just a courtesy to make the matching less sensitive
          to how many spaces the user leaves between 'operator'
          and <TYPENAME> or <OPERATOR>.  */
       const char *opend;
-      const char *opname = operator_chars (regexp, &opend);
+      const char *opname = operator_chars (symbol_name_regexp, &opend);
 
       if (*opname)
 	{
@@ -4529,28 +4508,30 @@
 	      char *tmp = (char *) alloca (8 + fix + strlen (opname) + 1);
 
 	      sprintf (tmp, "operator%.*s%s", fix, " ", opname);
-	      regexp = tmp;
+	      symbol_name_regexp = tmp;
 	    }
 	}
 
       int cflags = REG_NOSUB | (case_sensitivity == case_sensitive_off
 				? REG_ICASE : 0);
-      preg.emplace (regexp, cflags, _("Invalid regexp"));
+      preg.emplace (symbol_name_regexp, cflags,
+		    _("Invalid regexp"));
     }
 
-  if (t_regexp != NULL)
+  if (m_symbol_type_regexp != NULL)
     {
       int cflags = REG_NOSUB | (case_sensitivity == case_sensitive_off
 				? REG_ICASE : 0);
-      treg.emplace (t_regexp, cflags, _("Invalid regexp"));
+      treg.emplace (m_symbol_type_regexp, cflags,
+		    _("Invalid regexp"));
     }
 
-  /* Search through the partial symtabs *first* for all symbols
-     matching the regexp.  That way we don't have to reproduce all of
-     the machinery below.  */
+  /* Search through the partial symtabs *first* for all symbols matching
+     the m_symbol_name_regexp (in preg).  That way we don't have to
+     reproduce all of the machinery below.  */
   expand_symtabs_matching ([&] (const char *filename, bool basenames)
 			   {
-			     return file_matches (filename, files, nfiles,
+			     return file_matches (filename, filenames,
 						  basenames);
 			   },
 			   lookup_name_info::match_any (),
@@ -4561,7 +4542,7 @@
 						    0, NULL, 0) == 0);
 			   },
 			   NULL,
-			   kind);
+			   m_kind);
 
   /* Here, we search through the minimal symbol tables for functions
      and variables that match, and force their symbols to be read.
@@ -4579,7 +4560,8 @@
      all objfiles.  In large programs (1000s of shared libs) searching all
      objfiles is not worth the pain.  */
 
-  if (nfiles == 0 && (kind == VARIABLES_DOMAIN || kind == FUNCTIONS_DOMAIN))
+  if (filenames.empty () && (m_kind == VARIABLES_DOMAIN
+			     || m_kind == FUNCTIONS_DOMAIN))
     {
       for (objfile *objfile : current_program_space->objfiles ())
 	{
@@ -4603,7 +4585,7 @@
 			 lookup functions is to expand the symbol
 			 table if msymbol is found, for the benefit of
 			 the next loop on compunits.  */
-		      if (kind == FUNCTIONS_DOMAIN
+		      if (m_kind == FUNCTIONS_DOMAIN
 			  ? (find_pc_compunit_symtab
 			     (MSYMBOL_VALUE_ADDRESS (objfile, msymbol))
 			     == NULL)
@@ -4634,16 +4616,16 @@
 		  /* 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 (sym->natural_name (), 0,
 					  NULL, 0) == 0)
-			  && ((kind == VARIABLES_DOMAIN
+			  && ((m_kind == VARIABLES_DOMAIN
 			       && SYMBOL_CLASS (sym) != LOC_TYPEDEF
 			       && SYMBOL_CLASS (sym) != LOC_UNRESOLVED
 			       && SYMBOL_CLASS (sym) != LOC_BLOCK
@@ -4656,15 +4638,15 @@
 					== TYPE_CODE_ENUM))
 			       && (!treg.has_value ()
 				   || treg_matches_sym_type_name (*treg, sym)))
-			      || (kind == FUNCTIONS_DOMAIN
+			      || (m_kind == FUNCTIONS_DOMAIN
 				  && SYMBOL_CLASS (sym) == LOC_BLOCK
 				  && (!treg.has_value ()
 				      || treg_matches_sym_type_name (*treg,
 								     sym)))
-			      || (kind == TYPES_DOMAIN
+			      || (m_kind == TYPES_DOMAIN
 				  && SYMBOL_CLASS (sym) == LOC_TYPEDEF
 				  && SYMBOL_DOMAIN (sym) != MODULE_DOMAIN)
-			      || (kind == MODULES_DOMAIN
+			      || (m_kind == MODULES_DOMAIN
 				  && SYMBOL_DOMAIN (sym) == MODULE_DOMAIN
 				  && SYMBOL_LINE (sym) != 0))))
 		    {
@@ -4679,13 +4661,13 @@
   if (!result.empty ())
     sort_search_symbols_remove_dups (&result);
 
-  /* If there are no eyes, avoid all contact.  I mean, if there are
-     no debug symbols, then add matching minsyms.  But if the user wants
-     to see symbols matching a type regexp, then never give a minimal symbol,
-     as we assume that a minimal symbol does not have a type.  */
+  /* If there are no debug symbols, then add matching minsyms.  But if the
+     user wants to see symbols matching a type m_symbol_type_regexp, then
+     never give a minimal symbol, as we assume that a minimal symbol does
+     not have a type.  */
 
-  if ((found_misc || (nfiles == 0 && kind != FUNCTIONS_DOMAIN))
-      && !exclude_minsyms
+  if ((found_misc || (filenames.empty () && m_kind != FUNCTIONS_DOMAIN))
+      && !m_exclude_minsyms
       && !treg.has_value ())
     {
       for (objfile *objfile : current_program_space->objfiles ())
@@ -4708,7 +4690,7 @@
 		    {
 		      /* For functions we can do a quick check of whether the
 			 symbol might be found via find_pc_symtab.  */
-		      if (kind != FUNCTIONS_DOMAIN
+		      if (m_kind != FUNCTIONS_DOMAIN
 			  || (find_pc_compunit_symtab
 			      (MSYMBOL_VALUE_ADDRESS (objfile, msymbol))
 			      == NULL))
@@ -4847,10 +4829,10 @@
   if (regexp != nullptr && *regexp == '\0')
     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);
+  spec.set_symbol_type_regexp (t_regexp);
+  spec.set_exclude_minsyms (exclude_minsyms);
+  std::vector<symbol_search> symbols = spec.search ();
 
   if (!quiet)
     {
@@ -5081,11 +5063,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, ':');
 
@@ -5101,17 +5081,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)
@@ -6345,17 +6322,17 @@
   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);
+  spec1.set_exclude_minsyms (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);
+  spec2.set_symbol_type_regexp (type_regexp);
+  spec2.set_exclude_minsyms (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 9c2aea7..680c334 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -797,7 +797,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
@@ -2011,11 +2011,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_)
@@ -2064,12 +2062,68 @@
 				  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_name_regexp)
+    : m_kind (kind),
+      m_symbol_name_regexp (symbol_name_regexp)
+  {
+    /* The symbol searching is designed to only find one kind of thing.  */
+    gdb_assert (m_kind != ALL_DOMAIN);
+  }
+
+  /* Set the optional regexp that matches against the symbol type.  */
+  void set_symbol_type_regexp (const char *regexp)
+  {
+    m_symbol_type_regexp = regexp;
+  }
+
+  /* Set the flag to exclude minsyms from the search results.  */
+  void set_exclude_minsyms (bool exclude_minsyms)
+  {
+    m_exclude_minsyms = exclude_minsyms;
+  }
+
+  /* Search the symbols from all objfiles in the current program space
+     looking for matches as defined by the current state of this object.
+
+     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_name_regexp = nullptr;
+
+  /* Regular expression to match against the symbol type.  */
+  const char *m_symbol_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: 8
Gerrit-Owner: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-CC: Christian Biesinger <cbiesinger@google.com>
Gerrit-CC: Joel Brobecker <brobecker@adacore.com>
Gerrit-MessageType: merged

[pushed] gdb: Introduce global_symbol_searcher

[Sourceware to Gerrit sync (Code Review)]

The original change was created by Andrew Burgess.

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, update header comment.
	(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, 161 insertions(+), 128 deletions(-)



diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 6c6c000..56536c4 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,20 @@
+2019-11-27  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, update header comment.
+	(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-26  Tom Tromey  <tromey@adacore.com>
 
 	* cp-support.c (_initialize_cp_support): Conditionally initialize
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 2f4fb0f..fad54e9 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -644,19 +644,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;
@@ -666,7 +653,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,
@@ -683,6 +669,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
@@ -728,20 +720,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 b5c8109..e5ed42a 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -4347,27 +4347,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
+   true 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
@@ -4443,30 +4440,10 @@
 		 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 struct blockvector *bv;
   const struct block *b;
@@ -4490,21 +4467,23 @@
   gdb::optional<compiled_regex> preg;
   gdb::optional<compiled_regex> treg;
 
-  gdb_assert (kind != ALL_DOMAIN);
+  gdb_assert (m_kind != ALL_DOMAIN);
 
-  ourtype = types[kind];
-  ourtype2 = types2[kind];
-  ourtype3 = types3[kind];
-  ourtype4 = types4[kind];
+  ourtype = types[m_kind];
+  ourtype2 = types2[m_kind];
+  ourtype3 = types3[m_kind];
+  ourtype4 = types4[m_kind];
 
-  if (regexp != NULL)
+  if (m_symbol_name_regexp != NULL)
     {
+      const char *symbol_name_regexp = m_symbol_name_regexp;
+
       /* Make sure spacing is right for C++ operators.
          This is just a courtesy to make the matching less sensitive
          to how many spaces the user leaves between 'operator'
          and <TYPENAME> or <OPERATOR>.  */
       const char *opend;
-      const char *opname = operator_chars (regexp, &opend);
+      const char *opname = operator_chars (symbol_name_regexp, &opend);
 
       if (*opname)
 	{
@@ -4529,28 +4508,30 @@
 	      char *tmp = (char *) alloca (8 + fix + strlen (opname) + 1);
 
 	      sprintf (tmp, "operator%.*s%s", fix, " ", opname);
-	      regexp = tmp;
+	      symbol_name_regexp = tmp;
 	    }
 	}
 
       int cflags = REG_NOSUB | (case_sensitivity == case_sensitive_off
 				? REG_ICASE : 0);
-      preg.emplace (regexp, cflags, _("Invalid regexp"));
+      preg.emplace (symbol_name_regexp, cflags,
+		    _("Invalid regexp"));
     }
 
-  if (t_regexp != NULL)
+  if (m_symbol_type_regexp != NULL)
     {
       int cflags = REG_NOSUB | (case_sensitivity == case_sensitive_off
 				? REG_ICASE : 0);
-      treg.emplace (t_regexp, cflags, _("Invalid regexp"));
+      treg.emplace (m_symbol_type_regexp, cflags,
+		    _("Invalid regexp"));
     }
 
-  /* Search through the partial symtabs *first* for all symbols
-     matching the regexp.  That way we don't have to reproduce all of
-     the machinery below.  */
+  /* Search through the partial symtabs *first* for all symbols matching
+     the m_symbol_name_regexp (in preg).  That way we don't have to
+     reproduce all of the machinery below.  */
   expand_symtabs_matching ([&] (const char *filename, bool basenames)
 			   {
-			     return file_matches (filename, files, nfiles,
+			     return file_matches (filename, filenames,
 						  basenames);
 			   },
 			   lookup_name_info::match_any (),
@@ -4561,7 +4542,7 @@
 						    0, NULL, 0) == 0);
 			   },
 			   NULL,
-			   kind);
+			   m_kind);
 
   /* Here, we search through the minimal symbol tables for functions
      and variables that match, and force their symbols to be read.
@@ -4579,7 +4560,8 @@
      all objfiles.  In large programs (1000s of shared libs) searching all
      objfiles is not worth the pain.  */
 
-  if (nfiles == 0 && (kind == VARIABLES_DOMAIN || kind == FUNCTIONS_DOMAIN))
+  if (filenames.empty () && (m_kind == VARIABLES_DOMAIN
+			     || m_kind == FUNCTIONS_DOMAIN))
     {
       for (objfile *objfile : current_program_space->objfiles ())
 	{
@@ -4603,7 +4585,7 @@
 			 lookup functions is to expand the symbol
 			 table if msymbol is found, for the benefit of
 			 the next loop on compunits.  */
-		      if (kind == FUNCTIONS_DOMAIN
+		      if (m_kind == FUNCTIONS_DOMAIN
 			  ? (find_pc_compunit_symtab
 			     (MSYMBOL_VALUE_ADDRESS (objfile, msymbol))
 			     == NULL)
@@ -4634,16 +4616,16 @@
 		  /* 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 (sym->natural_name (), 0,
 					  NULL, 0) == 0)
-			  && ((kind == VARIABLES_DOMAIN
+			  && ((m_kind == VARIABLES_DOMAIN
 			       && SYMBOL_CLASS (sym) != LOC_TYPEDEF
 			       && SYMBOL_CLASS (sym) != LOC_UNRESOLVED
 			       && SYMBOL_CLASS (sym) != LOC_BLOCK
@@ -4656,15 +4638,15 @@
 					== TYPE_CODE_ENUM))
 			       && (!treg.has_value ()
 				   || treg_matches_sym_type_name (*treg, sym)))
-			      || (kind == FUNCTIONS_DOMAIN
+			      || (m_kind == FUNCTIONS_DOMAIN
 				  && SYMBOL_CLASS (sym) == LOC_BLOCK
 				  && (!treg.has_value ()
 				      || treg_matches_sym_type_name (*treg,
 								     sym)))
-			      || (kind == TYPES_DOMAIN
+			      || (m_kind == TYPES_DOMAIN
 				  && SYMBOL_CLASS (sym) == LOC_TYPEDEF
 				  && SYMBOL_DOMAIN (sym) != MODULE_DOMAIN)
-			      || (kind == MODULES_DOMAIN
+			      || (m_kind == MODULES_DOMAIN
 				  && SYMBOL_DOMAIN (sym) == MODULE_DOMAIN
 				  && SYMBOL_LINE (sym) != 0))))
 		    {
@@ -4679,13 +4661,13 @@
   if (!result.empty ())
     sort_search_symbols_remove_dups (&result);
 
-  /* If there are no eyes, avoid all contact.  I mean, if there are
-     no debug symbols, then add matching minsyms.  But if the user wants
-     to see symbols matching a type regexp, then never give a minimal symbol,
-     as we assume that a minimal symbol does not have a type.  */
+  /* If there are no debug symbols, then add matching minsyms.  But if the
+     user wants to see symbols matching a type m_symbol_type_regexp, then
+     never give a minimal symbol, as we assume that a minimal symbol does
+     not have a type.  */
 
-  if ((found_misc || (nfiles == 0 && kind != FUNCTIONS_DOMAIN))
-      && !exclude_minsyms
+  if ((found_misc || (filenames.empty () && m_kind != FUNCTIONS_DOMAIN))
+      && !m_exclude_minsyms
       && !treg.has_value ())
     {
       for (objfile *objfile : current_program_space->objfiles ())
@@ -4708,7 +4690,7 @@
 		    {
 		      /* For functions we can do a quick check of whether the
 			 symbol might be found via find_pc_symtab.  */
-		      if (kind != FUNCTIONS_DOMAIN
+		      if (m_kind != FUNCTIONS_DOMAIN
 			  || (find_pc_compunit_symtab
 			      (MSYMBOL_VALUE_ADDRESS (objfile, msymbol))
 			      == NULL))
@@ -4847,10 +4829,10 @@
   if (regexp != nullptr && *regexp == '\0')
     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);
+  spec.set_symbol_type_regexp (t_regexp);
+  spec.set_exclude_minsyms (exclude_minsyms);
+  std::vector<symbol_search> symbols = spec.search ();
 
   if (!quiet)
     {
@@ -5081,11 +5063,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, ':');
 
@@ -5101,17 +5081,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)
@@ -6345,17 +6322,17 @@
   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);
+  spec1.set_exclude_minsyms (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);
+  spec2.set_symbol_type_regexp (type_regexp);
+  spec2.set_exclude_minsyms (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 9c2aea7..680c334 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -797,7 +797,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
@@ -2011,11 +2011,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_)
@@ -2064,12 +2062,68 @@
 				  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_name_regexp)
+    : m_kind (kind),
+      m_symbol_name_regexp (symbol_name_regexp)
+  {
+    /* The symbol searching is designed to only find one kind of thing.  */
+    gdb_assert (m_kind != ALL_DOMAIN);
+  }
+
+  /* Set the optional regexp that matches against the symbol type.  */
+  void set_symbol_type_regexp (const char *regexp)
+  {
+    m_symbol_type_regexp = regexp;
+  }
+
+  /* Set the flag to exclude minsyms from the search results.  */
+  void set_exclude_minsyms (bool exclude_minsyms)
+  {
+    m_exclude_minsyms = exclude_minsyms;
+  }
+
+  /* Search the symbols from all objfiles in the current program space
+     looking for matches as defined by the current state of this object.
+
+     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_name_regexp = nullptr;
+
+  /* Regular expression to match against the symbol type.  */
+  const char *m_symbol_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: 8
Gerrit-Owner: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-CC: Christian Biesinger <cbiesinger@google.com>
Gerrit-CC: Joel Brobecker <brobecker@adacore.com>
Gerrit-MessageType: newpatchset