Merge pull request #7160 from hvitved/csharp/cfg/static-scope

C#: Extend `(Annotated)ExitNode` to also cover static fields

Author: hvitved

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

@@ -268,13 +268,13 @@ module ControlFlow {
 
     /** A node for a callable exit point, annotated with the type of exit. */
     class AnnotatedExitNode extends Node, TAnnotatedExitNode {
-      private Callable c;
+      private CfgScope scope;
       private boolean normal;
 
-      AnnotatedExitNode() { this = TAnnotatedExitNode(c, normal) }
+      AnnotatedExitNode() { this = TAnnotatedExitNode(scope, normal) }
 
       /** Gets the callable that this exit applies to. */
-      Callable getCallable() { result = c }
+      CfgScope getCallable() { result = scope }
 
       /** Holds if this node represents a normal exit. */
       predicate isNormal() { normal = true }
@@ -285,31 +285,35 @@ module ControlFlow {
 
       override Callable getEnclosingCallable() { result = this.getCallable() }
 
-      override Location getLocation() { result = this.getCallable().getLocation() }
+      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 " + this.getCallable() + " (" + s + ")"
+          result = "exit " + scope + " (" + s + ")"
         )
       }
     }
 
     /** A node for a callable exit point. */
     class ExitNode extends Node, TExitNode {
+      private CfgScope scope;
+
+      ExitNode() { this = TExitNode(scope) }
+
       /** Gets the callable that this exit applies to. */
-      Callable getCallable() { this = TExitNode(result) }
+      Callable getCallable() { result = scope }
 
       override BasicBlocks::ExitBlock getBasicBlock() { result = Node.super.getBasicBlock() }
 
       override Callable getEnclosingCallable() { result = this.getCallable() }
 
-      override Location getLocation() { result = this.getCallable().getLocation() }
+      override Location getLocation() { result = scope.getLocation() }
 
-      override string toString() { result = "exit " + this.getCallable().toString() }
+      override string toString() { result = "exit " + scope }
     }
 
     /**

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

@@ -97,6 +97,24 @@ module ControlFlowTree {
   predicate idOf(Range_ x, int y) = equivalenceRelation(id/2)(x, y)
 }
 
+/**
+ * The `expr_parent_top_level_adjusted()` relation restricted to exclude relations
+ * between properties and their getters' expression bodies in properties such as
+ * `int P => 0`.
+ *
+ * This is in order to only associate the expression body with one CFG scope, namely
+ * the getter (and not the declaration itself).
+ */
+private predicate expr_parent_top_level_adjusted2(
+  Expr child, int i, @top_level_exprorstmt_parent parent
+) {
+  expr_parent_top_level_adjusted(child, i, parent) and
+  not exists(Getter g |
+    g.getDeclaration() = parent and
+    i = 0
+  )
+}
+
 /** Holds if `first` is first executed when entering `scope`. */
 predicate scopeFirst(CfgScope scope, ControlFlowElement first) {
   scope =
@@ -109,17 +127,23 @@ predicate scopeFirst(CfgScope scope, ControlFlowElement first) {
         else first(c.getBody(), first)
     )
   or
-  expr_parent_top_level_adjusted(any(Expr e | first(e, first)), _, scope) and
+  expr_parent_top_level_adjusted2(any(Expr e | first(e, first)), _, scope) and
   not scope instanceof Callable
 }
 
 /** Holds if `scope` is exited when `last` finishes with completion `c`. */
