Commit: Extend objdump's --dwarf=follow-links option

Message ID 8736ocnjyb.fsf@redhat.com
State New
Headers show
Series
  • Commit: Extend objdump's --dwarf=follow-links option
Related show

Commit Message

Nick Clifton Feb. 25, 2019, 12:14 p.m.
Hi Guys,

  I am checking in the attached patch to extend objdump's
  --dwarf=follow-link option.  With this extension any dumping options,
  eg --syms, will also be applied to any linked debug information files.

  In addition any symbol tables in linked debug info files will be read
  in and used when disassembling.  This means that the disassembly
  should be more useful.  For example, compare:

    % objdump -d /usr/bin/xz
    [...]
    Disassembly of section .text:

    00000000000036e0 <.text>:
       36e0:	f3 0f 1e fa          	endbr64 
       36e4:	41 56                	push   %r14
    [...]

  with:
  
    % objdump -d --dwarf=follow-links /usr/bin/xz
    [...]
    Disassembly of section .text:

    00000000000036e0 <main>:
       36e0:	f3 0f 1e fa          	endbr64 
       36e4:	41 56                	push   %r14
       36e6:	41 55                	push   %r13
    [...]

Cheers
  Nick

binutils/ChangeLog
2019-02-25  Nick Clifton  <nickc@redhat.com>

	* objdump.c (sym_ok): New function.
	(find_symbol_for_address): Use new function.
	(disassemble_section): Compare sections by name, not pointer.
	(dump_dwarf): Move code to initialise byte_get pointer and iterate
	over separate debug files from here to ...
	(dump_bfd): ... here.  Add parameter indicating that a separate
	debug info file is being dumped.  For main file, pull in the
	symbol tables from all separate debug info files.
	(display_object): Update call to dump_bfd.
	* doc/binutils.texi: Document extened behaviour of the
	--dwarf=follow-links option.
	* NEWS: Mention this new feature.
	* testsuite/binutils-all/objdump.WK2: Update expected output.
	* testsuite/binutils-all/objdump.exp (test_follow_debuglink): Add
	options and dump file parameters.
	Add extra test.
	* testsuite/binutils-all/objdump.WK3: New file.
	* testsuite/binutils-all/readelf.exp: Change expected output for
	readelf -wKis test.
	* testsuite/binutils-all/readelf.wKis: New file.

Patch

2019-02-25  Nick Clifton  <nickc@redhat.com>

	* objdump.c (sym_ok): New function.
	(find_symbol_for_address): Use new function.
	(disassemble_section): Compare sections by name, not pointer.
	(dump_dwarf): Move code to initialise byte_get pointer and iterate
	over separate debug files from here to ...
	(dump_bfd): ... here.  Add parameter indicating that a separate
	debug info file is being dumped.  For main file, pull in the
	symbol tables from all separate debug info files.
	(display_object): Update call to dump_bfd.
	* doc/binutils.texi: Document extened behaviour of the
	--dwarf=follow-links option.
	* NEWS: Mention this new feature.
	* testsuite/binutils-all/objdump.WK2: Update expected output.
	* testsuite/binutils-all/objdump.exp (test_follow_debuglink): Add
	options and dump file parameters.
	Add extra test.
	* testsuite/binutils-all/objdump.WK3: New file.
	* testsuite/binutils-all/readelf.exp: Change expected output for
	readelf -wKis test.
	* testsuite/binutils-all/readelf.wKis: New file.

diff --git a/binutils/NEWS b/binutils/NEWS
index 5413321b4b..7c9d7bef30 100644
--- a/binutils/NEWS
+++ b/binutils/NEWS
@@ -6,6 +6,13 @@ 
   more than one are present in a file.  (This usually happens when gcc's
   -gsplit-dwarf option is used).
 
+  In addition objdump's --dwarf=follow-links now also affects its other
+  display options, so that for example, when combined with --syms it will
+  cause the symbol tables in any linked debug info files to also be
+  displayed.  In addition when combined with --disassemble the --dwarf=
+  follow-links option will ensure that any symbol tables in the linked
+  files are read and used when disassembling code in the main file.
+  
 Changes in 2.32:
 
 * The addr2line, c++filt, nm and objdump tools now have a limit on the
