]> git.ipfire.org Git - thirdparty/binutils-gdb.git/commitdiff
objdump, readelf: sframe: apply relocations before textual dump
authorIndu Bhagat <indu.bhagat@oracle.com>
Mon, 26 May 2025 22:05:26 +0000 (15:05 -0700)
committerIndu Bhagat <indu.bhagat@oracle.com>
Wed, 25 Jun 2025 19:56:19 +0000 (12:56 -0700)
PR libsframe/32589 - function start address is zero in SFrame section dump

Currently, readelf and objdump display the SFrame sections in ET_REL
object files with function start addresses of each function as 0.  This
makes it difficult to correlate SFrame stack trace information with the
individual functions in the object file.

For objdump, use the dump_dwarf () interface to dump SFrame section.
Similarly, for readelf, use the display_debug_section () interface to
dump SFrame section.  These existing interfaces (for DWARF debug
sections) already support relocating the section contents before
dumping, so lets use them for SFrame sections as well.

When adding a new entry for SFrame in debug_option_table[], use char
'nil' and the option name of "sframe-internal-only".  This is done so
that there is no additional (unnecessary) user-exposed ways of dumping
SFrame sections.  Additionally, we explicitly disallow the
"sframe-internal-only" from external/user input in --dwarf (objdump).
Similarly, "sframe-internal-only" is explicitly matched and disallowed
from --debug-dump (readelf).

For objdump and readelf, we continue to keep the same error messaging as
earlier:

  $ objdump --sframe=sframe bubble_sort.o
  ...
  No sframe section present

  $ objdump --sframe=.sfram bubble_sort.o
  ...
  No .sfram section present

  $ objdump --sframe=sframe-internal-only sort
  ...
  No sframe-internal-only section present

Similarly for readelf:

  $ readelf --sframe= bubble_sort.o
  readelf: Error: Section name must be provided
  $ readelf --sframe=.sfram bubble_sort.o
  readelf: Warning: Section '.sfram' was not dumped because it does not exist
  $ readelf --sframe=sframe bubble_sort.o
  readelf: Warning: Section 'sframe' was not dumped because it does not exist

PS: Note how this patch adds a new entry to debug_displays[] with a
    relocate value set to FALSE.  This will be set to TRUE in a subsequent
    patch ("bfd: gas: ld: libsframe: emit func start addr field as an offset
    from FDE") when fixes are made to emit the value of the
    'sfde_func_start_address' field in the new encoding
    SFRAME_F_FDE_FUNC_START_ADDR_PCREL across gas and ld.

binutils/
* dwarf.c (display_sframe): New definition.
(dwarf_select_sections_all): Enable SFrame section too.
(struct dwarf_section_display): Add entry for SFrame section.
* dwarf.h (enum dwarf_section_display_enum): Add enumerator for
SFrame.
* objdump.c (dump_section_sframe): Remove.
(dump_sframe_section): Add new definition.
(dump_bfd): Use dump_sframe_section.
* binutils/readelf.c (dump_section_as_sframe): Remove.
---
This patch was previously reviewed at part of other series previously:
https://inbox.sourceware.org/binutils/20250308073853.78738-3-indu.bhagat@oracle.com/

The review comments have been addressed in this patch.  The setting of
relocate to FALSE for the new record is the new diff.

 { { ".sframe",            "",                      "",         NO_ABBREVS },      display_sframe,         &do_sframe,         false },

This is necessary to keep each patch in the series bisectable and reviewable.

Notes:
  - V3 was OK'd https://sourceware.org/pipermail/binutils/2025-June/141796.html

[No changes in V4]
[No changes in V3]
[No changes in V2]

binutils/dwarf.c
binutils/dwarf.h
binutils/objdump.c
binutils/readelf.c

index 5b3ece58c8b26b9da85d40ec4237a3b61b411c39..29fbf541149831dc841427ee8ba9639346559c07 100644 (file)
@@ -30,6 +30,7 @@
 #include "gdb/gdb-index.h"
 #include "filenames.h"
 #include "safe-ctype.h"
+#include "sframe-api.h"
 #include <assert.h>
 
 #ifdef HAVE_LIBDEBUGINFOD
@@ -102,6 +103,7 @@ int do_debug_str;
 int do_debug_str_offsets;
 int do_debug_loc;
 int do_gdb_index;
+int do_sframe;
 int do_trace_info;
 int do_trace_abbrevs;
 int do_trace_aranges;
@@ -7688,6 +7690,37 @@ display_trace_info (struct dwarf_section *section, void *file)
   return process_debug_info (section, file, section->abbrev_sec, false, true);
 }
 
