Skip to content

Commit 40f80ff

Browse files
hvitvedhvitved
authored
Merge pull request #19442 from hvitved/rust/clone-modeling
Rust: Strengthen modeling of the `Clone` trait
2 parents 8ad6938 + 423e2da commit 40f80ff

File tree

6 files changed

+9
-33
lines changed

6 files changed

+9
-33
lines changed

‎rust/ql/lib/codeql/rust/frameworks/stdlib/Clone.qll

+1-3
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,7 @@ final class CloneCallable extends SummarizedCallable::Range {
1818
final override predicate propagatesFlow(
1919
string input, string output, boolean preservesValue, string model
2020
) {
21-
// The `clone` method takes a `&self` parameter and dereferences it;
22-
// make sure to not clone the reference itself
23-
input = ["Argument[self].Reference", "Argument[self].WithoutReference"] and
21+
input = "Argument[self].Reference" and
2422
output = "ReturnValue" and
2523
preservesValue = true and
2624
model = "generated"

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -1041,8 +1041,8 @@ private module Debug {
10411041
private Locatable getRelevantLocatable() {
10421042
exists(string filepath, int startline, int startcolumn, int endline, int endcolumn |
10431043
result.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn) and
1044-
filepath.matches("%/tauri/src/app/plugin.rs") and
1045-
startline = 54
1044+
filepath.matches("%/main.rs") and
1045+
startline = 28
10461046
)
10471047
}
10481048

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

-22
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ edges
1010
| main.rs:12:9:12:9 | a [Some] | main.rs:13:10:13:19 | a.unwrap() | provenance | MaD:2 |
1111
| main.rs:12:9:12:9 | a [Some] | main.rs:14:13:14:13 | a [Some] | provenance | |
1212
| main.rs:12:9:12:9 | a [Some] | main.rs:14:13:14:21 | a.clone() [Some] | provenance | MaD:1 |
13-
| main.rs:12:9:12:9 | a [Some] | main.rs:14:13:14:21 | a.clone() [Some] | provenance | generated |
1413
| main.rs:12:13:12:28 | Some(...) [Some] | main.rs:12:9:12:9 | a [Some] | provenance | |
1514
| main.rs:12:18:12:27 | source(...) | main.rs:12:13:12:28 | Some(...) [Some] | provenance | |
1615
| main.rs:14:9:14:9 | b [Some] | main.rs:15:10:15:19 | b.unwrap() | provenance | MaD:2 |
@@ -19,29 +18,19 @@ edges
1918
| main.rs:19:9:19:9 | a [Ok] | main.rs:20:10:20:19 | a.unwrap() | provenance | MaD:5 |
2019
| main.rs:19:9:19:9 | a [Ok] | main.rs:21:13:21:13 | a [Ok] | provenance | |
2120
| main.rs:19:9:19:9 | a [Ok] | main.rs:21:13:21:21 | a.clone() [Ok] | provenance | MaD:4 |
22-
| main.rs:19:9:19:9 | a [Ok] | main.rs:21:13:21:21 | a.clone() [Ok] | provenance | generated |
2321
| main.rs:19:31:19:44 | Ok(...) [Ok] | main.rs:19:9:19:9 | a [Ok] | provenance | |
2422
| main.rs:19:34:19:43 | source(...) | main.rs:19:31:19:44 | Ok(...) [Ok] | provenance | |
2523
| main.rs:21:9:21:9 | b [Ok] | main.rs:22:10:22:19 | b.unwrap() | provenance | MaD:5 |
2624
| main.rs:21:13:21:13 | a [Ok] | main.rs:21:13:21:21 | a.clone() [Ok] | provenance | generated |
2725
| main.rs:21:13:21:21 | a.clone() [Ok] | main.rs:21:9:21:9 | b [Ok] | provenance | |
2826
| main.rs:26:9:26:9 | a | main.rs:27:10:27:10 | a | provenance | |
29-
| main.rs:26:9:26:9 | a | main.rs:28:13:28:21 | a.clone() | provenance | generated |
3027
| main.rs:26:13:26:22 | source(...) | main.rs:26:9:26:9 | a | provenance | |
31-
| main.rs:28:9:28:9 | b | main.rs:29:10:29:10 | b | provenance | |
32-
| main.rs:28:13:28:21 | a.clone() | main.rs:28:9:28:9 | b | provenance | |
3328
| main.rs:41:13:41:13 | w [Wrapper] | main.rs:42:15:42:15 | w [Wrapper] | provenance | |
3429
| main.rs:41:17:41:41 | Wrapper {...} [Wrapper] | main.rs:41:13:41:13 | w [Wrapper] | provenance | |
3530
| main.rs:41:30:41:39 | source(...) | main.rs:41:17:41:41 | Wrapper {...} [Wrapper] | provenance | |
3631
| main.rs:42:15:42:15 | w [Wrapper] | main.rs:43:13:43:28 | Wrapper {...} [Wrapper] | provenance | |
37-
| main.rs:42:15:42:15 | w [Wrapper] | main.rs:45:17:45:25 | w.clone() [Wrapper] | provenance | generated |
3832
| main.rs:43:13:43:28 | Wrapper {...} [Wrapper] | main.rs:43:26:43:26 | n | provenance | |
3933
| main.rs:43:26:43:26 | n | main.rs:43:38:43:38 | n | provenance | |
40-
| main.rs:45:13:45:13 | u [Wrapper] | main.rs:46:15:46:15 | u [Wrapper] | provenance | |
41-
| main.rs:45:17:45:25 | w.clone() [Wrapper] | main.rs:45:13:45:13 | u [Wrapper] | provenance | |
42-
| main.rs:46:15:46:15 | u [Wrapper] | main.rs:47:13:47:28 | Wrapper {...} [Wrapper] | provenance | |
43-
| main.rs:47:13:47:28 | Wrapper {...} [Wrapper] | main.rs:47:26:47:26 | n | provenance | |
44-
| main.rs:47:26:47:26 | n | main.rs:47:38:47:38 | n | provenance | |
4534
| main.rs:58:13:58:13 | b [Some] | main.rs:59:23:59:23 | b [Some] | provenance | |
4635
| main.rs:58:17:58:32 | Some(...) [Some] | main.rs:58:13:58:13 | b [Some] | provenance | |
4736
| main.rs:58:22:58:31 | source(...) | main.rs:58:17:58:32 | Some(...) [Some] | provenance | |
@@ -75,22 +64,13 @@ nodes
7564
| main.rs:26:9:26:9 | a | semmle.label | a |
7665
| main.rs:26:13:26:22 | source(...) | semmle.label | source(...) |
7766
| main.rs:27:10:27:10 | a | semmle.label | a |
78-
| main.rs:28:9:28:9 | b | semmle.label | b |
79-
| main.rs:28:13:28:21 | a.clone() | semmle.label | a.clone() |
80-
| main.rs:29:10:29:10 | b | semmle.label | b |
8167
| main.rs:41:13:41:13 | w [Wrapper] | semmle.label | w [Wrapper] |
8268
| main.rs:41:17:41:41 | Wrapper {...} [Wrapper] | semmle.label | Wrapper {...} [Wrapper] |
8369
| main.rs:41:30:41:39 | source(...) | semmle.label | source(...) |
8470
| main.rs:42:15:42:15 | w [Wrapper] | semmle.label | w [Wrapper] |
8571
| main.rs:43:13:43:28 | Wrapper {...} [Wrapper] | semmle.label | Wrapper {...} [Wrapper] |
8672
| main.rs:43:26:43:26 | n | semmle.label | n |
8773
| main.rs:43:38:43:38 | n | semmle.label | n |
88-
| main.rs:45:13:45:13 | u [Wrapper] | semmle.label | u [Wrapper] |
89-
| main.rs:45:17:45:25 | w.clone() [Wrapper] | semmle.label | w.clone() [Wrapper] |
90-
| main.rs:46:15:46:15 | u [Wrapper] | semmle.label | u [Wrapper] |
91-
| main.rs:47:13:47:28 | Wrapper {...} [Wrapper] | semmle.label | Wrapper {...} [Wrapper] |
92-
| main.rs:47:26:47:26 | n | semmle.label | n |
93-
| main.rs:47:38:47:38 | n | semmle.label | n |
9474
| main.rs:58:13:58:13 | b [Some] | semmle.label | b [Some] |
9575
| main.rs:58:17:58:32 | Some(...) [Some] | semmle.label | Some(...) [Some] |
9676
| main.rs:58:22:58:31 | source(...) | semmle.label | source(...) |
@@ -114,8 +94,6 @@ testFailures
11494
| main.rs:20:10:20:19 | a.unwrap() | main.rs:19:34:19:43 | source(...) | main.rs:20:10:20:19 | a.unwrap() | $@ | main.rs:19:34:19:43 | source(...) | source(...) |
11595
| main.rs:22:10:22:19 | b.unwrap() | main.rs:19:34:19:43 | source(...) | main.rs:22:10:22:19 | b.unwrap() | $@ | main.rs:19:34:19:43 | source(...) | source(...) |
11696
| main.rs:27:10:27:10 | a | main.rs:26:13:26:22 | source(...) | main.rs:27:10:27:10 | a | $@ | main.rs:26:13:26:22 | source(...) | source(...) |
117-
| main.rs:29:10:29:10 | b | main.rs:26:13:26:22 | source(...) | main.rs:29:10:29:10 | b | $@ | main.rs:26:13:26:22 | source(...) | source(...) |
11897
| main.rs:43:38:43:38 | n | main.rs:41:30:41:39 | source(...) | main.rs:43:38:43:38 | n | $@ | main.rs:41:30:41:39 | source(...) | source(...) |
119-
| main.rs:47:38:47:38 | n | main.rs:41:30:41:39 | source(...) | main.rs:47:38:47:38 | n | $@ | main.rs:41:30:41:39 | source(...) | source(...) |
12098
| main.rs:63:22:63:22 | m | main.rs:58:22:58:31 | source(...) | main.rs:63:22:63:22 | m | $@ | main.rs:58:22:58:31 | source(...) | source(...) |
12199
| main.rs:85:18:85:34 | ...::read(...) | main.rs:84:32:84:41 | source(...) | main.rs:85:18:85:34 | ...::read(...) | $@ | main.rs:84:32:84:41 | source(...) | source(...) |

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ fn i64_clone() {
2626
let a = source(12);
2727
sink(a); // $ hasValueFlow=12
2828
let b = a.clone();
29-
sink(b); // $ hasValueFlow=12
29+
sink(b); // $ MISSING: hasValueFlow=12 - lack of builtins means that we cannot resolve clone call above, and hence not insert implicit borrow
3030
}
3131

3232
mod my_clone {
@@ -44,7 +44,7 @@ mod my_clone {
4444
}
4545
let u = w.clone();
4646
match u {
47-
Wrapper { n: n } => sink(n), // $ hasValueFlow=73
47+
Wrapper { n: n } => sink(n), // $ MISSING: hasValueFlow=73 - lack of expanded derives means that we cannot resolve clone call above, and hence not insert implicit borrow
4848
}
4949
}
5050
}

‎rust/ql/test/utils-tests/modelgenerator/CaptureSummaryModels.expected

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
unexpectedModel
22
| Unexpected summary found: repo::test;<crate::option::MyOption as crate::clone::Clone>::clone;Argument[self].Field[crate::option::MyOption::MySome(0)].Reference;ReturnValue.Field[crate::option::MyOption::MySome(0)];value;dfc-generated |
3-
| Unexpected summary found: repo::test;<crate::option::MyOption>::cloned;Argument[self].Field[crate::option::MyOption::MySome(0)].Reference;ReturnValue.Field[crate::option::MyOption::MySome(0)];value;dfc-generated |
43
expectedModel
4+
| Expected summary missing: repo::test;<crate::option::MyOption as crate::clone::Clone>::clone;Argument[self].Reference.Field[crate::option::MyOption::MySome(0)];ReturnValue.Field[crate::option::MyOption::MySome(0)];value;dfc-generated |

‎rust/ql/test/utils-tests/modelgenerator/option.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -414,7 +414,7 @@ impl<T> MyOption<&T> {
414414
}
415415
}
416416

