[lld] [lld][MachO] Key branch-extension thunks on (referent, addend) (PR #191808)

[Ellis Hoag via llvm-commits]

================
@@ -351,19 +351,34 @@ void TextOutputSection::finalize() {
       uint64_t highVA = callVA + forwardBranchRange;
       // Calculate our call referent address
       auto *funcSym = cast<Symbol *>(r.referent);
-      ThunkInfo &thunkInfo = thunkMap[funcSym];
-      // The referent is not reachable, so we need to use a thunk ...
-      if ((funcSym->isInStubs() ||
+      ThunkInfo &thunkInfo = thunkMap[ThunkKey{funcSym, r.addend}];
+      // The referent is not reachable, so we need to use a thunk... unless we
+      // are close enough to the end that branch target sections (__stubs,
+      // __objc_stubs) are now within range of a simple forward branch -- BUT
+      // only for zero-addend branches. The writer's resolveSymbolOffsetVA()
+      // resolves non-zero-addend branches against the symbol body rather than
+      // the stub, so __stubs reachability says nothing about whether such a
+      // call can be emitted directly. Hence the `r.addend == 0` guard below.
+      // See INTERP check lines in arm64-thunk-branch-addend.s.
+      if (r.addend == 0 &&
+          (funcSym->isInStubs() ||
            (in.objcStubs && in.objcStubs->isNeeded() &&
             ObjCStubsSection::isObjCStubSymbol(funcSym))) &&
           callVA >= branchTargetThresholdVA) {
         assert(callVA != TargetInfo::outOfRangeVA);
-        // ... Oh, wait! We are close enough to the end that branch target
-        // sections (__stubs, __objc_stubs) are now within range of a simple
-        // forward branch.
         continue;
       }
-      uint64_t funcVA = funcSym->resolveBranchVA();
+      // For non-zero addends, branch directly to the symbol body rather than
+      // through any stub (`resolveBranchVA()` would prefer the stub VA when
+      // present). This mirrors the writer-side rule in `resolveSymbolOffsetVA`:
+      // branching to the interior of a stub trampoline is meaningless, so for
+      // `bl _func+N` we resolve against `_func`'s own body. The two helpers
+      // only diverge when `funcSym` is a Defined that is *also* in stubs (e.g.
+      // an interposable extern), but `finalize()` has to agree with the writer
+      // in that case or reachability analysis drifts.
+      // See INTERP check lines in arm64-thunk-branch-addend.s.
+      uint64_t funcVA = r.addend != 0 ? funcSym->getVA() + r.addend
+                                      : funcSym->resolveBranchVA();
----------------
ellishg wrote:

Can we use `resolveSymbolOffsetVA()` directly to avoid code duplication? I assume it will be inlined anyway, so the non `RelocAttrBits::BRANCH` cases will probably be dead-stripped anyway.

https://github.com/llvm/llvm-project/pull/191808