From 1b432076c4bb5835b068d4bf86b1164d0f82ccb5 Mon Sep 17 00:00:00 2001 From: Taus Brock-Nannestad Date: Tue, 3 Sep 2019 13:38:46 +0200 Subject: [PATCH 1/4] Python: Prevent divergence in type-hint analysis. (ODASA-8075) --- python/ql/src/semmle/python/objects/Callables.qll | 8 ++++++++ python/ql/src/semmle/python/objects/Classes.qll | 4 ++++ python/ql/src/semmle/python/objects/Constants.qll | 2 ++ python/ql/src/semmle/python/objects/Descriptors.qll | 8 ++++++++ python/ql/src/semmle/python/objects/Instances.qll | 6 ++++++ python/ql/src/semmle/python/objects/Modules.qll | 4 ++++ python/ql/src/semmle/python/objects/ObjectInternal.qll | 10 ++++++++++ python/ql/src/semmle/python/objects/Sequences.qll | 8 ++++++++ python/ql/src/semmle/python/objects/TObject.qll | 2 ++ 9 files changed, 52 insertions(+) diff --git a/python/ql/src/semmle/python/objects/Callables.qll b/python/ql/src/semmle/python/objects/Callables.qll index 14cb0615a4e8..ba7d40ad7a56 100644 --- a/python/ql/src/semmle/python/objects/Callables.qll +++ b/python/ql/src/semmle/python/objects/Callables.qll @@ -163,6 +163,8 @@ class PythonFunctionObjectInternal extends CallableObjectInternal, TPythonFuncti override predicate useOriginAsLegacyObject() { none() } + override predicate isNotSubscriptedType() { any() } + } @@ -288,6 +290,8 @@ class BuiltinFunctionObjectInternal extends CallableObjectInternal, TBuiltinFunc override predicate useOriginAsLegacyObject() { none() } + override predicate isNotSubscriptedType() { any() } + } /** Class representing methods of built-in classes (otherwise known as method-descriptors) such as `list.append`. @@ -382,6 +386,8 @@ class BuiltinMethodObjectInternal extends CallableObjectInternal, TBuiltinMethod override predicate useOriginAsLegacyObject() { none() } + override predicate isNotSubscriptedType() { any() } + } /** Class representing bound-methods. @@ -473,4 +479,6 @@ class BoundMethodObjectInternal extends CallableObjectInternal, TBoundMethod { this.getFunction().contextSensitiveCallee() } + override predicate isNotSubscriptedType() { any() } + } \ No newline at end of file diff --git a/python/ql/src/semmle/python/objects/Classes.qll b/python/ql/src/semmle/python/objects/Classes.qll index 962f3c00909a..1e872e70c766 100644 --- a/python/ql/src/semmle/python/objects/Classes.qll +++ b/python/ql/src/semmle/python/objects/Classes.qll @@ -103,6 +103,8 @@ abstract class ClassObjectInternal extends ObjectInternal { Types::getBase(this, _).hasAttribute(name) } + override predicate isNotSubscriptedType() { any() } + } /** Class representing Python source classes */ @@ -445,6 +447,8 @@ class SubscriptedTypeInternal extends ObjectInternal, TSubscriptedType { /* Classes aren't usually iterable, but can e.g. Enums */ override ObjectInternal getIterNext() { result = ObjectInternal::unknown() } + override predicate isNotSubscriptedType() { none() } + } diff --git a/python/ql/src/semmle/python/objects/Constants.qll b/python/ql/src/semmle/python/objects/Constants.qll index 48c8872b66c9..1b93a3bb6f26 100644 --- a/python/ql/src/semmle/python/objects/Constants.qll +++ b/python/ql/src/semmle/python/objects/Constants.qll @@ -76,6 +76,8 @@ abstract class ConstantObjectInternal extends ObjectInternal { /** Gets an AST literal with the same value as this object */ abstract ImmutableLiteral getLiteral(); + override predicate isNotSubscriptedType() { any() } + } pragma[nomagic] diff --git a/python/ql/src/semmle/python/objects/Descriptors.qll b/python/ql/src/semmle/python/objects/Descriptors.qll index 348f58396aa7..eb6859f0c0f0 100644 --- a/python/ql/src/semmle/python/objects/Descriptors.qll +++ b/python/ql/src/semmle/python/objects/Descriptors.qll @@ -101,6 +101,8 @@ class PropertyInternal extends ObjectInternal, TProperty { /* Properties aren't iterable */ override ObjectInternal getIterNext() { none() } + override predicate isNotSubscriptedType() { any() } + } private class PropertySetterOrDeleter extends ObjectInternal, TPropertySetterOrDeleter { @@ -174,6 +176,8 @@ private class PropertySetterOrDeleter extends ObjectInternal, TPropertySetterOrD override predicate useOriginAsLegacyObject() { none() } + override predicate isNotSubscriptedType() { any() } + } @@ -267,6 +271,8 @@ class ClassMethodObjectInternal extends ObjectInternal, TClassMethod { /* Classmethods aren't iterable */ override ObjectInternal getIterNext() { none() } + override predicate isNotSubscriptedType() { any() } + } class StaticMethodObjectInternal extends ObjectInternal, TStaticMethod { @@ -345,4 +351,6 @@ class StaticMethodObjectInternal extends ObjectInternal, TStaticMethod { /* Staticmethods aren't iterable */ override ObjectInternal getIterNext() { none() } + override predicate isNotSubscriptedType() { any() } + } diff --git a/python/ql/src/semmle/python/objects/Instances.qll b/python/ql/src/semmle/python/objects/Instances.qll index 47b4a9bc55ac..ab5a12b50704 100644 --- a/python/ql/src/semmle/python/objects/Instances.qll +++ b/python/ql/src/semmle/python/objects/Instances.qll @@ -55,6 +55,8 @@ abstract class InstanceObject extends ObjectInternal { override ObjectInternal getIterNext() { result = ObjectInternal::unknown() } + override predicate isNotSubscriptedType() { any() } + } private predicate self_variable_reaching_init_exit(EssaVariable self) { @@ -387,6 +389,8 @@ class UnknownInstanceInternal extends TUnknownInstance, ObjectInternal { override ObjectInternal getIterNext() { result = ObjectInternal::unknown() } + override predicate isNotSubscriptedType() { any() } + } private int lengthFromClass(ClassObjectInternal cls) { @@ -499,5 +503,7 @@ class SuperInstance extends TSuperInstance, ObjectInternal { override ObjectInternal getIterNext() { result = ObjectInternal::unknown() } + override predicate isNotSubscriptedType() { any() } + } diff --git a/python/ql/src/semmle/python/objects/Modules.qll b/python/ql/src/semmle/python/objects/Modules.qll index 74feeb9c5a03..98e1000ed0a2 100644 --- a/python/ql/src/semmle/python/objects/Modules.qll +++ b/python/ql/src/semmle/python/objects/Modules.qll @@ -71,6 +71,8 @@ abstract class ModuleObjectInternal extends ObjectInternal { py_exports(this.getSourceModule(), name) } + override predicate isNotSubscriptedType() { any() } + } /** A class representing built-in modules */ @@ -445,5 +447,7 @@ class AbsentModuleAttributeObjectInternal extends ObjectInternal, TAbsentModuleA /* Modules aren't iterable */ override ObjectInternal getIterNext() { none() } + override predicate isNotSubscriptedType() { any() } + } diff --git a/python/ql/src/semmle/python/objects/ObjectInternal.qll b/python/ql/src/semmle/python/objects/ObjectInternal.qll index e57a01db7e97..09f6e883ae14 100644 --- a/python/ql/src/semmle/python/objects/ObjectInternal.qll +++ b/python/ql/src/semmle/python/objects/ObjectInternal.qll @@ -187,6 +187,8 @@ class ObjectInternal extends TObject { this.(ObjectInternal).attribute(name, _, _) } + abstract predicate isNotSubscriptedType(); + } @@ -276,6 +278,8 @@ class BuiltinOpaqueObjectInternal extends ObjectInternal, TBuiltinOpaqueObject { override ObjectInternal getIterNext() { result = ObjectInternal::unknown() } + override predicate isNotSubscriptedType() { any() } + } @@ -359,6 +363,8 @@ class UnknownInternal extends ObjectInternal, TUnknown { override ObjectInternal getIterNext() { result = ObjectInternal::unknown() } + override predicate isNotSubscriptedType() { any() } + } class UndefinedInternal extends ObjectInternal, TUndefined { @@ -445,6 +451,8 @@ class UndefinedInternal extends ObjectInternal, TUndefined { override ObjectInternal getIterNext() { none() } + override predicate isNotSubscriptedType() { any() } + } module ObjectInternal { @@ -630,6 +638,8 @@ class DecoratedFunction extends ObjectInternal, TDecoratedFunction { override predicate useOriginAsLegacyObject() { none() } + override predicate isNotSubscriptedType() { any() } + } /** Helper for boolean predicates returning both `true` and `false` */ diff --git a/python/ql/src/semmle/python/objects/Sequences.qll b/python/ql/src/semmle/python/objects/Sequences.qll index 83679c78fe83..e161cec01a90 100644 --- a/python/ql/src/semmle/python/objects/Sequences.qll +++ b/python/ql/src/semmle/python/objects/Sequences.qll @@ -131,6 +131,8 @@ class BuiltinTupleObjectInternal extends TBuiltinTuple, TupleObjectInternal { override predicate useOriginAsLegacyObject() { none() } + override predicate isNotSubscriptedType() { any() } + } /** A tuple declared by a tuple expression in the Python source code */ @@ -164,6 +166,8 @@ class PythonTupleObjectInternal extends TPythonTuple, TupleObjectInternal { override predicate useOriginAsLegacyObject() { none() } + override predicate isNotSubscriptedType() { any() } + } /** A tuple created by a `*` parameter */ @@ -195,6 +199,8 @@ class VarargsTupleObjectInternal extends TVarargsTuple, TupleObjectInternal { override predicate useOriginAsLegacyObject() { any() } + override predicate isNotSubscriptedType() { any() } + } @@ -277,4 +283,6 @@ class SysVersionInfoObjectInternal extends TSysVersionInfo, SequenceObjectIntern override predicate useOriginAsLegacyObject() { any() } + override predicate isNotSubscriptedType() { any() } + } diff --git a/python/ql/src/semmle/python/objects/TObject.qll b/python/ql/src/semmle/python/objects/TObject.qll index 58a089ef8f16..855351cbdd9c 100644 --- a/python/ql/src/semmle/python/objects/TObject.qll +++ b/python/ql/src/semmle/python/objects/TObject.qll @@ -240,6 +240,8 @@ cached newtype TObject = /* Represents a subscript operation applied to a type. For type-hint analysis */ TSubscriptedType(ObjectInternal generic, ObjectInternal index) { isType(generic) and + generic.isNotSubscriptedType() and + index.isNotSubscriptedType() and Expressions::subscriptPartsPointsTo(_, _, generic, index) } From 4440e02fa5faf1bfb001a4dea71efe7862deba02 Mon Sep 17 00:00:00 2001 From: Taus Brock-Nannestad Date: Wed, 4 Sep 2019 13:23:06 +0200 Subject: [PATCH 2/4] Add test case for divergence. --- .../3/library-tests/PointsTo/typehints/Values.expected | 7 +++++++ python/ql/test/3/library-tests/PointsTo/typehints/test.py | 8 ++++++++ 2 files changed, 15 insertions(+) diff --git a/python/ql/test/3/library-tests/PointsTo/typehints/Values.expected b/python/ql/test/3/library-tests/PointsTo/typehints/Values.expected index 692ebaba7afc..ffb65e13956f 100644 --- a/python/ql/test/3/library-tests/PointsTo/typehints/Values.expected +++ b/python/ql/test/3/library-tests/PointsTo/typehints/Values.expected @@ -10,3 +10,10 @@ | test.py:6:1:6:20 | test.py:6 | ControlFlowNode for FunctionExpr | import | test.py:6:1:6:20 | Function bar | | test.py:6:11:6:13 | test.py:6 | ControlFlowNode for set | import | file://:0:0:0:0 | builtin-class set | | test.py:6:17:6:19 | test.py:6 | ControlFlowNode for Set | import | ../../lib/typing.py:23:1:23:23 | class Set | +| test.py:11:1:11:12 | test.py:11 | ControlFlowNode for ClassExpr | import | test.py:11:1:11:12 | class baz | +| test.py:14:7:14:10 | test.py:14 | ControlFlowNode for True | import | file://:0:0:0:0 | bool True | +| test.py:15:11:15:13 | test.py:15 | ControlFlowNode for baz | import | file://:0:0:0:0 | class baz[class baz] | +| test.py:15:11:15:13 | test.py:15 | ControlFlowNode for baz | import | test.py:11:1:11:12 | class baz | +| test.py:15:11:15:18 | test.py:15 | ControlFlowNode for Subscript | import | file://:0:0:0:0 | class baz[class baz] | +| test.py:15:15:15:17 | test.py:15 | ControlFlowNode for baz | import | file://:0:0:0:0 | class baz[class baz] | +| test.py:15:15:15:17 | test.py:15 | ControlFlowNode for baz | import | test.py:11:1:11:12 | class baz | diff --git a/python/ql/test/3/library-tests/PointsTo/typehints/test.py b/python/ql/test/3/library-tests/PointsTo/typehints/test.py index b3214d3c16bb..cd39f6e079a3 100644 --- a/python/ql/test/3/library-tests/PointsTo/typehints/test.py +++ b/python/ql/test/3/library-tests/PointsTo/typehints/test.py @@ -5,3 +5,11 @@ def foo(x:Optional[int]) -> int: def bar(s:set)->Set: pass + + +# ODASA-8075 +class baz(): + pass + +while True: + baz = baz[baz] From 2d45c23d19ab8475a6e32b44c07ee139327b6c75 Mon Sep 17 00:00:00 2001 From: Taus Brock-Nannestad Date: Thu, 5 Sep 2019 13:18:01 +0200 Subject: [PATCH 3/4] Comment out diverging example for now. Otherwise it'll keep timing out until the fix has been pushed to LGTM.com --- .../library-tests/PointsTo/typehints/Values.expected | 7 ------- .../test/3/library-tests/PointsTo/typehints/test.py | 11 ++++++----- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/python/ql/test/3/library-tests/PointsTo/typehints/Values.expected b/python/ql/test/3/library-tests/PointsTo/typehints/Values.expected index ffb65e13956f..692ebaba7afc 100644 --- a/python/ql/test/3/library-tests/PointsTo/typehints/Values.expected +++ b/python/ql/test/3/library-tests/PointsTo/typehints/Values.expected @@ -10,10 +10,3 @@ | test.py:6:1:6:20 | test.py:6 | ControlFlowNode for FunctionExpr | import | test.py:6:1:6:20 | Function bar | | test.py:6:11:6:13 | test.py:6 | ControlFlowNode for set | import | file://:0:0:0:0 | builtin-class set | | test.py:6:17:6:19 | test.py:6 | ControlFlowNode for Set | import | ../../lib/typing.py:23:1:23:23 | class Set | -| test.py:11:1:11:12 | test.py:11 | ControlFlowNode for ClassExpr | import | test.py:11:1:11:12 | class baz | -| test.py:14:7:14:10 | test.py:14 | ControlFlowNode for True | import | file://:0:0:0:0 | bool True | -| test.py:15:11:15:13 | test.py:15 | ControlFlowNode for baz | import | file://:0:0:0:0 | class baz[class baz] | -| test.py:15:11:15:13 | test.py:15 | ControlFlowNode for baz | import | test.py:11:1:11:12 | class baz | -| test.py:15:11:15:18 | test.py:15 | ControlFlowNode for Subscript | import | file://:0:0:0:0 | class baz[class baz] | -| test.py:15:15:15:17 | test.py:15 | ControlFlowNode for baz | import | file://:0:0:0:0 | class baz[class baz] | -| test.py:15:15:15:17 | test.py:15 | ControlFlowNode for baz | import | test.py:11:1:11:12 | class baz | diff --git a/python/ql/test/3/library-tests/PointsTo/typehints/test.py b/python/ql/test/3/library-tests/PointsTo/typehints/test.py index cd39f6e079a3..e0742602d038 100644 --- a/python/ql/test/3/library-tests/PointsTo/typehints/test.py +++ b/python/ql/test/3/library-tests/PointsTo/typehints/test.py @@ -8,8 +8,9 @@ def bar(s:set)->Set: # ODASA-8075 -class baz(): - pass - -while True: - baz = baz[baz] +# Commented out until the fix has been pushed to LGTM.com +#class baz(): +# pass +# +#while True: +# baz = baz[baz] From 8882f1410a5f1a2735f5b0d24755934cfb6e51e8 Mon Sep 17 00:00:00 2001 From: Taus Brock-Nannestad Date: Fri, 6 Sep 2019 12:01:18 +0200 Subject: [PATCH 4/4] Add test cases for nested subscripts. --- .../3/library-tests/PointsTo/typehints/Values.expected | 9 +++++++++ .../ql/test/3/library-tests/PointsTo/typehints/test.py | 2 ++ 2 files changed, 11 insertions(+) diff --git a/python/ql/test/3/library-tests/PointsTo/typehints/Values.expected b/python/ql/test/3/library-tests/PointsTo/typehints/Values.expected index 692ebaba7afc..5b81d84d42a2 100644 --- a/python/ql/test/3/library-tests/PointsTo/typehints/Values.expected +++ b/python/ql/test/3/library-tests/PointsTo/typehints/Values.expected @@ -10,3 +10,12 @@ | test.py:6:1:6:20 | test.py:6 | ControlFlowNode for FunctionExpr | import | test.py:6:1:6:20 | Function bar | | test.py:6:11:6:13 | test.py:6 | ControlFlowNode for set | import | file://:0:0:0:0 | builtin-class set | | test.py:6:17:6:19 | test.py:6 | ControlFlowNode for Set | import | ../../lib/typing.py:23:1:23:23 | class Set | +| test.py:9:6:9:13 | test.py:9 | ControlFlowNode for Optional | import | ../../lib/typing.py:18:12:18:32 | _Optional() | +| test.py:9:6:9:28 | test.py:9 | ControlFlowNode for Subscript | import | file://:0:0:0:0 | _Optional()[Unknown value] | +| test.py:9:15:9:22 | test.py:9 | ControlFlowNode for Optional | import | ../../lib/typing.py:18:12:18:32 | _Optional() | +| test.py:9:15:9:27 | test.py:9 | ControlFlowNode for Subscript | import | file://:0:0:0:0 | _Optional()[builtin-class int] | +| test.py:9:24:9:26 | test.py:9 | ControlFlowNode for int | import | file://:0:0:0:0 | builtin-class int | +| test.py:10:6:10:13 | test.py:10 | ControlFlowNode for Optional | import | ../../lib/typing.py:18:12:18:32 | _Optional() | +| test.py:10:6:10:18 | test.py:10 | ControlFlowNode for Subscript | import | file://:0:0:0:0 | _Optional()[builtin-class int] | +| test.py:10:15:10:17 | test.py:10 | ControlFlowNode for int | import | file://:0:0:0:0 | builtin-class int | +| test.py:10:20:10:22 | test.py:10 | ControlFlowNode for int | import | file://:0:0:0:0 | builtin-class int | diff --git a/python/ql/test/3/library-tests/PointsTo/typehints/test.py b/python/ql/test/3/library-tests/PointsTo/typehints/test.py index e0742602d038..2f823db79198 100644 --- a/python/ql/test/3/library-tests/PointsTo/typehints/test.py +++ b/python/ql/test/3/library-tests/PointsTo/typehints/test.py @@ -6,6 +6,8 @@ def foo(x:Optional[int]) -> int: def bar(s:set)->Set: pass +t1 = Optional[Optional[int]] +t2 = Optional[int][int] # ODASA-8075 # Commented out until the fix has been pushed to LGTM.com