Merge pull request #21565 from aschackmull/csharp/cfg2

C#: Replace CFG with the shared implementation

Author: aschackmull

csharp/ql/campaigns/Solorigate/src/ModifiedFnvFunctionDetection.ql

@@ -13,11 +13,13 @@ import csharp
 import Solorigate
 import experimental.code.csharp.Cryptography.NonCryptographicHashes
 
+ControlFlowNode loopExitNode(LoopStmt loop) { result.isAfter(loop) }
+
 from Variable v, Literal l, LoopStmt loop, Expr additional_xor
 where
   maybeUsedInFnvFunction(v, _, _, loop) and
   exists(BitwiseXorOperation xor2 | xor2.getAnOperand() = l and additional_xor = xor2 |
-    loop.getAControlFlowExitNode().getASuccessor*() = xor2.getAControlFlowNode() and
+    loopExitNode(loop).getASuccessor*() = xor2.getControlFlowNode() and
     xor2.getAnOperand() = v.getAnAccess()
   )
 select l, "This literal is used in an $@ after an FNV-like hash calculation with variable $@.",

csharp/ql/consistency-queries/CfgConsistency.ql

@@ -1,5 +1,2 @@
 import csharp
-import semmle.code.csharp.controlflow.internal.Completion
-import ControlFlow
-import semmle.code.csharp.controlflow.internal.ControlFlowGraphImpl::Consistency
-import semmle.code.csharp.controlflow.internal.Splitting
+import ControlFlow::Consistency

csharp/ql/consistency-queries/DataFlowConsistency.ql

@@ -1,26 +1,11 @@
 import csharp
-private import semmle.code.csharp.controlflow.internal.ControlFlowGraphImpl as ControlFlowGraphImpl
 private import semmle.code.csharp.dataflow.internal.DataFlowImplSpecific
 private import semmle.code.csharp.dataflow.internal.TaintTrackingImplSpecific
 private import codeql.dataflow.internal.DataFlowImplConsistency
 
 private module Input implements InputSig<Location, CsharpDataFlow> {
   private import CsharpDataFlow
 
-  private predicate isStaticAssignable(Assignable a) { a.(Modifiable).isStatic() }
-
-  predicate uniqueEnclosingCallableExclude(Node node) {
-    // TODO: Remove once static initializers are folded into the
-    // static constructors
-    isStaticAssignable(ControlFlowGraphImpl::getNodeCfgScope(node.getControlFlowNode()))
-  }
-
-  predicate uniqueCallEnclosingCallableExclude(DataFlowCall call) {
-    // TODO: Remove once static initializers are folded into the
-    // static constructors
-    isStaticAssignable(ControlFlowGraphImpl::getNodeCfgScope(call.getControlFlowNode()))
-  }
-
   predicate uniqueNodeLocationExclude(Node n) {
     // Methods with multiple implementations
     n instanceof ParameterNode
@@ -70,17 +55,14 @@ private module Input implements InputSig<Location, CsharpDataFlow> {
           init.getInitializer().getNumberOfChildren() > 1
         )
       or
-      exists(ControlFlow::Nodes::ElementNode cfn, ControlFlow::Nodes::Split split |
-        exists(arg.asExprAtNode(cfn))
-      |
-        split = cfn.getASplit() and
-        not split = call.getControlFlowNode().getASplit()
-        or
-        split = call.getControlFlowNode().getASplit() and
-        not split = cfn.getASplit()
-      )
-      or
       call.(NonDelegateDataFlowCall).getDispatchCall().isReflection()
+      or
+      // Exclude calls that are both getter and setter calls, as they share the same argument nodes.
+      exists(AccessorCall ac |
+        call.(NonDelegateDataFlowCall).getDispatchCall().getCall() = ac and
+        ac instanceof AssignableRead and
+        ac instanceof AssignableWrite
+      )
     )
   }
 }

csharp/ql/consistency-queries/VariableCaptureConsistency.ql

@@ -1,13 +1,6 @@
 import csharp
 import semmle.code.csharp.dataflow.internal.DataFlowPrivate::VariableCapture::Flow::ConsistencyChecks
 private import semmle.code.csharp.dataflow.internal.DataFlowPrivate::VariableCapture::Flow::ConsistencyChecks as ConsistencyChecks
