From c68dfb9d68fd9998935bc29f1a35524470b232c3 Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Wed, 29 May 2019 09:18:41 +0200 Subject: [PATCH] C#: Switch expression guards --- .../semmle/code/csharp/controlflow/Guards.qll | 20 +++--- .../controlflow/guards/ExtractorOptions.cs | 2 +- .../guards/GuardedControlFlowNode.expected | 16 +++++ .../controlflow/guards/GuardedExpr.expected | 16 +++++ .../controlflow/guards/Guards.cs | 65 +++++++++++++++++++ .../controlflow/guards/Implications.expected | 26 ++++++++ .../guards/MatchingGuardedExpr.expected | 12 ++++ .../guards/NullGuardedExpr.expected | 3 + 8 files changed, 151 insertions(+), 9 deletions(-) diff --git a/csharp/ql/src/semmle/code/csharp/controlflow/Guards.qll b/csharp/ql/src/semmle/code/csharp/controlflow/Guards.qll index b82d36a00c00..44425a767651 100644 --- a/csharp/ql/src/semmle/code/csharp/controlflow/Guards.qll +++ b/csharp/ql/src/semmle/code/csharp/controlflow/Guards.qll @@ -70,13 +70,13 @@ module AbstractValues { } /** An integer value. */ - class IntergerValue extends AbstractValue, TIntegerValue { + class IntegerValue extends AbstractValue, TIntegerValue { /** Gets the underlying integer value. */ int getValue() { this = TIntegerValue(result) } override predicate branch(ControlFlowElement cfe, ConditionalSuccessor s, Expr e) { none() } - override BooleanValue getDualValue() { none() } + override IntegerValue getDualValue() { none() } override Expr getAnExpr() { result.getValue().toInt() = this.getValue() and @@ -199,7 +199,9 @@ class DereferenceableExpr extends Expr { // incorrectly `int`, while it should have been `int?`. We apply // `getNullEquivParent()` as a workaround this = getNullEquivParent*(e) and - t = e.getType() + t = e.getType() and + not this instanceof SwitchCaseExpr and + not this instanceof PatternExpr | t instanceof NullableType and isNullableType = true @@ -734,6 +736,8 @@ module Internal { Guard() { this.getType() instanceof BoolType and not this instanceof BoolLiteral and + not this instanceof SwitchCaseExpr and + not this instanceof PatternExpr and val = TBooleanValue(_) or this instanceof DereferenceableExpr and @@ -1037,9 +1041,9 @@ module Internal { ck.isInequality() and branch = false ) or - result = any(PatternMatch pm | - pm.getExpr() = e1 and - e2 = pm.getPattern().(ConstantPatternExpr) and + result = any(IsExpr ie | + ie.getExpr() = e1 and + e2 = ie.getPattern().(ConstantPatternExpr) and branch = true ) ) @@ -1068,11 +1072,11 @@ module Internal { * then `o` is guaranteed to be equal to `""`. */ private Expr getAMatchingEqualityCheck(Expr e1, MatchValue v, Expr e2) { - exists(Switch s, ConstCase case | case = v.getCase() | + exists(Switch s, Case case | case = v.getCase() | e1 = s.getExpr() and result = e1 and case = s.getACase() and - e2 = case.getPattern() and + e2 = case.getPattern().(ConstantPatternExpr) and v.isMatch() ) } diff --git a/csharp/ql/test/library-tests/controlflow/guards/ExtractorOptions.cs b/csharp/ql/test/library-tests/controlflow/guards/ExtractorOptions.cs index 5960523e227e..55b4f2fb2084 100644 --- a/csharp/ql/test/library-tests/controlflow/guards/ExtractorOptions.cs +++ b/csharp/ql/test/library-tests/controlflow/guards/ExtractorOptions.cs @@ -1 +1 @@ -// semmle-extractor-options: --cil +// semmle-extractor-options: --cil /langversion:8.0 diff --git a/csharp/ql/test/library-tests/controlflow/guards/GuardedControlFlowNode.expected b/csharp/ql/test/library-tests/controlflow/guards/GuardedControlFlowNode.expected index 08ed25c8efd1..28191f68ec9f 100644 --- a/csharp/ql/test/library-tests/controlflow/guards/GuardedControlFlowNode.expected +++ b/csharp/ql/test/library-tests/controlflow/guards/GuardedControlFlowNode.expected @@ -168,6 +168,22 @@ | Guards.cs:269:11:269:12 | access to parameter o1 | Guards.cs:268:13:268:41 | call to operator == | Guards.cs:268:13:268:14 | access to parameter o1 | true | | Guards.cs:269:11:269:12 | access to parameter o1 | Guards.cs:268:16:268:25 | call to method GetType | Guards.cs:268:13:268:14 | access to parameter o1 | non-null | | Guards.cs:271:11:271:12 | access to parameter o1 | Guards.cs:270:13:270:42 | call to operator == | Guards.cs:270:13:270:14 | access to parameter o1 | true | +| Guards.cs:279:17:279:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | match Action a | +| Guards.cs:279:17:279:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | non-null | +| Guards.cs:283:17:283:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | match "" | +| Guards.cs:283:17:283:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | non-match Action a | +| Guards.cs:283:17:283:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | non-match Action a | +| Guards.cs:283:17:283:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | non-null | +| Guards.cs:285:17:285:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | match null | +| Guards.cs:285:17:285:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | non-match "" | +| Guards.cs:285:17:285:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | non-match Action a | +| Guards.cs:285:17:285:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | non-match Action a | +| Guards.cs:285:17:285:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | null | +| Guards.cs:287:17:287:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | non-match "" | +| Guards.cs:287:17:287:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | non-match Action a | +| Guards.cs:287:17:287:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | non-match Action a | +| Guards.cs:287:17:287:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | non-match null | +| Guards.cs:287:17:287:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | non-null | | Splitting.cs:13:17:13:17 | [b (line 9): true] access to parameter o | Splitting.cs:12:17:12:17 | access to parameter o | Splitting.cs:12:17:12:17 | access to parameter o | non-null | | Splitting.cs:13:17:13:17 | [b (line 9): true] access to parameter o | Splitting.cs:12:17:12:25 | ... != ... | Splitting.cs:12:17:12:17 | access to parameter o | true | | Splitting.cs:14:13:14:13 | [b (line 9): false] access to parameter b | Splitting.cs:11:13:11:13 | access to parameter b | Splitting.cs:11:13:11:13 | access to parameter b | false | diff --git a/csharp/ql/test/library-tests/controlflow/guards/GuardedExpr.expected b/csharp/ql/test/library-tests/controlflow/guards/GuardedExpr.expected index 218641fda7a6..5a59ee7ea595 100644 --- a/csharp/ql/test/library-tests/controlflow/guards/GuardedExpr.expected +++ b/csharp/ql/test/library-tests/controlflow/guards/GuardedExpr.expected @@ -168,6 +168,22 @@ | Guards.cs:269:11:269:12 | access to parameter o1 | Guards.cs:268:13:268:41 | call to operator == | Guards.cs:268:13:268:14 | access to parameter o1 | true | | Guards.cs:269:11:269:12 | access to parameter o1 | Guards.cs:268:16:268:25 | call to method GetType | Guards.cs:268:13:268:14 | access to parameter o1 | non-null | | Guards.cs:271:11:271:12 | access to parameter o1 | Guards.cs:270:13:270:42 | call to operator == | Guards.cs:270:13:270:14 | access to parameter o1 | true | +| Guards.cs:279:17:279:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | match Action a | +| Guards.cs:279:17:279:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | non-null | +| Guards.cs:283:17:283:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | match "" | +| Guards.cs:283:17:283:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | non-match Action a | +| Guards.cs:283:17:283:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | non-match Action a | +| Guards.cs:283:17:283:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | non-null | +| Guards.cs:285:17:285:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | match null | +| Guards.cs:285:17:285:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | non-match "" | +| Guards.cs:285:17:285:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | non-match Action a | +| Guards.cs:285:17:285:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | non-match Action a | +| Guards.cs:285:17:285:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | null | +| Guards.cs:287:17:287:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | non-match "" | +| Guards.cs:287:17:287:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | non-match Action a | +| Guards.cs:287:17:287:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | non-match Action a | +| Guards.cs:287:17:287:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | non-match null | +| Guards.cs:287:17:287:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | non-null | | Splitting.cs:13:17:13:17 | access to parameter o | Splitting.cs:12:17:12:17 | access to parameter o | Splitting.cs:12:17:12:17 | access to parameter o | non-null | | Splitting.cs:13:17:13:17 | access to parameter o | Splitting.cs:12:17:12:25 | ... != ... | Splitting.cs:12:17:12:17 | access to parameter o | true | | Splitting.cs:23:24:23:24 | access to parameter o | Splitting.cs:22:17:22:17 | access to parameter o | Splitting.cs:22:17:22:17 | access to parameter o | non-null | diff --git a/csharp/ql/test/library-tests/controlflow/guards/Guards.cs b/csharp/ql/test/library-tests/controlflow/guards/Guards.cs index 5040d16fccec..404e179b3a90 100644 --- a/csharp/ql/test/library-tests/controlflow/guards/Guards.cs +++ b/csharp/ql/test/library-tests/controlflow/guards/Guards.cs @@ -270,4 +270,69 @@ void M22(object o1, object o2) if (o1?.GetType() == o2?.GetType()) o1.ToString(); // not null guarded } + + string M23(object o) + { + return o switch + { + Action a => + o.ToString(), // null guarded + Action a => + a.ToString(), // not null (but not a guard) + "" => + o.ToString(), // null guarded + null => + o.ToString(), // not null guarded + _ => + o.ToString() // null guarded + }; + } + + int M24(bool b1) + { + var b2 = true; + if (b1) + b2 = false; + return b2 switch + { + true => 0, + _ => 1 + }; + } + + int M25(bool b1) + { + var b2 = false; + if (b1) + b2 = true; + return b2 switch + { + true => 0, + _ => 1 + }; + } + + int M26(bool b) + { + var i = 0; + if (b) + i = 1; + return i switch + { + 1 => 0, + _ => 1 + }; + } + + int M27(bool b) + { + var e = E.A; + if (b) + e = E.B; + return e switch + { + E.B => 0, + _ => 1 + }; + } } diff --git a/csharp/ql/test/library-tests/controlflow/guards/Implications.expected b/csharp/ql/test/library-tests/controlflow/guards/Implications.expected index 6e3bbf402d87..05cdb7319eec 100644 --- a/csharp/ql/test/library-tests/controlflow/guards/Implications.expected +++ b/csharp/ql/test/library-tests/controlflow/guards/Implications.expected @@ -349,6 +349,32 @@ | Guards.cs:268:16:268:25 | call to method GetType | non-null | Guards.cs:268:13:268:14 | access to parameter o1 | non-null | | Guards.cs:270:16:270:25 | call to method GetType | non-null | Guards.cs:270:13:270:14 | access to parameter o1 | non-null | | Guards.cs:270:33:270:42 | call to method GetType | non-null | Guards.cs:270:30:270:31 | access to parameter o2 | non-null | +| Guards.cs:276:16:276:16 | access to parameter o | match "" | Guards.cs:276:16:276:16 | access to parameter o | non-null | +| Guards.cs:276:16:276:16 | access to parameter o | match Action a | Guards.cs:276:16:276:16 | access to parameter o | non-null | +| Guards.cs:276:16:276:16 | access to parameter o | match Action a | Guards.cs:276:16:276:16 | access to parameter o | non-null | +| Guards.cs:276:16:276:16 | access to parameter o | match null | Guards.cs:276:16:276:16 | access to parameter o | null | +| Guards.cs:276:16:276:16 | access to parameter o | non-match null | Guards.cs:276:16:276:16 | access to parameter o | non-null | +| Guards.cs:281:17:281:17 | access to local variable a | non-null | Guards.cs:276:16:276:16 | access to parameter o | non-null | +| Guards.cs:281:17:281:17 | access to local variable a | null | Guards.cs:276:16:276:16 | access to parameter o | null | +| Guards.cs:293:13:293:21 | Boolean b2 = ... | false | Guards.cs:293:13:293:14 | access to local variable b2 | false | +| Guards.cs:293:13:293:21 | Boolean b2 = ... | true | Guards.cs:293:13:293:14 | access to local variable b2 | true | +| Guards.cs:295:13:295:22 | ... = ... | false | Guards.cs:295:13:295:14 | access to local variable b2 | false | +| Guards.cs:295:13:295:22 | ... = ... | true | Guards.cs:295:13:295:14 | access to local variable b2 | true | +| Guards.cs:296:16:296:17 | access to local variable b2 | match true | Guards.cs:294:13:294:14 | access to parameter b1 | false | +| Guards.cs:296:16:296:17 | access to local variable b2 | match true | Guards.cs:296:16:296:17 | access to local variable b2 | true | +| Guards.cs:305:13:305:22 | Boolean b2 = ... | false | Guards.cs:305:13:305:14 | access to local variable b2 | false | +| Guards.cs:305:13:305:22 | Boolean b2 = ... | true | Guards.cs:305:13:305:14 | access to local variable b2 | true | +| Guards.cs:307:13:307:21 | ... = ... | false | Guards.cs:307:13:307:14 | access to local variable b2 | false | +| Guards.cs:307:13:307:21 | ... = ... | true | Guards.cs:307:13:307:14 | access to local variable b2 | true | +| Guards.cs:308:16:308:17 | access to local variable b2 | match true | Guards.cs:306:13:306:14 | access to parameter b1 | true | +| Guards.cs:308:16:308:17 | access to local variable b2 | match true | Guards.cs:308:16:308:17 | access to local variable b2 | true | +| Guards.cs:308:16:308:17 | access to local variable b2 | non-match true | Guards.cs:306:13:306:14 | access to parameter b1 | false | +| Guards.cs:320:16:320:16 | access to local variable i | match 1 | Guards.cs:318:13:318:13 | access to parameter b | true | +| Guards.cs:320:16:320:16 | access to local variable i | match 1 | Guards.cs:320:16:320:16 | access to local variable i | 1 | +| Guards.cs:320:16:320:16 | access to local variable i | non-match 1 | Guards.cs:318:13:318:13 | access to parameter b | false | +| Guards.cs:332:16:332:16 | access to local variable e | match access to constant B | Guards.cs:330:13:330:13 | access to parameter b | true | +| Guards.cs:332:16:332:16 | access to local variable e | match access to constant B | Guards.cs:332:16:332:16 | access to local variable e | 1 | +| Guards.cs:332:16:332:16 | access to local variable e | non-match access to constant B | Guards.cs:330:13:330:13 | access to parameter b | false | | Splitting.cs:12:17:12:25 | ... != ... | false | Splitting.cs:12:17:12:17 | access to parameter o | null | | Splitting.cs:12:17:12:25 | ... != ... | true | Splitting.cs:12:17:12:17 | access to parameter o | non-null | | Splitting.cs:22:17:22:25 | ... != ... | false | Splitting.cs:22:17:22:17 | access to parameter o | null | diff --git a/csharp/ql/test/library-tests/controlflow/guards/MatchingGuardedExpr.expected b/csharp/ql/test/library-tests/controlflow/guards/MatchingGuardedExpr.expected index b3b35e284566..8d901cb09eaf 100644 --- a/csharp/ql/test/library-tests/controlflow/guards/MatchingGuardedExpr.expected +++ b/csharp/ql/test/library-tests/controlflow/guards/MatchingGuardedExpr.expected @@ -10,3 +10,15 @@ | Guards.cs:162:24:162:24 | access to parameter o | Guards.cs:151:17:151:17 | access to parameter o | Guards.cs:155:13:155:34 | case ...: | non-match Action a | false | | Guards.cs:162:24:162:24 | access to parameter o | Guards.cs:151:17:151:17 | access to parameter o | Guards.cs:157:13:157:20 | case ...: | non-match "" | false | | Guards.cs:162:24:162:24 | access to parameter o | Guards.cs:151:17:151:17 | access to parameter o | Guards.cs:159:13:159:22 | case ...: | non-match null | false | +| Guards.cs:279:17:279:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:278:13:279:28 | ... => ... | match Action a | true | +| Guards.cs:283:17:283:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:278:13:279:28 | ... => ... | non-match Action a | false | +| Guards.cs:283:17:283:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:280:13:281:28 | ... => ... | non-match Action a | false | +| Guards.cs:283:17:283:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:282:13:283:28 | ... => ... | match "" | true | +| Guards.cs:285:17:285:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:278:13:279:28 | ... => ... | non-match Action a | false | +| Guards.cs:285:17:285:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:280:13:281:28 | ... => ... | non-match Action a | false | +| Guards.cs:285:17:285:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:282:13:283:28 | ... => ... | non-match "" | false | +| Guards.cs:285:17:285:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:284:13:285:28 | ... => ... | match null | true | +| Guards.cs:287:17:287:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:278:13:279:28 | ... => ... | non-match Action a | false | +| Guards.cs:287:17:287:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:280:13:281:28 | ... => ... | non-match Action a | false | +| Guards.cs:287:17:287:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:282:13:283:28 | ... => ... | non-match "" | false | +| Guards.cs:287:17:287:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:284:13:285:28 | ... => ... | non-match null | false | diff --git a/csharp/ql/test/library-tests/controlflow/guards/NullGuardedExpr.expected b/csharp/ql/test/library-tests/controlflow/guards/NullGuardedExpr.expected index a9f767b48ecb..41403ac74f05 100644 --- a/csharp/ql/test/library-tests/controlflow/guards/NullGuardedExpr.expected +++ b/csharp/ql/test/library-tests/controlflow/guards/NullGuardedExpr.expected @@ -42,6 +42,9 @@ | Guards.cs:205:13:205:13 | access to parameter o | | Guards.cs:208:17:208:17 | access to parameter o | | Guards.cs:269:11:269:12 | access to parameter o1 | +| Guards.cs:279:17:279:17 | access to parameter o | +| Guards.cs:283:17:283:17 | access to parameter o | +| Guards.cs:287:17:287:17 | access to parameter o | | Splitting.cs:13:17:13:17 | access to parameter o | | Splitting.cs:23:24:23:24 | access to parameter o | | Splitting.cs:35:13:35:13 | access to parameter o |