From 04cb9ca173c2f2d0f0f9db0710d96417547568fe Mon Sep 17 00:00:00 2001 From: Gergely Balint Date: Wed, 2 Apr 2025 10:07:52 +0200 Subject: [PATCH] [BOLT] Change annotateCFIState to not fail on errors - instead setIgnored() on failing functions - check isIgnored() in several Passes to skip previously ignored functions --- bolt/include/bolt/Core/BinaryFunction.h | 2 +- bolt/lib/Core/BinaryFunction.cpp | 21 +++++++++++++++++---- bolt/lib/Passes/ADRRelaxationPass.cpp | 2 +- bolt/lib/Passes/Aligner.cpp | 8 +++++++- bolt/lib/Passes/BinaryPasses.cpp | 10 ++++++++-- bolt/lib/Passes/FixRelaxationPass.cpp | 2 ++ bolt/lib/Passes/Instrumentation.cpp | 2 +- bolt/lib/Passes/LongJmp.cpp | 4 ++++ bolt/lib/Passes/MCF.cpp | 2 ++ bolt/lib/Passes/ValidateInternalCalls.cpp | 2 ++ bolt/lib/Passes/VeneerElimination.cpp | 7 +++---- bolt/lib/Rewrite/RewriteInstance.cpp | 3 +++ 12 files changed, 51 insertions(+), 14 deletions(-) diff --git a/bolt/include/bolt/Core/BinaryFunction.h b/bolt/include/bolt/Core/BinaryFunction.h index d3d11f8c5fb73..753e8ef183a40 100644 --- a/bolt/include/bolt/Core/BinaryFunction.h +++ b/bolt/include/bolt/Core/BinaryFunction.h @@ -453,7 +453,7 @@ class BinaryFunction { /// Annotate each basic block entry with its current CFI state. This is /// run right after the construction of CFG while basic blocks are in their /// original order. - void annotateCFIState(); + bool annotateCFIState(); /// Associate DW_CFA_GNU_args_size info with invoke instructions /// (call instructions with non-empty landing pad). diff --git a/bolt/lib/Core/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp index d1b293ada5fdc..638b2e89e8853 100644 --- a/bolt/lib/Core/BinaryFunction.cpp +++ b/bolt/lib/Core/BinaryFunction.cpp @@ -2426,7 +2426,13 @@ Error BinaryFunction::buildCFG(MCPlusBuilder::AllocatorIdTy AllocatorId) { recomputeLandingPads(); // Assign CFI information to each BB entry. - annotateCFIState(); + bool SuccessfulAnnotation = annotateCFIState(); + if (!SuccessfulAnnotation) { + BC.errs() << "BOLT-WARNING: failed to annotate CFI state for " + << this->getPrintName() << "\n"; + setIgnored(); + return createNonFatalBOLTError(""); + } // Annotate invoke instructions with GNU_args_size data. propagateGnuArgsSizeInfo(AllocatorId); @@ -2443,7 +2449,11 @@ Error BinaryFunction::buildCFG(MCPlusBuilder::AllocatorIdTy AllocatorId) { Layout.updateLayoutIndices(); - normalizeCFIState(); + if (SuccessfulAnnotation) + normalizeCFIState(); + else + BC.errs() << "BOLT-WARNING: cannot normalize CFI State for " + << this->getPrintName() << "\n"; // Clean-up memory taken by intermediate structures. // @@ -2618,7 +2628,7 @@ uint64_t BinaryFunction::getFunctionScore() const { return FunctionScore; } -void BinaryFunction::annotateCFIState() { +bool BinaryFunction::annotateCFIState() { assert(CurrentState == State::Disassembled && "unexpected function state"); assert(!BasicBlocks.empty() && "basic block list should not be empty"); @@ -2654,7 +2664,9 @@ void BinaryFunction::annotateCFIState() { EffectiveState = State; break; case MCCFIInstruction::OpRestoreState: - assert(!StateStack.empty() && "corrupt CFI stack"); + if (StateStack.empty()) { + return false; + } EffectiveState = StateStack.top(); StateStack.pop(); break; @@ -2673,6 +2685,7 @@ void BinaryFunction::annotateCFIState() { BC.errs() << "BOLT-WARNING: non-empty CFI stack at the end of " << *this << '\n'; } + return StateStack.empty(); } namespace { diff --git a/bolt/lib/Passes/ADRRelaxationPass.cpp b/bolt/lib/Passes/ADRRelaxationPass.cpp index 4b37a061ac12d..39e6b9529999b 100644 --- a/bolt/lib/Passes/ADRRelaxationPass.cpp +++ b/bolt/lib/Passes/ADRRelaxationPass.cpp @@ -36,7 +36,7 @@ namespace bolt { static bool PassFailed = false; void ADRRelaxationPass::runOnFunction(BinaryFunction &BF) { - if (PassFailed) + if (PassFailed || BF.isIgnored()) return; BinaryContext &BC = BF.getBinaryContext(); diff --git a/bolt/lib/Passes/Aligner.cpp b/bolt/lib/Passes/Aligner.cpp index c3ddedaaa1466..431d469fb9737 100644 --- a/bolt/lib/Passes/Aligner.cpp +++ b/bolt/lib/Passes/Aligner.cpp @@ -62,6 +62,8 @@ namespace bolt { // Align function to the specified byte-boundary (typically, 64) offsetting // the fuction by not more than the corresponding value static void alignMaxBytes(BinaryFunction &Function) { + if (Function.isIgnored()) + return; Function.setAlignment(opts::AlignFunctions); Function.setMaxAlignmentBytes(opts::AlignFunctionsMaxBytes); Function.setMaxColdAlignmentBytes(opts::AlignFunctionsMaxBytes); @@ -73,6 +75,9 @@ static void alignMaxBytes(BinaryFunction &Function) { // -- the specified number of bytes static void alignCompact(BinaryFunction &Function, const MCCodeEmitter *Emitter) { + + if (Function.isIgnored()) + return; const BinaryContext &BC = Function.getBinaryContext(); size_t HotSize = 0; size_t ColdSize = 0; @@ -97,7 +102,8 @@ static void alignCompact(BinaryFunction &Function, void AlignerPass::alignBlocks(BinaryFunction &Function, const MCCodeEmitter *Emitter) { - if (!Function.hasValidProfile() || !Function.isSimple()) + if (Function.isIgnored() || !Function.hasValidProfile() || + !Function.isSimple()) return; const BinaryContext &BC = Function.getBinaryContext(); diff --git a/bolt/lib/Passes/BinaryPasses.cpp b/bolt/lib/Passes/BinaryPasses.cpp index d8628c62d8654..e4c6aaaa00916 100644 --- a/bolt/lib/Passes/BinaryPasses.cpp +++ b/bolt/lib/Passes/BinaryPasses.cpp @@ -1810,10 +1810,14 @@ Error PrintProgramStats::runOnFunctions(BinaryContext &BC) { } Error InstructionLowering::runOnFunctions(BinaryContext &BC) { - for (auto &BFI : BC.getBinaryFunctions()) - for (BinaryBasicBlock &BB : BFI.second) + for (auto &BFI : BC.getBinaryFunctions()) { + BinaryFunction &BF = BFI.second; + if (BF.isIgnored()) + continue; + for (BinaryBasicBlock &BB : BF) for (MCInst &Instruction : BB) BC.MIB->lowerTailCall(Instruction); + } return Error::success(); } @@ -2029,6 +2033,8 @@ Error SpecializeMemcpy1::runOnFunctions(BinaryContext &BC) { } void RemoveNops::runOnFunction(BinaryFunction &BF) { + if (BF.isIgnored()) + return; const BinaryContext &BC = BF.getBinaryContext(); for (BinaryBasicBlock &BB : BF) { for (int64_t I = BB.size() - 1; I >= 0; --I) { diff --git a/bolt/lib/Passes/FixRelaxationPass.cpp b/bolt/lib/Passes/FixRelaxationPass.cpp index 7c970e464a94e..108749f1129ed 100644 --- a/bolt/lib/Passes/FixRelaxationPass.cpp +++ b/bolt/lib/Passes/FixRelaxationPass.cpp @@ -24,6 +24,8 @@ namespace bolt { // target of ADD is another symbol. When found change ADRP symbol reference to // the ADDs one. void FixRelaxations::runOnFunction(BinaryFunction &BF) { + if (BF.isIgnored()) + return; BinaryContext &BC = BF.getBinaryContext(); for (BinaryBasicBlock &BB : BF) { for (auto II = BB.begin(); II != BB.end(); ++II) { diff --git a/bolt/lib/Passes/Instrumentation.cpp b/bolt/lib/Passes/Instrumentation.cpp index fbf889279f1c0..e11a176399277 100644 --- a/bolt/lib/Passes/Instrumentation.cpp +++ b/bolt/lib/Passes/Instrumentation.cpp @@ -368,7 +368,7 @@ bool Instrumentation::instrumentOneTarget( void Instrumentation::instrumentFunction(BinaryFunction &Function, MCPlusBuilder::AllocatorIdTy AllocId) { - if (Function.hasUnknownControlFlow()) + if (Function.hasUnknownControlFlow() || Function.isIgnored()) return; BinaryContext &BC = Function.getBinaryContext(); diff --git a/bolt/lib/Passes/LongJmp.cpp b/bolt/lib/Passes/LongJmp.cpp index e6bd417705e6f..b8674f60ce969 100644 --- a/bolt/lib/Passes/LongJmp.cpp +++ b/bolt/lib/Passes/LongJmp.cpp @@ -557,6 +557,10 @@ Error LongJmpPass::relax(BinaryFunction &Func, bool &Modified) { const BinaryContext &BC = Func.getBinaryContext(); assert(BC.isAArch64() && "Unsupported arch"); + + if (Func.isIgnored()) + return Error::success(); + constexpr int InsnSize = 4; // AArch64 std::vector>> Insertions; diff --git a/bolt/lib/Passes/MCF.cpp b/bolt/lib/Passes/MCF.cpp index 77dea7369140e..c378803469661 100644 --- a/bolt/lib/Passes/MCF.cpp +++ b/bolt/lib/Passes/MCF.cpp @@ -435,6 +435,8 @@ void equalizeBBCounts(DataflowInfoManager &Info, BinaryFunction &BF) { } void EstimateEdgeCounts::runOnFunction(BinaryFunction &BF) { + if (BF.isIgnored()) + return; EdgeWeightMap PredEdgeWeights; EdgeWeightMap SuccEdgeWeights; if (!opts::IterativeGuess) { diff --git a/bolt/lib/Passes/ValidateInternalCalls.cpp b/bolt/lib/Passes/ValidateInternalCalls.cpp index bdab895b6ac2e..a5ee058fb9fe5 100644 --- a/bolt/lib/Passes/ValidateInternalCalls.cpp +++ b/bolt/lib/Passes/ValidateInternalCalls.cpp @@ -306,6 +306,8 @@ Error ValidateInternalCalls::runOnFunctions(BinaryContext &BC) { std::set NeedsValidation; for (auto &BFI : BC.getBinaryFunctions()) { BinaryFunction &Function = BFI.second; + if (Function.isIgnored()) + continue; for (BinaryBasicBlock &BB : Function) { for (MCInst &Inst : BB) { if (getInternalCallTarget(Function, Inst)) { diff --git a/bolt/lib/Passes/VeneerElimination.cpp b/bolt/lib/Passes/VeneerElimination.cpp index 99d0ffeca8cc2..ead875de15ab1 100644 --- a/bolt/lib/Passes/VeneerElimination.cpp +++ b/bolt/lib/Passes/VeneerElimination.cpp @@ -40,10 +40,7 @@ Error VeneerElimination::runOnFunctions(BinaryContext &BC) { std::unordered_map VeneerDestinations; uint64_t NumEliminatedVeneers = 0; for (BinaryFunction &BF : llvm::make_second_range(BC.getBinaryFunctions())) { - if (!isPossibleVeneer(BF)) - continue; - - if (BF.isIgnored()) + if (BF.isIgnored() || !isPossibleVeneer(BF)) continue; MCInst &FirstInstruction = *(BF.begin()->begin()); @@ -84,6 +81,8 @@ Error VeneerElimination::runOnFunctions(BinaryContext &BC) { uint64_t VeneerCallers = 0; for (BinaryFunction &BF : llvm::make_second_range(BC.getBinaryFunctions())) { + if (BF.isIgnored()) + continue; for (BinaryBasicBlock &BB : BF) { for (MCInst &Instr : BB) { if (!BC.MIB->isCall(Instr) || BC.MIB->isIndirectCall(Instr)) diff --git a/bolt/lib/Rewrite/RewriteInstance.cpp b/bolt/lib/Rewrite/RewriteInstance.cpp index f204aa3eb8a38..4f46a8dcb13da 100644 --- a/bolt/lib/Rewrite/RewriteInstance.cpp +++ b/bolt/lib/Rewrite/RewriteInstance.cpp @@ -3503,6 +3503,9 @@ void RewriteInstance::postProcessFunctions() { if (Function.empty()) continue; + if (Function.isIgnored()) + continue; + Function.postProcessCFG(); if (opts::PrintAll || opts::PrintCFG)