-private import semmle.code.csharp.controlflow.BasicBlocks
-private import semmle.code.csharp.controlflow.internal.ControlFlowGraphImpl
-
-query predicate uniqueEnclosingCallable(BasicBlock bb, string msg) {
-  ConsistencyChecks::uniqueEnclosingCallable(bb, msg) and
-  getNodeCfgScope(bb.getFirstNode()) instanceof Callable
-}
 
 query predicate consistencyOverview(string msg, int n) { none() }
 

csharp/ql/lib/change-notes/2026-04-13-cfg.md

@@ -0,0 +1,20 @@
+---
+category: breaking
+---
+* The C# control flow graph (CFG) implementation has been completely
+  rewritten. The CFG now includes additional nodes to more accurately represent
+  certain constructs. This also means that any existing code that implicitly
+  relies on very specific details about the CFG may need to be updated.
+  The CFG no longer uses splitting, which means that AST nodes now have a unique
+  CFG node representation.
+  Additionally, the following breaking changes have been made:
+  - `ControlFlow::Node` has been renamed to `ControlFlowNode`.
+  - `ControlFlow::Nodes` has been renamed to `ControlFlowNodes`.
+  - `BasicBlock.getCallable` has been renamed to `BasicBlock.getEnclosingCallable`.
+  - `BasicBlocks.qll` has been deleted.
+  - `ControlFlowNode.getAstNode` has changed its meaning. The AST-to-CFG
+    mapping remains one-to-many, but now for a different reason. It used to be
+    because of splitting, but now it's because of additional "helper" CFG
+    nodes. To get the (now canonical) CFG node for a given AST node, use
+    `ControlFlowNode.asExpr()` or `ControlFlowNode.asStmt()` or
+    `ControlFlowElement.getControlFlowNode()` instead.

csharp/ql/lib/experimental/code/csharp/Cryptography/NonCryptographicHashes.qll

@@ -30,7 +30,7 @@ predicate maybeUsedInFnvFunction(Variable v, Operation xor, Operation mul, LoopS
     e2.getAChild*() = v.getAnAccess() and
     e1 = xor.getAnOperand() and
     e2 = mul.getAnOperand() and
-    xor.getAControlFlowNode().getASuccessor*() = mul.getAControlFlowNode() and
+    xor.getControlFlowNode().getASuccessor*() = mul.getControlFlowNode() and
     (xor instanceof AssignXorExpr or xor instanceof BitwiseXorExpr) and
     (mul instanceof AssignMulExpr or mul instanceof MulExpr)
   ) and
