Skip to content

Java: Use the shared BasicBlocks library. #19505

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
May 21, 2025
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -10,7 +10,7 @@ private import SemanticExprSpecific::SemanticExprConfig as Specific
*/
class SemBasicBlock extends Specific::BasicBlock {
/** Holds if this block (transitively) dominates `otherblock`. */
final predicate bbDominates(SemBasicBlock otherBlock) { Specific::bbDominates(this, otherBlock) }
final predicate dominates(SemBasicBlock otherBlock) { Specific::bbDominates(this, otherBlock) }

/** Gets an expression that is evaluated in this basic block. */
final SemExpr getAnExpr() { result.getBasicBlock() = this }
4 changes: 4 additions & 0 deletions java/ql/lib/change-notes/2025-05-16-shared-basicblocks.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: deprecated
---
* Java now uses the shared `BasicBlock` library. This means that the names of several member predicates have been changed to align with the names used in other languages. The old predicates have been deprecated. The `BasicBlock` class itself no longer extends `ControlFlowNode` - the predicate `getFirstNode` can be used to fix any QL code that somehow relied on this.
1 change: 1 addition & 0 deletions java/ql/lib/qlpack.yml
Original file line number Diff line number Diff line change
@@ -6,6 +6,7 @@ extractor: java
library: true
upgrades: upgrades
dependencies:
codeql/controlflow: ${workspace}
codeql/dataflow: ${workspace}
codeql/mad: ${workspace}
codeql/quantum: ${workspace}
177 changes: 107 additions & 70 deletions java/ql/lib/semmle/code/java/controlflow/BasicBlocks.qll
Original file line number Diff line number Diff line change
@@ -4,91 +4,128 @@

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())
}
}

/**
* A control-flow node that represents the start of a basic block.
*
* 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
)
}
/** Hold if `t` represents a conditional successor type. */
predicate successorTypeIsCondition(SuccessorType t) { none() }

/** Gets an immediate successor of this basic block. */
cached
BasicBlock getABBSuccessor() {
BasicBlockStage::ref() and
result = this.getLastNode().getASuccessor()
}
/** A delineated part of the AST with its own CFG. */
class CfgScope = Callable;

/** Gets an immediate predecessor of this basic block. */
BasicBlock getABBPredecessor() { result.getABBSuccessor() = this }
/** The class of control flow nodes. */
class Node = ControlFlowNode;

/** Gets a control-flow node contained in this basic block. */
ControlFlowNode getANode() { result = this.getNode(_) }
/** Gets the CFG scope in which this node occurs. */
CfgScope nodeGetCfgScope(Node node) { node.getEnclosingCallable() = result }

