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

[Hiroshi Yamauchi via Phabricator via llvm-commits]

hjyamauchi added a comment.

In D155732#4519183 <https://reviews.llvm.org/D155732#4519183>, @compnerd wrote:

> 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.

F28371736: branch26-test-log.txt <https://reviews.llvm.org/F28371736>
The attach text file contains a test (*) and the command line trace that shows the relocation behaviors (without this patch) in the first half, and the same sequence of commands with this patch in the second half.

The first part shows that both link.exe and lld relocate the branches to the beginning of the text and the other section, as opposed to the ret instructions, respectively, because the offsets embedded in the instruction are ignored. The second part shows that, with this patch, the linkers relocate them as expected.

(*) A tweaked version of the test in this patch (because some important details were unexpectedly lost along the test reduction and nops were added to avoid accidental offset correctness as small offsets from a symbol can happen to be correct).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155732