https://github.com/atrosinenko updated https://github.com/llvm/llvm-project/pull/141665
>From 7a71b56676323327d012a9500f3e107d9b16d83c Mon Sep 17 00:00:00 2001 From: Anatoly Trosinenko <atrosine...@accesssoftek.com> Date: Tue, 27 May 2025 21:06:03 +0300 Subject: [PATCH] [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. --- bolt/lib/Passes/PAuthGadgetScanner.cpp | 67 +++++++++---------- .../AArch64/gs-pauth-debug-output.s | 14 ++-- 2 files changed, 38 insertions(+), 43 deletions(-) diff --git a/bolt/lib/Passes/PAuthGadgetScanner.cpp b/bolt/lib/Passes/PAuthGadgetScanner.cpp index 34b5b1d51de4e..dac274c0f4130 100644 --- a/bolt/lib/Passes/PAuthGadgetScanner.cpp +++ b/bolt/lib/Passes/PAuthGadgetScanner.cpp @@ -88,8 +88,8 @@ class TrackedRegisters { TrackedRegisters(ArrayRef<MCPhysReg> RegsToTrack) : Registers(RegsToTrack), RegToIndexMapping(getMappingSize(RegsToTrack), NoIndex) { - for (unsigned I = 0; I < RegsToTrack.size(); ++I) - RegToIndexMapping[RegsToTrack[I]] = I; + for (auto [MappedIndex, Reg] : llvm::enumerate(RegsToTrack)) + RegToIndexMapping[Reg] = MappedIndex; } ArrayRef<MCPhysReg> getRegisters() const { return Registers; } @@ -203,9 +203,9 @@ struct SrcState { SafeToDerefRegs &= StateIn.SafeToDerefRegs; TrustedRegs &= StateIn.TrustedRegs; - for (unsigned I = 0; I < LastInstWritingReg.size(); ++I) - for (const MCInst *J : StateIn.LastInstWritingReg[I]) - LastInstWritingReg[I].insert(J); + for (auto [ThisSet, OtherSet] : + llvm::zip_equal(LastInstWritingReg, StateIn.LastInstWritingReg)) + ThisSet.insert_range(OtherSet); return *this; } @@ -224,11 +224,9 @@ struct SrcState { static void printInstsShort(raw_ostream &OS, ArrayRef<SetOfRelatedInsts> Insts) { OS << "Insts: "; - for (unsigned I = 0; I < Insts.size(); ++I) { - auto &Set = Insts[I]; + for (auto [I, PtrSet] : llvm::enumerate(Insts)) { OS << "[" << I << "]("; - for (const MCInst *MCInstP : Set) - OS << MCInstP << " "; + interleave(PtrSet, OS, " "); OS << ")"; } } @@ -416,8 +414,9 @@ class SrcSafetyAnalysis { // ... an address can be updated in a safe manner, producing the result // which is as trusted as the input address. if (auto DstAndSrc = BC.MIB->analyzeAddressArithmeticsForPtrAuth(Point)) { - if (Cur.SafeToDerefRegs[DstAndSrc->second]) - Regs.push_back(DstAndSrc->first); + auto [DstReg, SrcReg] = *DstAndSrc; + if (Cur.SafeToDerefRegs[SrcReg]) + Regs.push_back(DstReg); } // Make sure explicit checker sequence keeps register safe-to-dereference @@ -469,8 +468,9 @@ class SrcSafetyAnalysis { // ... an address can be updated in a safe manner, producing the result // which is as trusted as the input address. if (auto DstAndSrc = BC.MIB->analyzeAddressArithmeticsForPtrAuth(Point)) { - if (Cur.TrustedRegs[DstAndSrc->second]) - Regs.push_back(DstAndSrc->first); + auto [DstReg, SrcReg] = *DstAndSrc; + if (Cur.TrustedRegs[SrcReg]) + Regs.push_back(DstReg); } return Regs; @@ -858,9 +858,9 @@ struct DstState { return (*this = StateIn); CannotEscapeUnchecked &= StateIn.CannotEscapeUnchecked; - for (unsigned I = 0; I < FirstInstLeakingReg.size(); ++I) - for (const MCInst *J : StateIn.FirstInstLeakingReg[I]) - FirstInstLeakingReg[I].insert(J); + for (auto [ThisSet, OtherSet] : + llvm::zip_equal(FirstInstLeakingReg, StateIn.FirstInstLeakingReg)) + ThisSet.insert_range(OtherSet); return *this; } @@ -1025,8 +1025,7 @@ class DstSafetyAnalysis { // ... an address can be updated in a safe manner, or if (auto DstAndSrc = BC.MIB->analyzeAddressArithmeticsForPtrAuth(Inst)) { - MCPhysReg DstReg, SrcReg; - std::tie(DstReg, SrcReg) = *DstAndSrc; + auto [DstReg, SrcReg] = *DstAndSrc; // Note that *all* registers containing the derived values must be safe, // both source and destination ones. No temporaries are supported at now. if (Cur.CannotEscapeUnchecked[SrcReg] && @@ -1065,7 +1064,7 @@ class DstSafetyAnalysis { // If this instruction terminates the program immediately, no // authentication oracles are possible past this point. if (BC.MIB->isTrap(Point)) { - LLVM_DEBUG({ traceInst(BC, "Trap instruction found", Point); }); + LLVM_DEBUG(traceInst(BC, "Trap instruction found", Point)); DstState Next(NumRegs, RegsToTrackInstsFor.getNumTrackedRegisters()); Next.CannotEscapeUnchecked.set(); return Next; @@ -1243,7 +1242,7 @@ class CFGUnawareDstSafetyAnalysis : public DstSafetyAnalysis, // starting to analyze Inst. if (BC.MIB->isCall(Inst) || BC.MIB->isBranch(Inst) || BC.MIB->isReturn(Inst)) { - LLVM_DEBUG({ traceInst(BC, "Control flow instruction", Inst); }); + LLVM_DEBUG(traceInst(BC, "Control flow instruction", Inst)); S = createUnsafeState(); } @@ -1381,12 +1380,12 @@ shouldReportUnsafeTailCall(const BinaryContext &BC, const BinaryFunction &BF, // such libc, ignore tail calls performed by ELF entry function. if (BC.StartFunctionAddress && *BC.StartFunctionAddress == Inst.getFunction()->getAddress()) { - LLVM_DEBUG({ dbgs() << " Skipping tail call in ELF entry function.\n"; }); + LLVM_DEBUG(dbgs() << " Skipping tail call in ELF entry function.\n"); return std::nullopt; } if (BC.MIB->isSafeJumpTableBranchForPtrAuth(Inst)) { - LLVM_DEBUG({ dbgs() << " Safe jump table detected, skipping.\n"; }); + LLVM_DEBUG(dbgs() << " Safe jump table detected, skipping.\n"); return std::nullopt; } @@ -1421,7 +1420,7 @@ shouldReportCallGadget(const BinaryContext &BC, const MCInstReference &Inst, return std::nullopt; if (BC.MIB->isSafeJumpTableBranchForPtrAuth(Inst)) { - LLVM_DEBUG({ dbgs() << " Safe jump table detected, skipping.\n"; }); + LLVM_DEBUG(dbgs() << " Safe jump table detected, skipping.\n"); return std::nullopt; } @@ -1466,7 +1465,7 @@ shouldReportAuthOracle(const BinaryContext &BC, const MCInstReference &Inst, }); if (S.empty()) { - LLVM_DEBUG({ dbgs() << " DstState is empty!\n"; }); + LLVM_DEBUG(dbgs() << " DstState is empty!\n"); return make_generic_report( Inst, "Warning: no state computed for an authentication instruction " "(possibly unreachable)"); @@ -1493,7 +1492,7 @@ collectRegsToTrack(ArrayRef<PartialReport<MCPhysReg>> Reports) { void FunctionAnalysisContext::findUnsafeUses( SmallVector<PartialReport<MCPhysReg>> &Reports) { auto Analysis = SrcSafetyAnalysis::create(BF, AllocatorId, {}); - LLVM_DEBUG({ dbgs() << "Running src register safety analysis...\n"; }); + LLVM_DEBUG(dbgs() << "Running src register safety analysis...\n"); Analysis->run(); LLVM_DEBUG({ dbgs() << "After src register safety analysis:\n"; @@ -1545,8 +1544,7 @@ void FunctionAnalysisContext::findUnsafeUses( const SrcState &S = Analysis->getStateBefore(Inst); if (S.empty()) { - LLVM_DEBUG( - { traceInst(BC, "Instruction has no state, skipping", Inst); }); + LLVM_DEBUG(traceInst(BC, "Instruction has no state, skipping", Inst)); assert(UnreachableBBReported && "Should be reported at least once"); (void)UnreachableBBReported; return; @@ -1573,8 +1571,7 @@ void FunctionAnalysisContext::augmentUnsafeUseReports( SmallVector<MCPhysReg> RegsToTrack = collectRegsToTrack(Reports); // Re-compute the analysis with register tracking. auto Analysis = SrcSafetyAnalysis::create(BF, AllocatorId, RegsToTrack); - LLVM_DEBUG( - { dbgs() << "\nRunning detailed src register safety analysis...\n"; }); + LLVM_DEBUG(dbgs() << "\nRunning detailed src register safety analysis...\n"); Analysis->run(); LLVM_DEBUG({ dbgs() << "After detailed src register safety analysis:\n"; @@ -1584,7 +1581,7 @@ void FunctionAnalysisContext::augmentUnsafeUseReports( // Augment gadget reports. for (auto &Report : Reports) { MCInstReference Location = Report.Issue->Location; - LLVM_DEBUG({ traceInst(BC, "Attaching clobbering info to", Location); }); + LLVM_DEBUG(traceInst(BC, "Attaching clobbering info to", Location)); assert(Report.RequestedDetails && "Should be removed by handleSimpleReports"); auto DetailedInfo = @@ -1602,7 +1599,7 @@ void FunctionAnalysisContext::findUnsafeDefs( return; auto Analysis = DstSafetyAnalysis::create(BF, AllocatorId, {}); - LLVM_DEBUG({ dbgs() << "Running dst register safety analysis...\n"; }); + LLVM_DEBUG(dbgs() << "Running dst register safety analysis...\n"); Analysis->run(); LLVM_DEBUG({ dbgs() << "After dst register safety analysis:\n"; @@ -1625,8 +1622,7 @@ void FunctionAnalysisContext::augmentUnsafeDefReports( SmallVector<MCPhysReg> RegsToTrack = collectRegsToTrack(Reports); // Re-compute the analysis with register tracking. auto Analysis = DstSafetyAnalysis::create(BF, AllocatorId, RegsToTrack); - LLVM_DEBUG( - { dbgs() << "\nRunning detailed dst register safety analysis...\n"; }); + LLVM_DEBUG(dbgs() << "\nRunning detailed dst register safety analysis...\n"); Analysis->run(); LLVM_DEBUG({ dbgs() << "After detailed dst register safety analysis:\n"; @@ -1636,7 +1632,7 @@ void FunctionAnalysisContext::augmentUnsafeDefReports( // Augment gadget reports. for (auto &Report : Reports) { MCInstReference Location = Report.Issue->Location; - LLVM_DEBUG({ traceInst(BC, "Attaching leakage info to", Location); }); + LLVM_DEBUG(traceInst(BC, "Attaching leakage info to", Location)); assert(Report.RequestedDetails && "Should be removed by handleSimpleReports"); auto DetailedInfo = std::make_shared<LeakageInfo>( @@ -1769,8 +1765,7 @@ static void printRelatedInstrs(raw_ostream &OS, const MCInstReference Location, // Sort the references to make output deterministic. SmallVector<MCInstReference> RI(RelatedInstrs); llvm::sort(RI); - for (unsigned I = 0; I < RI.size(); ++I) { - MCInstReference InstRef = RI[I]; + for (auto [I, InstRef] : llvm::enumerate(RI)) { OS << " " << (I + 1) << ". "; BC.printInstruction(OS, InstRef, getAddress(InstRef), &BF); }; diff --git a/bolt/test/binary-analysis/AArch64/gs-pauth-debug-output.s b/bolt/test/binary-analysis/AArch64/gs-pauth-debug-output.s index e99599c7463c6..f5e00c19dd26b 100644 --- a/bolt/test/binary-analysis/AArch64/gs-pauth-debug-output.s +++ b/bolt/test/binary-analysis/AArch64/gs-pauth-debug-output.s @@ -177,9 +177,9 @@ clobber: // CHECK-EMPTY: // CHECK-NEXT: Running detailed src register safety analysis... // CHECK-NEXT: SrcSafetyAnalysis::ComputeNext( mov w30, #0x0, src-state<SafeToDerefRegs: LR W30 W30_HI , TrustedRegs: LR W30 W30_HI , Insts: [0]()>) -// CHECK-NEXT: .. result: (src-state<SafeToDerefRegs: W30_HI , TrustedRegs: W30_HI , Insts: [0](0x{{[0-9a-f]+}} )>) -// CHECK-NEXT: SrcSafetyAnalysis::ComputeNext( ret x30, src-state<SafeToDerefRegs: W30_HI , TrustedRegs: W30_HI , Insts: [0](0x{{[0-9a-f]+}} )>) -// CHECK-NEXT: .. result: (src-state<SafeToDerefRegs: W30_HI , TrustedRegs: W30_HI , Insts: [0](0x{{[0-9a-f]+}} )>) +// CHECK-NEXT: .. result: (src-state<SafeToDerefRegs: W30_HI , TrustedRegs: W30_HI , Insts: [0](0x{{[0-9a-f]+}})>) +// CHECK-NEXT: SrcSafetyAnalysis::ComputeNext( ret x30, src-state<SafeToDerefRegs: W30_HI , TrustedRegs: W30_HI , Insts: [0](0x{{[0-9a-f]+}})>) +// CHECK-NEXT: .. result: (src-state<SafeToDerefRegs: W30_HI , TrustedRegs: W30_HI , Insts: [0](0x{{[0-9a-f]+}})>) // CHECK-NEXT: After detailed src register safety analysis: // CHECK-NEXT: Binary Function "clobber" { // ... @@ -189,7 +189,7 @@ clobber: // Iterating over the reports and attaching clobbering info: // CHECK-EMPTY: -// CHECK-NEXT: Attaching clobbering info to: 00000000: ret # DataflowSrcSafetyAnalysis: src-state<SafeToDerefRegs: BitVector, TrustedRegs: BitVector, Insts: [0](0x{{[0-9a-f]+}} )> +// CHECK-NEXT: Attaching clobbering info to: 00000000: ret # DataflowSrcSafetyAnalysis: src-state<SafeToDerefRegs: BitVector, TrustedRegs: BitVector, Insts: [0](0x{{[0-9a-f]+}})> .globl nocfg .type nocfg,@function @@ -315,7 +315,7 @@ auth_oracle: // AUTH-ORACLES-NEXT: DstSafetyAnalysis::ComputeNext( ret x30, dst-state<CannotEscapeUnchecked: , Insts: [0]()>) // AUTH-ORACLES-NEXT: .. result: (dst-state<CannotEscapeUnchecked: LR W30 W30_HI , Insts: [0]()>) // AUTH-ORACLES-NEXT: DstSafetyAnalysis::ComputeNext( autia x0, x1, dst-state<CannotEscapeUnchecked: LR W30 W30_HI , Insts: [0]()>) -// AUTH-ORACLES-NEXT: .. result: (dst-state<CannotEscapeUnchecked: LR W30 W30_HI , Insts: [0](0x{{[0-9a-f]+}} )>) +// AUTH-ORACLES-NEXT: .. result: (dst-state<CannotEscapeUnchecked: LR W30 W30_HI , Insts: [0](0x{{[0-9a-f]+}})>) // AUTH-ORACLES-NEXT: After detailed dst register safety analysis: // AUTH-ORACLES-NEXT: Binary Function "auth_oracle" { // AUTH-ORACLES-NEXT: Number : 4 @@ -325,14 +325,14 @@ auth_oracle: // AUTH-ORACLES-NEXT: } // AUTH-ORACLES-NEXT: [[BB0]] (2 instructions, align : 1) // AUTH-ORACLES-NEXT: Entry Point -// AUTH-ORACLES-NEXT: 00000000: autia x0, x1 # DataflowDstSafetyAnalysis: dst-state<CannotEscapeUnchecked: BitVector, Insts: [0](0x{{[0-9a-f]+}} )> +// AUTH-ORACLES-NEXT: 00000000: autia x0, x1 # DataflowDstSafetyAnalysis: dst-state<CannotEscapeUnchecked: BitVector, Insts: [0](0x{{[0-9a-f]+}})> // AUTH-ORACLES-NEXT: 00000004: ret # DataflowDstSafetyAnalysis: dst-state<CannotEscapeUnchecked: BitVector, Insts: [0]()> // AUTH-ORACLES-EMPTY: // AUTH-ORACLES-NEXT: DWARF CFI Instructions: // AUTH-ORACLES-NEXT: <empty> // AUTH-ORACLES-NEXT: End of Function "auth_oracle" // AUTH-ORACLES-EMPTY: -// AUTH-ORACLES-NEXT: Attaching leakage info to: 00000000: autia x0, x1 # DataflowDstSafetyAnalysis: dst-state<CannotEscapeUnchecked: BitVector, Insts: [0](0x{{[0-9a-f]+}} )> +// AUTH-ORACLES-NEXT: Attaching leakage info to: 00000000: autia x0, x1 # DataflowDstSafetyAnalysis: dst-state<CannotEscapeUnchecked: BitVector, Insts: [0](0x{{[0-9a-f]+}})> // Gadget scanner should not crash on CFI instructions, including when debug-printing them. // Note that the particular debug output is not checked, but BOLT should be _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits