Merge pull request #76 from hvitved/csharp/cfg/generic-splitting

C#: Generic control flow graph splitting

Author: calumgrant

change-notes/1.18/analysis-csharp.md

@@ -8,8 +8,7 @@
 
 ## General improvements
 
-> Changes that affect alerts in many files or from many queries
-> For example, changes to file classification
+* Control flow analysis has been improved for `catch` clauses with filters.
 
 ## New queries
 

csharp/ql/src/semmle/code/csharp/Stmt.qll

@@ -971,6 +971,8 @@ class TryStmt extends Stmt, @try_stmt {
  * general `catch` clause (`GeneralCatchClause`).
  */
 class CatchClause extends Stmt, @catch {
+  /** Gets the `try` statement that this `catch` clause belongs to. */
+  TryStmt getTryStmt() { result.getACatchClause() = this }
 
   /** Gets the block of this `catch` clause. */
   BlockStmt getBlock() { result.getParent() = this }
@@ -1007,6 +1009,15 @@ class CatchClause extends Stmt, @catch {
 
   /** Holds if this `catch` clause has a filter. */
   predicate hasFilterClause() { exists(getFilterClause()) }
+
+  /** Holds if this is the last `catch` clause in the `try` statement that it belongs to. */
+  predicate isLast() {
+    exists(TryStmt ts, int last |
+      ts = this.getTryStmt() and
+      last = max(int i | exists(ts.getCatchClause(i))) and
+      this = ts.getCatchClause(last)
+    )
+  }
 }
 
 /**

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

@@ -101,7 +101,7 @@ class Completion extends TCompletion {
       or
       not isNullnessConstant(cfe, _) and
       this = TNullnessCompletion(_)
-    else if mustHaveMatchingCompletion(_, cfe) then
+    else if mustHaveMatchingCompletion(cfe) then
       exists(boolean value |
         isMatchingConstant(cfe, value) |
         this = TMatchingCompletion(value)
@@ -123,11 +123,6 @@ class Completion extends TCompletion {
     this instanceof NormalCompletion or
     this instanceof ContinueCompletion
   }
-
-  /** Holds if this completion is a valid completion for exiting a callable. */
-  predicate isValidCallableExitCompletion() {
-    not this instanceof GotoCompletion
-  }
 }
 
 /** Holds if expression `e` has the Boolean constant value `value`. */
@@ -319,6 +314,11 @@ private predicate inBooleanContext(Expr e, boolean isBooleanCompletionForParent)
     isBooleanCompletionForParent = false
   )
   or
+  exists(SpecificCatchClause scc |
+    scc.getFilterClause() = e |
+    isBooleanCompletionForParent = false
+  )
+  or
   exists(LogicalNotExpr lne |
     lne.getAnOperand() = e |
     inBooleanContext(lne, _) and
@@ -401,14 +401,20 @@ private predicate inNullnessContext(Expr e, boolean isNullnessCompletionForParen
   )
 }
 
+private predicate mustHaveMatchingCompletion(SwitchStmt ss, ControlFlowElement cfe) {
+  cfe = ss.getAConstCase().getExpr()
+  or
+  cfe = ss.getATypeCase().getTypeAccess() // use type access to represent the type test
+}
+
 /**
- * Holds if a normal completion of `e` must be a matching completion. Thats is,
- * whether `e` determines a match in a `switch` statement.
+ * Holds if a normal completion of `cfe` must be a matching completion. Thats is,
+ * whether `cfe` determines a match in a `switch` statement or `catch` clause.
  */
-private predicate mustHaveMatchingCompletion(SwitchStmt ss, Expr e) {
-  e = ss.getAConstCase().getExpr()
+private predicate mustHaveMatchingCompletion(ControlFlowElement cfe) {
+  mustHaveMatchingCompletion(_, cfe)
   or
-  e = ss.getATypeCase().getTypeAccess() // use type access to represent the type test
+  cfe instanceof SpecificCatchClause
 }
 
 /**