This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 2/9] Change `function_symbols' to std::vector


On 08/11/2018 07:49 AM, Tom Tromey wrote:
>>>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes:
> 
> Keith> This patch changes the `function_symbols'  members in linespec.c structures
> Keith> from a VEC to a std::vector.
> 
> Keith> gdb/ChangeLog:
> 
> Keith> 	* linespec.c (struct linespec) <function_symbols>: Change type to
> Keith> 	std::vector.  Update all users.
> Keith> 	(struct collect_info) <function_symbols>: Likewise.
> Keith> 	(find_method, find_function_symbols, find_linespec_symbols)
> Keith> 	(find_label_symbols_in_block): Change `symbols' parameter to
> Keith> 	std::vector.  Update all callers.
> Keith> 	(find_label_symbols): Likewise for `function_symbols' and
> Keith> 	`label_funcs_ret'.
> 
> Thank you for the patch.
> This one looks ok to me.

I know you approved this, but based on feedback on the minimal_symbols patch
(#4 in this series), I thought I would also switch to sorting function_symbols
with std::sort instead of qsort.

This does involve introducing a new "std_compare_symbols" function since the
one remaining qsort cannot be eliminated until later. [By the end of the
series, though, we will be back to one Compare function again.]

This is the only change in this revision.

Keith

Subject: [PATCH] Change `function_symbols' to std::vector

This patch changes the `function_symbols'  members in linespec.c structures
from a VEC to a std::vector.

gdb/ChangeLog:

	* linespec.c (struct linespec) <function_symbols>: Change type to
	std::vector.  Update all users.
	(struct collect_info) <function_symbols>: Likewise.
	(convert_linespec_to_sals): Use std::sort to sort function_symbols.
	(std_compare_symbols): New function.
	(find_method, find_function_symbols, find_linespec_symbols)
	(find_label_symbols_in_block): Change `symbols' parameter to
	std::vector.  Update all callers.
	(find_label_symbols): Likewise for `function_symbols' and
	`label_funcs_ret'.
---
 gdb/ChangeLog  |  13 +++++
 gdb/linespec.c | 168 +++++++++++++++++++++++++++++++--------------------------
 2 files changed, 104 insertions(+), 77 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index b9d6cb165a..d3d2e6e644 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,18 @@
 YYYY-MM-DD  Keith Seitz  <keiths@redhat.com>
 
