Skip to content

Commit d0235cf

Browse files
committed
[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.
1 parent 14706d6 commit d0235cf

File tree

3 files changed

+187
-123
lines changed

3 files changed

+187
-123
lines changed

bolt/include/bolt/Passes/PAuthGadgetScanner.h

Lines changed: 71 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -217,39 +217,18 @@ struct Report {
217217
virtual void generateReport(raw_ostream &OS,
218218
const BinaryContext &BC) const = 0;
219219

220-
// The two methods below are called by Analysis::computeDetailedInfo when
221-
// iterating over the reports.
222-
virtual ArrayRef<MCPhysReg> getAffectedRegisters() const { return {}; }
223-
virtual void setOverwritingInstrs(ArrayRef<MCInstReference> Instrs) {}
224-
225220
void printBasicInfo(raw_ostream &OS, const BinaryContext &BC,
226221
StringRef IssueKind) const;
227222
};
228223

229224
struct GadgetReport : public Report {
230225
// The particular kind of gadget that is detected.
231226
const GadgetKind &Kind;
232-
// The set of registers related to this gadget report (possibly empty).
233-
SmallVector<MCPhysReg, 1> AffectedRegisters;
234-
// The instructions that clobber the affected registers.
235-
// There is no one-to-one correspondence with AffectedRegisters: for example,
236-
// the same register can be overwritten by different instructions in different
237-
// preceding basic blocks.
238-
SmallVector<MCInstReference> OverwritingInstrs;
239-
240-
GadgetReport(const GadgetKind &Kind, MCInstReference Location,
241-
MCPhysReg AffectedRegister)
242-
: Report(Location), Kind(Kind), AffectedRegisters({AffectedRegister}) {}
243-
244-
void generateReport(raw_ostream &OS, const BinaryContext &BC) const override;
245227

246-
ArrayRef<MCPhysReg> getAffectedRegisters() const override {
247-
return AffectedRegisters;
248-
}
228+
GadgetReport(const GadgetKind &Kind, MCInstReference Location)
229+
: Report(Location), Kind(Kind) {}
249230

250-
void setOverwritingInstrs(ArrayRef<MCInstReference> Instrs) override {
251-
OverwritingInstrs.assign(Instrs.begin(), Instrs.end());
252-
}
231+
void generateReport(raw_ostream &OS, const BinaryContext &BC) const override;
253232
};
254233

255234
/// Report with a free-form message attached.
@@ -261,8 +240,75 @@ struct GenericReport : public Report {
261240
const BinaryContext &BC) const override;
262241
};
263242

243+
/// An information about an issue collected on the slower, detailed,
244+
/// run of an analysis.
245+
class ExtraInfo {
246+
public:
247+
virtual void print(raw_ostream &OS, const MCInstReference Location) const = 0;
248+
249+
virtual ~ExtraInfo() {}
250+
};
251+
252+
class ClobberingInfo : public ExtraInfo {
253+
SmallVector<MCInstReference> ClobberingInstrs;
254+
255+
public:
256+
ClobberingInfo(const ArrayRef<MCInstReference> Instrs)
257+
: ClobberingInstrs(Instrs) {}
258+
259+
void print(raw_ostream &OS, const MCInstReference Location) const override;
260+
};
261+
262+
/// A brief version of a report that can be further augmented with the details.
263+
///
264+
/// It is common for a particular type of gadget detector to be tied to some
265+
/// specific kind of analysis. If an issue is returned by that detector, it may
266+
/// be further augmented with the detailed info in an analysis-specific way,
267+
/// or just be left as-is (f.e. if a free-form warning was reported).
268+
template <typename T> struct BriefReport {
269+
BriefReport(std::shared_ptr<Report> Issue,
270+
const std::optional<T> RequestedDetails)
271+
: Issue(Issue), RequestedDetails(RequestedDetails) {}
272+
273+
std::shared_ptr<Report> Issue;
274+
std::optional<T> RequestedDetails;
275+
};
276+
277+
/// A detailed version of a report.
278+
struct DetailedReport {
279+
DetailedReport(std::shared_ptr<Report> Issue,
280+
std::shared_ptr<ExtraInfo> Details)
281+
: Issue(Issue), Details(Details) {}
282+
283+
std::shared_ptr<Report> Issue;
284+
std::shared_ptr<ExtraInfo> Details;
285+
};
286+
264287
struct FunctionAnalysisResult {
265-
std::vector<std::shared_ptr<Report>> Diagnostics;
288+
std::vector<DetailedReport> Diagnostics;
289+
};
290+
291+
/// A helper class storing per-function context to be instantiated by Analysis.
292+
class FunctionAnalysis {
293+
BinaryContext &BC;
294+
BinaryFunction &BF;
295+
MCPlusBuilder::AllocatorIdTy AllocatorId;
296+
FunctionAnalysisResult Result;
297+
298+
bool PacRetGadgetsOnly;
299+
300+
void findUnsafeUses(SmallVector<BriefReport<MCPhysReg>> &Reports);
301+
void augmentUnsafeUseReports(const ArrayRef<BriefReport<MCPhysReg>> Reports);
302+
303+
public:
304+
FunctionAnalysis(BinaryFunction &BF, MCPlusBuilder::AllocatorIdTy AllocatorId,
305+
bool PacRetGadgetsOnly)
306+
: BC(BF.getBinaryContext()), BF(BF), AllocatorId(AllocatorId),
307+
PacRetGadgetsOnly(PacRetGadgetsOnly) {}
308+
309+
void run();
310+
311+
const FunctionAnalysisResult &getResult() const { return Result; }
266312
};
267313

268314
class Analysis : public BinaryFunctionPass {
@@ -271,12 +317,6 @@ class Analysis : public BinaryFunctionPass {
271317

272318
void runOnFunction(BinaryFunction &Function,
273319
MCPlusBuilder::AllocatorIdTy AllocatorId);
274-
FunctionAnalysisResult findGadgets(BinaryFunction &BF,
275-
MCPlusBuilder::AllocatorIdTy AllocatorId);
276-
277-
void computeDetailedInfo(BinaryFunction &BF,
278-
MCPlusBuilder::AllocatorIdTy AllocatorId,
279-
FunctionAnalysisResult &Result);
280320

281321
std::map<const BinaryFunction *, FunctionAnalysisResult> AnalysisResults;
282322
std::mutex AnalysisResultsMutex;

0 commit comments

Comments
 (0)