Skip to content

Commit 1f2cca7

Browse files
paldepindpaldepind
authored
Merge pull request #20547 from paldepind/rust/function-as-lambda
Rust: Handle functions as data flow lambdas
2 parents 402d58b + 98a20f9 commit 1f2cca7

File tree

8 files changed

+144
-71
lines changed

8 files changed

+144
-71
lines changed

‎rust/ql/lib/change-notes/2025-09-29-data-flow-function-pointer.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Improve data flow through functions being passed as function pointers.

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

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -295,13 +295,10 @@ module LocalFlow {
295295
class LambdaCallKind = Unit;
296296

297297
/** Holds if `creation` is an expression that creates a lambda of kind `kind`. */
298-
predicate lambdaCreationExpr(Expr creation, LambdaCallKind kind) {
299-
(
300-
creation instanceof ClosureExpr
301-
or
302-
creation instanceof Scope::AsyncBlockScope
303-
) and
304-
exists(kind)
298+
predicate lambdaCreationExpr(Expr creation) {
299+
creation instanceof ClosureExpr
300+
or
301+
creation instanceof Scope::AsyncBlockScope
305302
}
306303

307304
/**
@@ -810,8 +807,15 @@ module RustDataFlow implements InputSig<Location> {
810807

811808
/** Holds if `creation` is an expression that creates a lambda of kind `kind` for `c`. */
812809
predicate lambdaCreation(Node creation, LambdaCallKind kind, DataFlowCallable c) {
813-
exists(Expr e |
814-
e = creation.asExpr().getExpr() and lambdaCreationExpr(e, kind) and e = c.asCfgScope()
810+
exists(kind) and
811+
exists(Expr e | e = creation.asExpr().getExpr() |
812+
lambdaCreationExpr(e) and e = c.asCfgScope()
813+
or
814+
// A path expression, that resolves to a function, evaluates to a function
815+
// pointer. Except if the path occurs directly in a call, then it's just a
816+
// call to the function and not a function being passed as data.
817+
resolvePath(e.(PathExpr).getPath()) = c.asCfgScope() and
818+
not any(CallExpr call).getFunction() = e
815819
)
816820
}
817821

@@ -931,7 +935,7 @@ module VariableCapture {
931935
}
932936

933937
class ClosureExpr extends Expr instanceof ExprCfgNode {
934-
ClosureExpr() { lambdaCreationExpr(super.getExpr(), _) }
938+
ClosureExpr() { lambdaCreationExpr(super.getExpr()) }
935939

936940
predicate hasBody(Callable body) { body = super.getExpr() }
937941

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -454,7 +454,7 @@ newtype TNode =
454454
or
455455
lambdaCallExpr(_, _, e)
456456
or
457-
lambdaCreationExpr(e.getExpr(), _)
457+
lambdaCreationExpr(e.getExpr())
458458
or
459459
// Whenever `&mut e` has a post-update node we also create one for `e`.
460460
// E.g., for `e` in `f(..., &mut e, ...)` or `*(&mut e) = ...`.
@@ -478,5 +478,5 @@ newtype TNode =
478478
} or
479479
TSsaNode(SsaImpl::DataFlowIntegration::SsaNode node) or
480480
TFlowSummaryNode(FlowSummaryImpl::Private::SummaryNode sn) or
481-
TClosureSelfReferenceNode(CfgScope c) { lambdaCreationExpr(c, _) } or
481+
TClosureSelfReferenceNode(CfgScope c) { lambdaCreationExpr(c) } or
482482
TCaptureNode(VariableCapture::Flow::SynthesizedCaptureNode cn)

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

Lines changed: 0 additions & 50 deletions
This file was deleted.

‎rust/ql/test/library-tests/dataflow/closures/Cargo.lock‎ renamed to ‎rust/ql/test/library-tests/dataflow/lambdas/Cargo.lock‎

File renamed without changes.

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

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
models
2+
edges
3+
| main.rs:10:20:10:52 | if cond {...} else {...} | main.rs:11:10:11:16 | f(...) | provenance | |
4+
| main.rs:10:30:10:39 | source(...) | main.rs:10:20:10:52 | if cond {...} else {...} | provenance | |
5+
| main.rs:15:20:15:23 | ... | main.rs:17:18:17:21 | data | provenance | |
6+
| main.rs:22:9:22:9 | a | main.rs:23:13:23:13 | a | provenance | |
7+
| main.rs:22:13:22:22 | source(...) | main.rs:22:9:22:9 | a | provenance | |
8+
| main.rs:23:13:23:13 | a | main.rs:15:20:15:23 | ... | provenance | |
9+
| main.rs:27:20:27:23 | ... | main.rs:27:26:27:52 | if cond {...} else {...} | provenance | |
10+
| main.rs:28:9:28:9 | a | main.rs:29:21:29:21 | a | provenance | |
11+
| main.rs:28:13:28:22 | source(...) | main.rs:28:9:28:9 | a | provenance | |
12+
| main.rs:29:9:29:9 | b | main.rs:30:10:30:10 | b | provenance | |
13+
| main.rs:29:13:29:22 | f(...) | main.rs:29:9:29:9 | b | provenance | |
14+
| main.rs:29:21:29:21 | a | main.rs:27:20:27:23 | ... | provenance | |
15+
| main.rs:29:21:29:21 | a | main.rs:29:13:29:22 | f(...) | provenance | |
16+
| main.rs:37:16:37:25 | source(...) | main.rs:39:5:39:5 | [post] f [captured capt] | provenance | |
17+
| main.rs:39:5:39:5 | [post] f [captured capt] | main.rs:40:10:40:13 | capt | provenance | |
18+
| main.rs:39:5:39:5 | [post] f [captured capt] | main.rs:44:5:44:5 | g [captured capt] | provenance | |
19+
| main.rs:44:5:44:5 | g [captured capt] | main.rs:42:14:42:17 | capt | provenance | |
20+
| main.rs:47:29:49:1 | { ... } | main.rs:57:10:57:12 | f(...) | provenance | |
21+
| main.rs:48:5:48:14 | source(...) | main.rs:47:29:49:1 | { ... } | provenance | |
22+
| main.rs:51:17:51:25 | ...: i64 | main.rs:52:10:52:13 | data | provenance | |
23+
| main.rs:62:9:62:9 | a | main.rs:63:7:63:7 | a | provenance | |
24+
| main.rs:62:13:62:22 | source(...) | main.rs:62:9:62:9 | a | provenance | |
25+
| main.rs:63:7:63:7 | a | main.rs:51:17:51:25 | ...: i64 | provenance | |
26+
| main.rs:66:24:66:32 | ...: i64 | main.rs:66:42:72:1 | { ... } | provenance | |
27+
| main.rs:76:9:76:9 | a | main.rs:77:21:77:21 | a | provenance | |
28+
| main.rs:76:13:76:22 | source(...) | main.rs:76:9:76:9 | a | provenance | |
29+
| main.rs:77:9:77:9 | b | main.rs:78:10:78:10 | b | provenance | |
30+
| main.rs:77:13:77:22 | f(...) | main.rs:77:9:77:9 | b | provenance | |
31+
| main.rs:77:21:77:21 | a | main.rs:66:24:66:32 | ...: i64 | provenance | |
32+
| main.rs:77:21:77:21 | a | main.rs:77:13:77:22 | f(...) | provenance | |
33+
nodes
34+
| main.rs:10:20:10:52 | if cond {...} else {...} | semmle.label | if cond {...} else {...} |
35+
| main.rs:10:30:10:39 | source(...) | semmle.label | source(...) |
36+
| main.rs:11:10:11:16 | f(...) | semmle.label | f(...) |
37+
| main.rs:15:20:15:23 | ... | semmle.label | ... |
38+
| main.rs:17:18:17:21 | data | semmle.label | data |
39+
| main.rs:22:9:22:9 | a | semmle.label | a |
40+
| main.rs:22:13:22:22 | source(...) | semmle.label | source(...) |
41+
| main.rs:23:13:23:13 | a | semmle.label | a |
42+
| main.rs:27:20:27:23 | ... | semmle.label | ... |
43+
| main.rs:27:26:27:52 | if cond {...} else {...} | semmle.label | if cond {...} else {...} |
44+
| main.rs:28:9:28:9 | a | semmle.label | a |
45+
| main.rs:28:13:28:22 | source(...) | semmle.label | source(...) |
46+
| main.rs:29:9:29:9 | b | semmle.label | b |
47+
| main.rs:29:13:29:22 | f(...) | semmle.label | f(...) |
48+
| main.rs:29:21:29:21 | a | semmle.label | a |
49+
| main.rs:30:10:30:10 | b | semmle.label | b |
50+
| main.rs:37:16:37:25 | source(...) | semmle.label | source(...) |
51+
| main.rs:39:5:39:5 | [post] f [captured capt] | semmle.label | [post] f [captured capt] |
52+
| main.rs:40:10:40:13 | capt | semmle.label | capt |
53+
| main.rs:42:14:42:17 | capt | semmle.label | capt |
54+
| main.rs:44:5:44:5 | g [captured capt] | semmle.label | g [captured capt] |
55+
| main.rs:47:29:49:1 | { ... } | semmle.label | { ... } |
56+
| main.rs:48:5:48:14 | source(...) | semmle.label | source(...) |
57+
| main.rs:51:17:51:25 | ...: i64 | semmle.label | ...: i64 |
58+
| main.rs:52:10:52:13 | data | semmle.label | data |
59+
| main.rs:57:10:57:12 | f(...) | semmle.label | f(...) |
60+
| main.rs:62:9:62:9 | a | semmle.label | a |
61+
| main.rs:62:13:62:22 | source(...) | semmle.label | source(...) |
62+
| main.rs:63:7:63:7 | a | semmle.label | a |
63+
| main.rs:66:24:66:32 | ...: i64 | semmle.label | ...: i64 |
64+
| main.rs:66:42:72:1 | { ... } | semmle.label | { ... } |
65+
| main.rs:76:9:76:9 | a | semmle.label | a |
66+
| main.rs:76:13:76:22 | source(...) | semmle.label | source(...) |
67+
| main.rs:77:9:77:9 | b | semmle.label | b |
68+
| main.rs:77:13:77:22 | f(...) | semmle.label | f(...) |
69+
| main.rs:77:21:77:21 | a | semmle.label | a |
70+
| main.rs:78:10:78:10 | b | semmle.label | b |
71+
subpaths
72+
| main.rs:29:21:29:21 | a | main.rs:27:20:27:23 | ... | main.rs:27:26:27:52 | if cond {...} else {...} | main.rs:29:13:29:22 | f(...) |
73+
| main.rs:77:21:77:21 | a | main.rs:66:24:66:32 | ...: i64 | main.rs:66:42:72:1 | { ... } | main.rs:77:13:77:22 | f(...) |
74+
testFailures
75+
#select
76+
| main.rs:11:10:11:16 | f(...) | main.rs:10:30:10:39 | source(...) | main.rs:11:10:11:16 | f(...) | $@ | main.rs:10:30:10:39 | source(...) | source(...) |
77+
| main.rs:17:18:17:21 | data | main.rs:22:13:22:22 | source(...) | main.rs:17:18:17:21 | data | $@ | main.rs:22:13:22:22 | source(...) | source(...) |
78+
| main.rs:30:10:30:10 | b | main.rs:28:13:28:22 | source(...) | main.rs:30:10:30:10 | b | $@ | main.rs:28:13:28:22 | source(...) | source(...) |
79+
| main.rs:40:10:40:13 | capt | main.rs:37:16:37:25 | source(...) | main.rs:40:10:40:13 | capt | $@ | main.rs:37:16:37:25 | source(...) | source(...) |
80+
| main.rs:42:14:42:17 | capt | main.rs:37:16:37:25 | source(...) | main.rs:42:14:42:17 | capt | $@ | main.rs:37:16:37:25 | source(...) | source(...) |
81+
| main.rs:52:10:52:13 | data | main.rs:62:13:62:22 | source(...) | main.rs:52:10:52:13 | data | $@ | main.rs:62:13:62:22 | source(...) | source(...) |
82+
| main.rs:57:10:57:12 | f(...) | main.rs:48:5:48:14 | source(...) | main.rs:57:10:57:12 | f(...) | $@ | main.rs:48:5:48:14 | source(...) | source(...) |
83+
| main.rs:78:10:78:10 | b | main.rs:76:13:76:22 | source(...) | main.rs:78:10:78:10 | b | $@ | main.rs:76:13:76:22 | source(...) | source(...) |

‎rust/ql/test/library-tests/dataflow/closures/inline-flow.ql‎ renamed to ‎rust/ql/test/library-tests/dataflow/lambdas/inline-flow.ql‎

File renamed without changes.

‎rust/ql/test/library-tests/dataflow/closures/main.rs‎ renamed to ‎rust/ql/test/library-tests/dataflow/lambdas/main.rs‎

Lines changed: 41 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,30 +6,25 @@ fn sink(s: i64) {
66
println!("{}", s);
77
}
88

9-
109
fn closure_flow_out() {
1110
let f = |cond| if cond { source(92) } else { 0 };
1211
sink(f(true)); // $ hasValueFlow=92
1312
}
1413

1514
fn closure_flow_in() {
16-
let f = |cond, data|
15+
let f = |cond, data| {
1716
if cond {
1817
sink(data); // $ hasValueFlow=87
1918
} else {
2019
sink(0)
21-
};
20+
}
21+
};
2222
let a = source(87);
2323
f(true, a);
2424
}
2525

2626
fn closure_flow_through() {
27-
let f = |cond, data|
28-
if cond {
29-
data
30-
} else {
31-
0
32-
};
27+
let f = |cond, data| if cond { data } else { 0 };
3328
let a = source(43);
3429
let b = f(true, a);
3530
sink(b); // $ hasValueFlow=43
@@ -49,9 +44,46 @@ fn closure_captured_variable() {
4944
g();
5045
}
5146

47+
fn get_from_source() -> i64 {
48+
source(93)
49+
}
50+
51+
fn pass_to_sink(data: i64) {
52+
sink(data); // $ hasValueFlow=34
53+
}
54+
55+
fn function_flow_out() {
56+
let f = get_from_source;
57+
sink(f()); // $ hasValueFlow=93
58+
}
59+
60+
fn function_flow_in() {
61+
let f = pass_to_sink;
62+
let a = source(34);
63+
f(a);
64+
}
65+
66+
fn get_arg(cond: bool, data: i64) -> i64 {
67+
if cond {
68+
data
69+
} else {
70+
0
71+
}
72+
}
73+
74+
fn function_flows_through() {
75+
let f = get_arg;
76+
let a = source(56);
77+
let b = f(true, a);
78+
sink(b); // $ hasValueFlow=56
79+
}
80+
5281
fn main() {
5382
closure_flow_out();
5483
closure_flow_in();
5584
closure_flow_through();
5685
closure_captured_variable();
86+
function_flow_in();
87+
function_flow_out();
88+
function_flows_through();
5789
}

0 commit comments

Comments
 (0)