Skip to content

Commit a5f741e

Browse files
markshannonmarkshannon
committed
Python: Use aggressive dead-code elimination when pruning.
1 parent 4d77902 commit a5f741e

File tree

13 files changed

+309
-27
lines changed

13 files changed

+309
-27
lines changed

‎python/ql/src/semmle/python/Flow.qll‎

Lines changed: 86 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ private AstNode toAst(ControlFlowNode n) {
3434
class ControlFlowNode extends @py_flow_node {
3535

3636
cached ControlFlowNode() {
37-
not Pruner::unreachable(this)
37+
Pruner::reachable(this)
3838
}
3939

4040
/** Whether this control flow node is a load (including those in augmented assignments) */
@@ -175,12 +175,13 @@ class ControlFlowNode extends @py_flow_node {
175175

176176
/** Gets a predecessor of this flow node */
177177
ControlFlowNode getAPredecessor() {
178-
py_successors(result, this)
178+
this = result.getASuccessor()
179179
}
180180

181181
/** Gets a successor of this flow node */
182182
ControlFlowNode getASuccessor() {
183-
py_successors(this, result)
183+
py_successors(this, result) and
184+
not Pruner::unreachableEdge(this, result)
184185
}
185186

186187
/** Gets the immediate dominator of this flow node */
@@ -291,22 +292,25 @@ class ControlFlowNode extends @py_flow_node {
291292

292293
/** Gets a successor for this node if the relevant condition is True. */
293294
ControlFlowNode getATrueSuccessor() {
295+
result = this.getASuccessor() and
294296
py_true_successors(this, result)
295297
}
296298

297299
/** Gets a successor for this node if the relevant condition is False. */
298300
ControlFlowNode getAFalseSuccessor() {
301+
result = this.getASuccessor() and
299302
py_false_successors(this, result)
300303
}
301304

302305
/** Gets a successor for this node if an exception is raised. */
303306
ControlFlowNode getAnExceptionalSuccessor() {
307+
result = this.getASuccessor() and
304308
py_exception_successors(this, result)
305309
}
306310

307311
/** Gets a successor for this node if no exception is raised. */
308312
ControlFlowNode getANormalSuccessor() {
309-
py_successors(this, result) and not
313+
result = this.getASuccessor() and not
310314
py_exception_successors(this, result)
311315
}
312316

@@ -942,21 +946,91 @@ predicate defined_by(NameNode def, Variable v) {
942946
exists(NameNode p | defined_by(p, v) and p.getASuccessor() = def and not p.defines(v))
943947
}
944948

949+
/* Combine extractor-generated basic block after pruning */
950+
951+
private class BasicBlockPart extends @py_flow_node {
952+
953+
string toString() { result = "Basic block part" }
954+
955+
BasicBlockPart() {
956+
py_flow_bb_node(_, _, this, _) and
957+
Pruner::reachable(this)
958+
}
959+
960+
predicate isHead() {
961+
count(this.(ControlFlowNode).getAPredecessor()) != 1
962+
or
963+
exists(ControlFlowNode pred | pred = this.(ControlFlowNode).getAPredecessor() | strictcount(pred.getASuccessor()) > 1)
964+
}
965+
966+
private BasicBlockPart previous() {
967+
not this.isHead() and
968+
py_flow_bb_node(this.(ControlFlowNode).getAPredecessor(), _, result, _)
969+
}
970+
971+
BasicBlockPart getHead() {
972+
this.isHead() and result = this
973+
or
974+
result = this.previous().getHead()
975+
}
976+
977+
predicate isLast() {
978+
not exists(BasicBlockPart part | part.previous() = this)
979+
}
980+
981+
int length() {
982+
result = max(int j | py_flow_bb_node(_, _, this, j)) + 1
983+
}
984+
985+
int startIndex() {
986+
this.isHead() and result = 0
987+
or
988+
exists(BasicBlockPart prev |
989+
prev = this.previous() and
990+
result = prev.startIndex() + prev.length()
991+
)
992+
}
993+
994+
predicate contains(ControlFlowNode node) {
995+
py_flow_bb_node(node, _, this, _)
996+
}
997+
998+
int indexOf(ControlFlowNode node) {
999+
py_flow_bb_node(node, _, this, result)
1000+
}
1001+
1002+
ControlFlowNode lastNode() {
1003+
this.indexOf(result) = max(this.indexOf(_))
1004+
}
1005+
1006+
BasicBlockPart getImmediateDominator() {
1007+
result.contains(this.(ControlFlowNode).getImmediateDominator())
1008+
}
1009+
1010+
}
1011+
9451012
/** A basic block (ignoring exceptional flow edges to scope exit) */
9461013
class BasicBlock extends @py_flow_node {
9471014

9481015
BasicBlock() {
949-
py_flow_bb_node(_, _, this, _)
1016+
this.(BasicBlockPart).isHead()
1017+
}
1018+
1019+
private BasicBlockPart getAPart() {
1020+
result.getHead() = this
9501021
}
9511022

9521023
/** Whether this basic block contains the specified node */
9531024
predicate contains(ControlFlowNode node) {
954-
py_flow_bb_node(node, _, this, _)
1025+
this.getAPart().contains(node)
9551026
}
9561027

9571028
/** Gets the nth node in this basic block */
9581029
ControlFlowNode getNode(int n) {
959-
py_flow_bb_node(result, _, this, n)
1030+
exists(BasicBlockPart part |
1031+
part = this.getAPart() and
1032+
n = part.startIndex() + part.indexOf(result)
1033+
)
9601034
}
9611035

9621036
string toString() {
@@ -976,7 +1050,7 @@ class BasicBlock extends @py_flow_node {
9761050
}
9771051

9781052
BasicBlock getImmediateDominator() {
979-
this.firstNode().getImmediateDominator().getBasicBlock() = result
1053+
this.getAPart().getImmediateDominator() = result.getAPart()
9801054
}
9811055

9821056
/** Dominance frontier of a node x is the set of all nodes `other` such that `this` dominates a predecessor
@@ -991,9 +1065,10 @@ class BasicBlock extends @py_flow_node {
9911065

9921066
/** Gets the last node in this basic block */
9931067
ControlFlowNode getLastNode() {
994-
exists(int i |
995-
this.getNode(i) = result and
996-
i = max(int j | py_flow_bb_node(_, _, this, j))
1068+
exists(BasicBlockPart part |
1069+
part = this.getAPart() and
1070+
part.isLast() and
1071+
result = part.lastNode()
9971072
)
9981073
}
9991074

‎python/ql/src/semmle/python/Pruning.qll‎

Lines changed: 35 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -396,16 +396,25 @@ module Pruner {
396396
)
397397
}
398398

399+
predicate reachable(UnprunedCfgNode n) {
400+
exists(UnprunedBasicBlock bb |
401+
reachableBB(bb) and bb.contains(n)
402+
)
403+
}
404+
399405
/** Holds if the basic block `bb` is unreachable due to
400406
* one or more constraints.
401407
*/
402408
predicate unreachableBB(UnprunedBasicBlock bb) {
403-
not bb.isEntry() and
404-
forall(UnprunedBasicBlock pred |
405-
pred.getASuccessor() = bb
406-
|
407-
unreachableEdge(pred, bb)
408-
)
409+
not reachableBB(bb)
410+
}
411+
412+
/** Holds if the basic block `bb` is reachable despite
413+
* constraints
414+
*/
415+
predicate reachableBB(UnprunedBasicBlock bb) {
416+
bb.isEntry() or
417+
reachableEdge(_, bb)
409418
}
410419

411420
Constraint constraintFromTest(SsaVariable var, UnprunedCfgNode node) {
@@ -533,18 +542,30 @@ module Pruner {
533542
}
534543

535544
/** Holds if the edge `pred` -> `succ` should be pruned as it cannot be reached */
536-
predicate unreachableEdge(UnprunedBasicBlock pred, UnprunedBasicBlock succ) {
545+
predicate unreachableEdge(UnprunedCfgNode pred, UnprunedCfgNode succ) {
546+
exists(UnprunedBasicBlock predBB, UnprunedBasicBlock succBB |
547+
succBB = predBB.getASuccessor() and
548+
not reachableEdge(predBB, succBB) and
549+
pred = predBB.last() and succ = succBB.first()
550+
)
551+
}
552+
553+
/** Holds if the edge `pred` -> `succ` is reachable as a result of
554+
* `pred` being reachable and this edge not being pruned. */
555+
predicate reachableEdge(UnprunedBasicBlock pred, UnprunedBasicBlock succ) {
556+
reachableBB(pred) and succ = pred.getASuccessor() and
557+
not contradictoryEdge(pred, succ) and
558+
not simplyDead(pred, succ)
559+
}
560+
561+
predicate contradictoryEdge(UnprunedBasicBlock pred, UnprunedBasicBlock succ) {
537562
exists(Constraint pre, Constraint cond |
538563
controllingConditions(pred, succ, pre, cond) and
539564
contradicts(pre, cond)
540565
)
541-
or
542-
unreachableBB(pred) and succ = pred.getASuccessor()
543-
or
544-
simply_dead(pred, succ)
545566
}
546567

547-
/* Helper for `unreachableEdge`, deal with inequalities here to avoid blow up */
568+
/* Helper for `contradictoryEdge`, deal with inequalities here to avoid blow up */
548569
pragma [inline]
549570
private predicate contradicts(Constraint a, Constraint b) {
550571
a = TIsNone(true) and b.cannotBeNone()
@@ -567,13 +588,13 @@ module Pruner {
567588
}
568589

569590
/** Holds if edge is simply dead. Stuff like `if False: ...` */
570-
predicate simply_dead(UnprunedBasicBlock pred, UnprunedBasicBlock succ) {
591+
predicate simplyDead(UnprunedBasicBlock pred, UnprunedBasicBlock succ) {
571592
constTest(pred.last()) = true and pred.getAFalseSuccessor() = succ
572593
or
573594
constTest(pred.last()) = false and pred.getATrueSuccessor() = succ
574595
}
575596

576-
/* Helper for simply_dead */
597+
/* Helper for simplyDead */
577598
private boolean constTest(UnprunedCfgNode node) {
578599
exists(ImmutableLiteral lit |
579600
result = lit.booleanValue() and lit = node.getNode()

‎python/ql/test/library-tests/ControlFlow/pruning/Constraint.expected‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,3 +105,10 @@
105105
| 207 | x | x | Truthy |
106106
| 209 | Compare | x | < 4 |
107107
| 209 | x | x | Truthy |
108+
| 214 | None | None | Truthy |
109+
| 215 | x | x | Truthy |
110+
| 215 | y | y | Truthy |
111+
| 217 | x | x | Truthy |
112+
| 217 | y | y | Truthy |
113+
| 219 | x | x | Truthy |
114+
| 223 | y | y | Truthy |

‎python/ql/test/library-tests/ControlFlow/pruning/ContradictoryEdge.expected‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
| 10 | test | 11 | count |
2+
| 24 | t1 | 24 | count |
3+
| 25 | t1 | 26 | t2 |
4+
| 29 | t2 | 30 | BoolExpr |
5+
| 30 | t2 | 30 | count |
6+
| 37 | t5 | 37 | count |
7+
| 38 | t5 | 39 | BoolExpr |
8+
| 39 | t6 | 39 | count |
9+
| 40 | t6 | 20 | Function boolop |
10+
| 47 | t1 | 48 | BoolExpr |
11+
| 47 | t1 | 51 | BoolExpr |
12+
| 48 | t2 | 48 | count |
13+
| 49 | t2 | 42 | Function with_splitting |
14+
| 51 | t2 | 51 | count |
15+
| 52 | t2 | 42 | Function with_splitting |
16+
| 93 | UnaryExpr | 94 | count |
17+
| 95 | y | 96 | count |
18+
| 102 | UnaryExpr | 103 | count |
19+
| 111 | t1 | 113 | t2 |
20+
| 113 | UnaryExpr | 106 | Function negated_conditional_live |
21+
| 119 | UnaryExpr | 120 | count |
22+
| 160 | Compare | 161 | count |
23+
| 160 | Compare | 163 | count |
24+
| 197 | Compare | 198 | count |
25+
| 203 | UnaryExpr | 204 | count |
26+
| 209 | Compare | 210 | count |
27+
| 217 | x | 217 | UnaryExpr |
28+
| 217 | x | 217 | y |
29+
| 217 | y | 217 | UnaryExpr |
30+
| 219 | x | 220 | count |
31+
| 219 | x | 222 | count |
32+
| 223 | y | 224 | count |
33+
| 223 | y | 226 | count |

‎python/ql/test/library-tests/ControlFlow/pruning/ContradictoryEdge.ql‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
2+
import python
3+
4+
import semmle.python.Pruning
5+
6+
from Pruner::UnprunedBasicBlock pred, Pruner::UnprunedBasicBlock succ, int line1, int line2
7+
where Pruner::contradictoryEdge(pred, succ) and
8+
line1 = pred.last().getNode().getLocation().getStartLine() and
9+
line2 = succ.first().getNode().getLocation().getStartLine() and
10+
line1 > 0
11+
select line1, pred.last().getNode().toString(), line2, succ.first().getNode().toString()
12+

‎python/ql/test/library-tests/ControlFlow/pruning/Pruned.expected‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
| 10 | test | 11 | count |
2+
| 11 | count | 7 | Function conditional_dead |
3+
| 24 | count | 25 | BoolExpr |
4+
| 24 | t1 | 24 | count |
5+
| 25 | t1 | 26 | t2 |
6+
| 29 | t2 | 30 | BoolExpr |
7+
| 30 | count | 31 | BoolExpr |
8+
| 30 | t2 | 30 | count |
9+
| 37 | count | 38 | BoolExpr |
10+
| 37 | t5 | 37 | count |
11+
| 38 | t5 | 39 | BoolExpr |
12+
| 39 | count | 40 | BoolExpr |
13+
| 39 | t6 | 39 | count |
14+
| 40 | t6 | 20 | Function boolop |
15+
| 47 | t1 | 48 | BoolExpr |
16+
| 47 | t1 | 51 | BoolExpr |
17+
| 48 | count | 49 | BoolExpr |
18+
| 48 | t2 | 48 | count |
19+
| 48 | t2 | 49 | BoolExpr |
20+
| 49 | count | 42 | Function with_splitting |
21+
| 49 | t2 | 42 | Function with_splitting |
22+
| 49 | t2 | 49 | count |
23+
| 51 | count | 52 | BoolExpr |
24+
| 51 | t2 | 51 | count |
25+
| 51 | t2 | 52 | BoolExpr |
26+
| 52 | count | 42 | Function with_splitting |
27+
| 52 | t2 | 42 | Function with_splitting |
28+
| 52 | t2 | 52 | count |
29+
| 93 | UnaryExpr | 94 | count |
30+
| 94 | count | 95 | y |
31+
| 95 | y | 96 | count |
32+
| 96 | count | 99 | ImportExpr |
33+
| 102 | UnaryExpr | 103 | count |
34+
| 103 | count | 106 | FunctionExpr |
35+
| 111 | t1 | 113 | t2 |
36+
| 113 | UnaryExpr | 106 | Function negated_conditional_live |
37+
| 119 | UnaryExpr | 120 | count |
38+
| 120 | count | 116 | Function negated_conditional_dead |
39+
| 130 | None | 131 | count |
40+
| 132 | UnaryExpr | 133 | count |
41+
| 132 | UnaryExpr | 134 | False |
42+
| 133 | count | 134 | False |
43+
| 134 | False | 135 | count |
44+
| 134 | False | 137 | count |
45+
| 138 | True | 139 | count |
46+
| 138 | True | 141 | count |
47+
| 139 | count | 142 | IntegerLiteral |
48+
| 141 | count | 142 | IntegerLiteral |
49+
| 142 | IntegerLiteral | 143 | count |
50+
| 142 | IntegerLiteral | 145 | count |
51+
| 143 | count | 146 | IntegerLiteral |
52+
| 145 | count | 146 | IntegerLiteral |
53+
| 146 | UnaryExpr | 147 | count |
54+
| 146 | UnaryExpr | 149 | count |
55+
| 147 | count | 151 | False |
56+
| 149 | count | 151 | False |
57+
| 151 | UnaryExpr | 152 | count |
58+
| 151 | UnaryExpr | 153 | False |
59+
| 152 | count | 153 | False |
60+
| 153 | UnaryExpr | 129 | Function prune_const_branches |
61+
| 153 | UnaryExpr | 154 | count |
62+
| 154 | count | 129 | Function prune_const_branches |
63+
| 160 | Compare | 161 | count |
64+
| 160 | Compare | 163 | count |
65+
| 161 | count | 157 | Function attribute_lookup_cannot_effect_comparisons_with_immutable_constants |
66+
| 163 | count | 157 | Function attribute_lookup_cannot_effect_comparisons_with_immutable_constants |
67+
| 197 | Compare | 198 | count |
68+
| 198 | count | 194 | Function inequality1 |
69+
| 203 | UnaryExpr | 204 | count |
70+
| 204 | count | 200 | Function inequality2 |
71+
| 209 | Compare | 210 | count |
72+
| 210 | count | 206 | Function reversed_inequality |
73+
| 217 | x | 217 | UnaryExpr |
74+
| 217 | x | 217 | y |
75+
| 217 | y | 217 | UnaryExpr |
76+
| 219 | x | 220 | count |
77+
| 219 | x | 222 | count |
78+
| 220 | count | 223 | y |
79+
| 222 | count | 223 | y |
80+
| 223 | y | 224 | count |
81+
| 223 | y | 226 | count |
82+
| 224 | count | 214 | Function split_bool1 |
83+
| 226 | count | 214 | Function split_bool1 |

0 commit comments

Comments
 (0)