]> git.ipfire.org Git - thirdparty/binutils-gdb.git/commitdiff
Remove symbol lookup of arguments
authorTom Tromey <tromey@adacore.com>
Wed, 8 Apr 2026 13:00:59 +0000 (07:00 -0600)
committerTom Tromey <tromey@adacore.com>
Fri, 1 May 2026 16:06:18 +0000 (10:06 -0600)
gdb has some code that looks up argument variables a second time.  So
for instance in mi-cmd-stack.c:

      if (sym->is_argument ())
sym2 = (lookup_symbol_search_name
(sym->search_name (),
 block, SEARCH_VAR_DOMAIN).symbol);
      else
sym2 = sym;

I believe this is a leftover artifact of stabs.  I'm not really
certain (I never really learned stabs) but I think in stabs a function
argument had two symbols: one for the parameter and a second one that
described the location of the argument after the prologue.

DWARF doesn't do this, and there's no need to keep this code around
any more.

This patch removes these vestiges.  In the Ada code, removing the
special argument handling also left the "found_sym" code unused,
leading to more deletions.

For block_lookup_symbol, while the comment there explains that the
"best symbol" hack isn't needed in this situation, it seemed to me
that (1) it might in fact be (because I think a function block could
very well have locals and types as well), and (2) making this function
simpler and removing the argument hack is the better approach anyway.

Regression tested on x86-64 Fedora 43.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=34037

gdb/ada-lang.c
gdb/block.c
gdb/mi/mi-cmd-stack.c
gdb/stack.c

index 71a338ce17efabc0661d2e5fe5c095edac108f43..759b81d31e461846f97df11c7dd2166cb614e29b 100644 (file)
@@ -5429,26 +5429,10 @@ struct match_data
 
   void operator() (struct block_symbol *bsym);
 
-  void finish (const block *block);
-
   struct objfile *objfile = nullptr;
   std::vector<struct block_symbol> *resultp;
-  struct symbol *arg_sym = nullptr;
-  bool found_sym = false;
 };
 
-/* Finish iteration over one block.  If a symbol hasn't been found
-   already, add 'arg_sym' to the list of symbols.  */
-
-void
-match_data::finish (const block *block)
-{
-  if (!found_sym && arg_sym != nullptr)
-    add_defn_to_vec (*resultp, arg_sym, block);
-  found_sym = false;
-  arg_sym = nullptr;
-}
-
 /* A callback for add_nonlocal_symbols that adds symbol, found in
    BSYM, to a list of symbols.  */
 
@@ -5460,27 +5444,20 @@ match_data::operator() (struct block_symbol *bsym)
 
   if (sym->loc_class () == LOC_UNRESOLVED)
     return;
-  else if (sym->is_argument ())
-    arg_sym = sym;
   else
-    {
-      found_sym = true;
-      add_defn_to_vec (*resultp, sym, block);
-    }
+    add_defn_to_vec (*resultp, sym, block);
 }
 
 /* Helper for add_nonlocal_symbols.  Find symbols in DOMAIN which are
    targeted by renamings matching LOOKUP_NAME in BLOCK.  Add these
-   symbols to RESULT.  Return whether we found such symbols.  */
+   symbols to RESULT.  */
 
-static bool
+static void
 ada_add_block_renamings (std::vector<struct block_symbol> &result,
                         const struct block *block,
                         const lookup_name_info &lookup_name,
                         domain_search_flags domain)
 {
-  int defns_mark = result.size ();
-
   symbol_name_matcher_ftype *name_match
     = ada_get_symbol_name_matcher (lookup_name);
 
@@ -5531,7 +5508,6 @@ ada_add_block_renamings (std::vector<struct block_symbol> &result,
                               1, NULL);
        }
     }
