Skip to content

Commit f24fa40

Browse files
hvitvedhvitved
committed
Adjust CFG
1 parent 2b4217b commit f24fa40

File tree

6 files changed

+88
-31
lines changed

6 files changed

+88
-31
lines changed

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

Lines changed: 38 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -432,8 +432,36 @@ module ExprNodes {
432432
final ExprCfgNode getBody() { e.hasCfgChild(e.getBody(), this, result) }
433433
}
434434

435-
private class WhenClauseChildMapping extends NonExprChildMapping, WhenClause {
436-
override predicate relevantChild(AstNode e) { e = [this.getBody(), this.getAPattern()] }
435+
// `when` clauses need special treatment, since they are neither pre-order
436+
// nor post-order
437+
private class WhenClauseChildMapping extends WhenClause {
438+
predicate patternReachesBasicBlock(int i, CfgNode cfnPattern, BasicBlock bb) {
439+
exists(Expr pattern |
440+
pattern = this.getPattern(i) and
441+
cfnPattern.getNode() = pattern and
442+
bb.getANode() = cfnPattern
443+
)
444+
or
445+
exists(BasicBlock mid |
446+
this.patternReachesBasicBlock(i, cfnPattern, mid) and
447+
bb = mid.getASuccessor() and
448+
not mid.getANode().getNode() = this
449+
)
450+
}
451+
452+
predicate bodyReachesBasicBlock(CfgNode cfnBody, BasicBlock bb) {
453+
exists(Stmt body |
454+
body = this.getBody() and
455+
cfnBody.getNode() = body and
456+
bb.getANode() = cfnBody
457+
)
458+
or
459+
exists(BasicBlock mid |
460+
this.bodyReachesBasicBlock(cfnBody, mid) and
461+
bb = mid.getAPredecessor() and
462+
not mid.getANode().getNode() = this
463+
)
464+
}
437465
}
438466

439467
/** A control-flow node that wraps a `WhenClause` AST expression. */
@@ -443,10 +471,16 @@ module ExprNodes {
443471
override WhenClauseChildMapping e;
444472

445473
/** Gets the body of this `when`-clause. */
446-
final ExprCfgNode getBody() { e.hasCfgChild(e.getBody(), this, result) }
474+
final ExprCfgNode getBody() {
475+
result.getNode() = desugar(e.getBody()) and
476+
e.bodyReachesBasicBlock(result, this.getBasicBlock())
477+
}
447478

448479
/** Gets the `i`th pattern this `when`-clause. */
449-
final ExprCfgNode getPattern(int i) { result.getExpr() = e.getPattern(i) }
480+
final ExprCfgNode getPattern(int i) {
481+
result.getNode() = desugar(e.getPattern(i)) and
482+
e.patternReachesBasicBlock(i, result, this.getBasicBlock())
483+
}
450484
}
451485

452486
/** A control-flow node that wraps a `CasePattern`. */

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

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -211,8 +211,11 @@ private predicate inBooleanContext(AstNode n) {
211211
or
212212
exists(CaseExpr c, WhenClause w |
213213
not exists(c.getValue()) and
214-
c.getABranch() = w and
214+
c.getABranch() = w
215+
|
215216
w.getPattern(_) = n
217+
or
218+
w = n
216219
)
217220
}
218221

@@ -233,12 +236,11 @@ private predicate inMatchingContext(AstNode n) {
233236
or
234237
exists(CaseExpr c, WhenClause w |
235238
exists(c.getValue()) and
236-
(
237-
c.getABranch() = w and
238-
w.getPattern(_) = n
239-
or
240-
w = n
241-
)
239+
c.getABranch() = w
240+
|
241+
w.getPattern(_) = n
242+
or
243+
w = n
242244
)
243245
or
244246
n instanceof CasePattern

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -402,6 +402,7 @@ module Trees {
402402
exists(int i, WhenTree branch | branch = this.getBranch(i) |
403403
pred = branch and
404404
first(this.getBranch(i + 1), succ) and
405+
c.isValidFor(branch) and
405406
c.(ConditionalCompletion).getValue() = false
406407
)
407408
or
@@ -1411,6 +1412,7 @@ module Trees {
14111412

14121413
final override predicate last(AstNode last, Completion c) {
14131414
last = this and
1415+
c.isValidFor(this) and
14141416
c.(ConditionalCompletion).getValue() = false
14151417
or
14161418
last(this.getBody(), last, c) and

‎ruby/ql/lib/codeql/ruby/controlflow/internal/Splitting.qll

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,12 +85,9 @@ private module ConditionalCompletionSplitting {
8585
or
8686
last(succ.(ConditionalExpr).getBranch(_), pred, c) and
8787
completion = c
88-
or
89-
last(succ.(WhenClause).getAPattern(), pred, c) and completion = c
9088
)
9189
or
9290
succ(pred, succ, c) and
93-
succ(succ, _, completion) and
9491
succ instanceof WhenClause and
9592
completion = c
9693
}

‎ruby/ql/test/library-tests/controlflow/graph/Cfg.expected

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1916,16 +1916,15 @@ cfg.rb:
19161916
# 48| [false] when ...
19171917
#-----| false -> b
19181918

1919-
# 48| when ...
1920-
#-----| match -> self
1921-
#-----| no-match -> b
1919+
# 48| [true] when ...
1920+
#-----| true -> self
19221921

19231922
# 48| b
19241923
#-----| -> 1
19251924

19261925
# 48| ... == ...
19271926
#-----| false -> [false] when ...
1928-
#-----| true -> when ...
1927+
#-----| true -> [true] when ...
19291928

19301929
# 48| 1
19311930
#-----| -> ... == ...
@@ -1948,15 +1947,14 @@ cfg.rb:
19481947
# 49| [false] when ...
19491948
#-----| false -> case ...
19501949

1951-
# 49| when ...
1952-
#-----| no-match -> case ...
1953-
#-----| match -> self
1950+
# 49| [true] when ...
1951+
#-----| true -> self
19541952

19551953
# 49| b
19561954
#-----| -> 0
19571955

19581956
# 49| ... == ...
1959-
#-----| true -> when ...
1957+
#-----| true -> [true] when ...
19601958
#-----| false -> b
19611959

19621960
# 49| 0
@@ -1967,7 +1965,7 @@ cfg.rb:
19671965

19681966
# 49| ... > ...
19691967
#-----| false -> [false] when ...
1970-
#-----| true -> when ...
1968+
#-----| true -> [true] when ...
19711969

19721970
# 49| 1
19731971
#-----| -> ... > ...

‎ruby/ql/test/library-tests/dataflow/barrier-guards/barrier-guards.expected

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -262,21 +262,45 @@ controls
262262
| barrier-guards.rb:227:4:227:21 | [true] ... and ... | barrier-guards.rb:228:5:228:7 | self | true |
263263
| barrier-guards.rb:227:21:227:21 | call to y | barrier-guards.rb:227:4:227:21 | [true] ... and ... | true |
264264
| barrier-guards.rb:227:21:227:21 | call to y | barrier-guards.rb:228:5:228:7 | self | true |
265-
| barrier-guards.rb:232:1:233:19 | when ... | barrier-guards.rb:233:5:233:7 | foo | match |
265+
| barrier-guards.rb:232:1:233:19 | [true] when ... | barrier-guards.rb:233:5:233:7 | foo | true |
266266
| barrier-guards.rb:232:6:232:17 | ... == ... | barrier-guards.rb:232:1:233:19 | [false] when ... | false |
267-
| barrier-guards.rb:232:6:232:17 | ... == ... | barrier-guards.rb:232:1:233:19 | when ... | true |
267+
| barrier-guards.rb:232:6:232:17 | ... == ... | barrier-guards.rb:232:1:233:19 | [true] when ... | true |
268268
| barrier-guards.rb:232:6:232:17 | ... == ... | barrier-guards.rb:233:5:233:7 | foo | true |
269-
| barrier-guards.rb:237:1:237:38 | when ... | barrier-guards.rb:237:24:237:26 | foo | match |
269+
| barrier-guards.rb:237:1:237:38 | [false] when ... | barrier-guards.rb:238:1:238:26 | [false] when ... | false |
270+
| barrier-guards.rb:237:1:237:38 | [false] when ... | barrier-guards.rb:238:1:238:26 | [true] when ... | false |
271+
| barrier-guards.rb:237:1:237:38 | [false] when ... | barrier-guards.rb:238:6:238:8 | self | false |
272+
| barrier-guards.rb:237:1:237:38 | [false] when ... | barrier-guards.rb:238:24:238:26 | foo | false |
273+
| barrier-guards.rb:237:1:237:38 | [false] when ... | barrier-guards.rb:239:1:239:22 | [false] when ... | false |
274+
| barrier-guards.rb:237:1:237:38 | [false] when ... | barrier-guards.rb:239:1:239:22 | [true] when ... | false |
275+
| barrier-guards.rb:237:1:237:38 | [false] when ... | barrier-guards.rb:239:6:239:8 | foo | false |
276+
| barrier-guards.rb:237:1:237:38 | [false] when ... | barrier-guards.rb:239:20:239:22 | foo | false |
277+
| barrier-guards.rb:237:1:237:38 | [true] when ... | barrier-guards.rb:237:24:237:26 | foo | true |
270278
| barrier-guards.rb:237:6:237:17 | ... == ... | barrier-guards.rb:237:1:237:38 | [false] when ... | false |
271-
| barrier-guards.rb:237:6:237:17 | ... == ... | barrier-guards.rb:237:1:237:38 | when ... | true |
279+
| barrier-guards.rb:237:6:237:17 | ... == ... | barrier-guards.rb:237:1:237:38 | [true] when ... | true |
272280
| barrier-guards.rb:237:6:237:17 | ... == ... | barrier-guards.rb:237:24:237:26 | foo | true |
273-
| barrier-guards.rb:238:1:238:26 | when ... | barrier-guards.rb:238:24:238:26 | foo | match |
281+
| barrier-guards.rb:237:6:237:17 | ... == ... | barrier-guards.rb:238:1:238:26 | [false] when ... | false |
282+
| barrier-guards.rb:237:6:237:17 | ... == ... | barrier-guards.rb:238:1:238:26 | [true] when ... | false |
283+
| barrier-guards.rb:237:6:237:17 | ... == ... | barrier-guards.rb:238:6:238:8 | self | false |
284+
| barrier-guards.rb:237:6:237:17 | ... == ... | barrier-guards.rb:238:24:238:26 | foo | false |
285+
| barrier-guards.rb:237:6:237:17 | ... == ... | barrier-guards.rb:239:1:239:22 | [false] when ... | false |
286+
| barrier-guards.rb:237:6:237:17 | ... == ... | barrier-guards.rb:239:1:239:22 | [true] when ... | false |
287+
| barrier-guards.rb:237:6:237:17 | ... == ... | barrier-guards.rb:239:6:239:8 | foo | false |
288+
| barrier-guards.rb:237:6:237:17 | ... == ... | barrier-guards.rb:239:20:239:22 | foo | false |
289+
| barrier-guards.rb:238:1:238:26 | [false] when ... | barrier-guards.rb:239:1:239:22 | [false] when ... | false |
290+
| barrier-guards.rb:238:1:238:26 | [false] when ... | barrier-guards.rb:239:1:239:22 | [true] when ... | false |
291+
| barrier-guards.rb:238:1:238:26 | [false] when ... | barrier-guards.rb:239:6:239:8 | foo | false |
292+
| barrier-guards.rb:238:1:238:26 | [false] when ... | barrier-guards.rb:239:20:239:22 | foo | false |
293+
| barrier-guards.rb:238:1:238:26 | [true] when ... | barrier-guards.rb:238:24:238:26 | foo | true |
274294
| barrier-guards.rb:238:6:238:17 | ... == ... | barrier-guards.rb:238:1:238:26 | [false] when ... | false |
275-
| barrier-guards.rb:238:6:238:17 | ... == ... | barrier-guards.rb:238:1:238:26 | when ... | true |
295+
| barrier-guards.rb:238:6:238:17 | ... == ... | barrier-guards.rb:238:1:238:26 | [true] when ... | true |
276296
| barrier-guards.rb:238:6:238:17 | ... == ... | barrier-guards.rb:238:24:238:26 | foo | true |
277-
| barrier-guards.rb:239:1:239:22 | when ... | barrier-guards.rb:239:20:239:22 | foo | match |
297+
| barrier-guards.rb:238:6:238:17 | ... == ... | barrier-guards.rb:239:1:239:22 | [false] when ... | false |
298+
| barrier-guards.rb:238:6:238:17 | ... == ... | barrier-guards.rb:239:1:239:22 | [true] when ... | false |
299+
| barrier-guards.rb:238:6:238:17 | ... == ... | barrier-guards.rb:239:6:239:8 | foo | false |
300+
| barrier-guards.rb:238:6:238:17 | ... == ... | barrier-guards.rb:239:20:239:22 | foo | false |
301+
| barrier-guards.rb:239:1:239:22 | [true] when ... | barrier-guards.rb:239:20:239:22 | foo | true |
278302
| barrier-guards.rb:239:6:239:13 | ... == ... | barrier-guards.rb:239:1:239:22 | [false] when ... | false |
279-
| barrier-guards.rb:239:6:239:13 | ... == ... | barrier-guards.rb:239:1:239:22 | when ... | true |
303+
| barrier-guards.rb:239:6:239:13 | ... == ... | barrier-guards.rb:239:1:239:22 | [true] when ... | true |
280304
| barrier-guards.rb:239:6:239:13 | ... == ... | barrier-guards.rb:239:20:239:22 | foo | true |
281305
| barrier-guards.rb:243:4:243:8 | "foo" | barrier-guards.rb:244:5:244:7 | foo | match |
282306
| barrier-guards.rb:243:4:243:8 | "foo" | barrier-guards.rb:245:1:246:7 | in ... then ... | no-match |

0 commit comments

Comments
 (0)