[PATCH] D155732: [MC][COFF][AArch64] Avoid incorrect IMAGE_REL_ARM64_BRANCH26 relocations.

Thu Jul 20 08:16:18 PDT 2023

compnerd added a comment.

In D155732#4516320 <https://reviews.llvm.org/D155732#4516320>, @mstorsjo wrote:

> In D155732#4515825 <https://reviews.llvm.org/D155732#4515825>, @hjyamauchi wrote:
>
>> In D155732#4515655 <https://reviews.llvm.org/D155732#4515655>, @compnerd wrote:
>>
>>> https://learn.microsoft.com/en-us/windows/win32/debug/pe-format indicates that there does exist such a relocation:
>>>
>>>> IMAGE_REL_ARM64_BRANCH26    0x0003   The 26-bit relative displacement to the target, for B and BL instructions.
>>>
>>> If this relocation is unsupported, we should just disable that relocation from being formed or we should figure out what the linker expectations are.
>>
>> Yeah. To be clearer, what I observed was that the relocation type works as long as the offset that's embedded in the instruction is zero (I suspect that the linker doesn't use the offset in relocation or assumes it's zero.) I also tried a small local patch in lld to take this non-zero offset into account in the relocation and it also avoided the crashes I'm seeing.
>
> That sounds like the correct thing to do; if a relocation with an offset isn’t supported by lld (but it is by link.exe), we should fix the linker instead of trying to avoid generating it.

I would agree, but it seems that link also does not support the relocation with addend, and I would rather that we retain compatibility with the reference implementation.  It sounds like it is only the addend form that we would need to avoid on Windows, not the general relocation.

> I had expected those cases to be working correctly though, for many years now, but perhaps there is some case where the original offset indeed happens to get ignored.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155732/new/

https://reviews.llvm.org/D155732