Java: Use the shared BasicBlocks library.

java/ql/lib/qlpack.yml

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

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

@@ -4,18 +4,69 @@
 
 import java
 import Dominance
+private import codeql.controlflow.BasicBlock as BB
 
-cached
-private module BasicBlockStage {
-  cached
-  predicate ref() { any() }
+private module Input implements BB::InputSig<Location> {
+  import SuccessorType
 
-  cached
-  predicate backref() {
-    (exists(any(BasicBlock bb).getABBSuccessor()) implies any()) and
-    (exists(any(BasicBlock bb).getNode(_)) implies any()) and
-    (exists(any(BasicBlock bb).length()) implies any())
+  /** Hold if `t` represents a conditional successor type. */
+  predicate successorTypeIsCondition(SuccessorType t) { none() }
+
+  /** A delineated part of the AST with its own CFG. */
+  class CfgScope = Callable;
+
+  /** The class of control flow nodes. */
+  class Node = ControlFlowNode;
+
+  /** Gets the CFG scope in which this node occurs. */
+  CfgScope nodeGetCfgScope(Node node) { node.getEnclosingCallable() = result }
+
+  private Node getASpecificSuccessor(Node node, SuccessorType t) {
+    node.(ConditionNode).getABranchSuccessor(t.(BooleanSuccessor).getValue()) = result
+    or
+    node.getAnExceptionSuccessor() = result and t instanceof ExceptionSuccessor
   }
+
+  /** Gets an immediate successor of this node. */
+  Node nodeGetASuccessor(Node node, SuccessorType t) {
+    result = getASpecificSuccessor(node, t)
+    or
+    node.getASuccessor() = result and
+    t instanceof NormalSuccessor and
+    not result = getASpecificSuccessor(node, _)
+  }
+
+  /**
+   * Holds if `node` represents an entry node to be used when calculating
+   * dominance.
+   */
+  predicate nodeIsDominanceEntry(Node node) {
+    exists(Stmt entrystmt | entrystmt = node.asStmt() |
+      exists(Callable c | entrystmt = c.getBody())
+      or
+      // This disjunct is technically superfluous, but safeguards against extractor problems.
+      entrystmt instanceof BlockStmt and
+      not exists(entrystmt.getEnclosingCallable()) and
+      not entrystmt.getParent() instanceof Stmt
+    )
+  }
+
+  /**
+   * Holds if `node` represents an exit node to be used when calculating
+   * post dominance.
+   */
+  predicate nodeIsPostDominanceExit(Node node) { node instanceof ControlFlow::ExitNode }
+}
+
+private module BbImpl = BB::Make<Location, Input>;
+
+import BbImpl
+
+/** Holds if the dominance relation is calculated for `bb`. */
+predicate hasDominanceInformation(BasicBlock bb) {
+  exists(BasicBlock entry |
+    Input::nodeIsDominanceEntry(entry.getFirstNode()) and entry.getASuccessor*() = bb
+  )
 }
 
 /**
@@ -24,71 +75,27 @@ private module BasicBlockStage {
  * A basic block is a series of nodes with no control-flow branching, which can
  * often be treated as a unit in analyses.
  */
-class BasicBlock extends ControlFlowNode {
-  cached
-  BasicBlock() {
-    BasicBlockStage::ref() and
-    not exists(this.getAPredecessor()) and
-    exists(this.getASuccessor())
-    or
-    strictcount(this.getAPredecessor()) > 1
-    or
-    exists(ControlFlowNode pred | pred = this.getAPredecessor() |
-      strictcount(pred.getASuccessor()) > 1
-    )
-  }
+class BasicBlock extends BbImpl::BasicBlock {
+  /** Gets the immediately enclosing callable whose body contains this node. */
+  Callable getEnclosingCallable() { result = this.getScope() }
 
   /** Gets an immediate successor of this basic block. */
-  cached
-  BasicBlock getABBSuccessor() {
-    BasicBlockStage::ref() and
-    result = this.getLastNode().getASuccessor()
-  }
+  BasicBlock getABBSuccessor() { result = this.getASuccessor() }
 
   /** Gets an immediate predecessor of this basic block. */
   BasicBlock getABBPredecessor() { result.getABBSuccessor() = this }
 
-  /** Gets a control-flow node contained in this basic block. */
-  ControlFlowNode getANode() { result = this.getNode(_) }
-
-  /** Gets the control-flow node at a specific (zero-indexed) position in this basic block. */
-  cached
-  ControlFlowNode getNode(int pos) {
-    BasicBlockStage::ref() and
-    result = this and
-    pos = 0
-    or
-    exists(ControlFlowNode mid, int mid_pos | pos = mid_pos + 1 |
-      this.getNode(mid_pos) = mid and
-      mid.getASuccessor() = result and
-      not result instanceof BasicBlock
-    )
-  }
-
-  /** Gets the first control-flow node in this basic block. */
-  ControlFlowNode getFirstNode() { result = this }
-
-  /** Gets the last control-flow node in this basic block. */
-  ControlFlowNode getLastNode() { result = this.getNode(this.length() - 1) }
-
-  /** Gets the number of control-flow nodes contained in this basic block. */
-  cached
-  int length() {
-    BasicBlockStage::ref() and
-    result = strictcount(this.getANode())
-  }
-
   /** Holds if this basic block strictly dominates `node`. */
-  predicate bbStrictlyDominates(BasicBlock node) { bbStrictlyDominates(this, node) }
+  predicate bbStrictlyDominates(BasicBlock node) { this.strictlyDominates(node) }
 
   /** Holds if this basic block dominates `node`. (This is reflexive.) */
-  predicate bbDominates(BasicBlock node) { bbDominates(this, node) }
+  predicate bbDominates(BasicBlock node) { this.dominates(node) }
 
   /** Holds if this basic block strictly post-dominates `node`. */
-  predicate bbStrictlyPostDominates(BasicBlock node) { bbStrictlyPostDominates(this, node) }
+  predicate bbStrictlyPostDominates(BasicBlock node) { this.strictlyPostDominates(node) }
 
   /** Holds if this basic block post-dominates `node`. (This is reflexive.) */
-  predicate bbPostDominates(BasicBlock node) { bbPostDominates(this, node) }
+  predicate bbPostDominates(BasicBlock node) { this.postDominates(node) }
 }
 
 /** A basic block that ends in an exit node. */

java/ql/lib/semmle/code/java/controlflow/Dominance.qll

@@ -8,30 +8,8 @@ import java
  * Predicates for basic-block-level dominance.
  */
 
-/** Entry points for control-flow. */
-private predicate flowEntry(BasicBlock entry) {
-  exists(Stmt entrystmt | entrystmt = entry.getFirstNode().asStmt() |
-    exists(Callable c | entrystmt = c.getBody())
-    or
-    // This disjunct is technically superfluous, but safeguards against extractor problems.
-    entrystmt instanceof BlockStmt and
-    not exists(entry.getEnclosingCallable()) and
-    not entrystmt.getParent() instanceof Stmt
-  )
-}
-
-/** The successor relation for basic blocks. */
-private predicate bbSucc(BasicBlock pre, BasicBlock post) { post = pre.getABBSuccessor() }
-
 /** The immediate dominance relation for basic blocks. */
-cached
-predicate bbIDominates(BasicBlock dom, BasicBlock node) =
-  idominance(flowEntry/1, bbSucc/2)(_, dom, node)
-
-/** Holds if the dominance relation is calculated for `bb`. */
-predicate hasDominanceInformation(BasicBlock bb) {
-  exists(BasicBlock entry | flowEntry(entry) and bbSucc*(entry, bb))
-}
+predicate bbIDominates(BasicBlock dom, BasicBlock node) { dom.immediatelyDominates(node) }
 
 /** Exit points for basic-block control-flow. */
 private predicate bbSink(BasicBlock exit) { exit.getLastNode() instanceof ControlFlow::ExitNode }
@@ -78,21 +56,6 @@ predicate dominanceFrontier(BasicBlock x, BasicBlock w) {
   )
 }
 
-/**
- * Holds if `(bb1, bb2)` is an edge that dominates `bb2`, that is, all other
- * predecessors of `bb2` are dominated by `bb2`. This implies that `bb1` is the
- * immediate dominator of `bb2`.
- *
- * This is a necessary and sufficient condition for an edge to dominate anything,
- * and in particular `dominatingEdge(bb1, bb2) and bb2.bbDominates(bb3)` means
- * that the edge `(bb1, bb2)` dominates `bb3`.
- */
-predicate dominatingEdge(BasicBlock bb1, BasicBlock bb2) {
-  bbIDominates(bb1, bb2) and
-  bb1.getABBSuccessor() = bb2 and
-  forall(BasicBlock pred | pred = bb2.getABBPredecessor() and pred != bb1 | bbDominates(bb2, pred))
-}
-
 /*
  * Predicates for expression-level dominance.
  */

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

@@ -23,7 +23,7 @@ class ConditionBlock extends BasicBlock {
 
   /** Gets a `true`- or `false`-successor of the last node of this basic block. */
   BasicBlock getTestSuccessor(boolean testIsTrue) {
-    result = this.getConditionNode().getABranchSuccessor(testIsTrue)
+    result.getFirstNode() = this.getConditionNode().getABranchSuccessor(testIsTrue)
   }
 
   /*
@@ -300,7 +300,7 @@ private predicate preconditionBranchEdge(
 ) {
   conditionCheckArgument(ma, _, branch) and
   bb1.getLastNode() = ma.getControlFlowNode() and
-  bb2 = bb1.getLastNode().getANormalSuccessor()
+  bb2.getFirstNode() = bb1.getLastNode().getANormalSuccessor()
 }
 
 private predicate preconditionControls(MethodCall ma, BasicBlock controlled, boolean branch) {

java/ql/lib/semmle/code/java/controlflow/SuccessorType.qll

@@ -0,0 +1,25 @@
+import java
+private import codeql.util.Boolean
+
+private newtype TSuccessorType =
+  TNormalSuccessor() or
+  TBooleanSuccessor(Boolean branch) or
+  TExceptionSuccessor()
+
+class SuccessorType extends TSuccessorType {
+  string toString() { result = "SuccessorType" }
+}
+
+class NormalSuccessor extends SuccessorType, TNormalSuccessor { }
+
+class ExceptionSuccessor extends SuccessorType, TExceptionSuccessor { }
+
+class ConditionalSuccessor extends SuccessorType, TBooleanSuccessor {
+  boolean getValue() { this = TBooleanSuccessor(result) }
+}
+
+class BooleanSuccessor = ConditionalSuccessor;
+
+class NullnessSuccessor extends ConditionalSuccessor {
+  NullnessSuccessor() { none() }
+}

java/ql/lib/semmle/code/java/controlflow/internal/GuardsLogic.qll

@@ -290,10 +290,10 @@ private predicate guardImpliesEqual(Guard guard, boolean branch, SsaVariable v,
   )
 }
 
-private ControlFlowNode getAGuardBranchSuccessor(Guard g, boolean branch) {
-  result = g.(Expr).getControlFlowNode().(ConditionNode).getABranchSuccessor(branch)
+private BasicBlock getAGuardBranchSuccessor(Guard g, boolean branch) {
+  result.getFirstNode() = g.(Expr).getControlFlowNode().(ConditionNode).getABranchSuccessor(branch)
   or
-  result = g.(SwitchCase).getControlFlowNode() and branch = true
+  result.getFirstNode() = g.(SwitchCase).getControlFlowNode() and branch = true
 }
 
 /**

java/ql/lib/semmle/code/java/dataflow/Nullness.qll

@@ -299,8 +299,8 @@ private predicate leavingFinally(BasicBlock bb1, BasicBlock bb2, boolean normale
   exists(TryStmt try, BlockStmt finally |
     try.getFinally() = finally and
     bb1.getABBSuccessor() = bb2 and
-    bb1.getEnclosingStmt().getEnclosingStmt*() = finally and
-    not bb2.getEnclosingStmt().getEnclosingStmt*() = finally and
+    bb1.getFirstNode().getEnclosingStmt().getEnclosingStmt*() = finally and
+    not bb2.getFirstNode().getEnclosingStmt().getEnclosingStmt*() = finally and
     if bb1.getLastNode().getANormalSuccessor() = bb2.getFirstNode()
     then normaledge = true
     else normaledge = false

java/ql/lib/semmle/code/java/dataflow/RangeAnalysis.qll

@@ -215,7 +215,7 @@ module Sem implements Semantic<Location> {
 
   private predicate idOfAst(ExprParent x, int y) = equivalenceRelation(id/2)(x, y)
 
-  private predicate idOf(BasicBlock x, int y) { idOfAst(x.getAstNode(), y) }
+  private predicate idOf(BasicBlock x, int y) { idOfAst(x.getFirstNode().getAstNode(), y) }
 
   int getBlockId1(BasicBlock bb) { idOf(bb, result) }
 

java/ql/lib/semmle/code/java/dataflow/internal/BaseSSA.qll

@@ -145,7 +145,7 @@ private module BaseSsaImpl {
   /** Holds if `v` has an implicit definition at the entry, `b`, of the callable. */
   predicate hasEntryDef(BaseSsaSourceVariable v, BasicBlock b) {
     exists(LocalScopeVariable l, Callable c |
-      v = TLocalVar(c, l) and c.getBody().getControlFlowNode() = b
+      v = TLocalVar(c, l) and c.getBody().getBasicBlock() = b
     |
       l instanceof Parameter or
       l.getCallable() != c

java/ql/lib/semmle/code/java/dataflow/internal/SsaImpl.qll

@@ -144,13 +144,13 @@ private predicate certainVariableUpdate(TrackedVar v, ControlFlowNode n, BasicBl
 pragma[nomagic]
 private predicate hasEntryDef(TrackedVar v, BasicBlock b) {
   exists(LocalScopeVariable l, Callable c |
-    v = TLocalVar(c, l) and c.getBody().getControlFlowNode() = b
+    v = TLocalVar(c, l) and c.getBody().getBasicBlock() = b
   |
     l instanceof Parameter or
     l.getCallable() != c
   )
   or
-  v instanceof SsaSourceField and v.getEnclosingCallable().getBody().getControlFlowNode() = b
+  v instanceof SsaSourceField and v.getEnclosingCallable().getBody().getBasicBlock() = b
 }
 
 /** Holds if `n` might update the locally tracked variable `v`. */

java/ql/lib/semmle/code/java/dataflow/internal/rangeanalysis/SsaReadPositionSpecific.qll

@@ -19,7 +19,7 @@ private predicate id(BB::ExprParent x, BB::ExprParent y) { x = y }
 
 private predicate idOfAst(BB::ExprParent x, int y) = equivalenceRelation(id/2)(x, y)
 
-private predicate idOf(BasicBlock x, int y) { idOfAst(x.getAstNode(), y) }
+private predicate idOf(BasicBlock x, int y) { idOfAst(x.getFirstNode().getAstNode(), y) }
 
 private int getId(BasicBlock bb) { idOf(bb, result) }
 

java/ql/lib/semmle/code/java/security/Validation.qll

@@ -40,7 +40,7 @@ private predicate validatedAccess(VarAccess va) {
       guardcall.getControlFlowNode() = node
     |
       exists(BasicBlock succ |
-        succ = node.getANormalSuccessor() and
+        succ.getFirstNode() = node.getANormalSuccessor() and
         dominatingEdge(node.getBasicBlock(), succ) and
         succ.bbDominates(va.getBasicBlock())
       )

java/ql/src/Likely Bugs/Concurrency/UnreleasedLock.ql

@@ -99,8 +99,8 @@ predicate failedLock(LockType t, BasicBlock lockblock, BasicBlock exblock) {
       )
     ) and
     (
-      lock.getAnExceptionSuccessor() = exblock or
-      lock.(ConditionNode).getAFalseSuccessor() = exblock
+      lock.getAnExceptionSuccessor() = exblock.getFirstNode() or
+      lock.(ConditionNode).getAFalseSuccessor() = exblock.getFirstNode()
     )
   )
 }
@@ -113,7 +113,7 @@ predicate heldByCurrentThreadCheck(LockType t, BasicBlock checkblock, BasicBlock
   exists(ConditionBlock conditionBlock |
     conditionBlock.getCondition() = t.getIsHeldByCurrentThreadAccess()
   |
-    conditionBlock.getBasicBlock() = checkblock and
+    conditionBlock = checkblock and
     conditionBlock.getTestSuccessor(false) = falsesucc
   )
 }
@@ -133,7 +133,7 @@ predicate variableLockStateCheck(LockType t, BasicBlock checkblock, BasicBlock f
     conditionBlock.getTestSuccessor(true) = t.getUnlockAccess().getBasicBlock() and
     conditionBlock.getCondition() = v
   |
-    conditionBlock.getBasicBlock() = checkblock and
+    conditionBlock = checkblock and
     conditionBlock.getTestSuccessor(false) = falsesucc
   )
 }