[RFD] gas: elf: binutils: add new section type SHT_GNU_SFRAME
Indu Bhagat
indu.bhagat@oracle.com
Mon Jul 7 06:15:47 GMT 2025
On 6/26/25 11:29 PM, Jan Beulich wrote:
> On 27.06.2025 01:28, Indu Bhagat wrote:
>> On 6/25/25 12:02 AM, Jan Beulich wrote:
>>> On 25.06.2025 08:06, Indu Bhagat via Binutils wrote:
>>>> Following up on one of the suggestions for SFrame to be a good ELF
>>>> citizen was for it to have its own section type. This patch tries to
>>>> address the concern by adding a new section type SHT_GNU_SFRAME.
>>>>
>>>> As SFrame sections are not yet enabled extensivley in user space (and
>>>> support in LLVM being added now), we have a window of opportunity now to
>>>> add the section type without causing much damage. But such a judgement
>>>> needs careful thought, hence the RFD nature of this proposal.
>>>>
>>>> So far, SFrame sections were of type SHT_PROGBITS.
>>>>
>>>> As per ELF specification, SHT_PROGBITS indicates that the section holds
>>>> information defined by the program, whose format and meaning are
>>>> determined solely by the program.
>>>>
>>>> On the linker side, SHT_PROGBITS should be reserved for the simple "cat
>>>> contents after applying relocs" semantics.
>>>>
>>>> Currently, the only way to know that a section contains SFrame stack
>>>> trace data is if consumer checks for section name. Such a check for
>>>> section name is not quite conformant to ELF principles.
>>>>
>>>> Some of this was discussed here
>>>> https://sourceware.org/pipermail/binutils/2025-March/140181.html
>>>>
>>>> With this change, SFrame sections generated by GNU Binutils will be of
>>>> type SHT_GNU_SFRAME. The new section type is defined in the
>>>> SHT_LOOS/SHT_HIOS space.
>>>>
>>>> The SFrame sections generated by gas, ld will have section type set to
>>>> SHT_GNU_SFRAME. The SFrame parsing routine _bfd_elf_parse_sframe () now
>>>> checks if the section type is not SHT_GNU_SFRAME.
>>>
>>> I think this is a good move, as already indicated in the earlier
>>> discussion.
>>
>> OK. I will aim for this to go in the 2.45 release.
>
> Sounds ambitious (especially with H.J.'s valid suggestion to make an attempt
> first to get this into gABI), but let's see how it goes.
>
>>>> ChangeLog:
>>>>
>>>> * bfd/elf-sframe.c (_bfd_elf_parse_sframe): Check if section
>>>> type is SHT_GNU_SFRAME.
>>>> (_bfd_elf_set_section_sframe): Set SHT_GNU_SFRAME for output
>>>> SFrame section.
>>>> * bfd/elflink.c (_bfd_elf_default_action_discarded): Use section
>>>> type for check instead of section name.
>>>> * bfd/elfxx-x86.c: Set SHT_GNU_SFRAME for SFrame sections for
>>>> .plt* sections.
>>>> * binutils/readelf.c (get_os_specific_section_type_name): Add
>>>> SHT_GNU_SFRAME.
>>>> * gas/dw2gencfi.c (cfi_finish): Set SHT_GNU_SFRAME for emitted
>>>> SFrame section.
>>>> * gas/testsuite/gas/cfi-sframe/cfi-sframe.exp: Add new test.
>>>> * gas/testsuite/gas/cfi-sframe/cfi-sframe-common-1b.d: New test.
>>>> * gas/testsuite/gas/cfi-sframe/cfi-sframe-common-1b.s: New test.
>>>>
>>>> include/ChangeLog:
>>>>
>>>> * elf/common.h (SHT_GNU_SFRAME): Add new section type for SFrame
>>>> stack trace information.
>>>>
>>>> libsframe/ChangeLog:
>>>> * libsframe/doc/sframe-spec.texi: Add expected ELF section type.
>>>
>>> Some NEWS files will also want adding a note to.
>>>
>>
>> OK. Added a news entry in gas/NEWS.
>>
>> * ELF SFrame sections now have section type set to SHT_GNU_SFRAME.
>>
>> I think adding in ld/NEWS or binutils/NEW is not required.
>
> binutils I agree. For ld I'm less sure - people may want to be warned
> that objects with plain PROGBITS .sframe won't work anymore. In the longer
> run, if both series make it into the same release, the PC-rel related NEWS
> entry may want to be folding with the aspect here (i.e. whichever series
> comes later would modify the already existing entry, rather than adding a
> new one).
>
OK. I have added a one liner to gas/NEWS and ld/NEWS.
>>>> --- a/binutils/readelf.c
>>>> +++ b/binutils/readelf.c
>>>> @@ -5964,6 +5964,7 @@ get_os_specific_section_type_name (Filedata * filedata, unsigned int sh_type)
>>>> case SHT_GNU_HASH: return "GNU_HASH";
>>>> case SHT_GNU_LIBLIST: return "GNU_LIBLIST";
>>>> case SHT_GNU_OBJECT_ONLY: return "GNU_OBJECT_ONLY";
>>>> + case SHT_GNU_SFRAME: return "SFRAME";
>>>
>>> Like the other ones, better emit the GNU_ prefix here as well?
>>
>> Not a biggie but we have
>>
>> In readelf.c (get_os_specific_segment_type):
>>
>> case PT_GNU_SFRAME: return "GNU_SFRAME";
>>
>> And some string without "GNU_" prefix:
>>
>> case SHT_GNU_verdef: return "VERDEF";
>> case SHT_GNU_verneed: return "VERNEED";
>> case SHT_GNU_versym: return "VERSYM";
>>
>> That said, in bfd/elf.c (get_segment_type use by
>> _bfd_elf_print_private_bfd_data), we have :
>>
>> case PT_GNU_SFRAME: pt = "SFRAME"; break;
>>
>> Either way, I have no strong preferences. If it is better to keep
>> string as "GNU_SFRAME" for both PT_GNU_SFRAME and SHT_GNU_SFRAME in
>> readelf.c, please let me know.
>
> Let's assume the case that we need to start with this being a GNU
> extension, yet it later becoming part of gABI. Then, to distinguish
> both cases in output, having the GNU_ prefix is going to be beneficial.
>
>>>> --- a/gas/dw2gencfi.c
>>>> +++ b/gas/dw2gencfi.c
>>>> @@ -2601,6 +2601,7 @@ cfi_finish (void)
>>>> flag_traditional_format = save_flag_traditional_format;
>>>> }
>>>>
>>>> +#ifdef OBJ_ELF
>>>> /* Generate SFrame section if the user specifies:
>>>> - the command line option to gas, or
>>>> - .sframe in the .cfi_sections directive. */
>>>> @@ -2615,11 +2616,13 @@ cfi_finish (void)
>>>> (SEC_ALLOC | SEC_LOAD | SEC_DATA
>>>> | DWARF2_EH_FRAME_READ_ONLY),
>>>> alignment);
>>>> + elf_section_type (sframe_seg) = SHT_GNU_SFRAME;
>>>> output_sframe (sframe_seg);
>>>> }
>>>> else
>>>> as_bad (_(".sframe not supported for target"));
>>>> }
>>>> +#endif
>>>
>>> The #ifdef addition may want splitting out and then widening. Both
>>> flag_gen_sframe and CFI_EMIT_sframe would look to better be ELF only.
>>
>> Sorry I dont understand the "splitting out and then widening".
>
> Make it a separate patch, and add similar #ifdef elsewhere as well.
>
I have proposed a patch where only the if block is guarded with
support_sframe_p. Guarding both if and else block with OBJ_ELF was
incorrect.
>>>> --- a/include/elf/common.h
>>>> +++ b/include/elf/common.h
>>>> @@ -553,6 +553,8 @@
>>>>
>>>> #define SHT_GNU_INCREMENTAL_INPUTS 0x6fff4700 /* Incremental build data */
>>>>
>>>> +#define SHT_GNU_SFRAME 0x6fff4800 /* SFrame stack trace information. */
>>>
>>> On what basis did you pick this? It's not adjacent to either of the
>>> other SHT_GNU_ values / value ranges. Imo we better wouldn't fragment
>>> the SHT_{LO,HI}OS range too much.
>>
>> My reasoning was to leave some room for any future additions that may
>> fit better in that space:
>> - more sections which are implemented in gold only, (like
>> SHT_GNU_INCREMENTAL_INPUTS), so they could be subsequent.
>> - more sections that share this property of "useful for linker
>> implementation".
>> - ...
>>
>> What is a good number to use ?
>
> Without knowing why SHT_GNU_INCREMENTAL_INPUTS wasn't made adjacent to
> the other SHT_GNU_*, it's not as easy to make a suggestion. Assuming this
> wasn't really done that way for a (long-term) good reason, I think
> SHT_LOOS + 0xffffff4 would be the prime candidate.
>
>>> Having reached the end - I expect it is intentional that you don't alter
>>> gas'es obj_elf_section_type()? Yet you realize that sections of any type
>>> (except SHT_NULL) can be created anyway by specifying a numeric value?
>>
>> Yes, my thinking was to have checks in ld around section type. I didnt
>> clearly see if adding further validation / handling around
>> SFrame/SHT_GNU_SFRAME to obj_elf_section () code path were worth it.
>
> The question is whether having explicit support for @sframe there is going
> to make sense. On one hand .sframe is supposed to be generated by the
> assembler, not (directly) by the programmer. Otoh making such a section is
> easy enough even without @sframe, by using the respective numeric value.
> Yet another option would be to outright refuse creating such a section,
> but that may get interesting to get right because you'd need to limit the
> refusal to respective ELFOSABI_* settings.
>
> Perhaps leaving the function alone is the right thing to do, just that you
> may want to say a word on this in the description.
>
OK. I have added some function-level comments to obj_elf_section to
convey this.
More information about the Binutils
mailing list