Skip to content

Commit 8443f68

Browse files
taus-semmletaus-semmle
authored
Merge pull request #1624 from markshannon/python-fix-pruning-for-constants
Python: Fix up pruning in QL to better handle constraints from constants.
2 parents 0258f79 + 05e498d commit 8443f68

File tree

8 files changed

+200
-159
lines changed

8 files changed

+200
-159
lines changed

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -346,7 +346,6 @@ abstract class ImmutableLiteral extends Expr {
346346
abstract Object getLiteralObject();
347347

348348
abstract boolean booleanValue();
349-
350349
}
351350

352351
/** A numerical constant expression, such as `7` or `4.2` */
@@ -422,7 +421,6 @@ class FloatLiteral extends Num {
422421
or
423422
this.getValue() != 0.0 and this.getValue() != -0.0 and result = true
424423
}
425-
426424
}
427425

428426
/** An imaginary numeric constant, such as `3j` */
@@ -474,6 +472,10 @@ class NegativeIntegerLiteral extends ImmutableLiteral, UnaryExpr {
474472
py_cobjectnames(result, "-" + this.getOperand().(IntegerLiteral).getN())
475473
}
476474

475+
int getValue() {
476+
result = -this.getOperand().(IntegerLiteral).getValue()
477+
}
478+
477479
}
478480

479481
/** A unicode string expression, such as `u"\u20ac"`. Note that unadorned string constants such as
@@ -802,7 +804,6 @@ class None extends NameConstant {
802804
override boolean booleanValue() {
803805
result = false
804806
}
805-
806807
}
807808

808809
/** An await expression such as `await coro`. */

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