diff --git a/binutils/doc/binutils.texi b/binutils/doc/binutils.texi
index 01f1e5f56d..eb5c3e87a3 100644
--- a/binutils/doc/binutils.texi
+++ b/binutils/doc/binutils.texi
@@ -2236,6 +2236,10 @@  will stop at the end of the function, otherwise it will stop when the
 next symbol is encountered.  If there are no matches for @var{symbol}
 then nothing will be displayed.
 
+Note if the @option{--dwarf=follow-links} option has also been enabled
+then any symbol tables in linked debug info files will be read in and
+used when disassembling.
+
 @item -D
 @itemx --disassemble-all
 Like @option{-d}, but disassemble the contents of all sections, not just
@@ -2254,6 +2258,10 @@  If the target is an ARM architecture this switch also has the effect
 of forcing the disassembler to decode pieces of data found in code
 sections as if they were instructions.
 
+Note if the @option{--dwarf=follow-links} option has also been enabled
+then any symbol tables in linked debug info files will be read in and
+used when disassembling.
+
 @item --prefix-addresses
 When disassembling, print the complete address on each line.  This is
 the older disassembly format.
diff --git a/binutils/dwarf.c b/binutils/dwarf.c
index 07142af747..9f17af9ff8 100644
--- a/binutils/dwarf.c
+++ b/binutils/dwarf.c
@@ -9826,6 +9826,7 @@  parse_gnu_debuglink (struct dwarf_section * section, void * data)
      The CRC value is stored after the filename, aligned up to 4 bytes.  */
   name = (const char *) section->start;
 
+  
   crc_offset = strnlen (name, section->size) + 1;
   crc_offset = (crc_offset + 3) & ~3;
   if (crc_offset + 4 > section->size)
