Skip to content

Commit 1f9899d

Browse files
committed
Extend added type tracking step to related types
1 parent dd61dd2 commit 1f9899d

3 files changed

Lines changed: 105 additions & 10 deletions

File tree

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
---
22
category: minorAnalysis
33
---
4-
* Python type tracking now follows values stored in instance attributes such as `self.attr` across instance methods on the same class. As a result, analysis is more likely to recognize user-defined objects that are stored on `self` and used later in other methods, which may produce additional results.
4+
* Python type tracking now follows values stored in instance attributes such as `self.attr` across instance methods, including across a class hierarchy (for example, a value stored on `self.attr` in a base class and read in a subclass, or vice versa). As a result, analysis is more likely to recognize user-defined objects that are stored on `self` and used later in other methods, which may produce additional results.

python/ql/lib/semmle/python/dataflow/new/internal/TypeTrackingImpl.qll

Lines changed: 57 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,8 @@ module TypeTrackingInput implements Shared::TypeTrackingInput<Location> {
169169
/** Holds if there is a level step from `nodeFrom` to `nodeTo`, which may depend on the call graph. */
170170
predicate levelStepCall(Node nodeFrom, LocalSourceNode nodeTo) {
171171
instanceFieldStep(nodeFrom, nodeTo)
172+
or
173+
inheritedFieldStep(nodeFrom, nodeTo)
172174
}
173175

174176
/** Holds if there is a level step from `nodeFrom` to `nodeTo`, which does not depend on the call graph. */
@@ -366,6 +368,11 @@ module TypeTrackingInput implements Shared::TypeTrackingInput<Location> {
366368
* the written value to the read reference, for any pair of methods on the class (not
367369
* just from `__init__`).
368370
*
371+
* Flow across the class hierarchy (a write in one class observed in a method inherited
372+
* from, or contributed by, a related class) is handled separately by
373+
* `inheritedFieldStep`, because resolving superclasses depends on the call graph and so
374+
* cannot appear in this call-graph-independent step.
375+
*
369376
* This is an over-approximation: it is instance-insensitive (it does not distinguish
370377
* between different instances of the same class) and order-insensitive (it does not
371378
* require the write to happen before the read), matching the precision of
@@ -380,25 +387,66 @@ module TypeTrackingInput implements Shared::TypeTrackingInput<Location> {
380387
)
381388
}
382389

390+
/**
391+
* Holds if `nodeFrom` is written to attribute `self.attr` in an instance method of one
392+
* class, and `nodeTo` reads attribute `self.attr` in an instance method of a different
393+
* class that is related to it by inheritance (one is a transitive superclass of the
394+
* other).
395+
*
396+
* This is the cross-hierarchy counterpart of `localFieldStep`: at runtime the receiver
397+
* of both methods may be an instance of the more-derived class, whose behaviour is made
398+
* up of the methods it declares together with those inherited from all of its ancestors.
399+
* It therefore models the common pattern of a base class storing `self.attr` that a
400+
* subclass reads, and vice versa. Resolving the superclass relationship depends on the
401+
* call graph (via `getADirectSuperclass`), so this step is reported as `levelStepCall`
402+
* rather than `levelStepNoCall`.
403+
*
404+
* Like `localFieldStep`, this is an over-approximation: it is both instance-insensitive
405+
* and order-insensitive.
406+
*/
407+
private predicate inheritedFieldStep(Node nodeFrom, LocalSourceNode nodeTo) {
408+
exists(
409+
Class writeCls, Class readCls, string attr, DataFlowPublic::AttrWrite write,
410+
DataFlowPublic::AttrRead read
411+
|
412+
selfAttrRef(writeCls, attr, write) and
413+
nodeFrom = write.getValue() and
414+
selfAttrRef(readCls, attr, read) and
415+
nodeTo = read and
416+
writeCls != readCls and
417+
(
418+
writeCls = DataFlowDispatch::getADirectSuperclass*(readCls)
419+
or
420+
readCls = DataFlowDispatch::getADirectSuperclass*(writeCls)
421+
)
422+
)
423+
}
424+
383425
/**
384426
* Holds if `nodeFrom` is written to attribute `self.attr` in some instance method of a
385-
* class, and `nodeTo` reads attribute `attr` from an instance of the same class outside
386-
* its methods (e.g. `instance.attr`).
427+
* class, and `nodeTo` reads attribute `attr` from an instance of that class (or a
428+
* subclass of it) outside its methods (e.g. `instance.attr`).
387429
*
388430
* This is the cross-instance counterpart of `localFieldStep`: it relates a write of
389-
* `self.attr` inside the class to a read of `attr` on a reference to an instance of the
390-
* class. Identifying instances relies on the call graph (via `classInstanceTracker`), so
391-
* this step is reported as `levelStepCall` rather than `levelStepNoCall`.
431+
* `self.attr` inside a class to a read of `attr` on a reference to an instance of that
432+
* class or one of its subclasses. Identifying instances relies on the call graph (via
433+
* `classInstanceTracker`), so this step is reported as `levelStepCall` rather than
434+
* `levelStepNoCall`. The write may occur in the instance's own class or in any of its
435+
* superclasses, since those methods are inherited.
392436
*
393437
* Like `localFieldStep`, this is an over-approximation: it is both instance-insensitive
394438
* and order-insensitive.
395439
*/
396440
private predicate instanceFieldStep(Node nodeFrom, LocalSourceNode nodeTo) {
397-
exists(Class cls, string attr, DataFlowPublic::AttrWrite write, DataFlowPublic::AttrRead read |
398-
selfAttrRef(cls, attr, write) and
441+
exists(
442+
Class writeCls, Class instanceCls, string attr, DataFlowPublic::AttrWrite write,
443+
DataFlowPublic::AttrRead read
444+
|
445+
selfAttrRef(writeCls, attr, write) and
399446
nodeFrom = write.getValue() and
400-
instanceAttrRead(cls, attr, read) and
401-
nodeTo = read
447+
instanceAttrRead(instanceCls, attr, read) and
448+
nodeTo = read and
449+
writeCls = DataFlowDispatch::getADirectSuperclass*(instanceCls)
402450
)
403451
}
404452

python/ql/test/library-tests/dataflow/typetracking/attribute_tests.py

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,3 +177,50 @@ def possibly_uncalled_method(self): # $ MISSING: tracked=foo
177177
instance.print_self() # $ tracked=foo
178178
instance.foo = tracked # $ tracked=foo tracked
179179
instance.print_foo() # $ tracked=foo
180+
181+
182+
# ------------------------------------------------------------------------------
183+
# Tracking of instance attribute across the class hierarchy
184+
# ------------------------------------------------------------------------------
185+
186+
# attribute written in a base class method, read in a subclass method
187+
188+
class Base1(object):
189+
def __init__(self): # $ tracked=foo
190+
self.foo = tracked # $ tracked=foo tracked
191+
192+
class Sub1(Base1):
193+
def read_foo(self): # $ MISSING: tracked=foo
194+
print(self.foo) # $ tracked MISSING: tracked=foo
195+
196+
sub1 = Sub1()
197+
sub1.read_foo()
198+
print(sub1.foo) # $ tracked MISSING: tracked=foo
199+
200+
201+
# attribute written in a subclass method, read in an inherited base class method
202+
203+
class Base2(object):
204+
def read_bar(self): # $ MISSING: tracked=bar
205+
print(self.bar) # $ tracked MISSING: tracked=bar
206+
207+
class Sub2(Base2):
208+
def __init__(self): # $ tracked=bar
209+
self.bar = tracked # $ tracked=bar tracked
210+
211+
sub2 = Sub2()
212+
sub2.read_bar()
213+
print(sub2.bar) # $ tracked MISSING: tracked=bar
214+
215+
216+
# attribute written in a base class method, read on an instance of the subclass
217+
218+
class Base3(object):
219+
def __init__(self): # $ tracked=baz
220+
self.baz = tracked # $ tracked=baz tracked
221+
222+
class Sub3(Base3):
223+
pass
224+
225+
sub3 = Sub3()
226+
print(sub3.baz) # $ tracked MISSING: tracked=baz

0 commit comments

Comments
 (0)