Merge pull request #21224 from owen-mc/go/use-shared-basic-block-lib

Go: Use shared basic block lib

Author: owen-mc

go/ql/lib/change-notes/2026-01-28-shared-basic-block-library.md

@@ -0,0 +1,4 @@
+---
+category: breaking
+---
+* The `BasicBlock` class is now defined using the shared basic blocks library. `BasicBlock.getRoot` has been replaced by `BasicBlock.getScope`. `BasicBlock.getAPredecessor` and `BasicBlock.getASuccessor` now take a `SuccessorType` argument. `ReachableJoinBlock.inDominanceFrontierOf` has been removed, so use `BasicBlock.inDominanceFrontier` instead, swapping the receiver and the argument.

go/ql/lib/qlpack.yml

@@ -7,6 +7,7 @@ library: true
 upgrades: upgrades
 dependencies:
   codeql/concepts: ${workspace}
+  codeql/controlflow: ${workspace}
   codeql/dataflow: ${workspace}
   codeql/mad: ${workspace}
   codeql/threat-models: ${workspace}

go/ql/lib/semmle/go/controlflow/BasicBlocks.qll

@@ -4,201 +4,65 @@
 
 import go
 private import ControlFlowGraphImpl
+private import codeql.controlflow.BasicBlock as BB
+private import codeql.controlflow.SuccessorType
+
+private module Input implements BB::InputSig<Location> {
+  /** A delineated part of the AST with its own CFG. */
+  class CfgScope = ControlFlow::Root;
+
+  /** The class of control flow nodes. */
+  class Node = ControlFlowNode;
+
+  /** Gets the CFG scope in which this node occurs. */
+  CfgScope nodeGetCfgScope(Node node) { node.getRoot() = result }
+
+  /** Gets an immediate successor of this node. */
+  Node nodeGetASuccessor(Node node, SuccessorType t) {
+    result = node.getASuccessor() and
+    (
+      not result instanceof ControlFlow::ConditionGuardNode and t instanceof DirectSuccessor
+      or
+      t.(BooleanSuccessor).getValue() = result.(ControlFlow::ConditionGuardNode).getOutcome()
+    )
+  }
 
-/**
- * Holds if `nd` starts a new basic block.
- */
-private predicate startsBB(ControlFlow::Node nd) {
-  count(nd.getAPredecessor()) != 1
-  or
-  nd.getAPredecessor().isBranch()
-}
-
-/**
- * Holds if the first node of basic block `succ` is a control flow
- * successor of the last node of basic block `bb`.
- */
-private predicate succBB(BasicBlock bb, BasicBlock succ) { succ = bb.getLastNode().getASuccessor() }
-
-/**
- * Holds if the first node of basic block `bb` is a control flow
- * successor of the last node of basic block `pre`.
- */
-private predicate predBB(BasicBlock bb, BasicBlock pre) { succBB(pre, bb) }
-
-/** Holds if `bb` is an entry basic block. */
-private predicate entryBB(BasicBlock bb) { bb.getFirstNode().isEntryNode() }
-
-/** Holds if `bb` is an exit basic block. */
-private predicate exitBB(BasicBlock bb) { bb.getLastNode().isExitNode() }
-
-cached
-private module Internal {
   /**
-   * Holds if `succ` is a control flow successor of `nd` within the same basic block.
+   * Holds if `node` represents an entry node to be used when calculating
+   * dominance.
    */
-  private predicate intraBBSucc(ControlFlow::Node nd, ControlFlow::Node succ) {
-    succ = nd.getASuccessor() and
-    not startsBB(succ)
-  }
+  predicate nodeIsDominanceEntry(Node node) { node instanceof EntryNode }
 
   /**
-   * Holds if `nd` is the `i`th node in basic block `bb`.
-   *
-   * In other words, `i` is the shortest distance from a node `bb`
-   * that starts a basic block to `nd` along the `intraBBSucc` relation.
+   * Holds if `node` represents an exit node to be used when calculating
+   * post dominance.
    */
-  cached
-  predicate bbIndex(BasicBlock bb, ControlFlow::Node nd, int i) =
-    shortestDistances(startsBB/1, intraBBSucc/2)(bb, nd, i)
-
-  cached
-  int bbLength(BasicBlock bb) { result = strictcount(ControlFlow::Node nd | bbIndex(bb, nd, _)) }
-
-  cached
-  predicate reachableBB(BasicBlock bb) {
-    entryBB(bb)
-    or
-    exists(BasicBlock predBB | succBB(predBB, bb) | reachableBB(predBB))
-  }
+  predicate nodeIsPostDominanceExit(Node node) { node instanceof ExitNode }
 }
 
-private import Internal
-
-/** Holds if `dom` is an immediate dominator of `bb`. */
-cached
-private predicate bbIDominates(BasicBlock dom, BasicBlock bb) =
-  idominance(entryBB/1, succBB/2)(_, dom, bb)
-
-/** Holds if `dom` is an immediate post-dominator of `bb`. */
-cached
-private predicate bbIPostDominates(BasicBlock dom, BasicBlock bb) =
-  idominance(exitBB/1, predBB/2)(_, dom, bb)
-
-/**
- * A basic block, that is, a maximal straight-line sequence of control flow nodes
- * without branches or joins.
- *
- * At the database level, a basic block is represented by its first control flow node.
- */
-class BasicBlock extends TControlFlowNode {
-  BasicBlock() { startsBB(this) }
-
-  /** Gets a basic block succeeding this one. */
-  BasicBlock getASuccessor() { succBB(this, result) }
-
-  /** Gets a basic block preceding this one. */
-  BasicBlock getAPredecessor() { result.getASuccessor() = this }
-
-  /** Gets a node in this block. */
-  ControlFlow::Node getANode() { result = this.getNode(_) }
-
-  /** Gets the node at the given position in this block. */
-  ControlFlow::Node getNode(int pos) { bbIndex(this, result, pos) }
-
-  /** Gets the first node in this block. */
-  ControlFlow::Node getFirstNode() { result = this }
+private module BbImpl = BB::Make<Location, Input>;
 
-  /** Gets the last node in this block. */
-  ControlFlow::Node getLastNode() { result = this.getNode(this.length() - 1) }
+class BasicBlock = BbImpl::BasicBlock;
 
-  /** Gets the length of this block. */
-  int length() { result = bbLength(this) }
+class EntryBasicBlock = BbImpl::EntryBasicBlock;
 
-  /** Gets the basic block that immediately dominates this basic block. */
-  ReachableBasicBlock getImmediateDominator() { bbIDominates(result, this) }
-
-  /** Gets the innermost function or file to which this basic block belongs. */
-  ControlFlow::Root getRoot() { result = this.getFirstNode().getRoot() }
-
-  /** Gets a textual representation of this basic block. */
-  string toString() { result = "basic block" }
-
-  /** Gets the source location for this element. */
-  Location getLocation() { result = this.getFirstNode().getLocation() }
-
-  /**
-   * DEPRECATED: Use `getLocation()` instead.
-   *
-   * Holds if this basic block is at the specified location.
-   * The location spans column `startcolumn` of line `startline` to
-   * column `endcolumn` of line `endline` in file `filepath`.
-   * For more information, see
-   * [Locations](https://codeql.github.com/docs/writing-codeql-queries/providing-locations-in-codeql-queries/).
-   */
-  deprecated predicate hasLocationInfo(
-    string filepath, int startline, int startcolumn, int endline, int endcolumn
-  ) {
-    this.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
-  }
-}
-
-/**
- * An entry basic block, that is, a basic block whose first node is an entry node.
- */
-class EntryBasicBlock extends BasicBlock {
-  EntryBasicBlock() { entryBB(this) }
+cached
+private predicate reachableBB(BasicBlock bb) {
+  bb instanceof EntryBasicBlock
+  or
+  exists(BasicBlock predBB | predBB.getASuccessor(_) = bb | reachableBB(predBB))
 }
 
 /**
  * A basic block that is reachable from an entry basic block.
  */
 class ReachableBasicBlock extends BasicBlock {
   ReachableBasicBlock() { reachableBB(this) }
-
-  /**
-   * Holds if this basic block strictly dominates `bb`.
-   */
-  cached
-  predicate strictlyDominates(ReachableBasicBlock bb) { bbIDominates+(this, bb) }
-
-  /**
-   * Holds if this basic block dominates `bb`.
-   *
-   * This predicate is reflexive: each reachable basic block dominates itself.
-   */
-  predicate dominates(ReachableBasicBlock bb) {
-    bb = this or
-    this.strictlyDominates(bb)
-  }
-
-  /**
-   * Holds if this basic block strictly post-dominates `bb`.
-   */
-  cached
-  predicate strictlyPostDominates(ReachableBasicBlock bb) { bbIPostDominates+(this, bb) }
-
-  /**
-   * Holds if this basic block post-dominates `bb`.
-   *
-   * This predicate is reflexive: each reachable basic block post-dominates itself.
-   */
-  predicate postDominates(ReachableBasicBlock bb) {
-    bb = this or
-    this.strictlyPostDominates(bb)
-  }
 }
 
 /**
  * A reachable basic block with more than one predecessor.
  */
 class ReachableJoinBlock extends ReachableBasicBlock {
   ReachableJoinBlock() { this.getFirstNode().isJoin() }
-
-  /**
-   * Holds if this basic block belongs to the dominance frontier of `b`, that is
-   * `b` dominates a predecessor of this block, but not this block itself.
-   *
-   * Algorithm from Cooper et al., "A Simple, Fast Dominance Algorithm" (Figure 5),
-   * who in turn attribute it to Ferrante et al., "The program dependence graph and
-   * its use in optimization".
-   */
-  predicate inDominanceFrontierOf(ReachableBasicBlock b) {
-    b = this.getAPredecessor() and not b = this.getImmediateDominator()
-    or
-    exists(ReachableBasicBlock prev | this.inDominanceFrontierOf(prev) |
-      b = prev.getImmediateDominator() and
-      not b = this.getImmediateDominator()
-    )
-  }
 }

go/ql/lib/semmle/go/controlflow/ControlFlowGraph.qll

@@ -313,6 +313,9 @@ module ControlFlow {
      */
     Expr getCondition() { result = cond }
 
+    /** Gets the value of the condition that this node corresponds to. */
+    boolean getOutcome() { result = outcome }
+
     override Root getRoot() { result.isRootOf(cond) }
 
     override string toString() { result = cond + " is " + outcome }
@@ -350,4 +353,6 @@ module ControlFlow {
   }
 }
 
+class ControlFlowNode = ControlFlow::Node;
+
 class Write = ControlFlow::WriteNode;

go/ql/lib/semmle/go/dataflow/SSA.qll

@@ -144,7 +144,7 @@ class SsaDefinition extends TSsaDefinition {
   abstract string prettyPrintRef();
 
   /** Gets the innermost function or file to which this SSA definition belongs. */
-  ControlFlow::Root getRoot() { result = this.getBasicBlock().getRoot() }
+  ControlFlow::Root getRoot() { result = this.getBasicBlock().getScope() }
 
   /** Gets a textual representation of this element. */
   string toString() { result = this.prettyPrintDef() }
@@ -285,7 +285,7 @@ abstract class SsaPseudoDefinition extends SsaImplicitDefinition {
  */
 class SsaPhiNode extends SsaPseudoDefinition, TPhi {
   override SsaVariable getAnInput() {
-    result = getDefReachingEndOf(this.getBasicBlock().getAPredecessor(), this.getSourceVariable())
+    result = getDefReachingEndOf(this.getBasicBlock().getAPredecessor(_), this.getSourceVariable())
   }
 
   override predicate definesAt(ReachableBasicBlock bb, int i, SsaSourceVariable v) {

go/ql/lib/semmle/go/dataflow/SsaImpl.qll

@@ -71,7 +71,7 @@ private module Internal {
   private predicate inDefDominanceFrontier(ReachableJoinBlock bb, SsaSourceVariable v) {
     exists(ReachableBasicBlock defbb, SsaDefinition def |
       def.definesAt(defbb, _, v) and
-      bb.inDominanceFrontierOf(defbb)
+      defbb.inDominanceFrontier(bb)
     )
   }
 
@@ -86,15 +86,15 @@ private module Internal {
 
   /** Holds if the `i`th node of `bb` in function `f` is an entry node. */
   private predicate entryNode(FuncDef f, ReachableBasicBlock bb, int i) {
-    f = bb.getRoot() and
+    f = bb.getScope() and
     bb.getNode(i).isEntryNode()
   }
 
   /**
    * Holds if the `i`th node of `bb` in function `f` is a function call.
    */
   private predicate callNode(FuncDef f, ReachableBasicBlock bb, int i) {
-    f = bb.getRoot() and
+    f = bb.getScope() and
     bb.getNode(i).(IR::EvalInstruction).getExpr() instanceof CallExpr
   }
 
@@ -186,7 +186,7 @@ private module Internal {
    * Holds if `v` is live at the beginning of any successor of basic block `bb`.
    */
   private predicate liveAtSuccEntry(ReachableBasicBlock bb, SsaSourceVariable v) {
-    liveAtEntry(bb.getASuccessor(), v)
+    liveAtEntry(bb.getASuccessor(_), v)
   }
 
   /**
@@ -317,7 +317,7 @@ private module Internal {
       SsaSourceVariable v, ReachableBasicBlock b1, ReachableBasicBlock b2
     ) {
       varOccursInBlock(v, b1) and
-      b2 = b1.getASuccessor()
+      b2 = b1.getASuccessor(_)
     }
 
     /**
@@ -335,7 +335,7 @@ private module Internal {
     ) {
       varBlockReaches(v, b1, mid) and
       not varOccursInBlock(v, mid) and
-      b2 = mid.getASuccessor()
+      b2 = mid.getASuccessor(_)
     }
 
     /**

go/ql/src/Security/CWE-020/IncompleteHostnameRegexp.ql

@@ -45,7 +45,7 @@ predicate writesHttpError(ReachableBasicBlock b) {
 predicate onlyErrors(BasicBlock block) {
   writesHttpError(block)
   or
-  forex(ReachableBasicBlock pred | pred = block.getAPredecessor() | onlyErrors(pred))
+  forex(ReachableBasicBlock pred | pred = block.getAPredecessor(_) | onlyErrors(pred))
 }
 
 /** Gets a node that refers to a handler that is considered to return an HTTP error. */