Lines changed: 38 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,12 @@ module Pruner {
156156
private import Comparisons
157157
private import SSA
158158

159+
private int intValue(ImmutableLiteral lit) {
160+
result = lit.(IntegerLiteral).getValue()
161+
or
162+
result = lit.(NegativeIntegerLiteral).getValue()
163+
}
164+
159165
newtype TConstraint =
160166
TTruthy(boolean b) { b = true or b = false }
161167
or
@@ -164,7 +170,7 @@ module Pruner {
164170
TConstrainedByConstant(CompareOp op, int k) {
165171
int_test(_, _, op, k)
166172
or
167-
exists(Assign a | a.getValue().(IntegerLiteral).getValue() = k) and op = eq()
173+
exists(Assign a | intValue(a.getValue()) = k) and op = eq()
168174
}
169175

170176
/** A constraint that may be applied to an SSA variable.
@@ -417,7 +423,7 @@ module Pruner {
417423
reachableEdge(_, bb)
418424
}
419425

420-
Constraint constraintFromTest(SsaVariable var, UnprunedCfgNode node) {
426+
Constraint constraintFromExpr(SsaVariable var, UnprunedCfgNode node) {
421427
py_ssa_use(node, var) and result = TTruthy(true)
422428
or
423429
exists(boolean b |
@@ -429,7 +435,11 @@ module Pruner {
429435
result = TConstrainedByConstant(op, k)
430436
)
431437
or
432-
result = constraintFromTest(var, node.(UnprunedNot).getOperand()).invert()
438+
result = constraintFromExpr(var, node.(UnprunedNot).getOperand()).invert()
439+
}
440+
441+
Constraint constraintFromTest(SsaVariable var, UnprunedCfgNode node) {
442+
result = constraintFromExpr(var, node) and node.isBranch()
433443
}
434444

435445
predicate none_test(UnprunedCompareNode test, SsaVariable var, boolean is) {
@@ -450,56 +460,46 @@ module Pruner {
450460
|
451461
op.forOp(cop) and
452462
py_ssa_use(left, var) and
453-
right.getNode().(IntegerLiteral).getValue() = k
463+
intValue(right.getNode()) = k
454464
or
455465
op.reverse().forOp(cop) and
456466
py_ssa_use(right, var) and
457-
left.getNode().(IntegerLiteral).getValue() = k
467+
intValue(left.getNode()) = k
458468
)
459-
or
460-
int_test(test.(UnprunedNot).getOperand(), var, op.invert(), k)
461469
}
462470

463-
predicate int_assignment(UnprunedCfgNode asgn, SsaVariable var, CompareOp op, int k) {
464-
exists(Assign a |
465-
a.getATarget() = asgn.getNode() and
466-
py_ssa_use(asgn, var) and
467-
k = a.getValue().(IntegerLiteral).getValue() and
468-
op = eq()
469-
)
470-
}
471-
472-
predicate none_assignment(UnprunedCfgNode asgn, SsaVariable var) {
473-
exists(Assign a |
474-
a.getATarget() = asgn.getNode() and
475-
py_ssa_use(asgn, var) and
476-
a.getValue() instanceof None
471+
private predicate constrainingValue(Expr e) {
472+
exists(Assign a, UnprunedCfgNode asgn |
473+
a.getValue() = e and a.getATarget() = asgn.getNode() and py_ssa_defn(_, asgn)
477474
)
475+
or
476+
exists(UnaryExpr n | constrainingValue(n) and n.getOp() instanceof Not and e = n.getOperand())
478477
}
479478

480-
boolean truthy_assignment(UnprunedCfgNode asgn, SsaVariable var) {
481-
exists(Assign a |
482-
a.getATarget() = asgn.getNode() and
483-
py_ssa_use(asgn, var)
484-
|
485-
a.getValue() instanceof True and result = true
479+
private Constraint constraintFromValue(Expr e) {
480+
constrainingValue(e) and
481+
(
482+
result = TConstrainedByConstant(eq(), intValue(e))
483+
or
484+
e instanceof True and result = TTruthy(true)
485+
or
486+
e instanceof False and result = TTruthy(false)
487+
or
488+
e instanceof None and result = TIsNone(true)
486489
or
487-
a.getValue() instanceof False and result = false
490+
result = constraintFromValue(e.(UnaryExpr).getOperand()).invert()
488491
)
489-
or
490-
module_import(asgn, var) and result = true
491492
}
492493

493494
/** Gets the constraint on `var` resulting from the assignment in `asgn` */
494-
Constraint constraintFromAssignment(SsaVariable var, UnprunedBasicBlock asgn) {
495-
exists(CompareOp op, int k |
496-
int_assignment(asgn.getANode(), var, op, k) and
497-
result = TConstrainedByConstant(op, k)
495+
Constraint constraintFromAssignment(SsaVariable var, UnprunedCfgNode asgn) {
496+
exists(Assign a |
497+
a.getATarget() = asgn.getNode() and
498+
py_ssa_defn(var, asgn) and
499+
result = constraintFromValue(a.getValue())
498500
)
499501
or
500-
none_assignment(asgn.getANode(), var) and result = TIsNone(true)
501-
or
502-
result = TTruthy(truthy_assignment(asgn.getANode(), var))
502+
module_import(asgn, var) and result = TTruthy(true)
503503
}
504504

505505
/** Holds if the constraint `preval` holds for `var` on edge `pred` -> `succ` as a result of a prior test or assignment */
@@ -518,7 +518,7 @@ module Pruner {
518518
first.(UnprunedConditionBlock).controlsEdge(pred, succ, false) and
519519
preval = constraintFromTest(var, first.last()).invert()
520520
or
521-
preval = constraintFromAssignment(var, first) and
521+
preval = constraintFromAssignment(var, first.getANode()) and
522522
first.dominates(pred) and
523523
(succ = pred.getAFalseSuccessor() or succ = pred.getATrueSuccessor())
524524
)

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

Lines changed: 80 additions & 114 deletions
Original file line numberDiff line numberDiff line change
@@ -1,114 +1,80 @@
1-
| 8 | test | test | Truthy |
2-
| 10 | test | test | Truthy |
3-
| 14 | seq | seq | Truthy |
4-
| 16 | seq | seq | Truthy |
5-
| 17 | seq | seq | Truthy |
6-
| 21 | UnaryExpr | t1 | Falsey |
7-
| 21 | t1 | t1 | Truthy |
8-
| 24 | t1 | t1 | Truthy |
9-
| 25 | t1 | t1 | Truthy |
10-
| 26 | t2 | t2 | Truthy |
11-
| 29 | t2 | t2 | Truthy |
12-
| 30 | t2 | t2 | Truthy |
13-
| 31 | t3 | t3 | Truthy |
14-
| 31 | t4 | t4 | Truthy |
15-
| 32 | t3 | t3 | Truthy |
16-
| 33 | t3 | t3 | Truthy |
17-
| 34 | t3 | t3 | Truthy |
18-
| 35 | t4 | t4 | Truthy |
19-
| 36 | t5 | t5 | Truthy |
20-
| 36 | t6 | t6 | Truthy |
21-
| 37 | t5 | t5 | Truthy |
22-
| 38 | t5 | t5 | Truthy |
23-
| 39 | t6 | t6 | Truthy |
24-
| 40 | t6 | t6 | Truthy |
25-
| 43 | t1 | t1 | Truthy |
26-
| 44 | UnaryExpr | t2 | Falsey |
27-
| 44 | t2 | t2 | Truthy |
28-
| 47 | t1 | t1 | Truthy |
29-
| 48 | t2 | t2 | Truthy |
30-
| 49 | t2 | t2 | Truthy |
31-
| 51 | t2 | t2 | Truthy |
32-
| 52 | t2 | t2 | Truthy |
33-
| 55 | seq1 | seq1 | Truthy |
34-
| 57 | UnaryExpr | seq2 | Falsey |
35-
| 57 | seq2 | seq2 | Truthy |
36-
| 60 | seq1 | seq1 | Truthy |
37-
| 62 | seq1 | seq1 | Truthy |
38-
| 63 | seq2 | seq2 | Truthy |
39-
| 65 | seq2 | seq2 | Truthy |
40-
| 66 | seq3 | seq3 | Truthy |
41-
| 68 | UnaryExpr | seq4 | Falsey |
42-
| 68 | seq4 | seq4 | Truthy |
43-
| 71 | seq3 | seq3 | Truthy |
44-
| 73 | var | var | Truthy |
45-
| 74 | seq4 | seq4 | Truthy |
46-
| 76 | var | var | Truthy |
47-
| 78 | seq5 | seq5 | Truthy |
48-
| 80 | seq5 | seq5 | Truthy |
49-
| 81 | seq5 | seq5 | Truthy |
50-
| 83 | var | var | Truthy |
51-
| 88 | UnaryExpr | x | Falsey |
52-
| 88 | x | x | Truthy |
53-
| 89 | Exception | Exception | Truthy |
54-
| 90 | y | y | Truthy |
55-
| 91 | Exception | Exception | Truthy |
56-
| 92 | make_a_call | make_a_call | Truthy |
57-
| 93 | UnaryExpr | x | Falsey |
58-
| 93 | x | x | Truthy |
59-
| 94 | count | count | Truthy |
60-
| 95 | y | y | Truthy |
61-
| 96 | count | count | Truthy |
62-
| 101 | make_a_call | make_a_call | Truthy |
63-
| 102 | UnaryExpr | another_module | Falsey |
64-
| 102 | another_module | another_module | Truthy |
65-
| 103 | count | count | Truthy |
66-
| 107 | UnaryExpr | t1 | Falsey |
67-
| 107 | t1 | t1 | Truthy |
68-
| 109 | t2 | t2 | Truthy |
69-
| 111 | t1 | t1 | Truthy |
70-
| 113 | UnaryExpr | t2 | Falsey |
71-
| 113 | t2 | t2 | Truthy |
72-
| 117 | UnaryExpr | test | Falsey |
73-
| 117 | test | test | Truthy |
74-
| 119 | UnaryExpr | test | Falsey |
75-
| 119 | test | test | Truthy |
76-
| 123 | m | m | Truthy |
77-
| 125 | m | m | Truthy |
78-
| 126 | m | m | Truthy |
79-
| 158 | Compare | ps | Is not None |
80-
| 158 | ps | ps | Truthy |
81-
| 159 | ps | ps | Truthy |
82-
| 160 | Compare | ps | Is None |
83-
| 160 | ps | ps | Truthy |
84-
| 171 | __name__ | __name__ | Truthy |
85-
| 172 | None | None | Truthy |
86-
| 174 | func | func | Truthy |
87-
| 175 | Exception | Exception | Truthy |
88-
| 176 | count | count | Truthy |
89-
| 177 | Compare | escapes | Is None |
90-
| 177 | None | None | Truthy |
91-
| 177 | escapes | escapes | Truthy |
92-
| 178 | count | count | Truthy |
93-
| 180 | count | count | Truthy |
94-
| 188 | true12 | true12 | Truthy |
95-
| 195 | Compare | x | < 4 |
96-
| 195 | x | x | Truthy |
97-
| 197 | Compare | x | < 4 |
98-
| 197 | x | x | Truthy |
99-
| 201 | Compare | x | < 4 |
100-
| 201 | x | x | Truthy |
101-
| 203 | Compare | x | >= 4 |
102-
| 203 | UnaryExpr | x | < 4 |
103-
| 203 | x | x | Truthy |
104-
| 207 | Compare | x | < 4 |
105-
| 207 | x | x | Truthy |
106-
| 209 | Compare | x | < 4 |
107-
| 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 |
1+
| 8 | test | test | Truthy | test |
2+
| 10 | test | test | Truthy | test |
3+
| 14 | seq | seq | Truthy | test |
4+
| 17 | seq | seq | Truthy | test |
5+
| 21 | UnaryExpr | t1 | Falsey | test |
6+
| 24 | t1 | t1 | Truthy | test |
7+
| 25 | t1 | t1 | Truthy | test |
8+
| 26 | t2 | t2 | Truthy | test |
9+
| 29 | t2 | t2 | Truthy | test |
10+
| 30 | t2 | t2 | Truthy | test |
11+
| 31 | t3 | t3 | Truthy | test |
12+
| 31 | t4 | t4 | Truthy | test |
13+
| 32 | t3 | t3 | Truthy | test |
14+
| 33 | t3 | t3 | Truthy | test |
15+
| 34 | t3 | t3 | Truthy | test |
16+
| 35 | t4 | t4 | Truthy | test |
17+
| 36 | t5 | t5 | Truthy | test |
18+
| 36 | t6 | t6 | Truthy | test |
19+
| 37 | t5 | t5 | Truthy | test |
20+
| 38 | t5 | t5 | Truthy | test |
21+
| 39 | t6 | t6 | Truthy | test |
22+
| 40 | t6 | t6 | Truthy | test |
23+
| 43 | t1 | t1 | Truthy | test |
24+
| 44 | UnaryExpr | t2 | Falsey | test |
25+
| 47 | t1 | t1 | Truthy | test |
26+
| 48 | t2 | t2 | Truthy | test |
27+
| 49 | t2 | t2 | Truthy | test |
28+
| 51 | t2 | t2 | Truthy | test |
29+
| 52 | t2 | t2 | Truthy | test |
30+
| 55 | seq1 | seq1 | Truthy | test |
31+
| 57 | UnaryExpr | seq2 | Falsey | test |
32+
| 60 | seq1 | seq1 | Truthy | test |
33+
| 63 | seq2 | seq2 | Truthy | test |
34+
| 66 | seq3 | seq3 | Truthy | test |
35+
| 68 | UnaryExpr | seq4 | Falsey | test |
36+
| 78 | seq5 | seq5 | Truthy | test |
37+
| 88 | UnaryExpr | x | Falsey | test |
38+
| 90 | y | y | Truthy | test |
39+
| 93 | UnaryExpr | x | Falsey | test |
40+
| 95 | y | y | Truthy | test |
41+
| 99 | another_module | another_module | Truthy | assign |
42+
| 102 | UnaryExpr | another_module | Falsey | test |
43+
| 107 | UnaryExpr | t1 | Falsey | test |
44+
| 109 | t2 | t2 | Truthy | test |
45+
| 111 | t1 | t1 | Truthy | test |
46+
| 113 | UnaryExpr | t2 | Falsey | test |
47+
| 117 | UnaryExpr | test | Falsey | test |
48+
| 119 | UnaryExpr | test | Falsey | test |
49+
| 123 | m | m | Truthy | test |
50+
| 126 | m | m | Truthy | test |
51+
| 158 | Compare | ps | Is not None | test |
52+
| 160 | Compare | ps | Is None | test |
53+
| 172 | escapes | escapes | Is None | assign |
54+
| 177 | Compare | escapes | Is None | test |
55+
| 191 | true12 | true12 | == 0 | assign |
56+
| 195 | Compare | x | < 4 | test |
57+
| 197 | Compare | x | < 4 | test |
58+
| 201 | Compare | x | < 4 | test |
59+
| 203 | UnaryExpr | x | < 4 | test |
60+
| 207 | Compare | x | < 4 | test |
61+
| 209 | Compare | x | < 4 | test |
62+
| 215 | x | x | Truthy | test |
63+
| 215 | y | y | Truthy | test |
64+
| 217 | x | x | Truthy | test |
65+
| 217 | y | y | Truthy | test |
66+
| 219 | x | x | Truthy | test |
67+
| 223 | y | y | Truthy | test |
68+
| 229 | k | k | Falsey | assign |
69+
| 230 | k | k | Truthy | test |
70+
| 237 | k | k | == 3 | assign |
71+
| 238 | k | k | Truthy | test |
72+
| 245 | k | k | Is None | assign |
73+
| 246 | k | k | Truthy | test |
74+
| 253 | a | a | Truthy | test |
75+
| 254 | k | k | Truthy | assign |
76+
| 256 | k | k | Falsey | assign |
77+
| 257 | k | k | Truthy | test |
78+
| 264 | var | var | Truthy | assign |
79+
| 266 | var | var | Falsey | assign |
80+
| 267 | var | var | Truthy | test |

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

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,12 @@ import python
33

44
import semmle.python.Pruning
55

6-
from Pruner::Constraint c, SsaVariable var, Pruner::UnprunedCfgNode node, int line
7-
where c = Pruner::constraintFromTest(var, node) and line = node.getNode().getLocation().getStartLine() and
8-
line > 0
9-
select line, node.getNode().toString(), var.getId(), c
6+
from Pruner::Constraint c, SsaVariable var, Pruner::UnprunedCfgNode node, int line, string kind
7+
where line = node.getNode().getLocation().getStartLine() and line > 0 and
8+
(
9+
c = Pruner::constraintFromTest(var, node) and kind = "test"
10+
or
11+
c = Pruner::constraintFromAssignment(var, node) and kind = "assign"
12+
)
13+
select line, node.getNode().toString(), var.getId(), c, kind
1014

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,3 +31,10 @@
3131
| 219 | x | 222 | count |
3232
| 223 | y | 224 | count |
3333
| 223 | y | 226 | count |
34+
| 230 | k | 231 | count |
35+
| 238 | k | 241 | count |
36+
| 246 | k | 247 | count |
37+
| 257 | k | 258 | count |
38+
| 257 | k | 259 | Pass |
39+
| 267 | var | 268 | count |
40+
| 267 | var | 269 | Pass |

0 commit comments

Comments
 (0)