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

[Daniel Hoekwater via Phabricator via llvm-commits]

dhoekwater updated this revision to Diff 533699.
dhoekwater added a comment.

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

2. We shouldn't need to error out on a 4GiB+ section, right? If we're resolving branches internally (as is done when compiling C++ code in the BranchRelaxation pass <https://github.com/llvm/llvm-project/blob/main/llvm/lib/CodeGen/BranchRelaxation.cpp>), 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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153494

Files:
  llvm/lib/MC/MCAssembler.cpp


Index: llvm/lib/MC/MCAssembler.cpp
===================================================================
--- llvm/lib/MC/MCAssembler.cpp
+++ llvm/lib/MC/MCAssembler.cpp
@@ -273,7 +273,7 @@
     "FKF_IsAlignedDownTo32Bits is only allowed on PC-relative fixups!");
 
   if (IsPCRel) {
-    uint32_t Offset = Layout.getFragmentOffset(DF) + Fixup.getOffset();
+    uint64_t Offset = Layout.getFragmentOffset(DF) + Fixup.getOffset();
 
     // A number of ARM fixups in Thumb mode require that the effective PC
     // address be determined as the 32-bit aligned version of the actual offset.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D153494.533699.patch
Type: text/x-patch
Size: 586 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230622/f7341489/attachment.bin>