[PATCH] D153494: Detect out of range jumps further than 2^32 bytes

[Fangrui Song via Phabricator via llvm-commits]

MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.

In D153494#4442086 <https://reviews.llvm.org/D153494#4442086>, @dhoekwater wrote:

> Sounds good, I'll remove the test. Testing this felt kind of gross, but I wasn't sure how strict the "any behavior-modifying change requires a test" philosophy is within LLVM.
>
> In D153494#4440142 <https://reviews.llvm.org/D153494#4440142>, @peter.smith wrote:
>
>> 1. Exposing these to the linker like we do in AArch32 would permit branches that can reach outside the section to be range-extended by thunks.
>>
>> 2. Another not specifically part of this patch, is that for AArch64 we should probably give an error message when the section size gets bigger than 4 GiB as if we are resolving branches internally and not passing them on to the linker, that is the largest we can support without giving an obtuse error message.
>
>
>
> 1. Yeah, agreed. Emitting a relocation and letting the linker handle out-of-range branches should be intended behavior. One thing to keep in mind is that lld currently doesn't handle offsets greater than 4GiB for position-independent code <https://github.com/llvm/llvm-project/blob/main/lld/ELF/Thunks.cpp#L545>.

This limit should be fine. https://maskray.me/blog/2023-05-14-relocation-overflow-and-code-models#aarch64-code-models

"This larger range makes it unlikely for AArch64 to encounter relocation overflow issues before the binary becomes excessively oversized for x86-64."

> 2. We shouldn't need to error out on a 4GiB+ section, right? If we're resolving branches internally (as is done in the BranchRelaxation pass <https://github.com/llvm/llvm-project/blob/main/llvm/lib/CodeGen/BranchRelaxation.cpp> when compiling C++ code), we should generate branches that are either in-range or emit relocations and can be fixed up by the linker. If we're just validating branch offsets, as is done when compiling AArch64 assembly, our current error handling should produce reasonable error output.

I think we do not need arbitrary restricting what we can emit in the assembly output. It's difficult to get right/test and likely not useful in practice.

" Instead, make it return an error." this sentence should be removed. Ideally add `[MC] ` to the title of this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153494