@@ -9981,6 +9982,11 @@  load_separate_debug_info (const char *            main_filename,
   sprintf (debug_filename, "%s/%s", EXTRA_DEBUG_ROOT1, separate_filename);
   if (check_func (debug_filename, func_data))
     goto found;
+
+  /* Try the first extra debug file root.  */
+  sprintf (debug_filename, "%s/%s/%s", EXTRA_DEBUG_ROOT1, canon_dir, separate_filename);
+  if (check_func (debug_filename, func_data))
+    goto found;
 #endif
 
 #ifdef EXTRA_DEBUG_ROOT2
@@ -10010,6 +10016,9 @@  load_separate_debug_info (const char *            main_filename,
 #endif
 
 #ifdef EXTRA_DEBUG_ROOT1
+  sprintf (debug_filename, "%s/%s/%s", EXTRA_DEBUG_ROOT1, canon_dir, separate_filename);
+  warn (_("tried: %s\n"), debug_filename);
+
   sprintf (debug_filename, "%s/%s", EXTRA_DEBUG_ROOT1, separate_filename);
   warn (_("tried: %s\n"), debug_filename);
 #endif
diff --git a/binutils/dwarf.h b/binutils/dwarf.h
index a0287fc0bc..ca2f062efb 100644
--- a/binutils/dwarf.h
+++ b/binutils/dwarf.h
@@ -218,6 +218,7 @@  extern int do_debug_addr;
 extern int do_debug_cu_index;
 extern int do_wide;
 extern int do_debug_links;
+extern int do_follow_links;
 
 extern int dwarf_cutoff_level;
 extern unsigned long dwarf_start_die;
diff --git a/binutils/objdump.c b/binutils/objdump.c
index 19365b67a0..4a4272b113 100644
--- a/binutils/objdump.c
+++ b/binutils/objdump.c
@@ -987,6 +987,30 @@  objdump_print_symname (bfd *abfd, struct disassemble_info *inf,
     free (alloc);
 }
 
+static inline bfd_boolean
+sym_ok (bfd_boolean               want_section,
+	bfd *                     abfd ATTRIBUTE_UNUSED,
+	long                      place,
+	asection *                sec,
+	struct disassemble_info * inf)
+{
+  if (want_section)
+    {
+      /* Note - we cannot just compare section pointers because they could
+	 be different, but the same...  Ie the symbol that we are trying to
+	 find could have come from a separate debug info file.  Under such
+	 circumstances the symbol will be associated with a section in the
+	 debug info file, whilst the section we want is in a normal file.
+	 So the section pointers will be different, but the section names
+	 will be the same.  */
+      if (strcmp (bfd_section_name (abfd, sorted_syms[place]->section),
+		  bfd_section_name (abfd, sec)) != 0)
+	return FALSE;
+    }
+
+  return inf->symbol_is_valid (sorted_syms[place], inf);
+}
+
 /* Locate a symbol given a bfd and a section (from INFO->application_data),
    and a VMA.  If INFO->application_data->require_sec is TRUE, then always
    require the symbol to be in the section.  Returns NULL if there is no
@@ -1062,8 +1086,7 @@  find_symbol_for_address (bfd_vma vma,
 	 && (bfd_asymbol_value (sorted_syms[min])
 	     == bfd_asymbol_value (sorted_syms[thisplace])))
     {
-      if (sorted_syms[min]->section == sec
-	  && inf->symbol_is_valid (sorted_syms[min], inf))
+      if (sym_ok (TRUE, abfd, min, sec, inf))
 	{
 	  thisplace = min;
 
@@ -1090,16 +1113,15 @@  find_symbol_for_address (bfd_vma vma,
 		      && vma >= bfd_get_section_vma (abfd, sec)
 		      && vma < (bfd_get_section_vma (abfd, sec)
 				+ bfd_section_size (abfd, sec) / opb)));
-  if ((sorted_syms[thisplace]->section != sec && want_section)
-      || ! inf->symbol_is_valid (sorted_syms[thisplace], inf))
+  
+  if (! sym_ok (want_section, abfd, thisplace, sec, inf))
     {
       long i;
       long newplace = sorted_symcount;
 
       for (i = min - 1; i >= 0; i--)
 	{
-	  if ((sorted_syms[i]->section == sec || !want_section)
-	      && inf->symbol_is_valid (sorted_syms[i], inf))
+	  if (sym_ok (want_section, abfd, i, sec, inf))
 	    {
 	      if (newplace == sorted_symcount)
 		newplace = i;
@@ -1122,8 +1144,7 @@  find_symbol_for_address (bfd_vma vma,
 	     Look for one with a larger value.  */
 	  for (i = thisplace + 1; i < sorted_symcount; i++)
 	    {
-	      if ((sorted_syms[i]->section == sec || !want_section)
-		  && inf->symbol_is_valid (sorted_syms[i], inf))
+	      if (sym_ok (want_section, abfd, i, sec, inf))
 		{
 		  thisplace = i;
 		  break;
@@ -1131,8 +1152,7 @@  find_symbol_for_address (bfd_vma vma,
 	    }
 	}
 
-      if ((sorted_syms[thisplace]->section != sec && want_section)
-	  || ! inf->symbol_is_valid (sorted_syms[thisplace], inf))
+      if (! sym_ok (want_section, abfd, thisplace, sec, inf))
 	/* There is no suitable symbol.  */
 	return NULL;
     }
@@ -2460,7 +2480,7 @@  disassemble_section (bfd *abfd, asection *section, void *inf)
       else
 	{
 #define is_valid_next_sym(SYM) \
-  ((SYM)->section == section \
+  (strcmp (bfd_section_name (abfd, (SYM)->section), bfd_section_name (abfd, section)) == 0 \
    && (bfd_asymbol_value (SYM) > bfd_asymbol_value (sym)) \
    && pinfo->symbol_is_valid (SYM, pinfo))
 
@@ -2882,24 +2902,18 @@  dump_dwarf_section (bfd *abfd, asection *section,
 static void
 dump_dwarf (bfd *abfd)
 {
-  bfd_boolean have_separates;
-
-  is_relocatable = (abfd->flags & (EXEC_P | DYNAMIC)) == 0;
-
-  eh_addr_size = bfd_arch_bits_per_address (abfd) / 8;
-
-  if (bfd_big_endian (abfd))
-    byte_get = byte_get_big_endian;
-  else if (bfd_little_endian (abfd))
-    byte_get = byte_get_little_endian;
-  else
-    /* PR 17512: file: objdump-s-endless-loop.tekhex.  */
+  /* The byte_get pointer should have been set at the start of dump_bfd().  */
+  if (byte_get == NULL)
     {
       warn (_("File %s does not contain any dwarf debug information\n"),
 	    bfd_get_filename (abfd));
       return;
     }
 
+  is_relocatable = (abfd->flags & (EXEC_P | DYNAMIC)) == 0;
+
+  eh_addr_size = bfd_arch_bits_per_address (abfd) / 8;
+
   switch (bfd_get_arch (abfd))
     {
     case bfd_arch_i386:
@@ -2946,21 +2960,7 @@  dump_dwarf (bfd *abfd)
       break;
     }
 
-  have_separates = load_separate_debug_files (abfd, bfd_get_filename (abfd));
-
   bfd_map_over_sections (abfd, dump_dwarf_section, NULL);
-
-  if (have_separates)
-    {
-      separate_info * i;
-
-      for (i = first_separate_info; i != NULL; i = i->next)
-	bfd_map_over_sections (i->handle, dump_dwarf_section, NULL);
-
-      /* The file handles are closed by the call to free_debug_memory() below.  */
-    }
-
-  free_debug_memory ();
 }
 
 /* Read ABFD's stabs section STABSECT_NAME, and return a pointer to
@@ -3768,8 +3768,38 @@  adjust_addresses (bfd *abfd ATTRIBUTE_UNUSED,
 /* Dump selected contents of ABFD.  */
 
 static void
-dump_bfd (bfd *abfd)
+dump_bfd (bfd *abfd, bfd_boolean is_toplevel)
 {
+  if (bfd_big_endian (abfd))
+    byte_get = byte_get_big_endian;
+  else if (bfd_little_endian (abfd))
+    byte_get = byte_get_little_endian;
+  else
+    byte_get = NULL;
+
+  /* Load any separate debug information files.
+     We do this now and without checking do_follow_links because separate
+     debug info files may contain symbol tables that we will need when
+     displaying information about the main file.  Any memory allocated by
+     load_separate_debug_files will be released when we call
+     free_debug_memory below.
+     
+     The test on is_toplevel is there because the chain of separate debug
+     info files is a global variable shared by all invocations of dump_bfd.  */
+  if (is_toplevel)
+    {
+      load_separate_debug_files (abfd, bfd_get_filename (abfd));
+
+      /* If asked to do so, recursively dump the separate files.  */
+      if (do_follow_links)
+	{
+	  separate_info * i;
+
+	  for (i = first_separate_info; i != NULL; i = i->next)
+	    dump_bfd (i->handle, FALSE);
+	}
+    }
+
   /* If we are adjusting section VMA's, change them all now.  Changing
      the BFD information is a hack.  However, we must do it, or
      bfd_find_nearest_line will not do the right thing.  */
@@ -3799,7 +3829,40 @@  dump_bfd (bfd *abfd)
       || disassemble
       || dump_debugging
       || dump_dwarf_section_info)
-    syms = slurp_symtab (abfd);
+    {
+      syms = slurp_symtab (abfd);
+
+      /* If following links, load any symbol tables from the linked files as well.  */
+      if (do_follow_links && is_toplevel)
+	{
+	  separate_info * i;
+
+	  for (i = first_separate_info; i != NULL; i = i->next)
+	    {
+	      asymbol **  extra_syms;
+	      long        old_symcount = symcount;
+	      
+	      extra_syms = slurp_symtab (i->handle);
+
+	      if (extra_syms)
+		{
+		  if (old_symcount == 0)
+		    {
+		      syms = extra_syms;
+		    }
+		  else
+		    {
+		      syms = xrealloc (syms, (symcount + old_symcount) * sizeof (asymbol *));
+		      memcpy (syms + old_symcount,
+			      extra_syms,
+			      symcount * sizeof (asymbol *));
+		    }
+		}
+
+	      symcount += old_symcount;
+	    }
+	}
+    }
 
   if (dump_section_headers)
     dump_headers (abfd);
@@ -3807,6 +3870,7 @@  dump_bfd (bfd *abfd)
   if (dump_dynamic_symtab || dump_dynamic_reloc_info
       || (disassemble && bfd_get_dynamic_symtab_upper_bound (abfd) > 0))
     dynsyms = slurp_dynamic_symtab (abfd);
+
   if (disassemble)
     {
       synthcount = bfd_get_synthetic_symtab (abfd, symcount, syms,
@@ -3880,6 +3944,9 @@  dump_bfd (bfd *abfd)
   symcount = 0;
   dynsymcount = 0;
   synthcount = 0;
+
+  if (is_toplevel)
+    free_debug_memory ();
 }
 
 static void
@@ -3889,7 +3956,7 @@  display_object_bfd (bfd *abfd)
 
   if (bfd_check_format_matches (abfd, bfd_object, &matching))
     {
-      dump_bfd (abfd);
+      dump_bfd (abfd, TRUE);
       return;
     }
 
@@ -3909,7 +3976,7 @@  display_object_bfd (bfd *abfd)
 
   if (bfd_check_format_matches (abfd, bfd_core, &matching))
     {
-      dump_bfd (abfd);
+      dump_bfd (abfd, TRUE);
       return;
     }
 
diff --git a/binutils/testsuite/binutils-all/objdump.WK2 b/binutils/testsuite/binutils-all/objdump.WK2
index c98fdc15b5..539a0875cf 100644
--- a/binutils/testsuite/binutils-all/objdump.WK2
+++ b/binutils/testsuite/binutils-all/objdump.WK2
@@ -1,5 +1,11 @@ 
 #...
 .*debuglink.o: Found separate debug info file:.*linkdebug.debug
+#...
+Contents of the .debug_str section \(loaded from .*linkdebug.debug\):
+
+  0x00000000 73747269 6e672d33 00737472 696e672d string-3.string-
+  0x00000010 3400                                4.
+
 #...
 Contents of the .debug_str section \(loaded from .*debuglink.o\):
 
@@ -17,9 +23,4 @@  Contents of the .debug_info section \(loaded from .*debuglink.o\):
     <c>   DW_AT_name        : \(indirect string, offset: 0x0\): string-1
  <0><10>: Abbrev Number: 2 \(DW_TAG_subprogram\)
     <11>   DW_AT_name        : \(alt indirect string, offset: 0x0\) string-3
-
-Contents of the .debug_str section \(loaded from .*linkdebug.debug\):
-
-  0x00000000 73747269 6e672d33 00737472 696e672d string-3.string-
-  0x00000010 3400                                4.
-
+#pass
diff --git a/binutils/testsuite/binutils-all/objdump.exp b/binutils/testsuite/binutils-all/objdump.exp
index dd2e9bb02d..81a061f70c 100644
--- a/binutils/testsuite/binutils-all/objdump.exp
+++ b/binutils/testsuite/binutils-all/objdump.exp
@@ -623,12 +623,12 @@  if { [is_elf_format] } then {
 # Very similar to proc test_build_id_debuglink except this time we
 # display some of the contents of the separate debug info file.
 
-proc test_follow_debuglink {} {
+proc test_follow_debuglink { options dumpfile } {
     global srcdir
     global subdir
     global OBJDUMP
     
-    set test "follow-debuglink"
+    set test "follow-debuglink ($options)"
 
     if {![binutils_assemble $srcdir/$subdir/debuglink.s tmpdir/debuglink.o]} then {
 	fail "$test (reason: assemble first source file)"
@@ -647,13 +647,13 @@  proc test_follow_debuglink {} {
 	set tempfile [remote_download host tmpdir/debuglink.o]
     }
     
-    set got [remote_exec host "$OBJDUMP --dwarf=follow-links --dwarf=info --dwarf=str $tempfile" "" "/dev/null" "tmpdir/objdump.out"]
+    set got [remote_exec host "$OBJDUMP $options $tempfile" "" "/dev/null" "tmpdir/objdump.out"]
     if { [lindex $got 0] != 0 || ![string match "" [lindex $got 1]] } then {
 	fail "$test (reason: unexpected error output from objdump)"
 	return
     }
 
-    if { [regexp_diff tmpdir/objdump.out $srcdir/$subdir/objdump.WK2] } then {
+    if { [regexp_diff tmpdir/objdump.out $srcdir/$subdir/$dumpfile] } then {
 	fail $test
 	verbose "output is \n[file_contents objdump.out]" 2
 	return
@@ -668,7 +668,8 @@  proc test_follow_debuglink {} {
 }
 
 if {[is_elf_format]} then {
-    test_follow_debuglink
+    test_follow_debuglink "--dwarf=follow-links --dwarf=info --dwarf=str" objdump.WK2
+    test_follow_debuglink "--dwarf=follow-links --headers --wide" objdump.WK3
 }
 
 # Options which are not tested: -a -D -R -T -x -l --stabs
diff --git a/binutils/testsuite/binutils-all/readelf.exp b/binutils/testsuite/binutils-all/readelf.exp
index e41e22bf26..0d9a42fba8 100644
--- a/binutils/testsuite/binutils-all/readelf.exp
+++ b/binutils/testsuite/binutils-all/readelf.exp
@@ -489,7 +489,7 @@  if {![binutils_assemble $srcdir/$subdir/debuglink.s tmpdir/debuglink.o]} then {
 	    set tempfile2 [remote_download host tmpdir/linkdebug.debug]
 	}
 
-	readelf_test {-wKis} $tempfile objdump.WK2  {}
+	readelf_test {-wKis} $tempfile readelf.wKis  {}
     }
 }
 
--- /dev/null	2019-02-25 07:52:13.823999211 +0000
+++ binutils/testsuite/binutils-all/readelf.wKis	2019-02-25 09:59:41.316375339 +0000
@@ -0,0 +1,25 @@ 
+#...
+.*debuglink.o: Found separate debug info file:.*linkdebug.debug
+#...
+Contents of the .debug_str section \(loaded from .*debuglink.o\):
+
+  0x00000000 73747269 6e672d31 00737472 696e672d string-1.string-
+  0x00000010 3200                                2.
+#...
+Contents of the .debug_info section \(loaded from .*debuglink.o\):
+
+  Compilation Unit @ offset 0x0:
+   Length:        0x12 \(32-bit\)
+   Version:       4
+   Abbrev Offset: 0x0
+   Pointer Size:  4
+ <0><b>: Abbrev Number: 1 \(DW_TAG_compile_unit\)
+    <c>   DW_AT_name        : \(indirect string, offset: 0x0\): string-1
+ <0><10>: Abbrev Number: 2 \(DW_TAG_subprogram\)
+    <11>   DW_AT_name        : \(alt indirect string, offset: 0x0\) string-3
+#...
+Contents of the .debug_str section \(loaded from .*linkdebug.debug\):
+
+  0x00000000 73747269 6e672d33 00737472 696e672d string-3.string-
+  0x00000010 3400                                4.
+#pass
--- /dev/null	2019-02-25 07:52:13.823999211 +0000
+++ binutils/testsuite/binutils-all/objdump.WK3	2019-02-25 11:08:53.231571864 +0000
@@ -0,0 +1,17 @@ 
+#...
+.*debuglink.o: Found separate debug info file:.*linkdebug.debug
+#...
+.*linkdebug.debug:.*
+#...
+ .* .debug_abbrev .*
+ .* .debug_str .*
+#...
+.*debuglink.o:.*
+#...
+ .* .gnu_debuglink .*
+ .* .gnu_debugaltlink .*
+ .* .debug_str .*
+ .* .debug_info .*
+#...
+ .* .debug_line .*
+#pass