https://github.com/atrosinenko updated
https://github.com/llvm/llvm-project/pull/135661
>From b811de675aa768e8a7c944b7a25d137eb4b21985 Mon Sep 17 00:00:00 2001
From: Anatoly Trosinenko <atrosine...@accesssoftek.com>
Date: Mon, 14 Apr 2025 14:35:56 +0300
Subject: [PATCH 1/2] [BOLT] Gadget scanner: use more appropriate types (NFC)
* use more flexible `const ArrayRef<T>` and `StringRef` types instead of
`const std::vector<T> &` and `const std::string &`, correspondingly,
for function arguments
* return plain `const SrcState &` instead of `ErrorOr<const SrcState &>`
from `SrcSafetyAnalysis::getStateBefore`, as absent state is not
handled gracefully by any caller
---
bolt/include/bolt/Passes/PAuthGadgetScanner.h | 8 +---
bolt/lib/Passes/PAuthGadgetScanner.cpp | 39 ++++++++-----------
2 files changed, 19 insertions(+), 28 deletions(-)
diff --git a/bolt/include/bolt/Passes/PAuthGadgetScanner.h
b/bolt/include/bolt/Passes/PAuthGadgetScanner.h
index 6765e2aff414f..3e39b64e59e0f 100644
--- a/bolt/include/bolt/Passes/PAuthGadgetScanner.h
+++ b/bolt/include/bolt/Passes/PAuthGadgetScanner.h
@@ -12,7 +12,6 @@
#include "bolt/Core/BinaryContext.h"
#include "bolt/Core/BinaryFunction.h"
#include "bolt/Passes/BinaryPasses.h"
-#include "llvm/ADT/SmallSet.h"
#include "llvm/Support/raw_ostream.h"
#include <memory>
@@ -199,9 +198,6 @@ raw_ostream &operator<<(raw_ostream &OS, const
MCInstReference &);
namespace PAuthGadgetScanner {
-class SrcSafetyAnalysis;
-struct SrcState;
-
/// Description of a gadget kind that can be detected. Intended to be
/// statically allocated to be attached to reports by reference.
class GadgetKind {
@@ -210,7 +206,7 @@ class GadgetKind {
public:
GadgetKind(const char *Description) : Description(Description) {}
- const StringRef getDescription() const { return Description; }
+ StringRef getDescription() const { return Description; }
};
/// Base report located at some instruction, without any additional
information.
@@ -261,7 +257,7 @@ struct GadgetReport : public Report {
/// Report with a free-form message attached.
struct GenericReport : public Report {
std::string Text;
- GenericReport(MCInstReference Location, const std::string &Text)
+ GenericReport(MCInstReference Location, StringRef Text)
: Report(Location), Text(Text) {}
virtual void generateReport(raw_ostream &OS,
const BinaryContext &BC) const override;
diff --git a/bolt/lib/Passes/PAuthGadgetScanner.cpp
b/bolt/lib/Passes/PAuthGadgetScanner.cpp
index 4f601558dec4e..339673b600765 100644
--- a/bolt/lib/Passes/PAuthGadgetScanner.cpp
+++ b/bolt/lib/Passes/PAuthGadgetScanner.cpp
@@ -91,14 +91,14 @@ class TrackedRegisters {
const std::vector<MCPhysReg> Registers;
std::vector<uint16_t> RegToIndexMapping;
- static size_t getMappingSize(const std::vector<MCPhysReg> &RegsToTrack) {
+ static size_t getMappingSize(const ArrayRef<MCPhysReg> RegsToTrack) {
if (RegsToTrack.empty())
return 0;
return 1 + *llvm::max_element(RegsToTrack);
}
public:
- TrackedRegisters(const std::vector<MCPhysReg> &RegsToTrack)
+ TrackedRegisters(const ArrayRef<MCPhysReg> RegsToTrack)
: Registers(RegsToTrack),
RegToIndexMapping(getMappingSize(RegsToTrack), NoIndex) {
for (unsigned I = 0; I < RegsToTrack.size(); ++I)
@@ -234,7 +234,7 @@ struct SrcState {
static void printLastInsts(
raw_ostream &OS,
- const std::vector<SmallPtrSet<const MCInst *, 4>> &LastInstWritingReg) {
+ const ArrayRef<SmallPtrSet<const MCInst *, 4>> LastInstWritingReg) {
OS << "Insts: ";
for (unsigned I = 0; I < LastInstWritingReg.size(); ++I) {
auto &Set = LastInstWritingReg[I];
@@ -295,7 +295,7 @@ void SrcStatePrinter::print(raw_ostream &OS, const SrcState
&S) const {
class SrcSafetyAnalysis {
public:
SrcSafetyAnalysis(BinaryFunction &BF,
- const std::vector<MCPhysReg> &RegsToTrackInstsFor)
+ const ArrayRef<MCPhysReg> RegsToTrackInstsFor)
: BC(BF.getBinaryContext()), NumRegs(BC.MRI->getNumRegs()),
RegsToTrackInstsFor(RegsToTrackInstsFor) {}
@@ -303,11 +303,10 @@ class SrcSafetyAnalysis {
static std::shared_ptr<SrcSafetyAnalysis>
create(BinaryFunction &BF, MCPlusBuilder::AllocatorIdTy AllocId,
- const std::vector<MCPhysReg> &RegsToTrackInstsFor);
+ const ArrayRef<MCPhysReg> RegsToTrackInstsFor);
virtual void run() = 0;
- virtual ErrorOr<const SrcState &>
- getStateBefore(const MCInst &Inst) const = 0;
+ virtual const SrcState &getStateBefore(const MCInst &Inst) const = 0;
protected:
BinaryContext &BC;
@@ -347,7 +346,7 @@ class SrcSafetyAnalysis {
}
BitVector getClobberedRegs(const MCInst &Point) const {
- BitVector Clobbered(NumRegs, false);
+ BitVector Clobbered(NumRegs);
// Assume a call can clobber all registers, including callee-saved
// registers. There's a good chance that callee-saved registers will be
// saved on the stack at some point during execution of the callee.
@@ -408,8 +407,7 @@ class SrcSafetyAnalysis {
// FirstCheckerInst should belong to the same basic block, meaning
// it was deterministically processed a few steps before this
instruction.
- const SrcState &StateBeforeChecker =
- getStateBefore(*FirstCheckerInst).get();
+ const SrcState &StateBeforeChecker = getStateBefore(*FirstCheckerInst);
if (StateBeforeChecker.SafeToDerefRegs[CheckedReg])
Regs.push_back(CheckedReg);
}
@@ -522,10 +520,7 @@ class SrcSafetyAnalysis {
const ArrayRef<MCPhysReg> UsedDirtyRegs) const {
if (RegsToTrackInstsFor.empty())
return {};
- auto MaybeState = getStateBefore(Inst);
- if (!MaybeState)
- llvm_unreachable("Expected state to be present");
- const SrcState &S = *MaybeState;
+ const SrcState &S = getStateBefore(Inst);
// Due to aliasing registers, multiple registers may have been tracked.
std::set<const MCInst *> LastWritingInsts;
for (MCPhysReg TrackedReg : UsedDirtyRegs) {
@@ -536,7 +531,7 @@ class SrcSafetyAnalysis {
for (const MCInst *Inst : LastWritingInsts) {
MCInstReference Ref = MCInstReference::get(Inst, BF);
assert(Ref && "Expected Inst to be found");
- Result.push_back(MCInstReference(Ref));
+ Result.push_back(Ref);
}
return Result;
}
@@ -556,11 +551,11 @@ class DataflowSrcSafetyAnalysis
public:
DataflowSrcSafetyAnalysis(BinaryFunction &BF,
MCPlusBuilder::AllocatorIdTy AllocId,
- const std::vector<MCPhysReg> &RegsToTrackInstsFor)
+ const ArrayRef<MCPhysReg> RegsToTrackInstsFor)
: SrcSafetyAnalysis(BF, RegsToTrackInstsFor), DFParent(BF, AllocId) {}
- ErrorOr<const SrcState &> getStateBefore(const MCInst &Inst) const override {
- return DFParent::getStateBefore(Inst);
+ const SrcState &getStateBefore(const MCInst &Inst) const override {
+ return DFParent::getStateBefore(Inst).get();
}
void run() override {
@@ -669,7 +664,7 @@ class CFGUnawareSrcSafetyAnalysis : public
SrcSafetyAnalysis {
public:
CFGUnawareSrcSafetyAnalysis(BinaryFunction &BF,
MCPlusBuilder::AllocatorIdTy AllocId,
- const std::vector<MCPhysReg>
&RegsToTrackInstsFor)
+ const ArrayRef<MCPhysReg> RegsToTrackInstsFor)
: SrcSafetyAnalysis(BF, RegsToTrackInstsFor), BF(BF), AllocId(AllocId) {
StateAnnotationIndex =
BC.MIB->getOrCreateAnnotationIndex("CFGUnawareSrcSafetyAnalysis");
@@ -703,7 +698,7 @@ class CFGUnawareSrcSafetyAnalysis : public
SrcSafetyAnalysis {
}
}
- ErrorOr<const SrcState &> getStateBefore(const MCInst &Inst) const override {
+ const SrcState &getStateBefore(const MCInst &Inst) const override {
return BC.MIB->getAnnotationAs<SrcState>(Inst, StateAnnotationIndex);
}
@@ -713,7 +708,7 @@ class CFGUnawareSrcSafetyAnalysis : public
SrcSafetyAnalysis {
std::shared_ptr<SrcSafetyAnalysis>
SrcSafetyAnalysis::create(BinaryFunction &BF,
MCPlusBuilder::AllocatorIdTy AllocId,
- const std::vector<MCPhysReg> &RegsToTrackInstsFor) {
+ const ArrayRef<MCPhysReg> RegsToTrackInstsFor) {
if (BF.hasCFG())
return std::make_shared<DataflowSrcSafetyAnalysis>(BF, AllocId,
RegsToTrackInstsFor);
@@ -820,7 +815,7 @@ Analysis::findGadgets(BinaryFunction &BF,
BinaryContext &BC = BF.getBinaryContext();
iterateOverInstrs(BF, [&](MCInstReference Inst) {
- const SrcState &S = *Analysis->getStateBefore(Inst);
+ const SrcState &S = Analysis->getStateBefore(Inst);
// If non-empty state was never propagated from the entry basic block
// to Inst, assume it to be unreachable and report a warning.
>From cf7d31057572ed56e7ffafc040eaf56dda3a4faa Mon Sep 17 00:00:00 2001
From: Anatoly Trosinenko <atrosine...@accesssoftek.com>
Date: Tue, 22 Apr 2025 18:05:37 +0300
Subject: [PATCH 2/2] Drop redundant const qualifier from ArrayRef<T>
---
bolt/include/bolt/Passes/PAuthGadgetScanner.h | 8 +++---
bolt/lib/Passes/PAuthGadgetScanner.cpp | 25 +++++++++----------
2 files changed, 16 insertions(+), 17 deletions(-)
diff --git a/bolt/include/bolt/Passes/PAuthGadgetScanner.h
b/bolt/include/bolt/Passes/PAuthGadgetScanner.h
index 3e39b64e59e0f..4c1bef3d2265f 100644
--- a/bolt/include/bolt/Passes/PAuthGadgetScanner.h
+++ b/bolt/include/bolt/Passes/PAuthGadgetScanner.h
@@ -221,8 +221,8 @@ struct Report {
// The two methods below are called by Analysis::computeDetailedInfo when
// iterating over the reports.
- virtual const ArrayRef<MCPhysReg> getAffectedRegisters() const { return {}; }
- virtual void setOverwritingInstrs(const ArrayRef<MCInstReference> Instrs) {}
+ virtual ArrayRef<MCPhysReg> getAffectedRegisters() const { return {}; }
+ virtual void setOverwritingInstrs(ArrayRef<MCInstReference> Instrs) {}
void printBasicInfo(raw_ostream &OS, const BinaryContext &BC,
StringRef IssueKind) const;
@@ -245,11 +245,11 @@ struct GadgetReport : public Report {
void generateReport(raw_ostream &OS, const BinaryContext &BC) const override;
- const ArrayRef<MCPhysReg> getAffectedRegisters() const override {
+ ArrayRef<MCPhysReg> getAffectedRegisters() const override {
return AffectedRegisters;
}
- void setOverwritingInstrs(const ArrayRef<MCInstReference> Instrs) override {
+ void setOverwritingInstrs(ArrayRef<MCInstReference> Instrs) override {
OverwritingInstrs.assign(Instrs.begin(), Instrs.end());
}
};
diff --git a/bolt/lib/Passes/PAuthGadgetScanner.cpp
b/bolt/lib/Passes/PAuthGadgetScanner.cpp
index 339673b600765..6590d1a58146f 100644
--- a/bolt/lib/Passes/PAuthGadgetScanner.cpp
+++ b/bolt/lib/Passes/PAuthGadgetScanner.cpp
@@ -91,21 +91,21 @@ class TrackedRegisters {
const std::vector<MCPhysReg> Registers;
std::vector<uint16_t> RegToIndexMapping;
- static size_t getMappingSize(const ArrayRef<MCPhysReg> RegsToTrack) {
+ static size_t getMappingSize(ArrayRef<MCPhysReg> RegsToTrack) {
if (RegsToTrack.empty())
return 0;
return 1 + *llvm::max_element(RegsToTrack);
}
public:
- TrackedRegisters(const ArrayRef<MCPhysReg> RegsToTrack)
+ TrackedRegisters(ArrayRef<MCPhysReg> RegsToTrack)
: Registers(RegsToTrack),
RegToIndexMapping(getMappingSize(RegsToTrack), NoIndex) {
for (unsigned I = 0; I < RegsToTrack.size(); ++I)
RegToIndexMapping[RegsToTrack[I]] = I;
}
- const ArrayRef<MCPhysReg> getRegisters() const { return Registers; }
+ ArrayRef<MCPhysReg> getRegisters() const { return Registers; }
size_t getNumTrackedRegisters() const { return Registers.size(); }
@@ -232,9 +232,9 @@ struct SrcState {
bool operator!=(const SrcState &RHS) const { return !((*this) == RHS); }
};
-static void printLastInsts(
- raw_ostream &OS,
- const ArrayRef<SmallPtrSet<const MCInst *, 4>> LastInstWritingReg) {
+static void
+printLastInsts(raw_ostream &OS,
+ ArrayRef<SmallPtrSet<const MCInst *, 4>> LastInstWritingReg) {
OS << "Insts: ";
for (unsigned I = 0; I < LastInstWritingReg.size(); ++I) {
auto &Set = LastInstWritingReg[I];
@@ -294,8 +294,7 @@ void SrcStatePrinter::print(raw_ostream &OS, const SrcState
&S) const {
/// version for functions without reconstructed CFG.
class SrcSafetyAnalysis {
public:
- SrcSafetyAnalysis(BinaryFunction &BF,
- const ArrayRef<MCPhysReg> RegsToTrackInstsFor)
+ SrcSafetyAnalysis(BinaryFunction &BF, ArrayRef<MCPhysReg>
RegsToTrackInstsFor)
: BC(BF.getBinaryContext()), NumRegs(BC.MRI->getNumRegs()),
RegsToTrackInstsFor(RegsToTrackInstsFor) {}
@@ -303,7 +302,7 @@ class SrcSafetyAnalysis {
static std::shared_ptr<SrcSafetyAnalysis>
create(BinaryFunction &BF, MCPlusBuilder::AllocatorIdTy AllocId,
- const ArrayRef<MCPhysReg> RegsToTrackInstsFor);
+ ArrayRef<MCPhysReg> RegsToTrackInstsFor);
virtual void run() = 0;
virtual const SrcState &getStateBefore(const MCInst &Inst) const = 0;
@@ -517,7 +516,7 @@ class SrcSafetyAnalysis {
public:
std::vector<MCInstReference>
getLastClobberingInsts(const MCInst &Inst, BinaryFunction &BF,
- const ArrayRef<MCPhysReg> UsedDirtyRegs) const {
+ ArrayRef<MCPhysReg> UsedDirtyRegs) const {
if (RegsToTrackInstsFor.empty())
return {};
const SrcState &S = getStateBefore(Inst);
@@ -551,7 +550,7 @@ class DataflowSrcSafetyAnalysis
public:
DataflowSrcSafetyAnalysis(BinaryFunction &BF,
MCPlusBuilder::AllocatorIdTy AllocId,
- const ArrayRef<MCPhysReg> RegsToTrackInstsFor)
+ ArrayRef<MCPhysReg> RegsToTrackInstsFor)
: SrcSafetyAnalysis(BF, RegsToTrackInstsFor), DFParent(BF, AllocId) {}
const SrcState &getStateBefore(const MCInst &Inst) const override {
@@ -664,7 +663,7 @@ class CFGUnawareSrcSafetyAnalysis : public
SrcSafetyAnalysis {
public:
CFGUnawareSrcSafetyAnalysis(BinaryFunction &BF,
MCPlusBuilder::AllocatorIdTy AllocId,
- const ArrayRef<MCPhysReg> RegsToTrackInstsFor)
+ ArrayRef<MCPhysReg> RegsToTrackInstsFor)
: SrcSafetyAnalysis(BF, RegsToTrackInstsFor), BF(BF), AllocId(AllocId) {
StateAnnotationIndex =
BC.MIB->getOrCreateAnnotationIndex("CFGUnawareSrcSafetyAnalysis");
@@ -708,7 +707,7 @@ class CFGUnawareSrcSafetyAnalysis : public
SrcSafetyAnalysis {
std::shared_ptr<SrcSafetyAnalysis>
SrcSafetyAnalysis::create(BinaryFunction &BF,
MCPlusBuilder::AllocatorIdTy AllocId,
- const ArrayRef<MCPhysReg> RegsToTrackInstsFor) {
+ ArrayRef<MCPhysReg> RegsToTrackInstsFor) {
if (BF.hasCFG())
return std::make_shared<DataflowSrcSafetyAnalysis>(BF, AllocId,
RegsToTrackInstsFor);
_______________________________________________
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
