[RFC,SCHEME_B 4/7] ld: bfd: sframe: fix incorrect r_offset in RELA entries

[Indu Bhagat]

On 4/7/25 8:02 AM, Jan Beulich wrote:
> On 07.04.2025 02:25, Indu Bhagat wrote:
>> --- a/bfd/elflink.c
>> +++ b/bfd/elflink.c
>> @@ -11974,7 +11974,11 @@ elf_link_input_bfd (struct elf_final_link_info *flinfo, bfd *input_bfd)
>>   		      continue;
>>   		    }
>>   
>> -		  irela->r_offset += o->output_offset;
>> +		  /* All SFrame FDEs from input sections are clubbed together
>> +		     in the beginning of the output section.  RELA offsets are
>> +		     already up to date.  PR ld/32666.  */
>> +		  if (o->sec_info_type != SEC_INFO_TYPE_SFRAME)
>> +		    irela->r_offset += o->output_offset;
> 
> I'm irritated by there still being such special casing here. You go to some
> lengths in the description to explain this, but I fear I'm still not groking
> it even after reading multiple times. In particular the absence of any special
> casing here for SHF_MERGE sections makes we wonder why your content re-ordering
> is requiring such.
> 

A re-worded answer, in case it helps:

In SFrame, all FDEs (fixed length structure) follow the SFrame Header. The
correct offset (of the first field of the FDE) has been manually 
calculated via
_bfd_elf_sframe_section_offset () (i.e., it calculates the offset of the 
FDE in
the final output SFrame section).

BTW, the "o" variable above holds the input section; that often is a 
source of
confusion for me as well.

Regarding SHF_MERGE, I am not sure where the r_offset is updated once
elimination due to duplication is identified etc.  But as with other
(non-SFrame) sections, no special casing for SHF_MERGE is needed because it
works in a two step-process:
   - update the r_offset to the updated value within the section
   - adjust the r_offset with the output_offset of the section
     (o->output_offset).
Its only the SFrame section data layout that is not aligned to the two-step
process.

> To at least hide it from here, wouldn't it be possible to have
> _bfd_elf_sframe_section_offset() do the "inverse" adjustment, for that to be
> "undone" here? (For this it may help to remove the early return from the
> function - the sole caller ensures it's the right type.)
> 

It is possible to hide it in _bfd_elf_sframe_section_offset().  I purposely
chose to leave this in elflink.c (elf_link_input_bfd ()), because I 
thought it
is less bug-prone and easier to follow.  I can always leave a clear 
comment in
_bfd_elf_sframe_section_offset (); I think the current testcases already 
should
suffice to cover this code path.

Thanks for reviewing and apologies for the delay in replying,
Indu