[review] gdb: Split global symbol search into separate functions

Message ID gerrit.1575314132000.I06b26920f35c268f7a38d8203dc2c2813aa501c6@gnutoolchain-gerrit.osci.io
State Superseded
Headers show
Series
  • [review] gdb: Split global symbol search into separate functions
Related show

Commit Message

Tankut Baris Aktemur (Code Review) Dec. 2, 2019, 7:15 p.m.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/738
......................................................................

gdb: Split global symbol search into separate functions

In preparation for the next commit, this commit restructures the code
by splitting global_symbol_searcher::search into separate functions.
There should be no functional changes after this commit.

gdb/ChangeLog:

	* symtab.c (symbol_search::compare_search_syms): Update header
	comment.
	(global_symbol_searcher::is_suitable_msymbol): New function.
	(global_symbol_searcher::expand_symtabs): New function.
	(global_symbol_searcher::add_matching_symbols): New function.
	(global_symbol_searcher::add_matching_msymbols): New function.
	(global_symbol_searcher::search): Move most of the content
	into the new functions above, and call them as needed.
	* symtab.h: Add 'set' include.
	(global_symbol_searcher) <expand_symtabs>: New member function.
	(global_symbol_searcher) <add_matching_symbols>: New member
	function.
	(global_symbol_searcher) <add_matching_msymbols>: New member
	function.
	(global_symbol_searcher) <is_suitable_msymbol>: New member
	function.

Change-Id: I06b26920f35c268f7a38d8203dc2c2813aa501c6
---
M gdb/ChangeLog
M gdb/symtab.c
M gdb/symtab.h
3 files changed, 275 insertions(+), 203 deletions(-)




-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I06b26920f35c268f7a38d8203dc2c2813aa501c6
Gerrit-Change-Number: 738
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-MessageType: newchange

Comments

Tankut Baris Aktemur (Code Review) Dec. 2, 2019, 7:42 p.m. | #1
Simon Marchi has posted comments on this change.

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


Patch Set 1:

(1 comment)

| --- gdb/symtab.c
| +++ gdb/symtab.c
| @@ -4597,17 +4714,4 @@ global_symbol_searcher::search () const
| -			     .symbol == NULL))
| -			found_misc = 1;
| -		    }
| -		}
| -	    }
| -	}
| -    }
| -
| +  bool found_msymbol = false;
| +  std::set<symbol_search> result_set;

PS1, Line 4715:

Maybe a bit pedantic, but introducing the set in this patch does
possibly change the behavior, doesn't it?  Wouldn't it be better to
keep appending to a vector, as before, and then replace the vector
with a set in the following patch, with a justification of why that
change is needed?

|    for (objfile *objfile : current_program_space->objfiles ())
|      {
| -      for (compunit_symtab *cust : objfile->compunits ())
| -	{
| -	  bv = COMPUNIT_BLOCKVECTOR (cust);
| -	  for (i = GLOBAL_BLOCK; i <= STATIC_BLOCK; i++)
| -	    {
| -	      b = BLOCKVECTOR_BLOCK (bv, i);
| -	      ALL_BLOCK_SYMBOLS (b, iter, sym)

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I06b26920f35c268f7a38d8203dc2c2813aa501c6
Gerrit-Change-Number: 738
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-CC: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Comment-Date: Mon, 02 Dec 2019 19:42:16 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Tankut Baris Aktemur (Code Review) Dec. 2, 2019, 11:44 p.m. | #2
Andrew Burgess has posted comments on this change.

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


Patch Set 2:

(1 comment)

| --- gdb/symtab.c
| +++ gdb/symtab.c
| @@ -4597,17 +4714,4 @@ global_symbol_searcher::search () const
| -			     .symbol == NULL))
| -			found_misc = 1;
| -		    }
| -		}
| -	    }
| -	}
| -    }
| -
| +  bool found_msymbol = false;
| +  std::set<symbol_search> result_set;

PS1, Line 4715:

