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 05f9fde

Browse filesBrowse files
committed
Improve estimation of the initial state of unreachable BBs
1 parent 1d45ece commit 05f9fde
Copy full SHA for 05f9fde

File tree

Expand file treeCollapse file tree

2 files changed

+56
-24
lines changed
Filter options
Expand file treeCollapse file tree

2 files changed

+56
-24
lines changed

‎bolt/lib/Passes/PAuthGadgetScanner.cpp

Copy file name to clipboardExpand all lines: bolt/lib/Passes/PAuthGadgetScanner.cpp
+56-19Lines changed: 56 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,22 @@ namespace PAuthGadgetScanner {
8484
dbgs() << "\n";
8585
}
8686

87+
// Iterates over BinaryFunction's instructions like a range-based for loop:
88+
//
89+
// iterateOverInstrs(BF, [&](MCInstReference Inst) {
90+
// // loop body
91+
// });
92+
template <typename T> static void iterateOverInstrs(BinaryFunction &BF, T Fn) {
93+
if (BF.hasCFG()) {
94+
for (BinaryBasicBlock &BB : BF)
95+
for (int64_t I = 0, E = BB.size(); I < E; ++I)
96+
Fn(MCInstInBBReference(&BB, I));
97+
} else {
98+
for (auto I : BF.instrs())
99+
Fn(MCInstInBFReference(&BF, I.first));
100+
}
101+
}
102+
87103
// This class represents mapping from a set of arbitrary physical registers to
88104
// consecutive array indexes.
89105
class TrackedRegisters {
@@ -343,10 +359,27 @@ class SrcSafetyAnalysis {
343359
return S;
344360
}
345361

346-
/// Creates a state with all registers marked unsafe (not to be confused
347-
/// with empty state).
348-
SrcState createUnsafeState() const {
349-
return SrcState(NumRegs, RegsToTrackInstsFor.getNumTrackedRegisters());
362+
/// Computes a reasonably pessimistic estimation of the register state when
363+
/// the previous instruction is not known for sure. Takes the set of registers
364+
/// which are trusted at function entry and removes all registers that can be
365+
/// clobbered inside this function.
366+
SrcState computePessimisticState(BinaryFunction &BF) {
367+
BitVector ClobberedRegs(NumRegs);
368+
iterateOverInstrs(BF, [&](MCInstReference Inst) {
369+
BC.MIB->getClobberedRegs(Inst, ClobberedRegs);
370+
371+
// If this is a call instruction, no register is safe anymore, unless
372+
// it is a tail call. Ignore tail calls for the purpose of estimating the
373+
// worst-case scenario, assuming no instructions are executed in the
374+
// caller after this point anyway.
375+
if (BC.MIB->isCall(Inst) && !BC.MIB->isTailCall(Inst))
376+
ClobberedRegs.set();
377+
});
378+
379+
SrcState S = createEntryState();
380+
S.SafeToDerefRegs.reset(ClobberedRegs);
381+
S.TrustedRegs.reset(ClobberedRegs);
382+
return S;
350383
}
351384

352385
BitVector getClobberedRegs(const MCInst &Point) const {
@@ -553,6 +586,10 @@ class DataflowSrcSafetyAnalysis
553586
using SrcSafetyAnalysis::BC;
554587
using SrcSafetyAnalysis::computeNext;
555588

589+
// Pessimistic initial state for basic blocks without any predecessors
590+
// (not needed for most functions, thus initialized lazily).
591+
SrcState PessimisticState;
592+
556593
public:
557594
DataflowSrcSafetyAnalysis(BinaryFunction &BF,
558595
MCPlusBuilder::AllocatorIdTy AllocId,
@@ -593,10 +630,15 @@ class DataflowSrcSafetyAnalysis
593630

594631
// If a basic block without any predecessors is found in an optimized code,
595632
// 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();
633+
// assume any register that can ever be clobbered in this function to be
634+
// unsafe before this basic block.
635+
// Warn about this fact in FunctionAnalysis::findUnsafeUses(), as it likely
636+
// means imprecise CFG information.
637+
if (BB.pred_empty()) {
638+
if (PessimisticState.empty())
639+
PessimisticState = computePessimisticState(*BB.getParent());
640+
return PessimisticState;
641+
}
600642

601643
return SrcState();
602644
}
@@ -671,6 +713,12 @@ class CFGUnawareSrcSafetyAnalysis : public SrcSafetyAnalysis {
671713
BC.MIB->removeAnnotation(I.second, StateAnnotationIndex);
672714
}
673715

716+
/// Creates a state with all registers marked unsafe (not to be confused
717+
/// with empty state).
718+
SrcState createUnsafeState() const {
719+
return SrcState(NumRegs, RegsToTrackInstsFor.getNumTrackedRegisters());
720+
}
721+
674722
public:
675723
CFGUnawareSrcSafetyAnalysis(BinaryFunction &BF,
676724
MCPlusBuilder::AllocatorIdTy AllocId,
@@ -1318,17 +1366,6 @@ shouldReportAuthOracle(const BinaryContext &BC, const MCInstReference &Inst,
13181366
return make_gadget_report(AuthOracleKind, Inst, *AuthReg);
13191367
}
13201368

1321-
template <typename T> static void iterateOverInstrs(BinaryFunction &BF, T Fn) {
1322-
if (BF.hasCFG()) {
1323-
for (BinaryBasicBlock &BB : BF)
1324-
for (int64_t I = 0, E = BB.size(); I < E; ++I)
1325-
Fn(MCInstInBBReference(&BB, I));
1326-
} else {
1327-
for (auto I : BF.instrs())
1328-
Fn(MCInstInBFReference(&BF, I.first));
1329-
}
1330-
}
1331-
13321369
static SmallVector<MCPhysReg>
13331370
collectRegsToTrack(ArrayRef<PartialReport<MCPhysReg>> Reports) {
13341371
SmallSet<MCPhysReg, 4> RegsToTrack;

‎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
-5Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -218,14 +218,9 @@ f_unreachable_instruction:
218218
// CHECK-LABEL: GS-PAUTH: Warning: the function has unreachable basic blocks (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:
224221
b 1f
225222
add x0, x1, x2
226223
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.
229224
ret
230225
.size f_unreachable_instruction, .-f_unreachable_instruction
231226

0 commit comments

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