Skip to content

Commit 1fb9835

Browse files
hvitvedhvitved
authored
Merge pull request github#17557 from hvitved/rust/cfg-improvements
Rust: CFG improvements
2 parents 590e93d + 6e493f2 commit 1fb9835

File tree

9 files changed

+124
-44
lines changed

9 files changed

+124
-44
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/controlflow/ControlFlowGraph.qll

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,20 @@ private import BasicBlocks
99

1010
final class CfgScope = Scope::CfgScope;
1111

12+
final class SuccessorType = SuccessorTypeImpl;
13+
14+
final class NormalSuccessor = NormalSuccessorImpl;
15+
16+
final class ConditionalSuccessor = ConditionalSuccessorImpl;
17+
18+
final class BooleanSuccessor = BooleanSuccessorImpl;
19+
20+
final class MatchSuccessor = MatchSuccessorImpl;
21+
22+
final class LoopJumpSuccessor = LoopJumpSuccessorImpl;
23+
24+
final class ReturnSuccessor = ReturnSuccessorImpl;
25+
1226
/**
1327
* A control flow node.
1428
*

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ private import codeql.rust.controlflow.ControlFlowGraph
33
private import rust
44
private import SuccessorType
55

6-
private newtype TCompletion =
6+
newtype TCompletion =
77
TSimpleCompletion() or
88
TBooleanCompletion(Boolean b) or
99
TMatchCompletion(Boolean isMatch) or

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

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
11
private import rust
22
import codeql.controlflow.Cfg
33
import Completion
4-
import codeql.controlflow.Cfg
5-
private import SuccessorType as ST
64
private import Scope as Scope
5+
private import codeql.rust.controlflow.ControlFlowGraph as Cfg
76

8-
module CfgInput implements InputSig<Location> {
7+
private module CfgInput implements InputSig<Location> {
98
private import rust as Rust
109
private import Completion as C
1110
private import Splitting as S
@@ -29,21 +28,21 @@ module CfgInput implements InputSig<Location> {
2928

3029
class Split = S::Split;
3130

32-
class SuccessorType = ST::SuccessorType;
31+
class SuccessorType = Cfg::SuccessorType;
3332

3433
/** Gets a successor type that matches completion `c`. */
3534
SuccessorType getAMatchingSuccessorType(Completion c) { result = c.getAMatchingSuccessorType() }
3635

3736
/**
3837
* Hold if `c` represents simple (normal) evaluation of a statement or an expression.
3938
*/
40-
predicate successorTypeIsSimple(SuccessorType t) { t instanceof ST::NormalSuccessor }
39+
predicate successorTypeIsSimple(SuccessorType t) { t instanceof Cfg::NormalSuccessor }
4140

4241
/** Holds if `t` is an abnormal exit type out of a CFG scope. */
4342
predicate isAbnormalExitType(SuccessorType t) { none() }
4443

4544
/** Hold if `t` represents a conditional successor type. */
46-
predicate successorTypeIsCondition(SuccessorType t) { t instanceof ST::BooleanSuccessor }
45+
predicate successorTypeIsCondition(SuccessorType t) { t instanceof Cfg::BooleanSuccessor }
4746

