Skip to content

Commit 9e5d66a

Browse files
atrosinenkoatrosinenko
committed
[BOLT] Gadget scanner: make use of C++17 features and LLVM helpers
Perform trivial syntactical cleanups: * make use of structured binding declarations * use LLVM utility functions when appropriate * omit braces around single expression inside single-line LLVM_DEBUG() This patch is NFC aside from minor debug output changes.
1 parent 3690b20 commit 9e5d66a

File tree

2 files changed

+38
-43
lines changed

2 files changed

+38
-43
lines changed

‎bolt/lib/Passes/PAuthGadgetScanner.cpp‎

Lines changed: 31 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,8 @@ class TrackedRegisters {
8888
TrackedRegisters(ArrayRef<MCPhysReg> RegsToTrack)
8989
: Registers(RegsToTrack),
9090
RegToIndexMapping(getMappingSize(RegsToTrack), NoIndex) {
91-
for (unsigned I = 0; I < RegsToTrack.size(); ++I)
92-
RegToIndexMapping[RegsToTrack[I]] = I;
91+
for (auto [MappedIndex, Reg] : llvm::enumerate(RegsToTrack))
92+
RegToIndexMapping[Reg] = MappedIndex;
9393
}
9494

9595
ArrayRef<MCPhysReg> getRegisters() const { return Registers; }
@@ -203,9 +203,9 @@ struct SrcState {
203203

204204
SafeToDerefRegs &= StateIn.SafeToDerefRegs;
205205
TrustedRegs &= StateIn.TrustedRegs;
206-
for (unsigned I = 0; I < LastInstWritingReg.size(); ++I)
207-
for (const MCInst *J : StateIn.LastInstWritingReg[I])
208-
LastInstWritingReg[I].insert(J);
206+
for (auto [ThisSet, OtherSet] :
207+
llvm::zip_equal(LastInstWritingReg, StateIn.LastInstWritingReg))
208+
ThisSet.insert_range(OtherSet);
209209
return *this;
210210
}
211211

@@ -224,11 +224,9 @@ struct SrcState {
224224
static void printInstsShort(raw_ostream &OS,
225225
ArrayRef<SetOfRelatedInsts> Insts) {
226226
OS << "Insts: ";
227-
for (unsigned I = 0; I < Insts.size(); ++I) {
228-
auto &Set = Insts[I];
227+
for (auto [I, PtrSet] : llvm::enumerate(Insts)) {
229228
OS << "[" << I << "](";
230-
for (const MCInst *MCInstP : Set)
231-
OS << MCInstP << " ";
229+
interleave(PtrSet, OS, " ");
232230
OS << ")";
233231
}
234232
}
@@ -416,8 +414,9 @@ class SrcSafetyAnalysis {
416414
// ... an address can be updated in a safe manner, producing the result
417415
// which is as trusted as the input address.
418416
if (auto DstAndSrc = BC.MIB->analyzeAddressArithmeticsForPtrAuth(Point)) {
419-
if (Cur.SafeToDerefRegs[DstAndSrc->second])
420-
Regs.push_back(DstAndSrc->first);
417+
auto [DstReg, SrcReg] = *DstAndSrc;
418+
if (Cur.SafeToDerefRegs[SrcReg])
419+
Regs.push_back(DstReg);
421420
}
422421

423422
// Make sure explicit checker sequence keeps register safe-to-dereference
@@ -469,8 +468,9 @@ class SrcSafetyAnalysis {
469468
// ... an address can be updated in a safe manner, producing the result
470469
// which is as trusted as the input address.
471470
if (auto DstAndSrc = BC.MIB->analyzeAddressArithmeticsForPtrAuth(Point)) {
472-
if (Cur.TrustedRegs[DstAndSrc->second])
473-
Regs.push_back(DstAndSrc->first);
471+
auto [DstReg, SrcReg] = *DstAndSrc;
472+
if (Cur.TrustedRegs[SrcReg])
473+
Regs.push_back(DstReg);
474474
}
475475

476476
return Regs;
@@ -868,9 +868,9 @@ struct DstState {
868868
return (*this = StateIn);
869869

870870
CannotEscapeUnchecked &= StateIn.CannotEscapeUnchecked;
871-
for (unsigned I = 0; I < FirstInstLeakingReg.size(); ++I)
872-
for (const MCInst *J : StateIn.FirstInstLeakingReg[I])
873-
FirstInstLeakingReg[I].insert(J);
871+
for (auto [ThisSet, OtherSet] :
872+
llvm::zip_equal(FirstInstLeakingReg, StateIn.FirstInstLeakingReg))
873+
ThisSet.insert_range(OtherSet);
874874
return *this;
875875
}
876876

@@ -1036,8 +1036,7 @@ class DstSafetyAnalysis {
10361036

10371037
// ... an address can be updated in a safe manner, or
10381038
if (auto DstAndSrc = BC.MIB->analyzeAddressArithmeticsForPtrAuth(Inst)) {
1039-
MCPhysReg DstReg, SrcReg;
1040-
std::tie(DstReg, SrcReg) = *DstAndSrc;
1039+
auto [DstReg, SrcReg] = *DstAndSrc;
10411040
// Note that *all* registers containing the derived values must be safe,
10421041
// both source and destination ones. No temporaries are supported at now.
10431042
if (Cur.CannotEscapeUnchecked[SrcReg] &&
@@ -1077,7 +1076,7 @@ class DstSafetyAnalysis {
10771076
// If this instruction terminates the program immediately, no
10781077
// authentication oracles are possible past this point.
10791078
if (BC.MIB->isTrap(Point)) {
1080-
LLVM_DEBUG({ traceInst(BC, "Trap instruction found", Point); });
1079+
LLVM_DEBUG(traceInst(BC, "Trap instruction found", Point));
10811080
DstState Next(NumRegs, RegsToTrackInstsFor.getNumTrackedRegisters());
10821081
Next.CannotEscapeUnchecked.set();
10831082
return Next;
@@ -1255,7 +1254,7 @@ class CFGUnawareDstSafetyAnalysis : public DstSafetyAnalysis,
12551254
// starting to analyze Inst.
12561255
if (BC.MIB->isCall(Inst) || BC.MIB->isBranch(Inst) ||
12571256
BC.MIB->isReturn(Inst)) {
1258-
LLVM_DEBUG({ traceInst(BC, "Control flow instruction", Inst); });
1257+
LLVM_DEBUG(traceInst(BC, "Control flow instruction", Inst));
12591258
S = createUnsafeState();
12601259
}
12611260

@@ -1400,12 +1399,12 @@ shouldReportUnsafeTailCall(const BinaryContext &BC, const BinaryFunction &BF,
14001399
// such libc, ignore tail calls performed by ELF entry function.
14011400
if (BC.StartFunctionAddress &&
14021401
*BC.StartFunctionAddress == Inst.getFunction()->getAddress()) {
1403-
LLVM_DEBUG({ dbgs() << " Skipping tail call in ELF entry function.\n"; });
1402+
LLVM_DEBUG(dbgs() << " Skipping tail call in ELF entry function.\n");
14041403
return std::nullopt;
14051404
}
14061405

14071406
if (BC.MIB->isSafeJumpTableBranchForPtrAuth(Inst)) {
1408-
LLVM_DEBUG({ dbgs() << " Safe jump table detected, skipping.\n"; });
1407+
LLVM_DEBUG(dbgs() << " Safe jump table detected, skipping.\n");
14091408
return std::nullopt;
14101409
}
14111410

@@ -1440,7 +1439,7 @@ shouldReportCallGadget(const BinaryContext &BC, const MCInstReference &Inst,
14401439
return std::nullopt;
14411440

14421441
if (BC.MIB->isSafeJumpTableBranchForPtrAuth(Inst)) {
1443-
LLVM_DEBUG({ dbgs() << " Safe jump table detected, skipping.\n"; });
1442+
LLVM_DEBUG(dbgs() << " Safe jump table detected, skipping.\n");
14441443
return std::nullopt;
14451444
}
14461445

@@ -1484,7 +1483,7 @@ shouldReportAuthOracle(const BinaryContext &BC, const MCInstReference &Inst,
14841483
});
14851484

14861485
if (S.empty()) {
1487-
LLVM_DEBUG({ dbgs() << " DstState is empty!\n"; });
1486+
LLVM_DEBUG(dbgs() << " DstState is empty!\n");
14881487
return make_generic_report(
14891488
Inst, "Warning: no state computed for an authentication instruction "
14901489
"(possibly unreachable)");
@@ -1511,7 +1510,7 @@ collectRegsToTrack(ArrayRef<PartialReport<MCPhysReg>> Reports) {
15111510
void FunctionAnalysisContext::findUnsafeUses(
15121511
SmallVector<PartialReport<MCPhysReg>> &Reports) {
15131512
auto Analysis = SrcSafetyAnalysis::create(BF, AllocatorId, {});
1514-
LLVM_DEBUG({ dbgs() << "Running src register safety analysis...\n"; });
1513+
LLVM_DEBUG(dbgs() << "Running src register safety analysis...\n");
15151514
Analysis->run();
15161515
LLVM_DEBUG({
15171516
dbgs() << "After src register safety analysis:\n";
@@ -1568,8 +1567,7 @@ void FunctionAnalysisContext::findUnsafeUses(
15681567

15691568
const SrcState &S = Analysis->getStateBefore(Inst);
15701569
if (S.empty()) {
1571-
LLVM_DEBUG(
1572-
{ traceInst(BC, "Instruction has no state, skipping", Inst); });
1570+
LLVM_DEBUG(traceInst(BC, "Instruction has no state, skipping", Inst));
15731571
assert(UnreachableBBReported && "Should be reported at least once");
15741572
(void)UnreachableBBReported;
15751573
return;
@@ -1596,8 +1594,7 @@ void FunctionAnalysisContext::augmentUnsafeUseReports(
15961594
SmallVector<MCPhysReg> RegsToTrack = collectRegsToTrack(Reports);
15971595
// Re-compute the analysis with register tracking.
15981596
auto Analysis = SrcSafetyAnalysis::create(BF, AllocatorId, RegsToTrack);
1599-
LLVM_DEBUG(
1600-
{ dbgs() << "\nRunning detailed src register safety analysis...\n"; });
1597+
LLVM_DEBUG(dbgs() << "\nRunning detailed src register safety analysis...\n");
16011598
Analysis->run();
16021599
LLVM_DEBUG({
16031600
dbgs() << "After detailed src register safety analysis:\n";
@@ -1607,7 +1604,7 @@ void FunctionAnalysisContext::augmentUnsafeUseReports(
16071604
// Augment gadget reports.
16081605
for (auto &Report : Reports) {
16091606
MCInstReference Location = Report.Issue->Location;
1610-
LLVM_DEBUG({ traceInst(BC, "Attaching clobbering info to", Location); });
1607+
LLVM_DEBUG(traceInst(BC, "Attaching clobbering info to", Location));
16111608
assert(Report.RequestedDetails &&
16121609
"Should be removed by handleSimpleReports");
16131610
auto DetailedInfo =
@@ -1625,7 +1622,7 @@ void FunctionAnalysisContext::findUnsafeDefs(
16251622
return;
16261623

16271624
auto Analysis = DstSafetyAnalysis::create(BF, AllocatorId, {});
1628-
LLVM_DEBUG({ dbgs() << "Running dst register safety analysis...\n"; });
1625+
LLVM_DEBUG(dbgs() << "Running dst register safety analysis...\n");
16291626
Analysis->run();
16301627
LLVM_DEBUG({
16311628
dbgs() << "After dst register safety analysis:\n";
@@ -1648,8 +1645,7 @@ void FunctionAnalysisContext::augmentUnsafeDefReports(
16481645
SmallVector<MCPhysReg> RegsToTrack = collectRegsToTrack(Reports);
16491646
// Re-compute the analysis with register tracking.
16501647
auto Analysis = DstSafetyAnalysis::create(BF, AllocatorId, RegsToTrack);
1651-
LLVM_DEBUG(
1652-
{ dbgs() << "\nRunning detailed dst register safety analysis...\n"; });
1648+
LLVM_DEBUG(dbgs() << "\nRunning detailed dst register safety analysis...\n");
16531649
Analysis->run();
16541650
LLVM_DEBUG({
16551651
dbgs() << "After detailed dst register safety analysis:\n";
@@ -1659,7 +1655,7 @@ void FunctionAnalysisContext::augmentUnsafeDefReports(
16591655
// Augment gadget reports.
16601656
for (auto &Report : Reports) {
16611657
MCInstReference Location = Report.Issue->Location;
1662-
LLVM_DEBUG({ traceInst(BC, "Attaching leakage info to", Location); });
1658+
LLVM_DEBUG(traceInst(BC, "Attaching leakage info to", Location));
16631659
assert(Report.RequestedDetails &&
16641660
"Should be removed by handleSimpleReports");
16651661
auto DetailedInfo = std::make_shared<LeakageInfo>(
@@ -1794,8 +1790,7 @@ static void printRelatedInstrs(raw_ostream &OS, const MCInstReference Location,
17941790
llvm::sort(RI, [](const MCInstReference &A, const MCInstReference &B) {
17951791
return getAddress(A) < getAddress(B);
17961792
});
1797-
for (unsigned I = 0; I < RI.size(); ++I) {
1798-
MCInstReference InstRef = RI[I];
1793+
for (auto [I, InstRef] : llvm::enumerate(RI)) {
17991794
OS << " " << (I + 1) << ". ";
18001795
BC.printInstruction(OS, InstRef, getAddress(InstRef), &BF);
18011796
};

‎bolt/test/binary-analysis/AArch64/gs-pauth-debug-output.s‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -177,9 +177,9 @@ clobber:
177177
// CHECK-EMPTY:
178178
// CHECK-NEXT: Running detailed src register safety analysis...
179179
// CHECK-NEXT: SrcSafetyAnalysis::ComputeNext( mov w30, #0x0, src-state<SafeToDerefRegs: LR W30 W30_HI , TrustedRegs: LR W30 W30_HI , Insts: [0]()>)
180-
// CHECK-NEXT: .. result: (src-state<SafeToDerefRegs: W30_HI , TrustedRegs: W30_HI , Insts: [0](0x{{[0-9a-f]+}} )>)
181-
// CHECK-NEXT: SrcSafetyAnalysis::ComputeNext( ret x30, src-state<SafeToDerefRegs: W30_HI , TrustedRegs: W30_HI , Insts: [0](0x{{[0-9a-f]+}} )>)
182-
// CHECK-NEXT: .. result: (src-state<SafeToDerefRegs: W30_HI , TrustedRegs: W30_HI , Insts: [0](0x{{[0-9a-f]+}} )>)
180+
// CHECK-NEXT: .. result: (src-state<SafeToDerefRegs: W30_HI , TrustedRegs: W30_HI , Insts: [0](0x{{[0-9a-f]+}})>)
181+
// CHECK-NEXT: SrcSafetyAnalysis::ComputeNext( ret x30, src-state<SafeToDerefRegs: W30_HI , TrustedRegs: W30_HI , Insts: [0](0x{{[0-9a-f]+}})>)
182+
// CHECK-NEXT: .. result: (src-state<SafeToDerefRegs: W30_HI , TrustedRegs: W30_HI , Insts: [0](0x{{[0-9a-f]+}})>)
183183
// CHECK-NEXT: After detailed src register safety analysis:
184184
// CHECK-NEXT: Binary Function "clobber" {
185185
// ...
@@ -189,7 +189,7 @@ clobber:
189189
// Iterating over the reports and attaching clobbering info:
190190

191191
// CHECK-EMPTY:
192-
// CHECK-NEXT: Attaching clobbering info to: 00000000: ret # DataflowSrcSafetyAnalysis: src-state<SafeToDerefRegs: BitVector, TrustedRegs: BitVector, Insts: [0](0x{{[0-9a-f]+}} )>
192+
// CHECK-NEXT: Attaching clobbering info to: 00000000: ret # DataflowSrcSafetyAnalysis: src-state<SafeToDerefRegs: BitVector, TrustedRegs: BitVector, Insts: [0](0x{{[0-9a-f]+}})>
193193

194194
.globl nocfg
195195
.type nocfg,@function
@@ -315,7 +315,7 @@ auth_oracle:
315315
// AUTH-ORACLES-NEXT: DstSafetyAnalysis::ComputeNext( ret x30, dst-state<CannotEscapeUnchecked: , Insts: [0]()>)
316316
// AUTH-ORACLES-NEXT: .. result: (dst-state<CannotEscapeUnchecked: LR W30 W30_HI , Insts: [0]()>)
317317
// AUTH-ORACLES-NEXT: DstSafetyAnalysis::ComputeNext( autia x0, x1, dst-state<CannotEscapeUnchecked: LR W30 W30_HI , Insts: [0]()>)
318-
// AUTH-ORACLES-NEXT: .. result: (dst-state<CannotEscapeUnchecked: LR W30 W30_HI , Insts: [0](0x{{[0-9a-f]+}} )>)
318+
// AUTH-ORACLES-NEXT: .. result: (dst-state<CannotEscapeUnchecked: LR W30 W30_HI , Insts: [0](0x{{[0-9a-f]+}})>)
319319
// AUTH-ORACLES-NEXT: After detailed dst register safety analysis:
320320
// AUTH-ORACLES-NEXT: Binary Function "auth_oracle" {
321321
// AUTH-ORACLES-NEXT: Number : 4
@@ -325,14 +325,14 @@ auth_oracle:
325325
// AUTH-ORACLES-NEXT: }
326326
// AUTH-ORACLES-NEXT: [[BB0]] (2 instructions, align : 1)
327327
// AUTH-ORACLES-NEXT: Entry Point
328-
// AUTH-ORACLES-NEXT: 00000000: autia x0, x1 # DataflowDstSafetyAnalysis: dst-state<CannotEscapeUnchecked: BitVector, Insts: [0](0x{{[0-9a-f]+}} )>
328+
// AUTH-ORACLES-NEXT: 00000000: autia x0, x1 # DataflowDstSafetyAnalysis: dst-state<CannotEscapeUnchecked: BitVector, Insts: [0](0x{{[0-9a-f]+}})>
329329
// AUTH-ORACLES-NEXT: 00000004: ret # DataflowDstSafetyAnalysis: dst-state<CannotEscapeUnchecked: BitVector, Insts: [0]()>
330330
// AUTH-ORACLES-EMPTY:
331331
// AUTH-ORACLES-NEXT: DWARF CFI Instructions:
332332
// AUTH-ORACLES-NEXT: <empty>
333333
// AUTH-ORACLES-NEXT: End of Function "auth_oracle"
334334
// AUTH-ORACLES-EMPTY:
335-
// AUTH-ORACLES-NEXT: Attaching leakage info to: 00000000: autia x0, x1 # DataflowDstSafetyAnalysis: dst-state<CannotEscapeUnchecked: BitVector, Insts: [0](0x{{[0-9a-f]+}} )>
335+
// AUTH-ORACLES-NEXT: Attaching leakage info to: 00000000: autia x0, x1 # DataflowDstSafetyAnalysis: dst-state<CannotEscapeUnchecked: BitVector, Insts: [0](0x{{[0-9a-f]+}})>
336336

337337
// Gadget scanner should not crash on CFI instructions, including when debug-printing them.
338338
// Note that the particular debug output is not checked, but BOLT should be

0 commit comments

Comments
 (0)