Skip to content

Commit 38985db

Browse files
yoffCopilot
andcommitted
Python: migrate remaining tests off getAFlowNode() and fix star-import SSA step
Sweep the last few uses of legacy AstNode.getAFlowNode() in tests over to explicit ControlFlowNode joins after the shared-CFG migration. importflow.ql needs the new Cfg::ControlFlowNode/CompareNode types because DataFlow::Node. asCfgNode() now returns the shared-CFG node. Also extend ImportResolution::allowedEssaImportStep to walk back through uncertain-write SSA inputs, so that a later 'from X import *' does not hide the preceding explicit (re)assignment from module-export resolution. Without this, a reassigned name that survives a wildcard import was no longer recognised as the module export. Rebless ModuleExport.expected to drop the legacy 'ControlFlowNode for' toString prefix and pick up the two correct rows exposed by the fix. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 7bb9aeb commit 38985db

7 files changed

Lines changed: 139 additions & 128 deletions

File tree

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,14 @@ module ImportResolution {
8787
) {
8888
// to handle definitions guarded by if-then-else
8989
defFrom = defTo.(SsaImpl::PhiFunction).getAnInput()
90+
or
91+
// to handle uncertain writes such as `from X import *`, which create an
92+
// uncertain SSA definition for every name in the importing scope. The
93+
// immediately preceding definition is still potentially the value of the
94+
// module export.
95+
SsaImpl::Ssa::uncertainWriteDefinitionInput(defTo, defFrom)
9096
// Note: legacy ESSA refinement-step (e.g. for `foo.bar = X`) is
91-
// not modelled in the new SSA. We rely on phi steps only.
97+
// not modelled in the new SSA beyond the cases handled above.
9298
}
9399

94100
/**

python/ql/test/2/library-tests/comprehensions/ConsistencyCheck.ql

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,5 +5,7 @@
55
import python
66

77
select count(Comprehension c |
8-
count(c.toString()) != 1 or count(c.getLocation()) != 1 or not exists(c.getAFlowNode())
8+
count(c.toString()) != 1 or
9+
count(c.getLocation()) != 1 or
10+
not exists(ControlFlowNode n | n.getNode() = c)
911
)

python/ql/test/experimental/import-resolution/ModuleExport.expected

Lines changed: 120 additions & 120 deletions
Large diffs are not rendered by default.

python/ql/test/experimental/import-resolution/importflow.ql

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import semmle.python.dataflow.new.DataFlow
33
import semmle.python.ApiGraphs
44
import utils.test.InlineExpectationsTest
55
import semmle.python.dataflow.new.internal.ImportResolution
6+
private import semmle.python.controlflow.internal.Cfg as Cfg
67

78
/** A string that appears on the right hand side of an assignment. */
89
private class SourceString extends DataFlow::Node {
@@ -45,13 +46,15 @@ private class VersionGuardedNode extends DataFlow::Node {
4546

4647
VersionGuardedNode() {
4748
version in [2, 3] and
48-
exists(If parent, CompareNode c | parent.getBody().contains(this.asExpr()) |
49+
exists(If parent, Cfg::CompareNode c, Cfg::ControlFlowNode litCfg |
50+
parent.getBody().contains(this.asExpr()) and
51+
litCfg.getNode() = any(IntegerLiteral lit | lit.getValue() = version)
52+
|
4953
c.operands(API::moduleImport("sys")
5054
.getMember("version_info")
5155
.getASubscript()
5256
.asSource()
53-
.asCfgNode(), any(Eq eq),
54-
any(IntegerLiteral lit | lit.getValue() = version).getAFlowNode())
57+
.asCfgNode(), any(Eq eq), litCfg)
5558
)
5659
}
5760

python/ql/test/library-tests/ControlFlow/PointsToSupport/UseFromDefinition.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ Expr assignedValue(Name n) {
99

1010
from Name def, DefinitionNode d
1111
where
12-
d = def.getAFlowNode() and
12+
d.getNode() = def and
1313
exists(assignedValue(def)) and
1414
not d.getValue().getNode() = assignedValue(def)
1515
select def.toString(), assignedValue(def)

python/ql/test/library-tests/ControlFlow/splitting/NodeCount.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,4 +8,4 @@ where
88
not a instanceof ExprStmt and
99
a.getScope() = s and
1010
s instanceof Function
11-
select a.getLocation().getStartLine(), s.getName(), a, count(a.getAFlowNode())
11+
select a.getLocation().getStartLine(), s.getName(), a, count(ControlFlowNode n | n.getNode() = a)

python/ql/test/library-tests/dataflow/tainttracking/TestTaintLib.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ query predicate test_taint(string arg_location, string test_res, string scope_na
4444
// TODO: Replace with `hasFlowToExpr` once that is working
4545
if
4646
TestTaintTrackingFlow::flowTo(any(DataFlow::Node n |
47-
n.(DataFlow::CfgNode).getNode() = arg.getAFlowNode()
47+
n.(DataFlow::CfgNode).getNode().getNode() = arg
4848
))
4949
then has_taint = true
5050
else has_taint = false

0 commit comments

Comments
 (0)