Skip to content

Commit f8190fe

Browse files
committed
Python: Fix divergence in tuple/subscripted type toString
A slightly more complicated version of the situation in #2507 could cause the `toString` calculation to diverge. Although the previous PR took tuples nested inside tuples into account (and subscripted types cannot be nested inside each other in our modelling), it did not account for having this nesting be interleaved, and this is what caused the divergence. I have not done the usual "test case first to show the problem exists", since this would also diverge and take forever to fail. The instance observed in `scipy` was likely caused by something akin to ```python x = () while True: x = x[(x,)] ``` Finally, to prevent this from happening with other types, I went through and checked each instance where the string representation of an `ObjectInternal` might potentially contain a reference to itself (and thus explode). I encapsulated this in a `bounded_toString` helper predicate, and used this in all the cases where I was able to determine that the above _could_ happen.
1 parent b895641 commit f8190fe

File tree

4 files changed

+21
-7
lines changed

4 files changed

+21
-7
lines changed

python/ql/src/semmle/python/objects/Classes.qll

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,8 @@ class SubscriptedTypeInternal extends ObjectInternal, TSubscriptedType {
311311
override string getName() { result = this.getGeneric().getName() }
312312

313313
override string toString() {
314-
result = this.getGeneric().toString() + "[" + this.getSpecializer().toString() + "]"
314+
result =
315+
bounded_toString(this.getGeneric()) + "[" + bounded_toString(this.getSpecializer()) + "]"
315316
}
316317

317318
override predicate introducedAt(ControlFlowNode node, PointsToContext context) {

python/ql/src/semmle/python/objects/Instances.qll

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -379,7 +379,8 @@ private predicate cls_descriptor(ClassObjectInternal cls, string name, ObjectInt
379379
/** A class representing an instance of the `super` class */
380380
class SuperInstance extends TSuperInstance, ObjectInternal {
381381
override string toString() {
382-
result = "super(" + this.getStartClass().toString() + ", " + this.getSelf().toString() + ")"
382+
result =
383+
"super(" + this.getStartClass().toString() + ", " + bounded_toString(this.getSelf()) + ")"
383384
}
384385

385386
override boolean booleanValue() { result = true }

python/ql/src/semmle/python/objects/ObjectInternal.qll

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -515,7 +515,7 @@ class DecoratedFunction extends ObjectInternal, TDecoratedFunction {
515515
override string getName() { result = this.decoratedObject().getName() }
516516

517517
override string toString() {
518-
result = "Decorated " + this.decoratedObject().toString()
518+
result = "Decorated " + bounded_toString(this.decoratedObject())
519519
or
520520
not exists(this.decoratedObject()) and result = "Decorated function"
521521
}
@@ -592,3 +592,18 @@ pragma[nomagic]
592592
predicate receiver_type(AttrNode attr, string name, ObjectInternal value, ClassObjectInternal cls) {
593593
PointsToInternal::pointsTo(attr.getObject(name), _, value, _) and value.getClass() = cls
594594
}
595+
596+
/**
597+
* Returns a string representation of `obj`. Because some classes have (mutually) recursive
598+
* `toString` implementations, this predicate acts as a stop for these classes, preventing an
599+
* unbounded `toString` from being materialized.
600+
*/
601+
string bounded_toString(ObjectInternal obj) {
602+
if
603+
obj instanceof DecoratedFunction or
604+
obj instanceof TupleObjectInternal or
605+
obj instanceof SubscriptedTypeInternal or
606+
obj instanceof SuperInstance
607+
then result = "(...)"
608+
else result = obj.toString()
609+
}

python/ql/src/semmle/python/objects/Sequences.qll

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,10 +54,7 @@ abstract class TupleObjectInternal extends SequenceObjectInternal {
5454
}
5555

5656
private string item(int n) {
57-
exists(ObjectInternal item | item = this.getItem(n) |
58-
// To avoid infinite recursion, nested tuples are replaced with the string "...".
59-
if item instanceof TupleObjectInternal then result = "(...)" else result = item.toString()
60-
)
57+
result = bounded_toString(this.getItem(n))
6158
or
6259
n in [0 .. this.length() - 1] and
6360
not exists(this.getItem(n)) and

0 commit comments

Comments
 (0)