[lld] 9721197 - [lld/mac] Set branchRange a bit more carefully

Nico Weber via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 30 09:36:12 PDT 2021


Author: Nico Weber
Date: 2021-08-30T12:36:06-04:00
New Revision: 9721197520e5badd4e6385399f6b08575c6c8504

URL: https://github.com/llvm/llvm-project/commit/9721197520e5badd4e6385399f6b08575c6c8504
DIFF: https://github.com/llvm/llvm-project/commit/9721197520e5badd4e6385399f6b08575c6c8504.diff

LOG: [lld/mac] Set branchRange a bit more carefully

- Don't subtract thunkSize from branchRange. Most places care about
  the actual maximal branch range. Subtract thunkSize in the one place
  that wants to leave room for a thunk.
- Set it to 0x800_0000 instead of 0xFF_FFFF
- Subtract 4 for the positive branch direction since it's a
  two's complement 24bit number sign-extended mutiplied by 4,
  so its range is -0x800_0000..+0x7FF_FFFC
- Make boundary checks include the boundary values

This doesn't make a huge difference in practice. It's preparation
for a "real" fix for PR51578 -- but it also lets the repro in comment 0
in that bug place one more thunk before hitting the TODO.

Differential Revision: https://reviews.llvm.org/D108897

Added: 
    

Modified: 
    lld/MachO/Arch/ARM64.cpp
    lld/MachO/ConcatOutputSection.cpp
    lld/MachO/Target.h

Removed: 
    


################################################################################
diff  --git a/lld/MachO/Arch/ARM64.cpp b/lld/MachO/Arch/ARM64.cpp
index 36c3cc639284..001e112ae3ad 100644
--- a/lld/MachO/Arch/ARM64.cpp
+++ b/lld/MachO/Arch/ARM64.cpp
@@ -134,7 +134,13 @@ ARM64::ARM64() : ARM64Common(LP64()) {
 
   stubSize = sizeof(stubCode);
   thunkSize = sizeof(thunkCode);
-  branchRange = maxIntN(28) - thunkSize;
+
+  // Branch immediate is two's complement 26 bits, which is implicitly
+  // multiplied by 4 (since all functions are 4-aligned: The branch range
+  // is -4*(2**(26-1))..4*(2**(26-1) - 1).
+  backwardBranchRange = 128 * 1024 * 1024;
+  forwardBranchRange = backwardBranchRange - 4;
+
   stubHelperHeaderSize = sizeof(stubHelperHeaderCode);
   stubHelperEntrySize = sizeof(stubHelperEntryCode);
 }

