mirror of
https://github.com/github/codeql.git
synced 2026-07-10 22:15:35 +02:00
Python: deprecate AstNode.getAFlowNode() and rewrite internal callers
Preparatory refactor for the shared-CFG dataflow migration. Deprecates the AstNode.getAFlowNode() cached predicate on the public Python QL API and rewrites all ~140 internal callers across lib/, src/, test/, and tools/ from `expr.getAFlowNode() = cfgNode` to `cfgNode.getNode() = expr`, using ControlFlowNode.getNode() which already exists in Flow.qll. The predicate itself is preserved (with a deprecation note pointing at the new pattern) so external users do not experience churn — they can migrate at their own pace and the AST/CFG hierarchies still get the intended untangling once the deprecation eventually elapses. Semantic noop verified by: - All 361 lib/ + src/ queries compile clean. - All 122 ControlFlow + PointsTo library-tests pass. - All 64 dataflow library-tests pass. - All 113 Variables/Exceptions/Expressions/Statements/Functions/Imports/ Security/CWE-798/ModificationOfParameterWithDefault query-tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
@@ -5,24 +5,30 @@ private import semmle.python.dataflow.new.DataFlow
|
||||
|
||||
private predicate constCompare(DataFlow::GuardNode g, ControlFlowNode node, boolean branch) {
|
||||
exists(CompareNode cn | cn = g |
|
||||
exists(ImmutableLiteral const, Cmpop op |
|
||||
op = any(Eq eq) and branch = true
|
||||
or
|
||||
op = any(NotEq ne) and branch = false
|
||||
exists(ImmutableLiteral const, Cmpop op, ControlFlowNode c |
|
||||
c.getNode() = const and
|
||||
(
|
||||
op = any(Eq eq) and branch = true
|
||||
or
|
||||
op = any(NotEq ne) and branch = false
|
||||
)
|
||||
|
|
||||
cn.operands(const.getAFlowNode(), op, node)
|
||||
cn.operands(c, op, node)
|
||||
or
|
||||
cn.operands(node, op, const.getAFlowNode())
|
||||
cn.operands(node, op, c)
|
||||
)
|
||||
or
|
||||
exists(NameConstant const, Cmpop op |
|
||||
op = any(Is is_) and branch = true
|
||||
or
|
||||
op = any(IsNot isn) and branch = false
|
||||
exists(NameConstant const, Cmpop op, ControlFlowNode c |
|
||||
c.getNode() = const and
|
||||
(
|
||||
op = any(Is is_) and branch = true
|
||||
or
|
||||
op = any(IsNot isn) and branch = false
|
||||
)
|
||||
|
|
||||
cn.operands(const.getAFlowNode(), op, node)
|
||||
cn.operands(c, op, node)
|
||||
or
|
||||
cn.operands(node, op, const.getAFlowNode())
|
||||
cn.operands(node, op, c)
|
||||
)
|
||||
or
|
||||
exists(IterableNode const_iterable, Cmpop op |
|
||||
|
||||
@@ -228,7 +228,7 @@ private class ClassDefinitionAsAttrWrite extends AttrWrite, CfgNode {
|
||||
|
||||
override Node getValue() { result.asCfgNode() = node.getValue() }
|
||||
|
||||
override Node getObject() { result.asCfgNode() = cls.getAFlowNode() }
|
||||
override Node getObject() { result.asCfgNode().getNode() = cls }
|
||||
|
||||
override ExprNode getAttributeNameExpr() { none() }
|
||||
|
||||
|
||||
@@ -1913,8 +1913,8 @@ abstract class ReturnNode extends Node {
|
||||
class ExtractedReturnNode extends ReturnNode, CfgNode {
|
||||
// See `TaintTrackingImplementation::returnFlowStep`
|
||||
ExtractedReturnNode() {
|
||||
node = any(Return ret).getValue().getAFlowNode() or
|
||||
node = any(Yield yield).getAFlowNode()
|
||||
node.getNode() = any(Return ret).getValue() or
|
||||
node.getNode() = any(Yield yield)
|
||||
}
|
||||
|
||||
override ReturnKind getKind() { any() }
|
||||
@@ -1932,7 +1932,7 @@ class ExtractedReturnNode extends ReturnNode, CfgNode {
|
||||
class YieldNodeInContextManagerFunction extends ReturnNode, CfgNode {
|
||||
YieldNodeInContextManagerFunction() {
|
||||
hasContextmanagerDecorator(node.getScope()) and
|
||||
node = any(Yield yield).getValue().getAFlowNode()
|
||||
node.getNode() = any(Yield yield).getValue()
|
||||
}
|
||||
|
||||
override ReturnKind getKind() { any() }
|
||||
|
||||
@@ -185,8 +185,8 @@ private predicate synthDictSplatArgumentNodeStoreStep(
|
||||
*/
|
||||
predicate yieldStoreStep(Node nodeFrom, Content c, Node nodeTo) {
|
||||
exists(Yield yield |
|
||||
nodeTo.asCfgNode() = yield.getAFlowNode() and
|
||||
nodeFrom.asCfgNode() = yield.getValue().getAFlowNode() and
|
||||
nodeTo.asCfgNode().getNode() = yield and
|
||||
nodeFrom.asCfgNode().getNode() = yield.getValue() and
|
||||
// TODO: Consider if this will also need to transfer dictionary content
|
||||
// once dictionary comprehensions are supported.
|
||||
c instanceof ListElementContent
|
||||
|
||||
@@ -485,7 +485,7 @@ class ModuleVariableNode extends Node, TModuleVariableNode {
|
||||
|
||||
/** Gets a node that reads this variable, excluding reads that happen through `from ... import *`. */
|
||||
Node getALocalRead() {
|
||||
result.asCfgNode() = var.getALoad().getAFlowNode() and
|
||||
result.asCfgNode().getNode() = var.getALoad() and
|
||||
not result.getScope() = mod
|
||||
}
|
||||
|
||||
|
||||
@@ -61,7 +61,7 @@ private module CaptureInput implements Shared::InputSig<Location, Cfg::BasicBloc
|
||||
class VariableWrite extends ControlFlowNode {
|
||||
CapturedVariable v;
|
||||
|
||||
VariableWrite() { this = v.getAStore().getAFlowNode().(DefinitionNode).getValue() }
|
||||
VariableWrite() { exists(DefinitionNode d | d.getNode() = v.getAStore() | this = d.getValue()) }
|
||||
|
||||
CapturedVariable getVariable() { result = v }
|
||||
|
||||
@@ -71,7 +71,7 @@ private module CaptureInput implements Shared::InputSig<Location, Cfg::BasicBloc
|
||||
class VariableRead extends Expr {
|
||||
CapturedVariable v;
|
||||
|
||||
VariableRead() { this = v.getALoad().getAFlowNode() }
|
||||
VariableRead() { this.getNode() = v.getALoad() }
|
||||
|
||||
CapturedVariable getVariable() { result = v }
|
||||
}
|
||||
|
||||
@@ -448,8 +448,7 @@ class TaintTrackingImplementation extends string instanceof TaintTracking::Confi
|
||||
context = TNoParam() and
|
||||
src = TTaintTrackingNode_(retval, TNoParam(), path, kind, this) and
|
||||
node.asCfgNode() = call and
|
||||
retval.asCfgNode() =
|
||||
any(Return ret | ret.getScope() = pyfunc.getScope()).getValue().getAFlowNode()
|
||||
retval.asCfgNode().getNode() = any(Return ret | ret.getScope() = pyfunc.getScope()).getValue()
|
||||
) and
|
||||
edgeLabel = "return"
|
||||
}
|
||||
@@ -471,8 +470,7 @@ class TaintTrackingImplementation extends string instanceof TaintTracking::Confi
|
||||
this.callContexts(call, src, pyfunc, context, callee) and
|
||||
retnode = TTaintTrackingNode_(retval, callee, path, kind, this) and
|
||||
node.asCfgNode() = call and
|
||||
retval.asCfgNode() =
|
||||
any(Return ret | ret.getScope() = pyfunc.getScope()).getValue().getAFlowNode()
|
||||
retval.asCfgNode().getNode() = any(Return ret | ret.getScope() = pyfunc.getScope()).getValue()
|
||||
) and
|
||||
edgeLabel = "call"
|
||||
}
|
||||
@@ -716,8 +714,10 @@ private class EssaTaintTracking extends string instanceof TaintTracking::Configu
|
||||
src = TTaintTrackingNode_(srcnode, context, path, srckind, this) and
|
||||
path.noAttribute()
|
||||
|
|
||||
assign.getValue().getAFlowNode() = srcnode.asCfgNode() and
|
||||
depth = iterable_unpacking_descent(assign.getATarget().getAFlowNode(), defn.getDefiningNode()) and
|
||||
srcnode.asCfgNode().getNode() = assign.getValue() and
|
||||
exists(SequenceNode left_parent | left_parent.getNode() = assign.getATarget() |
|
||||
depth = iterable_unpacking_descent(left_parent, defn.getDefiningNode())
|
||||
) and
|
||||
kind = taint_at_depth(srckind, depth)
|
||||
)
|
||||
}
|
||||
@@ -964,7 +964,7 @@ private TaintKind taint_at_depth(SequenceKind parent_kind, int depth) {
|
||||
* - with `left_defn` = `*y`, `left_parent` = `((x, *y), ...)`, result = 1
|
||||
*/
|
||||
int iterable_unpacking_descent(SequenceNode left_parent, ControlFlowNode left_defn) {
|
||||
exists(Assign a | a.getATarget().getASubExpression*().getAFlowNode() = left_parent) and
|
||||
exists(Assign a | left_parent.getNode() = a.getATarget().getASubExpression*()) and
|
||||
left_parent.getAnElement() = left_defn and
|
||||
// Handle `a, *b = some_iterable`
|
||||
if left_defn instanceof StarredNode then result = 0 else result = 1
|
||||
|
||||
Reference in New Issue
Block a user