Skip to content

Commit c82cab5

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 51373db commit c82cab5

File tree

3 files changed

+187
-123
lines changed

3 files changed

+187
-123
lines changed

bolt/include/bolt/Passes/PAuthGadgetScanner.h

+71-31
Original file line numberDiff line numberDiff line change
@@ -219,39 +219,18 @@ struct Report {
219219
virtual void generateReport(raw_ostream &OS,
220220
const BinaryContext &BC) const = 0;
221221

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

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

248-
const ArrayRef<MCPhysReg> getAffectedRegisters() const override {
249-
return AffectedRegisters;
250-
}
230+
GadgetReport(const GadgetKind &Kind, MCInstReference Location)
231+
: Report(Location), Kind(Kind) {}
251232

252-
void setOverwritingInstrs(const ArrayRef<MCInstReference> Instrs) override {
253-
OverwritingInstrs.assign(Instrs.begin(), Instrs.end());
254-
}
233+
void generateReport(raw_ostream &OS, const BinaryContext &BC) const override;
255234
};
256235

257236
/// Report with a free-form message attached.
@@ -263,8 +242,75 @@ struct GenericReport : public Report {
263242
const BinaryContext &BC) const override;
264243
};
265244

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

270316
class Analysis : public BinaryFunctionPass {
@@ -273,12 +319,6 @@ class Analysis : public BinaryFunctionPass {
273319

274320
void runOnFunction(BinaryFunction &Function,
275321
MCPlusBuilder::AllocatorIdTy AllocatorId);
276-
FunctionAnalysisResult findGadgets(BinaryFunction &BF,
277-
MCPlusBuilder::AllocatorIdTy AllocatorId);
278-
279-
void computeDetailedInfo(BinaryFunction &BF,
280-
MCPlusBuilder::AllocatorIdTy AllocatorId,
281-
FunctionAnalysisResult &Result);
282322

283323
std::map<const BinaryFunction *, FunctionAnalysisResult> AnalysisResults;
284324
std::mutex AnalysisResultsMutex;

0 commit comments

Comments
 (0)