diff mbox

Make info_symbol_command use lookup_minimal_symbol_by_pc

Message ID 20190805201902.15900-1-cbiesinger@google.com
State New, archived
Headers

Commit Message

Terekhov, Mikhail via Gdb-patches Aug. 5, 2019, 8:19 p.m. UTC 
  Currently it reimplements that functionality.

Tested with the following two tests, which seem to be the only non-Cell
tests that use "info symbol":
gdb.base/default.exp gdb.asm/asm-source.exp

gdb/ChangeLog:

2019-08-05  Christian Biesinger  <cbiesinger@google.com>

	* printcmd.c (info_symbol_command): Use lookup_minimal_symbol_by_pc
	instead of reimplementing that functionality.
---
 gdb/printcmd.c | 146 +++++++++++++++++++++++--------------------------
 1 file changed, 67 insertions(+), 79 deletions(-)

Comments

Kevin Buettner Aug. 6, 2019, 7:51 p.m. UTC | #1 
On Mon,  5 Aug 2019 15:19:02 -0500
"Christian Biesinger via gdb-patches" <gdb-patches@sourceware.org> wrote:

> -  for (objfile *objfile : current_program_space->objfiles ())
> -    ALL_OBJFILE_OSECTIONS (objfile, osect)
> -      {
...

Can it ever happen that symbols in either different objfiles and/or
sections will have the same address?  I.e. some sort of alias?
Perhaps a weak symbol?

If so, it seems to me that the deleted code will print all such
symbols, whereas your proposed change will only print one of them.

I don't know if this ever happens or if it's an important case if it
should happen, but I'm wondering if you considered this possibility.

Kevin
Terekhov, Mikhail via Gdb-patches Aug. 6, 2019, 9:32 p.m. UTC | #2 
On Tue, Aug 6, 2019 at 2:51 PM Kevin Buettner <kevinb@redhat.com> wrote:
>
> On Mon,  5 Aug 2019 15:19:02 -0500
> "Christian Biesinger via gdb-patches" <gdb-patches@sourceware.org> wrote:
>
> > -  for (objfile *objfile : current_program_space->objfiles ())
> > -    ALL_OBJFILE_OSECTIONS (objfile, osect)
> > -      {
> ...
>
> Can it ever happen that symbols in either different objfiles and/or
> sections will have the same address?  I.e. some sort of alias?
> Perhaps a weak symbol?
>
> If so, it seems to me that the deleted code will print all such
> symbols, whereas your proposed change will only print one of them.
>
> I don't know if this ever happens or if it's an important case if it
> should happen, but I'm wondering if you considered this possibility.

That's a good question and I don't know the answer. I did not consider
that when writing this patch. I'll leave that to the gdb/symbol
experts on this list...

Christian
diff mbox

Patch

diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index 0c368a6f6d..1d7483c62a 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -1306,93 +1306,81 @@  set_command (const char *exp, int from_tty)
 static void
 info_symbol_command (const char *arg, int from_tty)
 {
-  struct minimal_symbol *msymbol;
+  bound_minimal_symbol msymbol;
   struct obj_section *osect;
   CORE_ADDR addr, sect_addr;
-  int matches = 0;
   unsigned int offset;
 
   if (!arg)
     error_no_arg (_("address"));
 
   addr = parse_and_eval_address (arg);
-  for (objfile *objfile : current_program_space->objfiles ())
-    ALL_OBJFILE_OSECTIONS (objfile, osect)
-      {
-	/* Only process each object file once, even if there's a separate
-	   debug file.  */
-	if (objfile->separate_debug_objfile_backlink)
-	  continue;
-
-	sect_addr = overlay_mapped_address (addr, osect);
-
-	if (obj_section_addr (osect) <= sect_addr
-	    && sect_addr < obj_section_endaddr (osect)
-	    && (msymbol
-		= lookup_minimal_symbol_by_pc_section (sect_addr,
-						       osect).minsym))
-	  {
-	    const char *obj_name, *mapped, *sec_name, *msym_name;
-	    const char *loc_string;
-
-	    matches = 1;
-	    offset = sect_addr - MSYMBOL_VALUE_ADDRESS (objfile, msymbol);
-	    mapped = section_is_mapped (osect) ? _("mapped") : _("unmapped");
-	    sec_name = osect->the_bfd_section->name;
-	    msym_name = MSYMBOL_PRINT_NAME (msymbol);
-
-	    /* Don't print the offset if it is zero.
-	       We assume there's no need to handle i18n of "sym + offset".  */
-	    std::string string_holder;
-	    if (offset)
-	      {
-		string_holder = string_printf ("%s + %u", msym_name, offset);
-		loc_string = string_holder.c_str ();
-	      }
-	    else
-	      loc_string = msym_name;
-
-	    gdb_assert (osect->objfile && objfile_name (osect->objfile));
-	    obj_name = objfile_name (osect->objfile);
-
-	    if (MULTI_OBJFILE_P ())
-	      if (pc_in_unmapped_range (addr, osect))
-		if (section_is_overlay (osect))
-		  printf_filtered (_("%s in load address range of "
-				     "%s overlay section %s of %s\n"),
-				   loc_string, mapped, sec_name, obj_name);
-		else
-		  printf_filtered (_("%s in load address range of "
-				     "section %s of %s\n"),
-				   loc_string, sec_name, obj_name);
-	      else
-		if (section_is_overlay (osect))
-		  printf_filtered (_("%s in %s overlay section %s of %s\n"),
-				   loc_string, mapped, sec_name, obj_name);
-		else
-		  printf_filtered (_("%s in section %s of %s\n"),
-				   loc_string, sec_name, obj_name);
-	    else
-	      if (pc_in_unmapped_range (addr, osect))
-		if (section_is_overlay (osect))
-		  printf_filtered (_("%s in load address range of %s overlay "
-				     "section %s\n"),
-				   loc_string, mapped, sec_name);
-		else
-		  printf_filtered
-		    (_("%s in load address range of section %s\n"),
-		     loc_string, sec_name);
-	      else
-		if (section_is_overlay (osect))
-		  printf_filtered (_("%s in %s overlay section %s\n"),
-				   loc_string, mapped, sec_name);
-		else
-		  printf_filtered (_("%s in section %s\n"),
-				   loc_string, sec_name);
-	  }
-      }
-  if (matches == 0)
-    printf_filtered (_("No symbol matches %s.\n"), arg);
+  msymbol = lookup_minimal_symbol_by_pc (addr);
+  if (msymbol.minsym == nullptr)
+    {
+      printf_filtered (_("No symbol matches %s.\n"), arg);
+      return;
+    }
+  osect = MSYMBOL_OBJ_SECTION (msymbol.objfile, msymbol.minsym);
+
+  sect_addr = overlay_mapped_address (addr, osect);
+
+  const char *obj_name, *mapped, *sec_name, *msym_name;
+  const char *loc_string;
+
+  offset = sect_addr - MSYMBOL_VALUE_ADDRESS (msymbol.objfile, msymbol.minsym);
+  mapped = section_is_mapped (osect) ? _("mapped") : _("unmapped");
+  sec_name = osect->the_bfd_section->name;
+  msym_name = MSYMBOL_PRINT_NAME (msymbol.minsym);
+
+  /* Don't print the offset if it is zero.
+     We assume there's no need to handle i18n of "sym + offset".  */
+  std::string string_holder;
+  if (offset)
+  {
+    string_holder = string_printf ("%s + %u", msym_name, offset);
+    loc_string = string_holder.c_str ();
+  }
+  else
+    loc_string = msym_name;
+
+  gdb_assert (osect->objfile && objfile_name (osect->objfile));
+  obj_name = objfile_name (osect->objfile);
+
+  if (MULTI_OBJFILE_P ())
+    if (pc_in_unmapped_range (addr, osect))
+      if (section_is_overlay (osect))
+	printf_filtered (_("%s in load address range of "
+	      "%s overlay section %s of %s\n"),
+	    loc_string, mapped, sec_name, obj_name);
+      else
+	printf_filtered (_("%s in load address range of "
+	      "section %s of %s\n"),
+	    loc_string, sec_name, obj_name);
+    else
+      if (section_is_overlay (osect))
+	printf_filtered (_("%s in %s overlay section %s of %s\n"),
+	    loc_string, mapped, sec_name, obj_name);
+      else
+	printf_filtered (_("%s in section %s of %s\n"),
+	    loc_string, sec_name, obj_name);
+  else
+    if (pc_in_unmapped_range (addr, osect))
+      if (section_is_overlay (osect))
+	printf_filtered (_("%s in load address range of %s overlay "
+	      "section %s\n"),
+	    loc_string, mapped, sec_name);
+      else
+	printf_filtered
+	  (_("%s in load address range of section %s\n"),
+	   loc_string, sec_name);
+    else
+      if (section_is_overlay (osect))
+	printf_filtered (_("%s in %s overlay section %s\n"),
+	    loc_string, mapped, sec_name);
+      else
+	printf_filtered (_("%s in section %s\n"),
+	    loc_string, sec_name);
 }
 
 static void