+static int
+display_sframe (struct dwarf_section *section, void *file ATTRIBUTE_UNUSED)
+{
+  sframe_decoder_ctx *sfd_ctx = NULL;
+  unsigned char *data = section->start;
+  size_t sf_size = section->size;
+  int err = 0;
+
+  if (strcmp (section->name, "") == 0)
+    {
+      error (_("Section name must be provided \n"));
+      return false;
+    }
+
+  /* Decode the contents of the section.  */
+  sfd_ctx = sframe_decode ((const char*)data, sf_size, &err);
+  if (!sfd_ctx || err)
+    {
+      error (_("SFrame decode failure: %s\n"), sframe_errmsg (err));
+      return false;
+    }
+
+  printf (_("Contents of the SFrame section %s:"), section->name);
+  /* Dump the contents as text.  */
+  dump_sframe (sfd_ctx, section->address);
+
+  sframe_decoder_free (&sfd_ctx);
+
+  return true;
+}
+
 static int
 display_debug_aranges (struct dwarf_section *section,
                       void *file ATTRIBUTE_UNUSED)
@@ -12671,6 +12704,7 @@ static const debug_dump_long_opts debug_option_table[] =
   /* For compatibility with earlier versions of readelf.  */
   { 'r', "ranges", &do_debug_aranges, 1 },
   { 's', "str", &do_debug_str, 1 },
+  { '\0', "sframe-internal-only", &do_sframe, 1 },
   { 'T', "trace_aranges", &do_trace_aranges, 1 },
   { 't', "pubtypes", &do_debug_pubtypes, 1 },
   { 'U', "trace_info", &do_trace_info, 1 },
@@ -12829,6 +12863,7 @@ struct dwarf_section_display debug_displays[] =
   { { ".debug_weaknames",   ".zdebug_weaknames",     "",        NO_ABBREVS },      display_debug_not_supported, NULL,          false },
   { { ".gdb_index",        "",                      "",         NO_ABBREVS },      display_gdb_index,      &do_gdb_index,      false },
   { { ".debug_names",      "",                      "",         NO_ABBREVS },      display_debug_names,    &do_gdb_index,      false },
+  { { ".sframe",           "",                      "",         NO_ABBREVS },      display_sframe,         &do_sframe,         false },
   { { ".trace_info",       "",                      "",         ABBREV (trace_abbrev) }, display_trace_info, &do_trace_info,   true },
   { { ".trace_abbrev",     "",                      "",         NO_ABBREVS },      display_debug_abbrev,   &do_trace_abbrevs,  false },
   { { ".trace_aranges",            "",                      "",         NO_ABBREVS },      display_debug_aranges,  &do_trace_aranges,  false },
index 3419027ac1e82b6540648e394913f5204e9ba78a..6f693b1eac2a73abb0bf6bc72552af25594d5bab 100644 (file)
@@ -102,6 +102,7 @@ enum dwarf_section_display_enum
   weaknames,
   gdb_index,
   debug_names,
+  sframe,
   trace_info,
   trace_abbrev,
   trace_aranges,
index 43cfb796feaca275d81eb7e3aefb2bb1ba879ff4..7bb6d766ba2c216b0e8b0491fc05ffd9fa4b28a0 100644 (file)
@@ -4987,44 +4987,20 @@ dump_ctf (bfd *abfd ATTRIBUTE_UNUSED, const char *sect_name ATTRIBUTE_UNUSED,
 #endif
 
 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;
-  bfd_vma sf_vma;
-  int err = 0;
-
-  if (sect_name == NULL)
-    sect_name = ".sframe";
+dump_sframe_section (bfd *abfd, const char *sect_name, bool is_mainfile)
 
-  sec = read_section (abfd, sect_name, &sframe_data);
-  if (sec == NULL)
-    {
-      my_bfd_nonfatal (bfd_get_filename (abfd));
-      return;
-    }
-  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);
-  if (!sfd_ctx)
+{
+  /* Error checking for user provided SFrame section name, if any.  */
+  if (sect_name)
     {
-      my_bfd_nonfatal (bfd_get_filename (abfd));
-      free (sframe_data);
-      return;
+      asection *sec = bfd_get_section_by_name (abfd, sect_name);
+      if (sec == NULL)
+       {
+         printf (_("No %s section present\n\n"), sanitize_string (sect_name));
+         return;
+       }
     }
-
-  printf (_("Contents of the SFrame section %s:"),
-         sanitize_string (sect_name));
-  /* Dump the contents as text.  */
-  dump_sframe (sfd_ctx, sf_vma);
-
-  sframe_decoder_free (&sfd_ctx);
-  free (sframe_data);
+  dump_dwarf (abfd, is_mainfile);
 }
 
 \f
