C#: Improve CFG for constructors when there are multiple implementations

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

@@ -49,6 +49,27 @@ private import SuccessorType
 private import SuccessorTypes
 private import Splitting
 private import semmle.code.csharp.ExprOrStmtParent
+private import semmle.code.csharp.commons.Compilation
+
+/**
+ * A compilation.
+ *
+ * Unlike the standard `Compilation` class, this class also supports buildless
+ * extraction.
+ */
+newtype CompilationExt =
+  TCompilation(Compilation c) { not extractionIsStandalone() } or
+  TBuildless() { extractionIsStandalone() }
+
+/** Gets the compilation that source file `f` belongs to. */
+CompilationExt getCompilation(SourceFile f) {
+  exists(Compilation c |
+    f = c.getAFileCompiled() and
+    result = TCompilation(c)
+  )
+  or
+  result = TBuildless()
+}
 
 /** An element that defines a new CFG scope. */
 class CfgScope extends Element, @top_level_exprorstmt_parent {
@@ -135,10 +156,7 @@ predicate scopeFirst(CfgScope scope, ControlFlowElement first) {
       then first(c.(Constructor).getInitializer(), first)
       else
         if InitializerSplitting::constructorInitializes(c, _)
-        then
-          first(any(InitializerSplitting::InitializedInstanceMember m |
-              InitializerSplitting::constructorInitializeOrder(c, m, 0)
-            ).getInitializer(), first)
+        then first(InitializerSplitting::constructorInitializeOrder(c, _, 0), first)
         else first(c.getBody(), first)
     )
   or
@@ -152,36 +170,36 @@ predicate scopeLast(Callable scope, ControlFlowElement last, Completion c) {
   last(scope.getBody(), last, c) and
   not c instanceof GotoCompletion
   or
-  exists(InitializerSplitting::InitializedInstanceMember m |
-    m = InitializerSplitting::lastConstructorInitializer(scope) and
-    last(m.getInitializer(), last, c) and
-    not scope.hasBody()
-  )
+  last(InitializerSplitting::lastConstructorInitializer(scope, _), last, c) and
+  not scope.hasBody()
 }
 
-private class CallableTree extends ControlFlowTree, Callable {
+private class ConstructorTree extends ControlFlowTree, Constructor {
   final override predicate propagatesAbnormal(ControlFlowElement child) { none() }
 
   final override predicate first(ControlFlowElement first) { none() }
 
   final override predicate last(ControlFlowElement last, Completion c) { none() }
 
+  /** Gets the body of this constructor belonging to compilation `comp`. */
+  pragma[noinline]
+  ControlFlowElement getBody(CompilationExt comp) {
+    result = this.getBody() and
+    comp = getCompilation(result.getFile())
+  }
+
   final override predicate succ(ControlFlowElement pred, ControlFlowElement succ, Completion c) {
-    exists(Constructor con, InitializerSplitting::InitializedInstanceMember m, int i |
-      this = con and
-      last(m.getInitializer(), pred, c) and
-      c instanceof NormalCompletion and
-      InitializerSplitting::constructorInitializeOrder(con, m, i)
+    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
-      exists(InitializerSplitting::InitializedInstanceMember next |
-        InitializerSplitting::constructorInitializeOrder(con, next, i + 1) and
-        first(next.getInitializer(), succ)
-      )
+      first(InitializerSplitting::constructorInitializeOrder(this, comp, i + 1), succ)
       or
       // Flow from last member initializer to constructor body
-      m = InitializerSplitting::lastConstructorInitializer(con) and
-      first(con.getBody(), succ)
+      ae = InitializerSplitting::lastConstructorInitializer(this, comp) and
+      first(this.getBody(comp), succ)
     )
   }
 }
@@ -884,21 +902,18 @@ module Expressions {
         first(this.getChildElement(i + 1), succ)
       )
       or
-      exists(Constructor con |
+      exists(ConstructorTree con, CompilationExt comp |
         last(this, pred, c) and
         con = this.getConstructor() and
+        comp = getCompilation(this.getFile()) and
         c instanceof NormalCompletion
       |
         // Flow from constructor initializer to first member initializer
-        exists(InitializerSplitting::InitializedInstanceMember m |
-          InitializerSplitting::constructorInitializeOrder(con, m, 0)
-        |
-          first(m.getInitializer(), succ)
-        )
+        first(InitializerSplitting::constructorInitializeOrder(con, comp, 0), succ)
         or
         // Flow from constructor initializer to first element of constructor body
-        not InitializerSplitting::constructorInitializeOrder(con, _, _) and
-        first(con.getBody(), succ)
+        not exists(InitializerSplitting::constructorInitializeOrder(con, comp, _)) and
+        first(con.getBody(comp), succ)
       )
     }
   }

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

@@ -218,23 +218,23 @@ module InitializerSplitting {
    * A non-static member with an initializer, for example a field `int Field = 0`.
    */
   class InitializedInstanceMember extends Member {
-    private AssignExpr ae;
-
     InitializedInstanceMember() {
-      not this.isStatic() and
-      expr_parent_top_level_adjusted(ae, _, this) and
-      not ae = any(Callable c).getExpressionBody()
+      exists(AssignExpr ae |
+        not this.isStatic() and
+        expr_parent_top_level(ae, _, this) and
+        not ae = any(Callable c).getExpressionBody()
+      )
     }
 
     /** Gets the initializer expression. */
-    AssignExpr getInitializer() { result = ae }
+    AssignExpr getInitializer() { expr_parent_top_level(result, _, this) }
 
     /**
      * Gets a control flow element that is a syntactic descendant of the
      * initializer expression.
      */
     ControlFlowElement getAnInitializerDescendant() {
-      result = ae
+      result = this.getInitializer()
       or
       result = this.getAnInitializerDescendant().getAChild()
     }
@@ -242,35 +242,41 @@ module InitializerSplitting {
 
   /**
    * Holds if `c` is a non-static constructor that performs the initialization
-   * of member `m`.
+   * of a member via assignment `init`.
    */
-  predicate constructorInitializes(InstanceConstructor c, InitializedInstanceMember m) {
-    c.isUnboundDeclaration() and
-    c.getDeclaringType().getAMember() = m and
-    not c.getInitializer().isThis()
+  predicate constructorInitializes(InstanceConstructor c, AssignExpr init) {
+    exists(InitializedInstanceMember m |
+      c.isUnboundDeclaration() and
+      c.getDeclaringType().getAMember() = m and
+      not c.getInitializer().isThis() and
+      init = m.getInitializer()
+    )
   }
 
   /**
-   * Holds if `m` is the `i`th member initialized by non-static constructor `c`.
+   * Gets the `i`th member initializer expression for non-static constructor `c`
+   * in compilation `comp`.
    */
-  predicate constructorInitializeOrder(Constructor c, InitializedInstanceMember m, int i) {
-    constructorInitializes(c, m) and
-    m =
-      rank[i + 1](InitializedInstanceMember m0 |
-        constructorInitializes(c, m0)
+  AssignExpr constructorInitializeOrder(Constructor c, CompilationExt comp, int i) {
+    constructorInitializes(c, result) and
+    result =
+      rank[i + 1](AssignExpr ae0, Location l |
+        constructorInitializes(c, ae0) and
+        l = ae0.getLocation() and
+        getCompilation(l.getFile()) = comp
       |
-        m0
-        order by
-          m0.getLocation().getStartLine(), m0.getLocation().getStartColumn(),
-          m0.getLocation().getFile().getAbsolutePath()
+        ae0 order by l.getStartLine(), l.getStartColumn(), l.getFile().getAbsolutePath()
       )
   }
 
-  /** Gets the last member initialized by non-static constructor `c`. */
-  InitializedInstanceMember lastConstructorInitializer(Constructor c) {
+  /**
+   * Gets the last member initializer expression for non-static constructor `c`
+   * in compilation `comp`.
+   */
+  AssignExpr lastConstructorInitializer(Constructor c, CompilationExt comp) {
     exists(int i |
-      constructorInitializeOrder(c, result, i) and
-      not constructorInitializeOrder(c, _, i + 1)
+      result = constructorInitializeOrder(c, comp, i) and
+      not exists(constructorInitializeOrder(c, comp, i + 1))
     )
   }
 
@@ -379,8 +385,9 @@ module InitializerSplitting {
       this.appliesTo(pred) and
       succ(pred, succ, c) and
       succ =
-        any(InitializedInstanceMember m | constructorInitializes(this.getConstructor(), m))
-            .getAnInitializerDescendant()
+        any(InitializedInstanceMember m |
+          constructorInitializes(this.getConstructor(), m.getInitializer())
+        ).getAnInitializerDescendant()
     }
   }
 }
@@ -1204,14 +1211,12 @@ module BooleanSplitting {
     exists(Callable c, int r | c = kind.getEnclosingCallable() |
       result = r + ExceptionHandlerSplitting::getNextListOrder() - 1 and
       kind =
-        rank[r](BooleanSplitSubKind kind0 |
+        rank[r](BooleanSplitSubKind kind0, Location l |
           kind0.getEnclosingCallable() = c and
-          kind0.startsSplit(_)
+          kind0.startsSplit(_) and
+          l = kind0.getLocation()
         |
-          kind0
-          order by
-            kind0.getLocation().getStartLine(), kind0.getLocation().getStartColumn(),
-            kind0.toString()
+          kind0 order by l.getStartLine(), l.getStartColumn(), kind0.toString()
         )
     )
   }
@@ -1430,10 +1435,11 @@ module LoopSplitting {
     exists(Callable c, int r | c = enclosingCallable(loop) |
       result = r + BooleanSplitting::getNextListOrder() - 1 and
       loop =
-        rank[r](AnalyzableLoopStmt loop0 |
-          enclosingCallable(loop0) = c
+        rank[r](AnalyzableLoopStmt loop0, Location l |
+          enclosingCallable(loop0) = c and
+          l = loop0.getLocation()
         |
-          loop0 order by loop0.getLocation().getStartLine(), loop0.getLocation().getStartColumn()
+          loop0 order by l.getStartLine(), l.getStartColumn()
         )
     )
   }