Python: unpin legacy CFG/ESSA from the AST cached stage (DCA measurement) - #22116
Draft
yoff wants to merge 4 commits into
Draft
Python: unpin legacy CFG/ESSA from the AST cached stage (DCA measurement)#22116yoff wants to merge 4 commits into
yoff wants to merge 4 commits into
Conversation
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>
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>
yoff
force-pushed
the
yoff/python-shared-cfg-dataflow-flip
branch
from
July 30, 2026 15:31
c2f439a to
fb7a5d4
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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-onStages::ASTcached stage viaStages::AST::ref()(11 sites) and the matchingStages::AST::backref()disjuncts.backref()is referenced nowhere and optimizes to1 = 1, so theref/backrefpattern 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()is1 = 1, the change is result-preserving — it only changes stage scheduling.Verification (result-preserving)
python-security-extendedsuite (52 queries) anddjango: legacy CFG/ESSA predicate families materialised drop from ~165 → 0, with byte-identical results.-j4); ~9.5s of legacy predicate self-time removed (≈1.3% of total analysis CPU).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