diff --git a/shared/controlflow/codeql/controlflow/BasicBlock.qll b/shared/controlflow/codeql/controlflow/BasicBlock.qll index df8348f6fbd6..f0f3ec428c54 100644 --- a/shared/controlflow/codeql/controlflow/BasicBlock.qll +++ b/shared/controlflow/codeql/controlflow/BasicBlock.qll @@ -126,18 +126,23 @@ module Make Input> { /** * Holds if `df` is in the dominance frontier of this basic block. That is, * this basic block dominates a predecessor of `df`, but does not dominate - * `df` itself. + * `df` itself. I.e., it is equivaluent to: + * ``` + * this.dominates(df.getAPredecessor()) and not this.strictlyDominates(df) + * ``` */ predicate inDominanceFrontier(BasicBlock df) { - this.dominatesPredecessor(df) and - not this.strictlyDominates(df) + // 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". + this = df.getAPredecessor() and not bbIDominates(this, df) + or + exists(BasicBlock prev | prev.inDominanceFrontier(df) | + bbIDominates(this, prev) and + not bbIDominates(this, df) + ) } - /** - * Holds if this basic block dominates a predecessor of `df`. - */ - private predicate dominatesPredecessor(BasicBlock df) { this.dominates(df.getAPredecessor()) } - /** * Gets the basic block that immediately dominates this basic block, if any. * @@ -219,7 +224,7 @@ module Make Input> { // In cases such as // // ```rb - // if x or y + // if x and y // foo // else // bar diff --git a/shared/controlflow/codeql/controlflow/Cfg.qll b/shared/controlflow/codeql/controlflow/Cfg.qll index 82e7fdd469e2..65fe28d49ef7 100644 --- a/shared/controlflow/codeql/controlflow/Cfg.qll +++ b/shared/controlflow/codeql/controlflow/Cfg.qll @@ -82,14 +82,12 @@ signature module InputSig { * Gets an `id` of `node`. This is used to order the predecessors of a join * basic block. */ - bindingset[node] int idOfAstNode(AstNode node); /** * Gets an `id` of `scope`. This is used to order the predecessors of a join * basic block. */ - bindingset[scope] int idOfCfgScope(CfgScope scope); } @@ -1008,6 +1006,41 @@ module MakeWithSplitting< ) } + private module JoinBlockPredecessors { + predicate hasIdAndKind(BasicBlocks::JoinPredecessorBasicBlock jbp, int id, int kind) { + id = idOfCfgScope(jbp.(BasicBlocks::EntryBasicBlock).getScope()) and + kind = 0 + or + not jbp instanceof BasicBlocks::EntryBasicBlock and + id = idOfAstNode(jbp.getFirstNode().(AstCfgNode).getAstNode()) and + kind = 1 + } + + string getSplitString(BasicBlocks::JoinPredecessorBasicBlock jbp) { + result = jbp.getFirstNode().(AstCfgNode).getSplitsString() + or + not exists(jbp.getFirstNode().(AstCfgNode).getSplitsString()) and + result = "" + } + } + + /** + * Gets the `i`th predecessor of join block `jb`, with respect to some + * arbitrary order. + */ + cached + BasicBlocks::JoinPredecessorBasicBlock getJoinBlockPredecessor( + BasicBlocks::JoinBasicBlock jb, int i + ) { + result = + rank[i + 1](BasicBlocks::JoinPredecessorBasicBlock jbp, int id, int kind | + jbp = jb.getAPredecessor() and + JoinBlockPredecessors::hasIdAndKind(jbp, id, kind) + | + jbp order by id, kind, JoinBlockPredecessors::getSplitString(jbp) + ) + } + cached module Public { /** @@ -1570,15 +1603,13 @@ module MakeWithSplitting< BasicBlock getASuccessor() { result = super.getASuccessor() } /** Gets an immediate successor of this basic block of a given type, if any. */ - BasicBlock getASuccessor(SuccessorType t) { - result.getFirstNode() = this.getLastNode().getASuccessor(t) - } + BasicBlock getASuccessor(SuccessorType t) { result = super.getASuccessor(t) } /** Gets an immediate predecessor of this basic block, if any. */ - BasicBlock getAPredecessor() { result.getASuccessor() = this } + BasicBlock getAPredecessor() { result = super.getAPredecessor() } /** Gets an immediate predecessor of this basic block of a given type, if any. */ - BasicBlock getAPredecessor(SuccessorType t) { result.getASuccessor(t) = this } + BasicBlock getAPredecessor(SuccessorType t) { result = super.getAPredecessor(t) } /** * Holds if this basic block immediately dominates basic block `bb`. @@ -1673,38 +1704,6 @@ module MakeWithSplitting< ExitBasicBlock() { this.getLastNode() instanceof ExitNode } } - private module JoinBlockPredecessors { - predicate hasIdAndKind(JoinPredecessorBasicBlock jbp, int id, int kind) { - id = idOfCfgScope(jbp.(EntryBasicBlock).getScope()) and - kind = 0 - or - not jbp instanceof EntryBasicBlock and - id = idOfAstNode(jbp.getFirstNode().(AstCfgNode).getAstNode()) and - kind = 1 - } - - string getSplitString(JoinPredecessorBasicBlock jbp) { - result = jbp.getFirstNode().(AstCfgNode).getSplitsString() - or - not exists(jbp.getFirstNode().(AstCfgNode).getSplitsString()) and - result = "" - } - } - - /** - * Gets the `i`th predecessor of join block `jb`, with respect to some - * arbitrary order. - */ - cached - JoinPredecessorBasicBlock getJoinBlockPredecessor(JoinBasicBlock jb, int i) { - result = - rank[i + 1](JoinPredecessorBasicBlock jbp, int id, int kind | - jbp = jb.getAPredecessor() and JoinBlockPredecessors::hasIdAndKind(jbp, id, kind) - | - jbp order by id, kind, JoinBlockPredecessors::getSplitString(jbp) - ) - } - /** A basic block with more than one predecessor. */ final class JoinBasicBlock extends BasicBlock { JoinBasicBlock() { this.getFirstNode().isJoin() }