4847
/** Gets the maximum number of splits allowed for a given node. */
4948
int maxSplits() { result = 0 }
@@ -357,24 +356,24 @@ class MatchArmTree extends ControlFlowTree instanceof MatchArm {
357356

358357
class MatchExprTree extends PostOrderTree instanceof MatchExpr {
359358
override predicate propagatesAbnormal(AstNode child) {
360-
child = [super.getExpr(), super.getMatchArmList().getAnArm().getExpr()]
359+
child = [super.getExpr(), super.getAnArm().getExpr()]
361360
}
362361

363362
override predicate first(AstNode node) { first(super.getExpr(), node) }
364363

365364
override predicate succ(AstNode pred, AstNode succ, Completion c) {
366365
// Edge from the scrutinee to the first arm.
367-
last(super.getExpr(), pred, c) and succ = super.getMatchArmList().getArm(0).getPat()
366+
last(super.getExpr(), pred, c) and succ = super.getArm(0).getPat()
368367
or
369368
// Edge from a failed match/guard in one arm to the beginning of the next arm.
370369
exists(int i |
371-
last(super.getMatchArmList().getArm(i), pred, c) and
372-
first(super.getMatchArmList().getArm(i + 1), succ) and
370+
last(super.getArm(i), pred, c) and
371+
first(super.getArm(i + 1), succ) and
373372
c.(ConditionalCompletion).failed()
374373
)
375374
or
376375
// Edge from the end of each arm to the match expression.
377-
last(super.getMatchArmList().getArm(_), pred, c) and succ = this and completionIsNormal(c)
376+
last(super.getArm(_).getExpr(), pred, c) and succ = this and completionIsNormal(c)
378377
}
379378
}
380379

@@ -387,6 +386,10 @@ class MethodCallExprTree extends StandardPostOrderTree instanceof MethodCallExpr
387386

388387
class OffsetOfExprTree extends LeafTree instanceof OffsetOfExpr { }
389388

389+
class ParenExprTree extends StandardPostOrderTree, ParenExpr {
390+
override ControlFlowTree getChildNode(int i) { i = 0 and result = super.getExpr() }
391+
}
392+
390393
// This covers all patterns as they all extend `Pat`
391394
class PatExprTree extends LeafTree instanceof Pat { }
392395

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

Lines changed: 22 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
private import rust
22
private import codeql.util.Boolean
3+
private import Completion
34

45
newtype TLoopJumpType =
56
TContinueJump() or
@@ -14,45 +15,43 @@ newtype TSuccessorType =
1415
TSuccessorSuccessor() or
1516
TBooleanSuccessor(Boolean b) or
1617
TMatchSuccessor(Boolean b) or
17-
TLoopSuccessor(TLoopJumpType kind, TLabelType label) or
18+
TLoopSuccessor(TLoopJumpType kind, TLabelType label) { exists(TLoopCompletion(kind, label)) } or
1819
TReturnSuccessor()
1920

2021
/** The type of a control flow successor. */
21-
abstract private class SuccessorTypeImpl extends TSuccessorType {
22+
abstract class SuccessorTypeImpl extends TSuccessorType {
2223
/** Gets a textual representation of successor type. */
2324
abstract string toString();
2425
}
2526

26-
final class SuccessorType = SuccessorTypeImpl;
27-
2827
/** A normal control flow successor. */
29-
final class NormalSuccessor extends SuccessorTypeImpl, TSuccessorSuccessor {
30-
final override string toString() { result = "successor" }
28+
class NormalSuccessorImpl extends SuccessorTypeImpl, TSuccessorSuccessor {
29+
override string toString() { result = "successor" }
3130
}
3231

3332
/** A conditional control flow successor. */
34-
abstract private class ConditionalSuccessor extends SuccessorTypeImpl {
33+
abstract class ConditionalSuccessorImpl extends SuccessorTypeImpl {
3534
boolean value;
3635

3736
bindingset[value]
38-
ConditionalSuccessor() { any() }
37+
ConditionalSuccessorImpl() { exists(value) }
3938

4039
/** Gets the Boolean value of this successor. */
41-
final boolean getValue() { result = value }
40+
boolean getValue() { result = value }
4241
}
4342

4443
/** A Boolean control flow successor for a boolean conditon. */
45-
final class BooleanSuccessor extends ConditionalSuccessor, TBooleanSuccessor {
46-
BooleanSuccessor() { this = TBooleanSuccessor(value) }
44+
class BooleanSuccessorImpl extends ConditionalSuccessorImpl, TBooleanSuccessor {
45+
BooleanSuccessorImpl() { this = TBooleanSuccessor(value) }
4746

4847
override string toString() { result = this.getValue().toString() }
4948
}
5049

5150
/**
5251
* A control flow successor of a pattern match.
5352
*/
54-
final class MatchSuccessor extends ConditionalSuccessor, TMatchSuccessor {
55-
MatchSuccessor() { this = TMatchSuccessor(value) }
53+
class MatchSuccessorImpl extends ConditionalSuccessorImpl, TMatchSuccessor {
54+
MatchSuccessorImpl() { this = TMatchSuccessor(value) }
5655

5756
override string toString() {
5857
if this.getValue() = true then result = "match" else result = "no-match"
@@ -62,20 +61,20 @@ final class MatchSuccessor extends ConditionalSuccessor, TMatchSuccessor {
6261
/**
6362
* A control flow successor of a loop control flow expression, `continue` or `break`.
6463
*/
65-
final class LoopJumpSuccessor extends SuccessorTypeImpl, TLoopSuccessor {
66-
final private TLoopJumpType getKind() { this = TLoopSuccessor(result, _) }
64+
class LoopJumpSuccessorImpl extends SuccessorTypeImpl, TLoopSuccessor {
65+
private TLoopJumpType getKind() { this = TLoopSuccessor(result, _) }
6766

68-
final private TLabelType getLabelType() { this = TLoopSuccessor(_, result) }
67+
private TLabelType getLabelType() { this = TLoopSuccessor(_, result) }
6968

70-
final predicate hasLabel() { this.getLabelType() = TLabel(_) }
69+
predicate hasLabel() { this.getLabelType() = TLabel(_) }
7170

72-
final string getLabelName() { this = TLoopSuccessor(_, TLabel(result)) }
71+
string getLabelName() { this = TLoopSuccessor(_, TLabel(result)) }
7372

74-
final predicate isContinue() { this.getKind() = TContinueJump() }
73+
predicate isContinue() { this.getKind() = TContinueJump() }
7574

76-
final predicate isBreak() { this.getKind() = TBreakJump() }
75+
predicate isBreak() { this.getKind() = TBreakJump() }
7776

78-
final override string toString() {
77+
override string toString() {
7978
exists(string kind, string label |
8079
(if this.isContinue() then kind = "continue" else kind = "break") and
8180
(if this.hasLabel() then label = "(" + this.getLabelName() + ")" else label = "") and
@@ -87,6 +86,6 @@ final class LoopJumpSuccessor extends SuccessorTypeImpl, TLoopSuccessor {
8786
/**
8887
* A `return` control flow successor.
8988
*/
90-
final class ReturnSuccessor extends SuccessorTypeImpl, TReturnSuccessor {
91-
final override string toString() { result = "return" }
89+
class ReturnSuccessorImpl extends SuccessorTypeImpl, TReturnSuccessor {
90+
override string toString() { result = "return" }
9291
}

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

Lines changed: 17 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 `MatchExpr`.
43
*
@@ -12,6 +11,7 @@ private import codeql.rust.elements.internal.generated.MatchExpr
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 match expression. For example:
1717
* ```rust
@@ -27,5 +27,20 @@ module Impl {
2727
* }
2828
* ```
2929
*/
30-
class MatchExpr extends Generated::MatchExpr { }
30+
class MatchExpr extends Generated::MatchExpr {
31+
/**
32+
* Gets the `index`th arm of this match expression.
33+
*/
34+
MatchArm getArm(int index) { result = this.getMatchArmList().getArm(index) }
35+
36+
/**
37+
* Gets any of the arms of this match expression.
38+
*/
39+
MatchArm getAnArm() { result = this.getArm(_) }
40+
41+
/**
42+
* Gets the number of arms of this match expression.
43+
*/
44+
int getNumberOfArms() { result = this.getMatchArmList().getNumberOfArms() }
45+
}
3146
}

‎rust/ql/test/library-tests/controlflow/Cfg.expected

Lines changed: 48 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,47 @@
149149
| test.rs:79:28:81:9 | BlockExpr | test.rs:79:9:81:9 | IfExpr | |
150150
| test.rs:80:13:80:13 | PathExpr | test.rs:79:28:81:9 | BlockExpr | |
151151
| test.rs:82:9:82:9 | LiteralExpr | test.rs:78:43:83:5 | BlockExpr | |
152+
| test.rs:85:5:91:5 | enter test_nested_if | test.rs:86:16:86:16 | PathExpr | |
153+
| test.rs:85:5:91:5 | exit test_nested_if (normal) | test.rs:85:5:91:5 | exit test_nested_if | |
154+
| test.rs:85:38:91:5 | BlockExpr | test.rs:85:5:91:5 | exit test_nested_if (normal) | |
155+
| test.rs:86:9:90:9 | IfExpr | test.rs:85:38:91:5 | BlockExpr | |
156+
| test.rs:86:12:86:48 | ParenExpr | test.rs:87:13:87:13 | LiteralExpr | true |
157+
| test.rs:86:12:86:48 | ParenExpr | test.rs:89:13:89:13 | LiteralExpr | false |
158+
| test.rs:86:13:86:47 | IfExpr | test.rs:86:12:86:48 | ParenExpr | |
159+
| test.rs:86:16:86:16 | PathExpr | test.rs:86:20:86:20 | LiteralExpr | |
160+
| test.rs:86:16:86:20 | BinaryExpr | test.rs:86:24:86:24 | PathExpr | true |
161+
| test.rs:86:16:86:20 | BinaryExpr | test.rs:86:41:86:41 | PathExpr | false |
162+
| test.rs:86:20:86:20 | LiteralExpr | test.rs:86:16:86:20 | BinaryExpr | |
163+
| test.rs:86:22:86:32 | BlockExpr | test.rs:86:13:86:47 | IfExpr | |
164+
| test.rs:86:24:86:24 | PathExpr | test.rs:86:29:86:30 | LiteralExpr | |
165+
| test.rs:86:24:86:30 | BinaryExpr | test.rs:86:22:86:32 | BlockExpr | |
166+
| test.rs:86:28:86:30 | PrefixExpr | test.rs:86:24:86:30 | BinaryExpr | |
167+
| test.rs:86:29:86:30 | LiteralExpr | test.rs:86:28:86:30 | PrefixExpr | |
168+
| test.rs:86:39:86:47 | BlockExpr | test.rs:86:13:86:47 | IfExpr | |
169+
| test.rs:86:41:86:41 | PathExpr | test.rs:86:45:86:46 | LiteralExpr | |
170+
| test.rs:86:41:86:46 | BinaryExpr | test.rs:86:39:86:47 | BlockExpr | |
171+
| test.rs:86:45:86:46 | LiteralExpr | test.rs:86:41:86:46 | BinaryExpr | |
172+
| test.rs:86:50:88:9 | BlockExpr | test.rs:86:9:90:9 | IfExpr | |
173+
| test.rs:87:13:87:13 | LiteralExpr | test.rs:86:50:88:9 | BlockExpr | |
174+
| test.rs:88:16:90:9 | BlockExpr | test.rs:86:9:90:9 | IfExpr | |
175+
| test.rs:89:13:89:13 | LiteralExpr | test.rs:88:16:90:9 | BlockExpr | |
176+
| test.rs:93:5:99:5 | enter test_nested_if_match | test.rs:94:19:94:19 | PathExpr | |
177+
| test.rs:93:5:99:5 | exit test_nested_if_match (normal) | test.rs:93:5:99:5 | exit test_nested_if_match | |
178+
| test.rs:93:44:99:5 | BlockExpr | test.rs:93:5:99:5 | exit test_nested_if_match (normal) | |
179+
| test.rs:94:9:98:9 | IfExpr | test.rs:93:44:99:5 | BlockExpr | |
180+
| test.rs:94:12:94:46 | ParenExpr | test.rs:95:13:95:13 | LiteralExpr | true |
181+
| test.rs:94:12:94:46 | ParenExpr | test.rs:97:13:97:13 | LiteralExpr | false |
182+
| test.rs:94:13:94:45 | MatchExpr | test.rs:94:12:94:46 | ParenExpr | |
183+
| test.rs:94:19:94:19 | PathExpr | test.rs:94:23:94:23 | LiteralPat | |
184+
| test.rs:94:23:94:23 | LiteralPat | test.rs:94:28:94:31 | LiteralExpr | match |
185+
| test.rs:94:23:94:23 | LiteralPat | test.rs:94:34:94:34 | WildcardPat | no-match |
186+
| test.rs:94:28:94:31 | LiteralExpr | test.rs:94:13:94:45 | MatchExpr | |
187+
| test.rs:94:34:94:34 | WildcardPat | test.rs:94:39:94:43 | LiteralExpr | match |
188+
| test.rs:94:39:94:43 | LiteralExpr | test.rs:94:13:94:45 | MatchExpr | |
189+
| test.rs:94:48:96:9 | BlockExpr | test.rs:94:9:98:9 | IfExpr | |
190+
| test.rs:95:13:95:13 | LiteralExpr | test.rs:94:48:96:9 | BlockExpr | |
191+
| test.rs:96:16:98:9 | BlockExpr | test.rs:94:9:98:9 | IfExpr | |
192+
| test.rs:97:13:97:13 | LiteralExpr | test.rs:96:16:98:9 | BlockExpr | |
152193
| test.rs:105:5:108:5 | enter test_and_operator | test.rs:106:9:106:28 | LetStmt | |
153194
| test.rs:105:5:108:5 | exit test_and_operator (normal) | test.rs:105:5:108:5 | exit test_and_operator | |
154195
| test.rs:105:61:108:5 | BlockExpr | test.rs:105:5:108:5 | exit test_and_operator (normal) | |
@@ -181,24 +222,28 @@
181222
| test.rs:116:9:116:36 | LetStmt | test.rs:116:17:116:35 | BinaryExpr | |
182223
| test.rs:116:13:116:13 | IdentPat | test.rs:117:9:117:9 | PathExpr | match, no-match |
183224
| test.rs:116:17:116:17 | PathExpr | test.rs:116:13:116:13 | IdentPat | true |
225+
| test.rs:116:17:116:17 | PathExpr | test.rs:116:23:116:23 | PathExpr | false |
184226
| test.rs:116:17:116:30 | BinaryExpr | test.rs:116:17:116:17 | PathExpr | |
185227
| test.rs:116:17:116:35 | BinaryExpr | test.rs:116:17:116:30 | BinaryExpr | |
228+
| test.rs:116:22:116:30 | ParenExpr | test.rs:116:13:116:13 | IdentPat | true |
229+
| test.rs:116:22:116:30 | ParenExpr | test.rs:116:35:116:35 | PathExpr | false |
230+
| test.rs:116:23:116:23 | PathExpr | test.rs:116:28:116:29 | LiteralExpr | |
231+
| test.rs:116:23:116:29 | BinaryExpr | test.rs:116:22:116:30 | ParenExpr | |
232+
| test.rs:116:28:116:29 | LiteralExpr | test.rs:116:23:116:29 | BinaryExpr | |
233+
| test.rs:116:35:116:35 | PathExpr | test.rs:116:13:116:13 | IdentPat | |
186234
| test.rs:117:9:117:9 | PathExpr | test.rs:115:61:118:5 | BlockExpr | |
187235
| test.rs:122:1:128:1 | enter test_match | test.rs:123:11:123:21 | PathExpr | |
188236
| test.rs:122:1:128:1 | exit test_match (normal) | test.rs:122:1:128:1 | exit test_match | |
189237
| test.rs:122:44:128:1 | BlockExpr | test.rs:122:1:128:1 | exit test_match (normal) | |
190238
| test.rs:123:5:127:5 | MatchExpr | test.rs:122:44:128:1 | BlockExpr | |
191239
| test.rs:123:11:123:21 | PathExpr | test.rs:124:9:124:23 | TupleStructPat | |
192-
| test.rs:124:9:124:23 | TupleStructPat | test.rs:123:5:127:5 | MatchExpr | no-match |
193240
| test.rs:124:9:124:23 | TupleStructPat | test.rs:124:28:124:28 | PathExpr | match |
194241
| test.rs:124:9:124:23 | TupleStructPat | test.rs:125:9:125:23 | TupleStructPat | no-match |
195242
| test.rs:124:28:124:28 | PathExpr | test.rs:124:32:124:33 | LiteralExpr | |
196243
| test.rs:124:32:124:33 | LiteralExpr | test.rs:124:28:124:33 | BinaryExpr | |
197-
| test.rs:125:9:125:23 | TupleStructPat | test.rs:123:5:127:5 | MatchExpr | no-match |
198244
| test.rs:125:9:125:23 | TupleStructPat | test.rs:125:28:125:28 | PathExpr | match |
199245
| test.rs:125:9:125:23 | TupleStructPat | test.rs:126:9:126:20 | PathPat | no-match |
200246
| test.rs:125:28:125:28 | PathExpr | test.rs:123:5:127:5 | MatchExpr | |
201-
| test.rs:126:9:126:20 | PathPat | test.rs:123:5:127:5 | MatchExpr | no-match |
202247
| test.rs:126:9:126:20 | PathPat | test.rs:126:25:126:25 | LiteralExpr | match |
203248
| test.rs:126:25:126:25 | LiteralExpr | test.rs:123:5:127:5 | MatchExpr | |
204249
| test.rs:131:5:136:5 | enter test_infinite_loop | test.rs:132:9:134:9 | ExprStmt | |

‎shared/controlflow/codeql/controlflow/Cfg.qll

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1273,8 +1273,14 @@ module Make<LocationSig Location, InputSig<Location> Input> {
12731273
string getOrderDisambiguation() { result = "" }
12741274
}
12751275

1276-
import TestOutput<RelevantNode>
1277-
import Mermaid
1276+
private module Output = TestOutput<RelevantNode>;
1277+
1278+
import Output::Mermaid
1279+
1280+
query predicate edges(RelevantNode pred, RelevantNode succ, string attr, string val) {
1281+
attr = "semmle.label" and
1282+
Output::edges(pred, succ, val)
1283+
}
12781284
}
12791285

12801286
/** Provides a set of consistency queries. */

0 commit comments

Comments
 (0)