Merge pull request #7288 from hvitved/cfg/enclosing-scope

Shared CFG: Include CFG scope in `TElementNode`

Author: hvitved

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

@@ -241,7 +241,7 @@ module ControlFlow {
     predicate isBranch() { strictcount(this.getASuccessor()) > 1 }
 
     /** Gets the enclosing callable of this control flow node. */
-    Callable getEnclosingCallable() { none() }
+    final Callable getEnclosingCallable() { result = getNodeCfgScope(this) }
   }
 
   /** Provides different types of control flow nodes. */
@@ -253,8 +253,6 @@ module ControlFlow {
 
       override BasicBlocks::EntryBlock getBasicBlock() { result = Node.super.getBasicBlock() }
 
-      override Callable getEnclosingCallable() { result = this.getCallable() }
-
       private Assignable getAssignable() { this = TEntryNode(result) }
 
       override Location getLocation() {
@@ -283,8 +281,6 @@ module ControlFlow {
         result = Node.super.getBasicBlock()
       }
 
-      override Callable getEnclosingCallable() { result = this.getCallable() }
-
       override Location getLocation() { result = scope.getLocation() }
 
       override string toString() {
@@ -309,8 +305,6 @@ module ControlFlow {
 
       override BasicBlocks::ExitBlock getBasicBlock() { result = Node.super.getBasicBlock() }
 
-      override Callable getEnclosingCallable() { result = this.getCallable() }
-
       override Location getLocation() { result = scope.getLocation() }
 
       override string toString() { result = "exit " + scope }
@@ -327,14 +321,7 @@ module ControlFlow {
       private Splits splits;
       private ControlFlowElement cfe;
 
-      ElementNode() { this = TElementNode(cfe, splits) }
-
-      override Callable getEnclosingCallable() {
-        result = cfe.getEnclosingCallable()
-        or
-        result =
-          this.getASplit().(Splitting::InitializerSplitting::InitializerSplit).getConstructor()
-      }
+      ElementNode() { this = TElementNode(_, cfe, splits) }
 
       override ControlFlowElement getElement() { result = cfe }
 

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

@@ -342,7 +342,7 @@ private predicate succExitSplits(
   ControlFlowElement pred, Splits predSplits, CfgScope succ, SuccessorType t
 ) {
   exists(Reachability::SameSplitsBlock b, Completion c | pred = b.getAnElement() |
-    b.isReachable(predSplits) and
+    b.isReachable(succ, predSplits) and
     t = getAMatchingSuccessorType(c) and
     scopeLast(succ, pred, c) and
     forall(SplitImpl predSplit | predSplit = predSplits.getASplit() |
@@ -399,7 +399,7 @@ private module SuccSplits {
     ControlFlowElement succ, Completion c
   ) {
     pred = b.getAnElement() and
-    b.isReachable(predSplits) and
+    b.isReachable(_, predSplits) and
     succ(pred, succ, c)
   }
 
@@ -728,12 +728,12 @@ private module Reachability {
      * Holds if the elements of this block are reachable from a callable entry
      *  point, with the splits `splits`.
      */
-    predicate isReachable(Splits splits) {
+    predicate isReachable(CfgScope scope, Splits splits) {
       // Base case
-      succEntrySplits(_, this, splits, _)
+      succEntrySplits(scope, this, splits, _)
       or
       // Recursive case
-      exists(SameSplitsBlock pred, Splits predSplits | pred.isReachable(predSplits) |
+      exists(SameSplitsBlock pred, Splits predSplits | pred.isReachable(scope, predSplits) |
         this = pred.getASuccessor(predSplits, splits)
       )
     }
@@ -791,43 +791,45 @@ private module Cached {
   newtype TCfgNode =
     TEntryNode(CfgScope scope) { succEntrySplits(scope, _, _, _) } or
     TAnnotatedExitNode(CfgScope scope, boolean normal) {
-      exists(Reachability::SameSplitsBlock b, SuccessorType t | b.isReachable(_) |
+      exists(Reachability::SameSplitsBlock b, SuccessorType t | b.isReachable(scope, _) |
         succExitSplits(b.getAnElement(), _, scope, t) and
         if isAbnormalExitType(t) then normal = false else normal = true
       )
     } or
     TExitNode(CfgScope scope) {
-      exists(Reachability::SameSplitsBlock b | b.isReachable(_) |
+      exists(Reachability::SameSplitsBlock b | b.isReachable(scope, _) |
         succExitSplits(b.getAnElement(), _, scope, _)
       )
     } or
-    TElementNode(ControlFlowElement cfe, Splits splits) {
-      exists(Reachability::SameSplitsBlock b | b.isReachable(splits) | cfe = b.getAnElement())
+    TElementNode(CfgScope scope, ControlFlowElement cfe, Splits splits) {
+      exists(Reachability::SameSplitsBlock b | b.isReachable(scope, splits) |
+        cfe = b.getAnElement()
+      )
     }
 
   /** Gets a successor node of a given flow type, if any. */
   cached
   TCfgNode getASuccessor(TCfgNode pred, SuccessorType t) {
     // Callable entry node -> callable body
     exists(ControlFlowElement succElement, Splits succSplits, CfgScope scope |
-      result = TElementNode(succElement, succSplits) and
+      result = TElementNode(scope, succElement, succSplits) and
       pred = TEntryNode(scope) and
       succEntrySplits(scope, succElement, succSplits, t)
     )
     or
-    exists(ControlFlowElement predElement, Splits predSplits |
-      pred = TElementNode(predElement, predSplits)
+    exists(CfgScope scope, ControlFlowElement predElement, Splits predSplits |
+      pred = TElementNode(pragma[only_bind_into](scope), predElement, predSplits)
     |
       // Element node -> callable exit (annotated)
-      exists(CfgScope scope, boolean normal |
-        result = TAnnotatedExitNode(scope, normal) and
+      exists(boolean normal |
+        result = TAnnotatedExitNode(pragma[only_bind_into](scope), normal) and
         succExitSplits(predElement, predSplits, scope, t) and
         if isAbnormalExitType(t) then normal = false else normal = true
       )
       or
       // Element node -> element node
       exists(ControlFlowElement succElement, Splits succSplits, Completion c |
-        result = TElementNode(succElement, succSplits)
+        result = TElementNode(pragma[only_bind_into](scope), succElement, succSplits)
       |
         succSplits(predElement, predSplits, succElement, succSplits, c) and
         t = getAMatchingSuccessorType(c)
@@ -853,6 +855,23 @@ private module Cached {
    */
   cached
   ControlFlowElement getAControlFlowExitNode(ControlFlowElement cfe) { last(cfe, result, _) }
+
+  /**
+   * Gets the CFG scope of node `n`. Unlike `getCfgScope`, this predicate
+   * is calculated based on reachability from an entry node, and it may
+   * yield different results for AST elements that are split into multiple
+   * scopes.
+   */
+  cached
+  CfgScope getNodeCfgScope(TCfgNode n) {
+    n = TEntryNode(result)
+    or
+    n = TAnnotatedExitNode(result, _)
+    or
+    n = TExitNode(result)
+    or
+    n = TElementNode(result, _, _)
+  }
 }
 
 import Cached

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

@@ -179,7 +179,7 @@ module InitializerSplitting {
    *
    * respectively.
    */
-  class InitializerSplit extends Split, TInitializerSplit {
+  private class InitializerSplit extends Split, TInitializerSplit {
     private Constructor c;
 
     InitializerSplit() { this = TInitializerSplit(c) }

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

@@ -8092,24 +8092,24 @@ Initializers.cs:
 #    5| ... = ...
 #-----|  -> this access
 
-#    5| ... = ...
-#-----|  -> this access
-
 #    5| this access
 #-----|  -> access to field H
 
+#    5| ... = ...
+#-----|  -> this access
+
 #    5| this access
 #-----|  -> access to field H
 
 #    5| ... + ...
 #-----|  -> ... = ...
 
-#    5| ... + ...
-#-----|  -> ... = ...
-
 #    5| access to field H
 #-----|  -> 1
 
+#    5| ... + ...
+#-----|  -> ... = ...
+
 #    5| access to field H
 #-----|  -> 1
 
@@ -8122,12 +8122,12 @@ Initializers.cs:
 #    6| access to property G
 #-----|  -> ... = ...
 
-#    6| access to property G
-#-----|  -> ... = ...
-
 #    6| this access
 #-----|  -> access to field H
 
+#    6| access to property G
+#-----|  -> ... = ...
+
 #    6| this access
 #-----|  -> access to field H
 
@@ -8140,12 +8140,12 @@ Initializers.cs:
 #    6| ... + ...
 #-----|  -> access to property G
 
-#    6| ... + ...
-#-----|  -> access to property G
-
 #    6| access to field H
 #-----|  -> 2
 
+#    6| ... + ...
+#-----|  -> access to property G
+
 #    6| access to field H
 #-----|  -> 2
 
@@ -8291,12 +8291,12 @@ Initializers.cs:
 #   28| ... = ...
 #-----|  -> {...}
 
-#   28| ... = ...
-#-----|  -> {...}
-
 #   28| this access
 #-----|  -> 2
 
+#   28| ... = ...
+#-----|  -> {...}
+
 #   28| this access
 #-----|  -> 2
 
@@ -10299,12 +10299,12 @@ PartialImplementationB.cs:
 #    3| ... = ...
 #-----|  -> this access
 
-#    3| ... = ...
-#-----|  -> this access
-
 #    3| this access
 #-----|  -> 0
 
+#    3| ... = ...
+#-----|  -> this access
+
 #    3| this access
 #-----|  -> 0
 
@@ -10331,12 +10331,12 @@ PartialImplementationB.cs:
 #    5| access to property P
 #-----|  -> ... = ...
 
-#    5| access to property P
-#-----|  -> ... = ...
-
 #    5| this access
 #-----|  -> 0
 
+#    5| access to property P
+#-----|  -> ... = ...
+
 #    5| this access
 #-----|  -> 0
 

ruby/ql/lib/codeql/ruby/controlflow/BasicBlocks.qll

@@ -15,7 +15,7 @@ private import SuccessorTypes
  */
 class BasicBlock extends TBasicBlockStart {
   /** Gets the scope of this basic block. */
-  CfgScope getScope() { result = this.getAPredecessor().getScope() }
+  final CfgScope getScope() { result = this.getFirstNode().getScope() }
 
   /** Gets an immediate successor of this basic block, if any. */
   BasicBlock getASuccessor() { result = this.getASuccessor(_) }
@@ -322,8 +322,6 @@ private predicate entryBB(BasicBlock bb) { bb.getFirstNode() instanceof EntryNod
  */
 class EntryBasicBlock extends BasicBlock {
   EntryBasicBlock() { entryBB(this) }
-
-  override CfgScope getScope() { this.getFirstNode() = TEntryNode(result) }
 }
 
 /**

ruby/ql/lib/codeql/ruby/controlflow/CfgNodes.qll

@@ -67,7 +67,7 @@ class AstCfgNode extends CfgNode, TElementNode {
   private Splits splits;
   private AstNode n;
 
-  AstCfgNode() { this = TElementNode(n, splits) }
+  AstCfgNode() { this = TElementNode(_, n, splits) }
 
   final override AstNode getNode() { result = n }
 

ruby/ql/lib/codeql/ruby/controlflow/ControlFlowGraph.qll

@@ -44,7 +44,7 @@ class CfgNode extends TCfgNode {
   final predicate isCondition() { exists(this.getASuccessor(any(BooleanSuccessor bs))) }
 
   /** Gets the scope of this node. */
-  final CfgScope getScope() { result = this.getBasicBlock().getScope() }
+  final CfgScope getScope() { result = getNodeCfgScope(this) }
 
   /** Gets the basic block that this control flow node belongs to. */
   BasicBlock getBasicBlock() { result.getANode() = this }