objdump: don't cache section contents in load_specific_debug_section

Message ID 20210215041435.GL5348@bubble.grove.modra.org
State New
Headers show
Series
  • objdump: don't cache section contents in load_specific_debug_section
Related show

Commit Message

H.J. Lu via Binutils Feb. 15, 2021, 4:14 a.m.
This finally gets to the point where I see no regressions from
enabling readelf and objdump to automatically follow links to separate
debug info files.

Caching section contents was done in git commit 0acf065b192, when the
bfd decompression interface was quite different.  I don't think it was
done for performance, but just as a means of passing around
decompressed contents.

	* objdump.c (load_specific_debug_section): Don't call
	bfd_cache_section_contents.  Rearrange so that
	bfd_get_full_section_contents is not called on path where
	bfd_simple_get_relocated_section_contents is called.
	Don't set section->user_data.
	(free_debug_section): Always free section->start.  Don't twiddle
	section flags.
	* readelf.c (load_specific_debug_section): Don't set user_data.
	* dwarf.h (struct dwarf_section): Remove use_data field.
	* dwarf.c (NO_ABBREVS, ABBREV): Adjust to suit.


-- 
Alan Modra
Australia Development Lab, IBM

Comments

H.J. Lu via Binutils Feb. 17, 2021, 11:02 a.m. | #1
Hi Alan,

> This finally gets to the point where I see no regressions from

> enabling readelf and objdump to automatically follow links to separate

> debug info files.


Thanks for fixing this. :-)

Cheers
   Nick

Patch

diff --git a/binutils/dwarf.c b/binutils/dwarf.c
index ce1aeff7ee..c863acfeda 100644
--- a/binutils/dwarf.c
+++ b/binutils/dwarf.c
@@ -11491,8 +11491,8 @@  dwarf_select_sections_all (void)
   do_debug_str_offsets = 1;
 }
 
-#define NO_ABBREVS   NULL, NULL, NULL, 0, 0, 0, NULL, 0, NULL
-#define ABBREV(N)    NULL, NULL, NULL, 0, 0, N, NULL, 0, NULL
+#define NO_ABBREVS   NULL, NULL, NULL, 0, 0, 0, NULL, 0
+#define ABBREV(N)    NULL, NULL, NULL, 0, 0, N, NULL, 0
 
 /* N.B. The order here must match the order in section_display_enum.  */
 
diff --git a/binutils/dwarf.h b/binutils/dwarf.h
index 756a5606fd..98fbef8183 100644
--- a/binutils/dwarf.h
+++ b/binutils/dwarf.h
@@ -143,8 +143,6 @@  struct dwarf_section
   /* Used by clients to help them implement the reloc_at callback.  */
   void *                           reloc_info;
   unsigned long                    num_relocs;
-  /* A spare field for random use.  */
-  void *                           user_data;
 };
 
 /* A structure containing the name of a debug section
diff --git a/binutils/objdump.c b/binutils/objdump.c
index 304785009b..47b23099ec 100644
--- a/binutils/objdump.c
+++ b/binutils/objdump.c
@@ -3552,6 +3552,7 @@  load_specific_debug_section (enum dwarf_section_display_enum debug,
   bfd_byte *contents;
   bfd_size_type amt;
   size_t alloced;
+  bfd_boolean ret;
 
   if (section->start != NULL)
     {
@@ -3565,7 +3566,6 @@  load_specific_debug_section (enum dwarf_section_display_enum debug,
   section->reloc_info = NULL;
   section->num_relocs = 0;
   section->address = bfd_section_vma (sec);
-  section->user_data = sec;
   section->size = bfd_section_size (sec);
   /* PR 24360: On 32-bit hosts sizeof (size_t) < sizeof (bfd_size_type). */
   alloced = amt = section->size + 1;
@@ -3578,57 +3578,50 @@  load_specific_debug_section (enum dwarf_section_display_enum debug,
 	      (unsigned long long) section->size);
       return FALSE;
     }
