Skip to content

Python: Add missing flow for AssignmentExpr nodes #14417

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions python/ql/lib/semmle/python/Flow.qll
Original file line number Diff line number Diff line change
Expand Up @@ -547,6 +547,31 @@ class IfExprNode extends ControlFlowNode {
override IfExp getNode() { result = super.getNode() }
}

/** A control flow node corresponding to an assignment expression such as `lhs := rhs`. */
class AssignmentExprNode extends ControlFlowNode {
AssignmentExprNode() { toAst(this) instanceof AssignExpr }

/** Gets the flow node corresponding to the left-hand side of the assignment expression */
ControlFlowNode getTarget() {
exists(AssignExpr a |
this.getNode() = a and
a.getTarget() = result.getNode() and
result.getBasicBlock().dominates(this.getBasicBlock())
)
}

/** Gets the flow node corresponding to the right-hand side of the assignment expression */
ControlFlowNode getValue() {
exists(AssignExpr a |
this.getNode() = a and
a.getValue() = result.getNode() and
result.getBasicBlock().dominates(this.getBasicBlock())
)
}

override AssignExpr getNode() { result = super.getNode() }
}

/** A control flow node corresponding to a binary expression, such as `x + y` */
class BinaryExprNode extends ControlFlowNode {
BinaryExprNode() { toAst(this) instanceof BinaryExpr }
Expand Down Expand Up @@ -630,6 +655,8 @@ class DefinitionNode extends ControlFlowNode {
Stages::AST::ref() and
exists(Assign a | a.getATarget().getAFlowNode() = this)
or
exists(AssignExpr a | a.getTarget().getAFlowNode() = this)
or
exists(AnnAssign a | a.getTarget().getAFlowNode() = this and exists(a.getValue()))
or
exists(Alias a | a.getAsname().getAFlowNode() = this)
Expand Down Expand Up @@ -787,6 +814,9 @@ private AstNode assigned_value(Expr lhs) {
/* lhs = result */
exists(Assign a | a.getATarget() = lhs and result = a.getValue())
or
/* lhs := result */
exists(AssignExpr a | a.getTarget() = lhs and result = a.getValue())
or
/* lhs : annotation = result */
exists(AnnAssign a | a.getTarget() = lhs and result = a.getValue())
or
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,9 @@ module EssaFlow {
// If expressions
nodeFrom.asCfgNode() = nodeTo.asCfgNode().(IfExprNode).getAnOperand()
or
// Assignment expressions
nodeFrom.asCfgNode() = nodeTo.asCfgNode().(AssignmentExprNode).getValue()
or
// boolean inline expressions such as `x or y` or `x and y`
nodeFrom.asCfgNode() = nodeTo.asCfgNode().(BoolExprNode).getAnOperand()
or
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,10 @@ identityLocalStep
| test.py:167:13:167:18 | ControlFlowNode for SOURCE | Node steps to itself |
| test.py:216:10:216:15 | ControlFlowNode for SOURCE | Node steps to itself |
| test.py:242:9:242:12 | ControlFlowNode for SINK | Node steps to itself |
| test.py:669:9:669:12 | ControlFlowNode for SINK | Node steps to itself |
| test.py:670:9:670:14 | ControlFlowNode for SINK_F | Node steps to itself |
| test.py:678:9:678:12 | ControlFlowNode for SINK | Node steps to itself |
| test.py:686:9:686:12 | ControlFlowNode for SINK | Node steps to itself |
| test.py:692:5:692:8 | ControlFlowNode for SINK | Node steps to itself |
| test.py:673:9:673:12 | ControlFlowNode for SINK | Node steps to itself |
| test.py:674:9:674:14 | ControlFlowNode for SINK_F | Node steps to itself |
| test.py:682:9:682:12 | ControlFlowNode for SINK | Node steps to itself |
| test.py:690:9:690:12 | ControlFlowNode for SINK | Node steps to itself |
| test.py:696:5:696:8 | ControlFlowNode for SINK | Node steps to itself |
missingArgumentCall
multipleArgumentCall
12 changes: 8 additions & 4 deletions python/ql/test/experimental/dataflow/coverage/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -435,10 +435,14 @@ def test_and(x = True):


# 6.12. Assignment expressions
def test_assignment_expression():
def test_assignment_expression_flow_lhs():
x = NONSOURCE
SINK(x := SOURCE) #$ MISSING:flow="SOURCE -> x"
if x := SOURCE:
SINK(x) #$ flow="SOURCE, l:-1 -> x"

def test_assignment_expression_flow_out():
x = NONSOURCE
SINK(x := SOURCE) #$ flow="SOURCE -> AssignExpr"

# 6.13. Conditional expressions
def test_conditional_true():
Expand All @@ -460,13 +464,13 @@ def test_conditional_false_guards():
# Condition is evaluated first, so x is SOURCE once chosen
def test_conditional_evaluation_true():
x = NONSOURCE
SINK(x if (SOURCE == (x := SOURCE)) else NONSOURCE) #$ MISSING:flow="SOURCE -> IfExp"
SINK(x if (SOURCE == (x := SOURCE)) else NONSOURCE) #$ flow="SOURCE -> IfExp"


# Condition is evaluated first, so x is SOURCE once chosen
def test_conditional_evaluation_false():
x = NONSOURCE
SINK(NONSOURCE if (NONSOURCE == (x := SOURCE)) else x) #$ MISSING:flow="SOURCE -> IfExp"
SINK(NONSOURCE if (NONSOURCE == (x := SOURCE)) else x) #$ flow="SOURCE -> IfExp"


# 6.14. Lambdas
Expand Down