/** 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
private Node getASpecificSuccessor(Node node, SuccessorType t) {
node.(ConditionNode).getABranchSuccessor(t.(BooleanSuccessor).getValue()) = result
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
)
node.getAnExceptionSuccessor() = result and t instanceof ExceptionSuccessor
}

/** 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 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, _)
}

/** Gets the number of control-flow nodes contained in this basic block. */
cached
int length() {
BasicBlockStage::ref() and
result = strictcount(this.getANode())
/**
* 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 this basic block strictly dominates `node`. */
predicate bbStrictlyDominates(BasicBlock node) { bbStrictlyDominates(this, node) }
/**
* 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>;

/** Holds if this basic block dominates `node`. (This is reflexive.) */
predicate bbDominates(BasicBlock node) { bbDominates(this, node) }
import BbImpl

/** Holds if this basic block strictly post-dominates `node`. */
predicate bbStrictlyPostDominates(BasicBlock node) { bbStrictlyPostDominates(this, node) }
/** Holds if the dominance relation is calculated for `bb`. */
predicate hasDominanceInformation(BasicBlock bb) {
exists(BasicBlock entry |
Input::nodeIsDominanceEntry(entry.getFirstNode()) and entry.getASuccessor*() = bb
)
}

/** Holds if this basic block post-dominates `node`. (This is reflexive.) */
predicate bbPostDominates(BasicBlock node) { bbPostDominates(this, node) }
/**
* A basic block, that is, a maximal straight-line sequence of control flow nodes
* without branches or joins.
*/
class BasicBlock extends BbImpl::BasicBlock {
/** Gets the immediately enclosing callable whose body contains this node. */
Callable getEnclosingCallable() { result = this.getScope() }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed offline it might be nice to agree on a name for this predicate that every language can use. Right now Java's basic block will have both getScope and getEnclosingCallable that does the exact same thing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but I think that's best left as a follow-up.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, absolutely :)


/**
* Holds if this basic block dominates basic block `bb`.
*
* That is, all paths reaching `bb` from the entry point basic block must
* go through this basic block.
*/
predicate dominates(BasicBlock bb) { super.dominates(bb) }

/**
* DEPRECATED: Use `getASuccessor` instead.
*
* Gets an immediate successor of this basic block.
*/
deprecated BasicBlock getABBSuccessor() { result = this.getASuccessor() }

/**
* DEPRECATED: Use `getAPredecessor` instead.
*
* Gets an immediate predecessor of this basic block.
*/
deprecated BasicBlock getABBPredecessor() { result.getASuccessor() = this }

/**
* DEPRECATED: Use `strictlyDominates` instead.
*
* Holds if this basic block strictly dominates `node`.
*/
deprecated predicate bbStrictlyDominates(BasicBlock node) { this.strictlyDominates(node) }

/**
* DEPRECATED: Use `dominates` instead.
*
* Holds if this basic block dominates `node`. (This is reflexive.)
*/
deprecated predicate bbDominates(BasicBlock node) { this.dominates(node) }

/**
* DEPRECATED: Use `strictlyPostDominates` instead.
*
* Holds if this basic block strictly post-dominates `node`.
*/
deprecated predicate bbStrictlyPostDominates(BasicBlock node) { this.strictlyPostDominates(node) }

/**
* DEPRECATED: Use `postDominates` instead.
*
* Holds if this basic block post-dominates `node`. (This is reflexive.)
*/
deprecated predicate bbPostDominates(BasicBlock node) { this.postDominates(node) }
}

/** A basic block that ends in an exit node. */
106 changes: 45 additions & 61 deletions java/ql/lib/semmle/code/java/controlflow/Dominance.qll
Original file line number Diff line number Diff line change
@@ -8,91 +8,75 @@
* 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))
/**
* DEPRECATED: Use `BasicBlock::immediatelyDominates` instead.
*
* The immediate dominance relation for basic blocks.
*/
deprecated 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 }

/** Reversed `bbSucc`. */
private predicate bbPred(BasicBlock post, BasicBlock pre) { post = pre.getABBSuccessor() }
private predicate bbPred(BasicBlock post, BasicBlock pre) { post = pre.getASuccessor() }

Check warning

Code scanning / CodeQL

Missing QLDoc for parameter Warning

The QLDoc has no documentation for post, or pre, but the QLDoc mentions bbSucc

/** The immediate post-dominance relation on basic blocks. */
cached
predicate bbIPostDominates(BasicBlock dominator, BasicBlock node) =
deprecated predicate bbIPostDominates(BasicBlock dominator, BasicBlock node) =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this have a DEPRECATED: Use ... doc like the other deprecated predicates?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Immediate post-dominance is not exposed in the shared lib - I'm not sure we've ever seen any use-cases for it, though, so adding it is probably of little value - hence why I simply opted to discretely deprecate this single public instance.

idominance(bbSink/1, bbPred/2)(_, dominator, node)

/** Holds if `dom` strictly dominates `node`. */
predicate bbStrictlyDominates(BasicBlock dom, BasicBlock node) { bbIDominates+(dom, node) }