@@ -55,11 +55,11 @@ private predicate maybeUsedInElfHashFunction(Variable v, Operation xor, Operatio
     v = addAssign.getTargetVariable() and
     addAssign.getAChild*() = add and
     (xor instanceof BitwiseXorExpr or xor instanceof AssignXorExpr) and
-    addAssign.getAControlFlowNode().getASuccessor*() = xor.getAControlFlowNode() and
+    addAssign.getControlFlowNode().getASuccessor*() = xor.getControlFlowNode() and
     xorAssign.getAChild*() = xor and
     v = xorAssign.getTargetVariable() and
     (notOp instanceof UnaryBitwiseOperation or notOp instanceof AssignBitwiseOperation) and
-    xor.getAControlFlowNode().getASuccessor*() = notOp.getAControlFlowNode() and
+    xor.getControlFlowNode().getASuccessor*() = notOp.getControlFlowNode() and
     notAssign.getAChild*() = notOp and
     v = notAssign.getTargetVariable() and
     loop.getAChild*() = add.getEnclosingStmt() and

csharp/ql/lib/printCfg.ql

@@ -7,7 +7,7 @@
  * @tags ide-contextual-queries/print-cfg
  */
 
-private import semmle.code.csharp.controlflow.internal.ControlFlowGraphImpl
+import csharp
 
 external string selectedSourceFile();
 
@@ -21,15 +21,15 @@ external int selectedSourceColumn();
 
 private predicate selectedSourceColumnAlias = selectedSourceColumn/0;
 
-module ViewCfgQueryInput implements ViewCfgQueryInputSig<File> {
+module ViewCfgQueryInput implements ControlFlow::ViewCfgQueryInputSig<File> {
   predicate selectedSourceFile = selectedSourceFileAlias/0;
 
   predicate selectedSourceLine = selectedSourceLineAlias/0;
 
   predicate selectedSourceColumn = selectedSourceColumnAlias/0;
 
   predicate cfgScopeSpan(
-    CfgScope scope, File file, int startLine, int startColumn, int endLine, int endColumn
+    Callable scope, File file, int startLine, int startColumn, int endLine, int endColumn
   ) {
     file = scope.getFile() and
     scope.getLocation().getStartLine() = startLine and
@@ -40,11 +40,20 @@ module ViewCfgQueryInput implements ViewCfgQueryInputSig<File> {
     |
       loc = scope.(Callable).getBody().getLocation()
       or
-      loc = scope.(Field).getInitializer().getLocation()
+      loc = any(AssignExpr init | scope.(ObjectInitMethod).initializes(init)).getLocation()
       or
-      loc = scope.(Property).getInitializer().getLocation()
+      exists(AssignableMember a, Constructor ctor |
+        scope = ctor and
+        ctor.isStatic() and
+        a.isStatic() and
+        a.getDeclaringType() = ctor.getDeclaringType()
+      |
+        loc = a.(Field).getInitializer().getLocation()
+        or
+        loc = a.(Property).getInitializer().getLocation()
+      )
     )
   }
 }
 
-import ViewCfgQuery<File, ViewCfgQueryInput>
+import ControlFlow::ViewCfgQuery<File, ViewCfgQueryInput>

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

@@ -85,8 +85,8 @@ class AssignableRead extends AssignableAccess {
   }
 
   pragma[noinline]
-  private ControlFlow::Node getAnAdjacentReadSameVar() {
-    SsaImpl::adjacentReadPairSameVar(_, this.getAControlFlowNode(), result)
+  private ControlFlowNode getAnAdjacentReadSameVar() {
+    SsaImpl::adjacentReadPairSameVar(_, this.getControlFlowNode(), result)
   }
 
   /**
@@ -114,11 +114,7 @@ class AssignableRead extends AssignableAccess {
    * - The read of `this.Field` on line 11 is next to the read on line 10.
    */
   pragma[nomagic]
-  AssignableRead getANextRead() {
-    forex(ControlFlow::Node cfn | cfn = result.getAControlFlowNode() |
-      cfn = this.getAnAdjacentReadSameVar()
-    )
-  }
+  AssignableRead getANextRead() { result.getControlFlowNode() = this.getAnAdjacentReadSameVar() }
 }
 
 /**
@@ -410,7 +406,7 @@ private import AssignableInternal
  */
 class AssignableDefinition extends TAssignableDefinition {
   /**
-   * DEPRECATED: Use `this.getExpr().getAControlFlowNode()` instead.
+   * DEPRECATED: Use `this.getExpr().getControlFlowNode()` instead.
    *
    * Gets a control flow node that updates the targeted assignable when
    * reached.
@@ -419,9 +415,7 @@ class AssignableDefinition extends TAssignableDefinition {
    * the definitions of `x` and `y` in `M(out x, out y)` and `(x, y) = (0, 1)`
    * relate to the same call to `M` and assignment node, respectively.
    */
-  deprecated ControlFlow::Node getAControlFlowNode() {
-    result = this.getExpr().getAControlFlowNode()
-  }
+  deprecated ControlFlowNode getAControlFlowNode() { result = this.getExpr().getControlFlowNode() }
 
   /**
    * Gets the underlying expression that updates the targeted assignable when
@@ -494,7 +488,7 @@ class AssignableDefinition extends TAssignableDefinition {
    */
   pragma[nomagic]
   AssignableRead getAFirstRead() {
-    forex(ControlFlow::Node cfn | cfn = result.getAControlFlowNode() |
+    exists(ControlFlowNode cfn | cfn = result.getControlFlowNode() |
       exists(Ssa::ExplicitDefinition def | result = def.getAFirstReadAtNode(cfn) |
         this = def.getADefinition()
       )
@@ -572,11 +566,9 @@ module AssignableDefinitions {
   }
 
   /** Holds if a node in basic block `bb` assigns to `ref` parameter `p` via definition `def`. */
-  private predicate basicBlockRefParamDef(
-    ControlFlow::BasicBlock bb, Parameter p, AssignableDefinition def
-  ) {
+  private predicate basicBlockRefParamDef(BasicBlock bb, Parameter p, AssignableDefinition def) {
     def = any(RefArg arg).getAnAnalyzableRefDef(p) and
-    bb.getANode() = def.getExpr().getAControlFlowNode()
+    bb.getANode() = def.getExpr().getControlFlowNode()
   }
 
   /**
@@ -585,17 +577,15 @@ module AssignableDefinitions {
    * any assignments to `p`.
    */
   pragma[nomagic]
-  private predicate parameterReachesWithoutDef(Parameter p, ControlFlow::BasicBlock bb) {
+  private predicate parameterReachesWithoutDef(Parameter p, BasicBlock bb) {
     forall(AssignableDefinition def | basicBlockRefParamDef(bb, p, def) |
       isUncertainRefCall(def.getTargetAccess())
     ) and
     (
       any(RefArg arg).isAnalyzable(p) and
       p.getCallable().getEntryPoint() = bb.getFirstNode()
       or
-      exists(ControlFlow::BasicBlock mid | parameterReachesWithoutDef(p, mid) |
-        bb = mid.getASuccessor()
-      )
+      exists(BasicBlock mid | parameterReachesWithoutDef(p, mid) | bb = mid.getASuccessor())
     )
   }
 
@@ -607,7 +597,7 @@ module AssignableDefinitions {
   cached
   predicate isUncertainRefCall(RefArg arg) {
     arg.isPotentialAssignment() and
-    exists(ControlFlow::BasicBlock bb, Parameter p | arg.isAnalyzable(p) |
+    exists(BasicBlock bb, Parameter p | arg.isAnalyzable(p) |
       parameterReachesWithoutDef(p, bb) and
       bb.getLastNode() = p.getCallable().getExitPoint()
     )
@@ -688,7 +678,7 @@ module AssignableDefinitions {
     /** Gets the underlying parameter. */
     Parameter getParameter() { result = p }
 
-    deprecated override ControlFlow::Node getAControlFlowNode() {
+    deprecated override ControlFlowNode getAControlFlowNode() {
       result = p.getCallable().getEntryPoint()
     }
 

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

@@ -7,23 +7,6 @@ private import csharp
  * in the same stage across different files.
  */
 module Stages {
-  cached
-  module ControlFlowStage {
-    private import semmle.code.csharp.controlflow.internal.Splitting
-
-    cached
-    predicate forceCachingInSameStage() { any() }
-
-    cached
-    private predicate forceCachingInSameStageRev() {
-      exists(Split s)
-      or
-      exists(ControlFlow::Node n)
-      or
-      forceCachingInSameStageRev()
-    }
-  }
-
   cached
   module GuardsStage {
     private import semmle.code.csharp.controlflow.Guards

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

@@ -22,7 +22,7 @@ private import TypeRef
  * an anonymous function (`AnonymousFunctionExpr`), or a local function
  * (`LocalFunction`).
  */
-class Callable extends Parameterizable, ExprOrStmtParent, @callable {
+class Callable extends Parameterizable, ControlFlowElementOrCallable, @callable {
   /** Gets the return type of this callable. */
   Type getReturnType() { none() }
 
@@ -157,10 +157,10 @@ class Callable extends Parameterizable, ExprOrStmtParent, @callable {
   final predicate hasExpressionBody() { exists(this.getExpressionBody()) }
 
   /** Gets the entry point in the control graph for this callable. */
-  ControlFlow::Nodes::EntryNode getEntryPoint() { result.getCallable() = this }
+  ControlFlow::EntryNode getEntryPoint() { result.getEnclosingCallable() = this }
 
   /** Gets the exit point in the control graph for this callable. */
-  ControlFlow::Nodes::ExitNode getExitPoint() { result.getCallable() = this }
+  ControlFlow::ExitNode getExitPoint() { result.getEnclosingCallable() = this }
 
   /**
    * Gets the enclosing callable of this callable, if any.