Skip to content

Commit f578f56

Browse files
authored
[BOLT] Gadget scanner: refactor issue reporting (#135662)
Remove `getAffectedRegisters` and `setOverwritingInstrs` methods from the base `Report` class. Instead, rename the `Report` class to `Diagnostic` and make it always represent the brief version of the report, which is kept unchanged since initially found. Throughout its life-cycle, an instance of `Diagnostic` is first wrapped into `PartialReport<ReqT>` together with an optional request for extra details. Then, on the second run of the analysis, it is re-wrapped into `FinalReport` together with the requested detailed information.
1 parent 882a01e commit f578f56

File tree

4 files changed

+247
-139
lines changed

4 files changed

+247
-139
lines changed

bolt/include/bolt/Passes/PAuthGadgetScanner.h

Lines changed: 109 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -196,73 +196,148 @@ raw_ostream &operator<<(raw_ostream &OS, const MCInstReference &);
196196

197197
namespace PAuthGadgetScanner {
198198

199+
// The report classes are designed to be used in an immutable manner.
200+
// When an issue report is constructed in multiple steps, an attempt is made
201+
// to distinguish intermediate and final results at the type level.
202+
//
203+
// Here is an overview of issue life-cycle:
204+
// * an analysis (SrcSafetyAnalysis at now, DstSafetyAnalysis will be added
205+
// later to support the detection of authentication oracles) computes register
206+
// state for each instruction in the function.
207+
// * for each instruction, it is checked whether it is a gadget of some kind,
208+
// taking the computed state into account. If a gadget is found, its kind
209+
// and location are stored into a subclass of Diagnostic wrapped into
210+
// PartialReport<ReqT>.
211+
// * if any issue is to be reported for the function, the same analysis is
212+
// re-run to collect extra information to provide to the user. Which extra
213+
// information can be requested depends on the particular analysis (for
214+
// example, SrcSafetyAnalysis is able to compute the set of instructions
215+
// clobbering the particular register, thus ReqT is MCPhysReg). At this stage,
216+
// `FinalReport`s are created.
217+
//
218+
// Here, the subclasses of Diagnostic store the pieces of information which
219+
// are kept unchanged since they are collected on the first run of the analysis.
220+
// PartialReport<T>::RequestedDetails, on the other hand, is replaced with
221+
// FinalReport::Details computed by the second run of the analysis.
222+
199223
/// Description of a gadget kind that can be detected. Intended to be
200-
/// statically allocated to be attached to reports by reference.
224+
/// statically allocated and attached to reports by reference.
201225
class GadgetKind {
202226
const char *Description;
203227

204228
public:
229+
/// Wraps a description string which must be a string literal.
205230
GadgetKind(const char *Description) : Description(Description) {}
206231

207232
StringRef getDescription() const { return Description; }
208233
};
209234

210-
/// Base report located at some instruction, without any additional information.
211-
struct Report {
235+
/// Basic diagnostic information, which is kept unchanged since it is collected
236+
/// on the first run of the analysis.
237+
struct Diagnostic {
212238
MCInstReference Location;
213239

214-
Report(MCInstReference Location) : Location(Location) {}
215-
virtual ~Report() {}
240+
Diagnostic(MCInstReference Location) : Location(Location) {}
241+
virtual ~Diagnostic() {}
216242

217243
virtual void generateReport(raw_ostream &OS,
218244
const BinaryContext &BC) const = 0;
219245

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-
225246
void printBasicInfo(raw_ostream &OS, const BinaryContext &BC,
226247
StringRef IssueKind) const;
227248
};
228249

229-
struct GadgetReport : public Report {
250+
struct GadgetDiagnostic : public Diagnostic {
230251
// The particular kind of gadget that is detected.
231252
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}) {}
243253

244-
void generateReport(raw_ostream &OS, const BinaryContext &BC) const override;
254+
GadgetDiagnostic(const GadgetKind &Kind, MCInstReference Location)
255+
: Diagnostic(Location), Kind(Kind) {}
245256

246-
ArrayRef<MCPhysReg> getAffectedRegisters() const override {
247-
return AffectedRegisters;
248-
}
249-
250-
void setOverwritingInstrs(ArrayRef<MCInstReference> Instrs) override {
251-
OverwritingInstrs.assign(Instrs.begin(), Instrs.end());
252-
}
257+
void generateReport(raw_ostream &OS, const BinaryContext &BC) const override;
253258
};
254259

255260
/// Report with a free-form message attached.
256-
struct GenericReport : public Report {
261+
struct GenericDiagnostic : public Diagnostic {
257262
std::string Text;
258-
GenericReport(MCInstReference Location, StringRef Text)
259-
: Report(Location), Text(Text) {}
263+
GenericDiagnostic(MCInstReference Location, StringRef Text)
264+
: Diagnostic(Location), Text(Text) {}
260265
virtual void generateReport(raw_ostream &OS,
261266
const BinaryContext &BC) const override;
262267
};
263268

269+
/// Extra information about an issue collected on the slower, detailed,
270+
/// run of the analysis.
271+
class ExtraInfo {
272+
public:
273+
virtual void print(raw_ostream &OS, const MCInstReference Location) const = 0;
274+
275+
virtual ~ExtraInfo() {}
276+
};
277+
278+
class ClobberingInfo : public ExtraInfo {
279+
SmallVector<MCInstReference> ClobberingInstrs;
280+
281+
public:
282+
ClobberingInfo(ArrayRef<MCInstReference> Instrs) : ClobberingInstrs(Instrs) {}
283+
284+
void print(raw_ostream &OS, const MCInstReference Location) const override;
285+
};
286+
287+
/// A brief version of a report that can be further augmented with the details.
288+
///
289+
/// A half-baked report produced on the first run of the analysis. An extra,
290+
/// analysis-specific information may be requested to be collected on the
291+
/// second run.
292+
template <typename T> struct PartialReport {
293+
PartialReport(std::shared_ptr<Diagnostic> Issue,
294+
const std::optional<T> RequestedDetails)
295+
: Issue(Issue), RequestedDetails(RequestedDetails) {}
296+
297+
std::shared_ptr<Diagnostic> Issue;
298+
std::optional<T> RequestedDetails;
299+
};
300+
301+
/// A final version of the report.
302+
struct FinalReport {
303+
FinalReport(std::shared_ptr<Diagnostic> Issue,
304+
std::shared_ptr<ExtraInfo> Details)
305+
: Issue(Issue), Details(Details) {}
306+
307+
std::shared_ptr<Diagnostic> Issue;
308+
std::shared_ptr<ExtraInfo> Details;
309+
};
310+
264311
struct FunctionAnalysisResult {
265-
std::vector<std::shared_ptr<Report>> Diagnostics;
312+
std::vector<FinalReport> Diagnostics;
313+
};
314+
315+
/// A helper class storing per-function context to be instantiated by Analysis.
316+
class FunctionAnalysisContext {
317+
BinaryContext &BC;
318+
BinaryFunction &BF;
319+
MCPlusBuilder::AllocatorIdTy AllocatorId;
320+
FunctionAnalysisResult Result;
321+
322+
bool PacRetGadgetsOnly;
323+
324+
void findUnsafeUses(SmallVector<PartialReport<MCPhysReg>> &Reports);
325+
void augmentUnsafeUseReports(ArrayRef<PartialReport<MCPhysReg>> Reports);
326+
327+
/// Process the reports which do not have to be augmented, and remove them
328+
/// from Reports.
329+
void handleSimpleReports(SmallVector<PartialReport<MCPhysReg>> &Reports);
330+
331+
public:
332+
FunctionAnalysisContext(BinaryFunction &BF,
333+
MCPlusBuilder::AllocatorIdTy AllocatorId,
334+
bool PacRetGadgetsOnly)
335+
: BC(BF.getBinaryContext()), BF(BF), AllocatorId(AllocatorId),
336+
PacRetGadgetsOnly(PacRetGadgetsOnly) {}
337+
338+
void run();
339+
340+
const FunctionAnalysisResult &getResult() const { return Result; }
266341
};
267342

268343
class Analysis : public BinaryFunctionPass {
@@ -271,12 +346,6 @@ class Analysis : public BinaryFunctionPass {
271346

272347
void runOnFunction(BinaryFunction &Function,
273348
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);
280349

281350
std::map<const BinaryFunction *, FunctionAnalysisResult> AnalysisResults;
282351
std::mutex AnalysisResultsMutex;

0 commit comments

Comments
 (0)