Skip to content

Commit 7cac226

Browse files
hvitvedhvitved
committed
Rust: Adjust SSA write node for (compound) assignments
1 parent 4e77b1b commit 7cac226

File tree

9 files changed

+113
-32
lines changed

9 files changed

+113
-32
lines changed

‎rust/ql/lib/codeql/rust/controlflow/CfgNodes.qll‎

Lines changed: 61 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,36 @@ class ExitCfgNode = CfgImpl::ExitNode;
1616

1717
class AnnotatedExitCfgNode = CfgImpl::AnnotatedExitNode;
1818

19+
/** A variable access. */
20+
final class VariableAccessCfgNode extends PathExprBaseCfgNode {
21+
private VariableAccess a;
22+
23+
VariableAccessCfgNode() { a = this.getAstNode() }
24+
25+
/** Gets the underlying `VariableAccess`. */
26+
VariableAccess getAccess() { result = a }
27+
}
28+
29+
/** A variable write. */
30+
final class VariableWriteAccessCfgNode extends VariableAccessCfgNode {
31+
private VariableWriteAccess a;
32+
33+
VariableWriteAccessCfgNode() { a = this.getAstNode() }
34+
35+
/** Gets the underlying `VariableWriteAccess`. */
36+
VariableWriteAccess getAccess() { result = a }
37+
}
38+
39+
/** A variable read. */
40+
final class VariableReadAccessCfgNode extends VariableAccessCfgNode {
41+
private VariableReadAccess a;
42+
43+
VariableReadAccessCfgNode() { a = this.getAstNode() }
44+
45+
/** Gets the underlying `VariableReadAccess`. */
46+
VariableReadAccess getAccess() { result = a }
47+
}
48+
1949
/**
2050
* An assignment expression, for example
2151
*
@@ -24,12 +54,42 @@ class AnnotatedExitCfgNode = CfgImpl::AnnotatedExitNode;
2454
* ```
2555
*/
2656
final class AssignmentExprCfgNode extends BinaryExprCfgNode {
27-
AssignmentExpr a;
57+
AssignmentExprChildMapping a;
2858

2959
AssignmentExprCfgNode() { a = this.getBinaryExpr() }
3060

3161
/** Gets the underlying `AssignmentExpr`. */
3262
AssignmentExpr getAssignmentExpr() { result = a }
63+
64+
/**
65+
* Gets a write access that occurs in the left-hand side of this assignment expression.
66+
*/
67+
VariableWriteAccessCfgNode getAWriteAccess() {
68+
a = result.getAccess().getAssignmentExpr() and
69+
any(ChildMapping mapping).hasCfgChild(a, result.getAccess(), this, result)
70+
}
71+
}
72+
73+
/**
74+
* A compound assignment expression, for example:
75+
* ```rust
76+
* x += y;
77+
* ```
78+
*
79+
* Note that compound assignment expressions are syntatic sugar for
80+
* trait invocations, i.e., the above actually means
81+
*
82+
* ```rust
83+
* (&mut x).add_assign(y);
84+
* ```
85+
*/
86+
final class CompoundAssignmentExprCfgNode extends BinaryExprCfgNode {
87+
CompoundAssignmentExpr a;
88+
89+
CompoundAssignmentExprCfgNode() { a = this.getBinaryExpr() }
90+
91+
/** Gets the underlying `CompoundAssignmentExpr`. */
92+
CompoundAssignmentExpr getCompoundAssignmentExpr() { result = a }
3393
}
3494

