Skip to content

Commit ad64e04

Browse files
hvitvedhvitved
authored
Merge pull request #19737 from hvitved/rust/type-inference-string-str-deref
Rust: Model `String` -> `str` implicit conversion in type inference
2 parents 1acd636 + 66c0ff6 commit ad64e04

File tree

10 files changed

+1439
-1236
lines changed

10 files changed

+1439
-1236
lines changed

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,3 +65,12 @@ class FutureTrait extends Trait {
6565
result.getName().getText() = "Output"
6666
}
6767
}
68+
69+
/**
70+
* The [`String` struct][1].
71+
*
72+
* [1]: https://doc.rust-lang.org/std/string/struct.String.html
73+
*/
74+
class StringStruct extends Struct {
75+
StringStruct() { this.getCanonicalPath() = "alloc::string::String" }
76+
}

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

Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -780,12 +780,18 @@ private Type inferCallExprBaseType(AstNode n, TypePath path) {
780780
not (path0.isEmpty() and result = TRefType()) and
781781
path = TypePath::cons(TRefTypeParameter(), path0)
782782
else (
783-
not path0.isCons(TRefTypeParameter(), _) and
784-
not (path0.isEmpty() and result = TRefType()) and
785-
path = path0
786-
or
787-
// adjust for implicit borrow
788-
path0.isCons(TRefTypeParameter(), path)
783+
not (
784+
receiverType.(StructType).asItemNode() instanceof StringStruct and
785+
result.(StructType).asItemNode() instanceof Builtins::Str
786+
) and
787+
(
788+
not path0.isCons(TRefTypeParameter(), _) and
789+
not (path0.isEmpty() and result = TRefType()) and
790+
path = path0
791+
or
792+
// adjust for implicit borrow
793+
path0.isCons(TRefTypeParameter(), path)
794+
)
789795
)
790796
)
791797
else path = path0
@@ -1130,12 +1136,27 @@ final class MethodCall extends Call {
11301136
Type getTypeAt(TypePath path) {
11311137
if this.receiverImplicitlyBorrowed()
11321138
then
1133-
exists(TypePath path0 | result = inferType(super.getReceiver(), path0) |
1134-
path0.isCons(TRefTypeParameter(), path)
1139+
exists(TypePath path0, Type t0 |
1140+
t0 = inferType(super.getReceiver(), path0) and
1141+
(
1142+
path0.isCons(TRefTypeParameter(), path)
1143+
or
1144+
not path0.isCons(TRefTypeParameter(), _) and
1145+
not (path0.isEmpty() and result = TRefType()) and
1146+
path = path0
1147+
)
1148+
|
1149+
result = t0
11351150
or
1136-
not path0.isCons(TRefTypeParameter(), _) and
1137-
not (path0.isEmpty() and result = TRefType()) and
1138-
path = path0
1151+
// We do not yet model the `Deref` trait, so we hard-code the fact that
1152+
// `String` dereferences to `str` here. This allows us e.g. to resolve
1153+
// `x.parse::<usize>()` to the function `<core::str>::parse` when `x` has
1154+
// type `String`.
1155+
//
1156+
// See also https://doc.rust-lang.org/reference/expressions/method-call-expr.html#r-expr.method.autoref-deref
1157+
path.isEmpty() and
1158+
t0.(StructType).asItemNode() instanceof StringStruct and
1159+
result.(StructType).asItemNode() instanceof Builtins::Str
11391160
)
11401161
else result = inferType(super.getReceiver(), path)
11411162
}

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -962,6 +962,8 @@ readStep
962962
| main.rs:442:25:442:29 | names | file://:0:0:0:0 | element | main.rs:442:9:442:20 | TuplePat |
963963
| main.rs:444:41:444:67 | [post] \|...\| ... | main.rs:441:9:441:20 | captured default_name | main.rs:444:41:444:67 | [post] default_name |
964964
| main.rs:444:44:444:55 | this | main.rs:441:9:441:20 | captured default_name | main.rs:444:44:444:55 | default_name |
965+
| main.rs:469:13:469:13 | [post] receiver for b | file://:0:0:0:0 | &ref | main.rs:469:13:469:13 | [post] b |
966+
| main.rs:470:19:470:19 | [post] receiver for b | file://:0:0:0:0 | &ref | main.rs:470:19:470:19 | [post] b |
965967
| main.rs:481:10:481:11 | vs | file://:0:0:0:0 | element | main.rs:481:10:481:14 | vs[0] |
966968
| main.rs:482:11:482:35 | ... .unwrap() | file://:0:0:0:0 | &ref | main.rs:482:10:482:35 | * ... |
967969
| main.rs:483:11:483:35 | ... .unwrap() | file://:0:0:0:0 | &ref | main.rs:483:10:483:35 | * ... |
@@ -1058,6 +1060,8 @@ storeStep
10581060
| main.rs:429:30:429:30 | 3 | file://:0:0:0:0 | element | main.rs:429:23:429:31 | [...] |
10591061
| main.rs:432:18:432:27 | source(...) | file://:0:0:0:0 | element | main.rs:432:5:432:11 | [post] mut_arr |
10601062
| main.rs:444:41:444:67 | default_name | main.rs:441:9:441:20 | captured default_name | main.rs:444:41:444:67 | \|...\| ... |
1063+
| main.rs:469:13:469:13 | b | file://:0:0:0:0 | &ref | main.rs:469:13:469:13 | receiver for b |
1064+
| main.rs:470:19:470:19 | b | file://:0:0:0:0 | &ref | main.rs:470:19:470:19 | receiver for b |
10611065
| main.rs:479:15:479:24 | source(...) | file://:0:0:0:0 | element | main.rs:479:14:479:34 | [...] |
10621066
| main.rs:479:27:479:27 | 2 | file://:0:0:0:0 | element | main.rs:479:14:479:34 | [...] |
10631067
| main.rs:479:30:479:30 | 3 | file://:0:0:0:0 | element | main.rs:479:14:479:34 | [...] |

‎rust/ql/test/library-tests/dataflow/sources/CONSISTENCY/PathResolutionConsistency.expected

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,10 @@ multipleMethodCallTargets
1313
| test_futures_io.rs:92:26:92:63 | pinned.poll_read(...) | file://:0:0:0:0 | fn poll_read |
1414
| test_futures_io.rs:115:22:115:50 | pinned.poll_fill_buf(...) | file://:0:0:0:0 | fn poll_fill_buf |
1515
| test_futures_io.rs:115:22:115:50 | pinned.poll_fill_buf(...) | file://:0:0:0:0 | fn poll_fill_buf |
16+
| web_frameworks.rs:88:14:88:23 | a.as_str() | file://:0:0:0:0 | fn as_str |
17+
| web_frameworks.rs:88:14:88:23 | a.as_str() | file://:0:0:0:0 | fn as_str |
18+
| web_frameworks.rs:89:14:89:25 | a.as_bytes() | file://:0:0:0:0 | fn as_bytes |
19+
| web_frameworks.rs:89:14:89:25 | a.as_bytes() | file://:0:0:0:0 | fn as_bytes |
1620
multiplePathResolutions
1721
| test.rs:112:62:112:73 | ...::from | file://:0:0:0:0 | fn from |
1822
| test.rs:112:62:112:73 | ...::from | file://:0:0:0:0 | fn from |

‎rust/ql/test/library-tests/dataflow/strings/CONSISTENCY/PathResolutionConsistency.expected

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
multipleMethodCallTargets
2+
| main.rs:64:16:64:25 | s.as_str() | file://:0:0:0:0 | fn as_str |
3+
| main.rs:64:16:64:25 | s.as_str() | file://:0:0:0:0 | fn as_str |
14
multiplePathResolutions
25
| main.rs:52:11:52:22 | ...::from | file://:0:0:0:0 | fn from |
36
| main.rs:52:11:52:22 | ...::from | file://:0:0:0:0 | fn from |

‎rust/ql/test/library-tests/sensitivedata/CONSISTENCY/PathResolutionConsistency.expected

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
multipleMethodCallTargets
2+
| test.rs:55:7:55:26 | ... .as_str() | file://:0:0:0:0 | fn as_str |
3+
| test.rs:55:7:55:26 | ... .as_str() | file://:0:0:0:0 | fn as_str |
4+
| test.rs:56:7:56:21 | ... .as_str() | file://:0:0:0:0 | fn as_str |
5+
| test.rs:56:7:56:21 | ... .as_str() | file://:0:0:0:0 | fn as_str |
6+
| test.rs:72:7:72:26 | ... .as_str() | file://:0:0:0:0 | fn as_str |
7+
| test.rs:72:7:72:26 | ... .as_str() | file://:0:0:0:0 | fn as_str |
8+
| test.rs:73:7:73:36 | ... .as_str() | file://:0:0:0:0 | fn as_str |
9+
| test.rs:73:7:73:36 | ... .as_str() | file://:0:0:0:0 | fn as_str |
10+
| test.rs:74:7:74:34 | ... .as_str() | file://:0:0:0:0 | fn as_str |
11+
| test.rs:74:7:74:34 | ... .as_str() | file://:0:0:0:0 | fn as_str |
12+
| test.rs:75:7:75:27 | ... .as_str() | file://:0:0:0:0 | fn as_str |
13+
| test.rs:75:7:75:27 | ... .as_str() | file://:0:0:0:0 | fn as_str |
14+
| test.rs:254:7:254:36 | ... .as_str() | file://:0:0:0:0 | fn as_str |
15+
| test.rs:254:7:254:36 | ... .as_str() | file://:0:0:0:0 | fn as_str |
16+
| test.rs:256:7:256:33 | ... .as_str() | file://:0:0:0:0 | fn as_str |
17+
| test.rs:256:7:256:33 | ... .as_str() | file://:0:0:0:0 | fn as_str |
18+
| test.rs:257:7:257:36 | ... .as_str() | file://:0:0:0:0 | fn as_str |
19+
| test.rs:257:7:257:36 | ... .as_str() | file://:0:0:0:0 | fn as_str |
20+
| test.rs:258:7:258:26 | ... .as_str() | file://:0:0:0:0 | fn as_str |
21+
| test.rs:258:7:258:26 | ... .as_str() | file://:0:0:0:0 | fn as_str |
22+
| test.rs:262:7:262:28 | ... .as_str() | file://:0:0:0:0 | fn as_str |
23+
| test.rs:262:7:262:28 | ... .as_str() | file://:0:0:0:0 | fn as_str |
24+
| test.rs:263:7:263:37 | ... .as_str() | file://:0:0:0:0 | fn as_str |
25+
| test.rs:263:7:263:37 | ... .as_str() | file://:0:0:0:0 | fn as_str |
26+
| test.rs:264:7:264:36 | ... .as_str() | file://:0:0:0:0 | fn as_str |
27+
| test.rs:264:7:264:36 | ... .as_str() | file://:0:0:0:0 | fn as_str |
28+
| test.rs:267:7:267:32 | ... .as_str() | file://:0:0:0:0 | fn as_str |
29+
| test.rs:267:7:267:32 | ... .as_str() | file://:0:0:0:0 | fn as_str |
30+
| test.rs:277:7:277:34 | ... .as_str() | file://:0:0:0:0 | fn as_str |
31+
| test.rs:277:7:277:34 | ... .as_str() | file://:0:0:0:0 | fn as_str |
32+
| test.rs:280:7:280:36 | ... .as_str() | file://:0:0:0:0 | fn as_str |
33+
| test.rs:280:7:280:36 | ... .as_str() | file://:0:0:0:0 | fn as_str |
34+
| test.rs:284:7:284:39 | ... .as_str() | file://:0:0:0:0 | fn as_str |
35+
| test.rs:284:7:284:39 | ... .as_str() | file://:0:0:0:0 | fn as_str |
36+
| test.rs:291:7:291:53 | ... .as_str() | file://:0:0:0:0 | fn as_str |
37+
| test.rs:291:7:291:53 | ... .as_str() | file://:0:0:0:0 | fn as_str |
38+
| test.rs:292:7:292:45 | ... .as_str() | file://:0:0:0:0 | fn as_str |
39+
| test.rs:292:7:292:45 | ... .as_str() | file://:0:0:0:0 | fn as_str |
40+
| test.rs:294:7:294:39 | ... .as_str() | file://:0:0:0:0 | fn as_str |
41+
| test.rs:294:7:294:39 | ... .as_str() | file://:0:0:0:0 | fn as_str |
42+
| test.rs:295:7:295:34 | ... .as_str() | file://:0:0:0:0 | fn as_str |
43+
| test.rs:295:7:295:34 | ... .as_str() | file://:0:0:0:0 | fn as_str |
44+
| test.rs:296:7:296:42 | ... .as_str() | file://:0:0:0:0 | fn as_str |
45+
| test.rs:296:7:296:42 | ... .as_str() | file://:0:0:0:0 | fn as_str |
46+
| test.rs:298:7:298:48 | ... .as_str() | file://:0:0:0:0 | fn as_str |
47+
| test.rs:298:7:298:48 | ... .as_str() | file://:0:0:0:0 | fn as_str |
48+
| test.rs:299:7:299:35 | ... .as_str() | file://:0:0:0:0 | fn as_str |
49+
| test.rs:299:7:299:35 | ... .as_str() | file://:0:0:0:0 | fn as_str |
50+
| test.rs:300:7:300:35 | ... .as_str() | file://:0:0:0:0 | fn as_str |
51+
| test.rs:300:7:300:35 | ... .as_str() | file://:0:0:0:0 | fn as_str |
52+
| test.rs:339:7:339:39 | ... .as_str() | file://:0:0:0:0 | fn as_str |
53+
| test.rs:339:7:339:39 | ... .as_str() | file://:0:0:0:0 | fn as_str |

‎rust/ql/test/library-tests/type-inference/main.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1107,6 +1107,11 @@ mod method_call_type_conversion {
11071107
// implicit dereference handling doesn't affect nested borrows.
11081108
let t = x7.m1(); // $ method=m1 type=t:& type=t:&T.S2
11091109
println!("{:?}", x7);
1110+
1111+
let x9 : String = "Hello".to_string(); // $ type=x9:String
1112+
// Implicit `String` -> `str` conversion happens via the `Deref` trait:
1113+
// https://doc.rust-lang.org/std/string/struct.String.html#deref.
1114+
let u = x9.parse::<u32>(); // $ method=parse type=u:T.u32
11101115
}
11111116
}
11121117

0 commit comments

Comments
 (0)