https://github.com/atrosinenko updated
https://github.com/llvm/llvm-project/pull/135661
>From 3ee58131f5d224a71e2ee32075009fde17772856 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 75a8d26c64537..451299327e3b2 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>
@@ -197,9 +196,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 {
@@ -208,7 +204,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.
@@ -259,7 +255,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 12eb9c66130b9..3d723456b6ffd 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.
@@ -409,8 +408,7 @@ class SrcSafetyAnalysis {
// FirstCheckerInst should belong to the same basic block (see the
// assertion in DataflowSrcSafetyAnalysis::run()), 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);
}
@@ -523,10 +521,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) {
@@ -537,7 +532,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;
}
@@ -557,11 +552,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 {
@@ -674,7 +669,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");
@@ -708,7 +703,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);
}
@@ -718,7 +713,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);
@@ -825,7 +820,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 eb984d9bc72f54a841d924bafaa15f0e7566f645 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 451299327e3b2..ccfe632889c7a 100644
--- a/bolt/include/bolt/Passes/PAuthGadgetScanner.h
+++ b/bolt/include/bolt/Passes/PAuthGadgetScanner.h
@@ -219,8 +219,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;
@@ -243,11 +243,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 3d723456b6ffd..92608aebce3ee 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;
@@ -518,7 +517,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);
@@ -552,7 +551,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 {
@@ -669,7 +668,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");
@@ -713,7 +712,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
