Skip to content

Commit 96de5cf

Browse files
yoffCopilot
andcommitted
Python: bring Cfg.qll's facade to API parity with Flow.qll
Adds the methods and type-narrowing overrides needed for Cfg.qll to be a drop-in replacement for Flow.qll's CFG API surface: * 'override getNode()' type narrowing on all AST-shape subclasses (CallNode -> Py::Call, AttrNode -> Py::Attribute, ImportExprNode -> Py::ImportExpr, etc.). This lets callers chain methods like 'iexpr.getNode().isRelative()' that previously failed because 'getNode()' returned the generic AstNode. * 'ControlFlowNode.isBranch()' -- true and/or false successor exists. * 'ControlFlowNode.getAChild()' -- CFG-level child traversal via the AST's getAChildNode, with dominance constraint. * 'ControlFlowNode.strictlyReaches(other)' -- node-level reachability. * 'NameNode.isSelf()' -- AST-level approximation: uses the 'Variable' that is the first parameter of an enclosing method. * 'BinaryExprNode.operands(left, op, right)' + 'getAnOperand()'. * 'BoolExprNode.getAnOperand()'. * 'ForNode.getSequence()' (alias for 'getIter') and 'ForNode.iterates(target, sequence)'. * 'ForNode' / 'RaiseStmtNode' type-narrowing overrides. * 'ExceptFlowNode.getName()' / 'ExceptGroupFlowNode.getName()' -- the bound 'as'-name CFG node. * 'DictNode.getAKey()' (only 'getAValue' was present). These additions are independent of the dataflow-migration approach (option 4 vs option 5). They close the API-parity gap identified during the Option-5 investigation; with them in place, hundreds of type-resolution errors that previously appeared when swapping Cfg for Flow at the python.qll level go away. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent f5bf8ae commit 96de5cf

1 file changed

Lines changed: 110 additions & 0 deletions

File tree

  • python/ql/lib/semmle/python/controlflow/internal

python/ql/lib/semmle/python/controlflow/internal/Cfg.qll

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,31 @@ class ControlFlowNode extends CfgImpl::ControlFlowNode {
213213
/** Holds if this flow node corresponds to a class definition expression. */
214214
predicate isClass() { toAst(this) instanceof Py::ClassExpr }
215215

216+
/**
217+
* Holds if this flow node is a branch (i.e. has both a true and a
218+
* false successor).
219+
*/
220+
predicate isBranch() { exists(this.getATrueSuccessor()) or exists(this.getAFalseSuccessor()) }
221+
222+
/**
223+
* Gets a CFG child of this node, defined as a CFG node whose AST node
224+
* is a child of this CFG node's AST node, restricted to nodes that
225+
* dominate this one (so the child has been evaluated by the time we
226+
* reach this node).
227+
*
228+
* Mirrors `Flow.qll`'s `getAChild`. UnaryExprNode is excluded because
229+
* its operand is its CFG predecessor (handled separately).
230+
*/
231+
pragma[nomagic]
232+
ControlFlowNode getAChild() {
233+
toAst(this).(Py::Expr).getAChildNode() = toAst(result) and
234+
result.getBasicBlock().dominates(this.getBasicBlock()) and
235+
not this instanceof UnaryExprNode
236+
}
237+
238+
/** Holds if this flow node strictly reaches `other`. */
239+
predicate strictlyReaches(ControlFlowNode other) { this.getASuccessor+() = other }
240+
216241
/** Internal: raw successor predicate that does NOT skip non-canonical nodes. */
217242
CfgImpl::ControlFlowNode getASuccessorRaw() { result = super.getASuccessor() }
218243
}
@@ -422,6 +447,23 @@ class NameNode extends ControlFlowNode {
422447

423448
/** Holds if this is a use of a global (including builtin) variable. */
424449
predicate isGlobal() { exists(Py::Variable v | this.uses(v) and v instanceof Py::GlobalVariable) }
450+
451+
/**
452+
* Holds if this is a use of `self` — the first parameter of an
453+
* enclosing method.
454+
*
455+
* AST-level approximation: matches when the Name uses a `Variable`
456+
* that is the first parameter of an enclosing `Function` defined
457+
* inside a `Class`.
458+
*/
459+
predicate isSelf() {
460+
exists(Py::Variable v, Py::Function f, Py::Class c |
461+
this.uses(v) and
462+
f = c.getAMethod() and
463+
v.getScope() = f and
464+
v = f.getArg(0).(Py::Name).getVariable()
465+
)
466+
}
425467
}
426468