417-
// summary=repo::test;<crate::option::MyOption>::cloned;Argument[self].Field[crate::option::MyOption::MySome(0)];ReturnValue.Field[crate::option::MyOption::MySome(0)];value;dfc-generated
417+
// summary=repo::test;<crate::option::MyOption>::cloned;Argument[self].Field[crate::option::MyOption::MySome(0)].Reference;ReturnValue.Field[crate::option::MyOption::MySome(0)];value;dfc-generated
418418
pub fn cloned(self) -> MyOption<T>
419419
where
420420
T: Clone,
@@ -438,7 +438,7 @@ impl<T> MyOption<&mut T> {
438438
}
439439
}
440440

441-
// summary=repo::test;<crate::option::MyOption>::cloned;Argument[self].Field[crate::option::MyOption::MySome(0)];ReturnValue.Field[crate::option::MyOption::MySome(0)];value;dfc-generated
441+
// summary=repo::test;<crate::option::MyOption>::cloned;Argument[self].Field[crate::option::MyOption::MySome(0)].Reference;ReturnValue.Field[crate::option::MyOption::MySome(0)];value;dfc-generated
442442
pub fn cloned(self) -> MyOption<T>
443443
where
444444
T: Clone,
@@ -466,7 +466,7 @@ impl<T> Clone for MyOption<T>
466466
where
467467
T: Clone,
468468
{
469-
// summary=repo::test;<crate::option::MyOption as crate::clone::Clone>::clone;Argument[self].Field[crate::option::MyOption::MySome(0)];ReturnValue.Field[crate::option::MyOption::MySome(0)];value;dfc-generated
469+
// summary=repo::test;<crate::option::MyOption as crate::clone::Clone>::clone;Argument[self].Reference.Field[crate::option::MyOption::MySome(0)];ReturnValue.Field[crate::option::MyOption::MySome(0)];value;dfc-generated
470470
fn clone(&self) -> Self {
471471
match self {
472472
MySome(x) => MySome(x.clone()),

0 commit comments

Comments
 (0)