3595
/**

‎rust/ql/lib/codeql/rust/controlflow/internal/CfgNodes.qll‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,12 @@ class FormatArgsExprChildMapping extends ParentAstNode, CfgImpl::ExprTrees::Form
8181
override predicate relevantChild(AstNode child) { child = this.getChildNode(_) }
8282
}
8383

84+
class AssignmentExprChildMapping extends ParentAstNode, AssignmentExpr {
85+
override predicate relevantChild(AstNode child) {
86+
child.(VariableWriteAccess).getAssignmentExpr() = this
87+
}
88+
}
89+
8490
private class ChildMappingImpl extends ChildMapping {
8591
/** Gets a CFG node for `child`, where `child` is a relevant child node of `parent`. */
8692
private CfgNode getRelevantChildCfgNode(AstNode parent, AstNode child) {

‎rust/ql/lib/codeql/rust/dataflow/Ssa.qll‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -171,9 +171,20 @@ module Ssa {
171171
private CfgNode write;
172172

173173
WriteDefinition() {
174-
exists(BasicBlock bb, int i, Variable v |
174+
exists(BasicBlock bb, int i, Variable v, CfgNode n |
175175
this.definesAt(v, bb, i) and
176-
SsaImpl::variableWriteActual(bb, i, v, write)
176+
SsaImpl::variableWriteActual(bb, i, v, n)
177+
|
178+
write.(VariableAccessCfgNode).getAccess().getVariable() = v and
179+
(
180+
write = n.(AssignmentExprCfgNode).getAWriteAccess()
181+
or
182+
write = n.(CompoundAssignmentExprCfgNode).getLhs()
183+
)
184+
or
185+
not n instanceof AssignmentExprCfgNode and
186+
not n instanceof CompoundAssignmentExprCfgNode and
187+
write = n
177188
)
178189
}
179190

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -263,7 +263,7 @@ module LocalFlow {
263263
or
264264
// An edge from a pattern/expression to its corresponding SSA definition.
265265
nodeFrom.(AstCfgFlowNode).getCfgNode() =
266-
nodeTo.(SsaNode).asDefinition().(Ssa::WriteDefinition).getControlFlowNode()
266+
nodeTo.(SsaNode).asDefinition().(Ssa::WriteDefinition).getWriteAccess()
267267
or
268268
nodeFrom.(SourceParameterNode).getParameter().(ParamCfgNode).getPat() = nodeTo.asPat()
269269
or

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

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,23 +18,22 @@ private predicate isInUninitializedLet(Name name) {
1818
)
1919
}
2020

21-
/** Holds if `write` writes to variable `v`. */
22-
predicate variableWrite(AstNode write, Variable v) {
21+
/** Holds if `write` writes to variable `v` via `access`. */
22+
predicate variableWrite(AstNode write, AstNode access, Variable v) {
2323
exists(Name name |
2424
name = write and
25+
access = write and
2526
name = v.getName() and
2627
not isInUninitializedLet(name)
2728
)
2829
or
29-
exists(VariableAccess access |
30-
access = write and
31-
access.getVariable() = v
32-
|
33-
access instanceof VariableWriteAccess
30+
v = access.(VariableAccess).getVariable() and
31+
(
32+
write = access.(VariableWriteAccess).getAssignmentExpr()
3433
or
3534
// Although compound assignments, like `x += y`, may in fact not update `x`,
3635
// it makes sense to treat them as such
37-
access = any(CompoundAssignmentExpr cae).getLhs()
36+
access = write.(CompoundAssignmentExpr).getLhs()
3837
)
3938
}
4039

@@ -226,7 +225,7 @@ private module Cached {
226225
cached
227226
predicate variableWriteActual(BasicBlock bb, int i, Variable v, CfgNode write) {
228227
bb.getNode(i) = write and
229-
variableWrite(write.getAstNode(), v)
228+
variableWrite(write.getAstNode(), _, v)
230229
}
231230

232231
cached

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

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -679,11 +679,11 @@ module Impl {
679679
}
680680

681681
/** Holds if `e` occurs in the LHS of an assignment or compound assignment. */
682-
private predicate assignmentExprDescendant(Expr e) {
683-
e = any(AssignmentExpr ae).getLhs()
682+
private predicate assignmentExprDescendant(AssignmentExpr ae, Expr e) {
683+
e = ae.getLhs()
684684
or
685685
exists(Expr mid |
686-
assignmentExprDescendant(mid) and
686+
assignmentExprDescendant(ae, mid) and
687687
getImmediateParentAdj(e) = mid and
688688
not mid instanceof DerefExpr and
689689
not mid instanceof FieldExpr and
@@ -693,11 +693,16 @@ module Impl {
693693

694694
/** A variable write. */
695695
class VariableWriteAccess extends VariableAccess {
696+
private AssignmentExpr ae;
697+
696698
cached
697699
VariableWriteAccess() {
698700
Cached::ref() and
699-
assignmentExprDescendant(this)
701+
assignmentExprDescendant(ae, this)
700702
}
703+
704+
/** Gets the assignment expression that has this write access in the left-hand side. */
705+
AssignmentExpr getAssignmentExpr() { result = ae }
701706
}
702707

703708
/** A variable read. */

‎rust/ql/src/queries/unusedentities/UnusedValue.ql‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import UnusedVariable
1616

1717
from AstNode write, Ssa::Variable v
1818
where
19-
variableWrite(write, v) and
19+
variableWrite(_, write, v) and
2020
not v instanceof DiscardVariable and
2121
not write.isInMacroExpansion() and
2222
not isAllowableUnused(v) and

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
multipleCallTargets
2-
| main.rs:89:19:89:40 | ...::from(...) |
3-
| main.rs:111:19:111:40 | ...::from(...) |
2+
| main.rs:91:19:91:40 | ...::from(...) |
3+
| main.rs:113:19:113:40 | ...::from(...) |

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

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,7 @@ read
198198
| main.rs:20:9:20:10 | x1 | main.rs:20:9:20:10 | x1 | main.rs:21:15:21:16 | x1 |
199199
| main.rs:25:13:25:14 | x2 | main.rs:25:13:25:14 | x2 | main.rs:26:15:26:16 | x2 |
200200
| main.rs:27:5:27:6 | x2 | main.rs:25:13:25:14 | x2 | main.rs:28:15:28:16 | x2 |
201-
| main.rs:29:5:29:6 | x2 | main.rs:25:13:25:14 | x2 | main.rs:29:10:29:11 | x2 |
201+
| main.rs:27:5:27:6 | x2 | main.rs:25:13:25:14 | x2 | main.rs:29:10:29:11 | x2 |
202202
| main.rs:29:5:29:6 | x2 | main.rs:25:13:25:14 | x2 | main.rs:30:15:30:16 | x2 |
203203
| main.rs:34:13:34:13 | x | main.rs:34:13:34:13 | x | main.rs:35:20:35:20 | x |
204204
| main.rs:36:5:36:5 | x | main.rs:34:13:34:13 | x | main.rs:37:20:37:20 | x |
@@ -285,14 +285,14 @@ read
285285
| main.rs:367:9:367:10 | c1 | main.rs:367:9:367:10 | c1 | main.rs:372:15:372:16 | c1 |
286286
| main.rs:375:20:375:55 | SSA phi(a9) | main.rs:375:20:375:55 | a9 | main.rs:377:15:377:16 | a9 |
287287
| main.rs:382:13:382:15 | a10 | main.rs:382:13:382:15 | a10 | main.rs:386:15:386:17 | a10 |
288+
| main.rs:382:13:382:15 | a10 | main.rs:382:13:382:15 | a10 | main.rs:395:9:395:11 | a10 |
288289
| main.rs:383:13:383:14 | b4 | main.rs:383:13:383:14 | b4 | main.rs:387:15:387:16 | b4 |
290+
| main.rs:383:13:383:14 | b4 | main.rs:383:13:383:14 | b4 | main.rs:396:9:396:10 | b4 |
289291
| main.rs:384:13:384:14 | c2 | main.rs:384:13:384:14 | c2 | main.rs:388:15:388:16 | c2 |
290-
| main.rs:391:9:391:10 | c2 | main.rs:384:13:384:14 | c2 | main.rs:397:9:397:10 | c2 |
292+
| main.rs:384:13:384:14 | c2 | main.rs:384:13:384:14 | c2 | main.rs:397:9:397:10 | c2 |
291293
| main.rs:391:9:391:10 | c2 | main.rs:384:13:384:14 | c2 | main.rs:401:15:401:16 | c2 |
292-
| main.rs:392:9:392:10 | b4 | main.rs:383:13:383:14 | b4 | main.rs:396:9:396:10 | b4 |
293294
| main.rs:392:9:392:10 | b4 | main.rs:383:13:383:14 | b4 | main.rs:400:15:400:16 | b4 |
294295
| main.rs:392:9:392:10 | b4 | main.rs:383:13:383:14 | b4 | main.rs:414:15:414:16 | b4 |
295-
| main.rs:393:9:393:11 | a10 | main.rs:382:13:382:15 | a10 | main.rs:395:9:395:11 | a10 |
296296
| main.rs:393:9:393:11 | a10 | main.rs:382:13:382:15 | a10 | main.rs:399:15:399:17 | a10 |
297297
| main.rs:393:9:393:11 | a10 | main.rs:382:13:382:15 | a10 | main.rs:413:15:413:17 | a10 |
298298
| main.rs:405:13:405:15 | a10 | main.rs:405:13:405:15 | a10 | main.rs:408:23:408:25 | a10 |
@@ -401,7 +401,7 @@ firstRead
401401
| main.rs:20:9:20:10 | x1 | main.rs:20:9:20:10 | x1 | main.rs:21:15:21:16 | x1 |
402402
| main.rs:25:13:25:14 | x2 | main.rs:25:13:25:14 | x2 | main.rs:26:15:26:16 | x2 |
403403
| main.rs:27:5:27:6 | x2 | main.rs:25:13:25:14 | x2 | main.rs:28:15:28:16 | x2 |
404-
| main.rs:29:5:29:6 | x2 | main.rs:25:13:25:14 | x2 | main.rs:29:10:29:11 | x2 |
404+
| main.rs:29:5:29:6 | x2 | main.rs:25:13:25:14 | x2 | main.rs:30:15:30:16 | x2 |
405405
| main.rs:34:13:34:13 | x | main.rs:34:13:34:13 | x | main.rs:35:20:35:20 | x |
406406
| main.rs:36:5:36:5 | x | main.rs:34:13:34:13 | x | main.rs:37:20:37:20 | x |
407407
| main.rs:41:9:41:10 | x3 | main.rs:41:9:41:10 | x3 | main.rs:42:15:42:16 | x3 |
@@ -473,9 +473,9 @@ firstRead
473473
| main.rs:382:13:382:15 | a10 | main.rs:382:13:382:15 | a10 | main.rs:386:15:386:17 | a10 |
474474
| main.rs:383:13:383:14 | b4 | main.rs:383:13:383:14 | b4 | main.rs:387:15:387:16 | b4 |
475475
| main.rs:384:13:384:14 | c2 | main.rs:384:13:384:14 | c2 | main.rs:388:15:388:16 | c2 |
476-
| main.rs:391:9:391:10 | c2 | main.rs:384:13:384:14 | c2 | main.rs:397:9:397:10 | c2 |
477-
| main.rs:392:9:392:10 | b4 | main.rs:383:13:383:14 | b4 | main.rs:396:9:396:10 | b4 |
478-
| main.rs:393:9:393:11 | a10 | main.rs:382:13:382:15 | a10 | main.rs:395:9:395:11 | a10 |
476+
| main.rs:391:9:391:10 | c2 | main.rs:384:13:384:14 | c2 | main.rs:401:15:401:16 | c2 |
477+
| main.rs:392:9:392:10 | b4 | main.rs:383:13:383:14 | b4 | main.rs:400:15:400:16 | b4 |
478+
| main.rs:393:9:393:11 | a10 | main.rs:382:13:382:15 | a10 | main.rs:399:15:399:17 | a10 |
479479
| main.rs:405:13:405:15 | a10 | main.rs:405:13:405:15 | a10 | main.rs:408:23:408:25 | a10 |
480480
| main.rs:406:13:406:14 | b4 | main.rs:406:13:406:14 | b4 | main.rs:409:23:409:24 | b4 |
481481
| main.rs:418:9:418:23 | example_closure | main.rs:418:9:418:23 | example_closure | main.rs:422:9:422:23 | example_closure |
@@ -556,7 +556,7 @@ firstRead
556556
| main.rs:726:20:726:20 | b | main.rs:726:20:726:20 | b | main.rs:728:20:728:20 | b |
557557
| main.rs:732:5:732:13 | <captured exit> x | main.rs:725:13:725:13 | x | main.rs:733:15:733:15 | x |
558558
adjacentReads
559-
| main.rs:29:5:29:6 | x2 | main.rs:25:13:25:14 | x2 | main.rs:29:10:29:11 | x2 | main.rs:30:15:30:16 | x2 |
559+
| 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 |
560560
| 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 |
561561
| main.rs:49:9:49:10 | x4 | main.rs:49:9:49:10 | x4 | main.rs:50:15:50:16 | x4 | main.rs:55:15:55:16 | x4 |
562562
| main.rs:100:9:100:9 | x | main.rs:100:9:100:9 | x | main.rs:102:7:102:7 | x | main.rs:105:13:105:13 | x |
@@ -574,10 +574,10 @@ adjacentReads
574574
| main.rs:334:9:334:9 | x | main.rs:334:9:334:9 | x | main.rs:335:11:335:11 | x | main.rs:343:15:343:15 | x |
575575
| main.rs:348:9:348:9 | x | main.rs:348:9:348:9 | x | main.rs:350:7:350:7 | x | main.rs:355:7:355:7 | x |
576576
| main.rs:348:9:348:9 | x | main.rs:348:9:348:9 | x | main.rs:355:7:355:7 | x | main.rs:359:19:359:19 | x |
577-
| main.rs:391:9:391:10 | c2 | main.rs:384:13:384:14 | c2 | main.rs:397:9:397:10 | c2 | main.rs:401:15:401:16 | c2 |
578-
| main.rs:392:9:392:10 | b4 | main.rs:383:13:383:14 | b4 | main.rs:396:9:396:10 | b4 | main.rs:400:15:400:16 | b4 |
577+
| main.rs:382:13:382:15 | a10 | main.rs:382:13:382:15 | a10 | main.rs:386:15:386:17 | a10 | main.rs:395:9:395:11 | a10 |
578+
| main.rs:383:13:383:14 | b4 | main.rs:383:13:383:14 | b4 | main.rs:387:15:387:16 | b4 | main.rs:396:9:396:10 | b4 |
579+
| main.rs:384:13:384:14 | c2 | main.rs:384:13:384:14 | c2 | main.rs:388:15:388:16 | c2 | main.rs:397:9:397:10 | c2 |
579580
| main.rs:392:9:392:10 | b4 | main.rs:383:13:383:14 | b4 | main.rs:400:15:400:16 | b4 | main.rs:414:15:414:16 | b4 |
580-
| main.rs:393:9:393:11 | a10 | main.rs:382:13:382:15 | a10 | main.rs:395:9:395:11 | a10 | main.rs:399:15:399:17 | a10 |
581581
| main.rs:393:9:393:11 | a10 | main.rs:382:13:382:15 | a10 | main.rs:399:15:399:17 | a10 | main.rs:413:15:413:17 | a10 |
582582
| main.rs:436:9:436:9 | f | main.rs:436:9:436:9 | f | main.rs:439:15:439:15 | f | main.rs:446:15:446:15 | f |
583583
| main.rs:477:5:477:5 | a | main.rs:476:13:476:13 | a | main.rs:478:15:478:15 | a | main.rs:479:11:479:11 | a |

0 commit comments

Comments
 (0)