Done.  I've extended a comment to explain why we now use a std::set.

|    for (objfile *objfile : current_program_space->objfiles ())
|      {
| -      for (compunit_symtab *cust : objfile->compunits ())
| -	{
| -	  bv = COMPUNIT_BLOCKVECTOR (cust);
| -	  for (i = GLOBAL_BLOCK; i <= STATIC_BLOCK; i++)
| -	    {
| -	      b = BLOCKVECTOR_BLOCK (bv, i);
| -	      ALL_BLOCK_SYMBOLS (b, iter, sym)

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I06b26920f35c268f7a38d8203dc2c2813aa501c6
Gerrit-Change-Number: 738
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-CC: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Comment-Date: Mon, 02 Dec 2019 23:44:40 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-MessageType: comment
Tankut Baris Aktemur (Code Review) Dec. 3, 2019, 2:18 a.m. | #3
Simon Marchi has posted comments on this change.

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


Patch Set 2:

(1 comment)

I left one comment, but otherwise LGTM.

| --- gdb/symtab.c
| +++ gdb/symtab.c
| @@ -4435,0 +4591,19 @@ global_symbol_searcher::add_matching_symbols
| +				  || treg_matches_sym_type_name (*treg,
| +								 sym)))
| +			  || (kind == TYPES_DOMAIN
| +			      && SYMBOL_CLASS (sym) == LOC_TYPEDEF
| +			      && SYMBOL_DOMAIN (sym) != MODULE_DOMAIN)
| +			  || (kind == MODULES_DOMAIN
| +			      && SYMBOL_DOMAIN (sym) == MODULE_DOMAIN
| +			      && SYMBOL_LINE (sym) != 0))))
| +		{
| +		  /* Match, insert if not already in the results.  */

PS2, Line 4600:

This part is not accurate for this patch, should be added by the
following patch, I guess.

| +		  results->emplace_back (block, sym);
| +		}
| +	    }
| +	}
| +    }
| +}
| +
| +/* See symtab.h.  */
| +

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I06b26920f35c268f7a38d8203dc2c2813aa501c6
Gerrit-Change-Number: 738
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-CC: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Comment-Date: Tue, 03 Dec 2019 02:18:43 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Tankut Baris Aktemur (Code Review) Dec. 3, 2019, 4:38 p.m. | #4
Simon Marchi has posted comments on this change.

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


Patch Set 4: Code-Review+2


-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I06b26920f35c268f7a38d8203dc2c2813aa501c6
Gerrit-Change-Number: 738
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-Comment-Date: Tue, 03 Dec 2019 16:38:29 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index ff3d233..a21e06c 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,24 @@ 
 2019-12-02  Andrew Burgess  <andrew.burgess@embecosm.com>
 
+	* symtab.c (symbol_search::compare_search_syms): Update header
+	comment.
+	(global_symbol_searcher::is_suitable_msymbol): New function.
+	(global_symbol_searcher::expand_symtabs): New function.
+	(global_symbol_searcher::add_matching_symbols): New function.
+	(global_symbol_searcher::add_matching_msymbols): New function.
+	(global_symbol_searcher::search): Move most of the content
+	into the new functions above, and call them as needed.
+	* symtab.h: Add 'set' include.
+	(global_symbol_searcher) <expand_symtabs>: New member function.
+	(global_symbol_searcher) <add_matching_symbols>: New member
+	function.
+	(global_symbol_searcher) <add_matching_msymbols>: New member
+	function.
+	(global_symbol_searcher) <is_suitable_msymbol>: New member
+	function.
+
+2019-12-02  Andrew Burgess  <andrew.burgess@embecosm.com>
+
 	* mi/mi-cmds.c (mi_cmds): Add -symbol-info-module-functions and
 	-symbol-info-module-variables entries.
 	* mi/mi-cmds.h (mi_cmd_symbol_info_module_functions): Declare.
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 894a323..0e92d4a 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -4370,8 +4370,8 @@ 
   return false;
 }
 
