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

[Jens Remus]

On 2/6/2026 8:41 AM, Indu Bhagat wrote:
> On 2/5/26 2:44 AM, Jens Remus wrote:
>> On 2/5/2026 10:04 AM, Indu Bhagat wrote:
>>> On 2/4/26 8:08 AM, Jens Remus wrote:
>>>> On 2/3/2026 11:37 PM, Indu Bhagat via Binutils wrote:
>>>
>>>>> diff --git a/binutils/objdump.c b/binutils/objdump.c

>>>>> +  /* 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);
>>
>>> free_debug_memory () invocation takes care of free'ing the debug section
>>> data (including sframe).  So free_debug_section (sframe) looks unnecessary.
>> Thanks!  That was not obvious to me.  Given that dump_dwarf_section()
>> does it (and you will likely send a v2 anyway) I wonder whether it would
>> still make sense so that others are not confused as well.
>>
> 
> I am not convinced that we should, as it looks to me that this is not needed.
> May be your opinion shifts after the following remarks...
> (Meanwhile I posted V2 without it, let me know)

Ok.  Let's not start bikeshedding.

>> Tracing objdump --sframe passing multiple objects with .sframe as input
>> and could confirm that free_debug_section() is called after processing
>> each input.  I was worried, that the section data of all inputs would
>> accumulate in a single objdump run.  Especially given it is only done for
>> (is_mainfile), while dump_sframe_section() might be called for
>> (is_mainfile || process_links).  Can we rule out that the latter may
>> trigger dump_sframe_section()?
>>
> 
> AFAICT, the opening of the debug link to add to abfd object accounts for
> only debug sections in load_separate_debug_files ().  So .sframe from
> linked debug will not trigger dump_sframe_section ().

Thanks!

>> At least I was not able to trick objdump to follow a link to .sframe:
>>
>> $ cd path-to-binutils-build/ld/tmpdir
>> $ ../ld-new -o sframe-sample sframe-foo.o sframe-bar.o
>> $ cp sframe-sample sframe-sample.full
>> $ ../../binutils/strip-new --strip-debug -R .sframe sframe-sample
>> $ ../../binutils/objcopy --add-gnu-debuglink=sframe-sample.full sframe-sample
>> $ ../../binutils/objdump --sframe sframe-sample
>> No .sframe section present
>>
> 
> Hmm.  SFrame section is allocated, non-debug section, intended to be
> shipped with the binary.  So the above workflow doesnt quite fit SFrame
> section.  But if the users do the above for whatsoever reason, the
> above behaviour is reasonable I think.

Regardless of whether this makes sense I wanted to verify that this
would not leak memory.  I think we once discussed this in context of how
to store .sframe compressed.  Something similar could be used to store
the .sframe in a separate ELF e.g. on a compressed file system and have
unwinders load it on demand.

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/