-  section->start = contents = malloc (alloced);
-  if (section->start == NULL
-      || !bfd_get_full_section_contents (abfd, sec, &contents))
-    {
-      free_debug_section (debug);
-      printf (_("\nCan't get contents for section '%s'.\n"),
-	      sanitize_string (section->name));
-      return FALSE;
-    }
+
+  section->start = contents = xmalloc (alloced);
   /* Ensure any string section has a terminating NUL.  */
   section->start[section->size] = 0;
 
   if ((abfd->flags & (EXEC_P | DYNAMIC)) == 0
       && debug_displays [debug].relocate)
     {
-      long         reloc_size;
-      bfd_boolean  ret;
-
-      bfd_cache_section_contents (sec, section->start);
-
       ret = bfd_simple_get_relocated_section_contents (abfd,
 						       sec,
 						       section->start,
 						       syms) != NULL;
-
-      if (! ret)
-        {
-          free_debug_section (debug);
-          printf (_("\nCan't get contents for section '%s'.\n"),
-	          sanitize_string (section->name));
-          return FALSE;
-        }
-
-      reloc_size = bfd_get_reloc_upper_bound (abfd, sec);
-      if (reloc_size > 0)
+      if (ret)
 	{
-	  unsigned long reloc_count;
-	  arelent **relocs;
-
-	  relocs = (arelent **) xmalloc (reloc_size);
+	  long reloc_size = bfd_get_reloc_upper_bound (abfd, sec);
 
-	  reloc_count = bfd_canonicalize_reloc (abfd, sec, relocs, NULL);
-	  if (reloc_count == 0)
-	    free (relocs);
-	  else
+	  if (reloc_size > 0)
 	    {
-	      section->reloc_info = relocs;
-	      section->num_relocs = reloc_count;
+	      unsigned long reloc_count;
+	      arelent **relocs;
+
+	      relocs = (arelent **) xmalloc (reloc_size);
+
+	      reloc_count = bfd_canonicalize_reloc (abfd, sec, relocs, NULL);
+	      if (reloc_count == 0)
+		free (relocs);
+	      else
+		{
+		  section->reloc_info = relocs;
+		  section->num_relocs = reloc_count;
+		}
 	    }
 	}
     }
+  else
+    ret = bfd_get_full_section_contents (abfd, sec, &contents);
+
+  if (!ret)
+    {
+      free_debug_section (debug);
+      printf (_("\nCan't get contents for section '%s'.\n"),
+	      sanitize_string (section->name));
+      return FALSE;
+    }
 
   return TRUE;
 }
@@ -3686,26 +3679,6 @@  free_debug_section (enum dwarf_section_display_enum debug)
 {
   struct dwarf_section *section = &debug_displays [debug].section;
 
-  if (section->start == NULL)
-    return;
-
-  /* PR 17512: file: 0f67f69d.  */
-  if (section->user_data != NULL)
-    {
-      asection * sec = (asection *) section->user_data;
-
-      /* If we are freeing contents that are also pointed to by the BFD
-	 library's section structure then make sure to update those pointers
-	 too.  Otherwise, the next time we try to load data for this section
-	 we can end up using a stale pointer.  */
-      if (section->start == sec->contents)
-	{
-	  sec->contents = NULL;
-	  sec->flags &= ~ SEC_IN_MEMORY;
-	  sec->compress_status = COMPRESS_SECTION_NONE;
-	}
-    }
-
   free ((char *) section->start);
   section->start = NULL;
   section->address = 0;
diff --git a/binutils/readelf.c b/binutils/readelf.c
index 77e450a619..755634dfe5 100644
--- a/binutils/readelf.c
+++ b/binutils/readelf.c
@@ -14898,7 +14898,6 @@  load_specific_debug_section (enum dwarf_section_display_enum  debug,
 
   snprintf (buf, sizeof (buf), _("%s section data"), section->name);
   section->address = sec->sh_addr;
-  section->user_data = NULL;
   section->filename = filedata->file_name;
   section->start = (unsigned char *) get_data (NULL, filedata,
                                                sec->sh_offset, 1,