Skip to content

Commit b6f1421

Browse files
yoffCopilot
andcommitted
Python: model from X import * as uncertain SSA writes
Add a 4th disjunct to `SsaImplInput::variableWrite` in the shared-SSA adapter that mirrors legacy ESSA's `ImportStarRefinement`: every variable whose scope is the import-star's scope, OR which is used in the import-star's scope, gets an uncertain write at the `import *` position. Uncertain writes do not kill prior definitions; shared SSA's `SsaUncertainWrite` joins the new value with the immediately-preceding definition via `uncertainWriteDefinitionInput`. This is the equivalent of legacy ESSA's two-input refinement. Cannot depend on `ImportStar` / `ImportResolution` (those modules import `SsaImpl`), so the predicate uses the structural heuristic on `Cfg::ImportStarNode` directly. This closes the two remaining failing dataflow library-tests: - `import-star/global` — `module_export` chains via `from X import *` re-exports now resolve: the importing module has an SSA def of every re-exported name, so `lastUseVar` finds the read at the use site. - `typetracking_imports/highlight_problem` — a direct `from .foo import foo` immediately followed by `from .other import *` is now correctly marked as dead at the direct import. Two scope-entry-def noise rows in `highlight_problem.expected` are also dropped — legacy ESSA needed them as refinement inputs, but shared SSA handles uncertain writes without an explicit prior def. They were always tagged `no use to normal exit` (dead). Dataflow library-tests: 62/64 → 64/64 passing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 1a2be46 commit b6f1421

2 files changed

Lines changed: 41 additions & 20 deletions

File tree

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

Lines changed: 34 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -48,14 +48,6 @@ private module CfgForSsa implements BB::CfgSig<Py::Location> {
4848
predicate dominatingEdge = CfgImpl::Cfg::dominatingEdge/2;
4949
}
5050

51-
/**
52-
* A source variable for SSA. Wraps a Python `Variable` (the AST-level
53-
* notion of a named binding within a scope) so that the shared SSA
54-
* implementation can use it as a `SourceVariable`.
55-
*
56-
* We only track variables that are read at least once in their scope —
57-
* tracking write-only variables is unnecessary work.
58-
*/
5951
/**
6052
* A source variable for SSA, wrapping a Python AST `Variable`.
6153
*
@@ -113,7 +105,9 @@ class SsaSourceVariable extends TSsaSourceVariable {
113105
* `SsaSourceVariable.getASourceUse()`.
114106
*/
115107
Cfg::ControlFlowNode getASourceUse() {
116-
exists(Cfg::NameNode n | result = n | n.uses(this.getVariable()) or n.deletes(this.getVariable()))
108+
exists(Cfg::NameNode n | result = n |
109+
n.uses(this.getVariable()) or n.deletes(this.getVariable())
110+
)
117111
}
118112

