[PATCH] objdump: sframe: fix multiple emission of SFrame and DWARF Frame

[Jens Remus]

On 2/3/2026 11:37 PM, Indu Bhagat via Binutils wrote:
> Currently, when a user specifies both -WF (to dump DWARF Frame) and
> --sframe, the dump_dwarf () call is triggered twice,
> 
> First, in dump_bfd ():
>   if (dump_dwarf_section_info)
>     dump_dwarf (abfd, is_mainfile);
> 
> And then again in dump_bfd () via:
>    if (dump_sframe_section_info)
>      dump_sframe_section (abfd, dump_sframe_section_name, is_mainfile);
> Where dump_sframe_section () itself invokes dump_dwarf () to emit SFrame
> section.
> 
> The original intent of making dump_sframe_section () to invoke
> dump_dwarf () was to reuse the existing (DWARF) machinery for relocation
> and emission of SFrame section (along with DWARF sections).  But this
> may cause duplicate emission as noted above (PR 33806).
> 
> So, carve out the necessary work into dump_sframe_section () to make it
> not invoke dump_dwarf (): explicitly load the SFrame section and then
> invoke the associated display function directly.  This ensures that the
> SFrame textual dump is emitted exactly once as intended.
> 
> Additionally, this patch removes:
>   - the temporary "sframe-internal-only" debug option because it not
>     needed anymore, now that we dont use dump_dwarf () for SFrame dumping
>     anymore.
>   - stubs from dump_dwarf_section () specific to SFrame, as it is not
>     called for it anymore.
> 
> While at it, xstrdup consistently and free the dump_sframe_section_name
> var.
> 
> Update the existing sframe-func.s to ensure .eh_frame section is emitted
> as well.  Add new test objdump-sframe-2.d to verify that textual dump
> using objdump contains single textual dumps of .eh_frame and .sframe (if
> both --sframe and -WF are specified).
> 
> PR libsframe/33806
> 
> binutils/
> 	PR libsframe/33806
> 	* dwarf.c (debug_option_table): Remove sframe-internal-only.
> 	* objdump.c (dump_sframe_section): Call display function
> 	directly instead of dump_dwarf.
> 	(dump_bfd): Update call to dump_sframe_section.
> 	(main): Remove 'sframe-internal-only' hack.
> binutils/testsuite/
> 	* binutils-all/x86-64/sframe-func.s: Emit .eh_frame section as
> 	well.
> 	* binutils-all/x86-64/objdump-sframe-2.d: New test.
> ---
>  binutils/dwarf.c                              |  1 -
>  binutils/objdump.c                            | 69 +++++++++----------
>  .../binutils-all/x86-64/objdump-sframe-2.d    | 33 +++++++++
>  .../binutils-all/x86-64/sframe-func.s         |  1 -
>  4 files changed, 66 insertions(+), 38 deletions(-)
>  create mode 100644 binutils/testsuite/binutils-all/x86-64/objdump-sframe-2.d

LGTM.  Thanks for resolving this issue!

One additional remark below besides Jan's excellent feedback.

> diff --git a/binutils/objdump.c b/binutils/objdump.c

> @@ -4983,33 +4979,40 @@ dump_ctf (bfd *abfd ATTRIBUTE_UNUSED, const char *sect_name ATTRIBUTE_UNUSED,
>  #endif
>  
>  static void
> -dump_sframe_section (bfd *abfd, const char *sect_name, bool is_mainfile)
> +dump_sframe_section (bfd *abfd, const char *sect_name)
>  
>  {
>    /* Error checking for user provided SFrame section name, if any.  */
> -  if (sect_name)
> +  asection *sec = bfd_get_section_by_name (abfd, sect_name);
> +  if (sec == NULL)
>      {
> -      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;
> -	}
> -      /* Starting with Binutils 2.45, SFrame sections have section type
> -	 SHT_GNU_SFRAME.  For SFrame sections from Binutils 2.44 or earlier,
> -	 check explcitly for SFrame sections of type SHT_PROGBITS and name
> -	 ".sframe" to allow them.  */
> -      else if (bfd_get_flavour (abfd) != bfd_target_elf_flavour
> -	       || (elf_section_type (sec) != SHT_GNU_SFRAME
> -		   && !(elf_section_type (sec) == SHT_PROGBITS
> -			&& strcmp (sect_name, ".sframe") == 0)))
> -	{
> -	  printf (_("Section %s does not contain SFrame data\n\n"),
> -		  sanitize_string (sect_name));
> -	  return;
> -	}
> +      printf (_("No %s section present\n\n"), sanitize_string (sect_name));
> +      return;
> +    }
> +  /* Starting with Binutils 2.45, SFrame sections have section type
> +     SHT_GNU_SFRAME.  For SFrame sections from Binutils 2.44 or earlier,
> +     check explcitly for SFrame sections of type SHT_PROGBITS and name
> +     ".sframe" to allow them.  */
> +  else if (bfd_get_flavour (abfd) != bfd_target_elf_flavour
> +	   || (elf_section_type (sec) != SHT_GNU_SFRAME
> +	       && !(elf_section_type (sec) == SHT_PROGBITS
> +		    && strcmp (sect_name, ".sframe") == 0)))
> +    {
> +      printf (_("Section %s does not contain SFrame data\n\n"),
> +	      sanitize_string (sect_name));
> +      return;
>      }
> -  dump_dwarf (abfd, is_mainfile);
> +
> +  /* Instead of invoking dump_dwarf () here, load the section and invoke the
> +     display function directly.  Otherwise, the textual dump may contain
> +     duplicate output due to two invocations of dump_dwarf ().  See PR
> +     libsframe/33806.  */
> +  if (!load_specific_debug_section (sframe, sec, (void*) abfd))
> +    return;
> +
> +  struct dwarf_section *section = &debug_displays[sframe].section;
> +  section->name = sect_name;
> +  debug_displays[sframe].display (section, abfd);

Why not free the loaded section afterwards, as dump_dwarf_section()
would have done?

  free_debug_section (sframe);

>  }
Thanks and regards,
Jens
-- 
Jens Remus
Linux on Z Development (D3303)
jremus@de.ibm.com / jremus@linux.ibm.com

IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Ehningen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/