+	* linespec.c (struct linespec) <function_symbols>: Change type to
+	std::vector.  Update all users.
+	(struct collect_info) <function_symbols>: Likewise.
+	(convert_linespec_to_sals): Use std::sort to sort function_symbols.
+	(std_compare_symbols): New function.
+	(find_method, find_function_symbols, find_linespec_symbols)
+	(find_label_symbols_in_block): Change `symbols' parameter to
+	std::vector.  Update all callers.
+	(find_label_symbols): Likewise for `function_symbols' and
+	`label_funcs_ret'.
+
+YYYY-MM-DD  Keith Seitz  <keiths@redhat.com>
+
 	* linespec.c (symtab_vector_up): Define.
 	(struct linespec) <file_symtabs>: Change type to std::vector *.
 	Update all uses.
diff --git a/gdb/linespec.c b/gdb/linespec.c
index 032f6af0d4..7fff8a2b71 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -114,8 +114,8 @@ struct linespec
   std::vector<symtab *> *file_symtabs;
 
   /* A list of matching function symbols and minimal symbols.  Both lists
-     may be NULL if no matching symbols were found.  */
-  VEC (symbolp) *function_symbols;
+     may be NULL (or empty) if no matching symbols were found.  */
+  std::vector<symbol *> *function_symbols;
   VEC (bound_minimal_symbol_d) *minimal_symbols;
 
   /* A structure of matching label symbols and the corresponding
@@ -124,7 +124,7 @@ struct linespec
   struct
   {
     VEC (symbolp) *label_symbols;
-    VEC (symbolp) *function_symbols;
+    std::vector<symbol *> *function_symbols;
   } labels;
 };
 typedef struct linespec *linespec_p;
@@ -201,7 +201,7 @@ struct collect_info
   /* The result being accumulated.  */
   struct
   {
-    VEC (symbolp) *symbols;
+    std::vector<symbol *> *symbols;
     VEC (bound_minimal_symbol_d) *minimal_symbols;
   } result;
 
@@ -215,7 +215,7 @@ collect_info::add_symbol (symbol *sym)
   /* In list mode, add all matching symbols, regardless of class.
      This allows the user to type "list a_global_variable".  */
   if (SYMBOL_CLASS (sym) == LOC_BLOCK || this->state->list_mode)
-    VEC_safe_push (symbolp, this->result.symbols, sym);
+    this->result.symbols->push_back (sym);
 
   /* Continue iterating.  */
   return true;
@@ -352,17 +352,16 @@ static std::vector<symtab_and_line> decode_objc (struct linespec_state *self,
 static symtab_vector_up symtabs_from_filename
   (const char *, struct program_space *pspace);
 
-static VEC (symbolp) *find_label_symbols (struct linespec_state *self,
-					  VEC (symbolp) *function_symbols,
-					  VEC (symbolp) **label_funcs_ret,
-					  const char *name,
-					  bool completion_mode = false);
+static VEC (symbolp) *find_label_symbols
+  (struct linespec_state *self, std::vector<symbol *> *function_symbols,
+   std::vector<symbol *> *label_funcs_ret, const char *name,
+   bool completion_mode = false);
 
 static void find_linespec_symbols (struct linespec_state *self,
 				   std::vector<symtab *> *file_symtabs,
 				   const char *name,
 				   symbol_name_match_type name_match_type,
-				   VEC (symbolp) **symbols,
+				   std::vector<symbol *> *symbols,
 				   VEC (bound_minimal_symbol_d) **minsyms);
 
 static struct line_offset
@@ -402,6 +401,8 @@ static void minsym_found (struct linespec_state *self, struct objfile *objfile,
 			  std::vector<symtab_and_line> *result);
 
 static int compare_symbols (const void *a, const void *b);
+static bool std_compare_symbols (const struct symbol *a,
+				 const struct symbol *b);
 
 static int compare_msymbols (const void *a, const void *b);
 
@@ -1767,7 +1768,8 @@ linespec_parse_basic (linespec_parser *parser)
 {
   gdb::unique_xmalloc_ptr<char> name;
   linespec_token token;
-  VEC (symbolp) *symbols, *labels;
+  std::vector<symbol *> symbols;
+  VEC (symbolp) *labels;
   VEC (bound_minimal_symbol_d) *minimal_symbols;
 
   /* Get the next token.  */
@@ -1871,12 +1873,12 @@ linespec_parse_basic (linespec_parser *parser)
 			     PARSER_EXPLICIT (parser)->func_name_match_type,
 			     &symbols, &minimal_symbols);
 
-      if (symbols != NULL || minimal_symbols != NULL)
+      if (!symbols.empty () || minimal_symbols != NULL)
 	{
-	  PARSER_RESULT (parser)->function_symbols = symbols;
+	  PARSER_RESULT (parser)->function_symbols
+	    = new std::vector<symbol *> (std::move (symbols));
 	  PARSER_RESULT (parser)->minimal_symbols = minimal_symbols;
 	  PARSER_EXPLICIT (parser)->function_name = name.release ();
-	  symbols = NULL;
 	}
       else
 	{
@@ -1887,9 +1889,9 @@ linespec_parse_basic (linespec_parser *parser)
 	  if (labels != NULL)
 	    {
 	      PARSER_RESULT (parser)->labels.label_symbols = labels;
-	      PARSER_RESULT (parser)->labels.function_symbols = symbols;
+	      PARSER_RESULT (parser)->labels.function_symbols
+		= new std::vector<symbol *> (std::move (symbols));
 	      PARSER_EXPLICIT (parser)->label_name = name.release ();
-	      symbols = NULL;
 	    }
 	  else if (token.type == LSTOKEN_STRING
 		   && *LS_TOKEN_STOKEN (token).ptr == '$')
@@ -1993,9 +1995,9 @@ linespec_parse_basic (linespec_parser *parser)
 	      if (labels != NULL)
 		{
 		  PARSER_RESULT (parser)->labels.label_symbols = labels;
-		  PARSER_RESULT (parser)->labels.function_symbols = symbols;
+		  PARSER_RESULT (parser)->labels.function_symbols
+		    = new std::vector<symbol *> (std::move (symbols));
 		  PARSER_EXPLICIT (parser)->label_name = name.release ();
-		  symbols = NULL;
 		}
 	      else
 		{
@@ -2061,13 +2063,10 @@ canonicalize_linespec (struct linespec_state *state, const linespec_p ls)
 
       if (explicit_loc->function_name == NULL)
 	{
-	  struct symbol *s;
-
 	  /* No function was specified, so add the symbol name.  */
-	  gdb_assert (ls->labels.function_symbols != NULL
-		      && (VEC_length (symbolp, ls->labels.function_symbols)
-			  == 1));
-	  s = VEC_index (symbolp, ls->labels.function_symbols, 0);
+	  gdb_assert (!ls->labels.function_symbols->empty ()
+		      && (ls->labels.function_symbols->size () == 1));
+	  struct symbol *s = ls->labels.function_symbols->front ();
 	  explicit_loc->function_name = xstrdup (SYMBOL_NATURAL_NAME (s));
 	}
     }
@@ -2265,12 +2264,11 @@ convert_linespec_to_sals (struct linespec_state *state, linespec_p ls)
 	{
 	  /* Sort symbols so that symbols with the same program space are next
 	     to each other.  */
-	  qsort (VEC_address (symbolp, ls->function_symbols),
-		 VEC_length (symbolp, ls->function_symbols),
-		 sizeof (symbolp), compare_symbols);
+	  std::sort (ls->function_symbols->begin (),
+		     ls->function_symbols->end (),
+		     std_compare_symbols);
 
-	  struct symbol *sym;
-	  for (int i = 0; VEC_iterate (symbolp, ls->function_symbols, i, sym); ++i)
+	  for (const auto &sym : *ls->function_symbols)
 	    {
 	      program_space *pspace = SYMTAB_PSPACE (symbol_symtab (sym));
 	      set_current_program_space (pspace);
@@ -2392,7 +2390,8 @@ convert_explicit_location_to_linespec (struct linespec_state *self,
 				       const char *label_name,
 				       struct line_offset line_offset)
 {
-  VEC (symbolp) *symbols, *labels;
+  std::vector<symbol *> symbols;
+  VEC (symbolp) *labels;
   VEC (bound_minimal_symbol_d) *minimal_symbols;
 
   result->explicit_loc.func_name_match_type = fname_match_type;
@@ -2424,18 +2423,18 @@ convert_explicit_location_to_linespec (struct linespec_state *self,
 			     function_name, fname_match_type,
 			     &symbols, &minimal_symbols);
 
-      if (symbols == NULL && minimal_symbols == NULL)
+      if (symbols.empty () && minimal_symbols == NULL)
 	symbol_not_found_error (function_name,
 				result->explicit_loc.source_filename);
 
       result->explicit_loc.function_name = xstrdup (function_name);
-      result->function_symbols = symbols;
+      result->function_symbols
+	= new std::vector<symbol *> (std::move (symbols));
       result->minimal_symbols = minimal_symbols;
     }
 
   if (label_name != NULL)
     {
-      symbols = NULL;
       labels = find_label_symbols (self, result->function_symbols,
 				   &symbols, label_name);
 
@@ -2445,7 +2444,8 @@ convert_explicit_location_to_linespec (struct linespec_state *self,
 
       result->explicit_loc.label_name = xstrdup (label_name);
       result->labels.label_symbols = labels;
-      result->labels.function_symbols = symbols;
+      result->labels.function_symbols
+	= new std::vector<symbol *> (std::move (symbols));
     }
 
   if (line_offset.sign != LINE_OFFSET_UNKNOWN)
@@ -2780,9 +2780,7 @@ linespec_parser_delete (void *arg)
   xfree (PARSER_EXPLICIT (parser)->function_name);
 
   delete PARSER_RESULT (parser)->file_symtabs;
-
-  if (PARSER_RESULT (parser)->function_symbols != NULL)
-    VEC_free (symbolp, PARSER_RESULT (parser)->function_symbols);
+  delete PARSER_RESULT (parser)->function_symbols;
 
   if (PARSER_RESULT (parser)->minimal_symbols != NULL)
     VEC_free (bound_minimal_symbol_d, PARSER_RESULT (parser)->minimal_symbols);
@@ -2790,8 +2788,7 @@ linespec_parser_delete (void *arg)
   if (PARSER_RESULT (parser)->labels.label_symbols != NULL)
     VEC_free (symbolp, PARSER_RESULT (parser)->labels.label_symbols);
 
-  if (PARSER_RESULT (parser)->labels.function_symbols != NULL)
-    VEC_free (symbolp, PARSER_RESULT (parser)->labels.function_symbols);
+  delete PARSER_RESULT (parser)->labels.function_symbols;
 
   linespec_state_destructor (PARSER_STATE (parser));
 }
@@ -2924,7 +2921,7 @@ complete_label (completion_tracker &tracker,
 		linespec_parser *parser,
 		const char *label_name)
 {
-  VEC (symbolp) *label_function_symbols = NULL;
+  std::vector<symbol *> label_function_symbols;
   VEC (symbolp) *labels
     = find_label_symbols (PARSER_STATE (parser),
 			  PARSER_RESULT (parser)->function_symbols,
@@ -3047,14 +3044,15 @@ linespec_complete (completion_tracker &tracker, const char *text,
 
       const char *func_name = PARSER_EXPLICIT (&parser)->function_name;
 
-      VEC (symbolp) *function_symbols;
+      std::vector<symbol *> function_symbols;
       VEC (bound_minimal_symbol_d) *minimal_symbols;
       find_linespec_symbols (PARSER_STATE (&parser),
 			     PARSER_RESULT (&parser)->file_symtabs,
 			     func_name, match_type,
 			     &function_symbols, &minimal_symbols);
 
-      PARSER_RESULT (&parser)->function_symbols = function_symbols;
+      PARSER_RESULT (&parser)->function_symbols
+	= new std::vector<symbol *> (std::move (function_symbols));
       PARSER_RESULT (&parser)->minimal_symbols = minimal_symbols;
 
       complete_label (tracker, &parser, parser.completion_word);
@@ -3452,7 +3450,9 @@ decode_objc (struct linespec_state *self, linespec_p ls, const char *arg)
   symtabs.push_back (nullptr);
 
   info.file_symtabs = &symtabs;
-  info.result.symbols = NULL;
+
+  std::vector<symbol *> symbols;
+  info.result.symbols = &symbols;
   info.result.minimal_symbols = NULL;
 
   new_argptr = find_imps (arg, &symbol_names);
@@ -3463,7 +3463,7 @@ decode_objc (struct linespec_state *self, linespec_p ls, const char *arg)
 				    FUNCTIONS_DOMAIN);
 
   std::vector<symtab_and_line> values;
-  if (!VEC_empty (symbolp, info.result.symbols)
+  if (!symbols.empty ()
       || !VEC_empty (bound_minimal_symbol_d, info.result.minimal_symbols))
     {
       char *saved_arg;
@@ -3473,7 +3473,7 @@ decode_objc (struct linespec_state *self, linespec_p ls, const char *arg)
       saved_arg[new_argptr - arg] = '\0';
 
       ls->explicit_loc.function_name = xstrdup (saved_arg);
-      ls->function_symbols = info.result.symbols;
+      ls->function_symbols = new std::vector<symbol *> (std::move (symbols));
       ls->minimal_symbols = info.result.minimal_symbols;
       values = convert_linespec_to_sals (self, ls);
 
@@ -3608,6 +3608,32 @@ lookup_prefix_sym (struct linespec_state *state,
   return collector.release_symbols ();
 }
 
+/* A std::sort comparison function for symbols.  The resulting order does
+   not actually matter; we just need to be able to sort them so that
+   symbols with the same program space end up next to each other.  */
+
+static bool
+std_compare_symbols (const struct symbol *a, const struct symbol *b)
+{
+  uintptr_t uia, uib;
+
+  uia = (uintptr_t) SYMTAB_PSPACE (symbol_symtab (a));
+  uib = (uintptr_t) SYMTAB_PSPACE (symbol_symtab (b));
+
+  if (uia < uib)
+    return true;
+  if (uia > uib)
+    return false;
+
+  uia = (uintptr_t) a;
+  uib = (uintptr_t) b;
+
+  if (uia < uib)
+    return true;
+
+  return false;
+}
+
 /* A qsort comparison function for symbols.  The resulting order does
    not actually matter; we just need to be able to sort them so that
    symbols with the same program space end up next to each other.  */
@@ -3713,7 +3739,7 @@ find_superclass_methods (std::vector<struct type *> &&superclasses,
 static void
 find_method (struct linespec_state *self, std::vector<symtab *> *file_symtabs,
 	     const char *class_name, const char *method_name,
-	     VEC (symbolp) *sym_classes, VEC (symbolp) **symbols,
+	     VEC (symbolp) *sym_classes, std::vector<symbol *> *symbols,
 	     VEC (bound_minimal_symbol_d) **minsyms)
 {
   struct symbol *sym;
@@ -3732,7 +3758,7 @@ find_method (struct linespec_state *self, std::vector<symtab *> *file_symtabs,
 
   info.state = self;
   info.file_symtabs = file_symtabs;
-  info.result.symbols = NULL;
+  info.result.symbols = symbols;
   info.result.minimal_symbols = NULL;
 
   /* Iterate over all the types, looking for the names of existing
@@ -3783,10 +3809,9 @@ find_method (struct linespec_state *self, std::vector<symtab *> *file_symtabs,
 	}
     }
 
-  if (!VEC_empty (symbolp, info.result.symbols)
+  if (!symbols->empty ()
       || !VEC_empty (bound_minimal_symbol_d, info.result.minimal_symbols))
     {
-      *symbols = info.result.symbols;
       *minsyms = info.result.minimal_symbols;
       return;
     }
@@ -3916,14 +3941,14 @@ static void
 find_function_symbols (struct linespec_state *state,
 		       std::vector<symtab *> *file_symtabs, const char *name,
 		       symbol_name_match_type name_match_type,
-		       VEC (symbolp) **symbols,
+		       std::vector<symbol *> *symbols,
 		       VEC (bound_minimal_symbol_d) **minsyms)
 {
   struct collect_info info;
   std::vector<const char *> symbol_names;
 
   info.state = state;
-  info.result.symbols = NULL;
+  info.result.symbols = symbols;
   info.result.minimal_symbols = NULL;
   info.file_symtabs = file_symtabs;
 
@@ -3936,14 +3961,6 @@ find_function_symbols (struct linespec_state *state,
     add_matching_symbols_to_info (name, name_match_type, FUNCTIONS_DOMAIN,
 				  &info, state->search_pspace);
 
-  if (VEC_empty (symbolp, info.result.symbols))
-    {
-      VEC_free (symbolp, info.result.symbols);
-      *symbols = NULL;
-    }
-  else
-    *symbols = info.result.symbols;
-
   if (VEC_empty (bound_minimal_symbol_d, info.result.minimal_symbols))
     {
       VEC_free (bound_minimal_symbol_d, info.result.minimal_symbols);
@@ -3961,7 +3978,7 @@ find_linespec_symbols (struct linespec_state *state,
 		       std::vector<symtab *> *file_symtabs,
 		       const char *lookup_name,
 		       symbol_name_match_type name_match_type,
-		       VEC (symbolp) **symbols,
+		       std::vector <symbol *> *symbols,
 		       VEC (bound_minimal_symbol_d) **minsyms)
 {
   std::string canon = cp_canonicalize_string_no_typedefs (lookup_name);
@@ -3978,13 +3995,12 @@ find_linespec_symbols (struct linespec_state *state,
      2) break class::method where method is in class (and not a baseclass)  */
 
   find_function_symbols (state, file_symtabs, lookup_name,
-			 name_match_type,
-			 symbols, minsyms);
+			 name_match_type, symbols, minsyms);
 
   /* If we were unable to locate a symbol of the same name, try dividing
      the name into class and method names and searching the class and its
      baseclasses.  */
-  if (VEC_empty (symbolp, *symbols)
+  if (symbols->empty ()
       && VEC_empty (bound_minimal_symbol_d, *minsyms))
     {
       std::string klass, method;
@@ -4058,7 +4074,7 @@ find_label_symbols_in_block (const struct block *block,
 			     const char *name, struct symbol *fn_sym,
 			     bool completion_mode,
 			     VEC (symbolp) **result,
-			     VEC (symbolp) **label_funcs_ret)
+			     std::vector<symbol *> *label_funcs_ret)
 {
   if (completion_mode)
     {
@@ -4076,7 +4092,7 @@ find_label_symbols_in_block (const struct block *block,
 	      && cmp (SYMBOL_SEARCH_NAME (sym), name, name_len) == 0)
 	    {
 	      VEC_safe_push (symbolp, *result, sym);
-	      VEC_safe_push (symbolp, *label_funcs_ret, fn_sym);
+	      label_funcs_ret->push_back (fn_sym);
 	    }
 	}
     }
@@ -4087,7 +4103,7 @@ find_label_symbols_in_block (const struct block *block,
       if (sym != NULL)
 	{
 	  VEC_safe_push (symbolp, *result, sym);
-	  VEC_safe_push (symbolp, *label_funcs_ret, fn_sym);
+	  label_funcs_ret->push_back (fn_sym);
 	}
     }
 }
@@ -4100,11 +4116,10 @@ find_label_symbols_in_block (const struct block *block,
 
 static VEC (symbolp) *
 find_label_symbols (struct linespec_state *self,
-		    VEC (symbolp) *function_symbols,
-		    VEC (symbolp) **label_funcs_ret, const char *name,
+		    std::vector<symbol *> *function_symbols,
+		    std::vector<symbol *> *label_funcs_ret, const char *name,
 		    bool completion_mode)
 {
-  int ix;
   const struct block *block;
   struct symbol *fn_sym;
   VEC (symbolp) *result = NULL;
@@ -4127,13 +4142,12 @@ find_label_symbols (struct linespec_state *self,
     }
   else
     {
-      for (ix = 0;
-	   VEC_iterate (symbolp, function_symbols, ix, fn_sym); ++ix)
+      for (const auto &elt : *function_symbols)
 	{
-	  set_current_program_space (SYMTAB_PSPACE (symbol_symtab (fn_sym)));
-	  block = SYMBOL_BLOCK_VALUE (fn_sym);
+	  set_current_program_space (SYMTAB_PSPACE (symbol_symtab (elt)));
+	  block = SYMBOL_BLOCK_VALUE (elt);
 
-	  find_label_symbols_in_block (block, name, fn_sym, completion_mode,
+	  find_label_symbols_in_block (block, name, elt, completion_mode,
 				       &result, label_funcs_ret);
 	}
     }
@@ -4489,7 +4503,7 @@ add_matching_symbols_to_info (const char *name,
 	}
       else if (pspace == NULL || pspace == SYMTAB_PSPACE (elt))
 	{
-	  int prev_len = VEC_length (symbolp, info->result.symbols);
+	  int prev_len = info->result.symbols->size ();
 
 	  /* Program spaces that are executing startup should have
 	     been filtered out earlier.  */
@@ -4503,7 +4517,7 @@ add_matching_symbols_to_info (const char *name,
 	     is in assembler, we might actually be looking for a label for
 	     which we don't have debug info.  Check for a minimal symbol in
 	     this case.  */
-	  if (prev_len == VEC_length (symbolp, info->result.symbols)
+	  if (prev_len == info->result.symbols->size ()
 	      && elt->language == language_asm)
 	    search_minsyms_for_name (info, lookup_name, pspace, elt);
 	}
-- 
2.13.6


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]