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 b3acb46

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 08a7baa commit b3acb46
Copy full SHA for b3acb46

File tree

3 files changed

+90
-15
lines changed
Filter options

3 files changed

+90
-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
@@ -346,6 +346,12 @@ class SrcSafetyAnalysis {
346346
return S;
347347
}
348348

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

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

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

661-
/// Creates a state with all registers marked unsafe (not to be confused
662-
/// with empty state).
663-
SrcState createUnsafeState() const {
664-
return SrcState(NumRegs, RegsToTrackInstsFor.getNumTrackedRegisters());
665-
}
666-
667674
public:
668675
CFGUnawareSrcSafetyAnalysis(BinaryFunction &BF,
669676
MCPlusBuilder::AllocatorIdTy AllocId,
@@ -1335,19 +1342,30 @@ void FunctionAnalysis::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
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ 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
b 1f
221221
add x0, x1, x2

‎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-NEXT: The 0 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-NEXT: The 0 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.