C#: Speedup ControlFlowElement::controlsBlock()

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

@@ -407,9 +407,47 @@ class ExitBasicBlock extends BasicBlock {
 /** Holds if `bb` is an exit basic block. */
 private predicate exitBB(BasicBlock bb) { bb.getLastNode() instanceof ControlFlow::Nodes::ExitNode }
 
+private module JoinBlockPredecessors {
+  private import ControlFlow::Nodes
+
+  private class CallableOrCFE extends Element {
+    CallableOrCFE() { this instanceof Callable or this instanceof ControlFlowElement }
+  }
+
+  private predicate id(CallableOrCFE x, CallableOrCFE y) { x = y }
+
+  private predicate idOf(CallableOrCFE x, int y) = equivalenceRelation(id/2)(x, y)
+
+  int getId(JoinBlockPredecessor jbp) {
+    idOf(jbp.getFirstNode().(ElementNode).getElement(), result)
+    or
+    idOf(jbp.(EntryBasicBlock).getCallable(), result)
+  }
+
+  string getSplitString(JoinBlockPredecessor jbp) {
+    result = jbp.getFirstNode().(ElementNode).getSplitsString()
+    or
+    not exists(jbp.getFirstNode().(ElementNode).getSplitsString()) and
+    result = ""
+  }
+}
+
 /** A basic block with more than one predecessor. */
 class JoinBlock extends BasicBlock {
   JoinBlock() { getFirstNode().isJoin() }
+
+  /**
+   * Gets the `i`th predecessor of this join block, with respect to some
+   * arbitrary order.
+   */
+  cached
+  JoinBlockPredecessor getJoinBlockPredecessor(int i) {
+    result = rank[i + 1](JoinBlockPredecessor jbp |
+        jbp = this.getAPredecessor()
+      |
+        jbp order by JoinBlockPredecessors::getId(jbp), JoinBlockPredecessors::getSplitString(jbp)
+      )
+  }
 }
 
 /** A basic block that is an immediate predecessor of a join block. */

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

@@ -101,32 +101,27 @@ class ControlFlowElement extends ExprOrStmtParent, @control_flow_element {
       |
         succ.dominates(pred)
         or
-        // `pred` might be another split of `cfe`
+        // `pred` might be another split of this element
         pred.getLastNode().getElement() = this and
-        pred.getASuccessorByType(t) = succ and
         t = s
       )
     )
   }
 
-  pragma[nomagic]
-  private JoinBlockPredecessor getAPossiblyControlledPredecessor(
-    JoinBlock controlled, ConditionalSuccessor s
-  ) {
-    exists(BasicBlock mid | this.immediatelyControlsBlockSplit(mid, s) |
-      result = mid.getASuccessor*()
-    ) and
-    result.getASuccessor() = controlled and
-    not controlled.dominates(result)
+  pragma[noinline]
+  private predicate controlsJoinBlockPredecessor(JoinBlock controlled, ConditionalSuccessor s, int i) {
+    this.controlsBlockSplit(controlled.getJoinBlockPredecessor(i), s)
   }
 
-  pragma[nomagic]
-  private predicate isPossiblyControlledJoinBlock(JoinBlock controlled, ConditionalSuccessor s) {
-    exists(this.getAPossiblyControlledPredecessor(controlled, s)) and
-    forall(BasicBlock pred | pred = controlled.getAPredecessor() |
-      pred = this.getAPossiblyControlledPredecessor(controlled, s)
+  private predicate controlsJoinBlockSplit(JoinBlock controlled, ConditionalSuccessor s, int i) {
+    i = -1 and
+    this.controlsJoinBlockPredecessor(controlled, s, _)
+    or
+    this.controlsJoinBlockSplit(controlled, s, i - 1) and
+    (
+      this.controlsJoinBlockPredecessor(controlled, s, i)
       or
-      controlled.dominates(pred)
+      controlled.dominates(controlled.getJoinBlockPredecessor(i))
     )
   }
 
@@ -134,13 +129,28 @@ class ControlFlowElement extends ExprOrStmtParent, @control_flow_element {
   private predicate controlsBlockSplit(BasicBlock controlled, ConditionalSuccessor s) {
     this.immediatelyControlsBlockSplit(controlled, s)
     or
-    if controlled instanceof JoinBlock
-    then
-      this.isPossiblyControlledJoinBlock(controlled, s) and
-      forall(BasicBlock pred | pred = this.getAPossiblyControlledPredecessor(controlled, s) |
-        this.controlsBlock(pred, s)
-      )
-    else this.controlsBlockSplit(controlled.getAPredecessor(), s)
+    // Equivalent with
+    //
+    // ```
+    // exists(JoinBlockPredecessor pred | pred = controlled.getAPredecessor() |
+    //   this.controlsBlockSplit(pred, s)
+    // ) and
+    // forall(JoinBlockPredecessor pred | pred = controlled.getAPredecessor() |
+    //   this.controlsBlockSplit(pred, s)
+    //   or
+    //   controlled.dominates(pred)
+    // )
+    // ```
+    //
+    // but uses no universal recursion for better performance.
+    exists(int last |
+      last = max(int i | exists(controlled.(JoinBlock).getJoinBlockPredecessor(i)))
+    |
+      this.controlsJoinBlockSplit(controlled, s, last)
+    )
+    or
+    not controlled instanceof JoinBlock and
+    this.controlsBlockSplit(controlled.getAPredecessor(), s)
   }
 
   /**