Skip to content

Commit 7947dbf

Browse files
paldepindpaldepind
committed
Rust: Use Call in data flow
1 parent f16238a commit 7947dbf

File tree

6 files changed

+51
-46
lines changed

6 files changed

+51
-46
lines changed

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

Lines changed: 16 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ private import codeql.util.Boolean
88
private import codeql.dataflow.DataFlow
99
private import codeql.dataflow.internal.DataFlowImpl
1010
private import rust
11+
private import codeql.rust.elements.Call
1112
private import SsaImpl as SsaImpl
1213
private import codeql.rust.controlflow.internal.Scope as Scope
1314
private import codeql.rust.internal.PathResolution
@@ -55,11 +56,7 @@ final class DataFlowCallable extends TDataFlowCallable {
5556

5657
final class DataFlowCall extends TDataFlowCall {
5758
/** Gets the underlying call in the CFG, if any. */
58-
CallExprCfgNode asCallExprCfgNode() { result = this.asCallBaseExprCfgNode() }
59-
60-
MethodCallExprCfgNode asMethodCallExprCfgNode() { result = this.asCallBaseExprCfgNode() }
61-
62-
CallExprBaseCfgNode asCallBaseExprCfgNode() { this = TCall(result) }
59+
CallCfgNode asCallCfgNode() { this = TCall(result) }
6360

6461
predicate isSummaryCall(
6562
FlowSummaryImpl::Public::SummarizedCallable c, FlowSummaryImpl::Private::SummaryNode receiver
@@ -68,7 +65,7 @@ final class DataFlowCall extends TDataFlowCall {
6865
}
6966

7067
DataFlowCallable getEnclosingCallable() {
71-
result = TCfgScope(this.asCallBaseExprCfgNode().getExpr().getEnclosingCfgScope())
68+
result = TCfgScope(this.asCallCfgNode().getExpr().getEnclosingCfgScope())
7269
or
7370
exists(FlowSummaryImpl::Public::SummarizedCallable c |
7471
this.isSummaryCall(c, _) and
@@ -77,7 +74,7 @@ final class DataFlowCall extends TDataFlowCall {
7774
}
7875

7976
string toString() {
80-
result = this.asCallBaseExprCfgNode().toString()
77+
result = this.asCallCfgNode().toString()
8178
or
8279
exists(
8380
FlowSummaryImpl::Public::SummarizedCallable c, FlowSummaryImpl::Private::SummaryNode receiver
@@ -87,7 +84,7 @@ final class DataFlowCall extends TDataFlowCall {
8784
)
8885
}
8986

90-
Location getLocation() { result = this.asCallBaseExprCfgNode().getLocation() }
87+
Location getLocation() { result = this.asCallCfgNode().getLocation() }
9188
}
9289

9390
/**
@@ -135,38 +132,23 @@ final class ParameterPosition extends TParameterPosition {
135132
*/
136133
final class ArgumentPosition extends ParameterPosition {
137134
/** Gets the argument of `call` at this position, if any. */
138-
Expr getArgument(CallExprBase call) {
139-
result = call.getArgList().getArg(this.getPosition())
135+
Expr getArgument(Call call) {
136+
result = call.getArgument(this.getPosition())
140137
or
141-
this.isSelf() and
142-
result = call.(MethodCallExpr).getReceiver()
138+
result = call.getReceiver() and this.isSelf()
143139
}
144140
}
145141

146-
/** Holds if `call` invokes a qualified path that resolves to a method. */
147-
private predicate callToMethod(CallExpr call) {
148-
exists(Path path |
149-
path = call.getFunction().(PathExpr).getPath() and
150-
path.hasQualifier() and
151-
resolvePath(path).(Function).getParamList().hasSelfParam()
152-
)
153-
}
154-
155142
/**
156143
* Holds if `arg` is an argument of `call` at the position `pos`.
157144
*
158145
* Note that this does not hold for the receiever expression of a method call
159146
* as the synthetic `ReceiverNode` is the argument for the `self` parameter.
160147
*/
161-
predicate isArgumentForCall(ExprCfgNode arg, CallExprBaseCfgNode call, ParameterPosition pos) {
162-
if callToMethod(call.(CallExprCfgNode).getCallExpr())
163-
then
164-
// The first argument is for the `self` parameter
165-
arg = call.getArgument(0) and pos.isSelf()
166-
or
167-
// Succeeding arguments are shifted left
168-
arg = call.getArgument(pos.getPosition() + 1)
169-
else arg = call.getArgument(pos.getPosition())
148+
predicate isArgumentForCall(ExprCfgNode arg, CallCfgNode call, ParameterPosition pos) {
149+
call.getArgument(pos.getPosition()) = arg
150+
or
151+
call.getReceiver() = arg and pos.isSelf() and not call.getCall().receiverImplicitlyBorrowed()
170152
}
171153

172154
/** Provides logic related to SSA. */
@@ -419,9 +401,9 @@ module RustDataFlow implements InputSig<Location> {
419401

420402
/** Gets a viable implementation of the target of the given `Call`. */
421403
DataFlowCallable viableCallable(DataFlowCall call) {
422-
result.asCfgScope() = call.asCallBaseExprCfgNode().getCallExprBase().getStaticTarget()
404+
result.asCfgScope() = call.asCallCfgNode().getCall().getStaticTarget()
423405
or
424-
result.asLibraryCallable().getACall() = call.asCallBaseExprCfgNode().getCallExprBase()
406+
result.asLibraryCallable().getACall() = call.asCallCfgNode().getCall()
425407
}
426408

427409
/**
@@ -812,7 +794,7 @@ module RustDataFlow implements InputSig<Location> {
812794
*/
813795
predicate lambdaCall(DataFlowCall call, LambdaCallKind kind, Node receiver) {
814796
(
815-
receiver.asExpr() = call.asCallExprCfgNode().getFunction() and
797+
receiver.asExpr() = call.asCallCfgNode().(CallExprCfgNode).getFunction() and
816798
// All calls to complex expressions and local variable accesses are lambda call.
817799
exists(Expr f | f = receiver.asExpr().getExpr() |
818800
f instanceof PathExpr implies f = any(Variable v).getAnAccess()
@@ -976,7 +958,7 @@ private module Cached {
976958

977959
cached
978960
newtype TDataFlowCall =
979-
TCall(CallExprBaseCfgNode c) { Stages::DataFlowStage::ref() } or
961+
TCall(CallCfgNode c) { Stages::DataFlowStage::ref() } or
980962
TSummaryCall(
981963
FlowSummaryImpl::Public::SummarizedCallable c, FlowSummaryImpl::Private::SummaryNode receiver
982964
) {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ private import Make<Location, RustDataFlow, Input> as Impl
153153

154154
private module StepsInput implements Impl::Private::StepsInputSig {
155155
DataFlowCall getACall(Public::SummarizedCallable sc) {
156-
result.asCallBaseExprCfgNode().getCallExprBase() = sc.(LibraryCallable).getACall()
156+
result.asCallCfgNode().getCall() = sc.(LibraryCallable).getACall()
157157
}
158158

159159
RustDataFlow::Node getSourceNode(Input::SourceBase source, Impl::Private::SummaryComponent sc) {

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

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -224,13 +224,13 @@ abstract class ArgumentNode extends Node {
224224
}
225225

226226
final class ExprArgumentNode extends ArgumentNode, ExprNode {
227-
private CallExprBaseCfgNode call_;
227+
private CallCfgNode call_;
228228
private RustDataFlow::ArgumentPosition pos_;
229229

230230
ExprArgumentNode() { isArgumentForCall(n, call_, pos_) }
231231

232232
override predicate isArgumentOf(DataFlowCall call, RustDataFlow::ArgumentPosition pos) {
233-
call.asCallBaseExprCfgNode() = call_ and pos = pos_
233+
call.asCallCfgNode() = call_ and pos = pos_
234234
}
235235
}
236236

@@ -239,7 +239,7 @@ final class ExprArgumentNode extends ArgumentNode, ExprNode {
239239
* has taken place.
240240
*/
241241
final class ReceiverNode extends ArgumentNode, TReceiverNode {
242-
private MethodCallExprCfgNode n;
242+
private CallCfgNode n;
243243

244244
ReceiverNode() { this = TReceiverNode(n, false) }
245245

@@ -248,7 +248,7 @@ final class ReceiverNode extends ArgumentNode, TReceiverNode {
248248
MethodCallExprCfgNode getMethodCall() { result = n }
249249

250250
override predicate isArgumentOf(DataFlowCall call, RustDataFlow::ArgumentPosition pos) {
251-
call.asMethodCallExprCfgNode() = n and pos = TSelfParameterPosition()
251+
call.asCallCfgNode() = n and pos = TSelfParameterPosition()
252252
}
253253

254254
override CfgScope getCfgScope() { result = n.getAstNode().getEnclosingCfgScope() }
@@ -281,7 +281,7 @@ final class ClosureArgumentNode extends ArgumentNode, ExprNode {
281281
ClosureArgumentNode() { lambdaCallExpr(call_, _, this.asExpr()) }
282282

283283
override predicate isArgumentOf(DataFlowCall call, RustDataFlow::ArgumentPosition pos) {
284-
call.asCallExprCfgNode() = call_ and
284+
call.asCallCfgNode() = call_ and
285285
pos.isClosureSelf()
286286
}
287287
}
@@ -330,11 +330,11 @@ abstract class OutNode extends Node {
330330
}
331331

332332
final private class ExprOutNode extends ExprNode, OutNode {
333-
ExprOutNode() { this.asExpr() instanceof CallExprBaseCfgNode }
333+
ExprOutNode() { this.asExpr() instanceof CallCfgNode }
334334

335335
/** Gets the underlying call CFG node that includes this out node. */
336336
override DataFlowCall getCall(ReturnKind kind) {
337-
result.asCallBaseExprCfgNode() = this.getCfgNode() and
337+
result.asCallCfgNode() = this.getCfgNode() and
338338
kind = TNormalReturnKind()
339339
}
340340
}
@@ -404,7 +404,7 @@ final class ExprPostUpdateNode extends PostUpdateNode, TExprPostUpdateNode {
404404
}
405405

406406
final class ReceiverPostUpdateNode extends PostUpdateNode, TReceiverNode {
407-
private MethodCallExprCfgNode n;
407+
private CallCfgNode n;
408408

409409
ReceiverPostUpdateNode() { this = TReceiverNode(n, true) }
410410

@@ -467,11 +467,12 @@ newtype TNode =
467467
any(FieldExprCfgNode access).getContainer(), //
468468
any(TryExprCfgNode try).getExpr(), //
469469
any(PrefixExprCfgNode pe | pe.getOperatorName() = "*").getExpr(), //
470-
any(AwaitExprCfgNode a).getExpr(), any(MethodCallExprCfgNode mc).getReceiver(), //
470+
any(AwaitExprCfgNode a).getExpr(), //
471+
any(MethodCallExprCfgNode mc).getReceiver(), //
471472
getPostUpdateReverseStep(any(PostUpdateNode n).getPreUpdateNode().asExpr(), _)
472473
]
473474
} or
474-
TReceiverNode(MethodCallExprCfgNode mc, Boolean isPost) or
475+
TReceiverNode(CallCfgNode mc, Boolean isPost) { mc.getCall().receiverImplicitlyBorrowed() } or
475476
TSsaNode(SsaImpl::DataFlowIntegration::SsaNode node) or
476477
TFlowSummaryNode(FlowSummaryImpl::Private::SummaryNode sn) or
477478
TClosureSelfReferenceNode(CfgScope c) { lambdaCreationExpr(c, _) } or

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,14 @@ edges
8585
| main.rs:175:9:175:35 | MyInt {...} [MyInt] | main.rs:173:42:176:5 | { ... } [MyInt] | provenance | |
8686
| main.rs:175:24:175:27 | self [MyInt] | main.rs:175:24:175:33 | self.value | provenance | |
8787
| main.rs:175:24:175:33 | self.value | main.rs:175:9:175:35 | MyInt {...} [MyInt] | provenance | |
88+
| main.rs:195:9:195:9 | a [MyInt] | main.rs:197:13:197:13 | a [MyInt] | provenance | |
89+
| main.rs:195:13:195:38 | MyInt {...} [MyInt] | main.rs:195:9:195:9 | a [MyInt] | provenance | |
90+
| main.rs:195:28:195:36 | source(...) | main.rs:195:13:195:38 | MyInt {...} [MyInt] | provenance | |
91+
| main.rs:197:9:197:9 | c [MyInt] | main.rs:198:10:198:10 | c [MyInt] | provenance | |
92+
| main.rs:197:13:197:13 | a [MyInt] | main.rs:173:12:173:15 | SelfParam [MyInt] | provenance | |
93+
| main.rs:197:13:197:13 | a [MyInt] | main.rs:197:13:197:17 | ... + ... [MyInt] | provenance | |
94+
| main.rs:197:13:197:17 | ... + ... [MyInt] | main.rs:197:9:197:9 | c [MyInt] | provenance | |
95+
| main.rs:198:10:198:10 | c [MyInt] | main.rs:198:10:198:16 | c.value | provenance | |
8896
| main.rs:205:9:205:9 | a [MyInt] | main.rs:173:12:173:15 | SelfParam [MyInt] | provenance | |
8997
| main.rs:205:9:205:9 | a [MyInt] | main.rs:207:13:207:20 | a.add(...) [MyInt] | provenance | |
9098
| main.rs:205:13:205:38 | MyInt {...} [MyInt] | main.rs:205:9:205:9 | a [MyInt] | provenance | |
@@ -213,6 +221,14 @@ nodes
213221
| main.rs:175:9:175:35 | MyInt {...} [MyInt] | semmle.label | MyInt {...} [MyInt] |
214222
| main.rs:175:24:175:27 | self [MyInt] | semmle.label | self [MyInt] |
215223
| main.rs:175:24:175:33 | self.value | semmle.label | self.value |
224+
| main.rs:195:9:195:9 | a [MyInt] | semmle.label | a [MyInt] |
225+
| main.rs:195:13:195:38 | MyInt {...} [MyInt] | semmle.label | MyInt {...} [MyInt] |
226+
| main.rs:195:28:195:36 | source(...) | semmle.label | source(...) |
227+
| main.rs:197:9:197:9 | c [MyInt] | semmle.label | c [MyInt] |
228+
| main.rs:197:13:197:13 | a [MyInt] | semmle.label | a [MyInt] |
229+
| main.rs:197:13:197:17 | ... + ... [MyInt] | semmle.label | ... + ... [MyInt] |
230+
| main.rs:198:10:198:10 | c [MyInt] | semmle.label | c [MyInt] |
231+
| main.rs:198:10:198:16 | c.value | semmle.label | c.value |
216232
| main.rs:205:9:205:9 | a [MyInt] | semmle.label | a [MyInt] |
217233
| main.rs:205:13:205:38 | MyInt {...} [MyInt] | semmle.label | MyInt {...} [MyInt] |
218234
| main.rs:205:28:205:36 | source(...) | semmle.label | source(...) |
@@ -262,6 +278,7 @@ subpaths
262278
| main.rs:134:29:134:29 | a | main.rs:110:27:110:32 | ...: i64 | main.rs:110:42:116:5 | { ... } | main.rs:134:13:134:30 | mn.data_through(...) |
263279
| main.rs:147:38:147:38 | a | main.rs:110:27:110:32 | ...: i64 | main.rs:110:42:116:5 | { ... } | main.rs:147:13:147:39 | ...::data_through(...) |
264280
| main.rs:165:24:165:33 | source(...) | main.rs:159:12:159:17 | ...: i64 | main.rs:159:28:161:5 | { ... } [MyInt] | main.rs:165:13:165:34 | ...::new(...) [MyInt] |
281+
| main.rs:197:13:197:13 | a [MyInt] | main.rs:173:12:173:15 | SelfParam [MyInt] | main.rs:173:42:176:5 | { ... } [MyInt] | main.rs:197:13:197:17 | ... + ... [MyInt] |
265282
| main.rs:205:9:205:9 | a [MyInt] | main.rs:173:12:173:15 | SelfParam [MyInt] | main.rs:173:42:176:5 | { ... } [MyInt] | main.rs:207:13:207:20 | a.add(...) [MyInt] |
266283
| main.rs:254:49:254:49 | a [MyInt] | main.rs:242:18:242:21 | SelfParam [MyInt] | main.rs:242:48:244:5 | { ... } [MyInt] | main.rs:254:30:254:53 | ...::take_self(...) [MyInt] |
267284
| main.rs:259:54:259:54 | b [MyInt] | main.rs:246:26:246:37 | ...: MyInt [MyInt] | main.rs:246:49:248:5 | { ... } [MyInt] | main.rs:259:30:259:55 | ...::take_second(...) [MyInt] |
@@ -280,6 +297,7 @@ testFailures
280297
| main.rs:135:10:135:10 | b | main.rs:133:13:133:21 | source(...) | main.rs:135:10:135:10 | b | $@ | main.rs:133:13:133:21 | source(...) | source(...) |
281298
| main.rs:148:10:148:10 | b | main.rs:146:13:146:22 | source(...) | main.rs:148:10:148:10 | b | $@ | main.rs:146:13:146:22 | source(...) | source(...) |
282299
| main.rs:167:10:167:10 | m | main.rs:165:24:165:33 | source(...) | main.rs:167:10:167:10 | m | $@ | main.rs:165:24:165:33 | source(...) | source(...) |
300+
| main.rs:198:10:198:16 | c.value | main.rs:195:28:195:36 | source(...) | main.rs:198:10:198:16 | c.value | $@ | main.rs:195:28:195:36 | source(...) | source(...) |
283301
| main.rs:208:10:208:16 | d.value | main.rs:205:28:205:36 | source(...) | main.rs:208:10:208:16 | d.value | $@ | main.rs:205:28:205:36 | source(...) | source(...) |
284302
| main.rs:255:10:255:10 | c | main.rs:252:28:252:36 | source(...) | main.rs:255:10:255:10 | c | $@ | main.rs:252:28:252:36 | source(...) | source(...) |
285303
| main.rs:260:10:260:10 | c | main.rs:258:28:258:37 | source(...) | main.rs:260:10:260:10 | c | $@ | main.rs:258:28:258:37 | source(...) | source(...) |

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ fn test_operator_overloading() {
195195
let a = MyInt { value: source(5) };
196196
let b = MyInt { value: 2 };
197197
let c = a + b;
198-
sink(c.value); // $ MISSING: hasValueFlow=5
198+
sink(c.value); // $ hasValueFlow=5
199199

200200
let a = MyInt { value: 2 };
201201
let b = MyInt { value: source(6) };

‎rust/ql/test/library-tests/dataflow/global/viableCallable.expected

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,19 +41,23 @@
4141
| main.rs:165:24:165:33 | source(...) | main.rs:1:1:3:1 | fn source |
4242
| main.rs:167:5:167:11 | sink(...) | main.rs:5:1:7:1 | fn sink |
4343
| main.rs:195:28:195:36 | source(...) | main.rs:1:1:3:1 | fn source |
44+
| main.rs:197:13:197:17 | ... + ... | main.rs:173:5:176:5 | fn add |
4445
| main.rs:198:5:198:17 | sink(...) | main.rs:5:1:7:1 | fn sink |
4546
| main.rs:201:28:201:36 | source(...) | main.rs:1:1:3:1 | fn source |
47+
| main.rs:202:13:202:17 | ... + ... | main.rs:173:5:176:5 | fn add |
4648
| main.rs:203:5:203:17 | sink(...) | main.rs:5:1:7:1 | fn sink |
4749
| main.rs:205:28:205:36 | source(...) | main.rs:1:1:3:1 | fn source |
4850
| main.rs:207:13:207:20 | a.add(...) | main.rs:173:5:176:5 | fn add |
4951
| main.rs:208:5:208:17 | sink(...) | main.rs:5:1:7:1 | fn sink |
5052
| main.rs:212:28:212:37 | source(...) | main.rs:1:1:3:1 | fn source |
5153
| main.rs:215:5:215:17 | sink(...) | main.rs:5:1:7:1 | fn sink |
5254
| main.rs:218:28:218:37 | source(...) | main.rs:1:1:3:1 | fn source |
55+
| main.rs:219:5:219:10 | ... *= ... | main.rs:180:5:182:5 | fn mul_assign |
5356
| main.rs:220:5:220:17 | sink(...) | main.rs:5:1:7:1 | fn sink |
5457
| main.rs:223:28:223:37 | source(...) | main.rs:1:1:3:1 | fn source |
5558
| main.rs:226:5:226:11 | sink(...) | main.rs:5:1:7:1 | fn sink |
5659
| main.rs:228:28:228:37 | source(...) | main.rs:1:1:3:1 | fn source |
60+
| main.rs:229:13:229:14 | * ... | main.rs:188:5:190:5 | fn deref |
5761
| main.rs:230:5:230:11 | sink(...) | main.rs:5:1:7:1 | fn sink |
5862
| main.rs:252:28:252:36 | source(...) | main.rs:1:1:3:1 | fn source |
5963
| main.rs:254:30:254:53 | ...::take_self(...) | main.rs:242:5:244:5 | fn take_self |

0 commit comments

Comments
 (0)