[PATCH] readelf: objdump: sframe: fix dumping with section name

[Indu Bhagat]

On 7/22/25 12:41 AM, Jan Beulich wrote:
> On 20.07.2025 07:34, Indu Bhagat via Binutils wrote:
>> --- a/binutils/objdump.c
>> +++ b/binutils/objdump.c
>> @@ -4495,6 +4495,8 @@ dump_dwarf_section (bfd *abfd, asection *section,
>>   
>>     if (startswith (name, ".gnu.linkonce.wi."))
>>       match = ".debug_info";
>> +  else if (elf_section_type (section) == SHT_GNU_SFRAME)
>> +    match = ".sframe";
>>     else
>>       match = name;
>>   
>> @@ -4999,6 +5001,12 @@ dump_sframe_section (bfd *abfd, const char *sect_name, bool is_mainfile)
>>   	  printf (_("No %s section present\n\n"), sanitize_string (sect_name));
>>   	  return;
>>   	}
>> +      else if (elf_section_type (sec) != SHT_GNU_SFRAME)
> 
> As mentioned before, can we please get away without "else" in such cases?
> 

OK.

>> +	{
>> +	  printf (_("Section %s does not contain SFrame data\n\n"),
>> +		  sanitize_string (sect_name));
> 
> Why "does"? When created with 2.44 it would be a PROGBITS section, so
> the message would end up being wrong. Unless there's some magic somewhere
> that I'm unaware of.
> 

Ah, thanks. To accommodate the SFrame sections from 2.44, I will need to 
do a:

/* Starting 2.45, SFrame sections have section type SHT_GNU_SFRAME.
  Check explcitly to allow dumping of previously generatated SFrame
  sections of type SHT_PROGBITS and name ".sframe.".  */
else if (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;
}

With this in place, for Binutils 2.44, users will see the following 
behavior:

$ objcopy --rename-section .sframe=.sframe2 sort
$ readelf -S sort | grep sframe
   [18] .sframe2          PROGBITS         0000000000402228  00002228
$ objdump --sframe=.sframe2 sort

sort:     file format elf64-x86-64

Section .sframe2 does not contain SFrame data
$ readelf --sframe sort
readelf: Warning: Section '.sframe' was not dumped because it does not exist
$ readelf --sframe=.sframe2 sort
Unrecognized debug section: .sframe2

Which is reasonable.

>> --- a/binutils/readelf.c
>> +++ b/binutils/readelf.c
>> @@ -17493,6 +17493,7 @@ display_debug_section (int shndx, Elf_Internal_Shdr * section, Filedata * fileda
>>   
>>         if (streq (sec->uncompressed_name, name)
>>   	  || (id == line && startswith (name, ".debug_line."))
>> +	  || (id == sframe && section->sh_type == SHT_GNU_SFRAME)
> 
> Using id is correct here; it escapes me why ...
> 
>> @@ -17502,6 +17503,8 @@ display_debug_section (int shndx, Elf_Internal_Shdr * section, Filedata * fileda
>>   
>>   	  if (i == line && startswith (name, ".debug_line."))
> 
> ... i is used here instead, and hence I'd like to ask that you use ...
> 
>>   	    sec->name = name;
>> +	  else if (i == sframe && section->sh_type == SHT_GNU_SFRAME)
> 
> ... id here.
> 

Earlier in the function display_debug_section (), it does a :
       enum dwarf_section_display_enum  id = (enum 
dwarf_section_display_enum) i;

But I can change the code to use id for the two checks you point to above.

>> +	    sec->name = name;
>>   	  else if (streq (sec->uncompressed_name, name))
>>   	    sec->name = sec->uncompressed_name;
>>   	  else
> 
> Overall, doesn't this change mean that readelf (unlike objdump) would
> properly display multiple GNU_SFRAME sections of arbitrary names?
> 

No.  The current behavior with readelf is:

$ objcopy --rename-section .sframe=.sframe2 sort
$ readelf --sframe sort
readelf: Warning: Section '.sframe' was not dumped because it does not exist
$ readelf --sframe=.sframe sort
readelf: Warning: Section '.sframe' was not dumped because it does not exist

Thats because in options parsing in readelf, we do a :

> 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.  */
>   else if (optarg != NULL)
>     request_dump (dumpdata, SFRAME_DUMP);
>   else 
>     {
>       do_dump = true;
>       const char *sframe_sec_name = strdup (".sframe");
>       request_dump_byname (sframe_sec_name, SFRAME_DUMP);
>     }


That is, we force the default name of ".sframe", and then in 
initialise_dumps_byname () when it looks for a section with the desired 
name and none is found, an error is issued:

Section '.sframe' was not dumped because it does not exist

I will test a bit more and send a V2.

Thanks for reviewing