119113
/**
@@ -223,6 +217,31 @@ private module SsaImplInput implements SsaImplCommon::InputSig<Py::Location, Cfg
223217
hasEntryDefIn(v, bb) and
224218
i = -1 and
225219
certain = true
220+
or
221+
// `from X import *` — possibly rebinds every name in the importing
222+
// scope. Modelled as an uncertain write at the import-star's CFG
223+
// position for every variable that lives in (or is referenced
224+
// from) the same scope as the import-star. Mirrors legacy ESSA's
225+
// `ImportStarRefinement` (see `essa/SsaDefinitions.qll`'s
226+
// `import_star_refinement` predicate). The write is uncertain so
227+
// that prior definitions of the variable remain available — the
228+
// shared-SSA `SsaUncertainWrite` merges the new value with the
229+
// immediately preceding definition.
230+
exists(Cfg::ImportStarNode imp |
231+
bb.getNode(i) = imp and
232+
certain = false and
233+
(
234+
v.getVariable().getScope() = imp.getScope()
235+
or
236+
// Variable is defined in some other scope but referenced in
237+
// the same scope as the import-star (matches legacy clause 2:
238+
// `other.uses(v) and def.getScope() = other.getScope()`).
239+
exists(Cfg::NameNode other |
240+
other.uses(v.getVariable()) and
241+
imp.getScope() = other.getScope()
242+
)
243+
)
244+
)
226245
}
227246

228247
predicate variableRead(CfgImpl::BasicBlock bb, int i, SourceVariable v, boolean certain) {
@@ -400,13 +419,17 @@ class MultiAssignmentDefinition extends EssaNodeDefinition {
400419

401420
override Cfg::ControlFlowNode getDefiningNode() {
402421
// Default: the underlying `Name` CFG node (where the SSA def lives).
403-
not exists(Cfg::StarredNode s | s.getNode().(Py::Starred).getValue() = super.getDefiningNode().getNode()) and
422+
not exists(Cfg::StarredNode s |
423+
s.getNode().(Py::Starred).getValue() = super.getDefiningNode().getNode()
424+
) and
404425
result = super.getDefiningNode()
405426
or
406427
// Exception: for `*rest`, expose the enclosing `Starred` CFG node
407428
// so that `IterableUnpacking::iterableUnpackingStarredElementStoreStep`
408429
// can attach the rest-list to it.
409-
exists(Cfg::StarredNode s | s.getNode().(Py::Starred).getValue() = super.getDefiningNode().getNode() |
430+
exists(Cfg::StarredNode s |
431+
s.getNode().(Py::Starred).getValue() = super.getDefiningNode().getNode()
432+
|
410433
result = s
411434
)
412435
}
Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,7 @@
1-
| pkg/alias_only_direct.py:0:0:0:0 | Module pkg.alias_only_direct | pkg/alias_only_direct.py:1:22:1:24 | GSSA Variable foo | use to normal exit |
2-
| pkg/alias_problem.py:0:0:0:0 | Module pkg.alias_problem | pkg/alias_problem.py:1:22:1:24 | GSSA Variable foo | no use to normal exit |
3-
| pkg/alias_problem.py:0:0:0:0 | Module pkg.alias_problem | pkg/alias_problem.py:2:1:2:20 | GSSA Variable foo | use to normal exit |
4-
| pkg/alias_problem_fixed.py:0:0:0:0 | Module pkg.alias_problem_fixed | pkg/alias_problem_fixed.py:0:0:0:0 | GSSA Variable foo | no use to normal exit |
5-
| pkg/alias_problem_fixed.py:0:0:0:0 | Module pkg.alias_problem_fixed | pkg/alias_problem_fixed.py:3:22:3:24 | GSSA Variable foo | use to normal exit |
6-
| pkg/problem_absolute_import.py:0:0:0:0 | Module pkg.problem_absolute_import | pkg/problem_absolute_import.py:1:25:1:27 | GSSA Variable foo | no use to normal exit |
7-
| pkg/problem_absolute_import.py:0:0:0:0 | Module pkg.problem_absolute_import | pkg/problem_absolute_import.py:2:1:2:23 | GSSA Variable foo | use to normal exit |
8-
| pkg/works_absolute_import.py:0:0:0:0 | Module pkg.works_absolute_import | pkg/works_absolute_import.py:0:0:0:0 | GSSA Variable foo | no use to normal exit |
9-
| pkg/works_absolute_import.py:0:0:0:0 | Module pkg.works_absolute_import | pkg/works_absolute_import.py:2:25:2:27 | GSSA Variable foo | use to normal exit |
1+
| pkg/alias_only_direct.py:0:0:0:0 | Module pkg.alias_only_direct | pkg/alias_only_direct.py:1:22:1:24 | SSA def(Global Variable foo) | use to normal exit |
2+
| pkg/alias_problem.py:0:0:0:0 | Module pkg.alias_problem | pkg/alias_problem.py:1:22:1:24 | SSA def(Global Variable foo) | no use to normal exit |
3+
| pkg/alias_problem.py:0:0:0:0 | Module pkg.alias_problem | pkg/alias_problem.py:2:1:2:20 | SSA def(Global Variable foo) | use to normal exit |
4+
| pkg/alias_problem_fixed.py:0:0:0:0 | Module pkg.alias_problem_fixed | pkg/alias_problem_fixed.py:3:22:3:24 | SSA def(Global Variable foo) | use to normal exit |
5+
| pkg/problem_absolute_import.py:0:0:0:0 | Module pkg.problem_absolute_import | pkg/problem_absolute_import.py:1:25:1:27 | SSA def(Global Variable foo) | no use to normal exit |
6+
| pkg/problem_absolute_import.py:0:0:0:0 | Module pkg.problem_absolute_import | pkg/problem_absolute_import.py:2:1:2:23 | SSA def(Global Variable foo) | use to normal exit |
7+
| pkg/works_absolute_import.py:0:0:0:0 | Module pkg.works_absolute_import | pkg/works_absolute_import.py:2:25:2:27 | SSA def(Global Variable foo) | use to normal exit |

0 commit comments

Comments
 (0)