C#: Unify goto completions

Author: hvitved

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

@@ -669,7 +669,10 @@ class ContinueStmt extends JumpStmt, @continue_stmt {
  * Either a `goto` label (`GotoLabelStmt`), a `goto case` (`GotoCaseStmt`), or
  * a `goto default` (`GotoDefaultStmt`).
  */
-class GotoStmt extends JumpStmt, @goto_any_stmt { }
+class GotoStmt extends JumpStmt, @goto_any_stmt {
+  /** Gets the label that this `goto` statement jumps to. */
+  string getLabel() { none() }
+}
 
 /**
  * A `goto` statement that jumps to a labeled statement, for example line 4 in
@@ -684,8 +687,7 @@ class GotoStmt extends JumpStmt, @goto_any_stmt { }
  * ```
  */
 class GotoLabelStmt extends GotoStmt, @goto_stmt {
-  /** Gets the label that this `goto` statement jumps to. */
-  string getLabel() { exprorstmt_name(this, result) }
+  override string getLabel() { exprorstmt_name(this, result) }
 
   override string toString() { result = "goto ...;" }
 
@@ -716,8 +718,7 @@ class GotoCaseStmt extends GotoStmt, @goto_case_stmt {
   /** Gets the constant expression that this `goto case` statement jumps to. */
   Expr getExpr() { result = this.getChild(0) }
 
-  /** Gets the label that this `goto case` statement jumps to. */
-  string getLabel() { result = getExpr().getValue() }
+  override string getLabel() { result = getExpr().getValue() }
 
   override string toString() { result = "goto case ...;" }
 }
@@ -740,6 +741,8 @@ class GotoCaseStmt extends GotoStmt, @goto_case_stmt {
  */
 class GotoDefaultStmt extends GotoStmt, @goto_default_stmt {
   override string toString() { result = "goto default;" }
+
+  override string getLabel() { result = "default" }
 }
 
 /**

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

@@ -1087,6 +1087,12 @@ module ControlFlow {
           )
       }
 
+      pragma[noinline]
+      private LabeledStmt getLabledStmt(string label, Callable c) {
+        result.getEnclosingCallable() = c and
+        label = result.getLabel()
+      }
+
       /**
        * Gets a potential last element executed within control flow element `cfe`,
        * as well as its completion.
@@ -1148,8 +1154,8 @@ module ControlFlow {
           rec = TLastRecSwitchAbnormalCompletion() and
           not c instanceof BreakCompletion and
           not c instanceof NormalCompletion and
-          not c instanceof GotoDefaultCompletion and
-          not c instanceof GotoCaseCompletion and
+          not getLabledStmt(c.(GotoCompletion).getLabel(), cfe.getEnclosingCallable()) instanceof
+            CaseStmt and
           c = c0
           or
           rec = TLastRecInvalidOperationException() and
@@ -1515,24 +1521,6 @@ module ControlFlow {
             c instanceof NormalCompletion and
             result = first(ss.getStmt(i + 1))
           )
-          or
-          exists(GotoCompletion gc |
-            cfe = last(ss.getAStmt(), gc) and
-            gc = c
-          |
-            // Flow from last element of a statement with a `goto default` completion
-            // to first element `default` statement
-            gc instanceof GotoDefaultCompletion and
-            result = first(ss.getDefaultCase())
-            or
-            // Flow from last element of a statement with a `goto case` completion
-            // to first element of relevant case
-            exists(ConstCase cc |
-              cc = ss.getAConstCase() and
-              cc.getLabel() = gc.(GotoCaseCompletion).getLabel() and
-              result = first(cc.getBody())
-            )
-          )
         )
         or
         exists(Case case |
@@ -1766,13 +1754,13 @@ module ControlFlow {
         or
         // Flow from element with `goto` completion to first element of relevant
         // target
-        c = any(GotoLabelCompletion glc |
-            cfe = last(_, glc) and
+        c = any(GotoCompletion gc |
+            cfe = last(_, gc) and
             // Special case: when a `goto` happens inside a `try` statement with a
             // `finally` block, flow does not go directly to the target, but instead
             // to the `finally` block (and from there possibly to the target)
             not cfe = getBlockOrCatchFinallyPred(any(TryStmt ts | ts.hasFinally()), _) and
-            result = first(glc.getGotoStmt().getTarget())
+            result = first(getLabledStmt(gc.getLabel(), cfe.getEnclosingCallable()))
           )
         or
         // Standard left-to-right evaluation

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

@@ -35,9 +35,7 @@ private newtype TCompletion =
   TBreakCompletion() or
   TBreakNormalCompletion() or
   TContinueCompletion() or
-  TGotoLabelCompletion(GotoLabelStmt goto) or
-  TGotoCaseCompletion(GotoCaseStmt goto) or
-  TGotoDefaultCompletion() or
+  TGotoCompletion(string label) { label = any(GotoStmt gs).getLabel() } or
   TThrowCompletion(ExceptionClass ec) or
   TExitCompletion() or
   TNestedCompletion(NormalCompletion inner, Completion outer) {
@@ -47,11 +45,7 @@ private newtype TCompletion =
     or
     outer = TContinueCompletion()
     or
-    outer = TGotoLabelCompletion(_)
-    or
-    outer = TGotoCaseCompletion(_)
-    or
-    outer = TGotoDefaultCompletion()
+    outer = TGotoCompletion(_)
     or
     outer = TThrowCompletion(_)
     or
@@ -122,22 +116,17 @@ class Completion extends TCompletion {
                   if cfe instanceof ContinueStmt
                   then this = TContinueCompletion()
                   else
-                    if cfe instanceof GotoDefaultStmt
-                    then this = TGotoDefaultCompletion()
+                    if cfe instanceof GotoStmt
+                    then this = TGotoCompletion(cfe.(GotoStmt).getLabel())
                     else
-                      if cfe instanceof GotoStmt
-                      then
-                        this = TGotoLabelCompletion(cfe) or
-                        this = TGotoCaseCompletion(cfe)
+                      if cfe instanceof ReturnStmt
+                      then this = TReturnCompletion()
                       else
-                        if cfe instanceof ReturnStmt
-                        then this = TReturnCompletion()
-                        else
-                          if cfe instanceof YieldBreakStmt
-                          then
-                            // `yield break` behaves like a return statement
-                            this = TReturnCompletion()
-                          else this = TNormalCompletion()
+                        if cfe instanceof YieldBreakStmt
+                        then
+                          // `yield break` behaves like a return statement
+                          this = TReturnCompletion()
+                        else this = TNormalCompletion()
     )
   }
 
@@ -726,69 +715,20 @@ class ContinueCompletion extends Completion {
  * A completion that represents evaluation of a statement or an
  * expression resulting in a `goto` jump.
  */
-abstract class GotoCompletion extends Completion { }
-
-/**
- * A completion that represents evaluation of a statement or an
- * expression resulting in a `goto label` jump.
- */
-class GotoLabelCompletion extends GotoCompletion {
-  private GotoLabelStmt goto;
-
-  GotoLabelCompletion() {
-    this = TGotoLabelCompletion(goto) or
-    this = TNestedCompletion(_, TGotoLabelCompletion(goto))
-  }
-
-  /** Gets the target of the `goto label` completion. */
-  string getLabel() { result = this.getGotoStmt().getLabel() }
-
-  /** Gets the statement that resulted in this `goto label` completion. */
-  GotoLabelStmt getGotoStmt() { result = goto }
-
-  override string toString() {
-    // `NestedCompletion` defines `toString()` for the other case
-    this = TGotoLabelCompletion(goto) and result = "goto(" + this.getLabel() + ")"
-  }
-}
-
-/**
- * A completion that represents evaluation of a statement or an
- * expression resulting in a `goto case` jump.
- */
-class GotoCaseCompletion extends GotoCompletion {
-  private GotoCaseStmt goto;
+class GotoCompletion extends Completion {
+  private string label;
 
-  GotoCaseCompletion() {
-    this = TGotoCaseCompletion(goto) or
-    this = TNestedCompletion(_, TGotoCaseCompletion(goto))
+  GotoCompletion() {
+    this = TGotoCompletion(label) or
+    this = TNestedCompletion(_, TGotoCompletion(label))
   }
 
-  /** Gets the target of the `goto case` completion. */
-  string getLabel() { result = this.getGotoStmt().getLabel() }
-
-  /** Gets the statement that resulted in this `goto case` completion. */
-  GotoCaseStmt getGotoStmt() { result = goto }
-
-  override string toString() {
-    // `NestedCompletion` defines `toString()` for the other case
-    this = TGotoCaseCompletion(goto) and result = "goto case(" + this.getLabel() + ")"
-  }
-}
-
-/**
- * A completion that represents evaluation of a statement or an
- * expression resulting in a `goto default` jump.
- */
-class GotoDefaultCompletion extends GotoCompletion {
-  GotoDefaultCompletion() {
-    this = TGotoDefaultCompletion() or
-    this = TNestedCompletion(_, TGotoDefaultCompletion())
-  }
+  /** Gets the label of the `goto` completion. */
+  string getLabel() { result = label }
 
   override string toString() {
     // `NestedCompletion` defines `toString()` for the other case
-    this = TGotoDefaultCompletion() and result = "goto default"
+    this = TGotoCompletion(label) and result = "goto(" + label + ")"
   }
 }
 

csharp/ql/src/semmle/code/csharp/controlflow/internal/SuccessorType.qll

@@ -18,9 +18,7 @@ private newtype TSuccessorType =
   TReturnSuccessor() or
   TBreakSuccessor() or
   TContinueSuccessor() or
-  TGotoLabelSuccessor(GotoLabelStmt goto) or
-  TGotoCaseSuccessor(GotoCaseStmt goto) or
-  TGotoDefaultSuccessor() or
+  TGotoSuccessor(string label) { label = any(GotoStmt gs).getLabel() } or
   TExceptionSuccessor(ExceptionClass ec) { exists(ThrowCompletion c | c.getExceptionClass() = ec) } or
   TExitSuccessor()
 
@@ -300,7 +298,7 @@ module SuccessorTypes {
   }
 
   /**
-   * A `goto label` control flow successor.
+   * A `goto` control flow successor.
    *
    * Example:
    *
@@ -319,68 +317,17 @@ module SuccessorTypes {
    * The node `Return: return x` is a `goto label` successor of the node
    * `goto Return;`.
    */
-  class GotoLabelSuccessor extends SuccessorType, TGotoLabelSuccessor {
-    /** Gets the statement that resulted in this `goto` successor. */
-    GotoLabelStmt getGotoStmt() { this = TGotoLabelSuccessor(result) }
+  class GotoSuccessor extends SuccessorType, TGotoSuccessor {
+    /** Gets the `goto` label. */
+    string getLabel() { this = TGotoSuccessor(result) }
 
-    override string toString() { result = "goto(" + getGotoStmt().getLabel() + ")" }
+    override string toString() { result = "goto(" + this.getLabel() + ")" }
 
     override predicate matchesCompletion(Completion c) {
-      c.(GotoLabelCompletion).getGotoStmt() = getGotoStmt()
+      c.(GotoCompletion).getLabel() = this.getLabel()
     }
   }
 
-  /**
-   * A `goto case` control flow successor.
-   *
-   * Example:
-   *
-   * ```
-   * switch (x)
-   * {
-   *     case 0  : return 1;
-   *     case 1  : goto case 0;
-   *     default : return -1;
-   * }
-   * ```
-   *
-   * The node `case 0  : return 1;` is a  `goto case` successor of the node
-   * `goto case 0;`.
-   */
-  class GotoCaseSuccessor extends SuccessorType, TGotoCaseSuccessor {
-    /** Gets the statement that resulted in this `goto case` successor. */
-    GotoCaseStmt getGotoStmt() { this = TGotoCaseSuccessor(result) }
-
-    override string toString() { result = "goto(" + getGotoStmt().getLabel() + ")" }
-
-    override predicate matchesCompletion(Completion c) {
-      c.(GotoCaseCompletion).getGotoStmt() = getGotoStmt()
-    }
-  }
-
-  /**
-   * A `goto default` control flow successor.
-   *
-   * Example:
-   *
-   * ```
-   * switch (x)
-   * {
-   *     case 0  : return 1;
-   *     case 1  : goto default;
-   *     default : return -1;
-   * }
-   * ```
-   *
-   * The node `default : return -1;` is a `goto default` successor of the node
-   * `goto default;`.
-   */
-  class GotoDefaultSuccessor extends SuccessorType, TGotoDefaultSuccessor {
-    override string toString() { result = "goto default" }
-
-    override predicate matchesCompletion(Completion c) { c instanceof GotoDefaultCompletion }
-  }
-
   /**
    * An exceptional control flow successor.
    *

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

@@ -425,8 +425,9 @@
 | cflow.cs:30:18:33:37 | if (...) ... | cflow.cs:30:22:30:31 | ... == ... | 6 |
 | cflow.cs:31:17:31:42 | ...; | cflow.cs:31:17:31:41 | call to method WriteLine | 3 |
 | cflow.cs:33:17:33:37 | ...; | cflow.cs:33:17:33:36 | call to method WriteLine | 3 |
-| cflow.cs:37:17:37:22 | enter Switch | cflow.cs:41:18:41:18 | 1 | 6 |
+| cflow.cs:37:17:37:22 | enter Switch | cflow.cs:39:17:39:17 | access to parameter a | 4 |
 | cflow.cs:37:17:37:22 | exit Switch | cflow.cs:37:17:37:22 | exit Switch | 1 |
+| cflow.cs:41:13:41:19 | case ...: | cflow.cs:41:18:41:18 | 1 | 2 |
 | cflow.cs:42:17:42:39 | ...; | cflow.cs:43:17:43:28 | goto case ...; | 5 |
 | cflow.cs:44:13:44:19 | case ...: | cflow.cs:44:18:44:18 | 2 | 2 |
 | cflow.cs:45:17:45:39 | ...; | cflow.cs:46:17:46:28 | goto case ...; | 5 |