-/* Helper function for sort_search_symbols_remove_dups and qsort.  Can only
-   sort symbols, not minimal symbols.  */
+/* Helper function for std::sort on symbol_search objects.  Can only sort
+   symbols, not minimal symbols.  */
 
 int
 symbol_search::compare_search_syms (const symbol_search &sym_a,
@@ -4432,15 +4432,221 @@ 
   return treg.exec (printed_sym_type_name.c_str (), 0, NULL, 0) == 0;
 }
 
+/* See symtab.h.  */
 
-/* Sort the symbols in RESULT and remove duplicates.  */
-
-static void
-sort_search_symbols_remove_dups (std::vector<symbol_search> *result)
+bool
+global_symbol_searcher::is_suitable_msymbol
+	(const enum search_domain kind, const minimal_symbol *msymbol)
 {
-  std::sort (result->begin (), result->end ());
-  result->erase (std::unique (result->begin (), result->end ()),
-		 result->end ());
+  switch (MSYMBOL_TYPE (msymbol))
+    {
+    case mst_data:
+    case mst_bss:
+    case mst_file_data:
+    case mst_file_bss:
+      return kind == VARIABLES_DOMAIN;
+    case mst_text:
+    case mst_file_text:
+    case mst_solib_trampoline:
+    case mst_text_gnu_ifunc:
+      return kind == FUNCTIONS_DOMAIN;
+    default:
+      return false;
+    }
+}
+
+/* See symtab.h.  */
+
+bool
+global_symbol_searcher::expand_symtabs
+	(objfile *objfile, const gdb::optional<compiled_regex> &preg) const
+{
+  enum search_domain kind = m_kind;
+  bool found_msymbol = false;
+
+  if (objfile->sf)
+    objfile->sf->qf->expand_symtabs_matching
+      (objfile,
+       [&] (const char *filename, bool basenames)
+       {
+	 return file_matches (filename, filenames, basenames);
+       },
+       lookup_name_info::match_any (),
+       [&] (const char *symname)
+       {
+	 return (!preg.has_value ()
+		 || preg->exec (symname, 0, NULL, 0) == 0);
+       },
+       NULL,
+       kind);
+
+  /* Here, we search through the minimal symbol tables for functions and
+     variables that match, and force their symbols to be read.  This is in
+     particular necessary for demangled variable names, which are no longer
+     put into the partial symbol tables.  The symbol will then be found
+     during the scan of symtabs later.
+
+     For functions, find_pc_symtab should succeed if we have debug info for
+     the function, for variables we have to call
+     lookup_symbol_in_objfile_from_linkage_name to determine if the
+     variable has debug info.  If the lookup fails, set found_msymbol so
+     that we will rescan to print any matching symbols without debug info.
+     We only search the objfile the msymbol came from, we no longer search
+     all objfiles.  In large programs (1000s of shared libs) searching all
+     objfiles is not worth the pain.  */
+  if (filenames.empty ()
+      && (kind == VARIABLES_DOMAIN || kind == FUNCTIONS_DOMAIN))
+    {
+      for (minimal_symbol *msymbol : objfile->msymbols ())
+	{
+	  QUIT;
+
+	  if (msymbol->created_by_gdb)
+	    continue;
+
+	  if (is_suitable_msymbol (kind, msymbol))
+	    {
+	      if (!preg.has_value ()
+		  || preg->exec (msymbol->natural_name (), 0,
+				 NULL, 0) == 0)
+		{
+		  /* An important side-effect of these lookup functions is
+		     to expand the symbol table if msymbol is found, later
+		     in the process we will add matching symbols or
+		     msymbols to the results list, and that requires that
+		     the symbols tables are expanded.  */
+		  if (kind == FUNCTIONS_DOMAIN
+		      ? (find_pc_compunit_symtab
+			 (MSYMBOL_VALUE_ADDRESS (objfile, msymbol))
+			 == NULL)
+		      : (lookup_symbol_in_objfile_from_linkage_name
+			 (objfile, msymbol->linkage_name (),
+			  VAR_DOMAIN)
+			 .symbol == NULL))
+		    found_msymbol = true;
+		}
+	    }
+	}
+    }
+
+  return found_msymbol;
+}
+
+/* See symtab.h.  */
+
+void
+global_symbol_searcher::add_matching_symbols
+	(objfile *objfile,
+	 const gdb::optional<compiled_regex> &preg,
+	 const gdb::optional<compiled_regex> &treg,
+	 std::set<symbol_search> *result_set) const
+{
+  enum search_domain kind = m_kind;
+
+  /* Add matching symbols (if not already present).  */
+  for (compunit_symtab *cust : objfile->compunits ())
+    {
+      const struct blockvector *bv  = COMPUNIT_BLOCKVECTOR (cust);
+
+      for (block_enum block : { GLOBAL_BLOCK, STATIC_BLOCK })
+	{
+	  struct block_iterator iter;
+	  struct symbol *sym;
+	  const struct block *b = BLOCKVECTOR_BLOCK (bv, block);
+
+	  ALL_BLOCK_SYMBOLS (b, iter, sym)
+	    {
+	      struct symtab *real_symtab = symbol_symtab (sym);
+
+	      QUIT;
+
+	      /* Check first sole REAL_SYMTAB->FILENAME.  It does
+		 not need to be a substring of symtab_to_fullname as
+		 it may contain "./" etc.  */
+	      if ((file_matches (real_symtab->filename, filenames, false)
+		   || ((basenames_may_differ
+			|| file_matches (lbasename (real_symtab->filename),
+					 filenames, true))
+		       && file_matches (symtab_to_fullname (real_symtab),
+					filenames, false)))
+		  && ((!preg.has_value ()
+		       || preg->exec (sym->natural_name (), 0,
+				      NULL, 0) == 0)
+		      && ((kind == VARIABLES_DOMAIN
+			   && SYMBOL_CLASS (sym) != LOC_TYPEDEF
+			   && SYMBOL_CLASS (sym) != LOC_UNRESOLVED
+			   && SYMBOL_CLASS (sym) != LOC_BLOCK
+			   /* LOC_CONST can be used for more than
+			      just enums, e.g., c++ static const
+			      members.  We only want to skip enums
+			      here.  */
+			   && !(SYMBOL_CLASS (sym) == LOC_CONST
+				&& (TYPE_CODE (SYMBOL_TYPE (sym))
+				    == TYPE_CODE_ENUM))
+			   && (!treg.has_value ()
+			       || treg_matches_sym_type_name (*treg, sym)))
+			  || (kind == FUNCTIONS_DOMAIN
+			      && SYMBOL_CLASS (sym) == LOC_BLOCK
+			      && (!treg.has_value ()
+				  || treg_matches_sym_type_name (*treg,
+								 sym)))
+			  || (kind == TYPES_DOMAIN
+			      && SYMBOL_CLASS (sym) == LOC_TYPEDEF
+			      && SYMBOL_DOMAIN (sym) != MODULE_DOMAIN)
+			  || (kind == MODULES_DOMAIN
+			      && SYMBOL_DOMAIN (sym) == MODULE_DOMAIN
+			      && SYMBOL_LINE (sym) != 0))))
+		{
+		  /* Match, insert if not already in the results.  */
+		  symbol_search ss (block, sym);
+		  if (result_set->find (ss) == result_set->end ())
+		    result_set->insert (ss);
+		}
+	    }
+	}
+    }
+}
+
+/* See symtab.h.  */
+
+void
+global_symbol_searcher::add_matching_msymbols
+	(objfile *objfile, const gdb::optional<compiled_regex> &preg,
+	 std::vector<symbol_search> *results) const
+{
+  enum search_domain kind = m_kind;
+
+  for (minimal_symbol *msymbol : objfile->msymbols ())
+    {
+      QUIT;
+
+      if (msymbol->created_by_gdb)
+	continue;
+
+      if (is_suitable_msymbol (kind, msymbol))
+	{
+	  if (!preg.has_value ()
+	      || preg->exec (msymbol->natural_name (), 0,
+			     NULL, 0) == 0)
+	    {
+	      /* For functions we can do a quick check of whether the
+		 symbol might be found via find_pc_symtab.  */
+	      if (kind != FUNCTIONS_DOMAIN
+		  || (find_pc_compunit_symtab
+		      (MSYMBOL_VALUE_ADDRESS (objfile, msymbol))
+		      == NULL))
+		{
+		  if (lookup_symbol_in_objfile_from_linkage_name
+		      (objfile, msymbol->linkage_name (),
+		       VAR_DOMAIN).symbol == NULL)
+		    {
+		      /* Matching msymbol, add it to the results list.  */
+		      results->emplace_back (GLOBAL_BLOCK, msymbol, objfile);
+		    }
+		}
+	    }
+	}
+    }
 }
 
 /* See symtab.h.  */
