diff mbox series

[RFC,SCHEME_B,1/7] ld: sframe: emit function start addr as offset from FDE

Message ID 20250407002559.6593-2-indu.bhagat@oracle.com
State New
Headers
Series Fix relocatable links with SFrame section |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 warning Skipped upon request
linaro-tcwg-bot/tcwg_binutils_build--master-arm warning Skipped upon request

Commit Message

Indu Bhagat April 7, 2025, 12:25 a.m. UTC 
  Adopt the new semantics of sfde_func_start_address consistently.  For
ld, this means even the in-memory contents of the FDE function start
address (buffer passed to libsframe sframe_encoder_write () for writing
out) are encoded in the new semantics.

ChangeLog:

        * bfd/elf-sframe.c (_bfd_elf_merge_section_sframe): Adopt new
	semantics of function start address encoding.
---
 bfd/elf-sframe.c | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Jens Remus April 7, 2025, 4:10 p.m. UTC | #1 
On 07.04.2025 02:25, Indu Bhagat wrote:
> Adopt the new semantics of sfde_func_start_address consistently.  For
> ld, this means even the in-memory contents of the FDE function start
> address (buffer passed to libsframe sframe_encoder_write () for writing
> out) are encoded in the new semantics.

> diff --git a/bfd/elf-sframe.c b/bfd/elf-sframe.c

> @@ -481,6 +481,13 @@ _bfd_elf_merge_section_sframe (bfd *abfd,
>   		address += sframe_read_value (abfd, contents,
>   					      pltn_r_offset, 4);
>   	      address += (sec->output_offset + r_offset);

It took me a while to understand why above does not get canceled out
by below and does not naturally fall into place if both were removed.
After all the SFrame FDE function start address field value should
now already be an offset to the function from the field itself.
IIUC address is an offset relative to the FDE in the "imaginary"
output .sframe section, as the linker assumes it to be, that is the
sections simply concatenated together, with alignment padding added
between concatenated input sections if needed.  Above makes address
an offset relative to the output .sframe section, which effectively
"removes" this "imaginary" linker view from the value.  Below then
makes it relative to the FDE of the planned output .sframe section,
as SFrame assumes it to be (prior to sorting of the FDEs), that is
one single header followed by all FDEs, followed by all FREs.

Is that correct?

> +	      /* SFrame FDE function start address is an offset from the
> +		 sfde_func_start_address field to the start PC.  The
> +		 calculation below is the distance of sfde_func_start_address
> +		 field from the start of the output SFrame section.  */
> +	      address -= (sframe_encoder_get_hdr_size (sfe_ctx)
> +			  + ((cur_fidx + num_enc_fidx) /* FDE index.  */
> +			     * sizeof (sframe_func_desc_entry)));

Thanks and regards,
Jens
Indu Bhagat April 8, 2025, 2:38 a.m. UTC | #2 
On 4/7/25 9:10 AM, Jens Remus wrote:
> On 07.04.2025 02:25, Indu Bhagat wrote:
>> Adopt the new semantics of sfde_func_start_address consistently.  For
>> ld, this means even the in-memory contents of the FDE function start
>> address (buffer passed to libsframe sframe_encoder_write () for writing
>> out) are encoded in the new semantics.
> 
>> diff --git a/bfd/elf-sframe.c b/bfd/elf-sframe.c
> 
>> @@ -481,6 +481,13 @@ _bfd_elf_merge_section_sframe (bfd *abfd,
>>           address += sframe_read_value (abfd, contents,
>>                             pltn_r_offset, 4);
>>             address += (sec->output_offset + r_offset);
> 
> It took me a while to understand why above does not get canceled out
> by below and does not naturally fall into place if both were removed.
> After all the SFrame FDE function start address field value should
> now already be an offset to the function from the field itself.
> IIUC address is an offset relative to the FDE in the "imaginary"
> output .sframe section, as the linker assumes it to be, that is the
> sections simply concatenated together, with alignment padding added
> between concatenated input sections if needed.  Above makes address
> an offset relative to the output .sframe section, which effectively
> "removes" this "imaginary" linker view from the value.  Below then
> makes it relative to the FDE of the planned output .sframe section,
> as SFrame assumes it to be (prior to sorting of the FDEs), that is
> one single header followed by all FDEs, followed by all FREs.
> 
> Is that correct?
> 

Yes.

I will update some code comments in this function to help understand 
some of this.

Thanks

>> +          /* SFrame FDE function start address is an offset from the
>> +         sfde_func_start_address field to the start PC.  The
>> +         calculation below is the distance of sfde_func_start_address
>> +         field from the start of the output SFrame section.  */
>> +          address -= (sframe_encoder_get_hdr_size (sfe_ctx)
>> +              + ((cur_fidx + num_enc_fidx) /* FDE index.  */
>> +                 * sizeof (sframe_func_desc_entry)));
> 
> Thanks and regards,
> Jens
diff mbox series

Patch

diff --git a/bfd/elf-sframe.c b/bfd/elf-sframe.c
index 97e00734515..e5e4c09b72c 100644
--- a/bfd/elf-sframe.c
+++ b/bfd/elf-sframe.c
@@ -481,6 +481,13 @@  _bfd_elf_merge_section_sframe (bfd *abfd,
 		address += sframe_read_value (abfd, contents,
 					      pltn_r_offset, 4);
 	      address += (sec->output_offset + r_offset);
+	      /* SFrame FDE function start address is an offset from the
+		 sfde_func_start_address field to the start PC.  The
+		 calculation below is the distance of sfde_func_start_address
+		 field from the start of the output SFrame section.  */
+	      address -= (sframe_encoder_get_hdr_size (sfe_ctx)
+			  + ((cur_fidx + num_enc_fidx) /* FDE index.  */
+			     * sizeof (sframe_func_desc_entry)));
 
 	      /* FIXME For testing only. Cleanup later.  */
 	      // address += (sec->output_section->vma);