From 156e6051630d136b48090c0d1cb79a4457564e9d Mon Sep 17 00:00:00 2001 From: alx32 <103613512+alx32@users.noreply.github.com> Date: Thu, 9 Jan 2025 14:14:13 -0800 Subject: [PATCH] [lld-macho] Fix branch extension thunk estimation logic (#120529) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch improves the linker’s ability to estimate stub reachability in the `TextOutputSection::estimateStubsInRangeVA` function. It does so by including thunks that have already been placed ahead of the current call site address when calculating the threshold for direct stub calls. Before this fix, the estimation process overlooked existing forward thunks. This could result in some thunks not being inserted where needed. In rare situations, particularly with large and specially arranged codebases, this might lead to branch instructions being out of range, causing linking errors. Although this patch successfully addresses the problem, it is not feasible to create a test for this issue. The specific layout and order of thunk creation required to reproduce the corner case are too complex, making test creation impractical. Example error messages the issue could generate: ``` ld64.lld: error: banana.o:(symbol OUTLINED_FUNCTION_24949_3875): relocation BRANCH26 is out of range: 134547892 is not in [-134217728, 134217727]; references objc_autoreleaseReturnValue ld64.lld: error: main.o:(symbol _main+0xc): relocation BRANCH26 is out of range: 134544132 is not in [-134217728, 134217727]; references objc_release ``` --- lld/MachO/ConcatOutputSection.cpp | 36 +++++++++++++++++++++++++++---- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/lld/MachO/ConcatOutputSection.cpp b/lld/MachO/ConcatOutputSection.cpp index 6a9301f84a03ef..d64816ec695ad5 100644 --- a/lld/MachO/ConcatOutputSection.cpp +++ b/lld/MachO/ConcatOutputSection.cpp @@ -184,15 +184,43 @@ uint64_t TextOutputSection::estimateStubsInRangeVA(size_t callIdx) const { InputSection *isec = inputs[i]; isecEnd = alignToPowerOf2(isecEnd, isec->align) + isec->getSize(); } - // Estimate the address after which call sites can safely call stubs - // directly rather than through intermediary thunks. + + // Tally up any thunks that have already been placed that have VA higher than + // inputs[callIdx]. First, find the index of the first thunk that is beyond + // the current inputs[callIdx]. + auto itPostcallIdxThunks = + llvm::partition_point(thunks, [isecVA](const ConcatInputSection *t) { + return t->getVA() <= isecVA; + }); + uint64_t existingForwardThunks = thunks.end() - itPostcallIdxThunks; + uint64_t forwardBranchRange = target->forwardBranchRange; assert(isecEnd > forwardBranchRange && "should not run thunk insertion if all code fits in jump range"); assert(isecEnd - isecVA <= forwardBranchRange && "should only finalize sections in jump range"); - uint64_t stubsInRangeVA = isecEnd + maxPotentialThunks * target->thunkSize + - in.stubs->getSize() - forwardBranchRange; + + // Estimate the maximum size of the code, right before the stubs section. + uint64_t maxTextSize = 0; + // Add the size of all the inputs, including the unprocessed ones. + maxTextSize += isecEnd; + + // Add the size of the thunks that have already been created that are ahead of + // inputs[callIdx]. These are already created thunks that will be interleaved + // with inputs[callIdx...end]. + maxTextSize += existingForwardThunks * target->thunkSize; + + // Add the size of the thunks that may be created in the future. Since + // 'maxPotentialThunks' overcounts, this is an estimate of the upper limit. + maxTextSize += maxPotentialThunks * target->thunkSize; + + // Estimated maximum VA of last stub. + uint64_t maxVAOfLastStub = maxTextSize + in.stubs->getSize(); + + // Estimate the address after which call sites can safely call stubs + // directly rather than through intermediary thunks. + uint64_t stubsInRangeVA = maxVAOfLastStub - forwardBranchRange; + log("thunks = " + std::to_string(thunkMap.size()) + ", potential = " + std::to_string(maxPotentialThunks) + ", stubs = " + std::to_string(in.stubs->getSize()) + ", isecVA = " +