https://github.com/atrosinenko updated
https://github.com/llvm/llvm-project/pull/135662
>From 1419bfe2de741c5832de0ceb1b1765ba7b92022d Mon Sep 17 00:00:00 2001
From: Anatoly Trosinenko <atrosine...@accesssoftek.com>
Date: Mon, 14 Apr 2025 15:08:54 +0300
Subject: [PATCH 1/3] [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 4c1bef3d2265f..3b6c1f6af94a0 100644
--- a/bolt/include/bolt/Passes/PAuthGadgetScanner.h
+++ b/bolt/include/bolt/Passes/PAuthGadgetScanner.h
@@ -219,11 +219,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;
};
@@ -231,27 +226,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.
@@ -263,8 +242,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 {
@@ -273,12 +319,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 6590d1a58146f..1ccb8ccf56bb2 100644
--- a/bolt/lib/Passes/PAuthGadgetScanner.cpp
+++ b/bolt/lib/Passes/PAuthGadgetScanner.cpp
@@ -516,18 +516,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);
@@ -715,16 +710,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");
}
@@ -735,26 +744,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({
@@ -763,19 +772,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);
@@ -783,9 +792,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) {
@@ -799,11 +808,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();
@@ -812,45 +828,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();
@@ -860,32 +866,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
@@ -913,16 +924,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);
}
@@ -945,31 +954,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);
@@ -989,11 +1009,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,@function
main:
>From 6778309b9776554a4e6753218c1382c116104bcc Mon Sep 17 00:00:00 2001
From: Anatoly Trosinenko <atrosine...@accesssoftek.com>
Date: Fri, 18 Apr 2025 21:14:40 +0300
Subject: [PATCH 2/3] 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 3b6c1f6af94a0..3bcd27592c6af 100644
--- a/bolt/include/bolt/Passes/PAuthGadgetScanner.h
+++ b/bolt/include/bolt/Passes/PAuthGadgetScanner.h
@@ -302,6 +302,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 1ccb8ccf56bb2..466730099a443 100644
--- a/bolt/lib/Passes/PAuthGadgetScanner.cpp
+++ b/bolt/lib/Passes/PAuthGadgetScanner.cpp
@@ -516,13 +516,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);
@@ -866,16 +864,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()
@@ -885,6 +896,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 36c3cb9975ede4301df6f654782500de999ddb74 Mon Sep 17 00:00:00 2001
From: Anatoly Trosinenko <atrosine...@accesssoftek.com>
Date: Tue, 22 Apr 2025 18:13:48 +0300
Subject: [PATCH 3/3] 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 3bcd27592c6af..9a4e9598c9f98 100644
--- a/bolt/include/bolt/Passes/PAuthGadgetScanner.h
+++ b/bolt/include/bolt/Passes/PAuthGadgetScanner.h
@@ -255,8 +255,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;
};
@@ -300,7 +299,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 466730099a443..4e718c712aec9 100644
--- a/bolt/lib/Passes/PAuthGadgetScanner.cpp
+++ b/bolt/lib/Passes/PAuthGadgetScanner.cpp
@@ -807,7 +807,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)
@@ -851,7 +851,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);
@@ -969,7 +969,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();
_______________________________________________
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
