diff --git a/go/ql/lib/change-notes/2026-01-28-shared-basic-block-library.md b/go/ql/lib/change-notes/2026-01-28-shared-basic-block-library.md new file mode 100644 index 000000000000..f26aeb9c07ab --- /dev/null +++ b/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. diff --git a/go/ql/lib/qlpack.yml b/go/ql/lib/qlpack.yml index 669112f115df..32769f6bd297 100644 --- a/go/ql/lib/qlpack.yml +++ b/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} diff --git a/go/ql/lib/semmle/go/controlflow/BasicBlocks.qll b/go/ql/lib/semmle/go/controlflow/BasicBlocks.qll index 8380c6d6d5d2..43b8c7e8dd39 100644 --- a/go/ql/lib/semmle/go/controlflow/BasicBlocks.qll +++ b/go/ql/lib/semmle/go/controlflow/BasicBlocks.qll @@ -4,140 +4,53 @@ import go private import ControlFlowGraphImpl +private import codeql.controlflow.BasicBlock as BB +private import codeql.controlflow.SuccessorType + +private module Input implements BB::InputSig { + /** 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; - /** 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)) } /** @@ -145,38 +58,6 @@ class EntryBasicBlock extends BasicBlock { */ 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) - } } /** @@ -184,21 +65,4 @@ class ReachableBasicBlock extends BasicBlock { */ 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() - ) - } } diff --git a/go/ql/lib/semmle/go/controlflow/ControlFlowGraph.qll b/go/ql/lib/semmle/go/controlflow/ControlFlowGraph.qll index 1e66bc61dc45..88adb88c0264 100644 --- a/go/ql/lib/semmle/go/controlflow/ControlFlowGraph.qll +++ b/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; diff --git a/go/ql/lib/semmle/go/dataflow/SSA.qll b/go/ql/lib/semmle/go/dataflow/SSA.qll index 98dae5f3d014..69fffa393c19 100644 --- a/go/ql/lib/semmle/go/dataflow/SSA.qll +++ b/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) { diff --git a/go/ql/lib/semmle/go/dataflow/SsaImpl.qll b/go/ql/lib/semmle/go/dataflow/SsaImpl.qll index 8549d9b497ad..026c8114f9fb 100644 --- a/go/ql/lib/semmle/go/dataflow/SsaImpl.qll +++ b/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,7 +86,7 @@ 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() } @@ -94,7 +94,7 @@ private module Internal { * 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(_) } /** diff --git a/go/ql/src/Security/CWE-020/IncompleteHostnameRegexp.ql b/go/ql/src/Security/CWE-020/IncompleteHostnameRegexp.ql index f6e3df7d1d91..a6321b7d7cb3 100644 --- a/go/ql/src/Security/CWE-020/IncompleteHostnameRegexp.ql +++ b/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. */