[llvm] [BOLT][AArch64] Support for pointer authentication (v2) (PR #120064)
Gergely Bálint via llvm-commits
llvm-commits at lists.llvm.org
Wed Jan 22 01:16:17 PST 2025
https://github.com/bgergely0 updated https://github.com/llvm/llvm-project/pull/120064
>From 3063eaf8525a9c88cf664256d973bd72732984bf Mon Sep 17 00:00:00 2001
From: Gergely Balint <gergely.balint at arm.com>
Date: Tue, 19 Nov 2024 09:43:25 +0100
Subject: [PATCH 1/7] [BOLT] Recognize paciasp and autiasp instructions
---
bolt/include/bolt/Core/MCPlusBuilder.h | 7 +++++++
bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp | 6 ++++++
2 files changed, 13 insertions(+)
diff --git a/bolt/include/bolt/Core/MCPlusBuilder.h b/bolt/include/bolt/Core/MCPlusBuilder.h
index 3634fed9757ceb..b720a56806bdb7 100644
--- a/bolt/include/bolt/Core/MCPlusBuilder.h
+++ b/bolt/include/bolt/Core/MCPlusBuilder.h
@@ -648,6 +648,13 @@ class MCPlusBuilder {
llvm_unreachable("not implemented");
return false;
}
+ virtual bool isPAuth(MCInst &Inst) const {
+ llvm_unreachable("not implemented");
+ }
+
+ virtual bool isPSign(MCInst &Inst) const {
+ llvm_unreachable("not implemented");
+ }
virtual bool isCleanRegXOR(const MCInst &Inst) const {
llvm_unreachable("not implemented");
diff --git a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp
index 7e08e5c81d26ff..c07a93c0a1460b 100644
--- a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp
+++ b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp
@@ -316,6 +316,12 @@ class AArch64MCPlusBuilder : public MCPlusBuilder {
}
return false;
}
+ bool isPAuth(MCInst &Inst) const override {
+ return Inst.getOpcode() == AArch64::AUTIASP;
+ }
+ bool isPSign(MCInst &Inst) const override {
+ return Inst.getOpcode() == AArch64::PACIASP;
+ }
bool isRegToRegMove(const MCInst &Inst, MCPhysReg &From,
MCPhysReg &To) const override {
>From 3e813ed14d27d399be0397cc69f768a3c8e78c4d Mon Sep 17 00:00:00 2001
From: Gergely Balint <gergely.balint at arm.com>
Date: Mon, 16 Dec 2024 10:45:41 +0100
Subject: [PATCH 2/7] [BOLT] Support for OpNegateRAState
- save OpNegateRAState, RememberState and RestoreState locations
when parsing input
- determine the RA states from these before other optimizations
in MarkRAStates Pass
- after optimizations, we can insert OpNegateRAState at state
boundaries and other needed locations (e.g. split functions)
in InsertNegateRAStatePass
---
bolt/include/bolt/Core/BinaryFunction.h | 46 ++++++
bolt/include/bolt/Core/MCPlus.h | 10 +-
bolt/include/bolt/Core/MCPlusBuilder.h | 59 +++++++
.../bolt/Passes/InsertNegateRAStatePass.h | 25 +++
bolt/include/bolt/Passes/MarkRAStates.h | 22 +++
bolt/lib/Core/BinaryBasicBlock.cpp | 6 +-
bolt/lib/Core/BinaryFunction.cpp | 1 +
bolt/lib/Core/Exceptions.cpp | 20 ++-
bolt/lib/Core/MCPlusBuilder.cpp | 86 ++++++++++
bolt/lib/Passes/CMakeLists.txt | 2 +
bolt/lib/Passes/InsertNegateRAStatePass.cpp | 153 ++++++++++++++++++
bolt/lib/Passes/MarkRAStates.cpp | 122 ++++++++++++++
bolt/lib/Rewrite/BinaryPassManager.cpp | 6 +
13 files changed, 554 insertions(+), 4 deletions(-)
create mode 100644 bolt/include/bolt/Passes/InsertNegateRAStatePass.h
create mode 100644 bolt/include/bolt/Passes/MarkRAStates.h
create mode 100644 bolt/lib/Passes/InsertNegateRAStatePass.cpp
create mode 100644 bolt/lib/Passes/MarkRAStates.cpp
diff --git a/bolt/include/bolt/Core/BinaryFunction.h b/bolt/include/bolt/Core/BinaryFunction.h
index 7560908c250c3a..a889457256f90f 100644
--- a/bolt/include/bolt/Core/BinaryFunction.h
+++ b/bolt/include/bolt/Core/BinaryFunction.h
@@ -1599,6 +1599,52 @@ class BinaryFunction {
void setHasInferredProfile(bool Inferred) { HasInferredProfile = Inferred; }
+ /// Find corrected offset the same way addCFIInstruction does it to skip NOPs.
+ std::optional<uint64_t> getCorrectedCFIOffset(uint64_t Offset) {
+ assert(!Instructions.empty());
+ auto I = Instructions.lower_bound(Offset);
+ if (Offset == getSize()) {
+ assert(I == Instructions.end() && "unexpected iterator value");
+ // Sometimes compiler issues restore_state after all instructions
+ // in the function (even after nop).
+ --I;
+ Offset = I->first;
+ }
+ assert(I->first == Offset && "CFI pointing to unknown instruction");
+ if (I == Instructions.begin()) {
+ return {};
+ }
+
+ --I;
+ while (I != Instructions.begin() && BC.MIB->isNoop(I->second)) {
+ Offset = I->first;
+ --I;
+ }
+ return Offset;
+ }
+
+ void setInstModifiesRAState(uint8_t CFIOpcode, uint64_t Offset) {
+ std::optional<uint64_t> CorrectedOffset = getCorrectedCFIOffset(Offset);
+ if (CorrectedOffset) {
+ auto I = Instructions.lower_bound(*CorrectedOffset);
+ I--;
+
+ switch (CFIOpcode) {
+ case dwarf::DW_CFA_GNU_window_save:
+ BC.MIB->setNegateRAState(I->second);
+ break;
+ case dwarf::DW_CFA_remember_state:
+ BC.MIB->setRememberState(I->second);
+ break;
+ case dwarf::DW_CFA_restore_state:
+ BC.MIB->setRestoreState(I->second);
+ break;
+ default:
+ assert(0 && "CFI Opcode not covered by function");
+ }
+ }
+ }
+
void addCFIInstruction(uint64_t Offset, MCCFIInstruction &&Inst) {
assert(!Instructions.empty());
diff --git a/bolt/include/bolt/Core/MCPlus.h b/bolt/include/bolt/Core/MCPlus.h
index 601d709712864e..78387ad8f5b984 100644
--- a/bolt/include/bolt/Core/MCPlus.h
+++ b/bolt/include/bolt/Core/MCPlus.h
@@ -72,7 +72,15 @@ class MCAnnotation {
kLabel, /// MCSymbol pointing to this instruction.
kSize, /// Size of the instruction.
kDynamicBranch, /// Jit instruction patched at runtime.
- kGeneric /// First generic annotation.
+ kUnkownSign, /// Signed state not determined yet
+ kSigning, /// Inst is a signing instruction (paciasp, etc.)
+ kSigned, /// Inst is in a range where RA is signed
+ kAuthenticating, /// Authenticating inst (e.g. autiasp)
+ kUnsigned, /// Inst is in a range where RA is unsigned
+ kRememberState, /// Inst has rememberState CFI
+ kRestoreState, /// Inst has restoreState CFI
+ kNegateState, /// Inst has OpNegateRAState CFI
+ kGeneric, /// First generic annotation.
};
virtual void print(raw_ostream &OS) const = 0;
diff --git a/bolt/include/bolt/Core/MCPlusBuilder.h b/bolt/include/bolt/Core/MCPlusBuilder.h
index b720a56806bdb7..f9fdfd3f63d3fb 100644
--- a/bolt/include/bolt/Core/MCPlusBuilder.h
+++ b/bolt/include/bolt/Core/MCPlusBuilder.h
@@ -66,6 +66,20 @@ class MCPlusBuilder {
public:
using AllocatorIdTy = uint16_t;
+ std::optional<int64_t> getAnnotationAtOpIndex(const MCInst &Inst,
+ unsigned OpIndex) const {
+ std::optional<unsigned> FirstAnnotationOp = getFirstAnnotationOpIndex(Inst);
+ if (!FirstAnnotationOp)
+ return std::nullopt;
+
+ if (*FirstAnnotationOp > OpIndex || Inst.getNumOperands() < OpIndex)
+ return std::nullopt;
+
+ auto Op = Inst.begin() + OpIndex;
+ const int64_t ImmValue = Op->getImm();
+ return extractAnnotationIndex(ImmValue);
+ }
+
private:
/// A struct that represents a single annotation allocator
struct AnnotationAllocator {
@@ -1129,6 +1143,51 @@ class MCPlusBuilder {
/// Return true if the instruction is a tail call.
bool isTailCall(const MCInst &Inst) const;
+ /// Stores NegateRAState annotation on Inst.
+ void setNegateRAState(MCInst &Inst) const;
+
+ /// Return true if Inst has NegateRAState annotation.
+ bool hasNegateRAState(const MCInst &Inst) const;
+
+ /// Sets RememberState annotation on Inst.
+ void setRememberState(MCInst &Inst) const;
+
+ /// Return true if Inst has RememberState annotation.
+ bool hasRememberState(const MCInst &Inst) const;
+
+ /// Stores RestoreState annotation on Inst.
+ void setRestoreState(MCInst &Inst) const;
+
+ /// Return true if Inst has RestoreState annotation.
+ bool hasRestoreState(const MCInst &Inst) const;
+
+ /// Stores RA Signed annotation on Inst.
+ void setRASigned(MCInst &Inst) const;
+
+ /// Return true if Inst has Signed RA annotation.
+ bool isRASigned(const MCInst &Inst) const;
+
+ /// Stores RA Signing annotation on Inst.
+ void setRASigning(MCInst &Inst) const;
+
+ /// Return true if Inst has Signing RA annotation.
+ bool isRASigning(const MCInst &Inst) const;
+
+ /// Stores Authenticating annotation on Inst.
+ void setAuthenticating(MCInst &Inst) const;
+
+ /// Return true if Inst has Authenticating annotation.
+ bool isAuthenticating(const MCInst &Inst) const;
+
+ /// Stores RA Unsigned annotation on Inst.
+ void setRAUnsigned(MCInst &Inst) const;
+
+ /// Return true if Inst has Unsigned RA annotation.
+ bool isRAUnsigned(const MCInst &Inst) const;
+
+ /// Return true if Inst doesn't have any annotation related to RA state.
+ bool isRAStateUnknown(const MCInst &Inst) const;
+
/// Return true if the instruction is a call with an exception handling info.
virtual bool isInvoke(const MCInst &Inst) const {
return isCall(Inst) && getEHInfo(Inst);
diff --git a/bolt/include/bolt/Passes/InsertNegateRAStatePass.h b/bolt/include/bolt/Passes/InsertNegateRAStatePass.h
new file mode 100644
index 00000000000000..8cf08add9402a3
--- /dev/null
+++ b/bolt/include/bolt/Passes/InsertNegateRAStatePass.h
@@ -0,0 +1,25 @@
+#ifndef BOLT_PASSES_INSERT_NEGATE_RA_STATE_PASS
+#define BOLT_PASSES_INSERT_NEGATE_RA_STATE_PASS
+
+#include "bolt/Passes/BinaryPasses.h"
+#include <stack>
+
+namespace llvm {
+namespace bolt {
+
+class InsertNegateRAState : public BinaryFunctionPass {
+public:
+ explicit InsertNegateRAState() : BinaryFunctionPass(false) {}
+
+ const char *getName() const override { return "insert-negate-ra-state-pass"; }
+
+ /// Pass entry point
+ Error runOnFunctions(BinaryContext &BC) override;
+ void runOnFunction(BinaryFunction &BF);
+ bool addNegateRAStateAfterPacOrAuth(BinaryFunction &BF);
+ void fixUnknownStates(BinaryFunction &BF);
+};
+
+} // namespace bolt
+} // namespace llvm
+#endif
diff --git a/bolt/include/bolt/Passes/MarkRAStates.h b/bolt/include/bolt/Passes/MarkRAStates.h
new file mode 100644
index 00000000000000..3cbd6044683da4
--- /dev/null
+++ b/bolt/include/bolt/Passes/MarkRAStates.h
@@ -0,0 +1,22 @@
+#ifndef BOLT_PASSES_MARK_RA_STATES
+#define BOLT_PASSES_MARK_RA_STATES
+
+#include "bolt/Passes/BinaryPasses.h"
+
+namespace llvm {
+namespace bolt {
+
+class MarkRAStates : public BinaryFunctionPass {
+public:
+ explicit MarkRAStates() : BinaryFunctionPass(false) {}
+
+ const char *getName() const override { return "mark-ra-states"; }
+
+ /// Pass entry point
+ Error runOnFunctions(BinaryContext &BC) override;
+ void runOnFunction(BinaryFunction &BF);
+};
+
+} // namespace bolt
+} // namespace llvm
+#endif
diff --git a/bolt/lib/Core/BinaryBasicBlock.cpp b/bolt/lib/Core/BinaryBasicBlock.cpp
index 2a2192b79bb4bf..58a5f8e3ae4c08 100644
--- a/bolt/lib/Core/BinaryBasicBlock.cpp
+++ b/bolt/lib/Core/BinaryBasicBlock.cpp
@@ -201,7 +201,11 @@ int32_t BinaryBasicBlock::getCFIStateAtInstr(const MCInst *Instr) const {
InstrSeen = (&Inst == Instr);
continue;
}
- if (Function->getBinaryContext().MIB->isCFI(Inst)) {
+ // Ignoring OpNegateRAState CFIs here, as they dont have a "State"
+ // number associated with them.
+ if (Function->getBinaryContext().MIB->isCFI(Inst) &&
+ (Function->getCFIFor(Inst)->getOperation() !=
+ MCCFIInstruction::OpNegateRAState)) {
LastCFI = &Inst;
break;
}
diff --git a/bolt/lib/Core/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp
index a9ccaea3c43850..9c4e161f66ad45 100644
--- a/bolt/lib/Core/BinaryFunction.cpp
+++ b/bolt/lib/Core/BinaryFunction.cpp
@@ -2587,6 +2587,7 @@ struct CFISnapshot {
void advanceTo(int32_t State) {
for (int32_t I = CurState, E = State; I != E; ++I) {
const MCCFIInstruction &Instr = FDE[I];
+ assert(Instr.getOperation() != MCCFIInstruction::OpNegateRAState);
if (Instr.getOperation() != MCCFIInstruction::OpRestoreState) {
update(Instr, I);
continue;
diff --git a/bolt/lib/Core/Exceptions.cpp b/bolt/lib/Core/Exceptions.cpp
index 0b2e63b8ca6a79..d38b37bcb3c612 100644
--- a/bolt/lib/Core/Exceptions.cpp
+++ b/bolt/lib/Core/Exceptions.cpp
@@ -568,10 +568,21 @@ bool CFIReaderWriter::fillCFIInfoFor(BinaryFunction &Function) const {
case DW_CFA_remember_state:
Function.addCFIInstruction(
Offset, MCCFIInstruction::createRememberState(nullptr));
+
+ if (Function.getBinaryContext().isAArch64())
+ // Support for pointer authentication:
+ // We need to annotate instructions that modify the RA State, to work
+ // out the state of each instruction in MarkRAStates Pass.
+ Function.setInstModifiesRAState(DW_CFA_remember_state, Offset);
break;
case DW_CFA_restore_state:
Function.addCFIInstruction(Offset,
MCCFIInstruction::createRestoreState(nullptr));
+ if (Function.getBinaryContext().isAArch64())
+ // Support for pointer authentication:
+ // We need to annotate instructions that modify the RA State, to work
+ // out the state of each instruction in MarkRAStates Pass.
+ Function.setInstModifiesRAState(DW_CFA_restore_state, Offset);
break;
case DW_CFA_def_cfa:
Function.addCFIInstruction(
@@ -632,8 +643,13 @@ bool CFIReaderWriter::fillCFIInfoFor(BinaryFunction &Function) const {
// DW_CFA_GNU_window_save and DW_CFA_GNU_NegateRAState just use the same
// id but mean different things. The latter is used in AArch64.
if (Function.getBinaryContext().isAArch64()) {
- Function.addCFIInstruction(
- Offset, MCCFIInstruction::createNegateRAState(nullptr));
+ // Not adding OpNegateRAState since the location they are needed
+ // depends on the order of BasicBlocks, which changes during
+ // optimizations. Instead, an annotation is added to the instruction, to
+ // mark that the instruction modifies the RA State. The actual state for
+ // instructions are worked out in MarkRAStates based on these
+ // annotations.
+ Function.setInstModifiesRAState(DW_CFA_GNU_window_save, Offset);
break;
}
if (opts::Verbosity >= 1)
diff --git a/bolt/lib/Core/MCPlusBuilder.cpp b/bolt/lib/Core/MCPlusBuilder.cpp
index 7ff7a2288451c8..7d172b1ade5e8c 100644
--- a/bolt/lib/Core/MCPlusBuilder.cpp
+++ b/bolt/lib/Core/MCPlusBuilder.cpp
@@ -147,6 +147,92 @@ bool MCPlusBuilder::isTailCall(const MCInst &Inst) const {
return false;
}
+void MCPlusBuilder::setNegateRAState(MCInst &Inst) const {
+ assert(!hasAnnotation(Inst, MCAnnotation::kNegateState));
+ setAnnotationOpValue(Inst, MCAnnotation::kNegateState, true);
+}
+
+bool MCPlusBuilder::hasNegateRAState(const MCInst &Inst) const {
+ if (hasAnnotation(Inst, MCAnnotation::kNegateState))
+ return true;
+ return false;
+}
+
+void MCPlusBuilder::setRememberState(MCInst &Inst) const {
+ assert(!hasAnnotation(Inst, MCAnnotation::kRememberState));
+ setAnnotationOpValue(Inst, MCAnnotation::kRememberState, true);
+}
+
+bool MCPlusBuilder::hasRememberState(const MCInst &Inst) const {
+ if (hasAnnotation(Inst, MCAnnotation::kRememberState))
+ return true;
+ return false;
+}
+
+void MCPlusBuilder::setRestoreState(MCInst &Inst) const {
+ assert(!hasAnnotation(Inst, MCAnnotation::kRestoreState));
+ setAnnotationOpValue(Inst, MCAnnotation::kRestoreState, true);
+}
+
+bool MCPlusBuilder::hasRestoreState(const MCInst &Inst) const {
+ if (hasAnnotation(Inst, MCAnnotation::kRestoreState))
+ return true;
+ return false;
+}
+
+void MCPlusBuilder::setRASigned(MCInst &Inst) const {
+ assert(!hasAnnotation(Inst, MCAnnotation::kSigned));
+ setAnnotationOpValue(Inst, MCAnnotation::kSigned, true);
+}
+
+bool MCPlusBuilder::isRASigned(const MCInst &Inst) const {
+ if (hasAnnotation(Inst, MCAnnotation::kSigned))
+ return true;
+ return false;
+}
+
+void MCPlusBuilder::setRASigning(MCInst &Inst) const {
+ assert(!hasAnnotation(Inst, MCAnnotation::kSigning));
+ setAnnotationOpValue(Inst, MCAnnotation::kSigning, true);
+}
+
+bool MCPlusBuilder::isRASigning(const MCInst &Inst) const {
+ if (hasAnnotation(Inst, MCAnnotation::kSigning))
+ return true;
+ return false;
+}
+
+void MCPlusBuilder::setAuthenticating(MCInst &Inst) const {
+ assert(!hasAnnotation(Inst, MCAnnotation::kAuthenticating));
+ setAnnotationOpValue(Inst, MCAnnotation::kAuthenticating, true);
+}
+
+bool MCPlusBuilder::isAuthenticating(const MCInst &Inst) const {
+ if (hasAnnotation(Inst, MCAnnotation::kAuthenticating))
+ return true;
+ return false;
+}
+
+void MCPlusBuilder::setRAUnsigned(MCInst &Inst) const {
+ assert(!hasAnnotation(Inst, MCAnnotation::kUnsigned));
+ setAnnotationOpValue(Inst, MCAnnotation::kUnsigned, true);
+}
+
+bool MCPlusBuilder::isRAUnsigned(const MCInst &Inst) const {
+ if (hasAnnotation(Inst, MCAnnotation::kUnsigned))
+ return true;
+ return false;
+}
+
+bool MCPlusBuilder::isRAStateUnknown(const MCInst &Inst) const {
+ if (hasAnnotation(Inst, MCAnnotation::kUnsigned) ||
+ hasAnnotation(Inst, MCAnnotation::kSigned) ||
+ hasAnnotation(Inst, MCAnnotation::kSigning) ||
+ hasAnnotation(Inst, MCAnnotation::kAuthenticating))
+ return false;
+ return true;
+}
+
std::optional<MCLandingPad> MCPlusBuilder::getEHInfo(const MCInst &Inst) const {
if (!isCall(Inst))
return std::nullopt;
diff --git a/bolt/lib/Passes/CMakeLists.txt b/bolt/lib/Passes/CMakeLists.txt
index 1c1273b3d2420d..330c9136515bad 100644
--- a/bolt/lib/Passes/CMakeLists.txt
+++ b/bolt/lib/Passes/CMakeLists.txt
@@ -17,12 +17,14 @@ add_llvm_library(LLVMBOLTPasses
IdenticalCodeFolding.cpp
IndirectCallPromotion.cpp
Inliner.cpp
+ InsertNegateRAStatePass.cpp
Instrumentation.cpp
JTFootprintReduction.cpp
LongJmp.cpp
LoopInversionPass.cpp
LivenessAnalysis.cpp
MCF.cpp
+ MarkRAStates.cpp
PatchEntries.cpp
PettisAndHansen.cpp
PLTCall.cpp
diff --git a/bolt/lib/Passes/InsertNegateRAStatePass.cpp b/bolt/lib/Passes/InsertNegateRAStatePass.cpp
new file mode 100644
index 00000000000000..b0945e4065f039
--- /dev/null
+++ b/bolt/lib/Passes/InsertNegateRAStatePass.cpp
@@ -0,0 +1,153 @@
+//===- bolt/Passes/InsertNegateRAStatePass.cpp ----------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file implements the InsertNegateRAStatePass class. It inserts
+// OpNegateRAState CFIs to places where the state of two consecutive
+// instructions are different.
+//
+//===----------------------------------------------------------------------===//
+#include "bolt/Passes/InsertNegateRAStatePass.h"
+#include "bolt/Core/BinaryFunction.h"
+#include "bolt/Core/ParallelUtilities.h"
+#include "bolt/Utils/CommandLineOpts.h"
+#include <cstdlib>
+#include <fstream>
+#include <iterator>
+
+using namespace llvm;
+
+namespace llvm {
+namespace bolt {
+
+void InsertNegateRAState::runOnFunction(BinaryFunction &BF) {
+ BinaryContext &BC = BF.getBinaryContext();
+
+ if (BF.getState() == BinaryFunction::State::Empty) {
+ return;
+ }
+
+ if (BF.getState() != BinaryFunction::State::CFG &&
+ BF.getState() != BinaryFunction::State::CFG_Finalized) {
+ BC.outs() << "BOLT-INFO: No CFG for " << BF.getPrintName()
+ << " in InsertNegateRAStatePass\n";
+ return;
+ }
+
+ // If none is inserted, the function doesn't need more work.
+ if (!addNegateRAStateAfterPacOrAuth(BF)) {
+ BC.outs() << "BOLT-INFO: no pacret found.\n";
+ return;
+ }
+
+ fixUnknownStates(BF);
+
+ bool FirstIter = true;
+ MCInst PrevInst;
+ BinaryBasicBlock *PrevBB = nullptr;
+ auto *Begin = BF.getLayout().block_begin();
+ auto *End = BF.getLayout().block_end();
+ for (auto *BB = Begin; BB != End; BB++) {
+
+ // Support for function splitting:
+ // if two consecutive BBs are going to end up in different functions,
+ // we have to negate the RA State, so the new function starts with a Signed
+ // state.
+ if (PrevBB != nullptr &&
+ PrevBB->getFragmentNum() != (*BB)->getFragmentNum() &&
+ BC.MIB->isRASigned(*((*BB)->begin()))) {
+ BF.addCFIInstruction(*BB, (*BB)->begin(),
+ MCCFIInstruction::createNegateRAState(nullptr));
+ }
+
+ for (auto It = (*BB)->begin(); It != (*BB)->end(); ++It) {
+
+ MCInst &Inst = *It;
+ if (BC.MIB->isCFI(Inst))
+ continue;
+
+ if (!FirstIter) {
+ if ((BC.MIB->isRASigned(PrevInst) && BC.MIB->isRAUnsigned(Inst)) ||
+ (BC.MIB->isRAUnsigned(PrevInst) && BC.MIB->isRASigned(Inst))) {
+
+ It = BF.addCFIInstruction(
+ *BB, It, MCCFIInstruction::createNegateRAState(nullptr));
+ }
+
+ } else {
+ FirstIter = false;
+ }
+ PrevInst = Inst;
+ }
+ PrevBB = *BB;
+ }
+}
+
+bool InsertNegateRAState::addNegateRAStateAfterPacOrAuth(BinaryFunction &BF) {
+ BinaryContext &BC = BF.getBinaryContext();
+ bool FoundAny = false;
+ for (BinaryBasicBlock &BB : BF) {
+ for (auto Iter = BB.begin(); Iter != BB.end(); ++Iter) {
+ MCInst &Inst = *Iter;
+ if (BC.MIB->isPSign(Inst)) {
+ Iter = BF.addCFIInstruction(
+ &BB, Iter + 1, MCCFIInstruction::createNegateRAState(nullptr));
+ FoundAny = true;
+ }
+
+ if (BC.MIB->isPAuth(Inst)) {
+ Iter = BF.addCFIInstruction(
+ &BB, Iter + 1, MCCFIInstruction::createNegateRAState(nullptr));
+ FoundAny = true;
+ }
+ }
+ }
+ return FoundAny;
+}
+
+void InsertNegateRAState::fixUnknownStates(BinaryFunction &BF) {
+ BinaryContext &BC = BF.getBinaryContext();
+ bool FirstIter = true;
+ MCInst PrevInst;
+ for (auto BBIt = BF.begin(); BBIt != BF.end(); ++BBIt) {
+ BinaryBasicBlock &BB = *BBIt;
+
+ for (auto It = BB.begin(); It != BB.end(); ++It) {
+
+ MCInst &Inst = *It;
+ if (BC.MIB->isCFI(Inst))
+ continue;
+
+ if (!FirstIter && BC.MIB->isRAStateUnknown(Inst)) {
+ if (BC.MIB->isRASigned(PrevInst) || BC.MIB->isRASigning(PrevInst)) {
+ BC.MIB->setRASigned(Inst);
+ } else if (BC.MIB->isRAUnsigned(PrevInst) ||
+ BC.MIB->isAuthenticating(PrevInst)) {
+ BC.MIB->setRAUnsigned(Inst);
+ }
+ } else {
+ FirstIter = false;
+ }
+ PrevInst = Inst;
+ }
+ }
+}
+
+Error InsertNegateRAState::runOnFunctions(BinaryContext &BC) {
+ ParallelUtilities::WorkFuncTy WorkFun = [&](BinaryFunction &BF) {
+ runOnFunction(BF);
+ };
+
+ ParallelUtilities::runOnEachFunction(
+ BC, ParallelUtilities::SchedulingPolicy::SP_TRIVIAL, WorkFun, nullptr,
+ "InsertNegateRAStatePass");
+
+ return Error::success();
+}
+
+} // end namespace bolt
+} // end namespace llvm
diff --git a/bolt/lib/Passes/MarkRAStates.cpp b/bolt/lib/Passes/MarkRAStates.cpp
new file mode 100644
index 00000000000000..03affab2f21546
--- /dev/null
+++ b/bolt/lib/Passes/MarkRAStates.cpp
@@ -0,0 +1,122 @@
+//===- bolt/Passes/MarkRAStates.cpp ---------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file implements the MarkRAStates class.
+// Three CFIs have an influence on the RA State of an instruction:
+// - NegateRAState flips the RA State,
+// - RememberState pushes the RA State to a stack,
+// - RestoreState pops the RA State from the stack.
+// These were saved as MCAnnotations on instructions they refer to at CFI
+// reading (in CFIReaderWriter::fillCFIInfoFor). In this pass, we can work out
+// the RA State of each instruction, and save it as new MCAnnotations. The new
+// annotations are Signing, Signed, Authenticating and Unsigned. After
+// optimizations, .cfi_negate_ra_state CFIs are added to the places where the
+// state changes in InsertNegateRAStatePass.
+//
+//===----------------------------------------------------------------------===//
+#include "bolt/Passes/MarkRAStates.h"
+#include "bolt/Core/BinaryFunction.h"
+#include "bolt/Core/ParallelUtilities.h"
+#include "bolt/Utils/CommandLineOpts.h"
+#include <cstdlib>
+#include <fstream>
+#include <iterator>
+
+#include <iostream>
+#include <optional>
+#include <stack>
+
+using namespace llvm;
+
+namespace llvm {
+namespace bolt {
+
+void MarkRAStates::runOnFunction(BinaryFunction &BF) {
+
+ if (BF.isIgnored())
+ return;
+
+ BinaryContext &BC = BF.getBinaryContext();
+
+ for (BinaryBasicBlock &BB : BF) {
+ for (auto It = BB.begin(); It != BB.end(); ++It) {
+ MCInst &Inst = *It;
+ if ((BC.MIB->isPSign(Inst) || BC.MIB->isPAuth(Inst)) &&
+ !BC.MIB->hasNegateRAState(Inst)) {
+ // no .cfi_negate_ra_state attached to signing or authenticating instr
+ // means, that this is a function with handwritten assembly, which might
+ // not respect Clang's conventions (e.g. tailcalls are always
+ // authenticated, so functions always start with unsigned RAState when
+ // working with compiler-generated code)
+ BF.setIgnored();
+ BC.outs() << "BOLT-INFO: ignoring RAStates in function "
+ << BF.getPrintName() << "\n";
+ return;
+ }
+ }
+ }
+
+ bool RAState = false;
+ std::stack<bool> RAStateStack;
+
+ for (BinaryBasicBlock &BB : BF) {
+ for (auto It = BB.begin(); It != BB.end(); ++It) {
+
+ MCInst &Inst = *It;
+ if (BC.MIB->isCFI(Inst))
+ continue;
+
+ if (BC.MIB->isPSign(Inst)) {
+ assert(!RAState && "Signed RA State before PSign");
+ BC.MIB->setRASigning(Inst);
+
+ } else if (BC.MIB->isPAuth(Inst)) {
+ assert(RAState && "Unsigned RA State before PAuth");
+ BC.MIB->setAuthenticating(Inst);
+ } else if (RAState) {
+ BC.MIB->setRASigned(Inst);
+ } else {
+ BC.MIB->setRAUnsigned(Inst);
+ }
+
+ // 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
+ // of just checking each in a predefined order.
+ for (unsigned int Idx = 0; Idx < Inst.getNumOperands(); Idx++) {
+ std::optional<int64_t> Annotation =
+ BC.MIB->getAnnotationAtOpIndex(Inst, Idx);
+ if (!Annotation)
+ continue;
+ if (Annotation == MCPlus::MCAnnotation::kNegateState)
+ RAState = !RAState;
+ if (Annotation == MCPlus::MCAnnotation::kRememberState)
+ RAStateStack.push(RAState);
+ if (Annotation == MCPlus::MCAnnotation::kRestoreState) {
+ RAState = RAStateStack.top();
+ RAStateStack.pop();
+ }
+ }
+ }
+ }
+}
+
+Error MarkRAStates::runOnFunctions(BinaryContext &BC) {
+ ParallelUtilities::WorkFuncTy WorkFun = [&](BinaryFunction &BF) {
+ runOnFunction(BF);
+ };
+
+ ParallelUtilities::runOnEachFunction(
+ BC, ParallelUtilities::SchedulingPolicy::SP_TRIVIAL, WorkFun, nullptr,
+ "MarkRAStates");
+
+ return Error::success();
+}
+
+} // end namespace bolt
+} // end namespace llvm
diff --git a/bolt/lib/Rewrite/BinaryPassManager.cpp b/bolt/lib/Rewrite/BinaryPassManager.cpp
index b0906041833484..ac30e27e668c9b 100644
--- a/bolt/lib/Rewrite/BinaryPassManager.cpp
+++ b/bolt/lib/Rewrite/BinaryPassManager.cpp
@@ -20,11 +20,13 @@
#include "bolt/Passes/IdenticalCodeFolding.h"
#include "bolt/Passes/IndirectCallPromotion.h"
#include "bolt/Passes/Inliner.h"
+#include "bolt/Passes/InsertNegateRAStatePass.h"
#include "bolt/Passes/Instrumentation.h"
#include "bolt/Passes/JTFootprintReduction.h"
#include "bolt/Passes/LongJmp.h"
#include "bolt/Passes/LoopInversionPass.h"
#include "bolt/Passes/MCF.h"
+#include "bolt/Passes/MarkRAStates.h"
#include "bolt/Passes/PLTCall.h"
#include "bolt/Passes/PatchEntries.h"
#include "bolt/Passes/RegReAssign.h"
@@ -345,6 +347,8 @@ Error BinaryFunctionPassManager::runPasses() {
Error BinaryFunctionPassManager::runAllPasses(BinaryContext &BC) {
BinaryFunctionPassManager Manager(BC);
+ Manager.registerPass(std::make_unique<MarkRAStates>());
+
Manager.registerPass(
std::make_unique<EstimateEdgeCounts>(PrintEstimateEdgeCounts));
@@ -499,6 +503,8 @@ Error BinaryFunctionPassManager::runAllPasses(BinaryContext &BC) {
// targets. No extra instructions after this pass, otherwise we may have
// relocations out of range and crash during linking.
Manager.registerPass(std::make_unique<LongJmpPass>(PrintLongJmp));
+
+ Manager.registerPass(std::make_unique<InsertNegateRAState>());
}
// This pass should always run last.*
>From 1b9d41cc457e1f6ee298a1b7589ad3c0f5b101dc Mon Sep 17 00:00:00 2001
From: Gergely Balint <gergely.balint at arm.com>
Date: Mon, 16 Dec 2024 12:19:57 +0100
Subject: [PATCH 3/7] [BOLT] only run MarkRAState pass on AArch64
---
bolt/lib/Rewrite/BinaryPassManager.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/bolt/lib/Rewrite/BinaryPassManager.cpp b/bolt/lib/Rewrite/BinaryPassManager.cpp
index ac30e27e668c9b..49ccdd3eebf6a3 100644
--- a/bolt/lib/Rewrite/BinaryPassManager.cpp
+++ b/bolt/lib/Rewrite/BinaryPassManager.cpp
@@ -347,7 +347,8 @@ Error BinaryFunctionPassManager::runPasses() {
Error BinaryFunctionPassManager::runAllPasses(BinaryContext &BC) {
BinaryFunctionPassManager Manager(BC);
- Manager.registerPass(std::make_unique<MarkRAStates>());
+ if (BC.isAArch64())
+ Manager.registerPass(std::make_unique<MarkRAStates>());
Manager.registerPass(
std::make_unique<EstimateEdgeCounts>(PrintEstimateEdgeCounts));
>From 31384046ab361e4bd0867b46c9f24a5efe732731 Mon Sep 17 00:00:00 2001
From: Gergely Balint <gergely.balint at arm.com>
Date: Mon, 16 Dec 2024 13:08:34 +0100
Subject: [PATCH 4/7] [BOLT] InsertNegateRAStatePass: remove print
---
bolt/lib/Passes/InsertNegateRAStatePass.cpp | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/bolt/lib/Passes/InsertNegateRAStatePass.cpp b/bolt/lib/Passes/InsertNegateRAStatePass.cpp
index b0945e4065f039..ecdd33fa36da62 100644
--- a/bolt/lib/Passes/InsertNegateRAStatePass.cpp
+++ b/bolt/lib/Passes/InsertNegateRAStatePass.cpp
@@ -39,10 +39,8 @@ void InsertNegateRAState::runOnFunction(BinaryFunction &BF) {
}
// If none is inserted, the function doesn't need more work.
- if (!addNegateRAStateAfterPacOrAuth(BF)) {
- BC.outs() << "BOLT-INFO: no pacret found.\n";
+ if (!addNegateRAStateAfterPacOrAuth(BF))
return;
- }
fixUnknownStates(BF);
>From dac8f161e868f57e34c5a12bc81c939a64322d15 Mon Sep 17 00:00:00 2001
From: Gergely Balint <gergely.balint at arm.com>
Date: Fri, 10 Jan 2025 13:44:45 +0100
Subject: [PATCH 5/7] [BOLT] Remove unused MCAnnotation
---
bolt/include/bolt/Core/MCPlus.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/bolt/include/bolt/Core/MCPlus.h b/bolt/include/bolt/Core/MCPlus.h
index 78387ad8f5b984..6ff3c580da0d44 100644
--- a/bolt/include/bolt/Core/MCPlus.h
+++ b/bolt/include/bolt/Core/MCPlus.h
@@ -72,7 +72,6 @@ class MCAnnotation {
kLabel, /// MCSymbol pointing to this instruction.
kSize, /// Size of the instruction.
kDynamicBranch, /// Jit instruction patched at runtime.
- kUnkownSign, /// Signed state not determined yet
kSigning, /// Inst is a signing instruction (paciasp, etc.)
kSigned, /// Inst is in a range where RA is signed
kAuthenticating, /// Authenticating inst (e.g. autiasp)
>From fee74748ef374b77d22710e45d1ef09c432f03c8 Mon Sep 17 00:00:00 2001
From: Gergely Balint <gergely.balint at arm.com>
Date: Wed, 15 Jan 2025 12:50:38 +0100
Subject: [PATCH 6/7] [BOLT] Add changes from code review (#120064)
---
bolt/include/bolt/Core/BinaryFunction.h | 5 ++-
bolt/include/bolt/Core/MCPlusBuilder.h | 30 ++++++++--------
.../bolt/Passes/InsertNegateRAStatePass.h | 19 +++++++++++
bolt/include/bolt/Passes/MarkRAStates.h | 11 ++++++
bolt/lib/Core/Exceptions.cpp | 16 ++++-----
bolt/lib/Core/MCPlusBuilder.cpp | 34 +++++--------------
bolt/lib/Passes/InsertNegateRAStatePass.cpp | 15 ++------
bolt/lib/Passes/MarkRAStates.cpp | 6 ++--
8 files changed, 70 insertions(+), 66 deletions(-)
diff --git a/bolt/include/bolt/Core/BinaryFunction.h b/bolt/include/bolt/Core/BinaryFunction.h
index a889457256f90f..d14dfa2d4b79b8 100644
--- a/bolt/include/bolt/Core/BinaryFunction.h
+++ b/bolt/include/bolt/Core/BinaryFunction.h
@@ -1611,9 +1611,8 @@ class BinaryFunction {
Offset = I->first;
}
assert(I->first == Offset && "CFI pointing to unknown instruction");
- if (I == Instructions.begin()) {
+ if (I == Instructions.begin())
return {};
- }
--I;
while (I != Instructions.begin() && BC.MIB->isNoop(I->second)) {
@@ -1630,7 +1629,7 @@ class BinaryFunction {
I--;
switch (CFIOpcode) {
- case dwarf::DW_CFA_GNU_window_save:
+ case dwarf::DW_CFA_AARCH64_negate_ra_state:
BC.MIB->setNegateRAState(I->second);
break;
case dwarf::DW_CFA_remember_state:
diff --git a/bolt/include/bolt/Core/MCPlusBuilder.h b/bolt/include/bolt/Core/MCPlusBuilder.h
index f9fdfd3f63d3fb..85da98b5eed9ab 100644
--- a/bolt/include/bolt/Core/MCPlusBuilder.h
+++ b/bolt/include/bolt/Core/MCPlusBuilder.h
@@ -1143,49 +1143,49 @@ class MCPlusBuilder {
/// Return true if the instruction is a tail call.
bool isTailCall(const MCInst &Inst) const;
- /// Stores NegateRAState annotation on Inst.
+ /// Stores NegateRAState annotation on \p Inst.
void setNegateRAState(MCInst &Inst) const;
- /// Return true if Inst has NegateRAState annotation.
+ /// Return true if \p Inst has NegateRAState annotation.
bool hasNegateRAState(const MCInst &Inst) const;
- /// Sets RememberState annotation on Inst.
+ /// Sets RememberState annotation on \p Inst.
void setRememberState(MCInst &Inst) const;
- /// Return true if Inst has RememberState annotation.
+ /// Return true if \p Inst has RememberState annotation.
bool hasRememberState(const MCInst &Inst) const;
- /// Stores RestoreState annotation on Inst.
+ /// Stores RestoreState annotation on \p Inst.
void setRestoreState(MCInst &Inst) const;
- /// Return true if Inst has RestoreState annotation.
+ /// Return true if \p Inst has RestoreState annotation.
bool hasRestoreState(const MCInst &Inst) const;
- /// Stores RA Signed annotation on Inst.
+ /// Stores RA Signed annotation on \p Inst.
void setRASigned(MCInst &Inst) const;
- /// Return true if Inst has Signed RA annotation.
+ /// Return true if \p Inst has Signed RA annotation.
bool isRASigned(const MCInst &Inst) const;
- /// Stores RA Signing annotation on Inst.
+ /// Stores RA Signing annotation on \p Inst.
void setRASigning(MCInst &Inst) const;
- /// Return true if Inst has Signing RA annotation.
+ /// Return true if \p Inst has Signing RA annotation.
bool isRASigning(const MCInst &Inst) const;
- /// Stores Authenticating annotation on Inst.
+ /// Stores Authenticating annotation on \p Inst.
void setAuthenticating(MCInst &Inst) const;
- /// Return true if Inst has Authenticating annotation.
+ /// Return true if \p Inst has Authenticating annotation.
bool isAuthenticating(const MCInst &Inst) const;
- /// Stores RA Unsigned annotation on Inst.
+ /// Stores RA Unsigned annotation on \p Inst.
void setRAUnsigned(MCInst &Inst) const;
- /// Return true if Inst has Unsigned RA annotation.
+ /// Return true if \p Inst has Unsigned RA annotation.
bool isRAUnsigned(const MCInst &Inst) const;
- /// Return true if Inst doesn't have any annotation related to RA state.
+ /// Return true if \p Inst doesn't have any annotation related to RA state.
bool isRAStateUnknown(const MCInst &Inst) const;
/// Return true if the instruction is a call with an exception handling info.
diff --git a/bolt/include/bolt/Passes/InsertNegateRAStatePass.h b/bolt/include/bolt/Passes/InsertNegateRAStatePass.h
index 8cf08add9402a3..e62006baa2efff 100644
--- a/bolt/include/bolt/Passes/InsertNegateRAStatePass.h
+++ b/bolt/include/bolt/Passes/InsertNegateRAStatePass.h
@@ -1,3 +1,14 @@
+//===- bolt/Passes/InsertNegateRAStatePass.cpp ----------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file implements the InsertNegateRAStatePass class.
+//
+//===----------------------------------------------------------------------===//
#ifndef BOLT_PASSES_INSERT_NEGATE_RA_STATE_PASS
#define BOLT_PASSES_INSERT_NEGATE_RA_STATE_PASS
@@ -16,7 +27,15 @@ class InsertNegateRAState : public BinaryFunctionPass {
/// Pass entry point
Error runOnFunctions(BinaryContext &BC) override;
void runOnFunction(BinaryFunction &BF);
+
+private:
+ /// Loops over all instructions and adds OpNegateRAState CFI
+ /// after any pointer signing or authenticating instructions.
+ /// Returns true, if any OpNegateRAState CFIs were added.
bool addNegateRAStateAfterPacOrAuth(BinaryFunction &BF);
+ /// Because states are tracked as MCAnnotations on individual instructions,
+ /// newly inserted instructions do not have a state associated with them.
+ /// New states are "inherited" from the last known state.
void fixUnknownStates(BinaryFunction &BF);
};
diff --git a/bolt/include/bolt/Passes/MarkRAStates.h b/bolt/include/bolt/Passes/MarkRAStates.h
index 3cbd6044683da4..e7a49f813b6a7c 100644
--- a/bolt/include/bolt/Passes/MarkRAStates.h
+++ b/bolt/include/bolt/Passes/MarkRAStates.h
@@ -1,3 +1,14 @@
+//===- bolt/Passes/MarkRAStates.cpp ---------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file implements the MarkRAStates class.
+//
+//===----------------------------------------------------------------------===//
#ifndef BOLT_PASSES_MARK_RA_STATES
#define BOLT_PASSES_MARK_RA_STATES
diff --git a/bolt/lib/Core/Exceptions.cpp b/bolt/lib/Core/Exceptions.cpp
index d38b37bcb3c612..63b7ad43b1deca 100644
--- a/bolt/lib/Core/Exceptions.cpp
+++ b/bolt/lib/Core/Exceptions.cpp
@@ -640,16 +640,16 @@ bool CFIReaderWriter::fillCFIInfoFor(BinaryFunction &Function) const {
BC.errs() << "BOLT-WARNING: DW_CFA_MIPS_advance_loc unimplemented\n";
return false;
case DW_CFA_GNU_window_save:
- // DW_CFA_GNU_window_save and DW_CFA_GNU_NegateRAState just use the same
- // id but mean different things. The latter is used in AArch64.
+ // DW_CFA_GNU_window_save and DW_CFA_AARCH64_negate_ra_state just use the
+ // same id but mean different things. The latter is used in AArch64.
if (Function.getBinaryContext().isAArch64()) {
- // Not adding OpNegateRAState since the location they are needed
+ // The location OpNegateRAState CFIs are needed
// depends on the order of BasicBlocks, which changes during
- // optimizations. Instead, an annotation is added to the instruction, to
- // mark that the instruction modifies the RA State. The actual state for
- // instructions are worked out in MarkRAStates based on these
- // annotations.
- Function.setInstModifiesRAState(DW_CFA_GNU_window_save, Offset);
+ // optimizations. Instead of adding OpNegateRAState CFIs, an annotation
+ // is added to the instruction, to mark that the instruction modifies
+ // the RA State. The actual state for instructions are worked out in
+ // MarkRAStates based on these annotations.
+ Function.setInstModifiesRAState(DW_CFA_AARCH64_negate_ra_state, Offset);
break;
}
if (opts::Verbosity >= 1)
diff --git a/bolt/lib/Core/MCPlusBuilder.cpp b/bolt/lib/Core/MCPlusBuilder.cpp
index 7d172b1ade5e8c..0a909dca35c681 100644
--- a/bolt/lib/Core/MCPlusBuilder.cpp
+++ b/bolt/lib/Core/MCPlusBuilder.cpp
@@ -153,9 +153,7 @@ void MCPlusBuilder::setNegateRAState(MCInst &Inst) const {
}
bool MCPlusBuilder::hasNegateRAState(const MCInst &Inst) const {
- if (hasAnnotation(Inst, MCAnnotation::kNegateState))
- return true;
- return false;
+ return hasAnnotation(Inst, MCAnnotation::kNegateState);
}
void MCPlusBuilder::setRememberState(MCInst &Inst) const {
@@ -164,9 +162,7 @@ void MCPlusBuilder::setRememberState(MCInst &Inst) const {
}
bool MCPlusBuilder::hasRememberState(const MCInst &Inst) const {
- if (hasAnnotation(Inst, MCAnnotation::kRememberState))
- return true;
- return false;
+ return hasAnnotation(Inst, MCAnnotation::kRememberState);
}
void MCPlusBuilder::setRestoreState(MCInst &Inst) const {
@@ -175,9 +171,7 @@ void MCPlusBuilder::setRestoreState(MCInst &Inst) const {
}
bool MCPlusBuilder::hasRestoreState(const MCInst &Inst) const {
- if (hasAnnotation(Inst, MCAnnotation::kRestoreState))
- return true;
- return false;
+ return hasAnnotation(Inst, MCAnnotation::kRestoreState);
}
void MCPlusBuilder::setRASigned(MCInst &Inst) const {
@@ -186,9 +180,7 @@ void MCPlusBuilder::setRASigned(MCInst &Inst) const {
}
bool MCPlusBuilder::isRASigned(const MCInst &Inst) const {
- if (hasAnnotation(Inst, MCAnnotation::kSigned))
- return true;
- return false;
+ return hasAnnotation(Inst, MCAnnotation::kSigned);
}
void MCPlusBuilder::setRASigning(MCInst &Inst) const {
@@ -197,9 +189,7 @@ void MCPlusBuilder::setRASigning(MCInst &Inst) const {
}
bool MCPlusBuilder::isRASigning(const MCInst &Inst) const {
- if (hasAnnotation(Inst, MCAnnotation::kSigning))
- return true;
- return false;
+ return hasAnnotation(Inst, MCAnnotation::kSigning);
}
void MCPlusBuilder::setAuthenticating(MCInst &Inst) const {
@@ -208,9 +198,7 @@ void MCPlusBuilder::setAuthenticating(MCInst &Inst) const {
}
bool MCPlusBuilder::isAuthenticating(const MCInst &Inst) const {
- if (hasAnnotation(Inst, MCAnnotation::kAuthenticating))
- return true;
- return false;
+ return hasAnnotation(Inst, MCAnnotation::kAuthenticating);
}
void MCPlusBuilder::setRAUnsigned(MCInst &Inst) const {
@@ -219,16 +207,12 @@ void MCPlusBuilder::setRAUnsigned(MCInst &Inst) const {
}
bool MCPlusBuilder::isRAUnsigned(const MCInst &Inst) const {
- if (hasAnnotation(Inst, MCAnnotation::kUnsigned))
- return true;
- return false;
+ return hasAnnotation(Inst, MCAnnotation::kUnsigned);
}
bool MCPlusBuilder::isRAStateUnknown(const MCInst &Inst) const {
- if (hasAnnotation(Inst, MCAnnotation::kUnsigned) ||
- hasAnnotation(Inst, MCAnnotation::kSigned) ||
- hasAnnotation(Inst, MCAnnotation::kSigning) ||
- hasAnnotation(Inst, MCAnnotation::kAuthenticating))
+ if (isRAUnsigned(Inst) || isRASigned(Inst) || isRASigning(Inst) ||
+ isAuthenticating(Inst))
return false;
return true;
}
diff --git a/bolt/lib/Passes/InsertNegateRAStatePass.cpp b/bolt/lib/Passes/InsertNegateRAStatePass.cpp
index ecdd33fa36da62..62e363a8bcbde0 100644
--- a/bolt/lib/Passes/InsertNegateRAStatePass.cpp
+++ b/bolt/lib/Passes/InsertNegateRAStatePass.cpp
@@ -27,9 +27,8 @@ namespace bolt {
void InsertNegateRAState::runOnFunction(BinaryFunction &BF) {
BinaryContext &BC = BF.getBinaryContext();
- if (BF.getState() == BinaryFunction::State::Empty) {
+ if (BF.getState() == BinaryFunction::State::Empty)
return;
- }
if (BF.getState() != BinaryFunction::State::CFG &&
BF.getState() != BinaryFunction::State::CFG_Finalized) {
@@ -91,13 +90,7 @@ bool InsertNegateRAState::addNegateRAStateAfterPacOrAuth(BinaryFunction &BF) {
for (BinaryBasicBlock &BB : BF) {
for (auto Iter = BB.begin(); Iter != BB.end(); ++Iter) {
MCInst &Inst = *Iter;
- if (BC.MIB->isPSign(Inst)) {
- Iter = BF.addCFIInstruction(
- &BB, Iter + 1, MCCFIInstruction::createNegateRAState(nullptr));
- FoundAny = true;
- }
-
- if (BC.MIB->isPAuth(Inst)) {
+ if (BC.MIB->isPSign(Inst) || BC.MIB->isPAuth(Inst)) {
Iter = BF.addCFIInstruction(
&BB, Iter + 1, MCCFIInstruction::createNegateRAState(nullptr));
FoundAny = true;
@@ -111,9 +104,7 @@ void InsertNegateRAState::fixUnknownStates(BinaryFunction &BF) {
BinaryContext &BC = BF.getBinaryContext();
bool FirstIter = true;
MCInst PrevInst;
- for (auto BBIt = BF.begin(); BBIt != BF.end(); ++BBIt) {
- BinaryBasicBlock &BB = *BBIt;
-
+ for (BinaryBasicBlock &BB : BF) {
for (auto It = BB.begin(); It != BB.end(); ++It) {
MCInst &Inst = *It;
diff --git a/bolt/lib/Passes/MarkRAStates.cpp b/bolt/lib/Passes/MarkRAStates.cpp
index 03affab2f21546..69d68e46a2d46d 100644
--- a/bolt/lib/Passes/MarkRAStates.cpp
+++ b/bolt/lib/Passes/MarkRAStates.cpp
@@ -11,7 +11,7 @@
// - NegateRAState flips the RA State,
// - RememberState pushes the RA State to a stack,
// - RestoreState pops the RA State from the stack.
-// These were saved as MCAnnotations on instructions they refer to at CFI
+// These are saved as MCAnnotations on instructions they refer to at CFI
// reading (in CFIReaderWriter::fillCFIInfoFor). In this pass, we can work out
// the RA State of each instruction, and save it as new MCAnnotations. The new
// annotations are Signing, Signed, Authenticating and Unsigned. After
@@ -95,9 +95,9 @@ void MarkRAStates::runOnFunction(BinaryFunction &BF) {
continue;
if (Annotation == MCPlus::MCAnnotation::kNegateState)
RAState = !RAState;
- if (Annotation == MCPlus::MCAnnotation::kRememberState)
+ else if (Annotation == MCPlus::MCAnnotation::kRememberState)
RAStateStack.push(RAState);
- if (Annotation == MCPlus::MCAnnotation::kRestoreState) {
+ else if (Annotation == MCPlus::MCAnnotation::kRestoreState) {
RAState = RAStateStack.top();
RAStateStack.pop();
}
>From 7e4165cb38cbd2278283bbcebe1cc20cc9495b99 Mon Sep 17 00:00:00 2001
From: Gergely Balint <gergely.balint at arm.com>
Date: Wed, 22 Jan 2025 09:57:31 +0100
Subject: [PATCH 7/7] [BOLT] Fix more review nits (#120064)
---
bolt/include/bolt/Core/MCPlus.h | 14 +++++++-------
bolt/lib/Core/MCPlusBuilder.cpp | 6 ++----
bolt/lib/Passes/MarkRAStates.cpp | 1 -
3 files changed, 9 insertions(+), 12 deletions(-)
diff --git a/bolt/include/bolt/Core/MCPlus.h b/bolt/include/bolt/Core/MCPlus.h
index 6ff3c580da0d44..a95bba36c5a6ee 100644
--- a/bolt/include/bolt/Core/MCPlus.h
+++ b/bolt/include/bolt/Core/MCPlus.h
@@ -72,13 +72,13 @@ class MCAnnotation {
kLabel, /// MCSymbol pointing to this instruction.
kSize, /// Size of the instruction.
kDynamicBranch, /// Jit instruction patched at runtime.
- kSigning, /// Inst is a signing instruction (paciasp, etc.)
- kSigned, /// Inst is in a range where RA is signed
- kAuthenticating, /// Authenticating inst (e.g. autiasp)
- kUnsigned, /// Inst is in a range where RA is unsigned
- kRememberState, /// Inst has rememberState CFI
- kRestoreState, /// Inst has restoreState CFI
- kNegateState, /// Inst has OpNegateRAState CFI
+ kSigning, /// Inst is a signing instruction (paciasp, etc.).
+ kSigned, /// Inst is in a range where RA is signed.
+ kAuthenticating, /// Authenticating inst (e.g. autiasp).
+ kUnsigned, /// Inst is in a range where RA is unsigned.
+ kRememberState, /// Inst has rememberState CFI.
+ kRestoreState, /// Inst has restoreState CFI.
+ kNegateState, /// Inst has OpNegateRAState CFI.
kGeneric, /// First generic annotation.
};
diff --git a/bolt/lib/Core/MCPlusBuilder.cpp b/bolt/lib/Core/MCPlusBuilder.cpp
index 0a909dca35c681..dd942ec0e0dccf 100644
--- a/bolt/lib/Core/MCPlusBuilder.cpp
+++ b/bolt/lib/Core/MCPlusBuilder.cpp
@@ -211,10 +211,8 @@ bool MCPlusBuilder::isRAUnsigned(const MCInst &Inst) const {
}
bool MCPlusBuilder::isRAStateUnknown(const MCInst &Inst) const {
- if (isRAUnsigned(Inst) || isRASigned(Inst) || isRASigning(Inst) ||
- isAuthenticating(Inst))
- return false;
- return true;
+ return !(isRAUnsigned(Inst) || isRASigned(Inst) || isRASigning(Inst) ||
+ isAuthenticating(Inst));
}
std::optional<MCLandingPad> MCPlusBuilder::getEHInfo(const MCInst &Inst) const {
diff --git a/bolt/lib/Passes/MarkRAStates.cpp b/bolt/lib/Passes/MarkRAStates.cpp
index 69d68e46a2d46d..4d1f126faf074f 100644
--- a/bolt/lib/Passes/MarkRAStates.cpp
+++ b/bolt/lib/Passes/MarkRAStates.cpp
@@ -74,7 +74,6 @@ void MarkRAStates::runOnFunction(BinaryFunction &BF) {
if (BC.MIB->isPSign(Inst)) {
assert(!RAState && "Signed RA State before PSign");
BC.MIB->setRASigning(Inst);
-
} else if (BC.MIB->isPAuth(Inst)) {
assert(RAState && "Unsigned RA State before PAuth");
BC.MIB->setAuthenticating(Inst);
More information about the llvm-commits
mailing list