[RFC 2/4] objdump, readelf: sframe: apply relocations before textual dump

[Indu Bhagat]

On 3/13/25 7:09 AM, Jan Beulich wrote:
> On 08.03.2025 08:38, Indu Bhagat wrote:
>> PR libsframe/32589 - function start address is zero in SFrame section dump
>>
>> Currently, readelf and objdump display SFrame section in object file
>> 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.
>>
>> Use the dump_dwarf () interface to dump SFrame section.  The current
>> infrastructure (for DWARF debug sections) already supports relocating
>> the section contents before dumping, so lets use that.
>>
>> As a side effect, objdump now adds two new ways of dumping SFrame sections:
>>    - objdump -WS <obj>
>>    - objdump --dwarf=sframe
>> We do not publicize these options.  The lone advertised user interfacing
>> option (in --help) remains:
>>    - objdump --sframe
> 
> How am I to understand "side effect" here? You don't need to ...
> 
>> @@ -12648,6 +12681,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 },
>> +  { 'S', "sframe", &do_sframe, 1 },
>>     { 'T', "trace_aranges", &do_trace_aranges, 1 },
>>     { 't', "pubtypes", &do_debug_pubtypes, 1 },
>>     { 'U', "trace_info", &do_trace_info, 1 },
> 
> ... make this and ...
> 

This is possible to bypass, but there will be added code. Explanation here:

In the current patch, in objdump.c we invoke the dump_sframe_section () 
(which invokes the dump_dwarf ()), and in readelf.c we invoke the 
display_debug_section ().

Note that, it is the dwarf_select_sections_by_names () that sets the 
do_sframe which are then later relied on by dump_dwarf () and 
display_debug_section ().

It should be possible to instead do something like the following in 
objdump.c:

static void
dump_sframe_section (bfd *abfd, const char *sect_name)

{
   load_debug_section ((enum dwarf_section_display_enum) sframe, abfd);

   struct dwarf_section *sframe_sec = &debug_displays [sframe].section;

   debug_displays [sframe].display (sframe_sec, abfd);

   free_debug_section ((enum dwarf_section_display_enum) sframe);
}

and similar stubs in readelf.c. Is this preferable ?

IOW, that is what I meant as "side effect": Using the available APIs as 
they are (load_debug_section, dump_dwarf, display_debug_section) to dump 
SFrame sections as well, requires us to add entries in the 
debug_option_table [].  Hence, supporting objdump -WS unnecessarily.

>> @@ -12806,6 +12841,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,		true },
>>     { { ".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 },
> 
> ... this change, do you? Yet then that's a deliberate extra change, not
> a side effect. Which in turn make me wonder why it's done, and then
> deliberately without documenting the options.
> 

... but this change cannot be skipped, as far as I can see.

For SFrame display, we would like to use the load_specific_debug_section 
() (or even load_debug_section () will work) somehow.  The interface and 
implementation of these load_(*_)debug_section () is such that they rely 
on debug_displays [] shown above.  So this addition to debug_displays[] 
is necessary, unless there is another preferable way to get relocated 
SFrame section contents (like, by refactoring the 
load_specific_debug_section () ? and duplicating some stubs in specific 
dump_sframe_section () APIs)

Indu