[PATCH] D144029: [lld-macho] Account for alignment in thunk insertion algorithm

[Jez Ng via Phabricator via llvm-commits]

int3 updated this revision to Diff 497371.
int3 added a comment.

update


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144029

Files:
  lld/MachO/ConcatOutputSection.cpp
  lld/test/MachO/arm64-thunk-for-alignment.s


Index: lld/test/MachO/arm64-thunk-for-alignment.s
===================================================================
--- /dev/null
+++ lld/test/MachO/arm64-thunk-for-alignment.s
@@ -0,0 +1,44 @@
+# REQUIRES: aarch64
+# RUN: rm -rf %t; split-file %s %t
+# RUN: llvm-mc -filetype=obj -triple=arm64-apple-darwin %t/foo.s -o %t/foo.o
+# RUN: llvm-mc -filetype=obj -triple=arm64-apple-darwin %t/bar.s -o %t/bar.o
+# RUN: %lld -dylib -arch arm64 -lSystem -o %t/out %t/foo.o %t/bar.o
+
+# RUN: llvm-objdump --macho --syms %t/out | FileCheck %s
+# CHECK: _bar.thunk.0
+
+## Regression test for PR59259. Previously, we neglected to check section
+## alignments when deciding when to create thunks.
+
+## If we ignore alignment, the total size of _spacer1 + _spacer2 below is just
+## under the limit at which we attempt to insert thunks between the spacers.
+## However, with alignment accounted for, their total size ends up being
+## 0x8000000, which is just above the max forward branch range, making thunk
+## insertion necessary. Thus, not accounting for alignment led to an error.
+
+#--- foo.s
+
+_foo:
+  b _bar
+
+## Size of a `b` instruction.
+.equ callSize, 4
+## Refer to `slop` in TextOutputSection::finalize().
+.equ slopSize, 12 * 256
+
+_spacer1:
+  .space 0x4000000 - slopSize - 2 * callSize - 1
+
+.subsections_via_symbols
+
+#--- bar.s
+.globl _bar
+
+.p2align 14
+_spacer2:
+  .space 0x4000000
+
+_bar:
+  ret
+
+.subsections_via_symbols
Index: lld/MachO/ConcatOutputSection.cpp
===================================================================
--- lld/MachO/ConcatOutputSection.cpp
+++ lld/MachO/ConcatOutputSection.cpp
@@ -246,10 +246,14 @@
     // contains several branch instructions in succession, then the distance
     // from the current position to the position where the thunks are inserted
     // grows. So leave room for a bunch of thunks.
-    unsigned slop = 1024 * thunkSize;
-    while (finalIdx < endIdx && addr + size + inputs[finalIdx]->getSize() <
-                                    isecVA + forwardBranchRange - slop)
+    unsigned slop = 256 * thunkSize;
+    while (finalIdx < endIdx) {
+      size_t expectedNewSize = alignTo(addr + size, inputs[finalIdx]->align) +
+                               inputs[finalIdx]->getSize();
+      if (expectedNewSize >= isecVA + forwardBranchRange - slop)
+        break;
       finalizeOne(inputs[finalIdx++]);
+    }
 
     if (!isec->hasCallSites)
       continue;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D144029.497371.patch
Type: text/x-patch
Size: 2446 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230214/a00354c0/attachment.bin>