[RFC,4/4] ld: bfd: sframe: fix incorrect r_addend in RELA entries
Checks
Commit Message
With the fix in GAS, we now use a different PC-relative RELA for
updating the SFrame FDE function start address: The value is the offset
of the start PC of the function from the start of the SFrame section.
When RELAs are output (e.g. for relocatable links), there is need to
adjust the r_addend. This is because the r_addend values still have the
correct values for the _input_ SFrame section being linked / relocated.
The values must now be (before outputing the RELAs) with respect to the
_output_ SFrame section.
PS: This patch should be merged with the previous commits before final
commit (Otherwise the tests will fail). It is currently a separate
patch as I would like to check if this is OK to do. If such a "addend
fixup" is risky or wrong, it seems we will need a new type of RELOC for
SFrame sections.
bfd/
* elf-bfd.h: New declaration.
* elf-sframe.c (_bfd_elf_sframe_section_addend): New
definition.
* elflink.c (elf_link_input_bfd): Fixup the r_addend of
SFrame RELAs.
---
bfd/elf-bfd.h | 2 ++
bfd/elf-sframe.c | 31 +++++++++++++++++++++++++++++++
bfd/elflink.c | 24 ++++++++++++++++++++----
3 files changed, 53 insertions(+), 4 deletions(-)
Comments
On 08.03.2025 08:38, Indu Bhagat wrote:
> With the fix in GAS, we now use a different PC-relative RELA for
> updating the SFrame FDE function start address: The value is the offset
> of the start PC of the function from the start of the SFrame section.
>
> When RELAs are output (e.g. for relocatable links), there is need to
> adjust the r_addend. This is because the r_addend values still have the
> correct values for the _input_ SFrame section being linked / relocated.
> The values must now be (before outputing the RELAs) with respect to the
> _output_ SFrame section.
>
> PS: This patch should be merged with the previous commits before final
> commit (Otherwise the tests will fail). It is currently a separate
> patch as I would like to check if this is OK to do. If such a "addend
> fixup" is risky or wrong, it seems we will need a new type of RELOC for
> SFrame sections.
> diff --git a/bfd/elf-sframe.c b/bfd/elf-sframe.c
> @@ -574,6 +574,37 @@ _bfd_elf_sframe_section_offset (bfd *output_bfd ATTRIBUTE_UNUSED,
> + out_fde_idx * sizeof (sframe_func_desc_entry));
> }
>
> +/* Get the "canonicalized" addend for the symbol reference corresponding to the
> + relocation at RELOC_INDEX. E.g., for the following reloc for the SFrame
Nit: s/reloc/relocs/
> + FDE function start address:
> + Offset Type Sym. Name + Addend
> + 00000000001c R_X86_64_PC32 .text + 1c
> + 000000000030 R_X86_64_PC32 .text + 3b
> + The canonicalized addend are 0 and b respectively as the relocs are for
> + symbols (.text + 0) and (.text + b) respectively.
Maybe it would help to use the function symbols foo and bar in the example:
relocation at RELOC_INDEX. E.g., for the following annotated relocs for the SFrame
FDE function start address of FDE[0] and FDE[1] for functions foo and bar:
Offset Type Sym. Name + Addend
00000000001c R_X86_64_PC32 .text + 1c // FDE[0] for foo at .text + 0
000000000030 R_X86_64_PC32 .text + 3b // FDE[1] for bar at .text + b
With:
1c = 0 + sizeof(sframe_header) + 0 * sizeof(sframe_fde)
3b = b + sizeof(sframe_header) + 1 * sizeof(sframe_fde)
The canonicalized addend are 0 and b respectively as the relocs are for
symbols (.text + 0) and (.text + b) respectively.
> +
> + This is used to manually adjust the RELA addends to ensure correct values
> + for relocatable links. */
> +
> +bfd_vma
> +_bfd_elf_sframe_section_addend (bfd *output_bfd ATTRIBUTE_UNUSED,
> + struct bfd_link_info *info ATTRIBUTE_UNUSED,
> + asection *sec,
> + unsigned int reloc_index,
> + bfd_vma addend)
> +{
> + struct sframe_dec_info *sfd_info;
> +
> + if (sec->sec_info_type != SEC_INFO_TYPE_SFRAME)
> + return addend;
> +
> + sfd_info = (struct sframe_dec_info *) elf_section_data (sec)->sec_info;
> + BFD_ASSERT (sfd_info && sfd_info->sfd_ctx);
> +
> + return (addend - (sframe_decoder_get_hdr_size (sfd_info->sfd_ctx)
> + + reloc_index * sizeof (sframe_func_desc_entry)));
IIUC this is only valid as long as reloc_index == fde_index.
> +}
> +
> /* Write out the .sframe section. This must be called after
> _bfd_elf_merge_section_sframe has been called on all input
> .sframe sections. */
> diff --git a/bfd/elflink.c b/bfd/elflink.c
> index 895fbb0206e..6bce2966359 100644
> --- a/bfd/elflink.c
> +++ b/bfd/elflink.c
> @@ -11939,7 +11939,9 @@ elf_link_input_bfd (struct elf_final_link_info *flinfo, bfd *input_bfd)
> last_offset = o->output_offset;
> if (!bfd_link_relocatable (flinfo->info))
> last_offset += o->output_section->vma;
> - for (next_erel = 0; irela < irelaend; irela++, next_erel++)
> + unsigned int num_reloc = 0;
> + for (next_erel = 0; irela < irelaend;
> + irela++, next_erel++, num_reloc++)
The introduction of num_reloc could be omitted. See below.
> {
> unsigned long r_symndx;
> asection *sec;
> @@ -12070,10 +12072,24 @@ elf_link_input_bfd (struct elf_final_link_info *flinfo, bfd *input_bfd)
> }
> }
>
> - /* Adjust the addend according to where the
> - section winds up in the output section. */
> if (rela_normal)
> - irela->r_addend += sec->output_offset;
> + {
> + if (o->sec_info_type == SEC_INFO_TYPE_SFRAME)
> + {
unsigned int num_reloc = irela - internal_relocs;
> + bfd_vma addend
> + = _bfd_elf_sframe_section_addend (output_bfd,
> + flinfo->info, o,
> + num_reloc,
> + irela->r_addend);
> + /* Adjust the addend in the output RELA. The
> + input SFrame section has already been
> + relocated. */
> + irela->r_addend = addend + irela->r_offset;
> + }
> + /* Adjust the addend according to where the
> + section winds up in the output section. */
> + irela->r_addend += sec->output_offset;
> + }
> }
> else
> {
Regards,
Jens
On 10.03.2025 14:16, Jens Remus wrote:
> On 08.03.2025 08:38, Indu Bhagat wrote:
>> With the fix in GAS, we now use a different PC-relative RELA for
>> updating the SFrame FDE function start address: The value is the offset
>> of the start PC of the function from the start of the SFrame section.
>>
>> When RELAs are output (e.g. for relocatable links), there is need to
>> adjust the r_addend. This is because the r_addend values still have the
>> correct values for the _input_ SFrame section being linked / relocated.
>> The values must now be (before outputing the RELAs) with respect to the
>> _output_ SFrame section.
>>
>> PS: This patch should be merged with the previous commits before final
>> commit (Otherwise the tests will fail). It is currently a separate
>> patch as I would like to check if this is OK to do. If such a "addend
>> fixup" is risky or wrong, it seems we will need a new type of RELOC for
>> SFrame sections.
>
>> diff --git a/bfd/elf-sframe.c b/bfd/elf-sframe.c
>
>> @@ -574,6 +574,37 @@ _bfd_elf_sframe_section_offset (bfd *output_bfd ATTRIBUTE_UNUSED,
>> + out_fde_idx * sizeof (sframe_func_desc_entry));
>> }
>>
>> +/* Get the "canonicalized" addend for the symbol reference corresponding to the
>> + relocation at RELOC_INDEX. E.g., for the following reloc for the SFrame
>
> Nit: s/reloc/relocs/
>
>> + FDE function start address:
>> + Offset Type Sym. Name + Addend
>> + 00000000001c R_X86_64_PC32 .text + 1c
>> + 000000000030 R_X86_64_PC32 .text + 3b
>> + The canonicalized addend are 0 and b respectively as the relocs are for
>> + symbols (.text + 0) and (.text + b) respectively.
>
> Maybe it would help to use the function symbols foo and bar in the example:
>
> relocation at RELOC_INDEX. E.g., for the following annotated relocs for the SFrame
> FDE function start address of FDE[0] and FDE[1] for functions foo and bar:
> Offset Type Sym. Name + Addend
> 00000000001c R_X86_64_PC32 .text + 1c // FDE[0] for foo at .text + 0
> 000000000030 R_X86_64_PC32 .text + 3b // FDE[1] for bar at .text + b
> With:
> 1c = 0 + sizeof(sframe_header) + 0 * sizeof(sframe_fde)
> 3b = b + sizeof(sframe_header) + 1 * sizeof(sframe_fde)
> The canonicalized addend are 0 and b respectively as the relocs are for
> symbols (.text + 0) and (.text + b) respectively.
Hmm, wait - there's a mix between .text and .sframe then, isn't there? That
would explain why custom handling is then necessary. Yet as said in the other
mail, custom handling should not be necessary. Not the least because other
ELF-consuming tools also need to work, not just what GNU binutils provides.
Jan
Hi Jan.
> On 10.03.2025 14:16, Jens Remus wrote:
>> On 08.03.2025 08:38, Indu Bhagat wrote:
>>> With the fix in GAS, we now use a different PC-relative RELA for
>>> updating the SFrame FDE function start address: The value is the offset
>>> of the start PC of the function from the start of the SFrame section.
>>>
>>> When RELAs are output (e.g. for relocatable links), there is need to
>>> adjust the r_addend. This is because the r_addend values still have the
>>> correct values for the _input_ SFrame section being linked / relocated.
>>> The values must now be (before outputing the RELAs) with respect to the
>>> _output_ SFrame section.
>>>
>>> PS: This patch should be merged with the previous commits before final
>>> commit (Otherwise the tests will fail). It is currently a separate
>>> patch as I would like to check if this is OK to do. If such a "addend
>>> fixup" is risky or wrong, it seems we will need a new type of RELOC for
>>> SFrame sections.
>>
>>> diff --git a/bfd/elf-sframe.c b/bfd/elf-sframe.c
>>
>>> @@ -574,6 +574,37 @@ _bfd_elf_sframe_section_offset (bfd *output_bfd ATTRIBUTE_UNUSED,
>>> + out_fde_idx * sizeof (sframe_func_desc_entry));
>>> }
>>>
>>> +/* Get the "canonicalized" addend for the symbol reference corresponding to the
>>> + relocation at RELOC_INDEX. E.g., for the following reloc for the SFrame
>>
>> Nit: s/reloc/relocs/
>>
>>> + FDE function start address:
>>> + Offset Type Sym. Name + Addend
>>> + 00000000001c R_X86_64_PC32 .text + 1c
>>> + 000000000030 R_X86_64_PC32 .text + 3b
>>> + The canonicalized addend are 0 and b respectively as the relocs are for
>>> + symbols (.text + 0) and (.text + b) respectively.
>>
>> Maybe it would help to use the function symbols foo and bar in the example:
>>
>> relocation at RELOC_INDEX. E.g., for the following annotated relocs for the SFrame
>> FDE function start address of FDE[0] and FDE[1] for functions foo and bar:
>> Offset Type Sym. Name + Addend
>> 00000000001c R_X86_64_PC32 .text + 1c // FDE[0] for foo at .text + 0
>> 000000000030 R_X86_64_PC32 .text + 3b // FDE[1] for bar at .text + b
>> With:
>> 1c = 0 + sizeof(sframe_header) + 0 * sizeof(sframe_fde)
>> 3b = b + sizeof(sframe_header) + 1 * sizeof(sframe_fde)
>> The canonicalized addend are 0 and b respectively as the relocs are for
>> symbols (.text + 0) and (.text + b) respectively.
>
> Hmm, wait - there's a mix between .text and .sframe then, isn't there? That
> would explain why custom handling is then necessary. Yet as said in the other
> mail, custom handling should not be necessary. Not the least because other
> ELF-consuming tools also need to work, not just what GNU binutils provides.
[At a risk of being reiterative, but this also helps to my own
understanding, because this is all very confusing :)]
I think there is nothing special about that "mix". As far as I can tell
this is just one rather peculiar particular case of a PCrel reloc.
Suppose you have the typical:
.text
...
ldinsn OFFSET
.data
...
somedata:
And then you install a relocation for the instruction's offset, which is
PC-relative. You use an expression like:
ldinsn somedata
Then I would expect GAS to emit a PCrel with:
.text.rela:
P: offset_of_ldins_within_text
S: .data
A: offset_of_somedata_within_data
With resolving expression:
S + A - P
to which the linker also adds .text, resulting in the expected:
.data + offset_of_somedata_within_data - .text + offset_of_ldins_within_text
i.e. the distance between somedata and the instruction which field is to
be patched.
Now, for SFrame, the expression used wants to denote the distance
between the beginning of some given function and the beginning of the
.sframe section containing the FDE to patch. GAS is given the
expression:
.text + text_offset - .sframe
And the assembler smartly uses a PCrel relocation (it has no other
choice I suppose) such as:
.sframe.rela:
P: offset_of_fde_within_sframe
S: .text
A: offset_of_function_within_text + offset_of_fde_within_sframe
~~~~~~~~~~~~~~~~~~~~~~~~~~~
[I have been trying to find the code in GAS that actually does this,
because it seems to me it must be some ad-hoc code, but couln't find
it.]
Which is a little weird, because it has the additional
offset_of_fde_within_sframe, intended to basically annull the same value
in P in the resolving function:
S + A - P
which is, with the linker-added .sframe:
.text + (offset_of_funtion_within_text + offset_of_fde_within_sframe)
- .sframe + offset_of_fde_within_sframe
which becomes:
.text + offset_of_funtion_within_text - .sframe
which is the desired distance between the beginning of the function and
the beginning of the .sframe section.
I don't think ELF consuming tools that just apply relocations need any
special code to solve these relocs.
But due how the SFrame format works, as Jens explained, tools that merge
SFrame input sections into a relocatable output section need to
basically recreate the relocations for the FDEs in the output. If this
merging consisted (like with ehframe) just in collating the input
sections together, then no special casing would be needed. But merging
SFrame sections imply that the offset of the FDEs wrt the containing
output .sframe will likely be different. This offset is embedded in the
input relocation's offset and addend, as shown above. So the merging
tool has no other choice that to "remove" the old FDE offset from the
input relocation (from both addend and offset) and "insert" the new FDE
offset (in the resulting merged .sframe section in the output) in both
addend and offset. Tools that merge SFrame sections must use
sframe-specific logic anyway, so I don't think this is a big deal...
The other changes in generic code in Indu's patch, as far as I can see,
are necessary because the linker assumes that the input section contents
are merged by stocking them (or a subset of them) in the merged output
section. This is not so in SFrame, so thats why Indu's patch makes ld
to not increase that offset within the output section every iteration in
the loop, if the sections being merged are .sframe. It is simply that
until now no format required this sort of merging, where I11,I12,I13 and
I21,I22,I23 areas in two input sections will be merged into for example
I11,I21,I12,I22,I13,I23 in the output section.
On 11.03.2025 10:26, Jose E. Marchesi wrote:
>> On 10.03.2025 14:16, Jens Remus wrote:
>>> On 08.03.2025 08:38, Indu Bhagat wrote:
>>>> With the fix in GAS, we now use a different PC-relative RELA for
>>>> updating the SFrame FDE function start address: The value is the offset
>>>> of the start PC of the function from the start of the SFrame section.
>>>>
>>>> When RELAs are output (e.g. for relocatable links), there is need to
>>>> adjust the r_addend. This is because the r_addend values still have the
>>>> correct values for the _input_ SFrame section being linked / relocated.
>>>> The values must now be (before outputing the RELAs) with respect to the
>>>> _output_ SFrame section.
>>>>
>>>> PS: This patch should be merged with the previous commits before final
>>>> commit (Otherwise the tests will fail). It is currently a separate
>>>> patch as I would like to check if this is OK to do. If such a "addend
>>>> fixup" is risky or wrong, it seems we will need a new type of RELOC for
>>>> SFrame sections.
>>>
>>>> diff --git a/bfd/elf-sframe.c b/bfd/elf-sframe.c
>>>
>>>> @@ -574,6 +574,37 @@ _bfd_elf_sframe_section_offset (bfd *output_bfd ATTRIBUTE_UNUSED,
>>>> + out_fde_idx * sizeof (sframe_func_desc_entry));
>>>> }
>>>>
>>>> +/* Get the "canonicalized" addend for the symbol reference corresponding to the
>>>> + relocation at RELOC_INDEX. E.g., for the following reloc for the SFrame
>>>
>>> Nit: s/reloc/relocs/
>>>
>>>> + FDE function start address:
>>>> + Offset Type Sym. Name + Addend
>>>> + 00000000001c R_X86_64_PC32 .text + 1c
>>>> + 000000000030 R_X86_64_PC32 .text + 3b
>>>> + The canonicalized addend are 0 and b respectively as the relocs are for
>>>> + symbols (.text + 0) and (.text + b) respectively.
>>>
>>> Maybe it would help to use the function symbols foo and bar in the example:
>>>
>>> relocation at RELOC_INDEX. E.g., for the following annotated relocs for the SFrame
>>> FDE function start address of FDE[0] and FDE[1] for functions foo and bar:
>>> Offset Type Sym. Name + Addend
>>> 00000000001c R_X86_64_PC32 .text + 1c // FDE[0] for foo at .text + 0
>>> 000000000030 R_X86_64_PC32 .text + 3b // FDE[1] for bar at .text + b
>>> With:
>>> 1c = 0 + sizeof(sframe_header) + 0 * sizeof(sframe_fde)
>>> 3b = b + sizeof(sframe_header) + 1 * sizeof(sframe_fde)
>>> The canonicalized addend are 0 and b respectively as the relocs are for
>>> symbols (.text + 0) and (.text + b) respectively.
>>
>> Hmm, wait - there's a mix between .text and .sframe then, isn't there? That
>> would explain why custom handling is then necessary. Yet as said in the other
>> mail, custom handling should not be necessary. Not the least because other
>> ELF-consuming tools also need to work, not just what GNU binutils provides.
>
> [At a risk of being reiterative, but this also helps to my own
> understanding, because this is all very confusing :)]
>
> I think there is nothing special about that "mix". As far as I can tell
> this is just one rather peculiar particular case of a PCrel reloc.
> Suppose you have the typical:
>
> .text
> ...
> ldinsn OFFSET
>
> .data
> ...
> somedata:
>
> And then you install a relocation for the instruction's offset, which is
> PC-relative. You use an expression like:
>
> ldinsn somedata
>
> Then I would expect GAS to emit a PCrel with:
>
> .text.rela:
> P: offset_of_ldins_within_text
> S: .data
> A: offset_of_somedata_within_data
>
> With resolving expression:
>
> S + A - P
>
> to which the linker also adds .text, resulting in the expected:
>
> .data + offset_of_somedata_within_data - .text + offset_of_ldins_within_text
>
> i.e. the distance between somedata and the instruction which field is to
> be patched.
>
> Now, for SFrame, the expression used wants to denote the distance
> between the beginning of some given function and the beginning of the
> .sframe section containing the FDE to patch. GAS is given the
> expression:
>
> .text + text_offset - .sframe
>
> And the assembler smartly uses a PCrel relocation (it has no other
> choice I suppose) such as:
>
> .sframe.rela:
> P: offset_of_fde_within_sframe
> S: .text
> A: offset_of_function_within_text + offset_of_fde_within_sframe
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~
I don't think this can work: You can't have offsets within two different
sections added together. When doing a relocatable link, _both_ offsets
may change. S+A has to describe the target of the relocation.
> [I have been trying to find the code in GAS that actually does this,
> because it seems to me it must be some ad-hoc code, but couln't find
> it.]
>
> Which is a little weird, because it has the additional
> offset_of_fde_within_sframe, intended to basically annull the same value
> in P in the resolving function:
>
> S + A - P
>
> which is, with the linker-added .sframe:
>
> .text + (offset_of_funtion_within_text + offset_of_fde_within_sframe)
> - .sframe + offset_of_fde_within_sframe
>
> which becomes:
>
> .text + offset_of_funtion_within_text - .sframe
>
> which is the desired distance between the beginning of the function and
> the beginning of the .sframe section.
>
> I don't think ELF consuming tools that just apply relocations need any
> special code to solve these relocs.
>
> But due how the SFrame format works, as Jens explained, tools that merge
> SFrame input sections into a relocatable output section need to
> basically recreate the relocations for the FDEs in the output.
But that's what a random ELF consuming tool wouldn't do. I.e. relocations
here want expressing in a way that even e.g. GNU ld pre-dating the
introduction of .sframe can properly handle them, whether or not it is
passed the -r option.
Of course there may be situations where this is entirely impossible. But
I very much hope here it isn't one of those. (If it was, measures would
imo need taking to make sure unaware tools would diagnose occurrences
rather than silently doing the wrong thing.)
> If this
> merging consisted (like with ehframe) just in collating the input
> sections together, then no special casing would be needed. But merging
> SFrame sections imply that the offset of the FDEs wrt the containing
> output .sframe will likely be different.
Is this really different from .eh_frame?
> This offset is embedded in the
> input relocation's offset and addend, as shown above. So the merging
> tool has no other choice that to "remove" the old FDE offset from the
> input relocation (from both addend and offset) and "insert" the new FDE
> offset (in the resulting merged .sframe section in the output) in both
> addend and offset. Tools that merge SFrame sections must use
> sframe-specific logic anyway, so I don't think this is a big deal...
As said above - as long as .sframe are ordinary PROGBITS sections, unaware
tools should be fine to use.
> The other changes in generic code in Indu's patch, as far as I can see,
> are necessary because the linker assumes that the input section contents
> are merged by stocking them (or a subset of them) in the merged output
> section. This is not so in SFrame, so thats why Indu's patch makes ld
> to not increase that offset within the output section every iteration in
> the loop, if the sections being merged are .sframe. It is simply that
> until now no format required this sort of merging, where I11,I12,I13 and
> I21,I22,I23 areas in two input sections will be merged into for example
> I11,I21,I12,I22,I13,I23 in the output section.
This part I fear I'm not following. Why would there be any re-ordering of
input "fragments"? What specifically do I11 etc denominate? Are sections
being taken apart and then - kind of randomly - re-assembled?
Jan
On 11.03.2025 10:26, Jose E. Marchesi wrote:
>> On 10.03.2025 14:16, Jens Remus wrote:
>>> On 08.03.2025 08:38, Indu Bhagat wrote:
>>>> With the fix in GAS, we now use a different PC-relative RELA for
>>>> updating the SFrame FDE function start address: The value is the offset
>>>> of the start PC of the function from the start of the SFrame section.
>>>>
>>>> When RELAs are output (e.g. for relocatable links), there is need to
>>>> adjust the r_addend. This is because the r_addend values still have the
>>>> correct values for the _input_ SFrame section being linked / relocated.
>>>> The values must now be (before outputing the RELAs) with respect to the
>>>> _output_ SFrame section.
>>>>
>>>> PS: This patch should be merged with the previous commits before final
>>>> commit (Otherwise the tests will fail). It is currently a separate
>>>> patch as I would like to check if this is OK to do. If such a "addend
>>>> fixup" is risky or wrong, it seems we will need a new type of RELOC for
>>>> SFrame sections.
>>>
>>>> diff --git a/bfd/elf-sframe.c b/bfd/elf-sframe.c
>>>
>>>> @@ -574,6 +574,37 @@ _bfd_elf_sframe_section_offset (bfd *output_bfd ATTRIBUTE_UNUSED,
>>>> + out_fde_idx * sizeof (sframe_func_desc_entry));
>>>> }
>>>>
>>>> +/* Get the "canonicalized" addend for the symbol reference corresponding to the
>>>> + relocation at RELOC_INDEX. E.g., for the following reloc for the SFrame
>>>
>>> Nit: s/reloc/relocs/
>>>
>>>> + FDE function start address:
>>>> + Offset Type Sym. Name + Addend
>>>> + 00000000001c R_X86_64_PC32 .text + 1c
>>>> + 000000000030 R_X86_64_PC32 .text + 3b
>>>> + The canonicalized addend are 0 and b respectively as the relocs are for
>>>> + symbols (.text + 0) and (.text + b) respectively.
>>>
>>> Maybe it would help to use the function symbols foo and bar in the example:
>>>
>>> relocation at RELOC_INDEX. E.g., for the following annotated relocs for the SFrame
>>> FDE function start address of FDE[0] and FDE[1] for functions foo and bar:
>>> Offset Type Sym. Name + Addend
>>> 00000000001c R_X86_64_PC32 .text + 1c // FDE[0] for foo at .text + 0
>>> 000000000030 R_X86_64_PC32 .text + 3b // FDE[1] for bar at .text + b
>>> With:
>>> 1c = 0 + sizeof(sframe_header) + 0 * sizeof(sframe_fde)
>>> 3b = b + sizeof(sframe_header) + 1 * sizeof(sframe_fde)
>>> The canonicalized addend are 0 and b respectively as the relocs are for
>>> symbols (.text + 0) and (.text + b) respectively.
>>
>> Hmm, wait - there's a mix between .text and .sframe then, isn't there? That
>> would explain why custom handling is then necessary. Yet as said in the other
>> mail, custom handling should not be necessary. Not the least because other
>> ELF-consuming tools also need to work, not just what GNU binutils provides.
>
> [At a risk of being reiterative, but this also helps to my own
> understanding, because this is all very confusing :)]
>
> I think there is nothing special about that "mix". As far as I can tell
> this is just one rather peculiar particular case of a PCrel reloc.
> Suppose you have the typical:
>
> .text
> ...
> ldinsn OFFSET
>
> .data
> ...
> somedata:
>
> And then you install a relocation for the instruction's offset, which is
> PC-relative. You use an expression like:
>
> ldinsn somedata
>
> Then I would expect GAS to emit a PCrel with:
>
> .text.rela:
> P: offset_of_ldins_within_text
> S: .data
> A: offset_of_somedata_within_data
>
> With resolving expression:
>
> S + A - P
>
> to which the linker also adds .text, resulting in the expected:
>
> .data + offset_of_somedata_within_data - .text + offset_of_ldins_within_text
>
> i.e. the distance between somedata and the instruction which field is to
> be patched.
Isn't above in real world use cases actually already like what you
describe below for SFrame, in regard that the addend often consists of
the sum of the target offset and some source offset?
On s390x PC-relative addressing is relative to the instruction, so that:
.text.rela:
P: offset_of_ldins_within_text
S: .data
A: offset_of_somedata_within_data + offset_of_field_within_ldins
$ echo "brasl %r14,func@PLT" | as && objdump -dr a.out
...
0: c0 e5 00 00 00 00 brasl %r14,0x0
2: R_390_PLT32DBL func+0x2
~~~~
The +0x2 is the offset_of_field_within_ldins, which is very similar
the offset_of_fde_within_sframe in your below SFrame explanation.
On x86-64 PC-relative addressing is relative to the next instruction,
which makes it a bit less apparent:
$ echo "call func@PLT" | as && objdump -dr a.out
...
0: e8 00 00 00 00 call 0x5
1: R_X86_64_PLT32 func-0x4
~~~~
The -0x4 is the offset_of_field_from_insn_following_ldins.
> Now, for SFrame, the expression used wants to denote the distance
> between the beginning of some given function and the beginning of the
> .sframe section containing the FDE to patch. GAS is given the
> expression:
>
> .text + text_offset - .sframe
>
> And the assembler smartly uses a PCrel relocation (it has no other
> choice I suppose) such as:
>
> .sframe.rela:
> P: offset_of_fde_within_sframe
> S: .text
> A: offset_of_function_within_text + offset_of_fde_within_sframe
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> [I have been trying to find the code in GAS that actually does this,
> because it seems to me it must be some ad-hoc code, but couln't find
> it.]
>
> Which is a little weird, because it has the additional
> offset_of_fde_within_sframe, intended to basically annull the same value
> in P in the resolving function:
>
> S + A - P
>
> which is, with the linker-added .sframe:
>
> .text + (offset_of_funtion_within_text + offset_of_fde_within_sframe)
> - .sframe + offset_of_fde_within_sframe
>
> which becomes:
>
> .text + offset_of_funtion_within_text - .sframe
>
> which is the desired distance between the beginning of the function and
> the beginning of the .sframe section.
>
> I don't think ELF consuming tools that just apply relocations need any
> special code to solve these relocs.
I agree. See the s390x and x86-64 examples above, which do not require
any special processing.
> But due how the SFrame format works, as Jens explained, tools that merge
> SFrame input sections into a relocatable output section need to
> basically recreate the relocations for the FDEs in the output. If this
> merging consisted (like with ehframe) just in collating the input
> sections together, then no special casing would be needed. But merging
> SFrame sections imply that the offset of the FDEs wrt the containing
> output .sframe will likely be different. This offset is embedded in the
> input relocation's offset and addend, as shown above. So the merging
> tool has no other choice that to "remove" the old FDE offset from the
> input relocation (from both addend and offset) and "insert" the new FDE
> offset (in the resulting merged .sframe section in the output) in both
> addend and offset. Tools that merge SFrame sections must use
> sframe-specific logic anyway, so I don't think this is a big deal...
>
> The other changes in generic code in Indu's patch, as far as I can see,
> are necessary because the linker assumes that the input section contents
> are merged by stocking them (or a subset of them) in the merged output
> section. This is not so in SFrame, so thats why Indu's patch makes ld
> to not increase that offset within the output section every iteration in
> the loop, if the sections being merged are .sframe. It is simply that
> until now no format required this sort of merging, where I11,I12,I13 and
> I21,I22,I23 areas in two input sections will be merged into for example
> I11,I21,I12,I22,I13,I23 in the output section.
Very good explanation! This is the main difference, why SFrame requires
special processing - but only for merging of .sframe sections.
Regards,
Jens
On 3/11/25 5:22 AM, Jan Beulich wrote:
> On 11.03.2025 10:26, Jose E. Marchesi wrote:
>>> On 10.03.2025 14:16, Jens Remus wrote:
>>>> On 08.03.2025 08:38, Indu Bhagat wrote:
>>>>> With the fix in GAS, we now use a different PC-relative RELA for
>>>>> updating the SFrame FDE function start address: The value is the offset
>>>>> of the start PC of the function from the start of the SFrame section.
>>>>>
>>>>> When RELAs are output (e.g. for relocatable links), there is need to
>>>>> adjust the r_addend. This is because the r_addend values still have the
>>>>> correct values for the _input_ SFrame section being linked / relocated.
>>>>> The values must now be (before outputing the RELAs) with respect to the
>>>>> _output_ SFrame section.
>>>>>
>>>>> PS: This patch should be merged with the previous commits before final
>>>>> commit (Otherwise the tests will fail). It is currently a separate
>>>>> patch as I would like to check if this is OK to do. If such a "addend
>>>>> fixup" is risky or wrong, it seems we will need a new type of RELOC for
>>>>> SFrame sections.
>>>>
>>>>> diff --git a/bfd/elf-sframe.c b/bfd/elf-sframe.c
>>>>
>>>>> @@ -574,6 +574,37 @@ _bfd_elf_sframe_section_offset (bfd *output_bfd ATTRIBUTE_UNUSED,
>>>>> + out_fde_idx * sizeof (sframe_func_desc_entry));
>>>>> }
>>>>>
>>>>> +/* Get the "canonicalized" addend for the symbol reference corresponding to the
>>>>> + relocation at RELOC_INDEX. E.g., for the following reloc for the SFrame
>>>>
>>>> Nit: s/reloc/relocs/
>>>>
>>>>> + FDE function start address:
>>>>> + Offset Type Sym. Name + Addend
>>>>> + 00000000001c R_X86_64_PC32 .text + 1c
>>>>> + 000000000030 R_X86_64_PC32 .text + 3b
>>>>> + The canonicalized addend are 0 and b respectively as the relocs are for
>>>>> + symbols (.text + 0) and (.text + b) respectively.
>>>>
>>>> Maybe it would help to use the function symbols foo and bar in the example:
>>>>
>>>> relocation at RELOC_INDEX. E.g., for the following annotated relocs for the SFrame
>>>> FDE function start address of FDE[0] and FDE[1] for functions foo and bar:
>>>> Offset Type Sym. Name + Addend
>>>> 00000000001c R_X86_64_PC32 .text + 1c // FDE[0] for foo at .text + 0
>>>> 000000000030 R_X86_64_PC32 .text + 3b // FDE[1] for bar at .text + b
>>>> With:
>>>> 1c = 0 + sizeof(sframe_header) + 0 * sizeof(sframe_fde)
>>>> 3b = b + sizeof(sframe_header) + 1 * sizeof(sframe_fde)
>>>> The canonicalized addend are 0 and b respectively as the relocs are for
>>>> symbols (.text + 0) and (.text + b) respectively.
>>>
>>> Hmm, wait - there's a mix between .text and .sframe then, isn't there? That
>>> would explain why custom handling is then necessary. Yet as said in the other
>>> mail, custom handling should not be necessary. Not the least because other
>>> ELF-consuming tools also need to work, not just what GNU binutils provides.
>>
>> [At a risk of being reiterative, but this also helps to my own
>> understanding, because this is all very confusing :)]
>>
>> I think there is nothing special about that "mix". As far as I can tell
>> this is just one rather peculiar particular case of a PCrel reloc.
>> Suppose you have the typical:
>>
>> .text
>> ...
>> ldinsn OFFSET
>>
>> .data
>> ...
>> somedata:
>>
>> And then you install a relocation for the instruction's offset, which is
>> PC-relative. You use an expression like:
>>
>> ldinsn somedata
>>
>> Then I would expect GAS to emit a PCrel with:
>>
>> .text.rela:
>> P: offset_of_ldins_within_text
>> S: .data
>> A: offset_of_somedata_within_data
>>
>> With resolving expression:
>>
>> S + A - P
>>
>> to which the linker also adds .text, resulting in the expected:
>>
>> .data + offset_of_somedata_within_data - .text + offset_of_ldins_within_text
>>
>> i.e. the distance between somedata and the instruction which field is to
>> be patched.
>>
>> Now, for SFrame, the expression used wants to denote the distance
>> between the beginning of some given function and the beginning of the
>> .sframe section containing the FDE to patch. GAS is given the
>> expression:
>>
>> .text + text_offset - .sframe
>>
>> And the assembler smartly uses a PCrel relocation (it has no other
>> choice I suppose) such as:
>>
>> .sframe.rela:
>> P: offset_of_fde_within_sframe
>> S: .text
>> A: offset_of_function_within_text + offset_of_fde_within_sframe
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> I don't think this can work: You can't have offsets within two different
> sections added together. When doing a relocatable link, _both_ offsets
> may change. S+A has to describe the target of the relocation.
>
>> [I have been trying to find the code in GAS that actually does this,
>> because it seems to me it must be some ad-hoc code, but couln't find
>> it.]
>>
>> Which is a little weird, because it has the additional
>> offset_of_fde_within_sframe, intended to basically annull the same value
>> in P in the resolving function:
>>
>> S + A - P
>>
>> which is, with the linker-added .sframe:
>>
>> .text + (offset_of_funtion_within_text + offset_of_fde_within_sframe)
>> - .sframe + offset_of_fde_within_sframe
>>
>> which becomes:
>>
>> .text + offset_of_funtion_within_text - .sframe
>>
>> which is the desired distance between the beginning of the function and
>> the beginning of the .sframe section.
>>
>> I don't think ELF consuming tools that just apply relocations need any
>> special code to solve these relocs.
>>
>> But due how the SFrame format works, as Jens explained, tools that merge
>> SFrame input sections into a relocatable output section need to
>> basically recreate the relocations for the FDEs in the output.
>
> But that's what a random ELF consuming tool wouldn't do. I.e. relocations
> here want expressing in a way that even e.g. GNU ld pre-dating the
> introduction of .sframe can properly handle them, whether or not it is
> passed the -r option.
>
> Of course there may be situations where this is entirely impossible. But
> I very much hope here it isn't one of those. (If it was, measures would
> imo need taking to make sure unaware tools would diagnose occurrences
> rather than silently doing the wrong thing.)
>
>> If this
>> merging consisted (like with ehframe) just in collating the input
>> sections together, then no special casing would be needed. But merging
>> SFrame sections imply that the offset of the FDEs wrt the containing
>> output .sframe will likely be different.
>
> Is this really different from .eh_frame?
>
Yes, SFrame merging is different from .eh_frame.
See some notes at the end of this email as the answer is related to your
question there...
>> This offset is embedded in the
>> input relocation's offset and addend, as shown above. So the merging
>> tool has no other choice that to "remove" the old FDE offset from the
>> input relocation (from both addend and offset) and "insert" the new FDE
>> offset (in the resulting merged .sframe section in the output) in both
>> addend and offset. Tools that merge SFrame sections must use
>> sframe-specific logic anyway, so I don't think this is a big deal...
>
> As said above - as long as .sframe are ordinary PROGBITS sections, unaware
> tools should be fine to use.
>
>> The other changes in generic code in Indu's patch, as far as I can see,
>> are necessary because the linker assumes that the input section contents
>> are merged by stocking them (or a subset of them) in the merged output
>> section. This is not so in SFrame, so thats why Indu's patch makes ld
>> to not increase that offset within the output section every iteration in
>> the loop, if the sections being merged are .sframe. It is simply that
>> until now no format required this sort of merging, where I11,I12,I13 and
>> I21,I22,I23 areas in two input sections will be merged into for example
>> I11,I21,I12,I22,I13,I23 in the output section.
>
> This part I fear I'm not following. Why would there be any re-ordering of
> input "fragments"? What specifically do I11 etc denominate? Are sections
> being taken apart and then - kind of randomly - re-assembled?
>
Reusing the figure used earlier in another thread, I have renamed FDE
entries to show what I11, I12 etc denominate. Basically I1X are FDEs
from input 1, I2x are FDEs from input 2...
--------------------- ---------------------
| SFrame Header | | SFrame Header |
--------------------- ---------------------
| SFrame FDE I11 | (Linking) | SFrame FDE I11 |
| SFrame FDE I12 | | SFrame FDE I12 |
| ... | |------>| ... |
| SFrame FDE I1n | | | SFrame FDE I1n |
--------------------- | | SFrame FDE I21 |
| ... | ------ | SFrame FDE I22 |
|SFrame FREs (Sec 1)| | | ... |
|(Frame Row Entries)| | | SFrame FDE I2n |
| ... | | |-------------------|
| ... | | | ... |
--------------------- | |SFrame FREs (Sec 1)|
| |SFrame FREs (Sec 2)|
| |(Frame Row Entries)|
| | ... |
--------------------- | ---------------------
| SFrame Header | |
--------------------- |
| SFrame FDE I21 | |
| SFrame FDE I22 | |
| ... | ------
| SFrame FDE I2n |
---------------------
| ... |
|SFrame FREs (Sec 2)|
|(Frame Row Entries)|
| ... |
| ... |
---------------------
Note how the SFrame FDEs in the output section are placed at the
beginning of the output section.
This is in contrast to EH_Frame where the CIE/FDE _as_a_single_unit_is
simply placed at an offset in the output section. In SFrame the stack
trace data is split, with there being ONE SFrame FDE + N SFrame FREs per
function.
The first member of the SFrame FDE is the function start address, which
has the relocation.
On 14.03.2025 05:48, Indu Bhagat wrote:
> On 3/11/25 5:22 AM, Jan Beulich wrote:
>> On 11.03.2025 10:26, Jose E. Marchesi wrote:
>>>> On 10.03.2025 14:16, Jens Remus wrote:
>>>>> On 08.03.2025 08:38, Indu Bhagat wrote:
>>>>>> With the fix in GAS, we now use a different PC-relative RELA for
>>>>>> updating the SFrame FDE function start address: The value is the offset
>>>>>> of the start PC of the function from the start of the SFrame section.
>>>>>>
>>>>>> When RELAs are output (e.g. for relocatable links), there is need to
>>>>>> adjust the r_addend. This is because the r_addend values still have the
>>>>>> correct values for the _input_ SFrame section being linked / relocated.
>>>>>> The values must now be (before outputing the RELAs) with respect to the
>>>>>> _output_ SFrame section.
>>>>>>
>>>>>> PS: This patch should be merged with the previous commits before final
>>>>>> commit (Otherwise the tests will fail). It is currently a separate
>>>>>> patch as I would like to check if this is OK to do. If such a "addend
>>>>>> fixup" is risky or wrong, it seems we will need a new type of RELOC for
>>>>>> SFrame sections.
>>>>>
>>>>>> diff --git a/bfd/elf-sframe.c b/bfd/elf-sframe.c
>>>>>
>>>>>> @@ -574,6 +574,37 @@ _bfd_elf_sframe_section_offset (bfd *output_bfd ATTRIBUTE_UNUSED,
>>>>>> + out_fde_idx * sizeof (sframe_func_desc_entry));
>>>>>> }
>>>>>>
>>>>>> +/* Get the "canonicalized" addend for the symbol reference corresponding to the
>>>>>> + relocation at RELOC_INDEX. E.g., for the following reloc for the SFrame
>>>>>
>>>>> Nit: s/reloc/relocs/
>>>>>
>>>>>> + FDE function start address:
>>>>>> + Offset Type Sym. Name + Addend
>>>>>> + 00000000001c R_X86_64_PC32 .text + 1c
>>>>>> + 000000000030 R_X86_64_PC32 .text + 3b
>>>>>> + The canonicalized addend are 0 and b respectively as the relocs are for
>>>>>> + symbols (.text + 0) and (.text + b) respectively.
>>>>>
>>>>> Maybe it would help to use the function symbols foo and bar in the example:
>>>>>
>>>>> relocation at RELOC_INDEX. E.g., for the following annotated relocs for the SFrame
>>>>> FDE function start address of FDE[0] and FDE[1] for functions foo and bar:
>>>>> Offset Type Sym. Name + Addend
>>>>> 00000000001c R_X86_64_PC32 .text + 1c // FDE[0] for foo at .text + 0
>>>>> 000000000030 R_X86_64_PC32 .text + 3b // FDE[1] for bar at .text + b
>>>>> With:
>>>>> 1c = 0 + sizeof(sframe_header) + 0 * sizeof(sframe_fde)
>>>>> 3b = b + sizeof(sframe_header) + 1 * sizeof(sframe_fde)
>>>>> The canonicalized addend are 0 and b respectively as the relocs are for
>>>>> symbols (.text + 0) and (.text + b) respectively.
>>>>
>>>> Hmm, wait - there's a mix between .text and .sframe then, isn't there? That
>>>> would explain why custom handling is then necessary. Yet as said in the other
>>>> mail, custom handling should not be necessary. Not the least because other
>>>> ELF-consuming tools also need to work, not just what GNU binutils provides.
>>>
>>> [At a risk of being reiterative, but this also helps to my own
>>> understanding, because this is all very confusing :)]
>>>
>>> I think there is nothing special about that "mix". As far as I can tell
>>> this is just one rather peculiar particular case of a PCrel reloc.
>>> Suppose you have the typical:
>>>
>>> .text
>>> ...
>>> ldinsn OFFSET
>>>
>>> .data
>>> ...
>>> somedata:
>>>
>>> And then you install a relocation for the instruction's offset, which is
>>> PC-relative. You use an expression like:
>>>
>>> ldinsn somedata
>>>
>>> Then I would expect GAS to emit a PCrel with:
>>>
>>> .text.rela:
>>> P: offset_of_ldins_within_text
>>> S: .data
>>> A: offset_of_somedata_within_data
>>>
>>> With resolving expression:
>>>
>>> S + A - P
>>>
>>> to which the linker also adds .text, resulting in the expected:
>>>
>>> .data + offset_of_somedata_within_data - .text + offset_of_ldins_within_text
>>>
>>> i.e. the distance between somedata and the instruction which field is to
>>> be patched.
>>>
>>> Now, for SFrame, the expression used wants to denote the distance
>>> between the beginning of some given function and the beginning of the
>>> .sframe section containing the FDE to patch. GAS is given the
>>> expression:
>>>
>>> .text + text_offset - .sframe
>>>
>>> And the assembler smartly uses a PCrel relocation (it has no other
>>> choice I suppose) such as:
>>>
>>> .sframe.rela:
>>> P: offset_of_fde_within_sframe
>>> S: .text
>>> A: offset_of_function_within_text + offset_of_fde_within_sframe
>>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> I don't think this can work: You can't have offsets within two different
>> sections added together. When doing a relocatable link, _both_ offsets
>> may change. S+A has to describe the target of the relocation.
>>
>>> [I have been trying to find the code in GAS that actually does this,
>>> because it seems to me it must be some ad-hoc code, but couln't find
>>> it.]
>>>
>>> Which is a little weird, because it has the additional
>>> offset_of_fde_within_sframe, intended to basically annull the same value
>>> in P in the resolving function:
>>>
>>> S + A - P
>>>
>>> which is, with the linker-added .sframe:
>>>
>>> .text + (offset_of_funtion_within_text + offset_of_fde_within_sframe)
>>> - .sframe + offset_of_fde_within_sframe
>>>
>>> which becomes:
>>>
>>> .text + offset_of_funtion_within_text - .sframe
>>>
>>> which is the desired distance between the beginning of the function and
>>> the beginning of the .sframe section.
>>>
>>> I don't think ELF consuming tools that just apply relocations need any
>>> special code to solve these relocs.
>>>
>>> But due how the SFrame format works, as Jens explained, tools that merge
>>> SFrame input sections into a relocatable output section need to
>>> basically recreate the relocations for the FDEs in the output.
>>
>> But that's what a random ELF consuming tool wouldn't do. I.e. relocations
>> here want expressing in a way that even e.g. GNU ld pre-dating the
>> introduction of .sframe can properly handle them, whether or not it is
>> passed the -r option.
>>
>> Of course there may be situations where this is entirely impossible. But
>> I very much hope here it isn't one of those. (If it was, measures would
>> imo need taking to make sure unaware tools would diagnose occurrences
>> rather than silently doing the wrong thing.)
>>
>>> If this
>>> merging consisted (like with ehframe) just in collating the input
>>> sections together, then no special casing would be needed. But merging
>>> SFrame sections imply that the offset of the FDEs wrt the containing
>>> output .sframe will likely be different.
>>
>> Is this really different from .eh_frame?
>>
>
> Yes, SFrame merging is different from .eh_frame.
>
> See some notes at the end of this email as the answer is related to your
> question there...
>
>>> This offset is embedded in the
>>> input relocation's offset and addend, as shown above. So the merging
>>> tool has no other choice that to "remove" the old FDE offset from the
>>> input relocation (from both addend and offset) and "insert" the new FDE
>>> offset (in the resulting merged .sframe section in the output) in both
>>> addend and offset. Tools that merge SFrame sections must use
>>> sframe-specific logic anyway, so I don't think this is a big deal...
>>
>> As said above - as long as .sframe are ordinary PROGBITS sections, unaware
>> tools should be fine to use.
>>
>>> The other changes in generic code in Indu's patch, as far as I can see,
>>> are necessary because the linker assumes that the input section contents
>>> are merged by stocking them (or a subset of them) in the merged output
>>> section. This is not so in SFrame, so thats why Indu's patch makes ld
>>> to not increase that offset within the output section every iteration in
>>> the loop, if the sections being merged are .sframe. It is simply that
>>> until now no format required this sort of merging, where I11,I12,I13 and
>>> I21,I22,I23 areas in two input sections will be merged into for example
>>> I11,I21,I12,I22,I13,I23 in the output section.
>>
>> This part I fear I'm not following. Why would there be any re-ordering of
>> input "fragments"? What specifically do I11 etc denominate? Are sections
>> being taken apart and then - kind of randomly - re-assembled?
>>
>
> Reusing the figure used earlier in another thread, I have renamed FDE
> entries to show what I11, I12 etc denominate. Basically I1X are FDEs
> from input 1, I2x are FDEs from input 2...
>
>
>
> --------------------- ---------------------
> | SFrame Header | | SFrame Header |
> --------------------- ---------------------
> | SFrame FDE I11 | (Linking) | SFrame FDE I11 |
> | SFrame FDE I12 | | SFrame FDE I12 |
> | ... | |------>| ... |
> | SFrame FDE I1n | | | SFrame FDE I1n |
> --------------------- | | SFrame FDE I21 |
> | ... | ------ | SFrame FDE I22 |
> |SFrame FREs (Sec 1)| | | ... |
> |(Frame Row Entries)| | | SFrame FDE I2n |
> | ... | | |-------------------|
> | ... | | | ... |
> --------------------- | |SFrame FREs (Sec 1)|
> | |SFrame FREs (Sec 2)|
> | |(Frame Row Entries)|
> | | ... |
> --------------------- | ---------------------
> | SFrame Header | |
> --------------------- |
> | SFrame FDE I21 | |
> | SFrame FDE I22 | |
> | ... | ------
> | SFrame FDE I2n |
> ---------------------
> | ... |
> |SFrame FREs (Sec 2)|
> |(Frame Row Entries)|
> | ... |
> | ... |
> ---------------------
>
> Note how the SFrame FDEs in the output section are placed at the
> beginning of the output section.
>
> This is in contrast to EH_Frame where the CIE/FDE _as_a_single_unit_is
> simply placed at an offset in the output section. In SFrame the stack
> trace data is split, with there being ONE SFrame FDE + N SFrame FREs per
> function.
Hmm, this is pretty odd imo. There should have been two section kinds (or
maybe even three, seeing that the SFrame headers end up fully folded in
the diagram above), which would be combined only when linking final
binaries. I now regret that I didn't look more closely at all of this when
it was first introduced.
Jan
Hello,
On Fri, 14 Mar 2025, Jan Beulich wrote:
> > Note how the SFrame FDEs in the output section are placed at the
> > beginning of the output section.
> >
> > This is in contrast to EH_Frame where the CIE/FDE _as_a_single_unit_is
> > simply placed at an offset in the output section. In SFrame the stack
> > trace data is split, with there being ONE SFrame FDE + N SFrame FREs
> > per function.
>
> Hmm, this is pretty odd imo. There should have been two section kinds (or
> maybe even three, seeing that the SFrame headers end up fully folded in
> the diagram above), which would be combined only when linking final
> binaries.
That's true. But overall this section rewriting by the link editor
(instead of just cat-ing selected individual sections together into one
output blob) is not something new. string section merging is doing the
same on a smaller scale, and relocations into those need to be rewritten
in a similar vain, and in particular by understanding the section
content/format (in string sections that's of course very simple, but it's
still interpreting the section content).
I don't know if the sframe merging is optional or not (like string merging
is), to retain normal reloc behaviour it better should be (so that, as you
say, unaware tools would still do the right thing). But either way,
there's precedent for reloc-mangling based on section content.
> I now regret that I didn't look more closely at all of this when it was
> first introduced.
(But yes, it would have been nice if this special handling hadn't been
necessary :-/ )
Ciao,
Michael.
On 24.03.2025 16:53, Michael Matz wrote:
> Hello,
>
> On Fri, 14 Mar 2025, Jan Beulich wrote:
>
>>> Note how the SFrame FDEs in the output section are placed at the
>>> beginning of the output section.
>>>
>>> This is in contrast to EH_Frame where the CIE/FDE _as_a_single_unit_is
>>> simply placed at an offset in the output section. In SFrame the stack
>>> trace data is split, with there being ONE SFrame FDE + N SFrame FREs
>>> per function.
>>
>> Hmm, this is pretty odd imo. There should have been two section kinds (or
>> maybe even three, seeing that the SFrame headers end up fully folded in
>> the diagram above), which would be combined only when linking final
>> binaries.
>
> That's true. But overall this section rewriting by the link editor
> (instead of just cat-ing selected individual sections together into one
> output blob) is not something new. string section merging is doing the
> same on a smaller scale, and relocations into those need to be rewritten
> in a similar vain, and in particular by understanding the section
> content/format (in string sections that's of course very simple, but it's
> still interpreting the section content).
>
> I don't know if the sframe merging is optional or not (like string merging
> is), to retain normal reloc behaviour it better should be (so that, as you
> say, unaware tools would still do the right thing). But either way,
> there's precedent for reloc-mangling based on section content.
Yes and no. SHF_MERGE (alone or together with SHF_STRING) properly describes
the contents, as to what needs doing to it when linking. Whereas here each
tool needs to learn what to do about sections of a certain, special name,
which aren't even otherwise recognizable or distinguishable.
Jan
Hello,
On Mon, 24 Mar 2025, Jan Beulich wrote:
> > That's true. But overall this section rewriting by the link editor
> > (instead of just cat-ing selected individual sections together into one
> > output blob) is not something new. string section merging is doing the
> > same on a smaller scale, and relocations into those need to be rewritten
> > in a similar vain, and in particular by understanding the section
> > content/format (in string sections that's of course very simple, but it's
> > still interpreting the section content).
> >
> > I don't know if the sframe merging is optional or not (like string merging
> > is), to retain normal reloc behaviour it better should be (so that, as you
> > say, unaware tools would still do the right thing). But either way,
> > there's precedent for reloc-mangling based on section content.
>
> Yes and no. SHF_MERGE (alone or together with SHF_STRING) properly describes
> the contents, as to what needs doing to it when linking. Whereas here each
> tool needs to learn what to do about sections of a certain, special name,
> which aren't even otherwise recognizable or distinguishable.
Yes, a separate ELF section type would have been definitely proper here
:-/
Ciao,
Michael.
(and sorry for reviving an old thread, I had some backlog and didn't
notice)
On 3/10/25 6:16 AM, Jens Remus wrote:
> On 08.03.2025 08:38, Indu Bhagat wrote:
>> With the fix in GAS, we now use a different PC-relative RELA for
>> updating the SFrame FDE function start address: The value is the offset
>> of the start PC of the function from the start of the SFrame section.
>>
>> When RELAs are output (e.g. for relocatable links), there is need to
>> adjust the r_addend. This is because the r_addend values still have the
>> correct values for the _input_ SFrame section being linked / relocated.
>> The values must now be (before outputing the RELAs) with respect to the
>> _output_ SFrame section.
>>
>> PS: This patch should be merged with the previous commits before final
>> commit (Otherwise the tests will fail). It is currently a separate
>> patch as I would like to check if this is OK to do. If such a "addend
>> fixup" is risky or wrong, it seems we will need a new type of RELOC for
>> SFrame sections.
>
>> diff --git a/bfd/elf-sframe.c b/bfd/elf-sframe.c
>
>> @@ -574,6 +574,37 @@ _bfd_elf_sframe_section_offset (bfd *output_bfd
>> ATTRIBUTE_UNUSED,
>> + out_fde_idx * sizeof (sframe_func_desc_entry));
>> }
>> +/* Get the "canonicalized" addend for the symbol reference
>> corresponding to the
>> + relocation at RELOC_INDEX. E.g., for the following reloc for the
>> SFrame
>
> Nit: s/reloc/relocs/
>
Done.
>> + FDE function start address:
>> + Offset Type Sym. Name + Addend
>> + 00000000001c R_X86_64_PC32 .text + 1c
>> + 000000000030 R_X86_64_PC32 .text + 3b
>> + The canonicalized addend are 0 and b respectively as the relocs
>> are for
>> + symbols (.text + 0) and (.text + b) respectively.
>
> Maybe it would help to use the function symbols foo and bar in the example:
>
> relocation at RELOC_INDEX. E.g., for the following annotated
> relocs for the SFrame
> FDE function start address of FDE[0] and FDE[1] for functions foo
> and bar:
> Offset Type Sym. Name + Addend
> 00000000001c R_X86_64_PC32 .text + 1c // FDE[0] for foo
> at .text + 0
> 000000000030 R_X86_64_PC32 .text + 3b // FDE[1] for bar
> at .text + b
> With:
> 1c = 0 + sizeof(sframe_header) + 0 * sizeof(sframe_fde)
> 3b = b + sizeof(sframe_header) + 1 * sizeof(sframe_fde)
> The canonicalized addend are 0 and b respectively as the relocs are for
> symbols (.text + 0) and (.text + b) respectively.
>
Amended.
>> +
>> + This is used to manually adjust the RELA addends to ensure correct
>> values
>> + for relocatable links. */
>> +
>> +bfd_vma
>> +_bfd_elf_sframe_section_addend (bfd *output_bfd ATTRIBUTE_UNUSED,
>> + struct bfd_link_info *info ATTRIBUTE_UNUSED,
>> + asection *sec,
>> + unsigned int reloc_index,
>> + bfd_vma addend)
>> +{
>> + struct sframe_dec_info *sfd_info;
>> +
>> + if (sec->sec_info_type != SEC_INFO_TYPE_SFRAME)
>> + return addend;
>> +
>> + sfd_info = (struct sframe_dec_info *) elf_section_data (sec)-
>> >sec_info;
>> + BFD_ASSERT (sfd_info && sfd_info->sfd_ctx);
>> +
>> + return (addend - (sframe_decoder_get_hdr_size (sfd_info->sfd_ctx)
>> + + reloc_index * sizeof (sframe_func_desc_entry)));
>
> IIUC this is only valid as long as reloc_index == fde_index.
>
Yes, and reloc_index should be the fde_index for input sections. Do you
see any issue ? I can add a comment around the same.
>> +}
>> +
>> /* Write out the .sframe section. This must be called after
>> _bfd_elf_merge_section_sframe has been called on all input
>> .sframe sections. */
>> diff --git a/bfd/elflink.c b/bfd/elflink.c
>> index 895fbb0206e..6bce2966359 100644
>> --- a/bfd/elflink.c
>> +++ b/bfd/elflink.c
>> @@ -11939,7 +11939,9 @@ elf_link_input_bfd (struct elf_final_link_info
>> *flinfo, bfd *input_bfd)
>> last_offset = o->output_offset;
>> if (!bfd_link_relocatable (flinfo->info))
>> last_offset += o->output_section->vma;
>> - for (next_erel = 0; irela < irelaend; irela++, next_erel++)
>> + unsigned int num_reloc = 0;
>> + for (next_erel = 0; irela < irelaend;
>> + irela++, next_erel++, num_reloc++)
>
> The introduction of num_reloc could be omitted. See below.
>
Done.
And thanks for reviewing.
>> {
>> unsigned long r_symndx;
>> asection *sec;
>> @@ -12070,10 +12072,24 @@ elf_link_input_bfd (struct
>> elf_final_link_info *flinfo, bfd *input_bfd)
>> }
>> }
>> - /* Adjust the addend according to where the
>> - section winds up in the output section. */
>> if (rela_normal)
>> - irela->r_addend += sec->output_offset;
>> + {
>> + if (o->sec_info_type == SEC_INFO_TYPE_SFRAME)
>> + {
>
> unsigned int num_reloc = irela - internal_relocs;
>
>> + bfd_vma addend
>> + = _bfd_elf_sframe_section_addend (output_bfd,
>> + flinfo->info, o,
>> + num_reloc,
>> + irela->r_addend);
>> + /* Adjust the addend in the output RELA. The
>> + input SFrame section has already been
>> + relocated. */
>> + irela->r_addend = addend + irela->r_offset;
>> + }
>> + /* Adjust the addend according to where the
>> + section winds up in the output section. */
>> + irela->r_addend += sec->output_offset;
>> + }
>> }
>> else
>> {
>
> Regards,
> Jens
On 3/24/25 9:08 AM, Michael Matz wrote:
> Hello,
>
Hi,
Thanks for chiming in.
> On Mon, 24 Mar 2025, Jan Beulich wrote:
>
>>> That's true. But overall this section rewriting by the link editor
>>> (instead of just cat-ing selected individual sections together into one
>>> output blob) is not something new. string section merging is doing the
>>> same on a smaller scale, and relocations into those need to be rewritten
>>> in a similar vain, and in particular by understanding the section
>>> content/format (in string sections that's of course very simple, but it's
>>> still interpreting the section content).
>>>
>>> I don't know if the sframe merging is optional or not (like string merging
>>> is), to retain normal reloc behaviour it better should be (so that, as you
>>> say, unaware tools would still do the right thing). But either way,
>>> there's precedent for reloc-mangling based on section content.
>>
SFrame merging is not optional. If simply concatenated, the output
SFrame section is not really correct SFrame information. As per
specification, there is one SFrame Header, N SFrame FDEs (function
descriptor entrues), M SFrame FREs (frame row entries). The SFrame FDE
and FRE are different semantic units of the stack trace data.
>> Yes and no. SHF_MERGE (alone or together with SHF_STRING) properly describes
>> the contents, as to what needs doing to it when linking. Whereas here each
>> tool needs to learn what to do about sections of a certain, special name,
>> which aren't even otherwise recognizable or distinguishable.
>
> Yes, a separate ELF section type would have been definitely proper here
> :-/
>
What would such a ELF section type want to indicate? That this is a say
SHT_SFRAME ? I am thinking anything more generic to accommodate (in
hindsight), say .eh_frame , or .ctf (or .BTF and more in future), in one
section flag will not work, as the merging semantics for each of these
sections are quite different:
- .eh_frame uses relocations (PC-rel)
- .ctf does not use relocations but the types are de-duplicated when
merging.
- .BTF, IIUC, will need some relocations. And similar to .ctf, there
will be de-duplication too.
On 24.03.2025 20:54, Indu Bhagat wrote:
> On 3/24/25 9:08 AM, Michael Matz wrote:
>> Yes, a separate ELF section type would have been definitely proper here
>> :-/
>>
>
> What would such a ELF section type want to indicate? That this is a say
> SHT_SFRAME ? I am thinking anything more generic to accommodate (in
> hindsight), say .eh_frame , or .ctf (or .BTF and more in future), in one
> section flag will not work, as the merging semantics for each of these
> sections are quite different:
>
> - .eh_frame uses relocations (PC-rel)
> - .ctf does not use relocations but the types are de-duplicated when
> merging.
> - .BTF, IIUC, will need some relocations. And similar to .ctf, there
> will be de-duplication too.
At least .eh_frame should also have had its own SHT_*, imo. Yet that's
clearly too late now. Plus the gABI was in nearly unmaintained state for
far too long. Whether the same goes for .ctf and .btf I don't know, as I
know too little about those features.
Jan
Hello,
On Mon, 24 Mar 2025, Indu Bhagat wrote:
> >>> I don't know if the sframe merging is optional or not (like string
> >>> merging is), to retain normal reloc behaviour it better should be
> >>> (so that, as you say, unaware tools would still do the right thing).
> >>> But either way, there's precedent for reloc-mangling based on
> >>> section content.
>
> SFrame merging is not optional. If simply concatenated, the output
> SFrame section is not really correct SFrame information.
I see. That's not ideal, but workable. Ideally the format would have
been designed such that a trivial link editor cat-ing the relevent
sections would create something working (albeit larger than necessary),
and the clever merging would merely be an optimization. So, ...
> As per specification, there is one SFrame Header, N SFrame FDEs
> (function descriptor entrues), M SFrame FREs (frame row entries). The
> SFrame FDE and FRE are different semantic units of the stack trace data.
... something that would keep the FRE and FDE entries in separate sections
in the .o files, with appropriate cross-section relocation to refer to
each other. The final linked format (where segments, not sections,
matter) could still specify that they need to be placed next to each other
(all FDEs, then all FREs). Alas, the ship has sailed, so we'll need to
make do :)
> > Yes, a separate ELF section type would have been definitely proper here
> > :-/
>
> What would such a ELF section type want to indicate? That this is a say
> SHT_SFRAME ?
Yes. Basically every section that has structured content that a link
editor usefully (or necessarily as here) needs to parse (and mangle) to do
its thing should have a separate SHT_ type.
The mental model for ELF and any extensions of it should be that section
names don't matter: when one designs something for the ELF file format,
and a thought experiment that nulls all section names from input .o files
reveals that something doesn't quite work or is awkward to do for the link
editor, then the design isn't yet complete. Every time someone does a
strcmp between sh_name and a const string a kitten dies!
SHT_PROGBITS should be reserved for the simple "cat contents after
applying relocs" semantics (the flags will say into which segment to cat
the contents). Of course, that ship also has sailed for many things in
existence, but it would be Really Nice if new improvements would follow
that model (again).
> I am thinking anything more generic to accommodate (in
> hindsight), say .eh_frame , or .ctf (or .BTF and more in future), in one
> section flag will not work, as the merging semantics for each of these
> sections are quite different:
>
> - .eh_frame uses relocations (PC-rel)
> - .ctf does not use relocations but the types are de-duplicated when
> merging.
> - .BTF, IIUC, will need some relocations. And similar to .ctf, there will
> be de-duplication too.
Different structured content would need different section types.
There are 0x60000000 of them and we've used 20. (and there are more in
the OS and arch range, and then even more after all those). Section flags
are fewer, those shouldn't be used lightly, but section types: go wild.
eh_frame is in the middle ground actually: SHT_PROGBITS does work, one can
simply cat all content and the result "works". It's not optimal, because
such simple link editor wouldn't build up eh_frame_hdr (because it can't
inspect the "unstructured" content), and hence it's slow to access at
runtime, but it works. It would have been better if it had its own
section type, but that's definitely too late now.
Perhaps its not too late for ctf and friends, I don't know.
(But again: it's not the specifics of relocations that should influence
the decision to create a new section type, it's the structure of content
of sections and the need for link editors to peek into it that wants a
section type. Peeking into content of course might mean that relocations
need to be taken into account, including rewriting them if the content
isn't just peeked into but actively changed)
Ciao,
Michael.
On 24.03.2025 19:11, Indu Bhagat wrote:
> On 3/10/25 6:16 AM, Jens Remus wrote:
>> On 08.03.2025 08:38, Indu Bhagat wrote:
>>> +bfd_vma
>>> +_bfd_elf_sframe_section_addend (bfd *output_bfd ATTRIBUTE_UNUSED,
>>> + struct bfd_link_info *info ATTRIBUTE_UNUSED,
>>> + asection *sec,
>>> + unsigned int reloc_index,
>>> + bfd_vma addend)
>>> +{
>>> + struct sframe_dec_info *sfd_info;
>>> +
>>> + if (sec->sec_info_type != SEC_INFO_TYPE_SFRAME)
>>> + return addend;
>>> +
>>> + sfd_info = (struct sframe_dec_info *) elf_section_data (sec)- >sec_info;
>>> + BFD_ASSERT (sfd_info && sfd_info->sfd_ctx);
>>> +
>>> + return (addend - (sframe_decoder_get_hdr_size (sfd_info->sfd_ctx)
>>> + + reloc_index * sizeof (sframe_func_desc_entry)));
>>
>> IIUC this is only valid as long as reloc_index == fde_index.
>>
>
> Yes, and reloc_index should be the fde_index for input sections. Do
> you see any issue ? I can add a comment around the same.
Thanks! No, not necessary,I just wanted to reassure that I understand
how/why it works.
Regards,
Jens
@@ -2551,6 +2551,8 @@ extern bool _bfd_elf_merge_section_sframe
(bfd *, struct bfd_link_info *, asection *, bfd_byte *);
extern bfd_vma _bfd_elf_sframe_section_offset
(bfd *, struct bfd_link_info *, asection *, bfd_vma);
+extern bfd_vma _bfd_elf_sframe_section_addend
+ (bfd *, struct bfd_link_info *, asection *, unsigned int, bfd_vma);
extern bool _bfd_elf_write_section_sframe
(bfd *, struct bfd_link_info *);
extern bool _bfd_elf_set_section_sframe (bfd *, struct bfd_link_info *);
@@ -574,6 +574,37 @@ _bfd_elf_sframe_section_offset (bfd *output_bfd ATTRIBUTE_UNUSED,
+ out_fde_idx * sizeof (sframe_func_desc_entry));
}
+/* Get the "canonicalized" addend for the symbol reference corresponding to the
+ relocation at RELOC_INDEX. E.g., for the following reloc for the SFrame
+ FDE function start address:
+ Offset Type Sym. Name + Addend
+ 00000000001c R_X86_64_PC32 .text + 1c
+ 000000000030 R_X86_64_PC32 .text + 3b
+ The canonicalized addend are 0 and b respectively as the relocs are for
+ symbols (.text + 0) and (.text + b) respectively.
+
+ This is used to manually adjust the RELA addends to ensure correct values
+ for relocatable links. */
+
+bfd_vma
+_bfd_elf_sframe_section_addend (bfd *output_bfd ATTRIBUTE_UNUSED,
+ struct bfd_link_info *info ATTRIBUTE_UNUSED,
+ asection *sec,
+ unsigned int reloc_index,
+ bfd_vma addend)
+{
+ struct sframe_dec_info *sfd_info;
+
+ if (sec->sec_info_type != SEC_INFO_TYPE_SFRAME)
+ return addend;
+
+ sfd_info = (struct sframe_dec_info *) elf_section_data (sec)->sec_info;
+ BFD_ASSERT (sfd_info && sfd_info->sfd_ctx);
+
+ return (addend - (sframe_decoder_get_hdr_size (sfd_info->sfd_ctx)
+ + reloc_index * sizeof (sframe_func_desc_entry)));
+}
+
/* Write out the .sframe section. This must be called after
_bfd_elf_merge_section_sframe has been called on all input
.sframe sections. */
@@ -11939,7 +11939,9 @@ elf_link_input_bfd (struct elf_final_link_info *flinfo, bfd *input_bfd)
last_offset = o->output_offset;
if (!bfd_link_relocatable (flinfo->info))
last_offset += o->output_section->vma;
- for (next_erel = 0; irela < irelaend; irela++, next_erel++)
+ unsigned int num_reloc = 0;
+ for (next_erel = 0; irela < irelaend;
+ irela++, next_erel++, num_reloc++)
{
unsigned long r_symndx;
asection *sec;
@@ -12070,10 +12072,24 @@ elf_link_input_bfd (struct elf_final_link_info *flinfo, bfd *input_bfd)
}
}
- /* Adjust the addend according to where the
- section winds up in the output section. */
if (rela_normal)
- irela->r_addend += sec->output_offset;
+ {
+ if (o->sec_info_type == SEC_INFO_TYPE_SFRAME)
+ {
+ bfd_vma addend
+ = _bfd_elf_sframe_section_addend (output_bfd,
+ flinfo->info, o,
+ num_reloc,
+ irela->r_addend);
+ /* Adjust the addend in the output RELA. The
+ input SFrame section has already been
+ relocated. */
+ irela->r_addend = addend + irela->r_offset;
+ }
+ /* Adjust the addend according to where the
+ section winds up in the output section. */
+ irela->r_addend += sec->output_offset;
+ }
}
else
{