[PATCH] x86-64: Remove sframe relocs against discarded sections

[Jan Beulich]

On 15.07.2025 23:16, Indu Bhagat wrote:
> On 7/15/25 12:20 AM, Jan Beulich wrote:
>> On 15.07.2025 07:51, Sam James wrote:
>>> From: "H.J. Lu" <hjl.tools@gmail.com>
>>>
>>> Since unlike eh_frame editing code, sframe editing code keeps
>>> R_X86_64_NONE reloc as is, its r_offset is wrong, we must not
>>> generate R_X86_64_NONE reloc in sframe section against discarded
>>> sections for "ld -r".
>>>
>>> bfd/
>>>
>>> 	PR ld/33156
>>> 	* elf64-x86-64.c (elf_x86_64_relocate_section): Also remove
>>> 	sframe relocations against discarded sections for "ld -r".
>>>
>>> ld/
>>>
>>> 	PR ld/33156
>>> 	* testsuite/ld-elf/eh-group.exp (as_gsframe): New.
>>> 	Assemble eh-group.o with $as_gsframe.
>>>
>>> Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
>>> ---
>>> Sent on H.J.'s behalf. OK?
>>
>> What's the relationship with what it now 5f9bf0cf711a ("sframe: Allow
>> input R_*_NONE relocations")? Indu's reply there suggests to me that
>> altering the number of relocs may be a problem.
>>
> 
> For SFrame sections, at parsing time, the code creates a cache of which 
> functions are discarded via _bfd_elf_discard_section_sframe (called via 
> bfd_elf_discard_info).  So for input sections, there is reliance on 
> relocs being in order of the functions.
> 
> At the time of relocation in elf_x86_64_relocate_section (), deleting 
> the R_*_NONE relocs will not pose problem, because we use the cache 
> created above. So the patch that H.J. proposes should be OK (as far as I 
> can reason so far).
> 
> That aside, taking a step back, the reason why these R_*_NONE are 
> causing issues is (as HJ said) that their r_offset is wrong.  We get 
> bfd_reloc_outofrange in elf_x86_64_relocate_section () --> 
> _bfd_final_link_relocate () --> bfd_reloc_offset_in_range ().  To fix it 
> the generic code in elf_link_input_bfd, for deleted relas, this would mean:
> 
>    last_offset = ...
>    last_offset += ...
> 
>    irela->r_offset = _bfd_elf_section_offset (output_bfd,
>                                               flinfo->info, o,
>                                               irela->r_offset);
> 
>    if (irela->r_offset >= (bfd_vma) -2)
>      {
>        /* This is a reloc for a deleted entry or somesuch.
>           Turn it into an R_*_NONE reloc, at the same
>           offset as the last reloc.  elf_eh_frame.c and
>           bfd_elf_discard_info rely on reloc offsets
>           being ordered.  */
> -     irela->r_offset = last_offset;
> +     if (elf_section_type (o) == SHT_GNU_SFRAME)
> +       irela->r_offset = sframe_last_dummy_offset;
> +     else
> +       irela->r_offset = last_offset;
>        irela->r_info = 0;
>        irela->r_addend = 0;
>        continue;
>      }
> 
>     last_offset = irela->r_offset;
> +  sframe_last_dummy_offset = irela->r_offset;

This would imo be yet worse than what H.J.'s patch does. I know I'm repeating
myself, but all of the special casing for SFrame is quite unhelpful.

Jan