[PATCH] aarch64: Set pcrel_off for COFF relocations

[Mark Harmstone]

On 01/09/2025 8.48 am, Jan Beulich wrote:
> On 30.08.2025 19:17, Mark Harmstone wrote:
>> The pcrel_off flag of reloc_howto_type specifies that "the displacement part
>> of an instruction [is left] empty", which is the case for ELF and COFF files.
>> This wasn't being set in bfd/coff-aarch64.c, with the result that when using
>> ld with a linker script, bfd_perform_relocation was writing incorrect
>> relocations.
> 
> But is this correct for all relocations? I can see it being like this for
> ones dealing with instruction fields, yet ones like ADDR64, ADDR32, REL32
> etc (dealing with data items) may be different?
> 
> Jan

I just realized there's a typo in the patch name: it's pcrel_offset, not
pcrel_off.

 From what I can see pcrel_offset is only checked when pc_relative is true,
so ADDR64 and ADDR32 aren't affected. For REL32 it's not clear, there's no
way of generating it with GAS so it's not tested.

Three years on it's not clear to me why we have two ways of doing the same
relocations in this file: the normal way in coff_pe_aarch64_relocate_section,
and the HOWTOs for anything unusual. Five minutes after sending this patch
I ran into another two issues in the same area, and I know trying to link
ELF object files into a PE file is also dangerous.

I think once the HOWTOs are fixed the way forward would be to remove
coff_pe_aarch64_relocate_section and rely on bfd_generic relocation instead -
that way we don't have untested code here. Unless there's something I'm
missing about why this would be impossible or a bad idea.

Mark

> 
>> --- a/bfd/coff-aarch64.c
>> +++ b/bfd/coff-aarch64.c
>> @@ -244,7 +244,7 @@ coff_aarch64_secrel_reloc (bfd *abfd,
>>   #define HOWTO_INSTALL_ADDEND 1
>>   #define HOW(type, right, size, bits, pcrel, left, ovf, func, mask) \
>>     HOWTO (type, right, size, bits, pcrel, left, complain_overflow_##ovf, \
>> -	 coff_aarch64_##func, #type, true, mask, mask, false)
>> +	 coff_aarch64_##func, #type, true, mask, mask, true)
>>   
>>   static const reloc_howto_type arm64_reloc_howto_abs
>>   = HOW (IMAGE_REL_ARM64_ABSOLUTE,
>