@@ -4448,35 +4654,11 @@ 
 std::vector<symbol_search>
 global_symbol_searcher::search () const
 {
-  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[]
-    = {mst_bss, mst_file_text, mst_unknown};
-  static const enum minimal_symbol_type types3[]
-    = {mst_file_data, mst_solib_trampoline, mst_unknown};
-  static const enum minimal_symbol_type types4[]
-    = {mst_file_bss, mst_text_gnu_ifunc, mst_unknown};
-  enum minimal_symbol_type ourtype;
-  enum minimal_symbol_type ourtype2;
-  enum minimal_symbol_type ourtype3;
-  enum minimal_symbol_type ourtype4;
-  std::vector<symbol_search> result;
   gdb::optional<compiled_regex> preg;
   gdb::optional<compiled_regex> treg;
 
   gdb_assert (m_kind != ALL_DOMAIN);
 
-  ourtype = types[m_kind];
-  ourtype2 = types2[m_kind];
-  ourtype3 = types3[m_kind];
-  ourtype4 = types4[m_kind];
-
   if (m_symbol_name_regexp != NULL)
     {
       const char *symbol_name_regexp = m_symbol_name_regexp;
@@ -4529,187 +4711,34 @@ 
 		    _("Invalid regexp"));
     }
 
-  /* 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, filenames,
-						  basenames);
-			   },
-			   lookup_name_info::match_any (),
-			   [&] (const char *symname)
-			   {
-			     return (!preg.has_value ()
-				     || preg->exec (symname,
-						    0, NULL, 0) == 0);
-			   },
-			   NULL,
-			   m_kind);
-
-  /* Here, we search through the minimal symbol tables for functions
-     and variables that match, and force their symbols to be read.
-     This is in particular necessary for demangled variable names,
-     which are no longer put into the partial symbol tables.
-     The symbol will then be found during the scan of symtabs below.
-
-     For functions, find_pc_symtab should succeed if we have debug info
-     for the function, for variables we have to call
-     lookup_symbol_in_objfile_from_linkage_name to determine if the variable
-     has debug info.
-     If the lookup fails, set found_misc so that we will rescan to print
-     any matching symbols without debug info.
-     We only search the objfile the msymbol came from, we no longer search
-     all objfiles.  In large programs (1000s of shared libs) searching all
-     objfiles is not worth the pain.  */
-
-  if (filenames.empty () && (m_kind == VARIABLES_DOMAIN
-			     || m_kind == FUNCTIONS_DOMAIN))
-    {
-      for (objfile *objfile : current_program_space->objfiles ())
-	{
-	  for (minimal_symbol *msymbol : objfile->msymbols ())
-	    {
-	      QUIT;
-
-	      if (msymbol->created_by_gdb)
-		continue;
-
-	      if (MSYMBOL_TYPE (msymbol) == ourtype
-		  || MSYMBOL_TYPE (msymbol) == ourtype2
-		  || MSYMBOL_TYPE (msymbol) == ourtype3
-		  || MSYMBOL_TYPE (msymbol) == ourtype4)
-		{
-		  if (!preg.has_value ()
-		      || preg->exec (msymbol->natural_name (), 0,
-				     NULL, 0) == 0)
-		    {
-		      /* Note: An important side-effect of these
-			 lookup functions is to expand the symbol
-			 table if msymbol is found, for the benefit of
-			 the next loop on compunits.  */
-		      if (m_kind == FUNCTIONS_DOMAIN
-			  ? (find_pc_compunit_symtab
-			     (MSYMBOL_VALUE_ADDRESS (objfile, msymbol))
-			     == NULL)
-			  : (lookup_symbol_in_objfile_from_linkage_name
-			     (objfile, msymbol->linkage_name (), VAR_DOMAIN)
-			     .symbol == NULL))
-			found_misc = 1;
-		    }
-		}
-	    }
-	}
-    }
-
+  bool found_msymbol = false;
+  std::set<symbol_search> result_set;
   for (objfile *objfile : current_program_space->objfiles ())
     {
-      for (compunit_symtab *cust : objfile->compunits ())
-	{
-	  bv = COMPUNIT_BLOCKVECTOR (cust);
-	  for (i = GLOBAL_BLOCK; i <= STATIC_BLOCK; i++)
-	    {
-	      b = BLOCKVECTOR_BLOCK (bv, i);
-	      ALL_BLOCK_SYMBOLS (b, iter, sym)
-		{
-		  struct symtab *real_symtab = symbol_symtab (sym);
+      /* Expand symtabs within objfile that possibly contain matching
+	 symbols.  */
+      found_msymbol |= expand_symtabs (objfile, preg);
 
-		  QUIT;
-
-		  /* Check first sole REAL_SYMTAB->FILENAME.  It does
-		     not need to be a substring of symtab_to_fullname as
-		     it may contain "./" etc.  */
-		  if ((file_matches (real_symtab->filename, filenames, false)
-		       || ((basenames_may_differ
-			    || file_matches (lbasename (real_symtab->filename),
-					     filenames, true))
-			   && file_matches (symtab_to_fullname (real_symtab),
-					    filenames, false)))
-		      && ((!preg.has_value ()
-			   || preg->exec (sym->natural_name (), 0,
-					  NULL, 0) == 0)
-			  && ((m_kind == VARIABLES_DOMAIN
-			       && SYMBOL_CLASS (sym) != LOC_TYPEDEF
-			       && SYMBOL_CLASS (sym) != LOC_UNRESOLVED
-			       && SYMBOL_CLASS (sym) != LOC_BLOCK
-			       /* LOC_CONST can be used for more than
-				  just enums, e.g., c++ static const
-				  members.  We only want to skip enums
-				  here.  */
-			       && !(SYMBOL_CLASS (sym) == LOC_CONST
-				    && (TYPE_CODE (SYMBOL_TYPE (sym))
-					== TYPE_CODE_ENUM))
-			       && (!treg.has_value ()
-				   || treg_matches_sym_type_name (*treg, sym)))
-			      || (m_kind == FUNCTIONS_DOMAIN
-				  && SYMBOL_CLASS (sym) == LOC_BLOCK
-				  && (!treg.has_value ()
-				      || treg_matches_sym_type_name (*treg,
-								     sym)))
-			      || (m_kind == TYPES_DOMAIN
-				  && SYMBOL_CLASS (sym) == LOC_TYPEDEF
-				  && SYMBOL_DOMAIN (sym) != MODULE_DOMAIN)
-			      || (m_kind == MODULES_DOMAIN
-				  && SYMBOL_DOMAIN (sym) == MODULE_DOMAIN
-				  && SYMBOL_LINE (sym) != 0))))
-		    {
-		      /* match */
-		      result.emplace_back (i, sym);
-		    }
-		}
-	    }
-	}
+      /* Find matching symbols within OBJFILE and add them in to
+	 RESULT_SET.  */
+      add_matching_symbols (objfile, preg, treg, &result_set);
     }
 
