[v2,3/3] sframe: Enhance comments and documentation on FDE function start address

Message ID 20250227141040.3005257-4-jremus@linux.ibm.com
State Rejected
Headers
Series sframe: Cleanups in .sframe generation |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_binutils_check--master-arm success Test passed

Commit Message

Jens Remus Feb. 27, 2025, 2:10 p.m. UTC
  The function start address in a SFrame FDE (sfde_func_start_address) is
a signed offset, that is relative to the (1) FDE in object files and
(2) SFrame section in executables and shared libraries after final link.
Internal in the GNU linker it is relative to the (3) FDE in linker
generated FDE for PLT0 and (4) PLT0 in linker generated FDE for PLTn.

While at it fix a typo in SFD_INFO and remove commented out debugging
code.

bfd/
	* elf-sframe.c (_bfd_elf_merge_section_sframe): Add comments on
	FDE function start address.
	(sframe_decoder_set_func_reloc_index): Correct typo in comment.

libsframe/doc/
	* sframe-spec.texi (sfde_func_start_address): Enhance
	documentation on FDE function start address.

Signed-off-by: Jens Remus <jremus@linux.ibm.com>
---
 bfd/elf-sframe.c               | 13 ++++++++-----
 libsframe/doc/sframe-spec.texi |  6 ++++--
 2 files changed, 12 insertions(+), 7 deletions(-)
  

Comments

Indu Bhagat Feb. 28, 2025, 2:42 a.m. UTC | #1
On 2/27/25 6:10 AM, Jens Remus wrote:
> The function start address in a SFrame FDE (sfde_func_start_address) is
> a signed offset, that is relative to the (1) FDE in object files and
> (2) SFrame section in executables and shared libraries after final link.
> Internal in the GNU linker it is relative to the (3) FDE in linker
> generated FDE for PLT0 and (4) PLT0 in linker generated FDE for PLTn.
> 
> While at it fix a typo in SFD_INFO and remove commented out debugging
> code.
> 
> bfd/
> 	* elf-sframe.c (_bfd_elf_merge_section_sframe): Add comments on
> 	FDE function start address.
> 	(sframe_decoder_set_func_reloc_index): Correct typo in comment.
> 
> libsframe/doc/
> 	* sframe-spec.texi (sfde_func_start_address): Enhance
> 	documentation on FDE function start address.
> 
> Signed-off-by: Jens Remus <jremus@linux.ibm.com>
> ---
>   bfd/elf-sframe.c               | 13 ++++++++-----
>   libsframe/doc/sframe-spec.texi |  6 ++++--
>   2 files changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/bfd/elf-sframe.c b/bfd/elf-sframe.c
> index 97e007345152..e3c7c52a548f 100644
> --- a/bfd/elf-sframe.c
> +++ b/bfd/elf-sframe.c
> @@ -93,7 +93,7 @@ sframe_decoder_set_func_reloc_index (struct sframe_dec_info *sfd_info,
>       sfd_info->sfd_func_bfdinfo[func_idx].func_reloc_index = reloc_index;
>   }
>   
> -/* Initialize the set of additional information in CFD_INFO,
> +/* Initialize the set of additional information in SFD_INFO,
>      needed for linking SEC.  Returns TRUE if setup is done successfully.  */
>   
>   static bool
> @@ -475,16 +475,19 @@ _bfd_elf_merge_section_sframe (bfd *abfd,
>   		    }
>   		}
>   
> -	      /* Get the SFrame FDE function start address after relocation.  */
> +	      /* Get the SFrame FDE function start address after relocation.
> +		 In object files it is the signed function offset from FDE.
> +		 For linker generated FDE for PLT0 it is the PLT0 offset
> +		 from FDE and for PLTn it is the PLTn offset from PLT0.  */
>   	      address = sframe_read_value (abfd, contents, r_offset, 4);
> +	      /* For PLTn add the PLTn offset from PLT0.  */
>   	      if (pltn_reloc_by_hand)
>   		address += sframe_read_value (abfd, contents,
>   					      pltn_r_offset, 4);
> +	      /* Fixup offset to be from SFrame section instead of FDE, by
> +		 adding the FDE offset from SFrame output section.  */
>   	      address += (sec->output_offset + r_offset);
>   
> -	      /* FIXME For testing only. Cleanup later.  */
> -	      // address += (sec->output_section->vma);
> -
>   	      func_start_addr = address;
>   	    }
>   
> diff --git a/libsframe/doc/sframe-spec.texi b/libsframe/doc/sframe-spec.texi
> index ae115705ca73..7f30d33cf4ef 100644
> --- a/libsframe/doc/sframe-spec.texi
> +++ b/libsframe/doc/sframe-spec.texi
> @@ -460,8 +460,10 @@ Following table describes each component of the SFrame FDE structure:
>   @tab @code{sfde_func_start_address}
>   @tab Signed 32-bit integral field denoting the virtual memory address of the
>   described function, for which the SFrame FDE applies.  The value encoded in
> -the @code{sfde_func_start_address} field is the offset in bytes of the
> -function's start address, from the SFrame section.
> +the @code{sfde_func_start_address} field is the signed offset in bytes to the
> +function's start address.  For object files from the FDE (using a 32-bit
> +PC-relative relocation).  For executables and shared libraries after final
> +link, from the SFrame section.
>   

