Skip to content

Commit 3346914

Browse files
hvitvedhvitved
committed
wip2
1 parent 645ff85 commit 3346914

File tree

9 files changed

+77
-66
lines changed

9 files changed

+77
-66
lines changed

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

Lines changed: 10 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,8 @@ final class DataFlowCallable extends TDataFlowCallable {
5757
}
5858

5959
final class DataFlowCall extends TDataFlowCall {
60-
/** Gets the underlying call in the CFG, if any. */
61-
Call asCall() { this = TCall(result) }
60+
/** Gets the underlying function call, if any. */
61+
FunctionCall asFunctionCall() { this = TFunctionCall(result) }
6262

6363
predicate isSummaryCall(
6464
FlowSummaryImpl::Public::SummarizedCallable c, FlowSummaryImpl::Private::SummaryNode receiver
@@ -67,13 +67,13 @@ final class DataFlowCall extends TDataFlowCall {
6767
}
6868

6969
DataFlowCallable getEnclosingCallable() {
70-
result.asCfgScope() = this.asCall().getEnclosingCfgScope()
70+
result.asCfgScope() = this.asFunctionCall().getEnclosingCfgScope()
7171
or
7272
this.isSummaryCall(result.asSummarizedCallable(), _)
7373
}
7474

7575
string toString() {
76-
result = this.asCall().toString()
76+
result = this.asFunctionCall().toString()
7777
or
7878
exists(
7979
FlowSummaryImpl::Public::SummarizedCallable c, FlowSummaryImpl::Private::SummaryNode receiver
@@ -83,7 +83,7 @@ final class DataFlowCall extends TDataFlowCall {
8383
)
8484
}
8585

86-
Location getLocation() { result = this.asCall().getLocation() }
86+
Location getLocation() { result = this.asFunctionCall().getLocation() }
8787
}
8888

8989
/**
@@ -141,13 +141,8 @@ final class ArgumentPosition extends ParameterPosition {
141141

142142
/**
143143
* Holds if `arg` is an argument of `call` at the position `pos`.
144-
*
145-
* Note that this does not hold for the receiever expression of a method call
146-
* as the synthetic `ReceiverNode` is the argument for the `self` parameter.
147144
*/
148-
predicate isArgumentForCall(Expr arg, Call call, ArgumentPosition pos) {
149-
// TODO: Handle index expressions as calls in data flow.
150-
not call instanceof IndexExpr and
145+
predicate isArgumentForCall(Expr arg, FunctionCall call, ArgumentPosition pos) {
151146
arg = pos.getArgument(call)
152147
}
153148

@@ -408,7 +403,7 @@ module RustDataFlow implements InputSig<Location> {
408403

409404
/** Gets a viable implementation of the target of the given `Call`. */
410405
DataFlowCallable viableCallable(DataFlowCall call) {
411-
exists(Call c | c = call.asCall() |
406+
exists(FunctionCall c | c = call.asFunctionCall() |
412407
result.asCfgScope() = c.getARuntimeTarget()
413408
or
414409
exists(SummarizedCallable sc, Function staticTarget |
@@ -831,11 +826,7 @@ module RustDataFlow implements InputSig<Location> {
831826
*/
832827
predicate lambdaCall(DataFlowCall call, LambdaCallKind kind, Node receiver) {
833828
(
834-
receiver.asExpr() = call.asCall().(CallExpr).getFunction() and
835-
// All calls to complex expressions and local variable accesses are lambda call.
836-
exists(Expr f | f = receiver.asExpr() |
837-
f instanceof PathExpr implies f = any(Variable v).getAnAccess()
838-
)
829+
receiver.asExpr() = call.asFunctionCall().(ClosureCall).getFunction()
839830
or
840831
call.isSummaryCall(_, receiver.(FlowSummaryNode).getSummaryNode())
841832
) and
@@ -997,11 +988,9 @@ private module Cached {
997988

998989
cached
999990
newtype TDataFlowCall =
1000-
TCall(Call call) {
991+
TFunctionCall(FunctionCall call) {
1001992
Stages::DataFlowStage::ref() and
1002-
call.hasEnclosingCfgScope() and
1003-
// TODO: Handle index expressions as calls in data flow.
1004-
not call instanceof IndexExpr
993+
call.hasEnclosingCfgScope()
1005994
} or
1006995
TSummaryCall(
1007996
FlowSummaryImpl::Public::SummarizedCallable c, FlowSummaryImpl::Private::SummaryNode receiver

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ module Input implements InputSig<Location, RustDataFlow> {
2222

2323
/** Holds if the associated call resolves to `path`. */
2424
final predicate callResolvesTo(string path) {
25-
path = this.getCall().getStaticTarget().(Addressable).getCanonicalPath()
25+
path = this.getCall().getStaticTarget().getCanonicalPath()
2626
}
2727
}
2828

@@ -128,7 +128,9 @@ module Input implements InputSig<Location, RustDataFlow> {
128128
private import Make<Location, RustDataFlow, Input> as Impl
129129

130130
private module StepsInput implements Impl::Private::StepsInputSig {
131-
DataFlowCall getACall(Public::SummarizedCallable sc) { result.asCall().getStaticTarget() = sc }
131+
DataFlowCall getACall(Public::SummarizedCallable sc) {
132+
result.asFunctionCall().getStaticTarget() = sc
133+
}
132134

133135
/** Gets the argument of `source` described by `sc`, if any. */
134136
private Expr getSourceNodeArgument(Input::SourceBase source, Impl::Private::SummaryComponent sc) {

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

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

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

230230
ExprArgumentNode() {
@@ -234,7 +234,7 @@ final class ExprArgumentNode extends ArgumentNode, ExprNode {
234234
}
235235

236236
override predicate isArgumentOf(DataFlowCall call, RustDataFlow::ArgumentPosition pos) {
237-
call.asCall() = call_ and pos = pos_
237+
call.asFunctionCall() = call_ and pos = pos_
238238
}
239239
}
240240

@@ -268,7 +268,7 @@ final class DerefBorrowArgNode extends DerefBorrowNode, ArgumentNode {
268268
private DataFlowCall call_;
269269
private RustDataFlow::ArgumentPosition pos_;
270270

271-
DerefBorrowArgNode() { isArgumentForCall(n, call_.asCall(), pos_) }
271+
DerefBorrowArgNode() { isArgumentForCall(n, call_.asFunctionCall(), pos_) }
272272

273273
override predicate isArgumentOf(DataFlowCall call, RustDataFlow::ArgumentPosition pos) {
274274
call = call_ and pos = pos_
@@ -298,7 +298,7 @@ final class ClosureArgumentNode extends ArgumentNode, ExprNode {
298298
ClosureArgumentNode() { lambdaCallExpr(call_, _, this.asExpr()) }
299299

300300
override predicate isArgumentOf(DataFlowCall call, RustDataFlow::ArgumentPosition pos) {
301-
call.asCall() = call_ and pos.isClosureSelf()
301+
call.asFunctionCall() = call_ and pos.isClosureSelf()
302302
}
303303
}
304304

@@ -346,11 +346,11 @@ abstract class OutNode extends Node {
346346
}
347347

348348
final private class ExprOutNode extends ExprNode, OutNode {
349-
ExprOutNode() { this.asExpr() instanceof Call }
349+
ExprOutNode() { this.asExpr() instanceof FunctionCall }
350350

351351
/** Gets the underlying call CFG node that includes this out node. */
352352
override DataFlowCall getCall(ReturnKind kind) {
353-
result.asCall() = n and
353+
result.asFunctionCall() = n and
354354
kind = TNormalReturnKind()
355355
}
356356
}
@@ -482,10 +482,8 @@ newtype TNode =
482482
or
483483
e =
484484
[
485-
any(IndexExpr i).getBase(), // todo: remove once `IndexExpr`s are treated as calls in data flow
486485
any(FieldExpr access).getContainer(), //
487486
any(TryExpr try).getExpr(), //
488-
any(PrefixExpr pe | pe.getOperatorName() = "*").getExpr(), //
489487
any(AwaitExpr a).getExpr(), //
490488
getPostUpdateReverseStep(any(PostUpdateNode n).getPreUpdateNode().asExpr(), _)
491489
]

‎rust/ql/lib/codeql/rust/elements/Call.qll‎

Lines changed: 27 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -7,22 +7,35 @@ private import internal.CallImpl
77

88
final class Call = Impl::Call;
99

10+
private predicate isClosureCallExpr(CallExpr call) {
11+
exists(Expr receiver | receiver = call.getFunction() |
12+
// All calls to complex expressions and local variable accesses are lambda calls
13+
receiver instanceof PathExpr implies receiver = any(Variable v).getAnAccess()
14+
)
15+
}
16+
17+
private predicate isGuaranteedMethodCall(Call call) {
18+
// even if we cannot resolve the target, we know that the following calls always
19+
// target functions
20+
call instanceof MethodCallExpr
21+
or
22+
call.(Operation).isOverloaded(_, _, _)
23+
or
24+
call instanceof IndexExpr
25+
}
26+
1027
/**
11-
* A call expression that targets a function.
28+
* A call expression that targets a function or a closure.
1229
*
1330
* For example, call expressions like `Some(42)` and `x && y` are _not_ function calls.
1431
*/
1532
final class FunctionCall extends Call {
1633
FunctionCall() {
1734
this.getStaticTarget() instanceof Function
1835
or
19-
// even if we cannot resolve the target, we know that the following calls always
20-
// target functions
21-
this instanceof MethodCallExpr
22-
or
23-
this.(Operation).isOverloaded(_, _, _)
36+
isClosureCallExpr(this)
2437
or
25-
this instanceof IndexExpr
38+
isGuaranteedMethodCall(this)
2639
}
2740

2841
/** Gets the static target of this function call, if any. */
@@ -34,28 +47,20 @@ final class FunctionCall extends Call {
3447
*/
3548
final class MethodCall extends FunctionCall {
3649
MethodCall() {
37-
this.getStaticTarget().(Function).hasSelfParam()
50+
this.getStaticTarget() instanceof Method
3851
or
39-
// even if we cannot resolve the target, we know that the following calls always
40-
// target methods
41-
this instanceof MethodCallExpr
42-
or
43-
this.(Operation).isOverloaded(_, _, _)
44-
or
45-
this instanceof IndexExpr
52+
isGuaranteedMethodCall(this)
4653
}
54+
55+
/** Gets the static target of this method call, if any. */
56+
Method getStaticTarget() { result = super.getStaticTarget() }
4757
}
4858

4959
/**
5060
* A call expression that targets a closure.
5161
*
5262
* For closure calls, the static target never exists.
5363
*/
54-
final class ClosureCall extends CallExpr {
55-
ClosureCall() {
56-
exists(Expr receiver | receiver = this.getFunction() |
57-
// All calls to complex expressions and local variable accesses are lambda calls
58-
receiver instanceof PathExpr implies receiver = any(Variable v).getAnAccess()
59-
)
60-
}
64+
final class ClosureCall extends FunctionCall, CallExpr {
65+
ClosureCall() { isClosureCallExpr(this) }
6166
}

‎rust/ql/lib/codeql/rust/elements/Method.qll‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
/**
2+
* This module provides the public class `Method`.
3+
*/
4+
5+
private import rust
6+
7+
/**
8+
* A method declaration. For example
9+
* ```rust
10+
* fn foo(self, x: u32) -> u64 { (x + 1).into() }
11+
* ```
12+
*
13+
* A method declaration within a trait might not have a body:
14+
* ```rust
15+
* trait Trait {
16+
* fn bar(self);
17+
* }
18+
* ```
19+
*/
20+
final class Method extends Function {
21+
Method() { this.hasSelfParam() }
22+
}

‎rust/ql/lib/codeql/rust/internal/TypeInference.qll‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -230,11 +230,6 @@ module Consistency {
230230
}
231231
}
232232

233-
/** A method, that is, a function with a `self` parameter. */
234-
private class Method extends Function {
235-
Method() { this.hasSelfParam() }
236-
}
237-
238233
/** A function without a `self` parameter. */
239234
private class NonMethodFunction extends Function {
240235
NonMethodFunction() { not this.hasSelfParam() }

‎rust/ql/lib/rust.qll‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,3 +19,4 @@ import codeql.rust.elements.NamedFormatArgument
1919
import codeql.rust.elements.PositionalFormatArgument
2020
import codeql.rust.elements.RangeExprExt
2121
import codeql.rust.elements.Call
22+
import codeql.rust.elements.Method

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@ localStep
3838
| main.rs:30:12:30:12 | x | main.rs:30:12:30:12 | [SSA] x |
3939
| main.rs:30:12:30:12 | x | main.rs:30:12:30:12 | x |
4040
| main.rs:30:16:30:16 | s | main.rs:30:12:30:12 | x |
41-
| main.rs:31:12:34:9 | [post] { ... } | main.rs:33:13:33:16 | [post] true |
4241
| main.rs:32:18:32:18 | [post] x | main.rs:36:14:36:14 | x |
4342
| main.rs:32:18:32:18 | x | main.rs:36:14:36:14 | x |
4443
| main.rs:33:13:33:16 | true | main.rs:31:12:34:9 | { ... } |
@@ -111,7 +110,6 @@ localStep
111110
| main.rs:76:13:76:21 | source(...) | main.rs:76:9:76:9 | k |
112111
| main.rs:77:5:77:5 | [SSA] j | main.rs:78:10:78:10 | j |
113112
| main.rs:77:5:77:5 | j | main.rs:77:5:77:5 | [SSA] j |
114-
| main.rs:77:9:77:9 | [post] k | main.rs:79:10:79:10 | k |
115113
| main.rs:77:9:77:9 | k | main.rs:77:5:77:5 | j |
116114
| main.rs:77:9:77:9 | k | main.rs:79:10:79:10 | k |
117115
| main.rs:81:9:81:13 | mut l | main.rs:81:13:81:13 | l |
@@ -332,7 +330,6 @@ localStep
332330
| main.rs:267:17:267:17 | n | main.rs:267:17:267:17 | [SSA] n |
333331
| main.rs:267:17:267:17 | n | main.rs:267:17:267:17 | n |
334332
| main.rs:267:22:267:23 | s1 | main.rs:267:12:267:18 | Some(...) |
335-
| main.rs:268:12:271:9 | [post] { ... } | main.rs:270:13:270:16 | [post] true |
336333
| main.rs:269:18:269:18 | [post] n | main.rs:273:14:273:14 | n |
337334
| main.rs:269:18:269:18 | n | main.rs:273:14:273:14 | n |
338335
| main.rs:270:13:270:16 | true | main.rs:268:12:271:9 | { ... } |
@@ -950,7 +947,6 @@ readStep
950947
| main.rs:546:11:546:39 | [post] ... .unwrap() [borrowed] | file://:0:0:0:0 | &ref | main.rs:546:11:546:39 | [post] ... .unwrap() |
951948
| main.rs:548:9:548:14 | &mut ... | file://:0:0:0:0 | &ref | main.rs:548:14:548:14 | v |
952949
| main.rs:548:19:548:35 | vs_mut.iter_mut() | file://:0:0:0:0 | element | main.rs:548:9:548:14 | &mut ... |
953-
| main.rs:560:14:560:15 | [post] &b | file://:0:0:0:0 | &ref | main.rs:560:15:560:15 | [post] b |
954950
| main.rs:562:11:562:15 | [post] c_ref [borrowed] | file://:0:0:0:0 | &ref | main.rs:562:11:562:15 | [post] c_ref |
955951
| main.rs:562:11:562:15 | c_ref | file://:0:0:0:0 | &ref | main.rs:562:10:562:15 | * ... |
956952
storeStep

‎rust/ql/test/library-tests/variables/Ssa.expected‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,7 @@ definition
152152
| main.rs:577:13:577:13 | y | main.rs:577:13:577:13 | y |
153153
| main.rs:580:13:580:20 | closure2 | main.rs:580:13:580:20 | closure2 |
154154
| main.rs:581:9:581:9 | y | main.rs:577:13:577:13 | y |
155+
| main.rs:583:5:583:14 | <captured exit> y | main.rs:577:13:577:13 | y |
155156
| main.rs:586:13:586:13 | z | main.rs:586:13:586:13 | z |
156157
| main.rs:589:13:589:20 | closure3 | main.rs:589:13:589:20 | closure3 |
157158
| main.rs:589:24:591:5 | <captured entry> z | main.rs:586:13:586:13 | z |
@@ -169,6 +170,7 @@ definition
169170
| main.rs:625:9:625:9 | x | main.rs:625:9:625:9 | x |
170171
| main.rs:648:20:648:23 | self | main.rs:648:20:648:23 | self |
171172
| main.rs:652:11:652:14 | self | main.rs:652:11:652:14 | self |
173+
| main.rs:656:23:656:26 | self | main.rs:656:23:656:26 | self |
172174
| main.rs:657:17:657:17 | f | main.rs:657:17:657:17 | f |
173175
| main.rs:657:21:660:9 | <captured entry> self | main.rs:656:23:656:26 | self |
174176
| main.rs:657:22:657:22 | n | main.rs:657:22:657:22 | n |
@@ -193,6 +195,7 @@ definition
193195
| main.rs:746:20:746:20 | b | main.rs:746:20:746:20 | b |
194196
| main.rs:748:17:750:9 | SSA phi(x) | main.rs:745:13:745:13 | x |
195197
| main.rs:749:13:749:13 | x | main.rs:745:13:745:13 | x |
198+
| main.rs:752:5:752:13 | <captured exit> x | main.rs:745:13:745:13 | x |
196199
read
197200
| main.rs:5:14:5:14 | s | main.rs:5:14:5:14 | s | main.rs:7:20:7:20 | s |
198201
| main.rs:10:14:10:14 | i | main.rs:10:14:10:14 | i | main.rs:12:20:12:20 | i |
@@ -352,8 +355,8 @@ read
352355
| main.rs:568:13:568:13 | x | main.rs:568:13:568:13 | x | main.rs:575:15:575:15 | x |
353356
| main.rs:571:9:571:16 | closure1 | main.rs:571:9:571:16 | closure1 | main.rs:574:5:574:12 | closure1 |
354357
| main.rs:571:20:573:5 | <captured entry> x | main.rs:568:13:568:13 | x | main.rs:572:19:572:19 | x |
355-
| main.rs:577:13:577:13 | y | main.rs:577:13:577:13 | y | main.rs:584:15:584:15 | y |
356358
| main.rs:580:13:580:20 | closure2 | main.rs:580:13:580:20 | closure2 | main.rs:583:5:583:12 | closure2 |
359+
| main.rs:583:5:583:14 | <captured exit> y | main.rs:577:13:577:13 | y | main.rs:584:15:584:15 | y |
357360
| main.rs:586:13:586:13 | z | main.rs:586:13:586:13 | z | main.rs:593:15:593:15 | z |
358361
| main.rs:589:13:589:20 | closure3 | main.rs:589:13:589:20 | closure3 | main.rs:592:5:592:12 | closure3 |
359362
| main.rs:589:24:591:5 | <captured entry> z | main.rs:586:13:586:13 | z | main.rs:590:9:590:9 | z |
@@ -398,9 +401,9 @@ read
398401
| main.rs:729:9:729:20 | var_in_macro | main.rs:729:9:729:20 | var_in_macro | main.rs:735:15:735:26 | var_in_macro |
399402
| main.rs:734:15:734:28 | var_in_macro | main.rs:734:15:734:28 | var_in_macro | main.rs:734:30:734:41 | var_in_macro |
400403
| main.rs:740:5:740:5 | x | main.rs:739:9:739:9 | x | main.rs:741:15:741:15 | x |
401-
| main.rs:745:13:745:13 | x | main.rs:745:13:745:13 | x | main.rs:753:15:753:15 | x |
402404
| main.rs:746:13:746:15 | cap | main.rs:746:13:746:15 | cap | main.rs:752:5:752:7 | cap |
403405
| main.rs:746:20:746:20 | b | main.rs:746:20:746:20 | b | main.rs:748:20:748:20 | b |
406+
| main.rs:752:5:752:13 | <captured exit> x | main.rs:745:13:745:13 | x | main.rs:753:15:753:15 | x |
404407
firstRead
405408
| main.rs:5:14:5:14 | s | main.rs:5:14:5:14 | s | main.rs:7:20:7:20 | s |
406409
| main.rs:10:14:10:14 | i | main.rs:10:14:10:14 | i | main.rs:12:20:12:20 | i |
@@ -528,8 +531,8 @@ firstRead
528531
| main.rs:568:13:568:13 | x | main.rs:568:13:568:13 | x | main.rs:575:15:575:15 | x |
529532
| main.rs:571:9:571:16 | closure1 | main.rs:571:9:571:16 | closure1 | main.rs:574:5:574:12 | closure1 |
530533
| main.rs:571:20:573:5 | <captured entry> x | main.rs:568:13:568:13 | x | main.rs:572:19:572:19 | x |
531-
| main.rs:577:13:577:13 | y | main.rs:577:13:577:13 | y | main.rs:584:15:584:15 | y |
532534
| main.rs:580:13:580:20 | closure2 | main.rs:580:13:580:20 | closure2 | main.rs:583:5:583:12 | closure2 |
535+
| main.rs:583:5:583:14 | <captured exit> y | main.rs:577:13:577:13 | y | main.rs:584:15:584:15 | y |
533536
| main.rs:586:13:586:13 | z | main.rs:586:13:586:13 | z | main.rs:593:15:593:15 | z |
534537
| main.rs:589:13:589:20 | closure3 | main.rs:589:13:589:20 | closure3 | main.rs:592:5:592:12 | closure3 |
535538
| main.rs:589:24:591:5 | <captured entry> z | main.rs:586:13:586:13 | z | main.rs:590:9:590:9 | z |
@@ -564,9 +567,9 @@ firstRead
564567
| main.rs:729:9:729:20 | var_in_macro | main.rs:729:9:729:20 | var_in_macro | main.rs:735:15:735:26 | var_in_macro |
565568
| main.rs:734:15:734:28 | var_in_macro | main.rs:734:15:734:28 | var_in_macro | main.rs:734:30:734:41 | var_in_macro |
566569
| main.rs:740:5:740:5 | x | main.rs:739:9:739:9 | x | main.rs:741:15:741:15 | x |
567-
| main.rs:745:13:745:13 | x | main.rs:745:13:745:13 | x | main.rs:753:15:753:15 | x |
568570
| main.rs:746:13:746:15 | cap | main.rs:746:13:746:15 | cap | main.rs:752:5:752:7 | cap |
569571
| main.rs:746:20:746:20 | b | main.rs:746:20:746:20 | b | main.rs:748:20:748:20 | b |
572+
| main.rs:752:5:752:13 | <captured exit> x | main.rs:745:13:745:13 | x | main.rs:753:15:753:15 | x |
570573
adjacentReads
571574
| main.rs:27:5:27:6 | x2 | main.rs:25:13:25:14 | x2 | main.rs:28:15:28:16 | x2 | main.rs:29:10:29:11 | x2 |
572575
| main.rs:41:9:41:10 | x3 | main.rs:41:9:41:10 | x3 | main.rs:42:15:42:16 | x3 | main.rs:44:9:44:10 | x3 |

0 commit comments

Comments
 (0)