[BOLT] Simplify RAState helpers (NFCI) (#162820)

- unify isRAStateSigned and isRAStateUnsigned to a common getRAState,
- unify setRASigned and setRAUnsigned into setRAState(MCInst, bool),
- update users of these to match the new implementations.
This commit is contained in:
Gergely Bálint 2025-11-10 16:45:39 +01:00 committed by GitHub
parent 2d381bf65d
commit cd68056d13
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 58 additions and 53 deletions

View File

@ -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<bool> 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 {

View File

@ -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<bool> 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<MCLandingPad> MCPlusBuilder::getEHInfo(const MCInst &Inst) const {

View File

@ -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();
}

View File

@ -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