Skip to content

Commit bf12204

Browse files
yoffCopilot
andcommitted
Python: unpin legacy CFG/ESSA from the AST cached stage
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>
1 parent c2f439a commit bf12204

5 files changed

Lines changed: 3 additions & 50 deletions

File tree

python/ql/lib/semmle/python/Flow.qll

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,6 @@ class ControlFlowNode extends @py_flow_node {
128128
/** Gets a textual representation of this element. */
129129
cached
130130
string toString() {
131-
Stages::AST::ref() and
132131
// Since modules can have ambigous names, entry nodes can too, if we do not collate them.
133132
exists(Py::Scope s | s.getEntryNode() = this |
134133
result = "Entry node for " + concat( | | s.toString(), ",")
@@ -152,7 +151,6 @@ class ControlFlowNode extends @py_flow_node {
152151
/** Gets the scope containing this flow node */
153152
cached
154153
Py::Scope getScope() {
155-
Stages::AST::ref() and
156154
if this.getNode() instanceof Py::Scope
157155
then
158156
/* Entry or exit node */
@@ -554,7 +552,6 @@ class UnaryExprNode extends ControlFlowNode {
554552
class DefinitionNode extends ControlFlowNode {
555553
cached
556554
DefinitionNode() {
557-
Stages::AST::ref() and
558555
exists(Py::Assign a | this.getNode() = a.getATarget())
559556
or
560557
exists(Py::AssignExpr a | this.getNode() = a.getTarget())
@@ -635,7 +632,6 @@ class TupleNode extends SequenceNode {
635632
TupleNode() { toAst(this) instanceof Py::Tuple }
636633

637634
override ControlFlowNode getElement(int n) {
638-
Stages::AST::ref() and
639635
exists(Py::Tuple t | this.getNode() = t and result.getNode() = t.getElt(n)) and
640636
(
641637
result.getBasicBlock().dominates(this.getBasicBlock())
@@ -1015,10 +1011,7 @@ class BasicBlock extends @py_flow_node {
10151011

10161012
/** Whether this basic block strictly dominates the other */
10171013
cached
1018-
predicate strictlyDominates(BasicBlock other) {
1019-
Stages::AST::ref() and
1020-
other.getImmediateDominator+() = this
1021-
}
1014+
predicate strictlyDominates(BasicBlock other) { other.getImmediateDominator+() = this }
10221015

10231016
/** Whether this basic block dominates the other */
10241017
predicate dominates(BasicBlock other) {
@@ -1029,7 +1022,6 @@ class BasicBlock extends @py_flow_node {
10291022

10301023
cached
10311024
BasicBlock getImmediateDominator() {
1032-
Stages::AST::ref() and
10331025
this.firstNode().getImmediateDominator().getBasicBlock() = result
10341026
}
10351027

@@ -1075,10 +1067,7 @@ class BasicBlock extends @py_flow_node {
10751067

10761068
/** Gets a successor to this basic block */
10771069
cached
1078-
BasicBlock getASuccessor() {
1079-
Stages::AST::ref() and
1080-
result = this.getLastNode().getASuccessor().getBasicBlock()
1081-
}
1070+
BasicBlock getASuccessor() { result = this.getLastNode().getASuccessor().getBasicBlock() }
10821071

10831072
/** Gets a predecessor to this basic block */
10841073
BasicBlock getAPredecessor() { result.getASuccessor() = this }
@@ -1140,10 +1129,7 @@ class BasicBlock extends @py_flow_node {
11401129

11411130
/** Holds if this basic block strictly reaches the other. Is the start of other reachable from the end of this. */
11421131
cached
1143-
predicate strictlyReaches(BasicBlock other) {
1144-
Stages::AST::ref() and
1145-
this.getASuccessor+() = other
1146-
}
1132+
predicate strictlyReaches(BasicBlock other) { this.getASuccessor+() = other }
11471133

11481134
/** Holds if this basic block reaches the other. Is the start of other reachable from the end of this. */
11491135
predicate reaches(BasicBlock other) { this = other or this.strictlyReaches(other) }

python/ql/lib/semmle/python/essa/Essa.qll

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,6 @@ class PhiFunction extends EssaDefinition, TPhiFunction {
274274
/** Gets the input variable for this phi node on the edge `pred` -> `this.getBasicBlock()`, if any. */
275275
cached
276276
EssaVariable getInput(BasicBlock pred) {
277-
Stages::AST::ref() and
278277
result.getDefinition() = this.reachingDefinition(pred)
279278
or
280279
result.getDefinition() = this.inputEdgeRefinement(pred)

python/ql/lib/semmle/python/essa/SsaCompute.qll

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,6 @@ private module SsaComputeImpl {
311311
*/
312312
cached
313313
predicate reachesEndOfBlock(SsaSourceVariable v, BasicBlock defbb, int defindex, BasicBlock b) {
314-
Stages::AST::ref() and
315314
Liveness::liveAtExit(v, b) and
316315
(
317316
defbb = b and

python/ql/lib/semmle/python/essa/SsaDefinitions.qll

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ module SsaSource {
2020
/** Holds if `v` is used as the receiver in a method call. */
2121
cached
2222
predicate method_call_refinement(Variable v, ControlFlowNode use, CallNode call) {
23-
Stages::AST::ref() and
2423
use = v.getAUse() and
2524
call.getFunction().(AttrNode).getObject() = use and
2625
not test_contains(_, call)

python/ql/lib/semmle/python/internal/CachedStages.qll

Lines changed: 0 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -45,13 +45,9 @@ module Stages {
4545
cached
4646
predicate ref() { 1 = 1 }
4747

48-
private import semmle.python.essa.SsaDefinitions as SsaDefinitions
49-
private import semmle.python.essa.SsaCompute as SsaCompute
50-
private import semmle.python.essa.Essa as Essa
5148
private import semmle.python.Module as PyModule
5249
private import semmle.python.Exprs as Exprs
5350
private import semmle.python.AstExtended as AstExtended
54-
private import semmle.python.Flow as PyFlow
5551

5652
/**
5753
* DONT USE!
@@ -61,12 +57,6 @@ module Stages {
6157
predicate backref() {
6258
1 = 1
6359
or
64-
SsaDefinitions::SsaSource::method_call_refinement(_, _, _)
65-
or
66-
SsaCompute::SsaDefinitions::reachesEndOfBlock(_, _, _, _)
67-
or
68-
exists(any(Essa::PhiFunction p).getInput(_))
69-
or
7060
exists(PyModule::moduleNameFromFile(_))
7161
or
7262
exists(any(Exprs::Expr e).toString())
@@ -76,26 +66,6 @@ module Stages {
7666
exists(any(AstExtended::AstNode n).getAChildNode())
7767
or
7868
exists(any(AstExtended::AstNode n).getParentNode())
79-
or
80-
exists(PyFlow::ControlFlowNode cfg, AstExtended::AstNode n | cfg.getNode() = n)
81-
or
82-
exists(any(PyFlow::BasicBlock b).getImmediateDominator())
83-
or
84-
exists(any(PyFlow::BasicBlock b).getScope())
85-
or
86-
any(PyFlow::BasicBlock b).strictlyDominates(_)
87-
or
88-
any(PyFlow::BasicBlock b).strictlyReaches(_)
89-
or
90-
exists(any(PyFlow::BasicBlock b).getASuccessor())
91-
or
92-
exists(any(PyFlow::ControlFlowNode b).getScope())
93-
or
94-
exists(PyFlow::DefinitionNode b)
95-
or
96-
exists(any(PyFlow::SequenceNode n).getElement(_))
97-
or
98-
exists(any(PyFlow::ControlFlowNode c).toString())
9969
}
10070
}
10171

0 commit comments

Comments
 (0)