| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Preparatory refactor for the shared-CFG dataflow migration. Adds the new Python SSA adapter additively, without changing any production behaviour. Library additions: - semmle.python.dataflow.new.internal.SsaImpl — Python SSA implementation built on the new (shared) CFG. Mirrors the Java SSA adapter (java/ql/lib/semmle/code/java/dataflow/internal/SsaImpl.qll): an InputSig is defined in terms of positional (BasicBlock, int) variable references, and the shared codeql.ssa.Ssa::Make<Location, Cfg, Input> module is then instantiated. SourceVariable is the AST-level Py::Variable. Variable references are looked up via the new CFG facade's NameNode.defines/uses/deletes predicates (added in the preceding PR), which themselves are one-line bridges to AST-level Name.defines/uses/deletes. Implicit-entry definitions are inserted for non-local/global/builtin reads, captured variables, and (when needed) parameters. Test additions: - library-tests/dataflow-new-ssa/ — exercises the new SSA over a representative test corpus and checks expected def/use chains. - library-tests/dataflow-new-ssa-vs-legacy/ — runs both new SSA and legacy ESSA over the same corpus and diffs the results, so any semantic divergence shows up as a test failure. Production impact: None. The new SSA adapter has zero callers in lib/ and src/ — the legacy ESSA SSA (semmle/python/essa/*) remains the default. The dataflow library is not migrated yet; that lands in a follow-up PR. Verified by: - All 367 lib + src + consistency-queries compile clean. - All 641 ControlFlow + PointsTo + dataflow + essa + consistency library-tests pass. - Both new dataflow-new-ssa[/vs-legacy] test packs pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- part of the ESSA adapter layer still refers to the raw SSA (now called Impl)
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Flips the Python dataflow trunk from the legacy CFG (semmle/python/Flow.qll) and legacy ESSA SSA (semmle/python/essa/*) to the new shared CFG facade (semmle.python.controlflow.internal.Cfg) and the new SSA adapter (semmle.python.dataflow.new.internal.SsaImpl), both introduced additively in the preceding PRs in this stack. This is the trunk-flip equivalent of the original draft PR #21894 (kept around as documentation), rebased on top of the four preparatory PRs: P1: Remove AstNode.getAFlowNode() and rewrite callers (#21919). P2: Qualify Flow.qll's AST references with Py:: prefix (#21920). P3: Add new shared-CFG-backed control flow graph (#21921). P4: Add new shared-SSA-backed SSA adapter (#21923). The Python dataflow library (semmle/python/dataflow/new/) now imports the new CFG facade and SSA adapter. All CFG-typed predicates (ControlFlowNode, CallNode, BasicBlock, NameNode, AttrNode, ...) are qualified with the Cfg:: prefix; SSA references switch from EssaVariable/EssaDefinition to SsaImpl::Definition/SourceVariable. GuardNode is redesigned to use the new CFG's outcome-node model (isAfterTrue / isAfterFalse) instead of the legacy ConditionBlock + flipped indirection. Only BarrierGuard<...> is preserved as public API. Framework files (Bottle, FastApi, Django, Tornado, Pyramid, Stdlib, ...) are updated to take CFG nodes from the new facade. A handful of dataflow consistency tweaks for the new CFG: - Augmented-assignment targets are treated as both load and store. - 'from X import *' produces uncertain SSA writes for unknown names. - CFG nodes are canonicalised so dataflow does not see equivalent pre/post-order pairs as distinct nodes. Two AST tweaks for the new CFG: - AstNodeImpl: omit PEP 695 type-parameter names from FunctionDefExpr / ClassDefExpr children. - ImportResolution: drop the legacy essa import. Test churn (~175 files): reblessed library- and query-test .expected files reflect slightly different CFG granularity, different toString output, and a handful of true alert deltas in security queries. Verification: all 367 lib + src + consistency-queries compile clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The `Cfg::ControlFlowNode` facade re-exports the shared CFG library's `dominates`/`strictlyDominates` predicates, which are declared `bindingset[this, that]` + `pragma[inline_late]` and are meant to be used as bound-pair membership checks. The facade wrappers dropped these annotations (using plain `pragma[inline]`), so even though the only callers — the `with` / `async with` taint steps in DataFlowPrivate.qll and TaintTrackingPrivate.qll — bind both endpoints, the optimizer was free to materialise `Cfg::ControlFlowNode.strictlyDominates/1` as a full O(nodes^2) relation over the (larger) shared-CFG node set. On some projects this dominated analysis time entirely (DCA showed e.g. ICTU/quality-time and biosimulations regressing ~75-160x). Restoring `bindingset[this, other]` + `pragma[inline_late]` on the wrappers turns the predicate back into a bound-pair check and is result-preserving (only binding annotations change, the predicate body is unchanged). Reproduced on ICTU/quality-time: full python-security-extended suite went from stalling >20min on `strictlyDominates` to completing in ~6min; all ControlFlow and dataflow/coverage library tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The legacy CFG (`Flow.qll`) and legacy ESSA (`Essa`/`SsaCompute`/ `SsaDefinitions`) were pinned into the always-on `Stages::AST` cached stage via `Stages::AST::ref()` and the matching `backref()` disjuncts. Because a cached stage is materialized as a unit once any of its predicates is demanded (and every query demands e.g. `Expr.toString()`), this forced the legacy CFG/ESSA to be computed for *every* query -- including the security/dataflow queries, which after the shared-CFG dataflow flip no longer depend on the legacy CFG at all. Since `Stages::AST::ref()` is `1 = 1`, removing it is result-preserving; it only changes stage scheduling. After this change the legacy CFG/ESSA is no longer materialised for queries that do not genuinely reference it. Verified on the full `python-security-extended` suite and on django: legacy CFG/ESSA families materialised drop from ~165 to 0 with byte-identical results. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| Back | FazBrowse Home | New Git URL |
Note
Draft for measurement. This PR exists to run DCA and quantify how much of the shared-CFG dataflow flip's overhead is recovered by not computing the legacy CFG. It is based on the flip branch (yoff/python-shared-cfg-dataflow-flip, #21925), so the diff is exactly the one-commit change and the DCA a/b isolates the effect of the unpin.
What
The legacy CFG (Flow.qll) and legacy ESSA (Essa/SsaCompute/SsaDefinitions) are pinned into the always-on Stages::AST cached stage via Stages::AST::ref() (11 sites) and the matching Stages::AST::backref() disjuncts.
backref() is referenced nowhere and optimizes to 1 = 1, so the ref/backref pattern only controls stage assignment. Because a cached stage is materialised as a unit once any of its predicates is demanded — and every query demands e.g. Expr.toString() — this forces the legacy CFG/ESSA to be computed for every query.
After the shared-CFG dataflow flip, the security/dataflow queries no longer depend on the legacy CFG at all, so on the flip branch this computation is pure dead weight.
This PR removes those pins. Since Stages::AST::ref() is 1 = 1, the change is result-preserving — it only changes stage scheduling.
Verification (result-preserving)
Question DCA should answer
The legacy CFG the security queries drag in (~9.5s CPU, 1.3%) is a fraction of the new CFG+SSA the flip adds (~37.6s CPU, 5.3%), so a-priori this recovers only ~a quarter of the new-CFG cost, not the full flip overhead. DCA across the target set will give the authoritative per-project number.
Caveats before this could merge