diff --git a/bolt/include/bolt/Core/MCPlusBuilder.h b/bolt/include/bolt/Core/MCPlusBuilder.h index 5e349cd69fb4..295558c0d3af 100644 --- a/bolt/include/bolt/Core/MCPlusBuilder.h +++ b/bolt/include/bolt/Core/MCPlusBuilder.h @@ -1371,20 +1371,13 @@ public: /// Return true if \p Inst has RestoreState annotation. bool hasRestoreState(const MCInst &Inst) const; - /// Stores RA Signed annotation on \p Inst. - void setRASigned(MCInst &Inst) const; + /// Sets kRASigned or kRAUnsigned annotation on \p Inst. + /// Fails if \p Inst has either annotation already set. + void setRAState(MCInst &Inst, bool State) const; - /// Return true if \p Inst has Signed RA annotation. - bool isRASigned(const MCInst &Inst) const; - - /// Stores RA Unsigned annotation on \p Inst. - void setRAUnsigned(MCInst &Inst) const; - - /// Return true if \p Inst has Unsigned RA annotation. - bool isRAUnsigned(const MCInst &Inst) const; - - /// Return true if \p Inst doesn't have any annotation related to RA state. - bool isRAStateUnknown(const MCInst &Inst) const; + /// Return true if \p Inst has kRASigned annotation, false if it has + /// kRAUnsigned annotation, and std::nullopt if neither annotation is set. + std::optional getRAState(const MCInst &Inst) const; /// Return true if the instruction is a call with an exception handling info. virtual bool isInvoke(const MCInst &Inst) const { diff --git a/bolt/lib/Core/MCPlusBuilder.cpp b/bolt/lib/Core/MCPlusBuilder.cpp index e96de80bfa70..0cb4ba1ebfbd 100644 --- a/bolt/lib/Core/MCPlusBuilder.cpp +++ b/bolt/lib/Core/MCPlusBuilder.cpp @@ -186,26 +186,21 @@ bool MCPlusBuilder::hasRestoreState(const MCInst &Inst) const { return hasAnnotation(Inst, MCAnnotation::kRestoreState); } -void MCPlusBuilder::setRASigned(MCInst &Inst) const { +void MCPlusBuilder::setRAState(MCInst &Inst, bool State) const { assert(!hasAnnotation(Inst, MCAnnotation::kRASigned)); - setAnnotationOpValue(Inst, MCAnnotation::kRASigned, true); -} - -bool MCPlusBuilder::isRASigned(const MCInst &Inst) const { - return hasAnnotation(Inst, MCAnnotation::kRASigned); -} - -void MCPlusBuilder::setRAUnsigned(MCInst &Inst) const { assert(!hasAnnotation(Inst, MCAnnotation::kRAUnsigned)); - setAnnotationOpValue(Inst, MCAnnotation::kRAUnsigned, true); + if (State) + setAnnotationOpValue(Inst, MCAnnotation::kRASigned, true); + else + setAnnotationOpValue(Inst, MCAnnotation::kRAUnsigned, true); } -bool MCPlusBuilder::isRAUnsigned(const MCInst &Inst) const { - return hasAnnotation(Inst, MCAnnotation::kRAUnsigned); -} - -bool MCPlusBuilder::isRAStateUnknown(const MCInst &Inst) const { - return !(isRAUnsigned(Inst) || isRASigned(Inst)); +std::optional MCPlusBuilder::getRAState(const MCInst &Inst) const { + if (hasAnnotation(Inst, MCAnnotation::kRASigned)) + return true; + if (hasAnnotation(Inst, MCAnnotation::kRAUnsigned)) + return false; + return std::nullopt; } std::optional MCPlusBuilder::getEHInfo(const MCInst &Inst) const { diff --git a/bolt/lib/Passes/InsertNegateRAStatePass.cpp b/bolt/lib/Passes/InsertNegateRAStatePass.cpp index 33664e1160a7..775b7795e77c 100644 --- a/bolt/lib/Passes/InsertNegateRAStatePass.cpp +++ b/bolt/lib/Passes/InsertNegateRAStatePass.cpp @@ -21,7 +21,12 @@ using namespace llvm; namespace llvm { namespace bolt { +static bool PassFailed = false; + void InsertNegateRAState::runOnFunction(BinaryFunction &BF) { + if (PassFailed) + return; + BinaryContext &BC = BF.getBinaryContext(); if (BF.getState() == BinaryFunction::State::Empty) @@ -39,7 +44,7 @@ void InsertNegateRAState::runOnFunction(BinaryFunction &BF) { for (FunctionFragment &FF : BF.getLayout().fragments()) { coverFunctionFragmentStart(BF, FF); bool FirstIter = true; - MCInst PrevInst; + bool PrevRAState = false; // As this pass runs after function splitting, we should only check // consecutive instructions inside FunctionFragments. for (BinaryBasicBlock *BB : FF) { @@ -47,18 +52,23 @@ void InsertNegateRAState::runOnFunction(BinaryFunction &BF) { MCInst &Inst = *It; if (BC.MIB->isCFI(Inst)) continue; + auto RAState = BC.MIB->getRAState(Inst); + if (!RAState) { + BC.errs() << "BOLT-ERROR: unknown RAState after inferUnknownStates " + << " in function " << BF.getPrintName() << "\n"; + PassFailed = true; + return; + } if (!FirstIter) { // Consecutive instructions with different RAState means we need to // add a OpNegateRAState. - if ((BC.MIB->isRASigned(PrevInst) && BC.MIB->isRAUnsigned(Inst)) || - (BC.MIB->isRAUnsigned(PrevInst) && BC.MIB->isRASigned(Inst))) { + if (*RAState != PrevRAState) It = BF.addCFIInstruction( BB, It, MCCFIInstruction::createNegateRAState(nullptr)); - } } else { FirstIter = false; } - PrevInst = *It; + PrevRAState = *RAState; } } } @@ -81,10 +91,17 @@ void InsertNegateRAState::coverFunctionFragmentStart(BinaryFunction &BF, }); // If a function is already split in the input, the first FF can also start // with Signed state. This covers that scenario as well. - if (BC.MIB->isRASigned(*((*FirstNonEmpty)->begin()))) { - BF.addCFIInstruction(*FirstNonEmpty, (*FirstNonEmpty)->begin(), - MCCFIInstruction::createNegateRAState(nullptr)); + auto II = (*FirstNonEmpty)->getFirstNonPseudo(); + auto RAState = BC.MIB->getRAState(*II); + if (!RAState) { + BC.errs() << "BOLT-ERROR: unknown RAState after inferUnknownStates " + << " in function " << BF.getPrintName() << "\n"; + PassFailed = true; + return; } + if (*RAState) + BF.addCFIInstruction(*FirstNonEmpty, II, + MCCFIInstruction::createNegateRAState(nullptr)); } void InsertNegateRAState::inferUnknownStates(BinaryFunction &BF) { @@ -96,15 +113,21 @@ void InsertNegateRAState::inferUnknownStates(BinaryFunction &BF) { if (BC.MIB->isCFI(Inst)) continue; - if (!FirstIter && BC.MIB->isRAStateUnknown(Inst)) { - if (BC.MIB->isRASigned(PrevInst) || BC.MIB->isPSignOnLR(PrevInst)) { - BC.MIB->setRASigned(Inst); - } else if (BC.MIB->isRAUnsigned(PrevInst) || - BC.MIB->isPAuthOnLR(PrevInst)) { - BC.MIB->setRAUnsigned(Inst); + auto RAState = BC.MIB->getRAState(Inst); + if (!FirstIter && !RAState) { + if (BC.MIB->isPSignOnLR(PrevInst)) + RAState = true; + else if (BC.MIB->isPAuthOnLR(PrevInst)) + RAState = false; + else { + auto PrevRAState = BC.MIB->getRAState(PrevInst); + RAState = PrevRAState ? *PrevRAState : false; } + BC.MIB->setRAState(Inst, *RAState); } else { FirstIter = false; + if (!RAState) + BC.MIB->setRAState(Inst, BF.getInitialRAState()); } PrevInst = Inst; } @@ -135,6 +158,8 @@ Error InsertNegateRAState::runOnFunctions(BinaryContext &BC) { << " functions " << format("(%.2lf%%).\n", (100.0 * FunctionsModified) / BC.getBinaryFunctions().size()); + if (PassFailed) + return createFatalBOLTError(""); return Error::success(); } diff --git a/bolt/lib/Passes/MarkRAStates.cpp b/bolt/lib/Passes/MarkRAStates.cpp index b262d66732b7..51075be0e1ac 100644 --- a/bolt/lib/Passes/MarkRAStates.cpp +++ b/bolt/lib/Passes/MarkRAStates.cpp @@ -72,9 +72,6 @@ bool MarkRAStates::runOnFunction(BinaryFunction &BF) { BF.setIgnored(); return false; } - // The signing instruction itself is unsigned, the next will be - // signed. - BC.MIB->setRAUnsigned(Inst); } else if (BC.MIB->isPAuthOnLR(Inst)) { if (!RAState) { // RA authenticating instructions should only follow signed RA state. @@ -86,15 +83,10 @@ bool MarkRAStates::runOnFunction(BinaryFunction &BF) { BF.setIgnored(); return false; } - // The authenticating instruction itself is signed, but the next will be - // unsigned. - BC.MIB->setRASigned(Inst); - } else if (RAState) { - BC.MIB->setRASigned(Inst); - } else { - BC.MIB->setRAUnsigned(Inst); } + BC.MIB->setRAState(Inst, RAState); + // Updating RAState. All updates are valid from the next instruction. // Because the same instruction can have remember and restore, the order // here is relevant. This is the reason to loop over Annotations instead