@@ -5846,7 +5822,7 @@ dump_bfd (bfd *abfd, bool is_mainfile)
        dump_ctf (abfd, dump_ctf_section_name, dump_ctf_parent_name,
                  dump_ctf_parent_section_name);
       if (dump_sframe_section_info)
-       dump_section_sframe (abfd, dump_sframe_section_name);
+       dump_sframe_section (abfd, dump_sframe_section_name, is_mainfile);
       if (dump_stab_section_info)
        dump_stabs (abfd);
       if (dump_reloc_info && ! disassemble)
@@ -6310,7 +6286,9 @@ main (int argc, char **argv)
          seenflag = true;
          if (optarg)
            {
-             if (dwarf_select_sections_by_names (optarg))
+             if (strcmp (optarg, "sframe-internal-only") == 0)
+               warn (_("Unrecognized debug option 'sframe-internal-only'\n"));
+             else if (dwarf_select_sections_by_names (optarg))
                dump_dwarf_section_info = true;
            }
          else
@@ -6351,8 +6329,15 @@ main (int argc, char **argv)
 #endif
        case OPTION_SFRAME:
          dump_sframe_section_info = true;
+
          if (optarg)
            dump_sframe_section_name = xstrdup (optarg);
+
+         /* Error checking for user-provided section name is done in
+            dump_sframe_section ().  Initialize for now with the default
+            internal name: "sframe-internal-only".  */
+         dwarf_select_sections_by_names ("sframe-internal-only");
+
          seenflag = true;
          break;
        case 'G':
index f49092f11b98a190836eebf5f6ba347ec1566498..573024727f23513d66fb70ce7fed9a04a1ad6de6 100644 (file)
@@ -6538,6 +6538,8 @@ parse_args (struct dump_data *dumpdata, int argc, char ** argv)
              dump_any_debugging = true;
              dwarf_select_sections_all ();
            }
+         else if (strcmp (optarg, "sframe-internal-only") == 0)
+           warn (_("Unrecognized debug option 'sframe-internal-only'\n"));
          else
            {
              do_debugging = false;
@@ -6583,9 +6585,15 @@ parse_args (struct dump_data *dumpdata, int argc, char ** argv)
          break;
        case OPTION_SFRAME_DUMP:
          do_sframe = true;
+         /* Fix PR/32589 but keep the error messaging same ?  */
+         if (optarg != NULL && strcmp (optarg, "") == 0)
+           {
+             do_dump = true;
+             error (_("Section name must be provided\n"));
+           }
          /* Providing section name is optional.  request_dump (), however,
             thrives on non NULL optarg.  Handle it explicitly here.  */
-         if (optarg != NULL)
+         else if (optarg != NULL)
            request_dump (dumpdata, SFRAME_DUMP);
          else
            {
@@ -17101,44 +17109,6 @@ dump_section_as_ctf (Elf_Internal_Shdr * section, Filedata * filedata)
 }
 #endif
 
-static bool
-dump_section_as_sframe (Elf_Internal_Shdr * section, Filedata * filedata)
-{
-  void *                 data = NULL;
-  sframe_decoder_ctx     *sfd_ctx = NULL;
-  const char *print_name = printable_section_name (filedata, section);
-
-  bool ret = true;
-  size_t sf_size;
-  int err = 0;
-
-  if (strcmp (print_name, "") == 0)
-    {
-      error (_("Section name must be provided \n"));
-      ret = false;
-      return ret;
-    }
-
-  data = get_section_contents (section, filedata);
-  sf_size = section->sh_size;
-  /* Decode the contents of the section.  */
-  sfd_ctx = sframe_decode ((const char*)data, sf_size, &err);
-  if (!sfd_ctx)
-    {
-      ret = false;
-      error (_("SFrame decode failure: %s\n"), sframe_errmsg (err));
-      goto fail;
-    }
-
-  printf (_("Contents of the SFrame section %s:"), print_name);
-  /* Dump the contents as text.  */
-  dump_sframe (sfd_ctx, section->sh_addr);
-
- fail:
-  free (data);
-  return ret;
-}
-
 static bool
 load_specific_debug_section (enum dwarf_section_display_enum  debug,
                             const Elf_Internal_Shdr *        sec,
@@ -17706,7 +17676,7 @@ process_section_contents (Filedata * filedata)
 #endif
       if (dump & SFRAME_DUMP)
        {
-         if (! dump_section_as_sframe (section, filedata))
+         if (! display_debug_section (i, section, filedata))
            res = false;
        }
     }