Skip to content

Commit 44b1ad5

Browse files
paldepindpaldepind
committed
Rust: Support self parameters in variable and SSA library
1 parent d06b583 commit 44b1ad5

File tree

8 files changed

+79
-48
lines changed

8 files changed

+79
-48
lines changed

‎rust/ql/.generated.list

Lines changed: 0 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎rust/ql/.gitattributes

Lines changed: 0 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ predicate variableWrite(AstNode write, Variable v) {
2424
not isUnitializedLet(pat, v)
2525
)
2626
or
27+
exists(SelfParam self | self = write and self = v.getSelfParam())
28+
or
2729
exists(VariableAccess access |
2830
access = write and
2931
access.getVariable() = v
@@ -477,7 +479,7 @@ private module DataFlowIntegrationInput implements Impl::DataFlowIntegrationInpu
477479
none() // handled in `DataFlowImpl.qll` instead
478480
}
479481

480-
class Parameter = CfgNodes::ParamCfgNode;
482+
class Parameter = CfgNodes::ParamBaseCfgNode;
481483

482484
/** Holds if SSA definition `def` initializes parameter `p` at function entry. */
483485
predicate ssaDefInitializesParam(WriteDefinition def, Parameter p) {

‎rust/ql/lib/codeql/rust/elements/internal/ParamListImpl.qll

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
// generated by codegen, remove this comment if you wish to edit this file
21
/**
32
* This module provides a hand-modifiable wrapper around the generated class `ParamList`.
43
*
@@ -12,11 +11,17 @@ private import codeql.rust.elements.internal.generated.ParamList
1211
* be referenced directly.
1312
*/
1413
module Impl {
14+
// the following QLdoc is generated: if you need to edit it, do it in the schema file
1515
/**
1616
* A ParamList. For example:
1717
* ```rust
1818
* todo!()
1919
* ```
2020
*/
21-
class ParamList extends Generated::ParamList { }
21+
class ParamList extends Generated::ParamList {
22+
/**
23+
* Gets any of the parameters of this parameter list.
24+
*/
25+
final ParamBase getAParamBase() { result = this.getParam(_) or result = this.getSelfParam() }
26+
}
2227
}

‎rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll

Lines changed: 45 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -73,24 +73,33 @@ module Impl {
7373
* where `definingNode` is the entire `Either::Left(x) | Either::Right(x)`
7474
* pattern.
7575
*/
76-
private predicate variableDecl(AstNode definingNode, IdentPat p, string name) {
77-
(
78-
definingNode = getOutermostEnclosingOrPat(p)
79-
or
80-
not exists(getOutermostEnclosingOrPat(p)) and
81-
definingNode = p.getName()
82-
) and
83-
name = p.getName().getText() and
84-
// exclude for now anything starting with an uppercase character, which may be a reference to
85-
// an enum constant (e.g. `None`). This excludes static and constant variables (UPPERCASE),
86-
// which we don't appear to recognize yet anyway. This also assumes programmers follow the
87-
// naming guidelines, which they generally do, but they're not enforced.
88-
not name.charAt(0).isUppercase() and
89-
// exclude parameters from functions without a body as these are trait method declarations
90-
// without implementations
91-
not exists(Function f | not f.hasBody() and f.getParamList().getAParam().getPat() = p) and
92-
// exclude parameters from function pointer types (e.g. `x` in `fn(x: i32) -> i32`)
93-
not exists(FnPtrType fp | fp.getParamList().getParam(_).getPat() = p)
76+
private predicate variableDecl(AstNode definingNode, AstNode p, string name) {
77+
exists(SelfParam sp | sp = p |
78+
definingNode = sp.getName() and
79+
name = sp.getName().getText() and
80+
// exclude self parameters from functions without a body as these are
81+
// trait method declarations without implementations
82+
not exists(Function f | not f.hasBody() and f.getParamList().getSelfParam() = sp)
83+
)
84+
or
85+
exists(IdentPat pat | pat = p |
86+
(
87+
definingNode = getOutermostEnclosingOrPat(pat)
88+
or
89+
not exists(getOutermostEnclosingOrPat(pat)) and definingNode = pat.getName()
90+
) and
91+
name = pat.getName().getText() and
92+
// exclude for now anything starting with an uppercase character, which may be a reference to
93+
// an enum constant (e.g. `None`). This excludes static and constant variables (UPPERCASE),
94+
// which we don't appear to recognize yet anyway. This also assumes programmers follow the
95+
// naming guidelines, which they generally do, but they're not enforced.
96+
not name.charAt(0).isUppercase() and
97+
// exclude parameters from functions without a body as these are trait method declarations
98+
// without implementations
99+
not exists(Function f | not f.hasBody() and f.getParamList().getAParam().getPat() = pat) and
100+
// exclude parameters from function pointer types (e.g. `x` in `fn(x: i32) -> i32`)
101+
not exists(FnPtrType fp | fp.getParamList().getParam(_).getPat() = pat)
102+
)
94103
}
95104

96105
/** A variable. */
@@ -112,8 +121,11 @@ module Impl {
112121
/** Gets an access to this variable. */
113122
VariableAccess getAnAccess() { result.getVariable() = this }
114123

124+
/** Gets the `self` parameter that declares this variable, if one exists. */
125+
SelfParam getSelfParam() { variableDecl(definingNode, result, name) }
126+
115127
/**
116-
* Gets the pattern that declares this variable.
128+
* Gets the pattern that declares this variable, if one exists.
117129
*
118130
* Normally, the pattern is unique, except when introduced in an or pattern:
119131
*
@@ -135,7 +147,9 @@ module Impl {
135147
predicate isCaptured() { this.getAnAccess().isCapture() }
136148

137149
/** Gets the parameter that introduces this variable, if any. */
138-
Param getParameter() { parameterDeclInScope(result, this, _) }
150+
ParamBase getParameter() {
151+
result = this.getSelfParam() or result = getAVariablePatAncestor(this).getParentNode()
152+
}
139153

140154
/** Hold is this variable is mutable. */
141155
predicate isMutable() { this.getPat().isMut() }
@@ -144,7 +158,11 @@ module Impl {
144158
predicate isImmutable() { not this.isMutable() }
145159
}
146160

147-
/** A path expression that may access a local variable. */
161+
/**
162+
* A path expression that may access a local variable. These are paths that
163+
* only consists of a simple name (i.e., without generic arguments,
164+
* qualifiers, etc.).
165+
*/
148166
private class VariableAccessCand extends PathExprBase {
149167
string name_;
150168

@@ -190,10 +208,7 @@ module Impl {
190208
private VariableScope getEnclosingScope(AstNode n) { result = getAnAncestorInVariableScope(n) }
191209

192210
private Pat getAVariablePatAncestor(Variable v) {
193-
exists(AstNode definingNode, string name |
194-
v = MkVariable(definingNode, name) and
195-
variableDecl(definingNode, result, name)
196-
)
211+
result = v.getPat()
197212
or
198213
exists(Pat mid |
199214
mid = getAVariablePatAncestor(v) and
@@ -205,20 +220,10 @@ module Impl {
205220
* Holds if parameter `p` introduces the variable `v` inside variable scope
206221
* `scope`.
207222
*/
208-
private predicate parameterDeclInScope(Param p, Variable v, VariableScope scope) {
209-
exists(Pat pat |
210-
pat = getAVariablePatAncestor(v) and
211-
p.getPat() = pat
212-
|
213-
exists(Function f |
214-
f.getParamList().getAParam() = p and
215-
scope = f.getBody()
216-
)
217-
or
218-
exists(ClosureExpr ce |
219-
ce.getParamList().getAParam() = p and
220-
scope = ce.getBody()
221-
)
223+
private predicate parameterDeclInScope(Variable v, VariableScope scope) {
224+
exists(Callable f |
225+
v.getParameter() = f.getParamList().getAParamBase() and
226+
scope = [f.(Function).getBody(), f.(ClosureExpr).getBody()]
222227
)
223228
}
224229

@@ -231,7 +236,7 @@ module Impl {
231236
) {
232237
name = v.getName() and
233238
(
234-
parameterDeclInScope(_, v, scope) and
239+
parameterDeclInScope(v, scope) and
235240
scope.getLocation().hasLocationFileInfo(_, line, column, _, _)
236241
or
237242
exists(Pat pat | pat = getAVariablePatAncestor(v) |

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,8 +134,11 @@ definition
134134
| variables.rs:461:13:461:14 | b1 | variables.rs:461:13:461:14 | b1 |
135135
| variables.rs:461:24:461:25 | b2 | variables.rs:461:24:461:25 | b2 |
136136
| variables.rs:462:9:462:9 | x | variables.rs:462:9:462:9 | x |
137+
| variables.rs:482:15:482:23 | SelfParam | variables.rs:482:20:482:23 | self |
138+
| variables.rs:486:11:486:14 | SelfParam | variables.rs:486:11:486:14 | self |
137139
| variables.rs:501:9:501:9 | x | variables.rs:501:9:501:9 | x |
138140
| variables.rs:505:9:505:9 | z | variables.rs:505:9:505:9 | z |
141+
| variables.rs:514:10:514:18 | SelfParam | variables.rs:514:15:514:18 | self |
139142
read
140143
| variables.rs:3:14:3:14 | s | variables.rs:3:14:3:14 | s | variables.rs:4:20:4:20 | s |
141144
| variables.rs:7:14:7:14 | i | variables.rs:7:14:7:14 | i | variables.rs:8:20:8:20 | i |
@@ -257,7 +260,10 @@ read
257260
| variables.rs:462:9:462:9 | x | variables.rs:462:9:462:9 | x | variables.rs:466:19:466:19 | x |
258261
| variables.rs:462:9:462:9 | x | variables.rs:462:9:462:9 | x | variables.rs:470:19:470:19 | x |
259262
| variables.rs:462:9:462:9 | x | variables.rs:462:9:462:9 | x | variables.rs:472:19:472:19 | x |
263+
| variables.rs:482:15:482:23 | SelfParam | variables.rs:482:20:482:23 | self | variables.rs:483:16:483:19 | self |
264+
| variables.rs:486:11:486:14 | SelfParam | variables.rs:486:11:486:14 | self | variables.rs:487:9:487:12 | self |
260265
| variables.rs:501:9:501:9 | x | variables.rs:501:9:501:9 | x | variables.rs:503:15:503:15 | x |
266+
| variables.rs:514:10:514:18 | SelfParam | variables.rs:514:15:514:18 | self | variables.rs:515:6:515:9 | self |
261267
firstRead
262268
| variables.rs:3:14:3:14 | s | variables.rs:3:14:3:14 | s | variables.rs:4:20:4:20 | s |
263269
| variables.rs:7:14:7:14 | i | variables.rs:7:14:7:14 | i | variables.rs:8:20:8:20 | i |
@@ -356,7 +362,10 @@ firstRead
356362
| variables.rs:461:24:461:25 | b2 | variables.rs:461:24:461:25 | b2 | variables.rs:469:8:469:9 | b2 |
357363
| variables.rs:462:9:462:9 | x | variables.rs:462:9:462:9 | x | variables.rs:464:19:464:19 | x |
358364
| variables.rs:462:9:462:9 | x | variables.rs:462:9:462:9 | x | variables.rs:466:19:466:19 | x |
365+
| variables.rs:482:15:482:23 | SelfParam | variables.rs:482:20:482:23 | self | variables.rs:483:16:483:19 | self |
366+
| variables.rs:486:11:486:14 | SelfParam | variables.rs:486:11:486:14 | self | variables.rs:487:9:487:12 | self |
359367
| variables.rs:501:9:501:9 | x | variables.rs:501:9:501:9 | x | variables.rs:503:15:503:15 | x |
368+
| variables.rs:514:10:514:18 | SelfParam | variables.rs:514:15:514:18 | self | variables.rs:515:6:515:9 | self |
360369
lastRead
361370
| variables.rs:3:14:3:14 | s | variables.rs:3:14:3:14 | s | variables.rs:4:20:4:20 | s |
362371
| variables.rs:7:14:7:14 | i | variables.rs:7:14:7:14 | i | variables.rs:8:20:8:20 | i |
@@ -456,7 +465,10 @@ lastRead
456465
| variables.rs:461:24:461:25 | b2 | variables.rs:461:24:461:25 | b2 | variables.rs:469:8:469:9 | b2 |
457466
| variables.rs:462:9:462:9 | x | variables.rs:462:9:462:9 | x | variables.rs:470:19:470:19 | x |
458467
| variables.rs:462:9:462:9 | x | variables.rs:462:9:462:9 | x | variables.rs:472:19:472:19 | x |
468+
| variables.rs:482:15:482:23 | SelfParam | variables.rs:482:20:482:23 | self | variables.rs:483:16:483:19 | self |
469+
| variables.rs:486:11:486:14 | SelfParam | variables.rs:486:11:486:14 | self | variables.rs:487:9:487:12 | self |
459470
| variables.rs:501:9:501:9 | x | variables.rs:501:9:501:9 | x | variables.rs:503:15:503:15 | x |
471+
| variables.rs:514:10:514:18 | SelfParam | variables.rs:514:15:514:18 | self | variables.rs:515:6:515:9 | self |
460472
adjacentReads
461473
| variables.rs:35:9:35:10 | x3 | variables.rs:35:9:35:10 | x3 | variables.rs:36:15:36:16 | x3 | variables.rs:38:9:38:10 | x3 |
462474
| variables.rs:43:9:43:10 | x4 | variables.rs:43:9:43:10 | x4 | variables.rs:44:15:44:16 | x4 | variables.rs:49:15:49:16 | x4 |

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,9 +95,12 @@ variable
9595
| variables.rs:461:13:461:14 | b1 |
9696
| variables.rs:461:24:461:25 | b2 |
9797
| variables.rs:462:9:462:9 | x |
98+
| variables.rs:482:20:482:23 | self |
99+
| variables.rs:486:11:486:14 | self |
98100
| variables.rs:492:13:492:13 | a |
99101
| variables.rs:501:9:501:9 | x |
100102
| variables.rs:505:9:505:9 | z |
103+
| variables.rs:514:15:514:18 | self |
101104
| variables.rs:520:11:520:11 | a |
102105
variableAccess
103106
| variables.rs:4:20:4:20 | s | variables.rs:3:14:3:14 | s |
@@ -246,6 +249,8 @@ variableAccess
246249
| variables.rs:469:8:469:9 | b2 | variables.rs:461:24:461:25 | b2 |
247250
| variables.rs:470:19:470:19 | x | variables.rs:462:9:462:9 | x |
248251
| variables.rs:472:19:472:19 | x | variables.rs:462:9:462:9 | x |
252+
| variables.rs:483:16:483:19 | self | variables.rs:482:20:482:23 | self |
253+
| variables.rs:487:9:487:12 | self | variables.rs:486:11:486:14 | self |
249254
| variables.rs:493:15:493:15 | a | variables.rs:492:13:492:13 | a |
250255
| variables.rs:494:5:494:5 | a | variables.rs:492:13:492:13 | a |
251256
| variables.rs:495:15:495:15 | a | variables.rs:492:13:492:13 | a |
@@ -254,6 +259,7 @@ variableAccess
254259
| variables.rs:502:20:502:20 | x | variables.rs:501:9:501:9 | x |
255260
| variables.rs:503:15:503:15 | x | variables.rs:501:9:501:9 | x |
256261
| variables.rs:506:20:506:20 | z | variables.rs:505:9:505:9 | z |
262+
| variables.rs:515:6:515:9 | self | variables.rs:514:15:514:18 | self |
257263
| variables.rs:521:3:521:3 | a | variables.rs:520:11:520:11 | a |
258264
| variables.rs:523:13:523:13 | a | variables.rs:520:11:520:11 | a |
259265
variableWriteAccess
@@ -396,11 +402,14 @@ variableReadAccess
396402
| variables.rs:469:8:469:9 | b2 | variables.rs:461:24:461:25 | b2 |
397403
| variables.rs:470:19:470:19 | x | variables.rs:462:9:462:9 | x |
398404
| variables.rs:472:19:472:19 | x | variables.rs:462:9:462:9 | x |
405+
| variables.rs:483:16:483:19 | self | variables.rs:482:20:482:23 | self |
406+
| variables.rs:487:9:487:12 | self | variables.rs:486:11:486:14 | self |
399407
| variables.rs:493:15:493:15 | a | variables.rs:492:13:492:13 | a |
400408
| variables.rs:494:5:494:5 | a | variables.rs:492:13:492:13 | a |
401409
| variables.rs:495:15:495:15 | a | variables.rs:492:13:492:13 | a |
402410
| variables.rs:497:15:497:15 | a | variables.rs:492:13:492:13 | a |
403411
| variables.rs:503:15:503:15 | x | variables.rs:501:9:501:9 | x |
412+
| variables.rs:515:6:515:9 | self | variables.rs:514:15:514:18 | self |
404413
| variables.rs:521:3:521:3 | a | variables.rs:520:11:520:11 | a |
405414
| variables.rs:523:13:523:13 | a | variables.rs:520:11:520:11 | a |
406415
variableInitializer

‎rust/ql/test/library-tests/variables/variables.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -480,11 +480,11 @@ struct MyStruct {
480480

481481
impl MyStruct {
482482
fn my_get(&mut self) -> i64 {
483-
return self.val; // $ MISSING: read_access=self
483+
return self.val; // $ read_access=self
484484
}
485485

486486
fn id(self) -> Self {
487-
self // $ MISSING: read_access=self
487+
self // $ read_access=self
488488
}
489489
}
490490

@@ -512,7 +512,7 @@ trait Bar {
512512

513513
impl MyStruct {
514514
fn bar(&mut self) {
515-
*self = MyStruct { val: 3 }; // MISSING: $ read_access=self
515+
*self = MyStruct { val: 3 }; // $ read_access=self
516516
}
517517
}
518518

0 commit comments

Comments
 (0)