Re: objdump read_section_stabs

Re: objdump read_section_stabs

[Alan Modra]

Also fix ubsan "applying zero offset to null pointer".

	* objdump.c (print_section_stabs): Avoid ubsan warning.

diff --git a/binutils/objdump.c b/binutils/objdump.c
index 40ed6882b45..d00eed054c4 100644
--- a/binutils/objdump.c
+++ b/binutils/objdump.c
@@ -4499,7 +4499,7 @@ print_section_stabs (bfd *abfd,
   bfd_byte *stabp, *stabs_end;
 
   stabp = stabs;
-  stabs_end = stabp + stab_size;
+  stabs_end = PTR_ADD (stabp, stab_size);
 
   printf (_("Contents of %s section:\n\n"), sanitize_string (stabsect_name));
   printf ("Symnum n_type n_othr n_desc n_value  n_strx String\n");

-- 
Alan Modra
Australia Development Lab, IBM

Re: objdump read_section_stabs

[Alan Modra]

Commit f9c36cc99518 changed (and renamed) read_section_stabs with one
difference in overall behaviour.  Previously read_section_stabs would
return a NULL for an empty section, which was then treated the same as
a missing section.  Now an empty section is recognized and dumped.
This leads to NULL stabp and stabs_end in print_section_stabs.  Since
stabs_end - STABSIZE is then a pointer to a very large address, the
test "stabp < stabs_end - STABSIZE" succeeds.

	* objdump.c (print_section_stabs): Correct STABSIZE comparison.

diff --git a/binutils/objdump.c b/binutils/objdump.c
index 984c7219a0c..40ed6882b45 100644
--- a/binutils/objdump.c
+++ b/binutils/objdump.c
@@ -4508,7 +4508,7 @@ print_section_stabs (bfd *abfd,
 
      We start the index at -1 because there is a dummy symbol on
      the front of stabs-in-{coff,elf} sections that supplies sizes.  */
-  for (i = -1; stabp <= stabs_end - STABSIZE; stabp += STABSIZE, i++)
+  for (i = -1; (size_t) (stabs_end - stabp) >= STABSIZE; stabp += STABSIZE, i++)
     {
       const char *name;
       unsigned long strx;

-- 
Alan Modra
Australia Development Lab, IBM

objdump read_section_stabs

[Alan Modra]

This function is used to read sections other than stabs, and there is
now another version of it that extracts different info from the bfd
section.  Rename it and return the bfd section instead of assorted
fields of the bfd section.

	* objcopy.c (read_section): Renamed from read_section_stabs.
	Delete size_ptr and entsize_ptr params, add contents param.
	Return asection pointer.  Don't unnecessarily free contents on
	failure from bfd_malloc_and_get_section.
	(find_stabs_section): Use read_section.
	(dump_ctf, dump_section_sframe): Likewise.
	(read_section_sframe): Delete.

diff --git a/binutils/objdump.c b/binutils/objdump.c
index 8a8bfba5c76..8cf9d059801 100644
--- a/binutils/objdump.c
+++ b/binutils/objdump.c
@@ -4443,39 +4443,32 @@ dump_dwarf (bfd *abfd, bool is_mainfile)
   bfd_map_over_sections (abfd, dump_dwarf_section, (void *) &is_mainfile);
 }
 \f
-/* Read ABFD's stabs section STABSECT_NAME, and return a pointer to
-   it.  Return NULL on failure.   */
+/* Read ABFD's section SECT_NAME into *CONTENTS, and return a pointer to
+   the section.  Return NULL on failure.   */
 
-static bfd_byte *
-read_section_stabs (bfd *abfd, const char *sect_name, bfd_size_type *size_ptr,
-		    bfd_size_type *entsize_ptr)
+static asection *
+read_section (bfd *abfd, const char *sect_name, bfd_byte **contents)
 {
-  asection *stabsect;
-  bfd_byte *contents;
+  asection *sec;
 
-  stabsect = bfd_get_section_by_name (abfd, sect_name);
-  if (stabsect == NULL)
+  *contents = NULL;
+  sec = bfd_get_section_by_name (abfd, sect_name);
+  if (sec == NULL)
     {
-      printf (_("No %s section present\n\n"),
-	      sanitize_string (sect_name));
-      return false;
+      printf (_("No %s section present\n\n"), sanitize_string (sect_name));
+      return NULL;
     }
 
-  if (!bfd_malloc_and_get_section (abfd, stabsect, &contents))
+  if (!bfd_malloc_and_get_section (abfd, sec, contents))
     {
       non_fatal (_("reading %s section of %s failed: %s"),
 		 sect_name, bfd_get_filename (abfd),
 		 bfd_errmsg (bfd_get_error ()));
       exit_status = 1;
-      free (contents);
       return NULL;
     }
 
-  *size_ptr = bfd_section_size (stabsect);
-  if (entsize_ptr)
-    *entsize_ptr = stabsect->entsize;
-
-  return contents;
+  return sec;
 }
 
 /* Stabs entries use a 12 byte format:
@@ -4595,15 +4588,20 @@ find_stabs_section (bfd *abfd, asection *section, void *names)
       && (section->name[len] == 0
 	  || (section->name[len] == '.' && ISDIGIT (section->name[len + 1]))))
     {
+      asection *s;
       if (strtab == NULL)
-	strtab = read_section_stabs (abfd, sought->string_section_name,
-				     &stabstr_size, NULL);
+	{
+	  s = read_section (abfd, sought->string_section_name, &strtab);
+	  if (s != NULL)
+	    stabstr_size = bfd_section_size (s);
+	}
 
       if (strtab)
 	{
-	  stabs = read_section_stabs (abfd, section->name, &stab_size, NULL);
-	  if (stabs)
+	  s = read_section (abfd, section->name, &stabs);
+	  if (s != NULL)
 	    {
+	      stab_size = bfd_section_size (s);
 	      print_section_stabs (abfd, section->name, &sought->string_offset);
 	      free (stabs);
 	    }
@@ -4776,9 +4774,9 @@ dump_ctf_archive_member (ctf_dict_t *ctf, const char *name, ctf_dict_t *parent,
 static void
 dump_ctf (bfd *abfd, const char *sect_name, const char *parent_name)
 {
+  asection *sec;
   ctf_archive_t *ctfa = NULL;
-  bfd_byte *ctfdata = NULL;
-  bfd_size_type ctfsize;
+  bfd_byte *ctfdata;
   ctf_sect_t ctfsect;
   ctf_dict_t *parent;
   ctf_dict_t *fp;
@@ -4790,13 +4788,14 @@ dump_ctf (bfd *abfd, const char *sect_name, const char *parent_name)
   if (sect_name == NULL)
     sect_name = ".ctf";
 
-  if ((ctfdata = read_section_stabs (abfd, sect_name, &ctfsize, NULL)) == NULL)
-      bfd_fatal (bfd_get_filename (abfd));
+  sec = read_section (abfd, sect_name, &ctfdata);
+  if (sec == NULL)
+    bfd_fatal (bfd_get_filename (abfd));
 
   /* Load the CTF file and dump it.  Preload the parent dict, since it will
      need to be imported into every child in turn. */
 
-  ctfsect = make_ctfsect (sect_name, ctfdata, ctfsize);
+  ctfsect = make_ctfsect (sect_name, ctfdata, bfd_section_size (sec));
   if ((ctfa = ctf_bfdopen_ctfsect (abfd, &ctfsect, &err)) == NULL)
     {
       dump_ctf_errs (NULL);
@@ -4831,54 +4830,25 @@ dump_ctf (bfd *abfd ATTRIBUTE_UNUSED, const char *sect_name ATTRIBUTE_UNUSED,
 	  const char *parent_name ATTRIBUTE_UNUSED) {}
 #endif
 
-static bfd_byte*
-read_section_sframe (bfd *abfd, const char *sect_name, bfd_size_type *size_ptr,
-		     bfd_vma *sframe_vma)
-{
-  asection *sframe_sect;
-  bfd_byte *contents;
-
-  sframe_sect = bfd_get_section_by_name (abfd, sect_name);
-  if (sframe_sect == NULL)
-    {
-      printf (_("No %s section present\n\n"),
-	      sanitize_string (sect_name));
-      return NULL;
-    }
-
-  if (!bfd_malloc_and_get_section (abfd, sframe_sect, &contents))
-    {
-      non_fatal (_("reading %s section of %s failed: %s"),
-		 sect_name, bfd_get_filename (abfd),
-		 bfd_errmsg (bfd_get_error ()));
-      exit_status = 1;
-      free (contents);
-      return NULL;
-    }
-
-  *size_ptr = bfd_section_size (sframe_sect);
-  *sframe_vma = bfd_section_vma (sframe_sect);
-
-  return contents;
-}
-
 static void
 dump_section_sframe (bfd *abfd ATTRIBUTE_UNUSED,
 		     const char * sect_name)
 {
+  asection *sec;
   sframe_decoder_ctx *sfd_ctx = NULL;
   bfd_size_type sf_size;
-  bfd_byte *sframe_data = NULL;
+  bfd_byte *sframe_data;
   bfd_vma sf_vma;
   int err = 0;
 
   if (sect_name == NULL)
     sect_name = ".sframe";
 
-  sframe_data = read_section_sframe (abfd, sect_name, &sf_size, &sf_vma);
-
-  if (sframe_data == NULL)
+  sec = read_section (abfd, sect_name, &sframe_data);
+  if (sec == NULL)
     bfd_fatal (bfd_get_filename (abfd));
+  sf_size = bfd_section_size (sec);
+  sf_vma = bfd_section_vma (sec);
 
   /* Decode the contents of the section.  */
   sfd_ctx = sframe_decode ((const char*)sframe_data, sf_size, &err);

-- 
Alan Modra
Australia Development Lab, IBM