/** Holds if `dom` dominates `node`. (This is reflexive.) */
predicate bbDominates(BasicBlock dom, BasicBlock node) {
bbStrictlyDominates(dom, node) or dom = node
/**
* DEPRECATED: Use `BasicBlock::strictlyDominates` instead.
*
* Holds if `dom` strictly dominates `node`.
*/
deprecated predicate bbStrictlyDominates(BasicBlock dom, BasicBlock node) {
dom.strictlyDominates(node)
}

/** Holds if `dom` strictly post-dominates `node`. */
predicate bbStrictlyPostDominates(BasicBlock dom, BasicBlock node) { bbIPostDominates+(dom, node) }
/**
* DEPRECATED: Use `BasicBlock::dominates` instead.
*
* Holds if `dom` dominates `node`. (This is reflexive.)
*/
deprecated predicate bbDominates(BasicBlock dom, BasicBlock node) { dom.dominates(node) }

/** Holds if `dom` post-dominates `node`. (This is reflexive.) */
predicate bbPostDominates(BasicBlock dom, BasicBlock node) {
bbStrictlyPostDominates(dom, node) or dom = node
/**
* DEPRECATED: Use `BasicBlock::strictlyPostDominates` instead.
*
* Holds if `dom` strictly post-dominates `node`.
*/
deprecated predicate bbStrictlyPostDominates(BasicBlock dom, BasicBlock node) {
dom.strictlyPostDominates(node)
}

/**
* DEPRECATED: Use `BasicBlock::postDominates` instead.
*
* Holds if `dom` post-dominates `node`. (This is reflexive.)
*/
deprecated predicate bbPostDominates(BasicBlock dom, BasicBlock node) { dom.postDominates(node) }

/**
* The dominance frontier relation for basic blocks.
*
* This is equivalent to:
*
* ```
* bbDominates(x, w.getABBPredecessor()) and not bbStrictlyDominates(x, w)
* x.dominates(w.getAPredecessor()) and not x.strictlyDominates(w)
* ```
*/
predicate dominanceFrontier(BasicBlock x, BasicBlock w) {
x = w.getABBPredecessor() and not bbIDominates(x, w)
x = w.getAPredecessor() and not x.immediatelyDominates(w)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about deprecating this in favor of inDominanceFrontier from the shared library?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think dominance frontier is used for anything else than SSA construction, and the shared SSA lib does its own calculation, so perhaps we should deprecate both. In any case, I think that's fine to postpone for now.

or
exists(BasicBlock prev | dominanceFrontier(prev, w) |
bbIDominates(x, prev) and
not bbIDominates(x, w)
x.immediatelyDominates(prev) and
not x.immediatelyDominates(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.
*/
@@ -102,7 +86,7 @@
exists(BasicBlock bb, int i | dominator = bb.getNode(i) and node = bb.getNode(i + 1))
or
exists(BasicBlock dom, BasicBlock bb |
bbIDominates(dom, bb) and
dom.immediatelyDominates(bb) and
dominator = dom.getLastNode() and
node = bb.getFirstNode()
)
@@ -112,7 +96,7 @@
pragma[inline]
predicate strictlyDominates(ControlFlowNode dom, ControlFlowNode node) {
// This predicate is gigantic, so it must be inlined.
bbStrictlyDominates(dom.getBasicBlock(), node.getBasicBlock())
dom.getBasicBlock().strictlyDominates(node.getBasicBlock())
or
exists(BasicBlock b, int i, int j | dom = b.getNode(i) and node = b.getNode(j) and i < j)
}
@@ -121,7 +105,7 @@
pragma[inline]
predicate dominates(ControlFlowNode dom, ControlFlowNode node) {
// This predicate is gigantic, so it must be inlined.
bbStrictlyDominates(dom.getBasicBlock(), node.getBasicBlock())
dom.getBasicBlock().strictlyDominates(node.getBasicBlock())
or
exists(BasicBlock b, int i, int j | dom = b.getNode(i) and node = b.getNode(j) and i <= j)
}
@@ -130,7 +114,7 @@
pragma[inline]
predicate strictlyPostDominates(ControlFlowNode dom, ControlFlowNode node) {
// This predicate is gigantic, so it must be inlined.
bbStrictlyPostDominates(dom.getBasicBlock(), node.getBasicBlock())
dom.getBasicBlock().strictlyPostDominates(node.getBasicBlock())
or
exists(BasicBlock b, int i, int j | dom = b.getNode(i) and node = b.getNode(j) and i > j)
}
@@ -139,7 +123,7 @@
pragma[inline]
predicate postDominates(ControlFlowNode dom, ControlFlowNode node) {
// This predicate is gigantic, so it must be inlined.
bbStrictlyPostDominates(dom.getBasicBlock(), node.getBasicBlock())
dom.getBasicBlock().strictlyPostDominates(node.getBasicBlock())
or
exists(BasicBlock b, int i, int j | dom = b.getNode(i) and node = b.getNode(j) and i >= j)
}
Loading
Oops, something went wrong.
Loading
Oops, something went wrong.