-  if (!result.empty ())
-    sort_search_symbols_remove_dups (&result);
+  /* Convert the result set into a sorted result list.  */
+  std::vector<symbol_search> result (result_set.begin (), result_set.end ());
+  std::sort (result.begin (), result.end ());
 
   /* 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 || (filenames.empty () && m_kind != FUNCTIONS_DOMAIN))
+     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 ((found_msymbol || (filenames.empty () && m_kind == VARIABLES_DOMAIN))
       && !m_exclude_minsyms
       && !treg.has_value ())
     {
+      gdb_assert (m_kind == VARIABLES_DOMAIN || m_kind == FUNCTIONS_DOMAIN);
       for (objfile *objfile : current_program_space->objfiles ())
-	{
-	  for (minimal_symbol *msymbol : objfile->msymbols ())
-	    {
-	      QUIT;
-
-	      if (msymbol->created_by_gdb)
-		continue;
-
-	      if (MSYMBOL_TYPE (msymbol) == ourtype
-		  || MSYMBOL_TYPE (msymbol) == ourtype2
-		  || MSYMBOL_TYPE (msymbol) == ourtype3
-		  || MSYMBOL_TYPE (msymbol) == ourtype4)
-		{
-		  if (!preg.has_value ()
-		      || preg->exec (msymbol->natural_name (), 0,
-				     NULL, 0) == 0)
-		    {
-		      /* For functions we can do a quick check of whether the
-			 symbol might be found via find_pc_symtab.  */
-		      if (m_kind != FUNCTIONS_DOMAIN
-			  || (find_pc_compunit_symtab
-			      (MSYMBOL_VALUE_ADDRESS (objfile, msymbol))
-			      == NULL))
-			{
-			  if (lookup_symbol_in_objfile_from_linkage_name
-			      (objfile, msymbol->linkage_name (), VAR_DOMAIN)
-			      .symbol == NULL)
-			    {
-			      /* match */
-			      result.emplace_back (i, msymbol, objfile);
-			    }
-			}
-		    }
-		}
-	    }
-	}
+	add_matching_msymbols (objfile, preg, &result);
     }
 
   return result;
