[RFD] gas: elf: binutils: add new section type SHT_GNU_SFRAME
Indu Bhagat
indu.bhagat@oracle.com
Thu Jun 26 23:28:18 GMT 2025
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.
>> 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.
>> --- a/bfd/elf-sframe.c
>> +++ b/bfd/elf-sframe.c
>> @@ -197,6 +197,16 @@ _bfd_elf_parse_sframe (bfd *abfd,
>> /* This file does not contain .sframe information. */
>> return false;
>> }
>> + /* Prior versions of assembler and ld were generating SFrame sections with
>> + section type SHT_PROGBITS. Issue an error for lack of support for such
>> + objects now. */
>> + else if (elf_section_type (sec) != SHT_GNU_SFRAME)
>> + {
>> + _bfd_error_handler
>> + (_("error in %pB(%pA); unexpected SFrame section type"),
>> + abfd, sec);
>> + return false;
>> + }
>
> I think this would better be the first check, as e.g. even a zero-size
> .sframe section ought to to have the correct type now. Also may I (again)
> ask to avoid "else" in situations like this one?
>
Thanks, makes sense. Moved this to be first check. And no else for the
subsequent check.
>> @@ -296,8 +306,7 @@ _bfd_elf_discard_section_sframe
>> BFD ABFD. Returns true if no error. */
>>
>> bool
>> -_bfd_elf_set_section_sframe (bfd *abfd,
>> - struct bfd_link_info *info)
>> +_bfd_elf_set_section_sframe (bfd *abfd, struct bfd_link_info *info)
>> {
>
> Looks like an unrelated change?
>
Yes, minor formatting fix since I was touching the function.
>> @@ -305,6 +314,7 @@ _bfd_elf_set_section_sframe (bfd *abfd,
>> if (!cfsec)
>> return false;
>>
>> + elf_section_type (cfsec) = SHT_GNU_SFRAME;
>> elf_sframe (abfd) = cfsec;
>>
>> return true;
>
> Aren't there more changes needed here? There could be multiple .sframe
> sections, one of which being of the correct type. Tangentially, what
> if there are multiple ones (of the correct type)?
>
The name of API is a misnomer (sorry). It sets the sframe section in
the output_bfd to be easily accessible by doing a
elf_sframe (abfd) = cfsec;
There is only one output SFrame section. For the linker generated .plt*
sections (which are eventually merged into the output section), we are
explicitly setting the section type in the relevant place.
>> --- 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.
>> --- 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".
>> --- 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 ?
>> --- a/libsframe/doc/sframe-spec.texi
>> +++ b/libsframe/doc/sframe-spec.texi
>> @@ -77,9 +77,10 @@ Appendices
>> @section Overview
>> @cindex Overview
>>
>> -The SFrame stack trace information is provided in a loaded section, known as the
>> -@code{.sframe} section. When available, the @code{.sframe} section appears in
>> -a new segment of its own, PT_GNU_SFRAME.
>> +The SFrame stack trace information is provided in a loaded section, known as
>> +the @code{.sframe} section. When available, the @code{.sframe} section appears
>> +in a new segment of its own, PT_GNU_SFRAME. An ELF SFrame section will have
>> +the type SHT_GNU_SFRAME.
>>
>> The SFrame format is currently supported only for select ABIs, namely, AMD64
>> and AAPCS64.
>
> 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.
Thanks for reviewing
More information about the Binutils
mailing list