Skip to content

Commit e6887f9

Browse files
hvitvedhvitved
committed
Rust: Use nodes from CfgNodes.qll in DataFlowImpl.qll 
1 parent ca18005 commit e6887f9

File tree

7 files changed

+49
-48
lines changed

7 files changed

+49
-48
lines changed

‎rust/ql/lib/codeql/rust/dataflow/Ssa.qll

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ module Ssa {
99
private import rust
1010
private import codeql.rust.controlflow.BasicBlocks
1111
private import codeql.rust.controlflow.ControlFlowGraph
12+
private import codeql.rust.controlflow.CfgNodes
1213
private import codeql.rust.controlflow.internal.ControlFlowGraphImpl as CfgImpl
1314
private import internal.SsaImpl as SsaImpl
1415

@@ -221,11 +222,11 @@ module Ssa {
221222
* end
222223
* ```
223224
*/
224-
predicate assigns(CfgNode value) {
225-
exists(AssignmentExpr ae, BasicBlock bb, int i |
225+
predicate assigns(ExprCfgNode value) {
226+
exists(AssignmentExprCfgNode ae, BasicBlock bb, int i |
226227
this.definesAt(_, bb, i) and
227-
ae.getLhs() = bb.getNode(i).getAstNode() and
228-
value.getAstNode() = ae.getRhs()
228+
ae.getLhs() = bb.getNode(i) and
229+
value = ae.getRhs()
229230
)
230231
}
231232

‎rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll

Lines changed: 28 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ final class DataFlowCall extends TDataFlowCall {
4848

4949
MethodCallExprCfgNode asMethodCallExprCfgNode() { this = TMethodCall(result) }
5050

51-
ExprCfgNode asExprCfgNode() {
51+
CallExprBaseCfgNode asExprCfgNode() {
5252
result = this.asCallExprCfgNode() or result = this.asMethodCallExprCfgNode()
5353
}
5454

@@ -76,7 +76,7 @@ module Node {
7676
/**
7777
* Gets the expression that corresponds to this node, if any.
7878
*/
79-
Expr asExpr() { none() }
79+
ExprCfgNode asExpr() { none() }
8080

8181
/** Gets the enclosing callable. */
8282
DataFlowCallable getEnclosingCallable() { result = TCfgScope(this.getCfgScope()) }
@@ -115,13 +115,13 @@ module Node {
115115
abstract class AstCfgFlowNode extends Node {
116116
AstCfgNode n;
117117

118-
override CfgNode getCfgNode() { result = n }
118+
final override CfgNode getCfgNode() { result = n }
119119

120-
override CfgScope getCfgScope() { result = n.getAstNode().getEnclosingCfgScope() }
120+
final override CfgScope getCfgScope() { result = n.getAstNode().getEnclosingCfgScope() }
121121

122-
override Location getLocation() { result = n.getAstNode().getLocation() }
122+
final override Location getLocation() { result = n.getAstNode().getLocation() }
123123

124-
override string toString() { result = n.getAstNode().toString() }
124+
final override string toString() { result = n.getAstNode().toString() }
125125
}
126126

127127
/**
@@ -137,7 +137,7 @@ module Node {
137137

138138
ExprNode() { this = TExprNode(n) }
139139

140-
override Expr asExpr() { result = n.getExpr() }
140+
override ExprCfgNode asExpr() { result = n }
141141
}
142142

143143
final class PatNode extends AstCfgFlowNode, TPatNode {
@@ -159,7 +159,7 @@ module Node {
159159
ParameterNode() { this = TParameterNode(n) }
160160

161161
/** Gets the parameter in the AST that this node corresponds to. */
162-
Param getParameter() { result = n.getParam() }
162+
ParamCfgNode getParameter() { result = n }
163163
}
164164

165165
final class ArgumentNode = NaNode;
@@ -197,7 +197,7 @@ module Node {
197197
}
198198

199199
final private class ExprOutNode extends ExprNode, OutNode {
200-
ExprOutNode() { this.asExpr() instanceof CallExpr }
200+
ExprOutNode() { this.asExpr() instanceof CallExprCfgNode }
201201

202202
/** Gets the underlying call CFG node that includes this out node. */
203203
override DataFlowCall getCall() { result.asExprCfgNode() = this.getCfgNode() }
@@ -228,13 +228,13 @@ final class Node = Node::Node;
228228
module SsaFlow {
229229
private module Impl = SsaImpl::DataFlowIntegration;
230230

231-
private Node::ParameterNode toParameterNode(Param p) { result.getParameter() = p }
231+
private Node::ParameterNode toParameterNode(ParamCfgNode p) { result.getParameter() = p }
232232

233233
/** Converts a control flow node into an SSA control flow node. */
234234
Impl::Node asNode(Node n) {
235235
n = TSsaNode(result)
236236
or
237-
result.(Impl::ExprNode).getExpr() = n.(Node::ExprNode).getCfgNode()
237+
result.(Impl::ExprNode).getExpr() = n.asExpr()
238238
or
239239
n = toParameterNode(result.(Impl::ParameterNode).getParameter())
240240
}
@@ -248,46 +248,40 @@ module SsaFlow {
248248
}
249249
}
250250

251-
/**
252-
* Holds for expressions `e` that evaluate to the value of any last (in
253-
* evaluation order) subexpressions within it. E.g., expressions that propagate
254-
* a values from a subexpression.
255-
*
256-
* For instance, the predicate holds for if expressions as `if b { e1 } else {
257-
* e2 }` evalates to the value of one of the subexpressions `e1` or `e2`.
258-
*/
259-
private predicate propagatesValue(Expr e) {
260-
e instanceof IfExpr or
261-
e instanceof LoopExpr or
262-
e instanceof ReturnExpr or
263-
e instanceof BreakExpr or
264-
e.(BlockExpr).getStmtList().hasTailExpr() or
265-
e instanceof MatchExpr
266-
}
267-
268251
/**
269252
* Gets a node that may execute last in `n`, and which, when it executes last,
270253
* will be the value of `n`.
271254
*/
272-
private ExprCfgNode getALastEvalNode(ExprCfgNode n) {
273-
propagatesValue(n.getExpr()) and result.getASuccessor() = n
255+
private ExprCfgNode getALastEvalNode(ExprCfgNode e) {
256+
e = any(IfExprCfgNode n | result = [n.getThen(), n.getElse()]) or
257+
result = e.(LoopExprCfgNode).getLoopBody() or
258+
result = e.(ReturnExprCfgNode).getExpr() or
259+
result = e.(BreakExprCfgNode).getExpr() or
260+
result = e.(BlockExprCfgNode).getTailExpr() or
261+
result = e.(MatchExprCfgNode).getArmExpr(_) or
262+
result.(BreakExprCfgNode).getTarget() = e
274263
}
275264

276265
module LocalFlow {
277266
pragma[nomagic]
278267
predicate localFlowStepCommon(Node nodeFrom, Node nodeTo) {
279268
nodeFrom.getCfgNode() = getALastEvalNode(nodeTo.getCfgNode())
280269
or
281-
exists(LetStmt s |
282-
nodeFrom.getCfgNode().getAstNode() = s.getInitializer() and
283-
nodeTo.getCfgNode().getAstNode() = s.getPat()
270+
exists(LetStmtCfgNode s |
271+
nodeFrom.getCfgNode() = s.getInitializer() and
272+
nodeTo.getCfgNode() = s.getPat()
284273
)
285274
or
286275
// An edge from a pattern/expression to its corresponding SSA definition.
287276
nodeFrom.(Node::AstCfgFlowNode).getCfgNode() =
288277
nodeTo.(Node::SsaNode).getDefinitionExt().(Ssa::WriteDefinition).getControlFlowNode()
289278
or
290279
SsaFlow::localFlowStep(_, nodeFrom, nodeTo, _)
280+
or
281+
exists(AssignmentExprCfgNode a |
282+
a.getRhs() = nodeFrom.getCfgNode() and
283+
a.getLhs() = nodeTo.getCfgNode()
284+
)
291285
}
292286
}
293287

@@ -329,7 +323,7 @@ module RustDataFlow implements InputSig<Location> {
329323
class DataFlowExpr = ExprCfgNode;
330324

331325
/** Gets the node corresponding to `e`. */
332-
Node exprNode(DataFlowExpr e) { result.getCfgNode() = e }
326+
Node exprNode(DataFlowExpr e) { result.asExpr() = e }
333327

334328
final class DataFlowCall = DataFlowCallAlias;
335329

‎rust/ql/lib/codeql/rust/dataflow/internal/SsaImpl.qll

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -474,14 +474,14 @@ private module DataFlowIntegrationInput implements Impl::DataFlowIntegrationInpu
474474

475475
/** Holds if SSA definition `def` assigns `value` to the underlying variable. */
476476
predicate ssaDefAssigns(WriteDefinition def, Expr value) {
477-
exists(BasicBlock bb, int i | def.definesAt(_, bb, i) and value = bb.getNode(i))
477+
none() // handled in `DataFlowImpl.qll` instead
478478
}
479479

480-
class Parameter = Param;
480+
class Parameter = CfgNodes::ParamCfgNode;
481481

482482
/** Holds if SSA definition `def` initializes parameter `p` at function entry. */
483483
predicate ssaDefInitializesParam(WriteDefinition def, Parameter p) {
484-
exists(BasicBlock bb, int i | bb.getNode(i).getAstNode() = p and def.definesAt(_, bb, i))
484+
none() // handled in `DataFlowImpl.qll` instead
485485
}
486486

487487
class Guard extends CfgNodes::AstCfgNode {

‎rust/ql/test/library-tests/dataflow/local/DataFlowStep.expected

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
| main.rs:51:17:51:17 | 1 | main.rs:51:9:51:13 | i |
4747
| main.rs:53:5:53:5 | [SSA] i | main.rs:54:10:54:10 | i |
4848
| main.rs:53:5:53:5 | i | main.rs:53:5:53:5 | [SSA] i |
49+
| main.rs:53:9:53:17 | CallExpr | main.rs:53:5:53:5 | i |
4950
| main.rs:61:9:61:9 | [SSA] i | main.rs:62:11:62:11 | i |
5051
| main.rs:61:9:61:9 | i | main.rs:61:9:61:9 | [SSA] i |
5152
| main.rs:61:13:61:31 | CallExpr | main.rs:61:9:61:9 | i |
@@ -93,7 +94,6 @@
9394
| main.rs:122:9:122:9 | [SSA] a | main.rs:128:5:128:5 | a |
9495
| main.rs:122:9:122:9 | a | main.rs:122:9:122:9 | [SSA] a |
9596
| main.rs:122:13:127:5 | BlockExpr | main.rs:122:9:122:9 | a |
96-
| main.rs:123:12:123:12 | b | main.rs:123:9:125:9 | IfExpr |
9797
| main.rs:124:13:124:26 | BreakExpr | main.rs:122:13:127:5 | BlockExpr |
9898
| main.rs:124:26:124:26 | 1 | main.rs:124:13:124:26 | BreakExpr |
9999
| main.rs:126:9:126:9 | 2 | main.rs:122:13:127:5 | BlockExpr |
@@ -103,7 +103,8 @@
103103
| main.rs:132:9:132:9 | [SSA] a | main.rs:138:5:138:5 | a |
104104
| main.rs:132:9:132:9 | a | main.rs:132:9:132:9 | [SSA] a |
105105
| main.rs:132:13:137:5 | BlockExpr | main.rs:132:9:132:9 | a |
106-
| main.rs:133:12:133:12 | b | main.rs:133:9:135:9 | IfExpr |
106+
| main.rs:134:13:134:26 | BreakExpr | main.rs:132:13:137:5 | BlockExpr |
107107
| main.rs:134:26:134:26 | 1 | main.rs:134:13:134:26 | BreakExpr |
108+
| main.rs:136:9:136:22 | BreakExpr | main.rs:132:13:137:5 | BlockExpr |
108109
| main.rs:136:22:136:22 | 2 | main.rs:136:9:136:22 | BreakExpr |
109110
| main.rs:138:5:138:5 | a | main.rs:131:38:139:1 | BlockExpr |

‎rust/ql/test/library-tests/dataflow/local/inline-flow.expected

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ edges
44
| main.rs:24:13:24:21 | CallExpr : unit | main.rs:27:10:27:10 | c | provenance | |
55
| main.rs:31:13:31:21 | CallExpr : unit | main.rs:36:10:36:10 | b | provenance | |
66
| main.rs:45:15:45:23 | CallExpr : unit | main.rs:47:10:47:10 | b | provenance | |
7+
| main.rs:53:9:53:17 | CallExpr : unit | main.rs:54:10:54:10 | i | provenance | |
78
nodes
89
| main.rs:15:10:15:18 | CallExpr | semmle.label | CallExpr |
910
| main.rs:19:13:19:21 | CallExpr : unit | semmle.label | CallExpr : unit |
@@ -14,6 +15,8 @@ nodes
1415
| main.rs:36:10:36:10 | b | semmle.label | b |
1516
| main.rs:45:15:45:23 | CallExpr : unit | semmle.label | CallExpr : unit |
1617
| main.rs:47:10:47:10 | b | semmle.label | b |
18+
| main.rs:53:9:53:17 | CallExpr : unit | semmle.label | CallExpr : unit |
19+
| main.rs:54:10:54:10 | i | semmle.label | i |
1720
subpaths
1821
testFailures
1922
#select
@@ -22,3 +25,4 @@ testFailures
2225
| main.rs:27:10:27:10 | c | main.rs:24:13:24:21 | CallExpr : unit | main.rs:27:10:27:10 | c | $@ | main.rs:24:13:24:21 | CallExpr : unit | CallExpr : unit |
2326
| main.rs:36:10:36:10 | b | main.rs:31:13:31:21 | CallExpr : unit | main.rs:36:10:36:10 | b | $@ | main.rs:31:13:31:21 | CallExpr : unit | CallExpr : unit |
2427
| main.rs:47:10:47:10 | b | main.rs:45:15:45:23 | CallExpr : unit | main.rs:47:10:47:10 | b | $@ | main.rs:45:15:45:23 | CallExpr : unit | CallExpr : unit |
28+
| main.rs:54:10:54:10 | i | main.rs:53:9:53:17 | CallExpr : unit | main.rs:54:10:54:10 | i | $@ | main.rs:53:9:53:17 | CallExpr : unit | CallExpr : unit |

‎rust/ql/test/library-tests/dataflow/local/main.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ fn assignment() {
5151
let mut i = 1;
5252
sink(i);
5353
i = source(2);
54-
sink(i); // $ MISSING: hasValueFlow=2
54+
sink(i); // $ hasValueFlow=2
5555
}
5656

5757
// -----------------------------------------------------------------------------

‎rust/ql/test/utils/InlineFlowTest.qll

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,26 +5,27 @@
55

66
import rust
77
private import codeql.dataflow.test.InlineFlowTest
8+
private import codeql.rust.controlflow.CfgNodes
89
private import codeql.rust.dataflow.DataFlow
910
private import codeql.rust.dataflow.internal.DataFlowImpl
1011
private import codeql.rust.dataflow.internal.TaintTrackingImpl
1112
private import internal.InlineExpectationsTestImpl as InlineExpectationsTestImpl
1213

1314
// Holds if the target expression of `call` is a path and the string representation of the path is `name`.
14-
private predicate callTargetName(CallExpr call, string name) {
15-
call.getExpr().(PathExpr).toString() = name
15+
private predicate callTargetName(CallExprCfgNode call, string name) {
16+
call.getExpr().(PathExprCfgNode).toString() = name
1617
}
1718

1819
private module FlowTestImpl implements InputSig<Location, RustDataFlow> {
1920
predicate defaultSource(DataFlow::Node source) { callTargetName(source.asExpr(), "source") }
2021

2122
predicate defaultSink(DataFlow::Node sink) {
22-
any(CallExpr call | callTargetName(call, "sink")).getArgList().getAnArg() = sink.asExpr()
23+
any(CallExprCfgNode call | callTargetName(call, "sink")).getArgument(_) = sink.asExpr()
2324
}
2425

2526
private string getSourceArgString(DataFlow::Node src) {
2627
defaultSource(src) and
27-
result = src.asExpr().(CallExpr).getArgList().getArg(0).toString()
28+
result = src.asExpr().(CallExprCfgNode).getArgument(0).toString()
2829
}
2930

3031
bindingset[src, sink]

0 commit comments

Comments
 (0)