Skip to content

Commit a45c415

Browse files
hvitvedhvitved
authored
Merge pull request #5067 from hvitved/csharp/cfg/patterns
C#: Adjust CFG for `{Recursive,Positional,Property}PatternExpr`
2 parents 653c900 + 1ffa15e commit a45c415

File tree

17 files changed

+544
-298
lines changed

17 files changed

+544
-298
lines changed

‎csharp/ql/src/semmle/code/csharp/controlflow/internal/Completion.qll‎

Lines changed: 28 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -540,38 +540,47 @@ predicate switchMatching(Switch s, Case c, PatternExpr pe) {
540540
}
541541

542542
/**
543-
* Holds if `pe` must have a matching completion, and `e` is the expression
544-
* that is being matched.
543+
* Holds if a normal completion of `cfe` must be a matching completion. Thats is,
544+
* whether `cfe` determines a match in a `switch/if` statement or `catch` clause.
545545
*/
546-
private predicate mustHaveMatchingCompletion(Expr e, PatternExpr pe) {
547-
exists(Switch s |
548-
switchMatching(s, _, pe) and
549-
e = s.getExpr()
550-
)
546+
private predicate mustHaveMatchingCompletion(ControlFlowElement cfe) {
547+
switchMatching(_, _, cfe)
551548
or
552-
pe = any(IsExpr ie | inBooleanContext(ie) and e = ie.getExpr()).getPattern()
549+
cfe instanceof SpecificCatchClause
553550
or
554-
pe = any(RecursivePatternExpr rpe | mustHaveMatchingCompletion(e, rpe)).getPositionalPatterns()
551+
cfe = any(IsExpr ie | inBooleanContext(ie)).getPattern()
555552
or
556-
pe = any(RecursivePatternExpr rpe | mustHaveMatchingCompletion(e, rpe)).getPropertyPatterns()
553+
cfe = any(RecursivePatternExpr rpe).getAChildExpr()
557554
or
558-
pe = any(PositionalPatternExpr ppe | mustHaveMatchingCompletion(e, ppe)).getPattern(_)
555+
cfe = any(PositionalPatternExpr ppe).getPattern(_)
559556
or
560-
pe = any(PropertyPatternExpr ppe | mustHaveMatchingCompletion(e, ppe)).getPattern(_)
557+
cfe = any(PropertyPatternExpr ppe).getPattern(_)
561558
or
562-
pe = any(UnaryPatternExpr upe | mustHaveMatchingCompletion(e, upe)).getPattern()
559+
cfe = any(UnaryPatternExpr upe | mustHaveMatchingCompletion(upe)).getPattern()
563560
or
564-
pe = any(BinaryPatternExpr bpe | mustHaveMatchingCompletion(e, bpe)).getAnOperand()
561+
cfe = any(BinaryPatternExpr bpe).getAnOperand()
565562
}
566563

567564
/**
568-
* Holds if a normal completion of `cfe` must be a matching completion. Thats is,
569-
* whether `cfe` determines a match in a `switch` statement or `catch` clause.
565+
* Holds if `pe` must have a matching completion, and `e` is the expression
566+
* that is being matched.
570567
*/
571-
private predicate mustHaveMatchingCompletion(ControlFlowElement cfe) {
572-
mustHaveMatchingCompletion(_, cfe)
568+
private predicate mustHaveMatchingCompletion(Expr e, PatternExpr pe) {
569+
exists(Switch s |
570+
switchMatching(s, _, pe) and
571+
e = s.getExpr()
572+
)
573573
or
574-
cfe instanceof SpecificCatchClause
574+
e = any(IsExpr ie | pe = ie.getPattern()).getExpr() and
575+
mustHaveMatchingCompletion(pe)
576+
or
577+
exists(PatternExpr mid | mustHaveMatchingCompletion(e, mid) |
578+
pe = mid.(UnaryPatternExpr).getPattern()
579+
or
580+
pe = mid.(RecursivePatternExpr).getAChildExpr()
581+
or
582+
pe = mid.(BinaryPatternExpr).getAnOperand()
583+
)
575584
}
576585

577586
/**

‎csharp/ql/src/semmle/code/csharp/controlflow/internal/ControlFlowGraphImpl.qll‎

Lines changed: 116 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,7 @@ module Expressions {
290290
private class SimpleNoNodeExpr extends NoNodeExpr {
291291
SimpleNoNodeExpr() {
292292
this instanceof TypeAccess and
293-
not this = any(PatternMatch pm).getPattern()
293+
not this instanceof TypeAccessPatternExpr
294294
}
295295
}
296296

@@ -371,7 +371,10 @@ module Expressions {
371371
not this instanceof ConstructorInitializer and
372372
not this instanceof NotPatternExpr and
373373
not this instanceof OrPatternExpr and
374-
not this instanceof AndPatternExpr
374+
not this instanceof AndPatternExpr and
375+
not this instanceof RecursivePatternExpr and
376+
not this instanceof PositionalPatternExpr and
377+
not this instanceof PropertyPatternExpr
375378
}
376379

377380
final override ControlFlowElement getChildElement(int i) { result = getExprChild(this, i) }
@@ -965,6 +968,117 @@ module Expressions {
965968
}
966969
}
967970

971+
private class RecursivePatternExprTree extends PostOrderTree, RecursivePatternExpr {
972+
private Expr getTypeExpr() {
973+
result = this.getVariableDeclExpr()
974+
or
975+
not exists(this.getVariableDeclExpr()) and
976+
result = this.getTypeAccess()
977+
}
978+
979+
private PatternExpr getChildPattern() {
980+
result = this.getPositionalPatterns()
981+
or
982+
result = this.getPropertyPatterns()
983+
}
984+
985+
final override predicate propagatesAbnormal(ControlFlowElement child) {
986+
child = this.getChildPattern()
987+
}
988+
989+
final override predicate first(ControlFlowElement first) {
990+
first(this.getTypeExpr(), first)
991+
or
992+
not exists(this.getTypeExpr()) and
993+
first(this.getChildPattern(), first)
994+
}
995+
996+
final override predicate succ(ControlFlowElement pred, ControlFlowElement succ, Completion c) {
997+
// Flow from type test to child pattern
998+
last(this.getTypeExpr(), pred, c) and
999+
first(this.getChildPattern(), succ) and
1000+
c.(MatchingCompletion).getValue() = true
1001+
or
1002+
// Flow from type test to self
1003+
last(this.getTypeExpr(), pred, c) and
1004+
succ = this and
1005+
c.(MatchingCompletion).getValue() = false
1006+
or
1007+
// Flow from child pattern to self
1008+
last(this.getChildPattern(), pred, c) and
1009+
succ = this and
1010+
c instanceof MatchingCompletion
1011+
}
1012+
}
1013+
1014+
private class PositionalPatternExprTree extends PreOrderTree, PositionalPatternExpr {
1015+
final override predicate propagatesAbnormal(ControlFlowElement child) {
1016+
child = this.getPattern(_)
1017+
}
1018+
1019+
final override predicate last(ControlFlowElement last, Completion c) {
1020+
last = this and
1021+
c.(MatchingCompletion).getValue() = false
1022+
or
1023+
last(this.getPattern(_), last, c) and
1024+
c.(MatchingCompletion).getValue() = false
1025+
or
1026+
exists(int lst |
1027+
last(this.getPattern(lst), last, c) and
1028+
not exists(this.getPattern(lst + 1))
1029+
)
1030+
}
1031+
1032+
final override predicate succ(ControlFlowElement pred, ControlFlowElement succ, Completion c) {
1033+
// Flow from self to first pattern
1034+
pred = this and
1035+
c.(MatchingCompletion).getValue() = true and
1036+
first(this.getPattern(0), succ)
1037+
or
1038+
// Flow from one pattern to the next
1039+
exists(int i |
1040+
last(this.getPattern(i), pred, c) and
1041+
c.(MatchingCompletion).getValue() = true and
1042+
first(this.getPattern(i + 1), succ)
1043+
)
1044+
}
1045+
}
1046+
1047+
private class PropertyPatternExprExprTree extends PostOrderTree, PropertyPatternExpr {
1048+
final override predicate propagatesAbnormal(ControlFlowElement child) {
1049+
child = this.getPattern(_)
1050+
}
1051+
1052+
final override predicate first(ControlFlowElement first) {
1053+
first(this.getPattern(0), first)
1054+
or
1055+
not exists(this.getPattern(0)) and
1056+
first = this
1057+
}
1058+
1059+
final override predicate succ(ControlFlowElement pred, ControlFlowElement succ, Completion c) {
1060+
// Flow from one pattern to the next
1061+
exists(int i |
1062+
last(this.getPattern(i), pred, c) and
1063+
c.(MatchingCompletion).getValue() = true and
1064+
first(this.getPattern(i + 1), succ)
1065+
)
1066+
or
1067+
// Post-order: flow from last element of failing pattern to element itself
1068+
last(this.getPattern(_), pred, c) and
1069+
c.(MatchingCompletion).getValue() = false and
1070+
succ = this
1071+
or
1072+
// Post-order: flow from last element of last pattern to element itself
1073+
exists(int last |
1074+
last(this.getPattern(last), pred, c) and
1075+
not exists(this.getPattern(last + 1)) and
1076+
c instanceof MatchingCompletion and
1077+
succ = this
1078+
)
1079+
}
1080+
}
1081+
9681082
module Statements {
9691083
private class StandardStmt extends StandardElement, PreOrderTree, Stmt {
9701084
StandardStmt() {

‎csharp/ql/src/semmle/code/csharp/controlflow/internal/Splitting.qll‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -477,6 +477,12 @@ module ConditionalCompletionSplitting {
477477
or
478478
last(succ.(OrPatternExpr).getAnOperand(), pred, c) and
479479
completion = c
480+
or
481+
last(succ.(RecursivePatternExpr).getAChildExpr(), pred, c) and
482+
completion = c
483+
or
484+
last(succ.(PropertyPatternExpr).getPattern(_), pred, c) and
485+
completion = c
480486
)
481487
}
482488

‎csharp/ql/src/semmle/code/csharp/exprs/Expr.qll‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -303,18 +303,18 @@ private predicate hasChildPattern(ControlFlowElement pm, Expr child) {
303303
exists(Expr mid |
304304
hasChildPattern(pm, mid) and
305305
mid instanceof @recursive_pattern_expr and
306-
child = mid.getChild([2, 3])
306+
child = mid.getAChildExpr()
307307
)
308308
or
309309
exists(Expr mid |
310310
hasChildPattern(pm, mid) and
311311
mid instanceof @unary_pattern_expr and
312-
child = mid.getChild(0)
312+
child = mid.getChildExpr(0)
313313
)
314314
or
315315
exists(Expr mid | hasChildPattern(pm, mid) and mid instanceof @binary_pattern_expr |
316-
child = mid.getChild(0) or
317-
child = mid.getChild(1)
316+
child = mid.getChildExpr(0) or
317+
child = mid.getChildExpr(1)
318318
)
319319
}
320320

‎csharp/ql/test/library-tests/controlflow/graph/BasicBlock.expected‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -962,7 +962,11 @@
962962
| Patterns.cs:51:14:51:21 | [no-match] not ... | Patterns.cs:51:14:51:21 | [no-match] not ... | 1 |
963963
| Patterns.cs:51:25:51:25 | access to parameter c | Patterns.cs:51:25:51:30 | ... is ... | 3 |
964964
| Patterns.cs:51:34:51:34 | access to parameter c | Patterns.cs:51:34:51:39 | ... is ... | 3 |
965-
| Patterns.cs:53:24:53:25 | enter M4 | Patterns.cs:53:24:53:25 | exit M4 | 10 |
965+
| Patterns.cs:53:24:53:25 | enter M4 | Patterns.cs:54:18:54:37 | Patterns u | 3 |
966+
| Patterns.cs:54:18:54:37 | { ... } | Patterns.cs:53:24:53:25 | exit M4 | 5 |
967+
| Patterns.cs:54:27:54:35 | [match] { ... } | Patterns.cs:54:27:54:35 | [match] { ... } | 1 |
968+
| Patterns.cs:54:27:54:35 | [no-match] { ... } | Patterns.cs:54:27:54:35 | [no-match] { ... } | 1 |
969+
| Patterns.cs:54:33:54:33 | 1 | Patterns.cs:54:33:54:33 | 1 | 1 |
966970
| Patterns.cs:56:26:56:27 | enter M5 | Patterns.cs:60:17:60:17 | 1 | 4 |
967971
| Patterns.cs:58:16:62:9 | ... switch { ... } | Patterns.cs:56:26:56:27 | exit M5 | 4 |
968972
| Patterns.cs:60:13:60:17 | [match] not ... | Patterns.cs:60:13:60:17 | [match] not ... | 1 |
@@ -1009,8 +1013,10 @@
10091013
| Patterns.cs:93:17:93:19 | exit M10 (normal) | Patterns.cs:93:17:93:19 | exit M10 | 2 |
10101014
| Patterns.cs:95:13:95:40 | [false] ... is ... | Patterns.cs:95:13:95:40 | [false] ... is ... | 1 |
10111015
| Patterns.cs:95:13:95:40 | [true] ... is ... | Patterns.cs:95:13:95:40 | [true] ... is ... | 1 |
1012-
| Patterns.cs:95:21:95:40 | { ... } | Patterns.cs:95:21:95:40 | { ... } | 1 |
1013-
| Patterns.cs:95:21:95:40 | { ... } | Patterns.cs:95:21:95:40 | { ... } | 1 |
1016+
| Patterns.cs:95:21:95:40 | [match] { ... } | Patterns.cs:95:21:95:40 | [match] { ... } | 1 |
1017+
| Patterns.cs:95:21:95:40 | [match] { ... } | Patterns.cs:95:21:95:40 | [match] { ... } | 1 |
1018+
| Patterns.cs:95:21:95:40 | [no-match] { ... } | Patterns.cs:95:21:95:40 | [no-match] { ... } | 1 |
1019+
| Patterns.cs:95:21:95:40 | [no-match] { ... } | Patterns.cs:95:21:95:40 | [no-match] { ... } | 1 |
10141020
| Patterns.cs:95:29:95:38 | [match] ... or ... | Patterns.cs:95:29:95:38 | [match] ... or ... | 1 |
10151021
| Patterns.cs:95:29:95:38 | [no-match] ... or ... | Patterns.cs:95:29:95:38 | [no-match] ... or ... | 1 |
10161022
| Patterns.cs:95:36:95:38 | access to constant B | Patterns.cs:95:36:95:38 | access to constant B | 1 |

‎csharp/ql/test/library-tests/controlflow/graph/Condition.expected‎

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1859,6 +1859,11 @@ conditionBlock
18591859
| Patterns.cs:51:14:51:21 | [match] not ... | Patterns.cs:51:25:51:25 | access to parameter c | true |
18601860
| Patterns.cs:51:14:51:21 | [no-match] not ... | Patterns.cs:51:9:51:21 | [false] ... is ... | false |
18611861
| Patterns.cs:51:14:51:21 | [no-match] not ... | Patterns.cs:51:34:51:34 | access to parameter c | false |
1862+
| Patterns.cs:53:24:53:25 | enter M4 | Patterns.cs:54:27:54:35 | [match] { ... } | true |
1863+
| Patterns.cs:53:24:53:25 | enter M4 | Patterns.cs:54:27:54:35 | [no-match] { ... } | true |
1864+
| Patterns.cs:53:24:53:25 | enter M4 | Patterns.cs:54:33:54:33 | 1 | true |
1865+
| Patterns.cs:54:33:54:33 | 1 | Patterns.cs:54:27:54:35 | [match] { ... } | true |
1866+
| Patterns.cs:54:33:54:33 | 1 | Patterns.cs:54:27:54:35 | [no-match] { ... } | false |
18621867
| Patterns.cs:56:26:56:27 | enter M5 | Patterns.cs:60:13:60:17 | [match] not ... | false |
18631868
| Patterns.cs:56:26:56:27 | enter M5 | Patterns.cs:60:13:60:17 | [no-match] not ... | true |
18641869
| Patterns.cs:56:26:56:27 | enter M5 | Patterns.cs:60:22:60:28 | "not 1" | false |
@@ -1930,22 +1935,30 @@ conditionBlock
19301935
| Patterns.cs:87:54:87:54 | 2 | Patterns.cs:87:50:87:54 | [match] not ... | false |
19311936
| Patterns.cs:87:54:87:54 | 2 | Patterns.cs:87:50:87:54 | [no-match] not ... | true |
19321937
| Patterns.cs:87:54:87:54 | 2 | Patterns.cs:87:58:87:60 | "1" | false |
1938+
| Patterns.cs:93:17:93:19 | enter M10 | Patterns.cs:95:13:95:40 | [false] ... is ... | false |
1939+
| Patterns.cs:93:17:93:19 | enter M10 | Patterns.cs:95:21:95:40 | [no-match] { ... } | false |
1940+
| Patterns.cs:93:17:93:19 | enter M10 | Patterns.cs:95:21:95:40 | [no-match] { ... } | false |
19331941
| Patterns.cs:93:17:93:19 | enter M10 | Patterns.cs:95:29:95:38 | [no-match] ... or ... | false |
19341942
| Patterns.cs:93:17:93:19 | enter M10 | Patterns.cs:95:36:95:38 | access to constant B | false |
19351943
| Patterns.cs:95:13:95:40 | [true] ... is ... | Patterns.cs:96:9:98:9 | {...} | true |
1936-
| Patterns.cs:95:21:95:40 | { ... } | Patterns.cs:93:17:93:19 | exit M10 (normal) | false |
1937-
| Patterns.cs:95:21:95:40 | { ... } | Patterns.cs:93:17:93:19 | exit M10 (normal) | true |
1938-
| Patterns.cs:95:21:95:40 | { ... } | Patterns.cs:95:13:95:40 | [false] ... is ... | false |
1939-
| Patterns.cs:95:21:95:40 | { ... } | Patterns.cs:95:13:95:40 | [false] ... is ... | false |
1940-
| Patterns.cs:95:21:95:40 | { ... } | Patterns.cs:95:13:95:40 | [false] ... is ... | true |
1941-
| Patterns.cs:95:21:95:40 | { ... } | Patterns.cs:95:13:95:40 | [true] ... is ... | false |
1942-
| Patterns.cs:95:21:95:40 | { ... } | Patterns.cs:95:13:95:40 | [true] ... is ... | true |
1943-
| Patterns.cs:95:21:95:40 | { ... } | Patterns.cs:95:13:95:40 | [true] ... is ... | true |
1944-
| Patterns.cs:95:21:95:40 | { ... } | Patterns.cs:95:21:95:40 | { ... } | false |
1945-
| Patterns.cs:95:21:95:40 | { ... } | Patterns.cs:95:21:95:40 | { ... } | true |
1946-
| Patterns.cs:95:21:95:40 | { ... } | Patterns.cs:96:9:98:9 | {...} | false |
1947-
| Patterns.cs:95:21:95:40 | { ... } | Patterns.cs:96:9:98:9 | {...} | true |
1948-
| Patterns.cs:95:21:95:40 | { ... } | Patterns.cs:96:9:98:9 | {...} | true |
1944+
| Patterns.cs:95:21:95:40 | [match] { ... } | Patterns.cs:95:13:95:40 | [true] ... is ... | true |
1945+
| Patterns.cs:95:21:95:40 | [match] { ... } | Patterns.cs:95:13:95:40 | [true] ... is ... | true |
1946+
| Patterns.cs:95:21:95:40 | [match] { ... } | Patterns.cs:95:21:95:40 | [match] { ... } | true |
1947+
| Patterns.cs:95:21:95:40 | [match] { ... } | Patterns.cs:96:9:98:9 | {...} | true |
1948+
| Patterns.cs:95:21:95:40 | [match] { ... } | Patterns.cs:96:9:98:9 | {...} | true |
1949+
| Patterns.cs:95:21:95:40 | [no-match] { ... } | Patterns.cs:95:13:95:40 | [false] ... is ... | false |
1950+
| Patterns.cs:95:21:95:40 | [no-match] { ... } | Patterns.cs:95:13:95:40 | [false] ... is ... | false |
1951+
| Patterns.cs:95:21:95:40 | [no-match] { ... } | Patterns.cs:95:21:95:40 | [no-match] { ... } | false |
1952+
| Patterns.cs:95:29:95:38 | [match] ... or ... | Patterns.cs:95:13:95:40 | [true] ... is ... | true |
1953+
| Patterns.cs:95:29:95:38 | [match] ... or ... | Patterns.cs:95:21:95:40 | [match] { ... } | true |
1954+
| Patterns.cs:95:29:95:38 | [match] ... or ... | Patterns.cs:95:21:95:40 | [match] { ... } | true |
1955+
| Patterns.cs:95:29:95:38 | [match] ... or ... | Patterns.cs:96:9:98:9 | {...} | true |
1956+
| Patterns.cs:95:29:95:38 | [no-match] ... or ... | Patterns.cs:95:13:95:40 | [false] ... is ... | false |
1957+
| Patterns.cs:95:29:95:38 | [no-match] ... or ... | Patterns.cs:95:21:95:40 | [no-match] { ... } | false |
1958+
| Patterns.cs:95:29:95:38 | [no-match] ... or ... | Patterns.cs:95:21:95:40 | [no-match] { ... } | false |
1959+
| Patterns.cs:95:36:95:38 | access to constant B | Patterns.cs:95:13:95:40 | [false] ... is ... | false |
1960+
| Patterns.cs:95:36:95:38 | access to constant B | Patterns.cs:95:21:95:40 | [no-match] { ... } | false |
1961+
| Patterns.cs:95:36:95:38 | access to constant B | Patterns.cs:95:21:95:40 | [no-match] { ... } | false |
19491962
| Patterns.cs:95:36:95:38 | access to constant B | Patterns.cs:95:29:95:38 | [no-match] ... or ... | false |
19501963
| PostDominance.cs:10:10:10:11 | enter M2 | PostDominance.cs:12:13:12:21 | [false] ... is ... | false |
19511964
| PostDominance.cs:10:10:10:11 | enter M2 | PostDominance.cs:12:13:12:21 | [true] ... is ... | true |

0 commit comments

Comments
 (0)