Merge pull request github#13595 from hvitved/csharp/use-shared-cfg-pack

C#: Adopt shared CFG construction library from shared `controlflow` pack

Author: hvitved

config/identical-files.json

@@ -483,10 +483,6 @@
     "ruby/ql/lib/codeql/ruby/security/internal/SensitiveDataHeuristics.qll",
     "swift/ql/lib/codeql/swift/security/internal/SensitiveDataHeuristics.qll"
   ],
-  "CFG": [
-    "csharp/ql/lib/semmle/code/csharp/controlflow/internal/ControlFlowGraphImplShared.qll",
-    "swift/ql/lib/codeql/swift/controlflow/internal/ControlFlowGraphImplShared.qll"
-  ],
   "TypeTracker": [
     "python/ql/lib/semmle/python/dataflow/new/internal/TypeTracker.qll",
     "ruby/ql/lib/codeql/ruby/typetracking/TypeTracker.qll"

csharp/ql/consistency-queries/CfgConsistency.ql

@@ -2,15 +2,14 @@ import csharp
 import semmle.code.csharp.controlflow.internal.Completion
 import semmle.code.csharp.controlflow.internal.PreBasicBlocks
 import ControlFlow
-import semmle.code.csharp.controlflow.internal.ControlFlowGraphImpl
+import semmle.code.csharp.controlflow.internal.ControlFlowGraphImpl::Consistency
 import semmle.code.csharp.controlflow.internal.Splitting
-import Consistency
 
 private predicate splitBB(ControlFlow::BasicBlock bb) {
   exists(ControlFlow::Node first |
     first = bb.getFirstNode() and
     first.isJoin() and
-    strictcount(first.getAPredecessor().getElement()) = 1
+    strictcount(first.getAPredecessor().getAstNode()) = 1
   )
 }
 

csharp/ql/consistency-queries/DataFlowConsistency.ql

@@ -10,7 +10,7 @@ private class MyConsistencyConfiguration extends ConsistencyConfiguration {
     // TODO: Remove once static initializers are folded into the
     // static constructors
     exists(ControlFlow::Node cfn |
-      cfn.getElement() = any(FieldOrProperty f | f.isStatic()).getAChild+() and
+      cfn.getAstNode() = any(FieldOrProperty f | f.isStatic()).getAChild+() and
       cfn = n.getControlFlowNode()
     )
   }
@@ -19,7 +19,7 @@ private class MyConsistencyConfiguration extends ConsistencyConfiguration {
     // TODO: Remove once static initializers are folded into the
     // static constructors
     exists(ControlFlow::Node cfn |
-      cfn.getElement() = any(FieldOrProperty f | f.isStatic()).getAChild+() and
+      cfn.getAstNode() = any(FieldOrProperty f | f.isStatic()).getAChild+() and
       cfn = call.getControlFlowNode()
     )
   }

csharp/ql/lib/qlpack.yml

@@ -6,6 +6,7 @@ extractor: csharp
 library: true
 upgrades: upgrades
 dependencies:
+  codeql/controlflow: ${workspace}
   codeql/dataflow: ${workspace}
   codeql/mad: ${workspace}
   codeql/ssa: ${workspace}

csharp/ql/lib/semmle/code/csharp/commons/Assertions.qll

@@ -1,12 +1,9 @@
 /** Provides classes for assertions. */
 
-private import semmle.code.csharp.controlflow.internal.ControlFlowGraphImpl
 private import semmle.code.csharp.frameworks.system.Diagnostics
 private import semmle.code.csharp.frameworks.system.diagnostics.Contracts
 private import semmle.code.csharp.frameworks.test.VisualStudio
 private import semmle.code.csharp.frameworks.System
-private import ControlFlow
-private import ControlFlow::BasicBlocks
 
 private newtype TAssertionFailure =
   TExceptionAssertionFailure(Class c) or

csharp/ql/lib/semmle/code/csharp/controlflow/BasicBlocks.qll

@@ -400,13 +400,13 @@ class ExitBasicBlock extends BasicBlock {
 
 private module JoinBlockPredecessors {
   private import ControlFlow::Nodes
-  private import semmle.code.csharp.controlflow.internal.ControlFlowGraphImpl
+  private import semmle.code.csharp.controlflow.internal.ControlFlowGraphImpl as Impl
 
   int getId(JoinBlockPredecessor jbp) {
-    exists(ControlFlowTree::Range_ t | ControlFlowTree::idOf(t, result) |
-      t = jbp.getFirstNode().getElement()
+    exists(Impl::AstNode n | result = n.getId() |
+      n = jbp.getFirstNode().getAstNode()
       or
-      t = jbp.(EntryBasicBlock).getCallable()
+      n = jbp.(EntryBasicBlock).getCallable()
     )
   }
 

csharp/ql/lib/semmle/code/csharp/controlflow/ControlFlowElement.qll

@@ -7,7 +7,7 @@ private import ControlFlow
 private import ControlFlow::BasicBlocks
 private import SuccessorTypes
 private import semmle.code.csharp.Caching
-private import internal.ControlFlowGraphImpl
+private import internal.ControlFlowGraphImpl as Impl
 
 /**
  * A program element that can possess control flow. That is, either a statement or
@@ -39,20 +39,20 @@ class ControlFlowElement extends ExprOrStmtParent, @control_flow_element {
    * several `ControlFlow::Node`s, for example to represent the continuation
    * flow in a `try/catch/finally` construction.
    */
-  Nodes::ElementNode getAControlFlowNode() { result.getElement() = this }
+  Nodes::ElementNode getAControlFlowNode() { result.getAstNode() = this }
 
   /**
    * Gets a first control flow node executed within this element.
    */
   Nodes::ElementNode getAControlFlowEntryNode() {
-    result = getAControlFlowEntryNode(this).getAControlFlowNode()
+    result = Impl::getAControlFlowEntryNode(this).(ControlFlowElement).getAControlFlowNode()
   }
 
   /**
    * Gets a potential last control flow node executed within this element.
    */
   Nodes::ElementNode getAControlFlowExitNode() {
-    result = getAControlFlowExitNode(this).getAControlFlowNode()
+    result = Impl::getAControlFlowExitNode(this).(ControlFlowElement).getAControlFlowNode()
   }
 
   /**
@@ -88,7 +88,7 @@ class ControlFlowElement extends ExprOrStmtParent, @control_flow_element {
   ) {
     // Only calculate dominance by explicit recursion for split nodes;
     // all other nodes can use regular CFG dominance
-    this instanceof SplitControlFlowElement and
+    this instanceof Impl::SplitAstNode and
     cb.getLastNode() = this.getAControlFlowNode() and
     succ = cb.getASuccessorByType(s)
   }
@@ -111,7 +111,7 @@ class ControlFlowElement extends ExprOrStmtParent, @control_flow_element {
       succ.dominates(pred)
       or
       // `pred` might be another split of this element
-      pred.getLastNode().getElement() = this and
+      pred.getLastNode().getAstNode() = this and
       t = s
     )
   }

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

@@ -7,7 +7,7 @@ module ControlFlow {
   private import semmle.code.csharp.controlflow.BasicBlocks as BBs
   import semmle.code.csharp.controlflow.internal.SuccessorType
   private import SuccessorTypes
-  private import internal.ControlFlowGraphImpl
+  private import internal.ControlFlowGraphImpl as Impl
   private import internal.Splitting as Splitting
 
   /**
@@ -25,18 +25,16 @@ module ControlFlow {
    * Only nodes that can be reached from the callable entry point are included in
    * the CFG.
    */
-  class Node extends TCfgNode {
-    /** Gets a textual representation of this control flow node. */
-    string toString() { none() }
-
+  class Node extends Impl::Node {
     /** Gets the control flow element that this node corresponds to, if any. */
-    ControlFlowElement getElement() { none() }
-
-    /** Gets the location of this control flow node. */
-    Location getLocation() { result = this.getElement().getLocation() }
+    final ControlFlowElement getAstNode() { result = super.getAstNode() }
 
-    /** Holds if this control flow node has conditional successors. */
-    predicate isCondition() { exists(this.getASuccessorByType(any(ConditionalSuccessor e))) }
+    /**
+     * DEPRECATED: Use `getAstNode` instead.
+     *
+     * Gets the control flow element that this node corresponds to, if any.
+     */
+    deprecated ControlFlowElement getElement() { result = this.getAstNode() }
 
     /** Gets the basic block that this control flow node belongs to. */
     BasicBlock getBasicBlock() { result.getANode() = this }
@@ -183,7 +181,7 @@ module ControlFlow {
     }
 
     /** Gets a successor node of a given type, if any. */
-    Node getASuccessorByType(SuccessorType t) { result = getASuccessor(this, t) }
+    Node getASuccessorByType(SuccessorType t) { result = this.getASuccessor(t) }
 
     /** Gets an immediate successor, if any. */
     Node getASuccessor() { result = this.getASuccessorByType(_) }
@@ -234,80 +232,39 @@ module ControlFlow {
       result = this.getASuccessorByType(any(BooleanSuccessor t | t.getValue() = false))
     }
 
-    /** Holds if this node has more than one predecessor. */
-    predicate isJoin() { strictcount(this.getAPredecessor()) > 1 }
-
-    /** Holds if this node has more than one successor. */
-    predicate isBranch() { strictcount(this.getASuccessor()) > 1 }
-
     /** Gets the enclosing callable of this control flow node. */
-    final Callable getEnclosingCallable() { result = getNodeCfgScope(this) }
+    final Callable getEnclosingCallable() { result = Impl::getNodeCfgScope(this) }
   }
 
   /** Provides different types of control flow nodes. */
   module Nodes {
     /** A node for a callable entry point. */
-    class EntryNode extends Node, TEntryNode {
+    class EntryNode extends Node instanceof Impl::EntryNode {
       /** Gets the callable that this entry applies to. */
-      Callable getCallable() { this = TEntryNode(result) }
+      Callable getCallable() { result = this.getScope() }
 
       override BasicBlocks::EntryBlock getBasicBlock() { result = Node.super.getBasicBlock() }
-
-      private Assignable getAssignable() { this = TEntryNode(result) }
-
-      override Location getLocation() {
-        result in [this.getCallable().getLocation(), this.getAssignable().getLocation()]
-      }
-
-      override string toString() {
-        result = "enter " + [this.getCallable().toString(), this.getAssignable().toString()]
-      }
     }
 
     /** A node for a callable exit point, annotated with the type of exit. */
-    class AnnotatedExitNode extends Node, TAnnotatedExitNode {
-      private CfgScope scope;
-      private boolean normal;
-
-      AnnotatedExitNode() { this = TAnnotatedExitNode(scope, normal) }
+    class AnnotatedExitNode extends Node instanceof Impl::AnnotatedExitNode {
+      /** Holds if this node represent a normal exit. */
+      final predicate isNormal() { super.isNormal() }
 
       /** Gets the callable that this exit applies to. */
-      CfgScope getCallable() { result = scope }
-
-      /** Holds if this node represents a normal exit. */
-      predicate isNormal() { normal = true }
+      Callable getCallable() { result = this.getScope() }
 
       override BasicBlocks::AnnotatedExitBlock getBasicBlock() {
         result = Node.super.getBasicBlock()
       }
-
-      override Location getLocation() { result = scope.getLocation() }
-
-      override string toString() {
-        exists(string s |
-          normal = true and s = "normal"
-          or
-          normal = false and s = "abnormal"
-        |
-          result = "exit " + scope + " (" + s + ")"
-        )
-      }
     }
 
     /** A node for a callable exit point. */
-    class ExitNode extends Node, TExitNode {
-      private CfgScope scope;
-
-      ExitNode() { this = TExitNode(scope) }
-
+    class ExitNode extends Node instanceof Impl::ExitNode {
       /** Gets the callable that this exit applies to. */
-      Callable getCallable() { result = scope }
+      Callable getCallable() { result = this.getScope() }
 
       override BasicBlocks::ExitBlock getBasicBlock() { result = Node.super.getBasicBlock() }
-
-      override Location getLocation() { result = scope.getLocation() }
-
-      override string toString() { result = "exit " + scope }
     }
 
     /**
@@ -317,35 +274,19 @@ module ControlFlow {
      * the element is in unreachable (dead) code, and multiple when there are
      * different splits for the element.
      */
-    class ElementNode extends Node, TElementNode {
-      private Splits splits;
-      private ControlFlowElement cfe;
-
-      ElementNode() { this = TElementNode(_, cfe, splits) }
-
-      override ControlFlowElement getElement() { result = cfe }
-
-      override string toString() {
-        result = "[" + this.getSplitsString() + "] " + cfe.toString()
-        or
-        not exists(this.getSplitsString()) and result = cfe.toString()
-      }
-
+    class ElementNode extends Node instanceof Impl::AstCfgNode {
       /** Gets a comma-separated list of strings for each split in this node, if any. */
-      string getSplitsString() {
-        result = splits.toString() and
-        result != ""
-      }
+      final string getSplitsString() { result = super.getSplitsString() }
 
       /** Gets a split for this control flow node, if any. */
-      Split getASplit() { result = splits.getASplit() }
+      final Split getASplit() { result = super.getASplit() }
     }
 
     /** A control-flow node for an expression. */
     class ExprNode extends ElementNode {
       Expr e;
 
-      ExprNode() { e = unique(Expr e_ | e_ = this.getElement() | e_) }
+      ExprNode() { e = unique(Expr e_ | e_ = this.getAstNode() | e_) }
 
       /** Gets the expression that this control-flow node belongs to. */
       Expr getExpr() { result = e }

csharp/ql/lib/semmle/code/csharp/controlflow/Guards.qll

@@ -1946,7 +1946,7 @@ module Internal {
       |
         def =
           guarded
-              .getElement()
+              .getAstNode()
               .(AccessOrCallExpr)
               .getAnSsaQualifier(guarded.getBasicBlock().getANode()) and
         if v.isReferentialProperty()