Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 1a4a3a2

Browse filesBrowse files
atrosinenkoatrosinenko
committed
Address the comments
1 parent 21f15ce commit 1a4a3a2
Copy full SHA for 1a4a3a2

File tree

2 files changed

+32
-29
lines changed
Filter options

2 files changed

+32
-29
lines changed

‎bolt/include/bolt/Passes/PAuthGadgetScanner.h

Copy file name to clipboardExpand all lines: bolt/include/bolt/Passes/PAuthGadgetScanner.h
+12-11Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -206,9 +206,10 @@ namespace PAuthGadgetScanner {
206206
// * an analysis (SrcSafetyAnalysis at now, DstSafetyAnalysis will be added
207207
// later to support the detection of authentication oracles) computes register
208208
// state for each instruction in the function.
209-
// * each instruction is checked to be a gadget of some kind, taking the
210-
// computed state into account. If a gadget is found, its kind and location
211-
// are stored into a subclass of Diagnostic wrapped into BriefReport<ReqT>.
209+
// * for each instruction, it is checked whether it is a gadget of some kind,
210+
// taking the computed state into account. If a gadget is found, its kind
211+
// and location are stored into a subclass of Diagnostic wrapped into
212+
// PartialReport<ReqT>.
212213
// * if any issue is to be reported for the function, the same analysis is
213214
// re-run to collect extra information to provide to the user. Which extra
214215
// information can be requested depends on the particular analysis (for
@@ -218,7 +219,7 @@ namespace PAuthGadgetScanner {
218219
//
219220
// Here, the subclasses of Diagnostic store the pieces of information which
220221
// are kept unchanged since they are collected on the first run of the analysis.
221-
// BriefReport<T>::RequestedDetails, on the other hand, is replaced with
222+
// PartialReport<T>::RequestedDetails, on the other hand, is replaced with
222223
// FinalReport::Details computed by the second run of the analysis.
223224

224225
/// Description of a gadget kind that can be detected. Intended to be
@@ -267,7 +268,7 @@ struct GenericDiagnostic : public Diagnostic {
267268
const BinaryContext &BC) const override;
268269
};
269270

270-
/// An information about an issue collected on the slower, detailed,
271+
/// Extra information about an issue collected on the slower, detailed,
271272
/// run of the analysis.
272273
class ExtraInfo {
273274
public:
@@ -290,9 +291,9 @@ class ClobberingInfo : public ExtraInfo {
290291
/// A half-baked report produced on the first run of the analysis. An extra,
291292
/// analysis-specific information may be requested to be collected on the
292293
/// second run.
293-
template <typename T> struct BriefReport {
294-
BriefReport(std::shared_ptr<Diagnostic> Issue,
295-
const std::optional<T> RequestedDetails)
294+
template <typename T> struct PartialReport {
295+
PartialReport(std::shared_ptr<Diagnostic> Issue,
296+
const std::optional<T> RequestedDetails)
296297
: Issue(Issue), RequestedDetails(RequestedDetails) {}
297298

298299
std::shared_ptr<Diagnostic> Issue;
@@ -322,12 +323,12 @@ class FunctionAnalysisContext {
322323

323324
bool PacRetGadgetsOnly;
324325

325-
void findUnsafeUses(SmallVector<BriefReport<MCPhysReg>> &Reports);
326-
void augmentUnsafeUseReports(ArrayRef<BriefReport<MCPhysReg>> Reports);
326+
void findUnsafeUses(SmallVector<PartialReport<MCPhysReg>> &Reports);
327+
void augmentUnsafeUseReports(ArrayRef<PartialReport<MCPhysReg>> Reports);
327328

328329
/// Process the reports which do not have to be augmented, and remove them
329330
/// from Reports.
330-
void handleSimpleReports(SmallVector<BriefReport<MCPhysReg>> &Reports);
331+
void handleSimpleReports(SmallVector<PartialReport<MCPhysReg>> &Reports);
331332

332333
public:
333334
FunctionAnalysisContext(BinaryFunction &BF,

‎bolt/lib/Passes/PAuthGadgetScanner.cpp

Copy file name to clipboardExpand all lines: bolt/lib/Passes/PAuthGadgetScanner.cpp
+20-18Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -713,21 +713,23 @@ SrcSafetyAnalysis::create(BinaryFunction &BF,
713713
RegsToTrackInstsFor);
714714
}
715715

716-
static BriefReport<MCPhysReg> make_generic_report(MCInstReference Location,
717-
StringRef Text) {
716+
// This function could return PartialReport<T>, but currently T is always
717+
// MCPhysReg, even though it is an implementation detail.
718+
static PartialReport<MCPhysReg> make_generic_report(MCInstReference Location,
719+
StringRef Text) {
718720
auto Report = std::make_shared<GenericDiagnostic>(Location, Text);
719-
return BriefReport<MCPhysReg>(Report, std::nullopt);
721+
return PartialReport<MCPhysReg>(Report, std::nullopt);
720722
}
721723

722724
template <typename T>
723-
static BriefReport<T> make_report(const GadgetKind &Kind,
724-
MCInstReference Location,
725-
T RequestedDetails) {
725+
static PartialReport<T> make_gadget_report(const GadgetKind &Kind,
726+
MCInstReference Location,
727+
T RequestedDetails) {
726728
auto Report = std::make_shared<GadgetDiagnostic>(Kind, Location);
727-
return BriefReport<T>(Report, RequestedDetails);
729+
return PartialReport<T>(Report, RequestedDetails);
728730
}
729731

730-
static std::optional<BriefReport<MCPhysReg>>
732+
static std::optional<PartialReport<MCPhysReg>>
731733
shouldReportReturnGadget(const BinaryContext &BC, const MCInstReference &Inst,
732734
const SrcState &S) {
733735
static const GadgetKind RetKind("non-protected ret found");
@@ -752,10 +754,10 @@ shouldReportReturnGadget(const BinaryContext &BC, const MCInstReference &Inst,
752754
if (S.SafeToDerefRegs[RetReg])
753755
return std::nullopt;
754756

755-
return make_report(RetKind, Inst, RetReg);
757+
return make_gadget_report(RetKind, Inst, RetReg);
756758
}
757759

758-
static std::optional<BriefReport<MCPhysReg>>
760+
static std::optional<PartialReport<MCPhysReg>>
759761
shouldReportCallGadget(const BinaryContext &BC, const MCInstReference &Inst,
760762
const SrcState &S) {
761763
static const GadgetKind CallKind("non-protected call found");
@@ -777,10 +779,10 @@ shouldReportCallGadget(const BinaryContext &BC, const MCInstReference &Inst,
777779
if (S.SafeToDerefRegs[DestReg])
778780
return std::nullopt;
779781

780-
return make_report(CallKind, Inst, DestReg);
782+
return make_gadget_report(CallKind, Inst, DestReg);
781783
}
782784

783-
static std::optional<BriefReport<MCPhysReg>>
785+
static std::optional<PartialReport<MCPhysReg>>
784786
shouldReportSigningOracle(const BinaryContext &BC, const MCInstReference &Inst,
785787
const SrcState &S) {
786788
static const GadgetKind SigningOracleKind("signing oracle found");
@@ -797,7 +799,7 @@ shouldReportSigningOracle(const BinaryContext &BC, const MCInstReference &Inst,
797799
if (S.TrustedRegs[SignedReg])
798800
return std::nullopt;
799801

800-
return make_report(SigningOracleKind, Inst, SignedReg);
802+
return make_gadget_report(SigningOracleKind, Inst, SignedReg);
801803
}
802804

803805
template <typename T> static void iterateOverInstrs(BinaryFunction &BF, T Fn) {
@@ -812,7 +814,7 @@ template <typename T> static void iterateOverInstrs(BinaryFunction &BF, T Fn) {
812814
}
813815

814816
static SmallVector<MCPhysReg>
815-
collectRegsToTrack(ArrayRef<BriefReport<MCPhysReg>> Reports) {
817+
collectRegsToTrack(ArrayRef<PartialReport<MCPhysReg>> Reports) {
816818
SmallSet<MCPhysReg, 4> RegsToTrack;
817819
for (auto Report : Reports)
818820
if (Report.RequestedDetails)
@@ -822,7 +824,7 @@ collectRegsToTrack(ArrayRef<BriefReport<MCPhysReg>> Reports) {
822824
}
823825

824826
void FunctionAnalysisContext::findUnsafeUses(
825-
SmallVector<BriefReport<MCPhysReg>> &Reports) {
827+
SmallVector<PartialReport<MCPhysReg>> &Reports) {
826828
auto Analysis = SrcSafetyAnalysis::create(BF, AllocatorId, {});
827829
LLVM_DEBUG({ dbgs() << "Running src register safety analysis...\n"; });
828830
Analysis->run();
@@ -856,7 +858,7 @@ void FunctionAnalysisContext::findUnsafeUses(
856858
}
857859

858860
void FunctionAnalysisContext::augmentUnsafeUseReports(
859-
ArrayRef<BriefReport<MCPhysReg>> Reports) {
861+
ArrayRef<PartialReport<MCPhysReg>> Reports) {
860862
SmallVector<MCPhysReg> RegsToTrack = collectRegsToTrack(Reports);
861863
// Re-compute the analysis with register tracking.
862864
auto Analysis = SrcSafetyAnalysis::create(BF, AllocatorId, RegsToTrack);
@@ -882,7 +884,7 @@ void FunctionAnalysisContext::augmentUnsafeUseReports(
882884
}
883885

884886
void FunctionAnalysisContext::handleSimpleReports(
885-
SmallVector<BriefReport<MCPhysReg>> &Reports) {
887+
SmallVector<PartialReport<MCPhysReg>> &Reports) {
886888
// Before re-running the detailed analysis, process the reports which do not
887889
// need any additional details to be attached.
888890
for (auto &Report : Reports) {
@@ -899,7 +901,7 @@ void FunctionAnalysisContext::run() {
899901
BF.dump();
900902
});
901903

902-
SmallVector<BriefReport<MCPhysReg>> UnsafeUses;
904+
SmallVector<PartialReport<MCPhysReg>> UnsafeUses;
903905
findUnsafeUses(UnsafeUses);
904906
handleSimpleReports(UnsafeUses);
905907
if (!UnsafeUses.empty())

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.