Skip to content

Commit 17aad4b

Browse files
hvitvedhvitved
committed
C#: Fix CFG for switch expressions in Boolean/nullness contexts
1 parent e538d8e commit 17aad4b

File tree

10 files changed

+86
-23
lines changed

10 files changed

+86
-23
lines changed

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -370,6 +370,12 @@ private predicate inBooleanContext(Expr e, boolean isBooleanCompletionForParent)
370370
inBooleanContext(nce, _) and
371371
isBooleanCompletionForParent = true
372372
)
373+
or
374+
exists(SwitchExpr se |
375+
inBooleanContext(se, _) and
376+
e = se.getACase().getBody() and
377+
isBooleanCompletionForParent = true
378+
)
373379
}
374380

375381
/**
@@ -406,6 +412,12 @@ private predicate inNullnessContext(Expr e, boolean isNullnessCompletionForParen
406412
e = nce.getRightOperand() and
407413
isNullnessCompletionForParent = true
408414
)
415+
or
416+
exists(SwitchExpr se |
417+
inNullnessContext(se, _) and
418+
e = se.getACase().getBody() and
419+
isNullnessCompletionForParent = true
420+
)
409421
}
410422

411423
/**

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -511,6 +511,8 @@ class SwitchExpr extends Expr, Switch, @switch_expr {
511511
override Expr getExpr() { result = this.getChild(-1) }
512512

513513
override SwitchCaseExpr getCase(int n) { result = this.getChild(n) }
514+
515+
override SwitchCaseExpr getACase() { result = this.getCase(_) }
514516
}
515517

516518
/** A `case` expression or statement. */

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -366,12 +366,15 @@
366366
| Switch.cs:118:42:118:42 | 2 | Switch.cs:118:35:118:43 | return ...; | 2 |
367367
| Switch.cs:120:17:120:17 | 1 | Switch.cs:120:9:120:18 | return ...; | 3 |
368368
| Switch.cs:123:10:123:12 | enter M11 | Switch.cs:125:24:125:29 | Boolean b | 7 |
369+
| Switch.cs:123:10:123:12 | exit M11 | Switch.cs:123:10:123:12 | exit M11 | 1 |
369370
| Switch.cs:125:34:125:34 | access to local variable b | Switch.cs:125:34:125:34 | access to local variable b | 1 |
370371
| Switch.cs:125:37:125:46 | ... => ... | Switch.cs:125:42:125:46 | false | 3 |
372+
| Switch.cs:126:13:126:19 | return ...; | Switch.cs:126:13:126:19 | return ...; | 1 |
371373
| Switch.cs:129:12:129:14 | enter M12 | Switch.cs:131:28:131:35 | String s | 6 |
374+
| Switch.cs:131:9:131:67 | return ...; | Switch.cs:129:12:129:14 | exit M12 | 2 |
372375
| Switch.cs:131:40:131:40 | access to local variable s | Switch.cs:131:40:131:40 | access to local variable s | 1 |
373376
| Switch.cs:131:43:131:51 | ... => ... | Switch.cs:131:48:131:51 | null | 3 |
374-
| Switch.cs:131:56:131:66 | call to method ToString | Switch.cs:129:12:129:14 | exit M12 | 3 |
377+
| Switch.cs:131:56:131:66 | call to method ToString | Switch.cs:131:56:131:66 | call to method ToString | 1 |
375378
| TypeAccesses.cs:3:10:3:10 | enter M | TypeAccesses.cs:7:13:7:22 | ... is ... | 14 |
376379
| TypeAccesses.cs:7:25:7:25 | ; | TypeAccesses.cs:7:25:7:25 | ; | 1 |
377380
| TypeAccesses.cs:8:9:8:28 | ... ...; | TypeAccesses.cs:3:10:3:10 | exit M | 4 |

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

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -723,14 +723,22 @@
723723
| post | Switch.cs:118:42:118:42 | 2 | Switch.cs:118:42:118:42 | 2 |
724724
| post | Switch.cs:120:17:120:17 | 1 | Switch.cs:120:17:120:17 | 1 |
725725
| post | Switch.cs:123:10:123:12 | enter M11 | Switch.cs:123:10:123:12 | enter M11 |
726+
| post | Switch.cs:123:10:123:12 | exit M11 | Switch.cs:123:10:123:12 | enter M11 |
727+
| post | Switch.cs:123:10:123:12 | exit M11 | Switch.cs:123:10:123:12 | exit M11 |
728+
| post | Switch.cs:123:10:123:12 | exit M11 | Switch.cs:125:34:125:34 | access to local variable b |
729+
| post | Switch.cs:123:10:123:12 | exit M11 | Switch.cs:125:37:125:46 | ... => ... |
730+
| post | Switch.cs:123:10:123:12 | exit M11 | Switch.cs:126:13:126:19 | return ...; |
726731
| post | Switch.cs:125:34:125:34 | access to local variable b | Switch.cs:125:34:125:34 | access to local variable b |
727732
| post | Switch.cs:125:37:125:46 | ... => ... | Switch.cs:125:37:125:46 | ... => ... |
733+
| post | Switch.cs:126:13:126:19 | return ...; | Switch.cs:126:13:126:19 | return ...; |
728734
| post | Switch.cs:129:12:129:14 | enter M12 | Switch.cs:129:12:129:14 | enter M12 |
735+
| post | Switch.cs:131:9:131:67 | return ...; | Switch.cs:129:12:129:14 | enter M12 |
736+
| post | Switch.cs:131:9:131:67 | return ...; | Switch.cs:131:9:131:67 | return ...; |
737+
| post | Switch.cs:131:9:131:67 | return ...; | Switch.cs:131:40:131:40 | access to local variable s |
738+
| post | Switch.cs:131:9:131:67 | return ...; | Switch.cs:131:43:131:51 | ... => ... |
739+
| post | Switch.cs:131:9:131:67 | return ...; | Switch.cs:131:56:131:66 | call to method ToString |
729740
| post | Switch.cs:131:40:131:40 | access to local variable s | Switch.cs:131:40:131:40 | access to local variable s |
730741
| post | Switch.cs:131:43:131:51 | ... => ... | Switch.cs:131:43:131:51 | ... => ... |
731-
| post | Switch.cs:131:56:131:66 | call to method ToString | Switch.cs:129:12:129:14 | enter M12 |
732-
| post | Switch.cs:131:56:131:66 | call to method ToString | Switch.cs:131:40:131:40 | access to local variable s |
733-
| post | Switch.cs:131:56:131:66 | call to method ToString | Switch.cs:131:43:131:51 | ... => ... |
734742
| post | Switch.cs:131:56:131:66 | call to method ToString | Switch.cs:131:56:131:66 | call to method ToString |
735743
| post | TypeAccesses.cs:3:10:3:10 | enter M | TypeAccesses.cs:3:10:3:10 | enter M |
736744
| post | TypeAccesses.cs:7:25:7:25 | ; | TypeAccesses.cs:7:25:7:25 | ; |
@@ -2255,15 +2263,23 @@
22552263
| pre | Switch.cs:118:42:118:42 | 2 | Switch.cs:118:42:118:42 | 2 |
22562264
| pre | Switch.cs:120:17:120:17 | 1 | Switch.cs:120:17:120:17 | 1 |
22572265
| pre | Switch.cs:123:10:123:12 | enter M11 | Switch.cs:123:10:123:12 | enter M11 |
2266+
| pre | Switch.cs:123:10:123:12 | enter M11 | Switch.cs:123:10:123:12 | exit M11 |
22582267
| pre | Switch.cs:123:10:123:12 | enter M11 | Switch.cs:125:34:125:34 | access to local variable b |
22592268
| pre | Switch.cs:123:10:123:12 | enter M11 | Switch.cs:125:37:125:46 | ... => ... |
2269+
| pre | Switch.cs:123:10:123:12 | enter M11 | Switch.cs:126:13:126:19 | return ...; |
2270+
| pre | Switch.cs:123:10:123:12 | exit M11 | Switch.cs:123:10:123:12 | exit M11 |
22602271
| pre | Switch.cs:125:34:125:34 | access to local variable b | Switch.cs:125:34:125:34 | access to local variable b |
2272+
| pre | Switch.cs:125:34:125:34 | access to local variable b | Switch.cs:126:13:126:19 | return ...; |
22612273
| pre | Switch.cs:125:37:125:46 | ... => ... | Switch.cs:125:37:125:46 | ... => ... |
2274+
| pre | Switch.cs:126:13:126:19 | return ...; | Switch.cs:126:13:126:19 | return ...; |
22622275
| pre | Switch.cs:129:12:129:14 | enter M12 | Switch.cs:129:12:129:14 | enter M12 |
2276+
| pre | Switch.cs:129:12:129:14 | enter M12 | Switch.cs:131:9:131:67 | return ...; |
22632277
| pre | Switch.cs:129:12:129:14 | enter M12 | Switch.cs:131:40:131:40 | access to local variable s |
22642278
| pre | Switch.cs:129:12:129:14 | enter M12 | Switch.cs:131:43:131:51 | ... => ... |
22652279
| pre | Switch.cs:129:12:129:14 | enter M12 | Switch.cs:131:56:131:66 | call to method ToString |
2280+
| pre | Switch.cs:131:9:131:67 | return ...; | Switch.cs:131:9:131:67 | return ...; |
22662281
| pre | Switch.cs:131:40:131:40 | access to local variable s | Switch.cs:131:40:131:40 | access to local variable s |
2282+
| pre | Switch.cs:131:40:131:40 | access to local variable s | Switch.cs:131:56:131:66 | call to method ToString |
22672283
| pre | Switch.cs:131:43:131:51 | ... => ... | Switch.cs:131:43:131:51 | ... => ... |
22682284
| pre | Switch.cs:131:56:131:66 | call to method ToString | Switch.cs:131:56:131:66 | call to method ToString |
22692285
| pre | TypeAccesses.cs:3:10:3:10 | enter M | TypeAccesses.cs:3:10:3:10 | enter M |

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -329,8 +329,12 @@
329329
| Switch.cs:118:25:118:31 | ... == ... | Switch.cs:118:42:118:42 | 2 | true |
330330
| Switch.cs:125:24:125:29 | Boolean b | Switch.cs:125:34:125:34 | access to local variable b | true |
331331
| Switch.cs:125:24:125:29 | Boolean b | Switch.cs:125:37:125:46 | ... => ... | false |
332+
| Switch.cs:125:24:125:29 | Boolean b | Switch.cs:126:13:126:19 | return ...; | true |
333+
| Switch.cs:125:34:125:34 | access to local variable b | Switch.cs:126:13:126:19 | return ...; | true |
332334
| Switch.cs:131:28:131:35 | String s | Switch.cs:131:40:131:40 | access to local variable s | true |
333335
| Switch.cs:131:28:131:35 | String s | Switch.cs:131:43:131:51 | ... => ... | false |
336+
| Switch.cs:131:28:131:35 | String s | Switch.cs:131:56:131:66 | call to method ToString | true |
337+
| Switch.cs:131:40:131:40 | access to local variable s | Switch.cs:131:56:131:66 | call to method ToString | false |
334338
| TypeAccesses.cs:7:13:7:22 | ... is ... | TypeAccesses.cs:7:25:7:25 | ; | true |
335339
| VarDecls.cs:25:20:25:20 | access to parameter b | VarDecls.cs:25:24:25:24 | access to local variable x | true |
336340
| VarDecls.cs:25:20:25:20 | access to parameter b | VarDecls.cs:25:28:25:28 | access to local variable y | false |

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,9 @@
197197
| 119 | 18 | Conditions.cs:119:18:119:21 | access to local variable last | true | 121 | 13 | Conditions.cs:121:13:122:25 | [last (line 118): true] if (...) ... |
198198
| 121 | 17 | Conditions.cs:121:17:121:20 | [last (line 118): false] access to local variable last | false | 116 | 41 | Conditions.cs:116:41:116:41 | access to local variable i |
199199
| 121 | 17 | Conditions.cs:121:17:121:20 | [last (line 118): true] access to local variable last | true | 122 | 17 | Conditions.cs:122:17:122:25 | ...; |
200+
| 125 | 34 | Switch.cs:125:34:125:34 | access to local variable b | false | 123 | 10 | Switch.cs:123:10:123:12 | exit M11 |
201+
| 125 | 34 | Switch.cs:125:34:125:34 | access to local variable b | true | 126 | 13 | Switch.cs:126:13:126:19 | return ...; |
202+
| 125 | 42 | Switch.cs:125:42:125:46 | false | false | 123 | 10 | Switch.cs:123:10:123:12 | exit M11 |
200203
| 127 | 32 | cflow.cs:127:32:127:44 | ... == ... | false | 127 | 53 | cflow.cs:127:53:127:57 | this access |
201204
| 127 | 32 | cflow.cs:127:32:127:44 | ... == ... | true | 127 | 48 | cflow.cs:127:48:127:49 | "" |
202205
| 131 | 16 | Conditions.cs:131:16:131:19 | [Field1 (line 129): false] true | true | 132 | 9 | Conditions.cs:132:9:140:9 | [Field1 (line 129): false] {...} |

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1485,6 +1485,9 @@
14851485
| post | Switch.cs:118:35:118:43 | return ...; | Switch.cs:118:42:118:42 | 2 |
14861486
| post | Switch.cs:120:9:120:18 | return ...; | Switch.cs:120:16:120:17 | -... |
14871487
| post | Switch.cs:120:16:120:17 | -... | Switch.cs:120:17:120:17 | 1 |
1488+
| post | Switch.cs:123:10:123:12 | exit M11 | Switch.cs:125:34:125:34 | access to local variable b |
1489+
| post | Switch.cs:123:10:123:12 | exit M11 | Switch.cs:125:42:125:46 | false |
1490+
| post | Switch.cs:123:10:123:12 | exit M11 | Switch.cs:126:13:126:19 | return ...; |
14881491
| post | Switch.cs:124:5:127:5 | {...} | Switch.cs:123:10:123:12 | enter M11 |
14891492
| post | Switch.cs:125:9:126:19 | if (...) ... | Switch.cs:124:5:127:5 | {...} |
14901493
| post | Switch.cs:125:13:125:13 | access to parameter o | Switch.cs:125:13:125:48 | ... switch { ... } |
@@ -1495,15 +1498,15 @@
14951498
| post | Switch.cs:125:42:125:46 | false | Switch.cs:125:37:125:37 | _ |
14961499
| post | Switch.cs:129:12:129:14 | exit M12 | Switch.cs:131:9:131:67 | return ...; |
14971500
| post | Switch.cs:130:5:132:5 | {...} | Switch.cs:129:12:129:14 | enter M12 |
1501+
| post | Switch.cs:131:9:131:67 | return ...; | Switch.cs:131:40:131:40 | access to local variable s |
1502+
| post | Switch.cs:131:9:131:67 | return ...; | Switch.cs:131:48:131:51 | null |
14981503
| post | Switch.cs:131:9:131:67 | return ...; | Switch.cs:131:56:131:66 | call to method ToString |
14991504
| post | Switch.cs:131:17:131:17 | access to parameter o | Switch.cs:131:17:131:53 | ... switch { ... } |
15001505
| post | Switch.cs:131:17:131:53 | ... switch { ... } | Switch.cs:130:5:132:5 | {...} |
15011506
| post | Switch.cs:131:28:131:35 | String s | Switch.cs:131:28:131:40 | ... => ... |
15021507
| post | Switch.cs:131:28:131:40 | ... => ... | Switch.cs:131:17:131:17 | access to parameter o |
15031508
| post | Switch.cs:131:43:131:43 | _ | Switch.cs:131:43:131:51 | ... => ... |
15041509
| post | Switch.cs:131:48:131:51 | null | Switch.cs:131:43:131:43 | _ |
1505-
| post | Switch.cs:131:56:131:66 | call to method ToString | Switch.cs:131:40:131:40 | access to local variable s |
1506-
| post | Switch.cs:131:56:131:66 | call to method ToString | Switch.cs:131:48:131:51 | null |
15071510
| post | TypeAccesses.cs:3:10:3:10 | exit M | TypeAccesses.cs:8:13:8:27 | Type t = ... |
15081511
| post | TypeAccesses.cs:4:5:9:5 | {...} | TypeAccesses.cs:3:10:3:10 | enter M |
15091512
| post | TypeAccesses.cs:5:9:5:26 | ... ...; | TypeAccesses.cs:4:5:9:5 | {...} |
@@ -3991,6 +3994,7 @@
39913994
| pre | Switch.cs:125:24:125:29 | Boolean b | Switch.cs:125:34:125:34 | access to local variable b |
39923995
| pre | Switch.cs:125:24:125:29 | Boolean b | Switch.cs:125:37:125:46 | ... => ... |
39933996
| pre | Switch.cs:125:24:125:34 | ... => ... | Switch.cs:125:24:125:29 | Boolean b |
3997+
| pre | Switch.cs:125:34:125:34 | access to local variable b | Switch.cs:126:13:126:19 | return ...; |
39943998
| pre | Switch.cs:125:37:125:37 | _ | Switch.cs:125:42:125:46 | false |
39953999
| pre | Switch.cs:125:37:125:46 | ... => ... | Switch.cs:125:37:125:37 | _ |
39964000
| pre | Switch.cs:129:12:129:14 | enter M12 | Switch.cs:130:5:132:5 | {...} |
@@ -4001,9 +4005,9 @@
40014005
| pre | Switch.cs:131:28:131:35 | String s | Switch.cs:131:40:131:40 | access to local variable s |
40024006
| pre | Switch.cs:131:28:131:35 | String s | Switch.cs:131:43:131:51 | ... => ... |
40034007
| pre | Switch.cs:131:28:131:40 | ... => ... | Switch.cs:131:28:131:35 | String s |
4008+
| pre | Switch.cs:131:40:131:40 | access to local variable s | Switch.cs:131:56:131:66 | call to method ToString |
40044009
| pre | Switch.cs:131:43:131:43 | _ | Switch.cs:131:48:131:51 | null |
40054010
| pre | Switch.cs:131:43:131:51 | ... => ... | Switch.cs:131:43:131:43 | _ |
4006-
| pre | Switch.cs:131:56:131:66 | call to method ToString | Switch.cs:131:9:131:67 | return ...; |
40074011
| pre | TypeAccesses.cs:3:10:3:10 | enter M | TypeAccesses.cs:4:5:9:5 | {...} |
40084012
| pre | TypeAccesses.cs:4:5:9:5 | {...} | TypeAccesses.cs:5:9:5:26 | ... ...; |
40094013
| pre | TypeAccesses.cs:5:9:5:26 | ... ...; | TypeAccesses.cs:5:25:5:25 | access to parameter o |

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1264,6 +1264,7 @@
12641264
| Switch.cs:125:24:125:29 | Boolean b | Switch.cs:125:34:125:34 | access to local variable b | semmle.label | match |
12651265
| Switch.cs:125:24:125:29 | Boolean b | Switch.cs:125:37:125:46 | ... => ... | semmle.label | no-match |
12661266
| Switch.cs:125:24:125:34 | ... => ... | Switch.cs:125:24:125:29 | Boolean b | semmle.label | successor |
1267+
| Switch.cs:125:34:125:34 | access to local variable b | Switch.cs:126:13:126:19 | return ...; | semmle.label | true |
12671268
| Switch.cs:125:37:125:37 | _ | Switch.cs:125:42:125:46 | false | semmle.label | match |
12681269
| Switch.cs:125:37:125:46 | ... => ... | Switch.cs:125:37:125:37 | _ | semmle.label | successor |
12691270
| Switch.cs:130:5:132:5 | {...} | Switch.cs:131:17:131:53 | ... switch { ... } | semmle.label | successor |
@@ -1272,10 +1273,11 @@
12721273
| Switch.cs:131:28:131:35 | String s | Switch.cs:131:40:131:40 | access to local variable s | semmle.label | match |
12731274
| Switch.cs:131:28:131:35 | String s | Switch.cs:131:43:131:51 | ... => ... | semmle.label | no-match |
12741275
| Switch.cs:131:28:131:40 | ... => ... | Switch.cs:131:28:131:35 | String s | semmle.label | successor |
1275-
| Switch.cs:131:40:131:40 | access to local variable s | Switch.cs:131:56:131:66 | call to method ToString | semmle.label | successor |
1276+
| Switch.cs:131:40:131:40 | access to local variable s | Switch.cs:131:9:131:67 | return ...; | semmle.label | null |
1277+
| Switch.cs:131:40:131:40 | access to local variable s | Switch.cs:131:56:131:66 | call to method ToString | semmle.label | non-null |
12761278
| Switch.cs:131:43:131:43 | _ | Switch.cs:131:48:131:51 | null | semmle.label | match |
12771279
| Switch.cs:131:43:131:51 | ... => ... | Switch.cs:131:43:131:43 | _ | semmle.label | successor |
1278-
| Switch.cs:131:48:131:51 | null | Switch.cs:131:56:131:66 | call to method ToString | semmle.label | successor |
1280+
| Switch.cs:131:48:131:51 | null | Switch.cs:131:9:131:67 | return ...; | semmle.label | null |
12791281
| Switch.cs:131:56:131:66 | call to method ToString | Switch.cs:131:9:131:67 | return ...; | semmle.label | successor |
12801282
| TypeAccesses.cs:4:5:9:5 | {...} | TypeAccesses.cs:5:9:5:26 | ... ...; | semmle.label | successor |
12811283
| TypeAccesses.cs:5:9:5:26 | ... ...; | TypeAccesses.cs:5:25:5:25 | access to parameter o | semmle.label | successor |

0 commit comments

Comments
 (0)