[PATCH v3 3/3] sframe: FDE function start address relative to SFrame section

[Indu Bhagat]

On 2/28/25 5:32 AM, Jens Remus wrote:
> The SFrame FDE sfde_func_start_address field is specified to contain
> the function offset from .sframe section.  This is true in executables
> and shared libraries after final link.  In assembler generated objects
> it contained the function offset from SFrame FDE (using a PC-relative
> relocation) and the linker performed a respective fixup when merging
> the .sframe sections.
> 
> Change the assembler to emit sfde_func_start_address relative from the
> .sframe section.  Change the linker to only apply the output .sframe
> section offset as fixup when merging the sections.
> 
> While at it enhance the comments in the code, fix a typo in SFD_INFO,
> and remove commented out debugging code.
> 

Thanks for the patch.

I would like to consider this patch along with some other patches that I 
have for fixing the outstanding SFrame issues (PR 32589) and its 
repercussions on relocatable links (PR 32666) and --gc-sections.

I think it is better to review this change along with the fixes for the 
above issues.  I will include this patch in a separate series I plan to 
post soon.

Thanks
Indu

> gas/
> 	* gen-sframe.c (output_sframe_internal): Create temporary symbol
> 	for start of SFrame section.  Pass it to output_sframe_funcdesc.
> 	(output_sframe_funcdesc): Emit SFrame FDE function start address
> 	as offset from SFrame section instead of FDE.
> 
> bfd/
> 	* elf-sframe.c (_bfd_elf_merge_section_sframe): Fixup SFrame FDE
> 	function start address by adding output .sframe offset.  Enhance
> 	comments.
> 	(sframe_decoder_set_func_reloc_index): Correct typo in comment.
> 
> Signed-off-by: Jens Remus <jremus@linux.ibm.com>
> ---
> 
> Notes (jremus):
>      Changes in V3:
>      - New patch.  Replaces V2 patch ("sframe: Enhance comments and
>        documentation on FDE function start address").
> 
>   bfd/elf-sframe.c | 44 ++++++++++++++++++++++++++------------------
>   gas/gen-sframe.c | 11 +++++++----
>   2 files changed, 33 insertions(+), 22 deletions(-)
> 
> diff --git a/bfd/elf-sframe.c b/bfd/elf-sframe.c
> index 97e007345152..83611921e015 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
> @@ -446,27 +446,28 @@ _bfd_elf_merge_section_sframe (bfd *abfd,
>   	    {
>   	      if (!(sec->flags & SEC_LINKER_CREATED))
>   		{
> -		  /* Get relocated contents by reading the value of the
> -		     relocated function start address at the beginning of the
> -		     function descriptor entry.  */
> +		  /* Offset to SFrame FDE sfde_func_start_address from input
> +		     .sframe section.  It contains the relocated function
> +		     offset from input .sframe section.  */
>   		  r_offset = sframe_decoder_get_func_r_offset (sfd_info, i);
>   		}
>   	      else
>   		{
>   		  /* Expected to land here when SFrame stack trace info is
> -		     created dynamically for the .plt* sections.  These
> -		     sections are expected to have upto two SFrame FDE entries.
> +		     created dynamically for the .plt* sections.  These .sframe
> +		     sections are expected to have upto two SFrame FDEs.
>   		     Although the code should work for > 2,  leaving this
>   		     assert here for safety.  */
>   		  BFD_ASSERT (num_fidx <= 2);
> -		  /* For the first entry, we know the offset of the SFrame FDE's
> -		     sfde_func_start_address.  Side note: see how the value
> -		     of PLT_SFRAME_FDE_START_OFFSET is also set to the
> -		     same.  */
> +		  /* Offset to the first linker-generated SFrame FDE (for PLT0)
> +		     sfde_func_start_address from input .sframe section.  It
> +		     contains the PLT0 offset from FDE.  NB: See how it is set
> +		     using PLT_SFRAME_FDE_START_OFFSET.  */
>   		  r_offset = sframe_decoder_get_hdr_size (sfd_ctx);
> -		  /* For any further SFrame FDEs, the generator has already put
> -		     in an offset in place of sfde_func_start_address of the
> -		     corresponding FDE.  We will use it by hand to relocate.  */
> +		  /* Offset to any subsequent linker-generated SFrame FDE (for
> +		     PLTn) sfde_func_start_address from input .sframe section.
> +		     It contains the PLTn offset from PLT0.  We will use it by
> +		     hand to relocate.  */
>   		  if (i > 0)
>   		    {
>   		      pltn_r_offset
> @@ -475,15 +476,22 @@ _bfd_elf_merge_section_sframe (bfd *abfd,
>   		    }
>   		}
>   
> -	      /* Get the SFrame FDE function start address after relocation.  */
> +	      /* Get the SFrame FDE sfde_func_start_address value after
> +		 relocation.  In object files it is the function offset
> +		 from input .sframe section.  For linker-generated FDE
> +		 it is the PLT0 offset from FDE.  */
>   	      address = sframe_read_value (abfd, contents, r_offset, 4);
> +	      /* For linker-generated PLTn add the PLTn offset from PLT0.  */
>   	      if (pltn_reloc_by_hand)
>   		address += sframe_read_value (abfd, contents,
>   					      pltn_r_offset, 4);
> -	      address += (sec->output_offset + r_offset);
> -
> -	      /* FIXME For testing only. Cleanup later.  */
> -	      // address += (sec->output_section->vma);
> +	      /* Fixup by adding the output .sframe section offset.  For
> +		 linker-generated FDE additionally add the first FDE
> +		 offset from input .sframe section.  */
> +	      if (!(sec->flags & SEC_LINKER_CREATED))
> +		address += sec->output_offset;
> +	      else
> +		address += (sec->output_offset + r_offset);
>   
>   	      func_start_addr = address;
>   	    }
> diff --git a/gas/gen-sframe.c b/gas/gen-sframe.c
> index dd5fb25e367f..b50c6613c5a6 100644
> --- a/gas/gen-sframe.c
> +++ b/gas/gen-sframe.c
> @@ -607,7 +607,8 @@ output_sframe_row_entry (symbolS *fde_start_addr,
>   }
>   
>   static void
> -output_sframe_funcdesc (symbolS *start_of_fre_section,
> +output_sframe_funcdesc (symbolS *start_of_sframe_section,
> +			symbolS *start_of_fre_section,
>   			symbolS *fre_symbol,
>   			struct sframe_func_entry *sframe_fde)
>   {
> @@ -618,10 +619,10 @@ output_sframe_funcdesc (symbolS *start_of_fre_section,
>     dw_fde_start_addrS = get_dw_fde_start_addrS (sframe_fde->dw_fde);
>     dw_fde_end_addrS = get_dw_fde_end_addrS (sframe_fde->dw_fde);
>   
> -  /* Start address of the function.  */
> +  /* Start address of the function (function offset from .sframe section).  */
>     exp.X_op = O_subtract;
>     exp.X_add_symbol = dw_fde_start_addrS; /* to location.  */
> -  exp.X_op_symbol = symbol_temp_new_now (); /* from location.  */
> +  exp.X_op_symbol = start_of_sframe_section; /* from location.  */
>     exp.X_add_number = 0;
>     emit_expr (&exp, sizeof_member (sframe_func_desc_entry,
>   				  sfde_func_start_address));
> @@ -672,6 +673,7 @@ output_sframe_internal (void)
>     expressionS exp;
>     unsigned int i = 0;
>   
> +  symbolS *start_of_sframe_section = symbol_temp_new_now ();
>     symbolS *end_of_frame_hdr;
>     symbolS *end_of_frame_section;
>     symbolS *start_of_func_desc_section;
> @@ -763,7 +765,8 @@ output_sframe_internal (void)
>     i = 0;
>     for (sframe_fde = all_sframe_fdes; sframe_fde; sframe_fde = sframe_fde->next)
>       {
> -      output_sframe_funcdesc (start_of_fre_section,
> +      output_sframe_funcdesc (start_of_sframe_section,
> +			      start_of_fre_section,
>   			      fre_symbols[i], sframe_fde);
>         i += sframe_fde->num_fres;
>       }