This indicates some problem.  Something else needs fixing, I am checking 
a few things in the current implementation in GAS/ld.  Will get back soon.

Thanks
Indu
  

Patch

diff --git a/bfd/elf-sframe.c b/bfd/elf-sframe.c
index 97e007345152..e3c7c52a548f 100644
--- a/bfd/elf-sframe.c
+++ b/bfd/elf-sframe.c
@@ -93,7 +93,7 @@  sframe_decoder_set_func_reloc_index (struct sframe_dec_info *sfd_info,
     sfd_info->sfd_func_bfdinfo[func_idx].func_reloc_index = reloc_index;
 }
 
-/* Initialize the set of additional information in CFD_INFO,
+/* Initialize the set of additional information in SFD_INFO,
    needed for linking SEC.  Returns TRUE if setup is done successfully.  */
 
 static bool
@@ -475,16 +475,19 @@  _bfd_elf_merge_section_sframe (bfd *abfd,
 		    }
 		}
 
-	      /* Get the SFrame FDE function start address after relocation.  */
+	      /* Get the SFrame FDE function start address after relocation.
+		 In object files it is the signed function offset from FDE.
+		 For linker generated FDE for PLT0 it is the PLT0 offset
+		 from FDE and for PLTn it is the PLTn offset from PLT0.  */
 	      address = sframe_read_value (abfd, contents, r_offset, 4);
+	      /* For PLTn add the PLTn offset from PLT0.  */
 	      if (pltn_reloc_by_hand)
 		address += sframe_read_value (abfd, contents,
 					      pltn_r_offset, 4);
+	      /* Fixup offset to be from SFrame section instead of FDE, by
+		 adding the FDE offset from SFrame output section.  */
 	      address += (sec->output_offset + r_offset);
 
-	      /* FIXME For testing only. Cleanup later.  */
-	      // address += (sec->output_section->vma);
-
 	      func_start_addr = address;
 	    }
 
diff --git a/libsframe/doc/sframe-spec.texi b/libsframe/doc/sframe-spec.texi
index ae115705ca73..7f30d33cf4ef 100644
--- a/libsframe/doc/sframe-spec.texi
+++ b/libsframe/doc/sframe-spec.texi
@@ -460,8 +460,10 @@  Following table describes each component of the SFrame FDE structure:
 @tab @code{sfde_func_start_address}
 @tab Signed 32-bit integral field denoting the virtual memory address of the
 described function, for which the SFrame FDE applies.  The value encoded in
-the @code{sfde_func_start_address} field is the offset in bytes of the
-function's start address, from the SFrame section.
+the @code{sfde_func_start_address} field is the signed offset in bytes to the
+function's start address.  For object files from the FDE (using a 32-bit
+PC-relative relocation).  For executables and shared libraries after final
+link, from the SFrame section.
 
 @item 0x04
 @tab @code{uint32_t}