https://github.com/atrosinenko updated
https://github.com/llvm/llvm-project/pull/136183
>From 629a4239c9d88f49ccf723ab0b7a13e5a9ad0144 Mon Sep 17 00:00:00 2001
From: Anatoly Trosinenko <atrosine...@accesssoftek.com>
Date: Thu, 17 Apr 2025 20:51:16 +0300
Subject: [PATCH] [BOLT] Gadget scanner: improve handling of unreachable basic
blocks
Instead of refusing to analyze an instruction completely, when it is
unreachable according to the CFG reconstructed by BOLT, pessimistically
assume all registers to be unsafe at the start of basic blocks without
any predecessors. Nevertheless, unreachable basic blocks found in
optimized code likely means imprecise CFG reconstruction, thus report a
warning once per basic block without predecessors.
---
bolt/lib/Passes/PAuthGadgetScanner.cpp | 46 ++++++++++-----
.../AArch64/gs-pacret-autiasp.s | 7 ++-
.../binary-analysis/AArch64/gs-pauth-calls.s | 57 +++++++++++++++++++
3 files changed, 95 insertions(+), 15 deletions(-)
diff --git a/bolt/lib/Passes/PAuthGadgetScanner.cpp
b/bolt/lib/Passes/PAuthGadgetScanner.cpp
index f7ac0b67d00da..0ce9f51c44af4 100644
--- a/bolt/lib/Passes/PAuthGadgetScanner.cpp
+++ b/bolt/lib/Passes/PAuthGadgetScanner.cpp
@@ -344,6 +344,12 @@ class SrcSafetyAnalysis {
return S;
}
+ /// Creates a state with all registers marked unsafe (not to be confused
+ /// with empty state).
+ SrcState createUnsafeState() const {
+ return SrcState(NumRegs, RegsToTrackInstsFor.getNumTrackedRegisters());
+ }
+
BitVector getClobberedRegs(const MCInst &Point) const {
BitVector Clobbered(NumRegs);
// Assume a call can clobber all registers, including callee-saved
@@ -581,6 +587,13 @@ class DataflowSrcSafetyAnalysis
if (BB.isEntryPoint())
return createEntryState();
+ // If a basic block without any predecessors is found in an optimized code,
+ // this likely means that some CFG edges were not detected. Pessimistically
+ // assume all registers to be unsafe before this basic block and warn about
+ // this fact in FunctionAnalysis::findUnsafeUses().
+ if (BB.pred_empty())
+ return createUnsafeState();
+
return SrcState();
}
@@ -654,12 +667,6 @@ class CFGUnawareSrcSafetyAnalysis : public
SrcSafetyAnalysis {
BC.MIB->removeAnnotation(I.second, StateAnnotationIndex);
}
- /// Creates a state with all registers marked unsafe (not to be confused
- /// with empty state).
- SrcState createUnsafeState() const {
- return SrcState(NumRegs, RegsToTrackInstsFor.getNumTrackedRegisters());
- }
-
public:
CFGUnawareSrcSafetyAnalysis(BinaryFunction &BF,
MCPlusBuilder::AllocatorIdTy AllocId,
@@ -1328,19 +1335,30 @@ void FunctionAnalysis::findUnsafeUses(
BF.dump();
});
+ if (BF.hasCFG()) {
+ // Warn on basic blocks being unreachable according to BOLT, as this
+ // likely means CFG is imprecise.
+ for (BinaryBasicBlock &BB : BF) {
+ if (!BB.pred_empty() || BB.isEntryPoint())
+ continue;
+ // Arbitrarily attach the report to the first instruction of BB.
+ MCInst *InstToReport = BB.getFirstNonPseudoInstr();
+ if (!InstToReport)
+ continue; // BB has no real instructions
+
+ Reports.push_back(
+ make_generic_report(MCInstReference::get(InstToReport, BF),
+ "Warning: no predecessor basic blocks detected "
+ "(possibly incomplete CFG)"));
+ }
+ }
+
iterateOverInstrs(BF, [&](MCInstReference Inst) {
if (BC.MIB->isCFI(Inst))
return;
const SrcState &S = Analysis->getStateBefore(Inst);
-
- // If non-empty state was never propagated from the entry basic block
- // to Inst, assume it to be unreachable and report a warning.
- if (S.empty()) {
- Reports.push_back(
- make_generic_report(Inst, "Warning: unreachable instruction found"));
- return;
- }
+ assert(!S.empty() && "Instruction has no associated state");
if (auto Report = shouldReportReturnGadget(BC, Inst, S))
Reports.push_back(*Report);
diff --git a/bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s
b/bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s
index 284f0bea607a5..6559ba336e8de 100644
--- a/bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s
+++ b/bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s
@@ -215,12 +215,17 @@ f_callclobbered_calleesaved:
.globl f_unreachable_instruction
.type f_unreachable_instruction,@function
f_unreachable_instruction:
-// CHECK-LABEL: GS-PAUTH: Warning: unreachable instruction found in function
f_unreachable_instruction, basic block {{[0-9a-zA-Z.]+}}, at address
+// CHECK-LABEL: GS-PAUTH: Warning: no predecessor basic blocks detected
(possibly incomplete CFG) in function f_unreachable_instruction, basic block
{{[0-9a-zA-Z.]+}}, at address
// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: add x0, x1,
x2
// CHECK-NOT: instructions that write to the affected registers after any
authentication are:
+// CHECK-LABEL: GS-PAUTH: non-protected ret found in function
f_unreachable_instruction, basic block {{[0-9a-zA-Z.]+}}, at address
+// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: ret
+// CHECK-NEXT: The 0 instructions that write to the affected registers after
any authentication are:
b 1f
add x0, x1, x2
1:
+ // "ret" is reported as unprotected, as LR is pessimistically assumed
+ // unsafe at "add x0, x1, x2", thus it is unsafe at "ret" as well.
ret
.size f_unreachable_instruction, .-f_unreachable_instruction
diff --git a/bolt/test/binary-analysis/AArch64/gs-pauth-calls.s
b/bolt/test/binary-analysis/AArch64/gs-pauth-calls.s
index c79c5926a05cd..9d01431e809ab 100644
--- a/bolt/test/binary-analysis/AArch64/gs-pauth-calls.s
+++ b/bolt/test/binary-analysis/AArch64/gs-pauth-calls.s
@@ -1428,6 +1428,63 @@ printed_instrs_nocfg:
br x0
.size printed_instrs_nocfg, .-printed_instrs_nocfg
+// Test handling of unreachable basic blocks.
+//
+// Basic blocks without any predecessors were observed in real-world optimized
+// code. At least sometimes they were actually reachable via jump table, which
+// was not detected, but the function was processed as if its CFG was
+// reconstructed successfully.
+//
+// As a more predictable model example, let's use really unreachable code
+// for testing.
+
+ .globl bad_unreachable_call
+ .type bad_unreachable_call,@function
+bad_unreachable_call:
+// CHECK-LABEL: GS-PAUTH: Warning: no predecessor basic blocks detected
(possibly incomplete CFG) in function bad_unreachable_call, basic block
{{[^,]+}}, at address
+// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: blr x0
+// CHECK-NOT: instructions that write to the affected registers after any
authentication are:
+// CHECK-LABEL: GS-PAUTH: non-protected call found in function
bad_unreachable_call, basic block {{[^,]+}}, at address
+// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: blr x0
+// CHECK-NEXT: The 0 instructions that write to the affected registers after
any authentication are:
+ paciasp
+ stp x29, x30, [sp, #-16]!
+ mov x29, sp
+
+ b 1f
+ // unreachable basic block:
+ blr x0
+
+1: // reachable basic block:
+ ldp x29, x30, [sp], #16
+ autiasp
+ ret
+ .size bad_unreachable_call, .-bad_unreachable_call
+
+ .globl good_unreachable_call
+ .type good_unreachable_call,@function
+good_unreachable_call:
+// CHECK-NOT: non-protected call{{.*}}good_unreachable_call
+// CHECK-LABEL: GS-PAUTH: Warning: no predecessor basic blocks detected
(possibly incomplete CFG) in function good_unreachable_call, basic block
{{[^,]+}}, at address
+// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: autia x0, x1
+// CHECK-NOT: instructions that write to the affected registers after any
authentication are:
+// CHECK-NOT: non-protected call{{.*}}good_unreachable_call
+ paciasp
+ stp x29, x30, [sp, #-16]!
+ mov x29, sp
+
+ b 1f
+ // unreachable basic block:
+ autia x0, x1
+ blr x0 // <-- this call is definitely protected provided at
least
+ // basic block boundaries are detected correctly
+
+1: // reachable basic block:
+ ldp x29, x30, [sp], #16
+ autiasp
+ ret
+ .size good_unreachable_call, .-good_unreachable_call
+
.globl main
.type main,@function
main:
_______________________________________________
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
