[RFC 4/4] ld: bfd: sframe: fix incorrect r_addend in RELA entries

[Jens Remus]

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
-- 
Jens Remus
Linux on Z Development (D3303)
+49-7031-16-1128 Office
jremus@de.ibm.com

IBM

IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Böblingen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/