[llvm-branch-commits] [llvm] [BOLT] Gadget scanner: refactor issue reporting (PR #135662)
Anatoly Trosinenko via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Fri May 16 10:10:25 PDT 2025
https://github.com/atrosinenko updated https://github.com/llvm/llvm-project/pull/135662
>From 7f4379ffa6bc0e30e3ea7354b7bf2a2c17e954e0 Mon Sep 17 00:00:00 2001
From: Anatoly Trosinenko <atrosinenko at accesssoftek.com>
Date: Mon, 14 Apr 2025 15:08:54 +0300
Subject: [PATCH 1/5] [BOLT] Gadget scanner: refactor issue reporting
Remove `getAffectedRegisters` and `setOverwritingInstrs` methods from
the base `Report` class. Instead, make `Report` always represent the
brief version of the report. When an issue is detected on the first run
of the analysis, return an optional request for extra details to attach
to the report on the second run.
---
bolt/include/bolt/Passes/PAuthGadgetScanner.h | 102 ++++++---
bolt/lib/Passes/PAuthGadgetScanner.cpp | 200 ++++++++++--------
.../AArch64/gs-pauth-debug-output.s | 8 +-
3 files changed, 187 insertions(+), 123 deletions(-)
diff --git a/bolt/include/bolt/Passes/PAuthGadgetScanner.h b/bolt/include/bolt/Passes/PAuthGadgetScanner.h
index ccfe632889c7a..1cd3f3f6d3233 100644
--- a/bolt/include/bolt/Passes/PAuthGadgetScanner.h
+++ b/bolt/include/bolt/Passes/PAuthGadgetScanner.h
@@ -217,11 +217,6 @@ struct Report {
virtual void generateReport(raw_ostream &OS,
const BinaryContext &BC) const = 0;
- // The two methods below are called by Analysis::computeDetailedInfo when
- // iterating over the reports.
- virtual ArrayRef<MCPhysReg> getAffectedRegisters() const { return {}; }
- virtual void setOverwritingInstrs(ArrayRef<MCInstReference> Instrs) {}
-
void printBasicInfo(raw_ostream &OS, const BinaryContext &BC,
StringRef IssueKind) const;
};
@@ -229,27 +224,11 @@ struct Report {
struct GadgetReport : public Report {
// The particular kind of gadget that is detected.
const GadgetKind &Kind;
- // The set of registers related to this gadget report (possibly empty).
- SmallVector<MCPhysReg, 1> AffectedRegisters;
- // The instructions that clobber the affected registers.
- // There is no one-to-one correspondence with AffectedRegisters: for example,
- // the same register can be overwritten by different instructions in different
- // preceding basic blocks.
- SmallVector<MCInstReference> OverwritingInstrs;
-
- GadgetReport(const GadgetKind &Kind, MCInstReference Location,
- MCPhysReg AffectedRegister)
- : Report(Location), Kind(Kind), AffectedRegisters({AffectedRegister}) {}
-
- void generateReport(raw_ostream &OS, const BinaryContext &BC) const override;
- ArrayRef<MCPhysReg> getAffectedRegisters() const override {
- return AffectedRegisters;
- }
+ GadgetReport(const GadgetKind &Kind, MCInstReference Location)
+ : Report(Location), Kind(Kind) {}
- void setOverwritingInstrs(ArrayRef<MCInstReference> Instrs) override {
- OverwritingInstrs.assign(Instrs.begin(), Instrs.end());
- }
+ void generateReport(raw_ostream &OS, const BinaryContext &BC) const override;
};
/// Report with a free-form message attached.
@@ -261,8 +240,75 @@ struct GenericReport : public Report {
const BinaryContext &BC) const override;
};
+/// An information about an issue collected on the slower, detailed,
+/// run of an analysis.
+class ExtraInfo {
+public:
+ virtual void print(raw_ostream &OS, const MCInstReference Location) const = 0;
+
+ virtual ~ExtraInfo() {}
+};
+
+class ClobberingInfo : public ExtraInfo {
+ SmallVector<MCInstReference> ClobberingInstrs;
+
+public:
+ ClobberingInfo(const ArrayRef<MCInstReference> Instrs)
+ : ClobberingInstrs(Instrs) {}
+
+ void print(raw_ostream &OS, const MCInstReference Location) const override;
+};
+
+/// A brief version of a report that can be further augmented with the details.
+///
+/// It is common for a particular type of gadget detector to be tied to some
+/// specific kind of analysis. If an issue is returned by that detector, it may
+/// be further augmented with the detailed info in an analysis-specific way,
+/// or just be left as-is (f.e. if a free-form warning was reported).
+template <typename T> struct BriefReport {
+ BriefReport(std::shared_ptr<Report> Issue,
+ const std::optional<T> RequestedDetails)
+ : Issue(Issue), RequestedDetails(RequestedDetails) {}
+
+ std::shared_ptr<Report> Issue;
+ std::optional<T> RequestedDetails;
+};
+
+/// A detailed version of a report.
+struct DetailedReport {
+ DetailedReport(std::shared_ptr<Report> Issue,
+ std::shared_ptr<ExtraInfo> Details)
+ : Issue(Issue), Details(Details) {}
+
+ std::shared_ptr<Report> Issue;
+ std::shared_ptr<ExtraInfo> Details;
+};
+
struct FunctionAnalysisResult {
- std::vector<std::shared_ptr<Report>> Diagnostics;
+ std::vector<DetailedReport> Diagnostics;
+};
+
+/// A helper class storing per-function context to be instantiated by Analysis.
+class FunctionAnalysis {
+ BinaryContext &BC;
+ BinaryFunction &BF;
+ MCPlusBuilder::AllocatorIdTy AllocatorId;
+ FunctionAnalysisResult Result;
+
+ bool PacRetGadgetsOnly;
+
+ void findUnsafeUses(SmallVector<BriefReport<MCPhysReg>> &Reports);
+ void augmentUnsafeUseReports(const ArrayRef<BriefReport<MCPhysReg>> Reports);
+
+public:
+ FunctionAnalysis(BinaryFunction &BF, MCPlusBuilder::AllocatorIdTy AllocatorId,
+ bool PacRetGadgetsOnly)
+ : BC(BF.getBinaryContext()), BF(BF), AllocatorId(AllocatorId),
+ PacRetGadgetsOnly(PacRetGadgetsOnly) {}
+
+ void run();
+
+ const FunctionAnalysisResult &getResult() const { return Result; }
};
class Analysis : public BinaryFunctionPass {
@@ -271,12 +317,6 @@ class Analysis : public BinaryFunctionPass {
void runOnFunction(BinaryFunction &Function,
MCPlusBuilder::AllocatorIdTy AllocatorId);
- FunctionAnalysisResult findGadgets(BinaryFunction &BF,
- MCPlusBuilder::AllocatorIdTy AllocatorId);
-
- void computeDetailedInfo(BinaryFunction &BF,
- MCPlusBuilder::AllocatorIdTy AllocatorId,
- FunctionAnalysisResult &Result);
std::map<const BinaryFunction *, FunctionAnalysisResult> AnalysisResults;
std::mutex AnalysisResultsMutex;
diff --git a/bolt/lib/Passes/PAuthGadgetScanner.cpp b/bolt/lib/Passes/PAuthGadgetScanner.cpp
index 92608aebce3ee..6457e3163c933 100644
--- a/bolt/lib/Passes/PAuthGadgetScanner.cpp
+++ b/bolt/lib/Passes/PAuthGadgetScanner.cpp
@@ -517,18 +517,13 @@ class SrcSafetyAnalysis {
public:
std::vector<MCInstReference>
getLastClobberingInsts(const MCInst &Inst, BinaryFunction &BF,
- ArrayRef<MCPhysReg> UsedDirtyRegs) const {
- if (RegsToTrackInstsFor.empty())
+ std::optional<MCPhysReg> ClobberedReg) const {
+ if (!ClobberedReg || RegsToTrackInstsFor.empty())
return {};
const SrcState &S = getStateBefore(Inst);
- // Due to aliasing registers, multiple registers may have been tracked.
- std::set<const MCInst *> LastWritingInsts;
- for (MCPhysReg TrackedReg : UsedDirtyRegs) {
- for (const MCInst *Inst : lastWritingInsts(S, TrackedReg))
- LastWritingInsts.insert(Inst);
- }
+
std::vector<MCInstReference> Result;
- for (const MCInst *Inst : LastWritingInsts) {
+ for (const MCInst *Inst : lastWritingInsts(S, *ClobberedReg)) {
MCInstReference Ref = MCInstReference::get(Inst, BF);
assert(Ref && "Expected Inst to be found");
Result.push_back(Ref);
@@ -720,16 +715,30 @@ SrcSafetyAnalysis::create(BinaryFunction &BF,
RegsToTrackInstsFor);
}
-static std::shared_ptr<Report>
+static BriefReport<MCPhysReg> make_generic_report(MCInstReference Location,
+ StringRef Text) {
+ auto Report = std::make_shared<GenericReport>(Location, Text);
+ return BriefReport<MCPhysReg>(Report, std::nullopt);
+}
+
+template <typename T>
+static BriefReport<T> make_report(const GadgetKind &Kind,
+ MCInstReference Location,
+ T RequestedDetails) {
+ auto Report = std::make_shared<GadgetReport>(Kind, Location);
+ return BriefReport<T>(Report, RequestedDetails);
+}
+
+static std::optional<BriefReport<MCPhysReg>>
shouldReportReturnGadget(const BinaryContext &BC, const MCInstReference &Inst,
const SrcState &S) {
static const GadgetKind RetKind("non-protected ret found");
if (!BC.MIB->isReturn(Inst))
- return nullptr;
+ return std::nullopt;
ErrorOr<MCPhysReg> MaybeRetReg = BC.MIB->getRegUsedAsRetDest(Inst);
if (MaybeRetReg.getError()) {
- return std::make_shared<GenericReport>(
+ return make_generic_report(
Inst, "Warning: pac-ret analysis could not analyze this return "
"instruction");
}
@@ -740,26 +749,26 @@ shouldReportReturnGadget(const BinaryContext &BC, const MCInstReference &Inst,
traceReg(BC, "Authenticated reg", BC.MIB->getAuthenticatedReg(Inst));
});
if (BC.MIB->isAuthenticationOfReg(Inst, RetReg))
- return nullptr;
+ return std::nullopt;
LLVM_DEBUG({ traceRegMask(BC, "SafeToDerefRegs", S.SafeToDerefRegs); });
if (S.SafeToDerefRegs[RetReg])
- return nullptr;
+ return std::nullopt;
- return std::make_shared<GadgetReport>(RetKind, Inst, RetReg);
+ return make_report(RetKind, Inst, RetReg);
}
-static std::shared_ptr<Report>
+static std::optional<BriefReport<MCPhysReg>>
shouldReportCallGadget(const BinaryContext &BC, const MCInstReference &Inst,
const SrcState &S) {
static const GadgetKind CallKind("non-protected call found");
if (!BC.MIB->isIndirectCall(Inst) && !BC.MIB->isIndirectBranch(Inst))
- return nullptr;
+ return std::nullopt;
bool IsAuthenticated = false;
MCPhysReg DestReg =
BC.MIB->getRegUsedAsIndirectBranchDest(Inst, IsAuthenticated);
if (IsAuthenticated)
- return nullptr;
+ return std::nullopt;
assert(DestReg != BC.MIB->getNoRegister());
LLVM_DEBUG({
@@ -768,19 +777,19 @@ shouldReportCallGadget(const BinaryContext &BC, const MCInstReference &Inst,
traceRegMask(BC, "SafeToDerefRegs", S.SafeToDerefRegs);
});
if (S.SafeToDerefRegs[DestReg])
- return nullptr;
+ return std::nullopt;
- return std::make_shared<GadgetReport>(CallKind, Inst, DestReg);
+ return make_report(CallKind, Inst, DestReg);
}
-static std::shared_ptr<Report>
+static std::optional<BriefReport<MCPhysReg>>
shouldReportSigningOracle(const BinaryContext &BC, const MCInstReference &Inst,
const SrcState &S) {
static const GadgetKind SigningOracleKind("signing oracle found");
MCPhysReg SignedReg = BC.MIB->getSignedReg(Inst);
if (SignedReg == BC.MIB->getNoRegister())
- return nullptr;
+ return std::nullopt;
LLVM_DEBUG({
traceInst(BC, "Found sign inst", Inst);
@@ -788,9 +797,9 @@ shouldReportSigningOracle(const BinaryContext &BC, const MCInstReference &Inst,
traceRegMask(BC, "TrustedRegs", S.TrustedRegs);
});
if (S.TrustedRegs[SignedReg])
- return nullptr;
+ return std::nullopt;
- return std::make_shared<GadgetReport>(SigningOracleKind, Inst, SignedReg);
+ return make_report(SigningOracleKind, Inst, SignedReg);
}
template <typename T> static void iterateOverInstrs(BinaryFunction &BF, T Fn) {
@@ -804,11 +813,18 @@ template <typename T> static void iterateOverInstrs(BinaryFunction &BF, T Fn) {
}
}
-FunctionAnalysisResult
-Analysis::findGadgets(BinaryFunction &BF,
- MCPlusBuilder::AllocatorIdTy AllocatorId) {
- FunctionAnalysisResult Result;
+static SmallVector<MCPhysReg>
+collectRegsToTrack(const ArrayRef<BriefReport<MCPhysReg>> Reports) {
+ SmallSet<MCPhysReg, 4> RegsToTrack;
+ for (auto Report : Reports)
+ if (Report.RequestedDetails)
+ RegsToTrack.insert(*Report.RequestedDetails);
+
+ return SmallVector<MCPhysReg>(RegsToTrack.begin(), RegsToTrack.end());
+}
+void FunctionAnalysis::findUnsafeUses(
+ SmallVector<BriefReport<MCPhysReg>> &Reports) {
auto Analysis = SrcSafetyAnalysis::create(BF, AllocatorId, {});
LLVM_DEBUG({ dbgs() << "Running src register safety analysis...\n"; });
Analysis->run();
@@ -817,45 +833,35 @@ Analysis::findGadgets(BinaryFunction &BF,
BF.dump();
});
- BinaryContext &BC = BF.getBinaryContext();
iterateOverInstrs(BF, [&](MCInstReference 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.
if (S.empty()) {
- Result.Diagnostics.push_back(std::make_shared<GenericReport>(
- Inst, "Warning: unreachable instruction found"));
+ Reports.push_back(
+ make_generic_report(Inst, "Warning: unreachable instruction found"));
return;
}
if (auto Report = shouldReportReturnGadget(BC, Inst, S))
- Result.Diagnostics.push_back(Report);
+ Reports.push_back(*Report);
if (PacRetGadgetsOnly)
return;
if (auto Report = shouldReportCallGadget(BC, Inst, S))
- Result.Diagnostics.push_back(Report);
+ Reports.push_back(*Report);
if (auto Report = shouldReportSigningOracle(BC, Inst, S))
- Result.Diagnostics.push_back(Report);
+ Reports.push_back(*Report);
});
- return Result;
}
-void Analysis::computeDetailedInfo(BinaryFunction &BF,
- MCPlusBuilder::AllocatorIdTy AllocatorId,
- FunctionAnalysisResult &Result) {
- BinaryContext &BC = BF.getBinaryContext();
-
- // Collect the affected registers across all gadgets found in this function.
- SmallSet<MCPhysReg, 4> RegsToTrack;
- for (auto Report : Result.Diagnostics)
- RegsToTrack.insert_range(Report->getAffectedRegisters());
- std::vector<MCPhysReg> RegsToTrackVec(RegsToTrack.begin(), RegsToTrack.end());
-
+void FunctionAnalysis::augmentUnsafeUseReports(
+ const ArrayRef<BriefReport<MCPhysReg>> Reports) {
+ SmallVector<MCPhysReg> RegsToTrack = collectRegsToTrack(Reports);
// Re-compute the analysis with register tracking.
- auto Analysis = SrcSafetyAnalysis::create(BF, AllocatorId, RegsToTrackVec);
+ auto Analysis = SrcSafetyAnalysis::create(BF, AllocatorId, RegsToTrack);
LLVM_DEBUG(
{ dbgs() << "\nRunning detailed src register safety analysis...\n"; });
Analysis->run();
@@ -865,32 +871,37 @@ void Analysis::computeDetailedInfo(BinaryFunction &BF,
});
// Augment gadget reports.
- for (auto Report : Result.Diagnostics) {
- LLVM_DEBUG(
- { traceInst(BC, "Attaching clobbering info to", Report->Location); });
- (void)BC;
- Report->setOverwritingInstrs(Analysis->getLastClobberingInsts(
- Report->Location, BF, Report->getAffectedRegisters()));
+ for (auto Report : Reports) {
+ MCInstReference Location = Report.Issue->Location;
+ LLVM_DEBUG({ traceInst(BC, "Attaching clobbering info to", Location); });
+ auto DetailedInfo =
+ std::make_shared<ClobberingInfo>(Analysis->getLastClobberingInsts(
+ Location, BF, Report.RequestedDetails));
+ Result.Diagnostics.emplace_back(Report.Issue, DetailedInfo);
}
}
-void Analysis::runOnFunction(BinaryFunction &BF,
- MCPlusBuilder::AllocatorIdTy AllocatorId) {
+void FunctionAnalysis::run() {
LLVM_DEBUG({
- dbgs() << "Analyzing in function " << BF.getPrintName() << ", AllocatorId "
- << AllocatorId << "\n";
+ dbgs() << "Analyzing function " << BF.getPrintName()
+ << ", AllocatorId = " << AllocatorId << "\n";
BF.dump();
});
- FunctionAnalysisResult FAR = findGadgets(BF, AllocatorId);
- if (FAR.Diagnostics.empty())
- return;
+ SmallVector<BriefReport<MCPhysReg>> UnsafeUses;
+ findUnsafeUses(UnsafeUses);
+ if (!UnsafeUses.empty())
+ augmentUnsafeUseReports(UnsafeUses);
+}
- // Redo the analysis, but now also track which instructions last wrote
- // to any of the registers in RetRegsWithGadgets, so that better
- // diagnostics can be produced.
+void Analysis::runOnFunction(BinaryFunction &BF,
+ MCPlusBuilder::AllocatorIdTy AllocatorId) {
+ FunctionAnalysis FA(BF, AllocatorId, PacRetGadgetsOnly);
+ FA.run();
- computeDetailedInfo(BF, AllocatorId, FAR);
+ const FunctionAnalysisResult &FAR = FA.getResult();
+ if (FAR.Diagnostics.empty())
+ return;
// `runOnFunction` is typically getting called from multiple threads in
// parallel. Therefore, use a lock to avoid data races when storing the
@@ -918,16 +929,14 @@ static void printBB(const BinaryContext &BC, const BinaryBasicBlock *BB,
}
}
-static void reportFoundGadgetInSingleBBSingleOverwInst(
- raw_ostream &OS, const BinaryContext &BC, const MCInstReference OverwInst,
+static void reportFoundGadgetInSingleBBSingleRelatedInst(
+ raw_ostream &OS, const BinaryContext &BC, const MCInstReference RelatedInst,
const MCInstReference Location) {
BinaryBasicBlock *BB = Location.getBasicBlock();
- assert(OverwInst.ParentKind == MCInstReference::BasicBlockParent);
+ assert(RelatedInst.ParentKind == MCInstReference::BasicBlockParent);
assert(Location.ParentKind == MCInstReference::BasicBlockParent);
- MCInstInBBReference OverwInstBB = OverwInst.U.BBRef;
- if (BB == OverwInstBB.BB) {
- // overwriting inst and ret instruction are in the same basic block.
- assert(OverwInstBB.BBIndex < Location.U.BBRef.BBIndex);
+ MCInstInBBReference RelatedInstBB = RelatedInst.U.BBRef;
+ if (BB == RelatedInstBB.BB) {
OS << " This happens in the following basic block:\n";
printBB(BC, BB);
}
@@ -950,31 +959,42 @@ void Report::printBasicInfo(raw_ostream &OS, const BinaryContext &BC,
void GadgetReport::generateReport(raw_ostream &OS,
const BinaryContext &BC) const {
printBasicInfo(OS, BC, Kind.getDescription());
+}
+
+static void printRelatedInstrs(raw_ostream &OS, const MCInstReference Location,
+ const ArrayRef<MCInstReference> RelatedInstrs) {
+ const BinaryFunction &BF = *Location.getFunction();
+ const BinaryContext &BC = BF.getBinaryContext();
- BinaryFunction *BF = Location.getFunction();
- OS << " The " << OverwritingInstrs.size()
- << " instructions that write to the affected registers after any "
- "authentication are:\n";
// Sort by address to ensure output is deterministic.
- SmallVector<MCInstReference> OI = OverwritingInstrs;
- llvm::sort(OI, [](const MCInstReference &A, const MCInstReference &B) {
+ SmallVector<MCInstReference> RI(RelatedInstrs);
+ llvm::sort(RI, [](const MCInstReference &A, const MCInstReference &B) {
return A.getAddress() < B.getAddress();
});
- for (unsigned I = 0; I < OI.size(); ++I) {
- MCInstReference InstRef = OI[I];
+ for (unsigned I = 0; I < RI.size(); ++I) {
+ MCInstReference InstRef = RI[I];
OS << " " << (I + 1) << ". ";
- BC.printInstruction(OS, InstRef, InstRef.getAddress(), BF);
+ BC.printInstruction(OS, InstRef, InstRef.getAddress(), &BF);
};
- if (OverwritingInstrs.size() == 1) {
- const MCInstReference OverwInst = OverwritingInstrs[0];
+ if (RelatedInstrs.size() == 1) {
+ const MCInstReference RelatedInst = RelatedInstrs[0];
// Printing the details for the MCInstReference::FunctionParent case
// is not implemented not to overcomplicate the code, as most functions
// are expected to have CFG information.
- if (OverwInst.ParentKind == MCInstReference::BasicBlockParent)
- reportFoundGadgetInSingleBBSingleOverwInst(OS, BC, OverwInst, Location);
+ if (RelatedInst.ParentKind == MCInstReference::BasicBlockParent)
+ reportFoundGadgetInSingleBBSingleRelatedInst(OS, BC, RelatedInst,
+ Location);
}
}
+void ClobberingInfo::print(raw_ostream &OS,
+ const MCInstReference Location) const {
+ OS << " The " << ClobberingInstrs.size()
+ << " instructions that write to the affected registers after any "
+ "authentication are:\n";
+ printRelatedInstrs(OS, Location, ClobberingInstrs);
+}
+
void GenericReport::generateReport(raw_ostream &OS,
const BinaryContext &BC) const {
printBasicInfo(OS, BC, Text);
@@ -994,11 +1014,15 @@ Error Analysis::runOnFunctions(BinaryContext &BC) {
BC, ParallelUtilities::SchedulingPolicy::SP_INST_LINEAR, WorkFun,
SkipFunc, "PAuthGadgetScanner");
- for (BinaryFunction *BF : BC.getAllBinaryFunctions())
- if (AnalysisResults.count(BF) > 0) {
- for (const std::shared_ptr<Report> &R : AnalysisResults[BF].Diagnostics)
- R->generateReport(outs(), BC);
+ for (BinaryFunction *BF : BC.getAllBinaryFunctions()) {
+ if (!AnalysisResults.count(BF))
+ continue;
+ for (const DetailedReport &R : AnalysisResults[BF].Diagnostics) {
+ R.Issue->generateReport(outs(), BC);
+ if (R.Details)
+ R.Details->print(outs(), R.Issue->Location);
}
+ }
return Error::success();
}
diff --git a/bolt/test/binary-analysis/AArch64/gs-pauth-debug-output.s b/bolt/test/binary-analysis/AArch64/gs-pauth-debug-output.s
index e0e8471c36a7e..078a8aca72d9c 100644
--- a/bolt/test/binary-analysis/AArch64/gs-pauth-debug-output.s
+++ b/bolt/test/binary-analysis/AArch64/gs-pauth-debug-output.s
@@ -24,7 +24,7 @@ simple:
ret
.size simple, .-simple
-// CHECK-LABEL:Analyzing in function simple, AllocatorId 1
+// CHECK-LABEL:Analyzing function simple, AllocatorId = 1
// CHECK-NEXT: Binary Function "simple" {
// CHECK-NEXT: Number : 1
// CHECK-NEXT: State : CFG constructed
@@ -129,7 +129,7 @@ clobber:
ret
.size clobber, .-clobber
-// CHECK-LABEL:Analyzing in function clobber, AllocatorId 1
+// CHECK-LABEL:Analyzing function clobber, AllocatorId = 1
// ...
// CHECK: Running src register safety analysis...
// CHECK-NEXT: SrcSafetyAnalysis::ComputeNext( mov w30, #0x0, src-state<SafeToDerefRegs: LR W30 W30_HI , TrustedRegs: LR W30 W30_HI , Insts: >)
@@ -174,7 +174,7 @@ nocfg:
ret
.size nocfg, .-nocfg
-// CHECK-LABEL:Analyzing in function nocfg, AllocatorId 1
+// CHECK-LABEL:Analyzing function nocfg, AllocatorId = 1
// CHECK-NEXT: Binary Function "nocfg" {
// CHECK-NEXT: Number : 3
// CHECK-NEXT: State : disassembled
@@ -254,7 +254,7 @@ nocfg:
// CHECK-EMPTY:
// CHECK-NEXT: Attaching clobbering info to: 00000000: ret # Offset: 8 # CFGUnawareSrcSafetyAnalysis: src-state<SafeToDerefRegs: BitVector, TrustedRegs: BitVector, Insts: [0]()>
-// CHECK-LABEL:Analyzing in function main, AllocatorId 1
+// CHECK-LABEL:Analyzing function main, AllocatorId = 1
.globl main
.type main, at function
main:
>From bf8bfa9ac23b2d26aadb05f507f4ca946cdc98e2 Mon Sep 17 00:00:00 2001
From: Anatoly Trosinenko <atrosinenko at accesssoftek.com>
Date: Fri, 18 Apr 2025 21:14:40 +0300
Subject: [PATCH 2/5] Omit clobbering info for warnings, simplify
---
bolt/include/bolt/Passes/PAuthGadgetScanner.h | 4 ++++
bolt/lib/Passes/PAuthGadgetScanner.cpp | 24 ++++++++++++++-----
.../AArch64/gs-pacret-autiasp.s | 1 +
3 files changed, 23 insertions(+), 6 deletions(-)
diff --git a/bolt/include/bolt/Passes/PAuthGadgetScanner.h b/bolt/include/bolt/Passes/PAuthGadgetScanner.h
index 1cd3f3f6d3233..3b472f8bbdf4c 100644
--- a/bolt/include/bolt/Passes/PAuthGadgetScanner.h
+++ b/bolt/include/bolt/Passes/PAuthGadgetScanner.h
@@ -300,6 +300,10 @@ class FunctionAnalysis {
void findUnsafeUses(SmallVector<BriefReport<MCPhysReg>> &Reports);
void augmentUnsafeUseReports(const ArrayRef<BriefReport<MCPhysReg>> Reports);
+ /// Process the reports which do not have to be augmented, and remove them
+ /// from Reports.
+ void handleSimpleReports(SmallVector<BriefReport<MCPhysReg>> &Reports);
+
public:
FunctionAnalysis(BinaryFunction &BF, MCPlusBuilder::AllocatorIdTy AllocatorId,
bool PacRetGadgetsOnly)
diff --git a/bolt/lib/Passes/PAuthGadgetScanner.cpp b/bolt/lib/Passes/PAuthGadgetScanner.cpp
index 6457e3163c933..213d224433419 100644
--- a/bolt/lib/Passes/PAuthGadgetScanner.cpp
+++ b/bolt/lib/Passes/PAuthGadgetScanner.cpp
@@ -517,13 +517,11 @@ class SrcSafetyAnalysis {
public:
std::vector<MCInstReference>
getLastClobberingInsts(const MCInst &Inst, BinaryFunction &BF,
- std::optional<MCPhysReg> ClobberedReg) const {
- if (!ClobberedReg || RegsToTrackInstsFor.empty())
- return {};
+ MCPhysReg ClobberedReg) const {
const SrcState &S = getStateBefore(Inst);
std::vector<MCInstReference> Result;
- for (const MCInst *Inst : lastWritingInsts(S, *ClobberedReg)) {
+ for (const MCInst *Inst : lastWritingInsts(S, ClobberedReg)) {
MCInstReference Ref = MCInstReference::get(Inst, BF);
assert(Ref && "Expected Inst to be found");
Result.push_back(Ref);
@@ -871,16 +869,29 @@ void FunctionAnalysis::augmentUnsafeUseReports(
});
// Augment gadget reports.
- for (auto Report : Reports) {
+ for (auto &Report : Reports) {
MCInstReference Location = Report.Issue->Location;
LLVM_DEBUG({ traceInst(BC, "Attaching clobbering info to", Location); });
+ assert(Report.RequestedDetails &&
+ "Should be removed by handleSimpleReports");
auto DetailedInfo =
std::make_shared<ClobberingInfo>(Analysis->getLastClobberingInsts(
- Location, BF, Report.RequestedDetails));
+ Location, BF, *Report.RequestedDetails));
Result.Diagnostics.emplace_back(Report.Issue, DetailedInfo);
}
}
+void FunctionAnalysis::handleSimpleReports(
+ SmallVector<BriefReport<MCPhysReg>> &Reports) {
+ // Before re-running the detailed analysis, process the reports which do not
+ // need any additional details to be attached.
+ for (auto &Report : Reports) {
+ if (!Report.RequestedDetails)
+ Result.Diagnostics.emplace_back(Report.Issue, nullptr);
+ }
+ llvm::erase_if(Reports, [](const auto &R) { return !R.RequestedDetails; });
+}
+
void FunctionAnalysis::run() {
LLVM_DEBUG({
dbgs() << "Analyzing function " << BF.getPrintName()
@@ -890,6 +901,7 @@ void FunctionAnalysis::run() {
SmallVector<BriefReport<MCPhysReg>> UnsafeUses;
findUnsafeUses(UnsafeUses);
+ handleSimpleReports(UnsafeUses);
if (!UnsafeUses.empty())
augmentUnsafeUseReports(UnsafeUses);
}
diff --git a/bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s b/bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s
index 2193d40131478..284f0bea607a5 100644
--- a/bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s
+++ b/bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s
@@ -217,6 +217,7 @@ f_callclobbered_calleesaved:
f_unreachable_instruction:
// CHECK-LABEL: GS-PAUTH: Warning: unreachable instruction found in function f_unreachable_instruction, basic block {{[0-9a-zA-Z.]+}}, at address
// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: add x0, x1, x2
+// CHECK-NOT: instructions that write to the affected registers after any authentication are:
b 1f
add x0, x1, x2
1:
>From 67d84bcbafbe9b5ba735186baf09fc5cda898934 Mon Sep 17 00:00:00 2001
From: Anatoly Trosinenko <atrosinenko at accesssoftek.com>
Date: Tue, 22 Apr 2025 18:13:48 +0300
Subject: [PATCH 3/5] Drop redundant const qualifier from ArrayRef<T>
---
bolt/include/bolt/Passes/PAuthGadgetScanner.h | 5 ++---
bolt/lib/Passes/PAuthGadgetScanner.cpp | 6 +++---
2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/bolt/include/bolt/Passes/PAuthGadgetScanner.h b/bolt/include/bolt/Passes/PAuthGadgetScanner.h
index 3b472f8bbdf4c..f7106b6518ba7 100644
--- a/bolt/include/bolt/Passes/PAuthGadgetScanner.h
+++ b/bolt/include/bolt/Passes/PAuthGadgetScanner.h
@@ -253,8 +253,7 @@ class ClobberingInfo : public ExtraInfo {
SmallVector<MCInstReference> ClobberingInstrs;
public:
- ClobberingInfo(const ArrayRef<MCInstReference> Instrs)
- : ClobberingInstrs(Instrs) {}
+ ClobberingInfo(ArrayRef<MCInstReference> Instrs) : ClobberingInstrs(Instrs) {}
void print(raw_ostream &OS, const MCInstReference Location) const override;
};
@@ -298,7 +297,7 @@ class FunctionAnalysis {
bool PacRetGadgetsOnly;
void findUnsafeUses(SmallVector<BriefReport<MCPhysReg>> &Reports);
- void augmentUnsafeUseReports(const ArrayRef<BriefReport<MCPhysReg>> Reports);
+ void augmentUnsafeUseReports(ArrayRef<BriefReport<MCPhysReg>> Reports);
/// Process the reports which do not have to be augmented, and remove them
/// from Reports.
diff --git a/bolt/lib/Passes/PAuthGadgetScanner.cpp b/bolt/lib/Passes/PAuthGadgetScanner.cpp
index 213d224433419..c316038c06aa6 100644
--- a/bolt/lib/Passes/PAuthGadgetScanner.cpp
+++ b/bolt/lib/Passes/PAuthGadgetScanner.cpp
@@ -812,7 +812,7 @@ template <typename T> static void iterateOverInstrs(BinaryFunction &BF, T Fn) {
}
static SmallVector<MCPhysReg>
-collectRegsToTrack(const ArrayRef<BriefReport<MCPhysReg>> Reports) {
+collectRegsToTrack(ArrayRef<BriefReport<MCPhysReg>> Reports) {
SmallSet<MCPhysReg, 4> RegsToTrack;
for (auto Report : Reports)
if (Report.RequestedDetails)
@@ -856,7 +856,7 @@ void FunctionAnalysis::findUnsafeUses(
}
void FunctionAnalysis::augmentUnsafeUseReports(
- const ArrayRef<BriefReport<MCPhysReg>> Reports) {
+ ArrayRef<BriefReport<MCPhysReg>> Reports) {
SmallVector<MCPhysReg> RegsToTrack = collectRegsToTrack(Reports);
// Re-compute the analysis with register tracking.
auto Analysis = SrcSafetyAnalysis::create(BF, AllocatorId, RegsToTrack);
@@ -974,7 +974,7 @@ void GadgetReport::generateReport(raw_ostream &OS,
}
static void printRelatedInstrs(raw_ostream &OS, const MCInstReference Location,
- const ArrayRef<MCInstReference> RelatedInstrs) {
+ ArrayRef<MCInstReference> RelatedInstrs) {
const BinaryFunction &BF = *Location.getFunction();
const BinaryContext &BC = BF.getBinaryContext();
>From 1b200d2871315c8f47d1b305fd6452a319615767 Mon Sep 17 00:00:00 2001
From: Anatoly Trosinenko <atrosinenko at accesssoftek.com>
Date: Fri, 25 Apr 2025 23:05:08 +0300
Subject: [PATCH 4/5] Rename classes, add an explanation
---
bolt/include/bolt/Passes/PAuthGadgetScanner.h | 79 ++++++++++++-------
bolt/lib/Passes/PAuthGadgetScanner.cpp | 28 +++----
2 files changed, 66 insertions(+), 41 deletions(-)
diff --git a/bolt/include/bolt/Passes/PAuthGadgetScanner.h b/bolt/include/bolt/Passes/PAuthGadgetScanner.h
index f7106b6518ba7..ed3ba731c7240 100644
--- a/bolt/include/bolt/Passes/PAuthGadgetScanner.h
+++ b/bolt/include/bolt/Passes/PAuthGadgetScanner.h
@@ -196,23 +196,48 @@ raw_ostream &operator<<(raw_ostream &OS, const MCInstReference &);
namespace PAuthGadgetScanner {
+// The report classes are designed to be used in an immutable manner.
+// When an issue report is constructed in multiple steps, an attempt is made
+// to distinguish intermediate and final results at the type level.
+//
+// Here is an overview of issue life-cycle:
+// * an analysis (SrcSafetyAnalysis at now, DstSafetyAnalysis will be added
+// later to support the detection of authentication oracles) computes register
+// state for each instruction in the function.
+// * each instruction is checked to be a gadget of some kind, taking the
+// computed state into account. If a gadget is found, its kind and location
+// are stored into a subclass of Diagnostic wrapped into BriefReport<ReqT>.
+// * if any issue is to be reported for the function, the same analysis is
+// re-run to collect extra information to provide to the user. Which extra
+// information can be requested depends on the particular analysis (for
+// example, SrcSafetyAnalysis is able to compute the set of instructions
+// clobbering the particular register, thus ReqT is MCPhysReg). At this stage,
+// `FinalReport`s are created.
+//
+// Here, the subclasses of Diagnostic store the pieces of information which
+// are kept unchanged since they are collected on the first run of the analysis.
+// BriefReport<T>::RequestedDetails, on the other hand, is replaced with
+// FinalReport::Details computed by the second run of the analysis.
+
/// Description of a gadget kind that can be detected. Intended to be
-/// statically allocated to be attached to reports by reference.
+/// statically allocated and attached to reports by reference.
class GadgetKind {
const char *Description;
public:
+ /// Wraps a description string which must be a string literal.
GadgetKind(const char *Description) : Description(Description) {}
StringRef getDescription() const { return Description; }
};
-/// Base report located at some instruction, without any additional information.
-struct Report {
+/// Basic diagnostic information, which is kept unchanged since it is collected
+/// on the first run of the analysis.
+struct Diagnostic {
MCInstReference Location;
- Report(MCInstReference Location) : Location(Location) {}
- virtual ~Report() {}
+ Diagnostic(MCInstReference Location) : Location(Location) {}
+ virtual ~Diagnostic() {}
virtual void generateReport(raw_ostream &OS,
const BinaryContext &BC) const = 0;
@@ -221,27 +246,27 @@ struct Report {
StringRef IssueKind) const;
};
-struct GadgetReport : public Report {
+struct GadgetDiagnostic : public Diagnostic {
// The particular kind of gadget that is detected.
const GadgetKind &Kind;
- GadgetReport(const GadgetKind &Kind, MCInstReference Location)
- : Report(Location), Kind(Kind) {}
+ GadgetDiagnostic(const GadgetKind &Kind, MCInstReference Location)
+ : Diagnostic(Location), Kind(Kind) {}
void generateReport(raw_ostream &OS, const BinaryContext &BC) const override;
};
/// Report with a free-form message attached.
-struct GenericReport : public Report {
+struct GenericDiagnostic : public Diagnostic {
std::string Text;
- GenericReport(MCInstReference Location, StringRef Text)
- : Report(Location), Text(Text) {}
+ GenericDiagnostic(MCInstReference Location, StringRef Text)
+ : Diagnostic(Location), Text(Text) {}
virtual void generateReport(raw_ostream &OS,
const BinaryContext &BC) const override;
};
/// An information about an issue collected on the slower, detailed,
-/// run of an analysis.
+/// run of the analysis.
class ExtraInfo {
public:
virtual void print(raw_ostream &OS, const MCInstReference Location) const = 0;
@@ -260,35 +285,34 @@ class ClobberingInfo : public ExtraInfo {
/// A brief version of a report that can be further augmented with the details.
///
-/// It is common for a particular type of gadget detector to be tied to some
-/// specific kind of analysis. If an issue is returned by that detector, it may
-/// be further augmented with the detailed info in an analysis-specific way,
-/// or just be left as-is (f.e. if a free-form warning was reported).
+/// A half-baked report produced on the first run of the analysis. An extra,
+/// analysis-specific information may be requested to be collected on the
+/// second run.
template <typename T> struct BriefReport {
- BriefReport(std::shared_ptr<Report> Issue,
+ BriefReport(std::shared_ptr<Diagnostic> Issue,
const std::optional<T> RequestedDetails)
: Issue(Issue), RequestedDetails(RequestedDetails) {}
- std::shared_ptr<Report> Issue;
+ std::shared_ptr<Diagnostic> Issue;
std::optional<T> RequestedDetails;
};
-/// A detailed version of a report.
-struct DetailedReport {
- DetailedReport(std::shared_ptr<Report> Issue,
- std::shared_ptr<ExtraInfo> Details)
+/// A final version of the report.
+struct FinalReport {
+ FinalReport(std::shared_ptr<Diagnostic> Issue,
+ std::shared_ptr<ExtraInfo> Details)
: Issue(Issue), Details(Details) {}
- std::shared_ptr<Report> Issue;
+ std::shared_ptr<Diagnostic> Issue;
std::shared_ptr<ExtraInfo> Details;
};
struct FunctionAnalysisResult {
- std::vector<DetailedReport> Diagnostics;
+ std::vector<FinalReport> Diagnostics;
};
/// A helper class storing per-function context to be instantiated by Analysis.
-class FunctionAnalysis {
+class FunctionAnalysisContext {
BinaryContext &BC;
BinaryFunction &BF;
MCPlusBuilder::AllocatorIdTy AllocatorId;
@@ -304,8 +328,9 @@ class FunctionAnalysis {
void handleSimpleReports(SmallVector<BriefReport<MCPhysReg>> &Reports);
public:
- FunctionAnalysis(BinaryFunction &BF, MCPlusBuilder::AllocatorIdTy AllocatorId,
- bool PacRetGadgetsOnly)
+ FunctionAnalysisContext(BinaryFunction &BF,
+ MCPlusBuilder::AllocatorIdTy AllocatorId,
+ bool PacRetGadgetsOnly)
: BC(BF.getBinaryContext()), BF(BF), AllocatorId(AllocatorId),
PacRetGadgetsOnly(PacRetGadgetsOnly) {}
diff --git a/bolt/lib/Passes/PAuthGadgetScanner.cpp b/bolt/lib/Passes/PAuthGadgetScanner.cpp
index c316038c06aa6..7eb9096f47d7f 100644
--- a/bolt/lib/Passes/PAuthGadgetScanner.cpp
+++ b/bolt/lib/Passes/PAuthGadgetScanner.cpp
@@ -715,7 +715,7 @@ SrcSafetyAnalysis::create(BinaryFunction &BF,
static BriefReport<MCPhysReg> make_generic_report(MCInstReference Location,
StringRef Text) {
- auto Report = std::make_shared<GenericReport>(Location, Text);
+ auto Report = std::make_shared<GenericDiagnostic>(Location, Text);
return BriefReport<MCPhysReg>(Report, std::nullopt);
}
@@ -723,7 +723,7 @@ template <typename T>
static BriefReport<T> make_report(const GadgetKind &Kind,
MCInstReference Location,
T RequestedDetails) {
- auto Report = std::make_shared<GadgetReport>(Kind, Location);
+ auto Report = std::make_shared<GadgetDiagnostic>(Kind, Location);
return BriefReport<T>(Report, RequestedDetails);
}
@@ -821,7 +821,7 @@ collectRegsToTrack(ArrayRef<BriefReport<MCPhysReg>> Reports) {
return SmallVector<MCPhysReg>(RegsToTrack.begin(), RegsToTrack.end());
}
-void FunctionAnalysis::findUnsafeUses(
+void FunctionAnalysisContext::findUnsafeUses(
SmallVector<BriefReport<MCPhysReg>> &Reports) {
auto Analysis = SrcSafetyAnalysis::create(BF, AllocatorId, {});
LLVM_DEBUG({ dbgs() << "Running src register safety analysis...\n"; });
@@ -855,7 +855,7 @@ void FunctionAnalysis::findUnsafeUses(
});
}
-void FunctionAnalysis::augmentUnsafeUseReports(
+void FunctionAnalysisContext::augmentUnsafeUseReports(
ArrayRef<BriefReport<MCPhysReg>> Reports) {
SmallVector<MCPhysReg> RegsToTrack = collectRegsToTrack(Reports);
// Re-compute the analysis with register tracking.
@@ -881,7 +881,7 @@ void FunctionAnalysis::augmentUnsafeUseReports(
}
}
-void FunctionAnalysis::handleSimpleReports(
+void FunctionAnalysisContext::handleSimpleReports(
SmallVector<BriefReport<MCPhysReg>> &Reports) {
// Before re-running the detailed analysis, process the reports which do not
// need any additional details to be attached.
@@ -892,7 +892,7 @@ void FunctionAnalysis::handleSimpleReports(
llvm::erase_if(Reports, [](const auto &R) { return !R.RequestedDetails; });
}
-void FunctionAnalysis::run() {
+void FunctionAnalysisContext::run() {
LLVM_DEBUG({
dbgs() << "Analyzing function " << BF.getPrintName()
<< ", AllocatorId = " << AllocatorId << "\n";
@@ -908,7 +908,7 @@ void FunctionAnalysis::run() {
void Analysis::runOnFunction(BinaryFunction &BF,
MCPlusBuilder::AllocatorIdTy AllocatorId) {
- FunctionAnalysis FA(BF, AllocatorId, PacRetGadgetsOnly);
+ FunctionAnalysisContext FA(BF, AllocatorId, PacRetGadgetsOnly);
FA.run();
const FunctionAnalysisResult &FAR = FA.getResult();
@@ -954,8 +954,8 @@ static void reportFoundGadgetInSingleBBSingleRelatedInst(
}
}
-void Report::printBasicInfo(raw_ostream &OS, const BinaryContext &BC,
- StringRef IssueKind) const {
+void Diagnostic::printBasicInfo(raw_ostream &OS, const BinaryContext &BC,
+ StringRef IssueKind) const {
BinaryFunction *BF = Location.getFunction();
BinaryBasicBlock *BB = Location.getBasicBlock();
@@ -968,8 +968,8 @@ void Report::printBasicInfo(raw_ostream &OS, const BinaryContext &BC,
BC.printInstruction(OS, Location, Location.getAddress(), BF);
}
-void GadgetReport::generateReport(raw_ostream &OS,
- const BinaryContext &BC) const {
+void GadgetDiagnostic::generateReport(raw_ostream &OS,
+ const BinaryContext &BC) const {
printBasicInfo(OS, BC, Kind.getDescription());
}
@@ -1007,8 +1007,8 @@ void ClobberingInfo::print(raw_ostream &OS,
printRelatedInstrs(OS, Location, ClobberingInstrs);
}
-void GenericReport::generateReport(raw_ostream &OS,
- const BinaryContext &BC) const {
+void GenericDiagnostic::generateReport(raw_ostream &OS,
+ const BinaryContext &BC) const {
printBasicInfo(OS, BC, Text);
}
@@ -1029,7 +1029,7 @@ Error Analysis::runOnFunctions(BinaryContext &BC) {
for (BinaryFunction *BF : BC.getAllBinaryFunctions()) {
if (!AnalysisResults.count(BF))
continue;
- for (const DetailedReport &R : AnalysisResults[BF].Diagnostics) {
+ for (const FinalReport &R : AnalysisResults[BF].Diagnostics) {
R.Issue->generateReport(outs(), BC);
if (R.Details)
R.Details->print(outs(), R.Issue->Location);
>From cab66b6dd3a83d7f91ab03e8dc8318bd241bfa85 Mon Sep 17 00:00:00 2001
From: Anatoly Trosinenko <atrosinenko at accesssoftek.com>
Date: Tue, 29 Apr 2025 16:35:30 +0300
Subject: [PATCH 5/5] Address the comments
---
bolt/include/bolt/Passes/PAuthGadgetScanner.h | 23 +++++------
bolt/lib/Passes/PAuthGadgetScanner.cpp | 38 ++++++++++---------
2 files changed, 32 insertions(+), 29 deletions(-)
diff --git a/bolt/include/bolt/Passes/PAuthGadgetScanner.h b/bolt/include/bolt/Passes/PAuthGadgetScanner.h
index ed3ba731c7240..98a49df862ebd 100644
--- a/bolt/include/bolt/Passes/PAuthGadgetScanner.h
+++ b/bolt/include/bolt/Passes/PAuthGadgetScanner.h
@@ -204,9 +204,10 @@ namespace PAuthGadgetScanner {
// * an analysis (SrcSafetyAnalysis at now, DstSafetyAnalysis will be added
// later to support the detection of authentication oracles) computes register
// state for each instruction in the function.
-// * each instruction is checked to be a gadget of some kind, taking the
-// computed state into account. If a gadget is found, its kind and location
-// are stored into a subclass of Diagnostic wrapped into BriefReport<ReqT>.
+// * for each instruction, it is checked whether it is a gadget of some kind,
+// taking the computed state into account. If a gadget is found, its kind
+// and location are stored into a subclass of Diagnostic wrapped into
+// PartialReport<ReqT>.
// * if any issue is to be reported for the function, the same analysis is
// re-run to collect extra information to provide to the user. Which extra
// information can be requested depends on the particular analysis (for
@@ -216,7 +217,7 @@ namespace PAuthGadgetScanner {
//
// Here, the subclasses of Diagnostic store the pieces of information which
// are kept unchanged since they are collected on the first run of the analysis.
-// BriefReport<T>::RequestedDetails, on the other hand, is replaced with
+// PartialReport<T>::RequestedDetails, on the other hand, is replaced with
// FinalReport::Details computed by the second run of the analysis.
/// Description of a gadget kind that can be detected. Intended to be
@@ -265,7 +266,7 @@ struct GenericDiagnostic : public Diagnostic {
const BinaryContext &BC) const override;
};
-/// An information about an issue collected on the slower, detailed,
+/// Extra information about an issue collected on the slower, detailed,
/// run of the analysis.
class ExtraInfo {
public:
@@ -288,9 +289,9 @@ class ClobberingInfo : public ExtraInfo {
/// A half-baked report produced on the first run of the analysis. An extra,
/// analysis-specific information may be requested to be collected on the
/// second run.
-template <typename T> struct BriefReport {
- BriefReport(std::shared_ptr<Diagnostic> Issue,
- const std::optional<T> RequestedDetails)
+template <typename T> struct PartialReport {
+ PartialReport(std::shared_ptr<Diagnostic> Issue,
+ const std::optional<T> RequestedDetails)
: Issue(Issue), RequestedDetails(RequestedDetails) {}
std::shared_ptr<Diagnostic> Issue;
@@ -320,12 +321,12 @@ class FunctionAnalysisContext {
bool PacRetGadgetsOnly;
- void findUnsafeUses(SmallVector<BriefReport<MCPhysReg>> &Reports);
- void augmentUnsafeUseReports(ArrayRef<BriefReport<MCPhysReg>> Reports);
+ void findUnsafeUses(SmallVector<PartialReport<MCPhysReg>> &Reports);
+ void augmentUnsafeUseReports(ArrayRef<PartialReport<MCPhysReg>> Reports);
/// Process the reports which do not have to be augmented, and remove them
/// from Reports.
- void handleSimpleReports(SmallVector<BriefReport<MCPhysReg>> &Reports);
+ void handleSimpleReports(SmallVector<PartialReport<MCPhysReg>> &Reports);
public:
FunctionAnalysisContext(BinaryFunction &BF,
diff --git a/bolt/lib/Passes/PAuthGadgetScanner.cpp b/bolt/lib/Passes/PAuthGadgetScanner.cpp
index 7eb9096f47d7f..25a3153759a58 100644
--- a/bolt/lib/Passes/PAuthGadgetScanner.cpp
+++ b/bolt/lib/Passes/PAuthGadgetScanner.cpp
@@ -713,21 +713,23 @@ SrcSafetyAnalysis::create(BinaryFunction &BF,
RegsToTrackInstsFor);
}
-static BriefReport<MCPhysReg> make_generic_report(MCInstReference Location,
- StringRef Text) {
+// This function could return PartialReport<T>, but currently T is always
+// MCPhysReg, even though it is an implementation detail.
+static PartialReport<MCPhysReg> make_generic_report(MCInstReference Location,
+ StringRef Text) {
auto Report = std::make_shared<GenericDiagnostic>(Location, Text);
- return BriefReport<MCPhysReg>(Report, std::nullopt);
+ return PartialReport<MCPhysReg>(Report, std::nullopt);
}
template <typename T>
-static BriefReport<T> make_report(const GadgetKind &Kind,
- MCInstReference Location,
- T RequestedDetails) {
+static PartialReport<T> make_gadget_report(const GadgetKind &Kind,
+ MCInstReference Location,
+ T RequestedDetails) {
auto Report = std::make_shared<GadgetDiagnostic>(Kind, Location);
- return BriefReport<T>(Report, RequestedDetails);
+ return PartialReport<T>(Report, RequestedDetails);
}
-static std::optional<BriefReport<MCPhysReg>>
+static std::optional<PartialReport<MCPhysReg>>
shouldReportReturnGadget(const BinaryContext &BC, const MCInstReference &Inst,
const SrcState &S) {
static const GadgetKind RetKind("non-protected ret found");
@@ -752,10 +754,10 @@ shouldReportReturnGadget(const BinaryContext &BC, const MCInstReference &Inst,
if (S.SafeToDerefRegs[RetReg])
return std::nullopt;
- return make_report(RetKind, Inst, RetReg);
+ return make_gadget_report(RetKind, Inst, RetReg);
}
-static std::optional<BriefReport<MCPhysReg>>
+static std::optional<PartialReport<MCPhysReg>>
shouldReportCallGadget(const BinaryContext &BC, const MCInstReference &Inst,
const SrcState &S) {
static const GadgetKind CallKind("non-protected call found");
@@ -777,10 +779,10 @@ shouldReportCallGadget(const BinaryContext &BC, const MCInstReference &Inst,
if (S.SafeToDerefRegs[DestReg])
return std::nullopt;
- return make_report(CallKind, Inst, DestReg);
+ return make_gadget_report(CallKind, Inst, DestReg);
}
-static std::optional<BriefReport<MCPhysReg>>
+static std::optional<PartialReport<MCPhysReg>>
shouldReportSigningOracle(const BinaryContext &BC, const MCInstReference &Inst,
const SrcState &S) {
static const GadgetKind SigningOracleKind("signing oracle found");
@@ -797,7 +799,7 @@ shouldReportSigningOracle(const BinaryContext &BC, const MCInstReference &Inst,
if (S.TrustedRegs[SignedReg])
return std::nullopt;
- return make_report(SigningOracleKind, Inst, SignedReg);
+ return make_gadget_report(SigningOracleKind, Inst, SignedReg);
}
template <typename T> static void iterateOverInstrs(BinaryFunction &BF, T Fn) {
@@ -812,7 +814,7 @@ template <typename T> static void iterateOverInstrs(BinaryFunction &BF, T Fn) {
}
static SmallVector<MCPhysReg>
-collectRegsToTrack(ArrayRef<BriefReport<MCPhysReg>> Reports) {
+collectRegsToTrack(ArrayRef<PartialReport<MCPhysReg>> Reports) {
SmallSet<MCPhysReg, 4> RegsToTrack;
for (auto Report : Reports)
if (Report.RequestedDetails)
@@ -822,7 +824,7 @@ collectRegsToTrack(ArrayRef<BriefReport<MCPhysReg>> Reports) {
}
void FunctionAnalysisContext::findUnsafeUses(
- SmallVector<BriefReport<MCPhysReg>> &Reports) {
+ SmallVector<PartialReport<MCPhysReg>> &Reports) {
auto Analysis = SrcSafetyAnalysis::create(BF, AllocatorId, {});
LLVM_DEBUG({ dbgs() << "Running src register safety analysis...\n"; });
Analysis->run();
@@ -856,7 +858,7 @@ void FunctionAnalysisContext::findUnsafeUses(
}
void FunctionAnalysisContext::augmentUnsafeUseReports(
- ArrayRef<BriefReport<MCPhysReg>> Reports) {
+ ArrayRef<PartialReport<MCPhysReg>> Reports) {
SmallVector<MCPhysReg> RegsToTrack = collectRegsToTrack(Reports);
// Re-compute the analysis with register tracking.
auto Analysis = SrcSafetyAnalysis::create(BF, AllocatorId, RegsToTrack);
@@ -882,7 +884,7 @@ void FunctionAnalysisContext::augmentUnsafeUseReports(
}
void FunctionAnalysisContext::handleSimpleReports(
- SmallVector<BriefReport<MCPhysReg>> &Reports) {
+ SmallVector<PartialReport<MCPhysReg>> &Reports) {
// Before re-running the detailed analysis, process the reports which do not
// need any additional details to be attached.
for (auto &Report : Reports) {
@@ -899,7 +901,7 @@ void FunctionAnalysisContext::run() {
BF.dump();
});
- SmallVector<BriefReport<MCPhysReg>> UnsafeUses;
+ SmallVector<PartialReport<MCPhysReg>> UnsafeUses;
findUnsafeUses(UnsafeUses);
handleSimpleReports(UnsafeUses);
if (!UnsafeUses.empty())
More information about the llvm-branch-commits
mailing list