diff --git a/cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/SemanticCFG.qll b/cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/SemanticCFG.qll index 5123434a35c8..8f164aa1b697 100644 --- a/cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/SemanticCFG.qll +++ b/cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/SemanticCFG.qll @@ -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 } diff --git a/java/ql/lib/change-notes/2025-05-16-shared-basicblocks.md b/java/ql/lib/change-notes/2025-05-16-shared-basicblocks.md new file mode 100644 index 000000000000..e71ae5c13175 --- /dev/null +++ b/java/ql/lib/change-notes/2025-05-16-shared-basicblocks.md @@ -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. diff --git a/java/ql/lib/qlpack.yml b/java/ql/lib/qlpack.yml index d34620678ba1..8e1e06ab6b5d 100644 --- a/java/ql/lib/qlpack.yml +++ b/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} diff --git a/java/ql/lib/semmle/code/java/controlflow/BasicBlocks.qll b/java/ql/lib/semmle/code/java/controlflow/BasicBlocks.qll index c2f9e8a6a697..284ee1dad0ce 100644 --- a/java/ql/lib/semmle/code/java/controlflow/BasicBlocks.qll +++ b/java/ql/lib/semmle/code/java/controlflow/BasicBlocks.qll @@ -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 { + 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; - /** 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() } + + /** + * 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. */ diff --git a/java/ql/lib/semmle/code/java/controlflow/Dominance.qll b/java/ql/lib/semmle/code/java/controlflow/Dominance.qll index 953bfa12e640..6f0cb3d255c5 100644 --- a/java/ql/lib/semmle/code/java/controlflow/Dominance.qll +++ b/java/ql/lib/semmle/code/java/controlflow/Dominance.qll @@ -8,91 +8,75 @@ 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)) +/** + * 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() } /** The immediate post-dominance relation on basic blocks. */ -cached -predicate bbIPostDominates(BasicBlock dominator, BasicBlock node) = +deprecated predicate bbIPostDominates(BasicBlock dominator, BasicBlock node) = 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) 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 @@ predicate iDominates(ControlFlowNode dominator, ControlFlowNode node) { 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 @@ predicate iDominates(ControlFlowNode dominator, ControlFlowNode node) { 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 @@ predicate strictlyDominates(ControlFlowNode dom, ControlFlowNode node) { 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 @@ predicate dominates(ControlFlowNode dom, ControlFlowNode node) { 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 @@ predicate strictlyPostDominates(ControlFlowNode dom, ControlFlowNode node) { 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) } diff --git a/java/ql/lib/semmle/code/java/controlflow/Guards.qll b/java/ql/lib/semmle/code/java/controlflow/Guards.qll index ff564b3a4469..99a832c08a8f 100644 --- a/java/ql/lib/semmle/code/java/controlflow/Guards.qll +++ b/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) } /* @@ -68,7 +68,7 @@ class ConditionBlock extends BasicBlock { exists(BasicBlock succ | succ = this.getTestSuccessor(testIsTrue) and dominatingEdge(this, succ) and - succ.bbDominates(controlled) + succ.dominates(controlled) ) } } @@ -287,7 +287,7 @@ private predicate switchCaseControls(SwitchCase sc, BasicBlock bb) { // Pattern cases are handled as condition blocks not sc instanceof PatternCase and caseblock.getFirstNode() = sc.getControlFlowNode() and - caseblock.bbDominates(bb) and + caseblock.dominates(bb) and // Check we can't fall through from a previous block: forall(ControlFlowNode pred | pred = sc.getControlFlowNode().getAPredecessor() | isNonFallThroughPredecessor(sc, pred) @@ -300,14 +300,14 @@ 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) { exists(BasicBlock check, BasicBlock succ | preconditionBranchEdge(ma, check, succ, branch) and dominatingEdge(check, succ) and - succ.bbDominates(controlled) + succ.dominates(controlled) ) } diff --git a/java/ql/lib/semmle/code/java/controlflow/Paths.qll b/java/ql/lib/semmle/code/java/controlflow/Paths.qll index 5a06a3a1ee5d..8f87e19404a6 100644 --- a/java/ql/lib/semmle/code/java/controlflow/Paths.qll +++ b/java/ql/lib/semmle/code/java/controlflow/Paths.qll @@ -47,7 +47,7 @@ private predicate callAlwaysPerformsAction(Call call, ActionConfiguration conf) private predicate actionDominatesExit(Callable callable, ActionConfiguration conf) { exists(ExitBlock exit | exit.getEnclosingCallable() = callable and - actionBlock(conf).bbDominates(exit) + actionBlock(conf).dominates(exit) ) } @@ -56,12 +56,12 @@ private BasicBlock nonDominatingActionBlock(ActionConfiguration conf) { exists(ExitBlock exit | result = actionBlock(conf) and exit.getEnclosingCallable() = result.getEnclosingCallable() and - not result.bbDominates(exit) + not result.dominates(exit) ) } private class JoinBlock extends BasicBlock { - JoinBlock() { 2 <= strictcount(this.getABBPredecessor()) } + JoinBlock() { 2 <= strictcount(this.getAPredecessor()) } } /** @@ -72,8 +72,8 @@ private predicate postActionBlock(BasicBlock bb, ActionConfiguration conf) { bb = nonDominatingActionBlock(conf) or if bb instanceof JoinBlock - then forall(BasicBlock pred | pred = bb.getABBPredecessor() | postActionBlock(pred, conf)) - else postActionBlock(bb.getABBPredecessor(), conf) + then forall(BasicBlock pred | pred = bb.getAPredecessor() | postActionBlock(pred, conf)) + else postActionBlock(bb.getAPredecessor(), conf) } /** Holds if every path through `callable` goes through at least one action node. */ diff --git a/java/ql/lib/semmle/code/java/controlflow/SuccessorType.qll b/java/ql/lib/semmle/code/java/controlflow/SuccessorType.qll new file mode 100644 index 000000000000..f03e4690a95a --- /dev/null +++ b/java/ql/lib/semmle/code/java/controlflow/SuccessorType.qll @@ -0,0 +1,72 @@ +/** + * Provides different types of control flow successor types. + */ + +import java +private import codeql.util.Boolean + +private newtype TSuccessorType = + TNormalSuccessor() or + TBooleanSuccessor(Boolean branch) or + TExceptionSuccessor() + +/** The type of a control flow successor. */ +class SuccessorType extends TSuccessorType { + /** Gets a textual representation of successor type. */ + string toString() { result = "SuccessorType" } +} + +/** A normal control flow successor. */ +class NormalSuccessor extends SuccessorType, TNormalSuccessor { } + +/** + * An exceptional control flow successor. + * + * This marks control flow edges that are taken when an exception is thrown. + */ +class ExceptionSuccessor extends SuccessorType, TExceptionSuccessor { } + +/** + * A conditional control flow successor. + * + * This currently only includes boolean successors (`BooleanSuccessor`). + */ +class ConditionalSuccessor extends SuccessorType, TBooleanSuccessor { + /** Gets the Boolean value of this successor. */ + boolean getValue() { this = TBooleanSuccessor(result) } +} + +/** + * A Boolean control flow successor. + * + * For example, this program fragment: + * + * ```java + * if (x < 0) + * return 0; + * else + * return 1; + * ``` + * + * has a control flow graph containing Boolean successors: + * + * ``` + * if + * | + * x < 0 + * / \ + * / \ + * / \ + * true false + * | \ + * return 0 return 1 + * ``` + */ +class BooleanSuccessor = ConditionalSuccessor; + +/** + * A nullness control flow successor. This is currently unused for Java. + */ +class NullnessSuccessor extends ConditionalSuccessor { + NullnessSuccessor() { none() } +} diff --git a/java/ql/lib/semmle/code/java/controlflow/UnreachableBlocks.qll b/java/ql/lib/semmle/code/java/controlflow/UnreachableBlocks.qll index 7bcc732de6a0..0ade780bc00c 100644 --- a/java/ql/lib/semmle/code/java/controlflow/UnreachableBlocks.qll +++ b/java/ql/lib/semmle/code/java/controlflow/UnreachableBlocks.qll @@ -209,7 +209,7 @@ class UnreachableBasicBlock extends BasicBlock { or // This block is not reachable in the CFG, and is not the entrypoint in a callable, an // expression in an assert statement, or a catch clause. - forall(BasicBlock bb | bb = this.getABBPredecessor() | bb instanceof UnreachableBasicBlock) and + forall(BasicBlock bb | bb = this.getAPredecessor() | bb instanceof UnreachableBasicBlock) and not exists(Callable c | c.getBody().getControlFlowNode() = this.getFirstNode()) and not this.getFirstNode().asExpr().getEnclosingStmt() instanceof AssertStmt and not this.getFirstNode().asStmt() instanceof CatchClause @@ -219,11 +219,10 @@ class UnreachableBasicBlock extends BasicBlock { // Not accessible from the switch expression unreachableCaseBlock = constSwitchStmt.getAFailingCase().getBasicBlock() and // Not accessible from the successful case - not constSwitchStmt.getMatchingCase().getBasicBlock().getABBSuccessor*() = - unreachableCaseBlock + not constSwitchStmt.getMatchingCase().getBasicBlock().getASuccessor*() = unreachableCaseBlock | // Blocks dominated by an unreachable case block are unreachable - unreachableCaseBlock.bbDominates(this) + unreachableCaseBlock.dominates(this) ) } } diff --git a/java/ql/lib/semmle/code/java/controlflow/internal/GuardsLogic.qll b/java/ql/lib/semmle/code/java/controlflow/internal/GuardsLogic.qll index 9fed7516ba31..4cb3bc74f97f 100644 --- a/java/ql/lib/semmle/code/java/controlflow/internal/GuardsLogic.qll +++ b/java/ql/lib/semmle/code/java/controlflow/internal/GuardsLogic.qll @@ -239,7 +239,7 @@ SsaVariable getADefinition(SsaVariable v, boolean fromBackEdge) { exists(SsaVariable inp, BasicBlock bb, boolean fbe | v.(SsaPhiNode).hasInputFromBlock(inp, bb) and result = getADefinition(inp, fbe) and - (if v.getBasicBlock().bbDominates(bb) then fromBackEdge = true else fromBackEdge = fbe) + (if v.getBasicBlock().dominates(bb) then fromBackEdge = true else fromBackEdge = fbe) ) } @@ -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 } /** @@ -306,7 +306,7 @@ private predicate guardControlsPhiBranch( guard.directlyControls(upd.getBasicBlock(), branch) and upd.getDefiningExpr().(VariableAssign).getSource() = e and upd = phi.getAPhiInput() and - guard.getBasicBlock().bbStrictlyDominates(phi.getBasicBlock()) + guard.getBasicBlock().strictlyDominates(phi.getBasicBlock()) } /** @@ -331,7 +331,7 @@ private predicate conditionalAssign(SsaVariable v, Guard guard, boolean branch, forall(SsaVariable other | other != upd and other = phi.getAPhiInput() | guard.directlyControls(other.getBasicBlock(), branch.booleanNot()) or - other.getBasicBlock().bbDominates(guard.getBasicBlock()) and + other.getBasicBlock().dominates(guard.getBasicBlock()) and not other.isLiveAtEndOfBlock(getAGuardBranchSuccessor(guard, branch)) ) ) diff --git a/java/ql/lib/semmle/code/java/dataflow/Nullness.qll b/java/ql/lib/semmle/code/java/dataflow/Nullness.qll index 17f1b4ae7021..786207d34865 100644 --- a/java/ql/lib/semmle/code/java/dataflow/Nullness.qll +++ b/java/ql/lib/semmle/code/java/dataflow/Nullness.qll @@ -298,9 +298,9 @@ private predicate impossibleEdge(BasicBlock bb1, BasicBlock bb2) { private predicate leavingFinally(BasicBlock bb1, BasicBlock bb2, boolean normaledge) { 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.getASuccessor() = bb2 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 @@ -339,7 +339,7 @@ private predicate nullVarStep( midssa.isLiveAtEndOfBlock(mid) and not ensureNotNull(midssa).getBasicBlock() = mid and not assertFail(mid, _) and - bb = mid.getABBSuccessor() and + bb = mid.getASuccessor() and not impossibleEdge(mid, bb) and not exists(boolean branch | nullGuard(midssa, branch, false).hasBranchEdge(mid, bb, branch)) and not (leavingFinally(mid, bb, true) and midstoredcompletion = true) and diff --git a/java/ql/lib/semmle/code/java/dataflow/RangeAnalysis.qll b/java/ql/lib/semmle/code/java/dataflow/RangeAnalysis.qll index b67dc2ba08de..b43990e14553 100644 --- a/java/ql/lib/semmle/code/java/dataflow/RangeAnalysis.qll +++ b/java/ql/lib/semmle/code/java/dataflow/RangeAnalysis.qll @@ -209,13 +209,13 @@ module Sem implements Semantic { class BasicBlock = J::BasicBlock; - BasicBlock getABasicBlockSuccessor(BasicBlock bb) { result = bb.getABBSuccessor() } + BasicBlock getABasicBlockSuccessor(BasicBlock bb) { result = bb.getASuccessor() } private predicate id(ExprParent x, ExprParent y) { x = y } 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) } diff --git a/java/ql/lib/semmle/code/java/dataflow/TypeFlow.qll b/java/ql/lib/semmle/code/java/dataflow/TypeFlow.qll index d29cc1ae5421..f2fcbc5951d4 100644 --- a/java/ql/lib/semmle/code/java/dataflow/TypeFlow.qll +++ b/java/ql/lib/semmle/code/java/dataflow/TypeFlow.qll @@ -321,14 +321,14 @@ private module Input implements TypeFlowInput { */ private predicate instanceofDisjunct(InstanceOfExpr ioe, BasicBlock bb, BaseSsaVariable v) { ioe.getExpr() = v.getAUse() and - strictcount(bb.getABBPredecessor()) > 1 and + strictcount(bb.getAPredecessor()) > 1 and exists(ConditionBlock cb | cb.getCondition() = ioe and cb.getTestSuccessor(true) = bb) } /** Holds if `bb` is disjunctively guarded by multiple `instanceof` tests on `v`. */ private predicate instanceofDisjunction(BasicBlock bb, BaseSsaVariable v) { strictcount(InstanceOfExpr ioe | instanceofDisjunct(ioe, bb, v)) = - strictcount(bb.getABBPredecessor()) + strictcount(bb.getAPredecessor()) } /** @@ -338,7 +338,7 @@ private module Input implements TypeFlowInput { predicate instanceofDisjunctionGuarded(TypeFlowNode n, RefType t) { exists(BasicBlock bb, InstanceOfExpr ioe, BaseSsaVariable v, VarAccess va | instanceofDisjunction(bb, v) and - bb.bbDominates(va.getBasicBlock()) and + bb.dominates(va.getBasicBlock()) and va = v.getAUse() and instanceofDisjunct(ioe, bb, v) and t = ioe.getSyntacticCheckedType() and diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/BaseSSA.qll b/java/ql/lib/semmle/code/java/dataflow/internal/BaseSSA.qll index eeac19e66a74..874aca871832 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/BaseSSA.qll +++ b/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 @@ -157,15 +157,14 @@ private import BaseSsaImpl private module SsaInput implements SsaImplCommon::InputSig { private import java as J - private import semmle.code.java.controlflow.Dominance as Dom class BasicBlock = J::BasicBlock; class ControlFlowNode = J::ControlFlowNode; - BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { Dom::bbIDominates(result, bb) } + BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { result.immediatelyDominates(bb) } - BasicBlock getABasicBlockSuccessor(BasicBlock bb) { result = bb.getABBSuccessor() } + BasicBlock getABasicBlockSuccessor(BasicBlock bb) { result = bb.getASuccessor() } class SourceVariable = BaseSsaSourceVariable; diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowPrivate.qll b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowPrivate.qll index d1df53a8a859..9e924df12780 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowPrivate.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowPrivate.qll @@ -83,12 +83,12 @@ private module CaptureInput implements VariableCapture::InputSig { class ControlFlowNode = J::ControlFlowNode; - BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { bbIDominates(result, bb) } - - BasicBlock getABasicBlockSuccessor(BasicBlock bb) { - result = bb.(J::BasicBlock).getABBSuccessor() + BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { + result.(J::BasicBlock).immediatelyDominates(bb) } + BasicBlock getABasicBlockSuccessor(BasicBlock bb) { result = bb.(J::BasicBlock).getASuccessor() } + //TODO: support capture of `this` in lambdas class CapturedVariable instanceof LocalScopeVariable { CapturedVariable() { diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowUtil.qll b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowUtil.qll index e87c92f3d6c5..6000c37c6cdd 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowUtil.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowUtil.qll @@ -40,14 +40,14 @@ private module ThisFlow { private int lastRank(BasicBlock b) { result = max(int rankix | thisRank(_, b, rankix)) } - private predicate blockPrecedesThisAccess(BasicBlock b) { thisAccess(_, b.getABBSuccessor*(), _) } + private predicate blockPrecedesThisAccess(BasicBlock b) { thisAccess(_, b.getASuccessor*(), _) } private predicate thisAccessBlockReaches(BasicBlock b1, BasicBlock b2) { - thisAccess(_, b1, _) and b2 = b1.getABBSuccessor() + thisAccess(_, b1, _) and b2 = b1.getASuccessor() or exists(BasicBlock mid | thisAccessBlockReaches(b1, mid) and - b2 = mid.getABBSuccessor() and + b2 = mid.getASuccessor() and not thisAccess(_, mid, _) and blockPrecedesThisAccess(b2) ) diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/SsaImpl.qll b/java/ql/lib/semmle/code/java/dataflow/internal/SsaImpl.qll index b5a42a975699..6054db635bc7 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/SsaImpl.qll +++ b/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`. */ @@ -165,15 +165,14 @@ private predicate uncertainVariableUpdate(TrackedVar v, ControlFlowNode n, Basic private module SsaInput implements SsaImplCommon::InputSig { private import java as J - private import semmle.code.java.controlflow.Dominance as Dom class BasicBlock = J::BasicBlock; class ControlFlowNode = J::ControlFlowNode; - BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { Dom::bbIDominates(result, bb) } + BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { result.immediatelyDominates(bb) } - BasicBlock getABasicBlockSuccessor(BasicBlock bb) { result = bb.getABBSuccessor() } + BasicBlock getABasicBlockSuccessor(BasicBlock bb) { result = bb.getASuccessor() } class SourceVariable = SsaSourceVariable; diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/rangeanalysis/SsaReadPositionSpecific.qll b/java/ql/lib/semmle/code/java/dataflow/internal/rangeanalysis/SsaReadPositionSpecific.qll index 8712ad635f5c..9b081150e893 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/rangeanalysis/SsaReadPositionSpecific.qll +++ b/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) } diff --git a/java/ql/lib/semmle/code/java/security/AndroidWebViewCertificateValidationQuery.qll b/java/ql/lib/semmle/code/java/security/AndroidWebViewCertificateValidationQuery.qll index 8e6a9c30141f..8d53766e0080 100644 --- a/java/ql/lib/semmle/code/java/security/AndroidWebViewCertificateValidationQuery.qll +++ b/java/ql/lib/semmle/code/java/security/AndroidWebViewCertificateValidationQuery.qll @@ -24,6 +24,6 @@ private class SslProceedCall extends MethodCall { /** Holds if `m` trusts all certificates by calling `SslErrorHandler.proceed` unconditionally. */ predicate trustsAllCerts(OnReceivedSslErrorMethod m) { exists(SslProceedCall pr | pr.getQualifier().(VarAccess).getVariable() = m.handlerArg() | - pr.getBasicBlock().bbPostDominates(m.getBody().getBasicBlock()) + pr.getBasicBlock().postDominates(m.getBody().getBasicBlock()) ) } diff --git a/java/ql/lib/semmle/code/java/security/PathSanitizer.qll b/java/ql/lib/semmle/code/java/security/PathSanitizer.qll index 8b08b5a78f2f..f3385c94646b 100644 --- a/java/ql/lib/semmle/code/java/security/PathSanitizer.qll +++ b/java/ql/lib/semmle/code/java/security/PathSanitizer.qll @@ -21,7 +21,7 @@ private module ValidationMethod { validationMethod(ma.getMethod(), pos) and ma.getArgument(pos) = rv and adjacentUseUseSameVar(rv, result.asExpr()) and - ma.getBasicBlock().bbDominates(result.asExpr().getBasicBlock()) + ma.getBasicBlock().dominates(result.asExpr().getBasicBlock()) ) } diff --git a/java/ql/lib/semmle/code/java/security/UrlForwardQuery.qll b/java/ql/lib/semmle/code/java/security/UrlForwardQuery.qll index bc3b40009270..7234b4c788f5 100644 --- a/java/ql/lib/semmle/code/java/security/UrlForwardQuery.qll +++ b/java/ql/lib/semmle/code/java/security/UrlForwardQuery.qll @@ -168,7 +168,7 @@ private class FullyDecodesUrlBarrier extends DataFlow::Node { exists(Variable v, Expr e | this.asExpr() = v.getAnAccess() | fullyDecodesUrlGuard(e) and e = v.getAnAccess() and - e.getBasicBlock().bbDominates(this.asExpr().getBasicBlock()) + e.getBasicBlock().dominates(this.asExpr().getBasicBlock()) ) } } diff --git a/java/ql/lib/semmle/code/java/security/Validation.qll b/java/ql/lib/semmle/code/java/security/Validation.qll index 50f0a9aab1b6..664c55e70d82 100644 --- a/java/ql/lib/semmle/code/java/security/Validation.qll +++ b/java/ql/lib/semmle/code/java/security/Validation.qll @@ -40,16 +40,16 @@ 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()) + succ.dominates(va.getBasicBlock()) ) or exists(BasicBlock bb, int i | bb.getNode(i) = node and bb.getNode(i + 1) = node.getANormalSuccessor() | - bb.bbStrictlyDominates(va.getBasicBlock()) or + bb.strictlyDominates(va.getBasicBlock()) or bb.getNode(any(int j | j > i)).asExpr() = va ) ) diff --git a/java/ql/src/Likely Bugs/Concurrency/UnreleasedLock.ql b/java/ql/src/Likely Bugs/Concurrency/UnreleasedLock.ql index 73c66c664f1a..118593e31fe8 100644 --- a/java/ql/src/Likely Bugs/Concurrency/UnreleasedLock.ql +++ b/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 ) } @@ -145,9 +145,7 @@ predicate variableLockStateCheck(LockType t, BasicBlock checkblock, BasicBlock f predicate blockIsLocked(LockType t, BasicBlock src, BasicBlock b, int locks) { lockUnlockBlock(t, b, locks) and src = b and locks > 0 or - exists(BasicBlock pred, int predlocks, int curlocks, int failedlock | - pred = b.getABBPredecessor() - | + exists(BasicBlock pred, int predlocks, int curlocks, int failedlock | pred = b.getAPredecessor() | // The number of net locks from the `src` block to the predecessor block `pred` is `predlocks`. blockIsLocked(t, src, pred, predlocks) and // The recursive call ensures that at least one lock is held, so do not consider the false diff --git a/java/ql/src/Likely Bugs/Likely Typos/NestedLoopsSameVariable.ql b/java/ql/src/Likely Bugs/Likely Typos/NestedLoopsSameVariable.ql index 83781f853778..f3f23e6893ba 100644 --- a/java/ql/src/Likely Bugs/Likely Typos/NestedLoopsSameVariable.ql +++ b/java/ql/src/Likely Bugs/Likely Typos/NestedLoopsSameVariable.ql @@ -18,6 +18,6 @@ where iteration = inner.getAnIterationVariable() and iteration = outer.getAnIterationVariable() and inner.getEnclosingStmt+() = outer and - inner.getBasicBlock().getABBSuccessor+() = outer.getCondition().getBasicBlock() + inner.getBasicBlock().getASuccessor+() = outer.getCondition().getBasicBlock() select inner.getCondition(), "Nested for statement uses loop variable $@ of enclosing $@.", iteration, iteration.getName(), outer, "for statement" diff --git a/java/ql/src/Violations of Best Practice/Dead Code/DeadLocals.qll b/java/ql/src/Violations of Best Practice/Dead Code/DeadLocals.qll index 773743370f64..26c5ed66a86d 100644 --- a/java/ql/src/Violations of Best Practice/Dead Code/DeadLocals.qll +++ b/java/ql/src/Violations of Best Practice/Dead Code/DeadLocals.qll @@ -37,7 +37,7 @@ predicate overwritten(VariableUpdate upd) { bb1.getNode(i) = upd.getControlFlowNode() and bb2.getNode(j) = overwrite.getControlFlowNode() | - bb1.getABBSuccessor+() = bb2 + bb1.getASuccessor+() = bb2 or bb1 = bb2 and i < j ) diff --git a/java/ql/src/Violations of Best Practice/Declarations/Common.qll b/java/ql/src/Violations of Best Practice/Declarations/Common.qll index 9211c4b0f290..9b3225db81a1 100644 --- a/java/ql/src/Violations of Best Practice/Declarations/Common.qll +++ b/java/ql/src/Violations of Best Practice/Declarations/Common.qll @@ -14,7 +14,7 @@ private predicate blockInSwitch(SwitchStmt s, BasicBlock b) { private predicate switchCaseControlFlow(SwitchStmt switch, BasicBlock b1, BasicBlock b2) { blockInSwitch(switch, b1) and - b1.getABBSuccessor() = b2 and + b1.getASuccessor() = b2 and blockInSwitch(switch, b2) } diff --git a/java/ql/src/meta/ssa/UseWithoutUniqueSsaVariable.ql b/java/ql/src/meta/ssa/UseWithoutUniqueSsaVariable.ql index 85bd192fe99a..76f6ee37fb1c 100644 --- a/java/ql/src/meta/ssa/UseWithoutUniqueSsaVariable.ql +++ b/java/ql/src/meta/ssa/UseWithoutUniqueSsaVariable.ql @@ -13,7 +13,7 @@ import semmle.code.java.dataflow.SSA class SsaConvertibleReadAccess extends VarRead { SsaConvertibleReadAccess() { - this.getEnclosingCallable().getBody().getBasicBlock().getABBSuccessor*() = this.getBasicBlock() and + this.getEnclosingCallable().getBody().getBasicBlock().getASuccessor*() = this.getBasicBlock() and ( not exists(this.getQualifier()) or diff --git a/java/ql/test-kotlin1/library-tests/controlflow/basic/bbStrictDominance.ql b/java/ql/test-kotlin1/library-tests/controlflow/basic/bbStrictDominance.ql index 9765b8e6cc5b..de1e23b649cc 100644 --- a/java/ql/test-kotlin1/library-tests/controlflow/basic/bbStrictDominance.ql +++ b/java/ql/test-kotlin1/library-tests/controlflow/basic/bbStrictDominance.ql @@ -1,6 +1,6 @@ -import default +import java import semmle.code.java.controlflow.Dominance from BasicBlock b, BasicBlock b2 -where bbStrictlyDominates(b, b2) +where b.strictlyDominates(b2) select b, b2 diff --git a/java/ql/test-kotlin1/library-tests/controlflow/basic/bbSuccessor.ql b/java/ql/test-kotlin1/library-tests/controlflow/basic/bbSuccessor.ql index 1d464c2a31a8..ae2d8a393b47 100644 --- a/java/ql/test-kotlin1/library-tests/controlflow/basic/bbSuccessor.ql +++ b/java/ql/test-kotlin1/library-tests/controlflow/basic/bbSuccessor.ql @@ -1,5 +1,5 @@ -import default +import java from BasicBlock b, BasicBlock b2 -where b.getABBSuccessor() = b2 +where b.getASuccessor() = b2 select b, b2 diff --git a/java/ql/test-kotlin1/library-tests/controlflow/dominance/dominanceWrong.ql b/java/ql/test-kotlin1/library-tests/controlflow/dominance/dominanceWrong.ql index 5ee23224d5f9..4eadcddc61a6 100644 --- a/java/ql/test-kotlin1/library-tests/controlflow/dominance/dominanceWrong.ql +++ b/java/ql/test-kotlin1/library-tests/controlflow/dominance/dominanceWrong.ql @@ -16,6 +16,6 @@ predicate dominanceCounterExample(ControlFlowNode entry, ControlFlowNode dom, Co from Callable c, ControlFlowNode dom, ControlFlowNode node where - (strictlyDominates(dom, node) or bbStrictlyDominates(dom, node)) and + strictlyDominates(dom, node) and dominanceCounterExample(c.getBody().getControlFlowNode(), dom, node) select c, dom, node diff --git a/java/ql/test-kotlin2/library-tests/controlflow/basic/bbStrictDominance.ql b/java/ql/test-kotlin2/library-tests/controlflow/basic/bbStrictDominance.ql index 9765b8e6cc5b..de1e23b649cc 100644 --- a/java/ql/test-kotlin2/library-tests/controlflow/basic/bbStrictDominance.ql +++ b/java/ql/test-kotlin2/library-tests/controlflow/basic/bbStrictDominance.ql @@ -1,6 +1,6 @@ -import default +import java import semmle.code.java.controlflow.Dominance from BasicBlock b, BasicBlock b2 -where bbStrictlyDominates(b, b2) +where b.strictlyDominates(b2) select b, b2 diff --git a/java/ql/test-kotlin2/library-tests/controlflow/basic/bbSuccessor.ql b/java/ql/test-kotlin2/library-tests/controlflow/basic/bbSuccessor.ql index 1d464c2a31a8..ae2d8a393b47 100644 --- a/java/ql/test-kotlin2/library-tests/controlflow/basic/bbSuccessor.ql +++ b/java/ql/test-kotlin2/library-tests/controlflow/basic/bbSuccessor.ql @@ -1,5 +1,5 @@ -import default +import java from BasicBlock b, BasicBlock b2 -where b.getABBSuccessor() = b2 +where b.getASuccessor() = b2 select b, b2 diff --git a/java/ql/test-kotlin2/library-tests/controlflow/dominance/dominanceWrong.ql b/java/ql/test-kotlin2/library-tests/controlflow/dominance/dominanceWrong.ql index 5ee23224d5f9..4eadcddc61a6 100644 --- a/java/ql/test-kotlin2/library-tests/controlflow/dominance/dominanceWrong.ql +++ b/java/ql/test-kotlin2/library-tests/controlflow/dominance/dominanceWrong.ql @@ -16,6 +16,6 @@ predicate dominanceCounterExample(ControlFlowNode entry, ControlFlowNode dom, Co from Callable c, ControlFlowNode dom, ControlFlowNode node where - (strictlyDominates(dom, node) or bbStrictlyDominates(dom, node)) and + strictlyDominates(dom, node) and dominanceCounterExample(c.getBody().getControlFlowNode(), dom, node) select c, dom, node diff --git a/java/ql/test/library-tests/controlflow/basic/bbStrictDominance.ql b/java/ql/test/library-tests/controlflow/basic/bbStrictDominance.ql index 9765b8e6cc5b..de1e23b649cc 100644 --- a/java/ql/test/library-tests/controlflow/basic/bbStrictDominance.ql +++ b/java/ql/test/library-tests/controlflow/basic/bbStrictDominance.ql @@ -1,6 +1,6 @@ -import default +import java import semmle.code.java.controlflow.Dominance from BasicBlock b, BasicBlock b2 -where bbStrictlyDominates(b, b2) +where b.strictlyDominates(b2) select b, b2 diff --git a/java/ql/test/library-tests/controlflow/basic/bbSuccessor.ql b/java/ql/test/library-tests/controlflow/basic/bbSuccessor.ql index 1d464c2a31a8..ae2d8a393b47 100644 --- a/java/ql/test/library-tests/controlflow/basic/bbSuccessor.ql +++ b/java/ql/test/library-tests/controlflow/basic/bbSuccessor.ql @@ -1,5 +1,5 @@ -import default +import java from BasicBlock b, BasicBlock b2 -where b.getABBSuccessor() = b2 +where b.getASuccessor() = b2 select b, b2 diff --git a/java/ql/test/library-tests/controlflow/dominance/dominanceWrong.ql b/java/ql/test/library-tests/controlflow/dominance/dominanceWrong.ql index 5ee23224d5f9..4eadcddc61a6 100644 --- a/java/ql/test/library-tests/controlflow/dominance/dominanceWrong.ql +++ b/java/ql/test/library-tests/controlflow/dominance/dominanceWrong.ql @@ -16,6 +16,6 @@ predicate dominanceCounterExample(ControlFlowNode entry, ControlFlowNode dom, Co from Callable c, ControlFlowNode dom, ControlFlowNode node where - (strictlyDominates(dom, node) or bbStrictlyDominates(dom, node)) and + strictlyDominates(dom, node) and dominanceCounterExample(c.getBody().getControlFlowNode(), dom, node) select c, dom, node diff --git a/shared/controlflow/codeql/controlflow/BasicBlock.qll b/shared/controlflow/codeql/controlflow/BasicBlock.qll index 74dc8ca5f512..9c26b18c0938 100644 --- a/shared/controlflow/codeql/controlflow/BasicBlock.qll +++ b/shared/controlflow/codeql/controlflow/BasicBlock.qll @@ -51,23 +51,11 @@ signature module InputSig { module Make Input> { private import Input - final class BasicBlock = BasicBlockImpl; - - private Node nodeGetAPredecessor(Node node, SuccessorType s) { - nodeGetASuccessor(result, s) = node - } - - /** Holds if this node has more than one predecessor. */ - private predicate nodeIsJoin(Node node) { strictcount(nodeGetAPredecessor(node, _)) > 1 } - - /** Holds if this node has more than one successor. */ - private predicate nodeIsBranch(Node node) { strictcount(nodeGetASuccessor(node, _)) > 1 } - /** * A basic block, that is, a maximal straight-line sequence of control flow nodes * without branches or joins. */ - private class BasicBlockImpl extends TBasicBlockStart { + final class BasicBlock extends TBasicBlockStart { /** Gets the CFG scope of this basic block. */ CfgScope getScope() { result = nodeGetCfgScope(this.getFirstNode()) } @@ -78,9 +66,7 @@ module Make Input> { BasicBlock getASuccessor() { result = this.getASuccessor(_) } /** Gets an immediate successor of this basic block of a given type, if any. */ - BasicBlock getASuccessor(SuccessorType t) { - result.getFirstNode() = nodeGetASuccessor(this.getLastNode(), t) - } + BasicBlock getASuccessor(SuccessorType t) { bbSuccessor(this, result, t) } /** Gets an immediate predecessor of this basic block, if any. */ BasicBlock getAPredecessor() { result.getASuccessor(_) = this } @@ -259,6 +245,10 @@ module Make Input> { * only be reached from the entry block by going through `(bb1, bb2)`. This * implies that `(bb1, bb2)` dominates its endpoint `bb2`. I.e., `bb2` can * only be reached from the entry block by going via `(bb1, bb2)`. + * + * This is a necessary and sufficient condition for an edge to dominate some + * block, and therefore `dominatingEdge(bb1, bb2) and bb2.dominates(bb3)` + * means that the edge `(bb1, bb2)` dominates `bb3`. */ pragma[nomagic] predicate dominatingEdge(BasicBlock bb1, BasicBlock bb2) { @@ -285,6 +275,16 @@ module Make Input> { cached private module Cached { + private Node nodeGetAPredecessor(Node node, SuccessorType s) { + nodeGetASuccessor(result, s) = node + } + + /** Holds if this node has more than one predecessor. */ + private predicate nodeIsJoin(Node node) { strictcount(nodeGetAPredecessor(node, _)) > 1 } + + /** Holds if this node has more than one successor. */ + private predicate nodeIsBranch(Node node) { strictcount(nodeGetASuccessor(node, _)) > 1 } + /** * Internal representation of basic blocks. A basic block is represented * by its first CFG node. @@ -341,11 +341,19 @@ module Make Input> { cached Node getNode(BasicBlock bb, int pos) { bbIndex(bb.getFirstNode(), result, pos) } + /** Holds if `bb` is an entry basic block. */ + private predicate entryBB(BasicBlock bb) { nodeIsDominanceEntry(bb.getFirstNode()) } + + cached + predicate bbSuccessor(BasicBlock bb1, BasicBlock bb2, SuccessorType t) { + bb2.getFirstNode() = nodeGetASuccessor(bb1.getLastNode(), t) + } + /** * Holds if the first node of basic block `succ` is a control flow * successor of the last node of basic block `pred`. */ - private predicate succBB(BasicBlock pred, BasicBlock succ) { pred.getASuccessor(_) = succ } + private predicate succBB(BasicBlock pred, BasicBlock succ) { bbSuccessor(pred, succ, _) } /** Holds if `dom` is an immediate dominator of `bb`. */ cached @@ -365,7 +373,4 @@ module Make Input> { } private import Cached - - /** Holds if `bb` is an entry basic block. */ - private predicate entryBB(BasicBlock bb) { nodeIsDominanceEntry(bb.getFirstNode()) } } diff --git a/shared/rangeanalysis/codeql/rangeanalysis/RangeAnalysis.qll b/shared/rangeanalysis/codeql/rangeanalysis/RangeAnalysis.qll index d0fc084e6c50..60888c9f93f6 100644 --- a/shared/rangeanalysis/codeql/rangeanalysis/RangeAnalysis.qll +++ b/shared/rangeanalysis/codeql/rangeanalysis/RangeAnalysis.qll @@ -146,7 +146,7 @@ signature module Semantic { class BasicBlock { /** Holds if this block (transitively) dominates `otherblock`. */ - predicate bbDominates(BasicBlock otherBlock); + predicate dominates(BasicBlock otherBlock); } /** Gets an immediate successor of basic block `bb`, if any. */ diff --git a/shared/rangeanalysis/codeql/rangeanalysis/internal/RangeUtils.qll b/shared/rangeanalysis/codeql/rangeanalysis/internal/RangeUtils.qll index ee6e3a4c958a..05aef4805081 100644 --- a/shared/rangeanalysis/codeql/rangeanalysis/internal/RangeUtils.qll +++ b/shared/rangeanalysis/codeql/rangeanalysis/internal/RangeUtils.qll @@ -205,7 +205,7 @@ module MakeUtils Lang, DeltaSig D> { predicate backEdge(SsaPhiNode phi, SsaVariable inp, SsaReadPositionPhiInputEdge edge) { edge.phiInput(phi, inp) and ( - phi.getBasicBlock().bbDominates(edge.getOrigBlock()) or + phi.getBasicBlock().dominates(edge.getOrigBlock()) or irreducibleSccEdge(edge.getOrigBlock(), phi.getBasicBlock()) ) } @@ -227,7 +227,7 @@ module MakeUtils Lang, DeltaSig D> { private predicate trimmedEdge(BasicBlock pred, BasicBlock succ) { getABasicBlockSuccessor(pred) = succ and - not succ.bbDominates(pred) + not succ.dominates(pred) } /**