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

Fri Jul 21 10:19:46 PDT 2023

hjyamauchi added a comment.

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

>> Thanks for the test transcript! It does indeed look like link.exe ignores the existing offset. I also independently tested this, and it actually looks like link.exe blindly `or`s the target offset bits on top of the existing opcode, just like lld does: https://github.com/llvm/llvm-project/blob/llvmorg-16.0.6/lld/COFF/Chunks.cpp#L306-L322 Depending on the contents of the original offset and the intended symbol difference, the offset may end up being partially visible or not. In any case - the linker doesn't indeed seem to be able to handle immediate offsets here.

The lld code linked here is exactly what I locally modified to experiment and saw the crash go away with.

> My recollection is that for most relocations, link does not support (in GNU parlance) partial in-place relocations, that is, any addend is //generally// (though not always) ignored.  It was a pretty big divergence between GNU and MSVC semantics.
>
>> Strictly speaking I would consider that a bug in both linkers, but I totally agree we should avoid producing such relocations. I didn't have time to look at the implementation strategy itself yet (but trust others' judgement on it).
>
> I'm not sure I consider it a bug since that is the normal handling of relocations, but certainly a lacking useful feature.  At least, it sounds like everyone is now in agreement on avoiding producing the relocation for Windows then.

Yeah, as I implied by creating this revision, I also vote for avoiding this relocation type for compatibility.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155732