diff --git a/gdb/symtab.h b/gdb/symtab.h
index 4cfdf06..f183083 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -23,6 +23,7 @@ 
 #include <array>
 #include <vector>
 #include <string>
+#include <set>
 #include "gdbsupport/gdb_vecs.h"
 #include "gdbtypes.h"
 #include "gdb_obstack.h"
@@ -2124,6 +2125,29 @@ 
   /* 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;
+
+  /* Expand symtabs in OBJFILE that match PREG, are of type M_KIND.  Return
+     true if any msymbols were seen that we should later consider adding to
+     the results list.  */
+  bool expand_symtabs (objfile *objfile,
+		       const gdb::optional<compiled_regex> &preg) const;
+
+  /* Add symbols from symtabs in OBJFILE that match PREG, and TREG, and are
+     of type M_KIND, to the results set RESULTS_SET.  */
+  void add_matching_symbols (objfile *objfile,
+			     const gdb::optional<compiled_regex> &preg,
+			     const gdb::optional<compiled_regex> &treg,
+			     std::set<symbol_search> *result_set) const;
+
+  /* Add msymbols from OBJFILE that match PREG and M_KIND, to the
+     results vector RESULTS.  */
+  void add_matching_msymbols (objfile *objfile,
+			      const gdb::optional<compiled_regex> &preg,
+			      std::vector<symbol_search> *results) const;
+
+  /* Return true if MSYMBOL is of type KIND.  */
+  static bool is_suitable_msymbol (const enum search_domain kind,
+				   const minimal_symbol *msymbol);
 };
 
 /* When searching for Fortran symbols within modules (functions/variables)