-predicate scopeLast(Callable scope, ControlFlowElement last, Completion c) {
-  last(scope.getBody(), last, c) and
-  not c instanceof GotoCompletion
+predicate scopeLast(CfgScope scope, ControlFlowElement last, Completion c) {
+  scope =
+    any(Callable callable |
+      last(callable.getBody(), last, c) and
+      not c instanceof GotoCompletion
+      or
+      last(InitializerSplitting::lastConstructorInitializer(scope, _), last, c) and
+      not callable.hasBody()
+    )
   or
-  last(InitializerSplitting::lastConstructorInitializer(scope, _), last, c) and
-  not scope.hasBody()
+  expr_parent_top_level_adjusted2(any(Expr e | last(e, last, c)), _, scope) and
+  not scope instanceof Callable
 }
 
 private class ConstructorTree extends ControlFlowTree, Constructor {

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

@@ -677,7 +677,7 @@
 | Initializers.cs:8:5:8:16 | enter Initializers | Initializers.cs:8:5:8:16 | exit Initializers | 16 |
 | Initializers.cs:10:5:10:16 | enter Initializers | Initializers.cs:10:5:10:16 | exit Initializers | 16 |
 | Initializers.cs:12:10:12:10 | enter M | Initializers.cs:12:10:12:10 | exit M | 22 |
-| Initializers.cs:18:16:18:16 | enter H | Initializers.cs:18:16:18:20 | ... = ... | 3 |
+| Initializers.cs:18:16:18:16 | enter H | Initializers.cs:18:16:18:16 | exit H | 5 |
 | Initializers.cs:20:11:20:23 | enter NoConstructor | Initializers.cs:20:11:20:23 | exit NoConstructor | 9 |
 | Initializers.cs:31:9:31:11 | enter Sub | Initializers.cs:31:9:31:11 | exit Sub | 12 |
 | Initializers.cs:33:9:33:11 | enter Sub | Initializers.cs:33:9:33:11 | exit Sub | 9 |

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

@@ -2505,6 +2505,8 @@ dominance
 | Initializers.cs:15:42:15:61 | object creation of type Initializers | Initializers.cs:15:37:15:63 | { ..., ... } |
 | Initializers.cs:15:59:15:60 | "" | Initializers.cs:15:42:15:61 | object creation of type Initializers |
 | Initializers.cs:18:16:18:16 | enter H | Initializers.cs:18:20:18:20 | 1 |
+| Initializers.cs:18:16:18:16 | exit H (normal) | Initializers.cs:18:16:18:16 | exit H |
+| Initializers.cs:18:16:18:20 | ... = ... | Initializers.cs:18:16:18:16 | exit H (normal) |
 | Initializers.cs:18:20:18:20 | 1 | Initializers.cs:18:16:18:20 | ... = ... |
 | Initializers.cs:20:11:20:23 | enter NoConstructor | Initializers.cs:22:23:22:23 | this access |
 | Initializers.cs:20:11:20:23 | exit NoConstructor (normal) | Initializers.cs:20:11:20:23 | exit NoConstructor |
@@ -6631,6 +6633,8 @@ postDominance
 | Initializers.cs:15:39:15:39 | access to local variable i | Initializers.cs:15:18:15:63 | array creation of type Initializers[] |
 | Initializers.cs:15:42:15:61 | object creation of type Initializers | Initializers.cs:15:59:15:60 | "" |
 | Initializers.cs:15:59:15:60 | "" | Initializers.cs:15:39:15:39 | access to local variable i |
+| Initializers.cs:18:16:18:16 | exit H | Initializers.cs:18:16:18:16 | exit H (normal) |
+| Initializers.cs:18:16:18:16 | exit H (normal) | Initializers.cs:18:16:18:20 | ... = ... |
 | Initializers.cs:18:16:18:20 | ... = ... | Initializers.cs:18:20:18:20 | 1 |
 | Initializers.cs:18:20:18:20 | 1 | Initializers.cs:18:16:18:16 | enter H |
 | Initializers.cs:20:11:20:23 | exit NoConstructor | Initializers.cs:20:11:20:23 | exit NoConstructor (normal) |

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

@@ -8252,6 +8252,12 @@ Initializers.cs:
 #-----|  -> 1
 
 #   18| ... = ...
+#-----|  -> exit H (normal)
+
+#   18| exit H
+
+#   18| exit H (normal)
+#-----|  -> exit H
 
 #   18| 1
 #-----|  -> ... = ...