-  return result.size () != defns_mark;
 }
 
 /* Convenience function to get at the Ada encoded lookup name for
@@ -5562,7 +5538,6 @@ map_matching_symbols (struct objfile *objfile,
       const struct block *block
        = symtab->blockvector ()->block (block_kind);
       for_each_symbol (block, lookup_name, domain, data);
-      data.finish (block);
       return iteration_status::keep_going;
     };
 
@@ -5594,9 +5569,7 @@ add_nonlocal_symbols (std::vector<struct block_symbol> &result,
          const struct block *global_block
            = cu.blockvector ()->global_block ();
 
-         if (ada_add_block_renamings (result, global_block, lookup_name,
-                                      domain))
-           data.found_sym = true;
+         ada_add_block_renamings (result, global_block, lookup_name, domain);
        }
     }
 
@@ -6046,44 +6019,18 @@ ada_add_block_symbols (std::vector<struct block_symbol> &result,
                       const lookup_name_info &lookup_name,
                       domain_search_flags domain, struct objfile *objfile)
 {
-  /* A matching argument symbol, if any.  */
-  struct symbol *arg_sym;
-  /* Set true when we find a matching non-argument symbol.  */
-  bool found_sym;
-
-  arg_sym = NULL;
-  found_sym = false;
   for (struct symbol *sym : block_iterator_range (block, &lookup_name))
     {
-      if (sym->matches (domain))
-       {
-         if (sym->loc_class () != LOC_UNRESOLVED)
-           {
-             if (sym->is_argument ())
-               arg_sym = sym;
-             else
-               {
-                 found_sym = true;
-                 add_defn_to_vec (result, sym, block);
-               }
-           }
-       }
+      if (sym->matches (domain) && sym->loc_class () != LOC_UNRESOLVED)
+       add_defn_to_vec (result, sym, block);
     }
 
   /* Handle renamings.  */
 
-  if (ada_add_block_renamings (result, block, lookup_name, domain))
-    found_sym = true;
-
-  if (!found_sym && arg_sym != NULL)
-    {
-      add_defn_to_vec (result, arg_sym, block);
-    }
+  ada_add_block_renamings (result, block, lookup_name, domain);
 
   if (!lookup_name.ada ().wild_match_p ())
     {
-      arg_sym = NULL;
-      found_sym = false;
       const std::string &ada_lookup_name = lookup_name.ada ().lookup_name ();
       const char *name = ada_lookup_name.c_str ();
       size_t name_len = ada_lookup_name.size ();
@@ -6107,25 +6054,10 @@ ada_add_block_symbols (std::vector<struct block_symbol> &result,
                && is_name_suffix (sym->linkage_name () + name_len + 5))
              {
                if (sym->loc_class () != LOC_UNRESOLVED)
-                 {
-                   if (sym->is_argument ())
-                     arg_sym = sym;
-                   else
-                     {
-                       found_sym = true;
-                       add_defn_to_vec (result, sym, block);
-                     }
-                 }
+                 add_defn_to_vec (result, sym, block);
              }
          }
       }
-
-      /* NOTE: This really shouldn't be needed for _ada_ symbols.
-        They aren't parameters, right?  */
-      if (!found_sym && arg_sym != NULL)
-       {
-         add_defn_to_vec (result, arg_sym, block);
-       }
     }
 }
 \f
index e7424c52affde2414981cb0f781ac3594b44c978..ae9af08acb81f0ec655e68e81dd95a31b8958705 100644 (file)
@@ -640,38 +640,9 @@ struct symbol *
 block_lookup_symbol (const struct block *block, const lookup_name_info &name,
                     const domain_search_flags domain)
 {
-  if (!block->function ())
-    {
-      best_symbol_tracker tracker;
-      tracker.search (nullptr, block, name, domain);
-      return tracker.currently_best.symbol;
-    }
-  else
-    {
-      /* Note that parameter symbols do not always show up last in the
-        list; this loop makes sure to take anything else other than
-        parameter symbols first; it only uses parameter symbols as a
-        last resort.  Note that this only takes up extra computation
-        time on a match.
-        It's hard to define types in the parameter list (at least in
-        C/C++) so we don't do the same PR 16253 hack here that is done
-        for the !BLOCK_FUNCTION case.  */
-
-      struct symbol *sym_found = NULL;
-
-      for (struct symbol *sym : block_iterator_range (block, &name))
-       {
-         if (sym->matches (domain))
-           {
-             sym_found = sym;
-             if (!sym->is_argument ())
-               {
-                 break;
-               }
-           }
-       }
-      return (sym_found);      /* Will be NULL if not found.  */
-    }
+  best_symbol_tracker tracker;
+  tracker.search (nullptr, block, name, domain);
+  return tracker.currently_best.symbol;
 }
 
 /* See block.h.  */