427469
/** A control flow node corresponding to a named constant (`None`, `True`, `False`). */
@@ -433,6 +475,8 @@ class NameConstantNode extends NameNode {
433475
class CallNode extends ControlFlowNode {
434476
CallNode() { toAst(this) instanceof Py::Call }
435477

478+
override Py::Call getNode() { result = super.getNode() }
479+
436480
/** Gets the underlying Python `Call`. */
437481
Py::Call getCall() { result = toAst(this) }
438482

@@ -501,6 +545,8 @@ class CallNode extends ControlFlowNode {
501545
class AttrNode extends ControlFlowNode {
502546
AttrNode() { toAst(this) instanceof Py::Attribute }
503547

548+
override Py::Attribute getNode() { result = super.getNode() }
549+
504550
/** Gets the flow node for the object of the attribute expression. */
505551
ControlFlowNode getObject() {
506552
exists(Py::Attribute a |
@@ -527,12 +573,16 @@ class AttrNode extends ControlFlowNode {
527573
/** A control flow node corresponding to an import statement (`import x`). */
528574
class ImportExprNode extends ControlFlowNode {
529575
ImportExprNode() { toAst(this) instanceof Py::ImportExpr }
576+
577+
override Py::ImportExpr getNode() { result = super.getNode() }
530578
}
531579

532580
/** A control flow node corresponding to a `from ... import name` expression. */
533581
class ImportMemberNode extends ControlFlowNode {
534582
ImportMemberNode() { toAst(this) instanceof Py::ImportMember }
535583

584+
override Py::ImportMember getNode() { result = super.getNode() }
585+
536586
/** Gets the flow node for the module being imported from, with the matching name. */
537587
ControlFlowNode getModule(string name) {
538588
exists(Py::ImportMember i |
@@ -548,6 +598,8 @@ class ImportMemberNode extends ControlFlowNode {
548598
class ImportStarNode extends ControlFlowNode {
549599
ImportStarNode() { toAst(this) instanceof Py::ImportStar }
550600

601+
override Py::ImportStar getNode() { result = super.getNode() }
602+
551603
/** Gets the flow node for the module being imported from. */
552604
ControlFlowNode getModule() {
553605
exists(Py::ImportStar i |
@@ -562,6 +614,8 @@ class ImportStarNode extends ControlFlowNode {
562614
class SubscriptNode extends ControlFlowNode {
563615
SubscriptNode() { toAst(this) instanceof Py::Subscript }
564616

617+
override Py::Subscript getNode() { result = super.getNode() }
618+
565619
/** Gets the flow node for the value being subscripted. */
566620
ControlFlowNode getObject() {
567621
exists(Py::Subscript s |
@@ -585,6 +639,8 @@ class SubscriptNode extends ControlFlowNode {
585639
class CompareNode extends ControlFlowNode {
586640
CompareNode() { toAst(this) instanceof Py::Compare }
587641

642+
override Py::Compare getNode() { result = super.getNode() }
643+
588644
/** Holds if `left` and `right` are a pair of operands for this comparison. */
589645
predicate operands(ControlFlowNode left, Py::Cmpop op, ControlFlowNode right) {
590646
exists(Py::Compare c, Py::Expr eleft, Py::Expr eright |
@@ -605,6 +661,8 @@ class CompareNode extends ControlFlowNode {
605661
class IfExprNode extends ControlFlowNode {
606662
IfExprNode() { toAst(this) instanceof Py::IfExp }
607663

664+
override Py::IfExp getNode() { result = super.getNode() }
665+
608666
/** Gets the flow node for one of the operands of an if-expression. */
609667
ControlFlowNode getAnOperand() { result = this.getAPredecessor() }
610668
}
@@ -613,6 +671,8 @@ class IfExprNode extends ControlFlowNode {
613671
class AssignmentExprNode extends ControlFlowNode {
614672
AssignmentExprNode() { toAst(this) instanceof Py::AssignExpr }
615673

674+
override Py::AssignExpr getNode() { result = super.getNode() }
675+
616676
/** Gets the flow node for the left-hand side. */
617677
ControlFlowNode getTarget() {
618678
exists(Py::AssignExpr a |
@@ -636,6 +696,8 @@ class AssignmentExprNode extends ControlFlowNode {
636696
class BinaryExprNode extends ControlFlowNode {
637697
BinaryExprNode() { toAst(this) instanceof Py::BinaryExpr }
638698

699+
override Py::BinaryExpr getNode() { result = super.getNode() }
700+
639701
ControlFlowNode getLeft() {
640702
exists(Py::BinaryExpr be |
641703
be = toAst(this) and
@@ -653,19 +715,40 @@ class BinaryExprNode extends ControlFlowNode {
653715
}
654716

655717
Py::Operator getOp() { result = toAst(this).(Py::BinaryExpr).getOp() }
718+
719+
/** Holds if `left` and `right` are the operands and `op` is the operator. */
720+
predicate operands(ControlFlowNode left, Py::Operator op, ControlFlowNode right) {
721+
left = this.getLeft() and right = this.getRight() and op = this.getOp()
722+
}
723+
724+
/** Gets either operand. */
725+
ControlFlowNode getAnOperand() { result = this.getLeft() or result = this.getRight() }
656726
}
657727

658728
/** A control flow node corresponding to a boolean expression (`a and b`, `a or b`). */
659729
class BoolExprNode extends ControlFlowNode {
660730
BoolExprNode() { toAst(this) instanceof Py::BoolExpr }
661731

732+
override Py::BoolExpr getNode() { result = super.getNode() }
733+
662734
Py::Boolop getOp() { result = toAst(this).(Py::BoolExpr).getOp() }
735+
736+
/** Gets any operand of this boolean expression. */
737+
ControlFlowNode getAnOperand() {
738+
exists(Py::BoolExpr be |
739+
be = toAst(this) and
740+
be.getAValue() = toAst(result) and
741+
result.getBasicBlock().dominates(this.getBasicBlock())
742+
)
743+
}
663744
}
664745

665746
/** A control flow node corresponding to a unary expression (`-x`, `not x`, etc.). */
666747
class UnaryExprNode extends ControlFlowNode {
667748
UnaryExprNode() { toAst(this) instanceof Py::UnaryExpr }
668749

750+
override Py::UnaryExpr getNode() { result = super.getNode() }
751+
669752
ControlFlowNode getOperand() {
670753
exists(Py::UnaryExpr u |
671754
u = toAst(this) and
@@ -709,24 +792,36 @@ class DeletionNode extends ControlFlowNode {
709792
class ForNode extends ControlFlowNode {
710793
ForNode() { exists(Py::For f | toAst(this) = f.getIter()) }
711794

795+
override Py::For getNode() { exists(Py::For f | toAst(this) = f.getIter() | result = f) }
796+
712797
/** Gets the iterable expression. */
713798
ControlFlowNode getIter() {
714799
result = this and result = result // canonical "after" of the iterable
715800
}
716801

802+
/** Gets the sequence expression (alias for `getIter()`, matches legacy Flow naming). */
803+
ControlFlowNode getSequence() { result = this.getIter() }
804+
717805
/** Gets the target (loop variable) of the `for` loop. */
718806
ControlFlowNode getTarget() {
719807
exists(Py::For f |
720808
f.getIter() = toAst(this) and
721809
f.getTarget() = toAst(result)
722810
)
723811
}
812+
813+
/** Holds if `target` is the loop variable and `sequence` is the iterable. */
814+
predicate iterates(ControlFlowNode target, ControlFlowNode sequence) {
815+
target = this.getTarget() and sequence = this.getSequence()
816+
}
724817
}
725818

726819
/** A control flow node corresponding to a `raise` statement. */
727820
class RaiseStmtNode extends ControlFlowNode {
728821
RaiseStmtNode() { toAst(this) instanceof Py::Raise }
729822

823+
override Py::Raise getNode() { result = super.getNode() }
824+
730825
/** Gets the exception expression, if any. */
731826
ControlFlowNode getException() {
732827
exists(Py::Raise r |
@@ -755,6 +850,9 @@ class StarredNode extends ControlFlowNode {
755850
class ExceptFlowNode extends ControlFlowNode {
756851
ExceptFlowNode() { exists(Py::ExceptStmt e | toAst(this) = e.getName()) }
757852

853+
/** Gets the CFG node for the bound `as`-name itself. */
854+
ControlFlowNode getName() { result = this }
855+
758856
/** Gets the type expression of this exception handler. */
759857
ControlFlowNode getType() {
760858
exists(Py::ExceptStmt e |
@@ -768,6 +866,9 @@ class ExceptFlowNode extends ControlFlowNode {
768866
/** A control flow node corresponding to an `except*` clause's name binding. */
769867
class ExceptGroupFlowNode extends ControlFlowNode {
770868
ExceptGroupFlowNode() { exists(Py::ExceptGroupStmt e | toAst(this) = e.getName()) }
869+
870+
/** Gets the CFG node for the bound `as`-name itself. */
871+
ControlFlowNode getName() { result = this }
771872
}
772873

773874
/** Abstract base class for sequence nodes (tuple, list). */
@@ -823,6 +924,15 @@ class SetNode extends ControlFlowNode {
823924
class DictNode extends ControlFlowNode {
824925
DictNode() { toAst(this) instanceof Py::Dict }
825926

927+
/** Gets the flow node for a key of the dict. */
928+
ControlFlowNode getAKey() {
929+
exists(Py::Dict d |
930+
d = toAst(this) and
931+
d.getAKey() = toAst(result) and
932+
result.getBasicBlock().dominates(this.getBasicBlock())
933+
)
934+
}
935+
826936
/** Gets the flow node for a value of the dict. */
827937
ControlFlowNode getAValue() {
828938
exists(Py::Dict d |

0 commit comments

Comments
 (0)