diff  --git a/lld/MachO/ConcatOutputSection.cpp b/lld/MachO/ConcatOutputSection.cpp
index b41495b4fc64..73cc27fbb903 100644
--- a/lld/MachO/ConcatOutputSection.cpp
+++ b/lld/MachO/ConcatOutputSection.cpp
@@ -126,7 +126,8 @@ bool ConcatOutputSection::needsThunks() const {
   uint64_t isecAddr = addr;
   for (InputSection *isec : inputs)
     isecAddr = alignTo(isecAddr, isec->align) + isec->getSize();
-  if (isecAddr - addr + in.stubs->getSize() <= target->branchRange)
+  if (isecAddr - addr + in.stubs->getSize() <=
+      std::min(target->backwardBranchRange, target->forwardBranchRange))
     return false;
   // Yes, this program is large enough to need thunks.
   for (InputSection *isec : inputs) {
@@ -153,7 +154,6 @@ bool ConcatOutputSection::needsThunks() const {
 // Since __stubs is placed after __text, we must estimate the address
 // beyond which stubs are within range of a simple forward branch.
 uint64_t ConcatOutputSection::estimateStubsInRangeVA(size_t callIdx) const {
-  uint64_t branchRange = target->branchRange;
   size_t endIdx = inputs.size();
   ConcatInputSection *isec = inputs[callIdx];
   uint64_t isecVA = isec->getVA();
@@ -174,15 +174,16 @@ uint64_t ConcatOutputSection::estimateStubsInRangeVA(size_t callIdx) const {
   }
   // Estimate the address after which call sites can safely call stubs
   // directly rather than through intermediary thunks.
+  uint64_t forwardBranchRange = target->forwardBranchRange;
   uint64_t stubsInRangeVA = isecEnd + maxPotentialThunks * target->thunkSize +
-                            in.stubs->getSize() - branchRange;
+                            in.stubs->getSize() - forwardBranchRange;
   log("thunks = " + std::to_string(thunkMap.size()) +
       ", potential = " + std::to_string(maxPotentialThunks) +
       ", stubs = " + std::to_string(in.stubs->getSize()) + ", isecVA = " +
       to_hexString(isecVA) + ", threshold = " + to_hexString(stubsInRangeVA) +
       ", isecEnd = " + to_hexString(isecEnd) +
       ", tail = " + to_hexString(isecEnd - isecVA) +
-      ", slop = " + to_hexString(branchRange - (isecEnd - isecVA)));
+      ", slop = " + to_hexString(forwardBranchRange - (isecEnd - isecVA)));
   return stubsInRangeVA;
 }
 
@@ -206,7 +207,8 @@ void ConcatOutputSection::finalize() {
     return;
   }
 
-  uint64_t branchRange = target->branchRange;
+  uint64_t forwardBranchRange = target->forwardBranchRange;
+  uint64_t backwardBranchRange = target->backwardBranchRange;
   uint64_t stubsInRangeVA = TargetInfo::outOfRangeVA;
   size_t thunkSize = target->thunkSize;
   size_t relocCount = 0;
@@ -225,8 +227,8 @@ void ConcatOutputSection::finalize() {
     assert(isec->isFinal);
     uint64_t isecVA = isec->getVA();
     // Assign addresses up-to the forward branch-range limit
-    while (finalIdx < endIdx &&
-           isecAddr + inputs[finalIdx]->getSize() < isecVA + branchRange)
+    while (finalIdx < endIdx && isecAddr + inputs[finalIdx]->getSize() <
+                                    isecVA + forwardBranchRange - thunkSize)
       finalizeOne(inputs[finalIdx++]);
     if (isec->callSiteCount == 0)
       continue;
@@ -254,8 +256,9 @@ void ConcatOutputSection::finalize() {
       ++callSiteCount;
       // Calculate branch reachability boundaries
       uint64_t callVA = isecVA + r.offset;
-      uint64_t lowVA = branchRange < callVA ? callVA - branchRange : 0;
-      uint64_t highVA = callVA + branchRange;
+      uint64_t lowVA =
+          backwardBranchRange < callVA ? callVA - backwardBranchRange : 0;
+      uint64_t highVA = callVA + forwardBranchRange;
       // Calculate our call referent address
       auto *funcSym = r.referent.get<Symbol *>();
       ThunkInfo &thunkInfo = thunkMap[funcSym];
@@ -267,7 +270,7 @@ void ConcatOutputSection::finalize() {
       }
       uint64_t funcVA = funcSym->resolveBranchVA();
       ++thunkInfo.callSitesUsed;
-      if (lowVA < funcVA && funcVA < highVA) {
+      if (lowVA <= funcVA && funcVA <= highVA) {
         // The referent is reachable with a simple call instruction.
         continue;
       }
@@ -276,7 +279,7 @@ void ConcatOutputSection::finalize() {
       // If an existing thunk is reachable, use it ...
       if (thunkInfo.sym) {
         uint64_t thunkVA = thunkInfo.isec->getVA();
-        if (lowVA < thunkVA && thunkVA < highVA) {
+        if (lowVA <= thunkVA && thunkVA <= highVA) {
           r.referent = thunkInfo.sym;
           continue;
         }

diff  --git a/lld/MachO/Target.h b/lld/MachO/Target.h
index a5da7644a84e..9c021c611f7b 100644
--- a/lld/MachO/Target.h
+++ b/lld/MachO/Target.h
@@ -88,7 +88,8 @@ class TargetInfo {
   size_t wordSize;
 
   size_t thunkSize = 0;
-  uint64_t branchRange = 0;
+  uint64_t forwardBranchRange = 0;
+  uint64_t backwardBranchRange = 0;
 
   // We contrive this value as sufficiently far from any valid address that it
   // will always be out-of-range for any architecture. UINT64_MAX is not a


        


More information about the llvm-commits mailing list