index f2a4b3044b0f873eb85e23db51a1d786a2f62a5e..3281be2b4dcc60a6a0a4b5b506ebd96f4811862a 100644 (file)
@@ -633,34 +633,25 @@ list_args_or_locals (const frame_print_options &fp_opts,
            }
          if (print_me)
            {
-             struct symbol *sym2;
              struct frame_arg arg, entryarg;
 
-             if (sym->is_argument ())
-               sym2 = (lookup_symbol_search_name
-                       (sym->search_name (),
-                        block, SEARCH_VAR_DOMAIN).symbol);
-             else
-               sym2 = sym;
-             gdb_assert (sym2 != NULL);
-
-             arg.sym = sym2;
+             arg.sym = sym;
              arg.entry_kind = print_entry_values_no;
-             entryarg.sym = sym2;
+             entryarg.sym = sym;
              entryarg.entry_kind = print_entry_values_no;
 
              switch (values)
                {
                case PRINT_SIMPLE_VALUES:
-                 if (!mi_simple_type_p (sym2->type ()))
+                 if (!mi_simple_type_p (sym->type ()))
                    break;
                  [[fallthrough]];
 
                case PRINT_ALL_VALUES:
                  if (sym->is_argument ())
-                   read_frame_arg (fp_opts, sym2, fi, &arg, &entryarg);
+                   read_frame_arg (fp_opts, sym, fi, &arg, &entryarg);
                  else
-                   read_frame_local (sym2, fi, &arg);
+                   read_frame_local (sym, fi, &arg);
                  break;
                }
 
index 6fcc26417e29dafaef53c880ddf8398ff97260c5..7329430adab4288b1dafbf7a834a374b0d187beb 100644 (file)
@@ -801,70 +801,6 @@ print_frame_args (const frame_print_options &fp_opts,
              break;
            }
 
-         /* We have to look up the symbol because arguments can have
-            two entries (one a parameter, one a local) and the one we
-            want is the local, which lookup_symbol will find for us.
-            This includes gcc1 (not gcc2) on SPARC when passing a
-            small structure and gcc2 when the argument type is float
-            and it is passed as a double and converted to float by
-            the prologue (in the latter case the type of the LOC_ARG
-            symbol is double and the type of the LOC_LOCAL symbol is
-            float).  */
-         /* But if the parameter name is null, don't try it.  Null
-            parameter names occur on the RS/6000, for traceback
-            tables.  FIXME, should we even print them?  */
-
-         if (*sym->linkage_name ())
-           {
-             struct symbol *nsym;
-
-             nsym = lookup_symbol_search_name (sym->search_name (),
-                                               b, SEARCH_VAR_DOMAIN).symbol;
-             gdb_assert (nsym != NULL);
-             if (nsym->loc_class () == LOC_REGISTER
-                 && !nsym->is_argument ())
-               {
-                 /* There is a LOC_ARG/LOC_REGISTER pair.  This means
-                    that it was passed on the stack and loaded into a
-                    register, or passed in a register and stored in a
-                    stack slot.  GDB 3.x used the LOC_ARG; GDB
-                    4.0-4.11 used the LOC_REGISTER.
-
-                    Reasons for using the LOC_ARG:
-
-                    (1) Because find_saved_registers may be slow for
-                        remote debugging.
-
-                    (2) Because registers are often reused and stack
-                        slots rarely (never?) are.  Therefore using
-                        the stack slot is much less likely to print
-                        garbage.
-
-                    Reasons why we might want to use the LOC_REGISTER:
-
-                    (1) So that the backtrace prints the same value
-                        as "print foo".  I see no compelling reason
-                        why this needs to be the case; having the
-                        backtrace print the value which was passed
-                        in, and "print foo" print the value as
-                        modified within the called function, makes
-                        perfect sense to me.
-
-                    Additional note: It might be nice if "info args"
-                    displayed both values.
-
-                    One more note: There is a case with SPARC
-                    structure passing where we need to use the
-                    LOC_REGISTER, but this is dealt with by creating
-                    a single LOC_REGPARM in symbol reading.  */
-
-                 /* Leave sym (the LOC_ARG) alone.  */
-                 ;
-               }
-             else
-               sym = nsym;
-           }
-
          /* Print the current arg.  */
          if (!first)
            uiout->text (", ");
@@ -2446,23 +2382,7 @@ iterate_over_block_arg_vars (const struct block *b,
     {
       /* Don't worry about things which aren't arguments.  */
       if (sym->is_argument ())
-       {
-         /* We have to look up the symbol because arguments can have
-            two entries (one a parameter, one a local) and the one we
-            want is the local, which lookup_symbol will find for us.
-            This includes gcc1 (not gcc2) on the sparc when passing a
-            small structure and gcc2 when the argument type is float
-            and it is passed as a double and converted to float by
-            the prologue (in the latter case the type of the LOC_ARG
-            symbol is double and the type of the LOC_LOCAL symbol is
-            float).  There are also LOC_ARG/LOC_REGISTER pairs which
-            are not combined in symbol-reading.  */
-
-         struct symbol *sym2
-           = lookup_symbol_search_name (sym->search_name (),
-                                        b, SEARCH_VAR_DOMAIN).symbol;
-         cb (sym->print_name (), sym2);
-       }
+       cb (sym->print_name (), sym);
     }
 }