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

[Jez Ng via Phabricator via llvm-commits]

int3 created this revision.
int3 added a reviewer: lld-macho.
Herald added projects: lld-macho, All.
int3 requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

We previously neglected this, leading us to underestimate the maximum
possible branch address offset.

Fixing this should allow us to reduce `slopSize` to more reasonable
levels. I've lowered it to 256 for now, though I suspect we could go
lower.

Fixes https://github.com/llvm/llvm-project/issues/59259.


Repository:
  rG LLVM Github Monorepo

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,8 +246,17 @@
     // 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() <
+    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++]);
+    }
+
+    while (finalIdx < endIdx && alignTo(addr + size, inputs[finalIdx]->align) +
+                                        inputs[finalIdx]->getSize() <
                                     isecVA + forwardBranchRange - slop)
       finalizeOne(inputs[finalIdx++]);
 


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