Home Original page

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

Indu Bhagat indu.bhagat@oracle.com
Wed Mar 19 06:23:19 GMT 2025 
On 3/17/25 1:05 AM, Jan Beulich wrote:
> On 17.03.2025 05:49, Indu Bhagat wrote:
>> 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 ?
> 
> Not sure.
> 
>> 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.
> 
> So what about using nil instead of 'S' in the table? That'll require a
> minimal adjustment in dwarf_select_sections_by_letters(), but would avoid
> such a side effect.
> 

Sure.  This could work.

We could add an entry like the following in debug_option_table[]:

{ '\0', "sframe-internal-only", &do_sframe, 1 }.

And then in objdump.c, we do something like:

         case OPTION_DWARF:
           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
             {
               dwarf_select_sections_all ();
               dump_dwarf_section_info = true;
             }
           break;


             dump_sframe_section_name = xstrdup (optarg);

and then for OPTION_SFRAME, something like:

         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 name:
-            "sframe".  */
-         dwarf_select_sections_by_names ("sframe");
+            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;


Basically we disallow the "sframe-internal-only" explicitly from 
external/user input in --dwarf.  Similarly some stubs for readelf 
disallowing sframe from --debug-dump.

>>>> @@ -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)
> 
> Duplicating code is often better avoided. However, somewhat along the lines
> of the above suggestion, can't we add another flag to dwarf_section_display
> to suppress the use of an entry when matching command line arguments?
> 

I am not very sure how this will help...

> Ftaod - neither of the adjustments needs to be done. But imo command line
> options that we recognize would better also be documented. Which here means
> if you want to avoid documenting some, let's try to avoid recognizing them.
> 

... but the above suggestion using "sframe-internal-only" should help us 
avoid the said unnecessary options:

$ objdump -WS sort.o
objdump: Warning: Unrecognized debug letter option 'S'
...

$ objdump --dwarf=sframe sort.o
objdump: Warning: Unrecognized debug option 'sframe'
...

$ objdump --dwarf=sframe-internal-only sort.o
objdump: Warning: Unrecognized debug option 'sframe-internal-only'
...

Indu
More information about the Binutils mailing list