Skip to content

Commit f716f96

Browse files
authored
Merge pull request #4132 from yoff/SharedDataflow_NestedComprehensions
Python: Shared dataflow, nested comprehensions
2 parents a9f322e + 50cc5d5 commit f716f96

File tree

5 files changed

+496
-260
lines changed

5 files changed

+496
-260
lines changed

python/ql/src/experimental/dataflow/internal/DataFlowPrivate.qll

Lines changed: 19 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -461,6 +461,10 @@ predicate comprehensionStoreStep(CfgNode nodeFrom, Content c, CfgNode nodeTo) {
461461
// Dictionary
462462
nodeTo.getNode().getNode().(DictComp).getElt() = nodeFrom.getNode().getNode() and
463463
c instanceof DictionaryElementAnyContent
464+
or
465+
// Generator
466+
nodeTo.getNode().getNode().(GeneratorExp).getElt() = nodeFrom.getNode().getNode() and
467+
c instanceof ListElementContent
464468
}
465469

466470
/**
@@ -538,33 +542,23 @@ predicate comprehensionReadStep(CfgNode nodeFrom, Content c, EssaNode nodeTo) {
538542
// nodeFrom is `l`, cfg node
539543
// nodeTo is `x`, essa var
540544
// c denotes element of list or set
541-
exists(For f, Comp comp |
542-
f = getCompFor(comp) and
543-
nodeFrom.getNode().getNode() = getCompIter(comp) and
545+
exists(Comp comp |
546+
// outermost for
547+
nodeFrom.getNode().getNode() = comp.getIterable() and
544548
nodeTo.getVar().getDefinition().(AssignmentDefinition).getDefiningNode().getNode() =
545-
f.getTarget() and
546-
(
547-
c instanceof ListElementContent
548-
or
549-
c instanceof SetElementContent
549+
comp.getIterationVariable(0).getAStore()
550+
or
551+
// an inner for
552+
exists(int n | n > 0 |
553+
nodeFrom.getNode().getNode() = comp.getNthInnerLoop(n).getIter() and
554+
nodeTo.getVar().getDefinition().(AssignmentDefinition).getDefiningNode().getNode() =
555+
comp.getNthInnerLoop(n).getTarget()
550556
)
551-
)
552-
}
553-
554-
/** This seems to compensate for extractor shortcomings */
555-
For getCompFor(Comp c) {
556-
c.contains(result) and
557-
c.getFunction() = result.getScope()
558-
}
559-
560-
/** This seems to compensate for extractor shortcomings */
561-
AstNode getCompIter(Comp c) {
562-
c.contains(result) and
563-
c.getScope() = result.getScope() and
564-
not result = c.getFunction() and
565-
not exists(AstNode between |
566-
c.contains(between) and
567-
between.contains(result)
557+
) and
558+
(
559+
c instanceof ListElementContent
560+
or
561+
c instanceof SetElementContent
568562
)
569563
}
570564

python/ql/src/semmle/python/Comprehensions.qll

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,18 +4,22 @@ import python
44
abstract class Comp extends Expr {
55
abstract Function getFunction();
66

7-
/** Gets the iteration variable for the nth innermost generator of this list comprehension */
7+
/** Gets the iterable of this set comprehension. */
8+
abstract Expr getIterable();
9+
10+
/** Gets the iteration variable for the nth innermost generator of this comprehension. */
811
Variable getIterationVariable(int n) {
912
result.getAnAccess() = this.getNthInnerLoop(n).getTarget()
1013
}
1114

12-
private For getNthInnerLoop(int n) {
15+
/** Gets the nth innermost For expression of this comprehension. */
16+
For getNthInnerLoop(int n) {
1317
n = 0 and result = this.getFunction().getStmt(0)
1418
or
1519
result = this.getNthInnerLoop(n - 1).getStmt(0)
1620
}
1721

18-
/** Gets the iteration variable for a generator of this list comprehension */
22+
/** Gets the iteration variable for a generator of this list comprehension. */
1923
Variable getAnIterationVariable() { result = this.getIterationVariable(_) }
2024

2125
/** Gets the scope in which the body of this list comprehension evaluates. */
@@ -62,6 +66,8 @@ class ListComp extends ListComp_, Comp {
6266

6367
override Function getFunction() { result = ListComp_.super.getFunction() }
6468

69+
override Expr getIterable() { result = ListComp_.super.getIterable() }
70+
6571
override string toString() { result = ListComp_.super.toString() }
6672

6773
override Expr getElt() { result = Comp.super.getElt() }
@@ -79,6 +85,8 @@ class SetComp extends SetComp_, Comp {
7985
override predicate hasSideEffects() { any() }
8086

8187
override Function getFunction() { result = SetComp_.super.getFunction() }
88+
89+
override Expr getIterable() { result = SetComp_.super.getIterable() }
8290
}
8391

8492
/** A dictionary comprehension, such as `{ k:v for k, v in enumerate("0123456789") }` */
@@ -93,6 +101,8 @@ class DictComp extends DictComp_, Comp {
93101
override predicate hasSideEffects() { any() }
94102

95103
override Function getFunction() { result = DictComp_.super.getFunction() }
104+
105+
override Expr getIterable() { result = DictComp_.super.getIterable() }
96106
}
97107

98108
/** A generator expression, such as `(var for var in iterable)` */
@@ -107,4 +117,6 @@ class GeneratorExp extends GeneratorExp_, Comp {
107117
override predicate hasSideEffects() { any() }
108118

109119
override Function getFunction() { result = GeneratorExp_.super.getFunction() }
120+
121+
override Expr getIterable() { result = GeneratorExp_.super.getIterable() }
110122
}

0 commit comments

Comments
 (0)