From fe6369ae90d7a69dc013781678b54b6ac39ea9d0 Mon Sep 17 00:00:00 2001 From: Alex B Date: Wed, 18 Dec 2024 23:37:48 -0800 Subject: [PATCH 1/6] [lld-macho] Fix branch extension thunk estimation logic --- lld/MachO/ConcatOutputSection.cpp | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/lld/MachO/ConcatOutputSection.cpp b/lld/MachO/ConcatOutputSection.cpp index 6a9301f84a03e..9610a80e309ba 100644 --- a/lld/MachO/ConcatOutputSection.cpp +++ b/lld/MachO/ConcatOutputSection.cpp @@ -184,6 +184,15 @@ uint64_t TextOutputSection::estimateStubsInRangeVA(size_t callIdx) const { InputSection *isec = inputs[i]; isecEnd = alignToPowerOf2(isecEnd, isec->align) + isec->getSize(); } + + // Tally up any thunks that have already been placed that have address higher + // than the equivalent callIdx. We first find the index of the first thunk + // that is beyond the current inputs[callIdx]. + auto itPostcallIdxThunks = std::partition_point( + thunks.begin(), thunks.end(), + [isecVA](const ConcatInputSection *t) { return t->getVA() <= isecVA; }); + uint32_t existingForwardThunks = thunks.end() - itPostcallIdxThunks; + // Estimate the address after which call sites can safely call stubs // directly rather than through intermediary thunks. uint64_t forwardBranchRange = target->forwardBranchRange; @@ -191,8 +200,21 @@ uint64_t TextOutputSection::estimateStubsInRangeVA(size_t callIdx) const { "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 + uint32_t maxTextSize = 0; + // Add the size of all the inputs, including the unprocessed ones. + maxTextSize += isecEnd; + + // Add the size of the thunks that may be created in the future + maxTextSize += maxPotentialThunks * target->thunkSize; + + // Add the size of the thunks that have already been created that are ahead + maxTextSize += existingForwardThunks * target->thunkSize; + + uint64_t stubsInRangeVA = + maxTextSize + in.stubs->getSize() - forwardBranchRange; + log("thunks = " + std::to_string(thunkMap.size()) + ", potential = " + std::to_string(maxPotentialThunks) + ", stubs = " + std::to_string(in.stubs->getSize()) + ", isecVA = " + From bc5e3426d0f71a2fc364258a4009b3848f856d95 Mon Sep 17 00:00:00 2001 From: Alex B Date: Thu, 19 Dec 2024 09:53:27 -0800 Subject: [PATCH 2/6] uint32 => uint64 + extra comments --- lld/MachO/ConcatOutputSection.cpp | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/lld/MachO/ConcatOutputSection.cpp b/lld/MachO/ConcatOutputSection.cpp index 9610a80e309ba..cf3ab72c351e4 100644 --- a/lld/MachO/ConcatOutputSection.cpp +++ b/lld/MachO/ConcatOutputSection.cpp @@ -191,7 +191,7 @@ uint64_t TextOutputSection::estimateStubsInRangeVA(size_t callIdx) const { auto itPostcallIdxThunks = std::partition_point( thunks.begin(), thunks.end(), [isecVA](const ConcatInputSection *t) { return t->getVA() <= isecVA; }); - uint32_t existingForwardThunks = thunks.end() - itPostcallIdxThunks; + uint64_t existingForwardThunks = thunks.end() - itPostcallIdxThunks; // Estimate the address after which call sites can safely call stubs // directly rather than through intermediary thunks. @@ -201,26 +201,30 @@ uint64_t TextOutputSection::estimateStubsInRangeVA(size_t callIdx) const { assert(isecEnd - isecVA <= forwardBranchRange && "should only finalize sections in jump range"); - // Estimate the maximum size of the code, right before the stubs section - uint32_t maxTextSize = 0; + // 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 may be created in the future + // 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; // Add the size of the thunks that have already been created that are ahead maxTextSize += existingForwardThunks * target->thunkSize; - uint64_t stubsInRangeVA = - maxTextSize + in.stubs->getSize() - forwardBranchRange; + // Estimated maximum VA of last stub. + uint64_t maxVAOfLastStub = maxTextSize + in.stubs->getSize(); + + // Calculaate the first address that is gueranteed to not need a thunk to + // reach any stub.git + 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 = " + - utohexstr(isecVA) + ", threshold = " + utohexstr(stubsInRangeVA) + - ", isecEnd = " + utohexstr(isecEnd) + - ", tail = " + utohexstr(isecEnd - isecVA) + + ", potential = " + std::to_string(maxPotentialThunks) + ", stubs = " + + std::to_string(in.stubs->getSize()) + ", isecVA = " + utohexstr(isecVA) + + ", threshold = " + utohexstr(stubsInRangeVA) + ", isecEnd = " + + utohexstr(isecEnd) + ", tail = " + utohexstr(isecEnd - isecVA) + ", slop = " + utohexstr(forwardBranchRange - (isecEnd - isecVA))); return stubsInRangeVA; } From 43949e0e11fe3f8d26ae3d8367744582d631ae5f Mon Sep 17 00:00:00 2001 From: Alex B Date: Thu, 19 Dec 2024 09:59:44 -0800 Subject: [PATCH 3/6] Undo fixing unrelated formatting --- lld/MachO/ConcatOutputSection.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/lld/MachO/ConcatOutputSection.cpp b/lld/MachO/ConcatOutputSection.cpp index cf3ab72c351e4..a55e8db2cd27a 100644 --- a/lld/MachO/ConcatOutputSection.cpp +++ b/lld/MachO/ConcatOutputSection.cpp @@ -221,10 +221,11 @@ uint64_t TextOutputSection::estimateStubsInRangeVA(size_t callIdx) const { 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 = " + utohexstr(isecVA) + - ", threshold = " + utohexstr(stubsInRangeVA) + ", isecEnd = " + - utohexstr(isecEnd) + ", tail = " + utohexstr(isecEnd - isecVA) + + ", potential = " + std::to_string(maxPotentialThunks) + + ", stubs = " + std::to_string(in.stubs->getSize()) + ", isecVA = " + + utohexstr(isecVA) + ", threshold = " + utohexstr(stubsInRangeVA) + + ", isecEnd = " + utohexstr(isecEnd) + + ", tail = " + utohexstr(isecEnd - isecVA) + ", slop = " + utohexstr(forwardBranchRange - (isecEnd - isecVA))); return stubsInRangeVA; } From e1408deac4bf34854c4696ed092d486c28050258 Mon Sep 17 00:00:00 2001 From: Alex B Date: Thu, 19 Dec 2024 11:28:41 -0800 Subject: [PATCH 4/6] Fix typo + add better comments to code. --- lld/MachO/ConcatOutputSection.cpp | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/lld/MachO/ConcatOutputSection.cpp b/lld/MachO/ConcatOutputSection.cpp index a55e8db2cd27a..e6dc65b9d4d2c 100644 --- a/lld/MachO/ConcatOutputSection.cpp +++ b/lld/MachO/ConcatOutputSection.cpp @@ -185,9 +185,9 @@ uint64_t TextOutputSection::estimateStubsInRangeVA(size_t callIdx) const { isecEnd = alignToPowerOf2(isecEnd, isec->align) + isec->getSize(); } - // Tally up any thunks that have already been placed that have address higher - // than the equivalent callIdx. We first find the index of the first thunk - // that is beyond the current inputs[callIdx]. + // 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 = std::partition_point( thunks.begin(), thunks.end(), [isecVA](const ConcatInputSection *t) { return t->getVA() <= isecVA; }); @@ -206,18 +206,20 @@ uint64_t TextOutputSection::estimateStubsInRangeVA(size_t callIdx) const { // 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; - // Add the size of the thunks that have already been created that are ahead - maxTextSize += existingForwardThunks * target->thunkSize; - // Estimated maximum VA of last stub. uint64_t maxVAOfLastStub = maxTextSize + in.stubs->getSize(); // Calculaate the first address that is gueranteed to not need a thunk to - // reach any stub.git + // reach any stub. uint64_t stubsInRangeVA = maxVAOfLastStub - forwardBranchRange; log("thunks = " + std::to_string(thunkMap.size()) + From ec0c7ea321913dcc7b362571f7130676e8a7d689 Mon Sep 17 00:00:00 2001 From: Alex B Date: Thu, 19 Dec 2024 12:00:34 -0800 Subject: [PATCH 5/6] std::partition_point => llvm::partition_point --- lld/MachO/ConcatOutputSection.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lld/MachO/ConcatOutputSection.cpp b/lld/MachO/ConcatOutputSection.cpp index e6dc65b9d4d2c..294c2eb6579e3 100644 --- a/lld/MachO/ConcatOutputSection.cpp +++ b/lld/MachO/ConcatOutputSection.cpp @@ -188,9 +188,10 @@ uint64_t TextOutputSection::estimateStubsInRangeVA(size_t callIdx) const { // 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 = std::partition_point( - thunks.begin(), thunks.end(), - [isecVA](const ConcatInputSection *t) { return t->getVA() <= isecVA; }); + auto itPostcallIdxThunks = + llvm::partition_point(thunks, [isecVA](const ConcatInputSection *t) { + return t->getVA() <= isecVA; + }); uint64_t existingForwardThunks = thunks.end() - itPostcallIdxThunks; // Estimate the address after which call sites can safely call stubs From f8782e44bc5b32dc3a001637850b95bad6317e4f Mon Sep 17 00:00:00 2001 From: Alex B Date: Tue, 7 Jan 2025 16:05:08 -0800 Subject: [PATCH 6/6] Address Feedback Nr.2 --- lld/MachO/ConcatOutputSection.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lld/MachO/ConcatOutputSection.cpp b/lld/MachO/ConcatOutputSection.cpp index 294c2eb6579e3..d64816ec695ad 100644 --- a/lld/MachO/ConcatOutputSection.cpp +++ b/lld/MachO/ConcatOutputSection.cpp @@ -194,8 +194,6 @@ uint64_t TextOutputSection::estimateStubsInRangeVA(size_t callIdx) const { }); uint64_t existingForwardThunks = thunks.end() - itPostcallIdxThunks; - // Estimate the address after which call sites can safely call stubs - // directly rather than through intermediary thunks. uint64_t forwardBranchRange = target->forwardBranchRange; assert(isecEnd > forwardBranchRange && "should not run thunk insertion if all code fits in jump range"); @@ -219,8 +217,8 @@ uint64_t TextOutputSection::estimateStubsInRangeVA(size_t callIdx) const { // Estimated maximum VA of last stub. uint64_t maxVAOfLastStub = maxTextSize + in.stubs->getSize(); - // Calculaate the first address that is gueranteed to not need a thunk to - // reach any stub. + // 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()) +