Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 8c62a8a

Browse filesBrowse files
atrosinenkoatrosinenko
committed
[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.
1 parent e48c57b commit 8c62a8a
Copy full SHA for 8c62a8a

File tree

Expand file treeCollapse file tree

3 files changed

+95
-15
lines changed
Filter options
Expand file treeCollapse file tree

3 files changed

+95
-15
lines changed

‎bolt/lib/Passes/PAuthGadgetScanner.cpp

Copy file name to clipboardExpand all lines: bolt/lib/Passes/PAuthGadgetScanner.cpp
+32-14Lines changed: 32 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -344,6 +344,12 @@ class SrcSafetyAnalysis {
344344
return S;
345345
}
346346

347+
/// Creates a state with all registers marked unsafe (not to be confused
348+
/// with empty state).
349+
SrcState createUnsafeState() const {
350+
return SrcState(NumRegs, RegsToTrackInstsFor.getNumTrackedRegisters());
351+
}
352+
347353
BitVector getClobberedRegs(const MCInst &Point) const {
348354
BitVector Clobbered(NumRegs);
349355
// Assume a call can clobber all registers, including callee-saved
@@ -586,6 +592,13 @@ class DataflowSrcSafetyAnalysis
586592
if (BB.isEntryPoint())
587593
return createEntryState();
588594

595+
// If a basic block without any predecessors is found in an optimized code,
596+
// this likely means that some CFG edges were not detected. Pessimistically
597+
// assume all registers to be unsafe before this basic block and warn about
598+
// this fact in FunctionAnalysis::findUnsafeUses().
599+
if (BB.pred_empty())
600+
return createUnsafeState();
601+
589602
return SrcState();
590603
}
591604

@@ -659,12 +672,6 @@ class CFGUnawareSrcSafetyAnalysis : public SrcSafetyAnalysis {
659672
BC.MIB->removeAnnotation(I.second, StateAnnotationIndex);
660673
}
661674

662-
/// Creates a state with all registers marked unsafe (not to be confused
663-
/// with empty state).
664-
SrcState createUnsafeState() const {
665-
return SrcState(NumRegs, RegsToTrackInstsFor.getNumTrackedRegisters());
666-
}
667-
668675
public:
669676
CFGUnawareSrcSafetyAnalysis(BinaryFunction &BF,
670677
MCPlusBuilder::AllocatorIdTy AllocId,
@@ -1335,19 +1342,30 @@ void FunctionAnalysisContext::findUnsafeUses(
13351342
BF.dump();
13361343
});
13371344

1345+
if (BF.hasCFG()) {
1346+
// Warn on basic blocks being unreachable according to BOLT, as this
1347+
// likely means CFG is imprecise.
1348+
for (BinaryBasicBlock &BB : BF) {
1349+
if (!BB.pred_empty() || BB.isEntryPoint())
1350+
continue;
1351+
// Arbitrarily attach the report to the first instruction of BB.
1352+
MCInst *InstToReport = BB.getFirstNonPseudoInstr();
1353+
if (!InstToReport)
1354+
continue; // BB has no real instructions
1355+
1356+
Reports.push_back(
1357+
make_generic_report(MCInstReference::get(InstToReport, BF),
1358+
"Warning: no predecessor basic blocks detected "
1359+
"(possibly incomplete CFG)"));
1360+
}
1361+
}
1362+
13381363
iterateOverInstrs(BF, [&](MCInstReference Inst) {
13391364
if (BC.MIB->isCFI(Inst))
13401365
return;
13411366

13421367
const SrcState &S = Analysis->getStateBefore(Inst);
1343-
1344-
// If non-empty state was never propagated from the entry basic block
1345-
// to Inst, assume it to be unreachable and report a warning.
1346-
if (S.empty()) {
1347-
Reports.push_back(
1348-
make_generic_report(Inst, "Warning: unreachable instruction found"));
1349-
return;
1350-
}
1368+
assert(!S.empty() && "Instruction has no associated state");
13511369

13521370
if (auto Report = shouldReportReturnGadget(BC, Inst, S))
13531371
Reports.push_back(*Report);

‎bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s

Copy file name to clipboardExpand all lines: bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s
+6-1Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,12 +215,17 @@ f_callclobbered_calleesaved:
215215
.globl f_unreachable_instruction
216216
.type f_unreachable_instruction,@function
217217
f_unreachable_instruction:
218-
// CHECK-LABEL: GS-PAUTH: Warning: unreachable instruction found in function f_unreachable_instruction, basic block {{[0-9a-zA-Z.]+}}, at address
218+
// 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
219219
// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: add x0, x1, x2
220220
// CHECK-NOT: instructions that write to the affected registers after any authentication are:
221+
// CHECK-LABEL: GS-PAUTH: non-protected ret found in function f_unreachable_instruction, basic block {{[0-9a-zA-Z.]+}}, at address
222+
// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: ret
223+
// CHECK-NEXT: The 0 instructions that write to the affected registers after any authentication are:
221224
b 1f
222225
add x0, x1, x2
223226
1:
227+
// "ret" is reported as unprotected, as LR is pessimistically assumed
228+
// unsafe at "add x0, x1, x2", thus it is unsafe at "ret" as well.
224229
ret
225230
.size f_unreachable_instruction, .-f_unreachable_instruction
226231

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

Copy file name to clipboardExpand all lines: bolt/test/binary-analysis/AArch64/gs-pauth-calls.s
+57Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1428,6 +1428,63 @@ printed_instrs_nocfg:
14281428
br x0
14291429
.size printed_instrs_nocfg, .-printed_instrs_nocfg
14301430

1431+
// Test handling of unreachable basic blocks.
1432+
//
1433+
// Basic blocks without any predecessors were observed in real-world optimized
1434+
// code. At least sometimes they were actually reachable via jump table, which
1435+
// was not detected, but the function was processed as if its CFG was
1436+
// reconstructed successfully.
1437+
//
1438+
// As a more predictable model example, let's use really unreachable code
1439+
// for testing.
1440+
1441+
.globl bad_unreachable_call
1442+
.type bad_unreachable_call,@function
1443+
bad_unreachable_call:
1444+
// CHECK-LABEL: GS-PAUTH: Warning: no predecessor basic blocks detected (possibly incomplete CFG) in function bad_unreachable_call, basic block {{[^,]+}}, at address
1445+
// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: blr x0
1446+
// CHECK-NOT: instructions that write to the affected registers after any authentication are:
1447+
// CHECK-LABEL: GS-PAUTH: non-protected call found in function bad_unreachable_call, basic block {{[^,]+}}, at address
1448+
// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: blr x0
1449+
// CHECK-NEXT: The 0 instructions that write to the affected registers after any authentication are:
1450+
paciasp
1451+
stp x29, x30, [sp, #-16]!
1452+
mov x29, sp
1453+
1454+
b 1f
1455+
// unreachable basic block:
1456+
blr x0
1457+
1458+
1: // reachable basic block:
1459+
ldp x29, x30, [sp], #16
1460+
autiasp
1461+
ret
1462+
.size bad_unreachable_call, .-bad_unreachable_call
1463+
1464+
.globl good_unreachable_call
1465+
.type good_unreachable_call,@function
1466+
good_unreachable_call:
1467+
// CHECK-NOT: non-protected call{{.*}}good_unreachable_call
1468+
// CHECK-LABEL: GS-PAUTH: Warning: no predecessor basic blocks detected (possibly incomplete CFG) in function good_unreachable_call, basic block {{[^,]+}}, at address
1469+
// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: autia x0, x1
1470+
// CHECK-NOT: instructions that write to the affected registers after any authentication are:
1471+
// CHECK-NOT: non-protected call{{.*}}good_unreachable_call
1472+
paciasp
1473+
stp x29, x30, [sp, #-16]!
1474+
mov x29, sp
1475+
1476+
b 1f
1477+
// unreachable basic block:
1478+
autia x0, x1
1479+
blr x0 // <-- this call is definitely protected provided at least
1480+
// basic block boundaries are detected correctly
1481+
1482+
1: // reachable basic block:
1483+
ldp x29, x30, [sp], #16
1484+
autiasp
1485+
ret
1486+
.size good_unreachable_call, .-good_unreachable_call
1487+
14311488
.globl main
14321489
.type main,@function
14331490
main:

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.