C#: Replace initializer splitting with ObjectInitMethod.

Author: aschackmull

csharp/ql/lib/semmle/code/csharp/Callable.qll

@@ -281,7 +281,6 @@ class Method extends Callable, Virtualizable, Attributable, @method {
   /** Holds if this method has a `params` parameter. */
   predicate hasParams() { exists(this.getParamsType()) }
 
-  // Remove when `Callable.isOverridden()` is removed
   override predicate fromSource() {
     Callable.super.fromSource() and
     not this.isCompilerGenerated()
@@ -317,6 +316,19 @@ class ExtensionMethod extends Method {
   override string getAPrimaryQlClass() { result = "ExtensionMethod" }
 }
 
+/**
+ * An object initializer method.
+ *
+ * This is an extractor-synthesized method that executes the field
+ * initializers. Note that the AST nodes for the field initializers are nested
+ * directly under the class, and therefore this method has no body in the AST.
+ * On the other hand, this provides the unique enclosing callable for the field
+ * initializers and their control flow graph.
+ */
+class ObjectInitMethod extends Method {
+  ObjectInitMethod() { this.getName() = "<object initializer>" }
+}
+
 /**
  * A constructor, for example `public C() { }` on line 2 in
  *
@@ -350,6 +362,9 @@ class Constructor extends Callable, Member, Attributable, @constructor {
    */
   ConstructorInitializer getInitializer() { result = this.getChildExpr(-1) }
 
+  /** Gets the object initializer call of this constructor, if any. */
+  MethodCall getObjectInitializerCall() { result = this.getChildExpr(-2) }
+
   /** Holds if this constructor has an initializer. */
   predicate hasInitializer() { exists(this.getInitializer()) }
 

csharp/ql/lib/semmle/code/csharp/ExprOrStmtParent.qll

@@ -55,7 +55,8 @@ class TopLevelExprParent extends Element, @top_level_expr_parent {
 /** INTERNAL: Do not use. */
 Expr getExpressionBody(Callable c) {
   result = c.getAChildExpr() and
-  not result = c.(Constructor).getInitializer()
+  not result = c.(Constructor).getInitializer() and
+  not result = c.(Constructor).getObjectInitializerCall()
 }
 
 /** INTERNAL: Do not use. */
@@ -211,6 +212,8 @@ private module Cached {
     enclosingBody(cfe, getBody(c))
     or
     parent*(enclosingStart(cfe), c.(Constructor).getInitializer())
+    or
+    parent*(cfe, c.(Constructor).getObjectInitializerCall())
   }
 
   /** Holds if the enclosing statement of expression `e` is `s`. */

csharp/ql/lib/semmle/code/csharp/controlflow/internal/ControlFlowGraphImpl.qll

@@ -19,7 +19,7 @@ class CfgScope extends Element, @top_level_exprorstmt_parent {
         any(Callable c |
           c.(Constructor).hasInitializer()
           or
-          InitializerSplitting::constructorInitializes(c, _)
+          InitializerSplitting::obinitInitializes(c, _)
           or
           c.hasBody()
         )
@@ -146,14 +146,16 @@ private predicate expr_parent_top_level_adjusted2(
 predicate scopeFirst(CfgScope scope, AstNode first) {
   scope =
     any(Callable c |
-      if exists(c.(Constructor).getInitializer())
-      then first(c.(Constructor).getInitializer(), first)
+      if exists(c.(Constructor).getObjectInitializerCall())
+      then first(c.(Constructor).getObjectInitializerCall(), first)
       else
-        if InitializerSplitting::constructorInitializes(c, _)
-        then first(InitializerSplitting::constructorInitializeOrder(c, _, 0), first)
+        if exists(c.(Constructor).getInitializer())
+        then first(c.(Constructor).getInitializer(), first)
         else first(c.getBody(), first)
     )
   or
+  first(InitializerSplitting::initializedInstanceMemberOrder(scope, _, 0), first)
+  or
   expr_parent_top_level_adjusted2(any(Expr e | first(e, first)), _, scope) and
   not scope instanceof Callable
 }
@@ -165,14 +167,33 @@ predicate scopeLast(CfgScope scope, AstNode last, Completion c) {
       last(callable.getBody(), last, c) and
       not c instanceof GotoCompletion
       or
-      last(InitializerSplitting::lastConstructorInitializer(scope, _), last, c) and
+      last(callable.(Constructor).getInitializer(), last, c) and
       not callable.hasBody()
     )
   or
+  last(InitializerSplitting::lastInitializer(scope, _), last, c)
+  or
   expr_parent_top_level_adjusted2(any(Expr e | last(e, last, c)), _, scope) and
   not scope instanceof Callable
 }
 
+private class ObjectInitTree extends ControlFlowTree instanceof ObjectInitMethod {
+  final override predicate propagatesAbnormal(AstNode child) { none() }
+
+  final override predicate first(AstNode first) { none() }
+
+  final override predicate last(AstNode last, Completion c) { none() }
+
+  final override predicate succ(AstNode pred, AstNode succ, Completion c) {
+    exists(CompilationExt comp, int i |
+      // Flow from one member initializer to the next
+      last(InitializerSplitting::initializedInstanceMemberOrder(this, comp, i), pred, c) and
+      c instanceof NormalCompletion and
+      first(InitializerSplitting::initializedInstanceMemberOrder(this, comp, i + 1), succ)
+    )
+  }
+}
+
 private class ConstructorTree extends ControlFlowTree instanceof Constructor {
   final override predicate propagatesAbnormal(AstNode child) { none() }
 
@@ -187,18 +208,23 @@ private class ConstructorTree extends ControlFlowTree instanceof Constructor {
     comp = getCompilation(result.getFile())
   }
 
+  pragma[noinline]
+  private MethodCall getObjectInitializerCall(CompilationExt comp) {
+    result = super.getObjectInitializerCall() and
+    comp = getCompilation(result.getFile())
+  }
+
+  pragma[noinline]
+  private ConstructorInitializer getInitializer(CompilationExt comp) {
+    result = super.getInitializer() and
+    comp = getCompilation(result.getFile())
+  }
+
   final override predicate succ(AstNode pred, AstNode succ, Completion c) {
-    exists(CompilationExt comp, int i, AssignExpr ae |
-      ae = InitializerSplitting::constructorInitializeOrder(this, comp, i) and
-      last(ae, pred, c) and
-      c instanceof NormalCompletion
-    |
-      // Flow from one member initializer to the next
-      first(InitializerSplitting::constructorInitializeOrder(this, comp, i + 1), succ)
-      or
-      // Flow from last member initializer to constructor body
-      ae = InitializerSplitting::lastConstructorInitializer(this, comp) and
-      first(this.getBody(comp), succ)
+    exists(CompilationExt comp |
+      last(this.getObjectInitializerCall(comp), pred, c) and
+      c instanceof NormalCompletion and
+      first(this.getInitializer(comp), succ)
     )
   }
 }
@@ -837,13 +863,7 @@ module Expressions {
         last(this, pred, c) and
         con = super.getConstructor() and
         comp = getCompilation(this.getFile()) and
-        c instanceof NormalCompletion
-      |
-        // Flow from constructor initializer to first member initializer
-        first(InitializerSplitting::constructorInitializeOrder(con, comp, 0), succ)
-        or
-        // Flow from constructor initializer to first element of constructor body
-        not exists(InitializerSplitting::constructorInitializeOrder(con, comp, _)) and
+        c instanceof NormalCompletion and
         first(con.getBody(comp), succ)
       )
     }

csharp/ql/lib/semmle/code/csharp/controlflow/internal/Splitting.qll

@@ -22,14 +22,10 @@ private module Cached {
     }
 
   cached
-  newtype TSplitKind =
-    TInitializerSplitKind() or
-    TConditionalCompletionSplitKind()
+  newtype TSplitKind = TConditionalCompletionSplitKind()
 
   cached
-  newtype TSplit =
-    TInitializerSplit(Constructor c) { InitializerSplitting::constructorInitializes(c, _) } or
-    TConditionalCompletionSplit(ConditionalCompletion c)
+  newtype TSplit = TConditionalCompletionSplit(ConditionalCompletion c)
 }
 
 import Cached
@@ -44,8 +40,6 @@ class Split extends TSplit {
 }
 
 module InitializerSplitting {
-  private import semmle.code.csharp.ExprOrStmtParent
-
   /**
    * A non-static member with an initializer, for example a field `int Field = 0`.
    */
@@ -60,40 +54,28 @@ module InitializerSplitting {
 
     /** Gets the initializer expression. */
     AssignExpr getInitializer() { expr_parent_top_level(result, _, this) }
-
-    /**
-     * Gets a control flow element that is a syntactic descendant of the
-     * initializer expression.
-     */
-    AstNode getAnInitializerDescendant() {
-      result = this.getInitializer()
-      or
-      result = this.getAnInitializerDescendant().getAChild()
-    }
   }
 
   /**
-   * Holds if `c` is a non-static constructor that performs the initialization
+   * Holds if `obinit` is an object initializer method that performs the initialization
    * of a member via assignment `init`.
    */
-  predicate constructorInitializes(InstanceConstructor c, AssignExpr init) {
+  predicate obinitInitializes(ObjectInitMethod obinit, AssignExpr init) {
     exists(InitializedInstanceMember m |
-      c.isUnboundDeclaration() and
-      c.getDeclaringType().getAMember() = m and
-      not c.getInitializer().isThis() and
+      obinit.getDeclaringType().getAMember() = m and
       init = m.getInitializer()
     )
   }
 
   /**
-   * Gets the `i`th member initializer expression for non-static constructor `c`
+   * Gets the `i`th member initializer expression for object initializer method `obinit`
    * in compilation `comp`.
    */
-  AssignExpr constructorInitializeOrder(Constructor c, CompilationExt comp, int i) {
-    constructorInitializes(c, result) and
+  AssignExpr initializedInstanceMemberOrder(ObjectInitMethod obinit, CompilationExt comp, int i) {
+    obinitInitializes(obinit, result) and
     result =
       rank[i + 1](AssignExpr ae0, Location l |
-        constructorInitializes(c, ae0) and
+        obinitInitializes(obinit, ae0) and
         l = ae0.getLocation() and
         getCompilation(l.getFile()) = comp
       |
@@ -105,122 +87,12 @@ module InitializerSplitting {
    * Gets the last member initializer expression for non-static constructor `c`
    * in compilation `comp`.
    */
-  AssignExpr lastConstructorInitializer(Constructor c, CompilationExt comp) {
+  AssignExpr lastInitializer(ObjectInitMethod obinit, CompilationExt comp) {
     exists(int i |
-      result = constructorInitializeOrder(c, comp, i) and
-      not exists(constructorInitializeOrder(c, comp, i + 1))
+      result = initializedInstanceMemberOrder(obinit, comp, i) and
+      not exists(initializedInstanceMemberOrder(obinit, comp, i + 1))
     )
   }
-
-  /**
-   * A split for non-static member initializers belonging to a given non-static
-   * constructor. For example, in
-   *
-   * ```csharp
-   * class C
-   * {
-   *     int Field1 = 0;
-   *     int Field2 = Field1 + 1;
-   *     int Field3;
-   *
-   *     public C()
-   *     {
-   *         Field3 = 2;
-   *     }
-   *
-   *     public C(int i)
-   *     {
-   *         Field3 = 3;
-   *     }
-   * }
-   * ```
-   *
-   * the initializer expressions `Field1 = 0` and `Field2 = Field1 + 1` are split
-   * on the two constructors. This is in order to generate CFGs for the two
-   * constructors that mimic
-   *
-   * ```csharp
-   * public C()
-   * {
-   *     Field1 = 0;
-   *     Field2 = Field1 + 1;
-   *     Field3 = 2;
-   * }
-   * ```
-   *
-   * and
-   *
-   * ```csharp
-   * public C()
-   * {
-   *     Field1 = 0;
-   *     Field2 = Field1 + 1;
-   *     Field3 = 3;
-   * }
-   * ```
-   *
-   * respectively.
-   */
-  private class InitializerSplit extends Split, TInitializerSplit {
-    private Constructor c;
-
-    InitializerSplit() { this = TInitializerSplit(c) }
-
-    /** Gets the constructor. */
-    Constructor getConstructor() { result = c }
-
-    override string toString() { result = "" }
-  }
-
-  private class InitializerSplitKind extends SplitKind, TInitializerSplitKind {
-    override int getListOrder() { result = 0 }
-
-    override predicate isEnabled(AstNode cfe) { this.appliesTo(cfe) }
-
-    override string toString() { result = "Initializer" }
-  }
-
-  int getNextListOrder() { result = 1 }
-
-  private class InitializerSplitImpl extends SplitImpl instanceof InitializerSplit {
-    override InitializerSplitKind getKind() { any() }
-
-    override predicate hasEntry(AstNode pred, AstNode succ, Completion c) {
-      exists(ConstructorInitializer ci |
-        last(ci, pred, c) and
-        succ(pred, succ, c) and
-        succ = any(InitializedInstanceMember m).getAnInitializerDescendant() and
-        super.getConstructor() = ci.getConstructor()
-      )
-    }
-
-    override predicate hasEntryScope(CfgScope scope, AstNode first) {
-      scopeFirst(scope, first) and
-      scope = super.getConstructor() and
-      first = any(InitializedInstanceMember m).getAnInitializerDescendant()
-    }
-
-    override predicate hasExit(AstNode pred, AstNode succ, Completion c) {
-      this.appliesTo(pred) and
-      succ(pred, succ, c) and
-      not succ = any(InitializedInstanceMember m).getAnInitializerDescendant() and
-      succ.(ControlFlowElement).getEnclosingCallable() = super.getConstructor()
-    }
-
-    override predicate hasExitScope(CfgScope scope, AstNode last, Completion c) {
-      this.appliesTo(last) and
-      scopeLast(scope, last, c) and
-      scope = super.getConstructor()
-    }
-
-    override predicate hasSuccessor(AstNode pred, AstNode succ, Completion c) {
-      this.appliesSucc(pred, succ, c) and
-      succ =
-        any(InitializedInstanceMember m |
-          constructorInitializes(super.getConstructor(), m.getInitializer())
-        ).getAnInitializerDescendant()
-    }
-  }
 }
 
 module ConditionalCompletionSplitting {
@@ -249,7 +121,7 @@ module ConditionalCompletionSplitting {
   }
 
   private class ConditionalCompletionSplitKind_ extends SplitKind, TConditionalCompletionSplitKind {
-    override int getListOrder() { result = InitializerSplitting::getNextListOrder() }
+    override int getListOrder() { result = 0 }
 
     override predicate isEnabled(AstNode cfe) { this.appliesTo(cfe) }
 
@@ -312,6 +184,4 @@ module ConditionalCompletionSplitting {
       )
     }
   }
-
-  int getNextListOrder() { result = InitializerSplitting::getNextListOrder() + 1 }
 }

csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowDispatch.qll

@@ -16,7 +16,7 @@ private import semmle.code.csharp.internal.Location
  */
 Callable getCallableForDataFlow(Callable c) {
   result = c.getUnboundDeclaration() and
-  result.hasBody() and
+  (result.hasBody() or result instanceof ObjectInitMethod) and
   result.getFile().fromSource()
 }