Skip to content

Commit 56c3a4d

Browse files
psygnisfivepsygnisfive
authored
Merge pull request github#1632 from markshannon/python-account-for-dynamically-defined-builtin-instances
Python points-to: track more instances.
2 parents bb19d45 + e6b27b3 commit 56c3a4d

14 files changed

+184
-29
lines changed

‎python/ql/src/Expressions/IncorrectComparisonUsingIs.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
import python
1414
import IsComparisons
1515

16-
from Compare comp, Cmpop op, ClassObject c, string alt
16+
from Compare comp, Cmpop op, ClassValue c, string alt
1717
where invalid_portable_is_comparison(comp, op, c) and
1818
not cpython_interned_constant(comp.getASubExpression()) and
1919
(op instanceof Is and alt = "==" or op instanceof IsNot and alt = "!=")

‎python/ql/src/Expressions/IsComparisons.qll

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7,25 +7,31 @@ predicate comparison_using_is(Compare comp, ControlFlowNode left, Cmpop op, Cont
77
)
88
}
99

10-
predicate overrides_eq_or_cmp(ClassObject c) {
10+
predicate overrides_eq_or_cmp(ClassValue c) {
1111
major_version() = 2 and c.hasAttribute("__eq__")
1212
or
13-
c.declaresAttribute("__eq__") and not c = theObjectType()
13+
c.declaresAttribute("__eq__") and not c = Value::named("object")
1414
or
15-
exists(ClassObject sup |
16-
sup = c.getASuperType() and not sup = theObjectType() |
15+
exists(ClassValue sup |
16+
sup = c.getASuperType() and not sup = Value::named("object") |
1717
sup.declaresAttribute("__eq__")
1818
)
1919
or
2020
major_version() = 2 and c.hasAttribute("__cmp__")
2121
}
2222

23-
predicate invalid_to_use_is_portably(ClassObject c) {
23+
predicate probablySingleton(ClassValue cls) {
24+
strictcount(Value inst | inst.getClass() = cls) = 1
25+
or
26+
cls = Value::named("None").getClass()
27+
}
28+
29+
predicate invalid_to_use_is_portably(ClassValue c) {
2430
overrides_eq_or_cmp(c) and
2531
/* Exclude type/builtin-function/bool as it is legitimate to compare them using 'is' but they implement __eq__ */
26-
not c = theTypeType() and not c = theBuiltinFunctionType() and not c = theBoolType() and
32+
not c = Value::named("type") and not c = ClassValue::builtinFunction() and not c = Value::named("bool") and
2733
/* OK to compare with 'is' if a singleton */
28-
not exists(c.getProbableSingletonInstance())
34+
not probablySingleton(c)
2935
}
3036

3137
predicate simple_constant(ControlFlowNode f) {
@@ -72,30 +78,30 @@ predicate universally_interned_constant(Expr e) {
7278
)
7379
}
7480

75-
private predicate comparison_both_types(Compare comp, Cmpop op, ClassObject cls1, ClassObject cls2) {
81+
private predicate comparison_both_types(Compare comp, Cmpop op, ClassValue cls1, ClassValue cls2) {
7682
exists(ControlFlowNode op1, ControlFlowNode op2 |
7783
comparison_using_is(comp, op1, op, op2) or comparison_using_is(comp, op2, op, op1) |
78-
op1.refersTo(_, cls1, _) and
79-
op2.refersTo(_, cls2, _)
84+
op1.inferredValue().getClass() = cls1 and
85+
op2.inferredValue().getClass() = cls2
8086
)
8187
}
8288

83-
private predicate comparison_one_type(Compare comp, Cmpop op, ClassObject cls) {
89+
private predicate comparison_one_type(Compare comp, Cmpop op, ClassValue cls) {
8490
not comparison_both_types(comp, _, _, _) and
8591
exists(ControlFlowNode operand |
8692
comparison_using_is(comp, operand, op, _) or comparison_using_is(comp, _, op, operand) |
87-
operand.refersTo(_, cls, _)
93+
operand.inferredValue().getClass() = cls
8894
)
8995
}
9096

91-
predicate invalid_portable_is_comparison(Compare comp, Cmpop op, ClassObject cls) {
97+
predicate invalid_portable_is_comparison(Compare comp, Cmpop op, ClassValue cls) {
9298
/* OK to use 'is' when defining '__eq__' */
9399
not exists(Function eq | eq.getName() = "__eq__" or eq.getName() = "__ne__" | eq = comp.getScope().getScope*())
94100
and
95101
(
96102
comparison_one_type(comp, op, cls) and invalid_to_use_is_portably(cls)
97103
or
98-
exists(ClassObject other | comparison_both_types(comp, op, cls, other) |
104+
exists(ClassValue other | comparison_both_types(comp, op, cls, other) |
99105
invalid_to_use_is_portably(cls) and
100106
invalid_to_use_is_portably(other)
101107
)
@@ -123,9 +129,9 @@ predicate invalid_portable_is_comparison(Compare comp, Cmpop op, ClassObject cls
123129
}
124130

125131
private predicate enum_member(AstNode obj) {
126-
exists(ClassObject cls, AssignStmt asgn |
132+
exists(ClassValue cls, AssignStmt asgn |
127133
cls.getASuperType().getName() = "Enum" |
128-
cls.getPyClass() = asgn.getScope() and
134+
cls.getScope() = asgn.getScope() and
129135
asgn.getValue() = obj
130136
)
131137
}

‎python/ql/src/Expressions/NonPortableComparisonUsingIs.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
import python
1414
import IsComparisons
1515

16-
from Compare comp, Cmpop op, ClassObject c
16+
from Compare comp, Cmpop op, ClassValue c
1717
where invalid_portable_is_comparison(comp, op, c) and
1818
exists(Expr sub |
1919
sub = comp.getASubExpression() |

‎python/ql/src/semmle/python/objects/Constants.qll

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,14 @@ abstract class ConstantObjectInternal extends ObjectInternal {
7878

7979
}
8080

81+
private boolean callToBool(CallNode call, PointsToContext context) {
82+
PointsToInternal::pointsTo(call.getFunction(), context, ClassValue::bool(), _) and
83+
exists(ObjectInternal arg |
84+
PointsToInternal::pointsTo(call.getArg(0),context, arg, _) and
85+
arg.booleanValue() = result
86+
)
87+
}
88+
8189
private abstract class BooleanObjectInternal extends ConstantObjectInternal {
8290

8391
override ObjectInternal getClass() {
@@ -111,6 +119,8 @@ private class TrueObjectInternal extends BooleanObjectInternal, TTrue {
111119

112120
override predicate introducedAt(ControlFlowNode node, PointsToContext context) {
113121
node.(NameNode).getId() = "True" and context.appliesTo(node)
122+
or
123+
callToBool(node, context) = true
114124
}
115125

116126
override int intValue() {
@@ -135,6 +145,8 @@ private class FalseObjectInternal extends BooleanObjectInternal, TFalse {
135145

136146
override predicate introducedAt(ControlFlowNode node, PointsToContext context) {
137147
node.(NameNode).getId() = "False" and context.appliesTo(node)
148+
or
149+
callToBool(node, context) = false
138150
}
139151

140152
override int intValue() {

‎python/ql/src/semmle/python/objects/TObject.qll

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -469,7 +469,7 @@ library class ClassDecl extends @py_object {
469469
result = this.getClass().getName()
470470
}
471471

472-
/** Whether this is a class whose instances we treat specially, rather than as a generic instance.
472+
/** Whether this is a class whose instances must be treated specially, rather than as generic instances.
473473
*/
474474
predicate isSpecial() {
475475
exists(string name |
@@ -478,20 +478,13 @@ library class ClassDecl extends @py_object {
478478
name = "super" or
479479
name = "bool" or
480480
name = "NoneType" or
481-
name = "int" or
482-
name = "long" or
483-
name = "str" or
484-
name = "bytes" or
485-
name = "unicode" or
486481
name = "tuple" or
487482
name = "property" or
488483
name = "ClassMethod" or
489484
name = "StaticMethod" or
490485
name = "MethodType" or
491486
name = "ModuleType"
492487
)
493-
or
494-
this = Builtin::builtin("float")
495488
}
496489

497490
/** Holds if for class `C`, `C()` returns an instance of `C` */

‎python/ql/src/semmle/python/pointsto/PointsTo.qll

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1249,7 +1249,13 @@ module Expressions {
12491249
not op instanceof BitOr and
12501250
(operand = left or operand = right) and
12511251
PointsToInternal::pointsTo(operand, context, opvalue, _) and
1252-
value = ObjectInternal::unknown()
1252+
(
1253+
op instanceof Add and
1254+
value = TUnknownInstance(opvalue.getClass())
1255+
or
1256+
not op instanceof Add and
1257+
value = ObjectInternal::unknown()
1258+
)
12531259
or
12541260
op instanceof BitOr and
12551261
exists(ObjectInternal lobj, ObjectInternal robj |

‎python/ql/test/library-tests/PointsTo/general/LocalPointsTo.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,7 @@
159159
| 106 | ControlFlowNode for inner | Function inner |
160160
| 108 | ControlFlowNode for IntegerLiteral | int 2 |
161161
| 108 | ControlFlowNode for z | int 2 |
162+
| 109 | ControlFlowNode for BinaryExpr | BinaryExpr |
162163
| 109 | ControlFlowNode for y | int 1 |
163164
| 109 | ControlFlowNode for z | int 2 |
164165
| 110 | ControlFlowNode for inner | Function inner |

‎python/ql/test/library-tests/PointsTo/general/LocalPointsToType.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,7 @@
159159
| 106 | ControlFlowNode for inner | Function inner | builtin-class function |
160160
| 108 | ControlFlowNode for IntegerLiteral | int 2 | builtin-class int |
161161
| 108 | ControlFlowNode for z | int 2 | builtin-class int |
162+
| 109 | ControlFlowNode for BinaryExpr | BinaryExpr | builtin-class int |
162163
| 109 | ControlFlowNode for y | int 1 | builtin-class int |
163164
| 109 | ControlFlowNode for z | int 2 | builtin-class int |
164165
| 110 | ControlFlowNode for inner | Function inner | builtin-class function |

‎python/ql/test/library-tests/PointsTo/new/PointsToUnknown.expected

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -167,12 +167,10 @@
167167
| c_tests.py:63 | ControlFlowNode for cond | 63 |
168168
| c_tests.py:73 | ControlFlowNode for x | 71 |
169169
| c_tests.py:73 | ControlFlowNode for y | 71 |
170-
| c_tests.py:74 | ControlFlowNode for BinaryExpr | 74 |
171170
| c_tests.py:74 | ControlFlowNode for x | 71 |
172171
| c_tests.py:74 | ControlFlowNode for y | 71 |
173172
| c_tests.py:76 | ControlFlowNode for x | 71 |
174173
| c_tests.py:76 | ControlFlowNode for y | 71 |
175-
| c_tests.py:77 | ControlFlowNode for BinaryExpr | 77 |
176174
| c_tests.py:77 | ControlFlowNode for x | 71 |
177175
| c_tests.py:77 | ControlFlowNode for y | 71 |
178176
| c_tests.py:80 | ControlFlowNode for IfExp | 80 |

‎python/ql/test/library-tests/PointsTo/new/PointsToWithContext.expected

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -416,6 +416,22 @@ WARNING: Predicate points_to has been deprecated and may be removed in future (P
416416
| h_classes.py:50 | ControlFlowNode for m | Function f | builtin-class function | 45 | import |
417417
| h_classes.py:52 | ControlFlowNode for FunctionExpr | Function n | builtin-class function | 52 | import |
418418
| h_classes.py:52 | ControlFlowNode for n | Function n | builtin-class function | 52 | import |
419+
| h_classes.py:55 | ControlFlowNode for int | builtin-class int | builtin-class type | 55 | import |
420+
| h_classes.py:55 | ControlFlowNode for int() | int() | builtin-class int | 55 | import |
421+
| h_classes.py:56 | ControlFlowNode for Str | '' | builtin-class str | 56 | import |
422+
| h_classes.py:56 | ControlFlowNode for type | builtin-class type | builtin-class type | 56 | import |
423+
| h_classes.py:56 | ControlFlowNode for type() | builtin-class str | builtin-class type | 56 | import |
424+
| h_classes.py:56 | ControlFlowNode for type()() | type()() | builtin-class str | 56 | import |
425+
| h_classes.py:57 | ControlFlowNode for list | builtin-class list | builtin-class type | 57 | import |
426+
| h_classes.py:57 | ControlFlowNode for list() | list() | builtin-class list | 57 | import |
427+
| h_classes.py:58 | ControlFlowNode for dict | builtin-class dict | builtin-class type | 58 | import |
428+
| h_classes.py:58 | ControlFlowNode for dict() | dict() | builtin-class dict | 58 | import |
429+
| h_classes.py:59 | ControlFlowNode for Str | 'hi' | builtin-class str | 59 | import |
430+
| h_classes.py:59 | ControlFlowNode for bool | builtin-class bool | builtin-class type | 59 | import |
431+
| h_classes.py:59 | ControlFlowNode for bool() | bool True | builtin-class bool | 59 | import |
432+
| h_classes.py:60 | ControlFlowNode for IntegerLiteral | int 0 | builtin-class int | 60 | import |
433+
| h_classes.py:60 | ControlFlowNode for bool | builtin-class bool | builtin-class type | 60 | import |
434+
| h_classes.py:60 | ControlFlowNode for bool() | bool False | builtin-class bool | 60 | import |
419435
| i_imports.py:3 | ControlFlowNode for IntegerLiteral | int 1 | builtin-class int | 3 | import |
420436
| i_imports.py:3 | ControlFlowNode for a | int 1 | builtin-class int | 3 | import |
421437
| i_imports.py:4 | ControlFlowNode for IntegerLiteral | int 2 | builtin-class int | 4 | import |

0 commit comments

Comments
 (0)