From bb3abc815f527aff46a41c8d2b2808c728ab26bc Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Mon, 18 Aug 2025 14:03:49 +0200 Subject: [PATCH] SSA: Update input to use member predicates. --- .../ir/dataflow/internal/SsaImplCommon.qll | 10 ++- .../ql/consistency-queries/CfgConsistency.ql | 4 +- .../code/csharp/controlflow/BasicBlocks.qll | 2 + .../controlflow/internal/PreBasicBlocks.qll | 19 ++++- .../csharp/controlflow/internal/PreSsa.qll | 20 ++--- .../code/csharp/dataflow/internal/BaseSSA.qll | 6 -- .../dataflow/internal/DataFlowPrivate.qll | 12 +-- .../code/csharp/dataflow/internal/SsaImpl.qll | 4 - .../code/java/controlflow/BasicBlocks.qll | 6 ++ .../code/java/dataflow/internal/BaseSSA.qll | 4 - .../dataflow/internal/DataFlowPrivate.qll | 12 +-- .../code/java/dataflow/internal/SsaImpl.qll | 4 - .../dataflow/internal/VariableCapture.qll | 16 ++-- .../dataflow/internal/sharedlib/Ssa.qll | 29 ++++--- python/ql/lib/semmle/python/Flow.qll | 12 ++- .../dataflow/new/internal/VariableCapture.qll | 14 ++-- .../dataflow/internal/DataFlowPrivate.qll | 12 +-- .../codeql/ruby/dataflow/internal/SsaImpl.qll | 4 - .../rust/dataflow/internal/DataFlowImpl.qll | 12 +-- .../codeql/rust/dataflow/internal/SsaImpl.qll | 4 - .../codeql/dataflow/VariableCapture.qll | 47 +++-------- shared/ssa/codeql/ssa/Ssa.qll | 82 +++++-------------- swift/ql/lib/codeql/swift/dataflow/Ssa.qll | 6 -- .../dataflow/internal/DataFlowPrivate.qll | 12 +-- 24 files changed, 147 insertions(+), 206 deletions(-) diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaImplCommon.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaImplCommon.qll index 617e2be8cc3..d86e1ec613f 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaImplCommon.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaImplCommon.qll @@ -778,11 +778,13 @@ module InputSigCommon { ControlFlowNode getNode(int i) { result = this.getInstruction(i) } int length() { result = this.getInstructionCount() } + + BasicBlock getASuccessor() { result = super.getASuccessor() } + + BasicBlock getImmediateDominator() { result.immediatelyDominates(this) } + + predicate inDominanceFrontier(BasicBlock df) { super.dominanceFrontier() = df } } class ControlFlowNode = Instruction; - - BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { result.immediatelyDominates(bb) } - - BasicBlock getABasicBlockSuccessor(BasicBlock bb) { result = bb.getASuccessor() } } diff --git a/csharp/ql/consistency-queries/CfgConsistency.ql b/csharp/ql/consistency-queries/CfgConsistency.ql index 3caf64f9aec..5fb52b3b09e 100644 --- a/csharp/ql/consistency-queries/CfgConsistency.ql +++ b/csharp/ql/consistency-queries/CfgConsistency.ql @@ -45,8 +45,8 @@ predicate bbIntraSuccInconsistency(ControlFlowElement pred, ControlFlowElement s bb.getASuccessor().getFirstElement() = succ ) and not exists(PreBasicBlock bb, int i | - bb.getElement(i) = pred and - bb.getElement(i + 1) = succ + bb.getNode(i) = pred and + bb.getNode(i + 1) = succ ) } diff --git a/csharp/ql/lib/semmle/code/csharp/controlflow/BasicBlocks.qll b/csharp/ql/lib/semmle/code/csharp/controlflow/BasicBlocks.qll index 9e548838ade..7079f2cb0a0 100644 --- a/csharp/ql/lib/semmle/code/csharp/controlflow/BasicBlocks.qll +++ b/csharp/ql/lib/semmle/code/csharp/controlflow/BasicBlocks.qll @@ -58,6 +58,8 @@ final class BasicBlock extends BasicBlocksImpl::BasicBlock { result.getFirstNode() = this.getLastNode().getAFalseSuccessor() } + BasicBlock getASuccessor() { result = super.getASuccessor() } + /** Gets the control flow node at a specific (zero-indexed) position in this basic block. */ ControlFlow::Node getNode(int pos) { result = super.getNode(pos) } diff --git a/csharp/ql/lib/semmle/code/csharp/controlflow/internal/PreBasicBlocks.qll b/csharp/ql/lib/semmle/code/csharp/controlflow/internal/PreBasicBlocks.qll index 08debc21c0d..3e6bf7f1f76 100644 --- a/csharp/ql/lib/semmle/code/csharp/controlflow/internal/PreBasicBlocks.qll +++ b/csharp/ql/lib/semmle/code/csharp/controlflow/internal/PreBasicBlocks.qll @@ -63,16 +63,20 @@ class PreBasicBlock extends ControlFlowElement { PreBasicBlock getAPredecessor() { result.getASuccessor() = this } - ControlFlowElement getElement(int pos) { bbIndex(this, result, pos) } + ControlFlowElement getNode(int pos) { bbIndex(this, result, pos) } - ControlFlowElement getAnElement() { result = this.getElement(_) } + deprecated ControlFlowElement getElement(int pos) { result = this.getNode(pos) } + + ControlFlowElement getAnElement() { result = this.getNode(_) } ControlFlowElement getFirstElement() { result = this } - ControlFlowElement getLastElement() { result = this.getElement(this.length() - 1) } + ControlFlowElement getLastElement() { result = this.getNode(this.length() - 1) } int length() { result = strictcount(this.getAnElement()) } + PreBasicBlock getImmediateDominator() { bbIDominates(result, this) } + predicate immediatelyDominates(PreBasicBlock bb) { bbIDominates(this, bb) } pragma[inline] @@ -84,6 +88,15 @@ class PreBasicBlock extends ControlFlowElement { or this.strictlyDominates(bb) } + + predicate inDominanceFrontier(PreBasicBlock df) { + this = df.getAPredecessor() and not bbIDominates(this, df) + or + exists(PreBasicBlock prev | prev.inDominanceFrontier(df) | + bbIDominates(this, prev) and + not bbIDominates(this, df) + ) + } } private Completion getConditionalCompletion(ConditionalCompletion cc) { diff --git a/csharp/ql/lib/semmle/code/csharp/controlflow/internal/PreSsa.qll b/csharp/ql/lib/semmle/code/csharp/controlflow/internal/PreSsa.qll index 6507bbbe04b..9205974853e 100644 --- a/csharp/ql/lib/semmle/code/csharp/controlflow/internal/PreSsa.qll +++ b/csharp/ql/lib/semmle/code/csharp/controlflow/internal/PreSsa.qll @@ -14,7 +14,7 @@ module PreSsa { private predicate definitionAt( AssignableDefinition def, SsaInput::BasicBlock bb, int i, SsaInput::SourceVariable v ) { - bb.getElement(i) = def.getExpr() and + bb.getNode(i) = def.getExpr() and v = def.getTarget() and // In cases like `(x, x) = (0, 1)`, we discard the first (dead) definition of `x` not exists(TupleAssignmentDefinition first, TupleAssignmentDefinition second | first = def | @@ -80,16 +80,10 @@ module PreSsa { } module SsaInput implements SsaImplCommon::InputSig { - class BasicBlock extends PreBasicBlocks::PreBasicBlock { - ControlFlowNode getNode(int i) { result = this.getElement(i) } - } + class BasicBlock = PreBasicBlocks::PreBasicBlock; class ControlFlowNode = ControlFlowElement; - BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { result.immediatelyDominates(bb) } - - BasicBlock getABasicBlockSuccessor(BasicBlock bb) { result = bb.getASuccessor() } - private class ExitBasicBlock extends BasicBlock { ExitBasicBlock() { scopeLast(_, this.getLastElement(), _) } } @@ -142,7 +136,7 @@ module PreSsa { predicate variableRead(BasicBlock bb, int i, SourceVariable v, boolean certain) { exists(AssignableRead read | - read = bb.getElement(i) and + read = bb.getNode(i) and read.getTarget() = v and certain = true ) @@ -163,7 +157,7 @@ module PreSsa { final AssignableRead getARead() { exists(SsaInput::BasicBlock bb, int i | SsaImpl::ssaDefReachesRead(_, this, bb, i) and - result = bb.getElement(i) + result = bb.getNode(i) ) } @@ -177,7 +171,7 @@ module PreSsa { final AssignableRead getAFirstRead() { exists(SsaInput::BasicBlock bb, int i | SsaImpl::firstUse(this, bb, i, true) and - result = bb.getElement(i) + result = bb.getNode(i) ) } @@ -214,9 +208,9 @@ module PreSsa { predicate adjacentReadPairSameVar(AssignableRead read1, AssignableRead read2) { exists(SsaInput::BasicBlock bb1, int i1, SsaInput::BasicBlock bb2, int i2 | - read1 = bb1.getElement(i1) and + read1 = bb1.getNode(i1) and SsaImpl::adjacentUseUse(bb1, i1, bb2, i2, _, true) and - read2 = bb2.getElement(i2) + read2 = bb2.getNode(i2) ) } } diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/BaseSSA.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/BaseSSA.qll index ec1b5a0188e..dc056e2f0eb 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/BaseSSA.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/BaseSSA.qll @@ -49,12 +49,6 @@ module BaseSsa { class ControlFlowNode = ControlFlow::Node; - BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { - result = bb.getImmediateDominator() - } - - BasicBlock getABasicBlockSuccessor(BasicBlock bb) { result = bb.getASuccessor() } - class SourceVariable = PreSsa::SimpleLocalScopeVariable; predicate variableWrite(BasicBlock bb, int i, SourceVariable v, boolean certain) { diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll index ff2bf709251..c5e96e46450 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll @@ -283,16 +283,16 @@ module VariableCapture { class BasicBlock extends BasicBlocks::BasicBlock { Callable getEnclosingCallable() { result = super.getCallable() } + + BasicBlock getASuccessor() { result = super.getASuccessor() } + + BasicBlock getImmediateDominator() { result = super.getImmediateDominator() } + + predicate inDominanceFrontier(BasicBlock df) { super.inDominanceFrontier(df) } } class ControlFlowNode = Cfg::ControlFlow::Node; - BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { - result = bb.getImmediateDominator() - } - - BasicBlock getABasicBlockSuccessor(BasicBlock bb) { result = bb.getASuccessor() } - private predicate thisAccess(ControlFlow::Node cfn, InstanceCallable c) { ThisFlow::thisAccessExpr(cfn.getAstNode()) and cfn.getEnclosingCallable().getEnclosingCallable*() = c diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/SsaImpl.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/SsaImpl.qll index d1490c84916..dd77506ed68 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/SsaImpl.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/SsaImpl.qll @@ -13,10 +13,6 @@ private module SsaInput implements SsaImplCommon::InputSig { class ControlFlowNode = ControlFlow::Node; - BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { result = bb.getImmediateDominator() } - - BasicBlock getABasicBlockSuccessor(BasicBlock bb) { result = bb.getASuccessor() } - class SourceVariable = Ssa::SourceVariable; /** diff --git a/java/ql/lib/semmle/code/java/controlflow/BasicBlocks.qll b/java/ql/lib/semmle/code/java/controlflow/BasicBlocks.qll index e974f711ec4..b7e52f658fd 100644 --- a/java/ql/lib/semmle/code/java/controlflow/BasicBlocks.qll +++ b/java/ql/lib/semmle/code/java/controlflow/BasicBlocks.qll @@ -98,6 +98,12 @@ class BasicBlock extends BbImpl::BasicBlock { /** Gets an immediate successor of this basic block of a given type, if any. */ BasicBlock getASuccessor(Input::SuccessorType t) { result = super.getASuccessor(t) } + BasicBlock getASuccessor() { result = super.getASuccessor() } + + BasicBlock getImmediateDominator() { result = super.getImmediateDominator() } + + predicate inDominanceFrontier(BasicBlock df) { super.inDominanceFrontier(df) } + /** * DEPRECATED: Use `getASuccessor` instead. * 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 5c0fbb88d66..444d8f3cbd9 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/BaseSSA.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/BaseSSA.qll @@ -164,10 +164,6 @@ private module SsaInput implements SsaImplCommon::InputSig { class ControlFlowNode = J::ControlFlowNode; - BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { result.immediatelyDominates(bb) } - - 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 8b9087ecbdc..22b5cc62d57 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowPrivate.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowPrivate.qll @@ -82,16 +82,16 @@ private module CaptureInput implements VariableCapture::InputSig { Callable getEnclosingCallable() { result = super.getEnclosingCallable() } Location getLocation() { result = super.getLocation() } + + BasicBlock getASuccessor() { result = super.getASuccessor() } + + BasicBlock getImmediateDominator() { result = super.getImmediateDominator() } + + predicate inDominanceFrontier(BasicBlock df) { super.inDominanceFrontier(df) } } class ControlFlowNode = J::ControlFlowNode; - 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/SsaImpl.qll b/java/ql/lib/semmle/code/java/dataflow/internal/SsaImpl.qll index 51da69e9d64..20e7fc02336 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/SsaImpl.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/SsaImpl.qll @@ -173,10 +173,6 @@ private module SsaInput implements SsaImplCommon::InputSig { class ControlFlowNode = J::ControlFlowNode; - BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { result.immediatelyDominates(bb) } - - BasicBlock getABasicBlockSuccessor(BasicBlock bb) { result = bb.getASuccessor() } - class SourceVariable = SsaSourceVariable; /** diff --git a/javascript/ql/lib/semmle/javascript/dataflow/internal/VariableCapture.qll b/javascript/ql/lib/semmle/javascript/dataflow/internal/VariableCapture.qll index 6cdb95bc4d9..7881f487e9b 100644 --- a/javascript/ql/lib/semmle/javascript/dataflow/internal/VariableCapture.qll +++ b/javascript/ql/lib/semmle/javascript/dataflow/internal/VariableCapture.qll @@ -108,8 +108,18 @@ module VariableCaptureConfig implements InputSig { class ControlFlowNode = js::ControlFlowNode; - class BasicBlock extends js::BasicBlock { + final private class JsBasicBlock = js::BasicBlock; + + class BasicBlock extends JsBasicBlock { Callable getEnclosingCallable() { result = this.getContainer().getFunctionBoundary() } + + BasicBlock getASuccessor() { result = super.getASuccessor() } + + BasicBlock getImmediateDominator() { result = super.getImmediateDominator() } + + predicate inDominanceFrontier(BasicBlock df) { + df.(js::ReachableJoinBlock).inDominanceFrontierOf(this) + } } class Callable extends js::StmtContainer { @@ -235,10 +245,6 @@ module VariableCaptureConfig implements InputSig { } } - BasicBlock getABasicBlockSuccessor(BasicBlock bb) { result = bb.getASuccessor() } - - BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { result = bb.getImmediateDominator() } - predicate entryBlock(BasicBlock bb) { bb instanceof js::EntryBasicBlock } } diff --git a/javascript/ql/lib/semmle/javascript/dataflow/internal/sharedlib/Ssa.qll b/javascript/ql/lib/semmle/javascript/dataflow/internal/sharedlib/Ssa.qll index 1172a64a057..2d672468946 100644 --- a/javascript/ql/lib/semmle/javascript/dataflow/internal/sharedlib/Ssa.qll +++ b/javascript/ql/lib/semmle/javascript/dataflow/internal/sharedlib/Ssa.qll @@ -12,7 +12,17 @@ private import semmle.javascript.dataflow.internal.VariableOrThis module SsaConfig implements InputSig { class ControlFlowNode = js::ControlFlowNode; - class BasicBlock = js::BasicBlock; + final private class JsBasicBlock = js::BasicBlock; + + class BasicBlock extends JsBasicBlock { + BasicBlock getASuccessor() { result = super.getASuccessor() } + + BasicBlock getImmediateDominator() { result = super.getImmediateDominator() } + + predicate inDominanceFrontier(BasicBlock df) { + df.(js::ReachableJoinBlock).inDominanceFrontierOf(this) + } + } class SourceVariable extends LocalVariableOrThis { SourceVariable() { not this.isCaptured() } @@ -40,11 +50,6 @@ module SsaConfig implements InputSig { certain = true and bb.getNode(i).(ThisUse).getBindingContainer() = v.asThisContainer() } - - predicate getImmediateBasicBlockDominator = BasicBlockInternal::immediateDominator/1; - - pragma[inline] - BasicBlock getABasicBlockSuccessor(BasicBlock bb) { result = bb.getASuccessor() } } import Make @@ -55,7 +60,7 @@ module SsaDataflowInput implements DataFlowIntegrationInputSig { class Expr extends js::ControlFlowNode { Expr() { this = any(SsaConfig::SourceVariable v).getAUse() } - predicate hasCfgNode(js::BasicBlock bb, int i) { this = bb.getNode(i) } + predicate hasCfgNode(SsaConfig::BasicBlock bb, int i) { this = bb.getNode(i) } } predicate ssaDefHasSource(WriteDefinition def) { @@ -82,7 +87,9 @@ module SsaDataflowInput implements DataFlowIntegrationInputSig { * Holds if the evaluation of this guard to `branch` corresponds to the edge * from `bb1` to `bb2`. */ - predicate hasValueBranchEdge(js::BasicBlock bb1, js::BasicBlock bb2, GuardValue branch) { + predicate hasValueBranchEdge( + SsaConfig::BasicBlock bb1, SsaConfig::BasicBlock bb2, GuardValue branch + ) { exists(js::ConditionGuardNode g | g.getTest() = this and bb1 = this.getBasicBlock() and @@ -96,13 +103,15 @@ module SsaDataflowInput implements DataFlowIntegrationInputSig { * branch edge from `bb1` to `bb2`. That is, following the edge from * `bb1` to `bb2` implies that this guard evaluated to `branch`. */ - predicate valueControlsBranchEdge(js::BasicBlock bb1, js::BasicBlock bb2, GuardValue branch) { + predicate valueControlsBranchEdge( + SsaConfig::BasicBlock bb1, SsaConfig::BasicBlock bb2, GuardValue branch + ) { this.hasValueBranchEdge(bb1, bb2, branch) } } pragma[inline] - predicate guardDirectlyControlsBlock(Guard guard, js::BasicBlock bb, GuardValue branch) { + predicate guardDirectlyControlsBlock(Guard guard, SsaConfig::BasicBlock bb, GuardValue branch) { exists(js::ConditionGuardNode g | g.getTest() = guard and g.dominates(bb) and diff --git a/python/ql/lib/semmle/python/Flow.qll b/python/ql/lib/semmle/python/Flow.qll index 90633651f11..8fd8502ede4 100644 --- a/python/ql/lib/semmle/python/Flow.qll +++ b/python/ql/lib/semmle/python/Flow.qll @@ -1082,9 +1082,15 @@ class BasicBlock extends @py_flow_node { * Dominance frontier of a node x is the set of all nodes `other` such that `this` dominates a predecessor * of `other` but does not strictly dominate `other` */ - pragma[noinline] - predicate dominanceFrontier(BasicBlock other) { - this.dominates(other.getAPredecessor()) and not this.strictlyDominates(other) + predicate dominanceFrontier(BasicBlock other) { this.inDominanceFrontier(other) } + + predicate inDominanceFrontier(BasicBlock df) { + this = df.getAPredecessor() and not this = df.getImmediateDominator() + or + exists(BasicBlock prev | prev.inDominanceFrontier(df) | + this = prev.getImmediateDominator() and + not this = df.getImmediateDominator() + ) } private ControlFlowNode firstNode() { result = this } diff --git a/python/ql/lib/semmle/python/dataflow/new/internal/VariableCapture.qll b/python/ql/lib/semmle/python/dataflow/new/internal/VariableCapture.qll index 6cb80881e2a..8ba5247ce52 100644 --- a/python/ql/lib/semmle/python/dataflow/new/internal/VariableCapture.qll +++ b/python/ql/lib/semmle/python/dataflow/new/internal/VariableCapture.qll @@ -23,7 +23,9 @@ private module CaptureInput implements Shared::InputSig { predicate isConstructor() { none() } } - class BasicBlock extends PY::BasicBlock { + final private class PyBasicBlock = PY::BasicBlock; + + class BasicBlock extends PyBasicBlock { int length() { result = count(int i | exists(this.getNode(i))) } Callable getEnclosingCallable() { result = this.getScope() } @@ -34,14 +36,16 @@ private module CaptureInput implements Shared::InputSig { // and we just need a way to identify the basic block // during debugging, so this will be serviceable. Location getLocation() { result = super.getNode(0).getLocation() } + + BasicBlock getASuccessor() { result = super.getASuccessor() } + + BasicBlock getImmediateDominator() { result = super.getImmediateDominator() } + + predicate inDominanceFrontier(BasicBlock df) { super.inDominanceFrontier(df) } } class ControlFlowNode = PY::ControlFlowNode; - BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { result = bb.getImmediateDominator() } - - BasicBlock getABasicBlockSuccessor(BasicBlock bb) { result = bb.getASuccessor() } - class CapturedVariable extends LocalVariable { Function f; diff --git a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll index 59fe0238c7f..266400edb5b 100644 --- a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll +++ b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll @@ -297,16 +297,16 @@ module VariableCapture { class BasicBlock extends BasicBlocks::BasicBlock { Callable getEnclosingCallable() { result = this.getScope() } + + BasicBlock getASuccessor() { result = super.getASuccessor() } + + BasicBlock getImmediateDominator() { result = super.getImmediateDominator() } + + predicate inDominanceFrontier(BasicBlock df) { super.inDominanceFrontier(df) } } class ControlFlowNode = Cfg::CfgNode; - BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { - result = bb.getImmediateDominator() - } - - BasicBlock getABasicBlockSuccessor(BasicBlock bb) { result = bb.getASuccessor() } - class CapturedVariable extends LocalVariable { CapturedVariable() { this.isCaptured() and diff --git a/ruby/ql/lib/codeql/ruby/dataflow/internal/SsaImpl.qll b/ruby/ql/lib/codeql/ruby/dataflow/internal/SsaImpl.qll index fd1619b1c63..fe7e2498a71 100644 --- a/ruby/ql/lib/codeql/ruby/dataflow/internal/SsaImpl.qll +++ b/ruby/ql/lib/codeql/ruby/dataflow/internal/SsaImpl.qll @@ -17,10 +17,6 @@ module SsaInput implements SsaImplCommon::InputSig { class ControlFlowNode = Cfg::CfgNode; - BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { result = bb.getImmediateDominator() } - - BasicBlock getABasicBlockSuccessor(BasicBlock bb) { result = bb.getASuccessor() } - class SourceVariable = LocalVariable; /** diff --git a/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll b/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll index 727f14bb94a..70ae830c2d0 100644 --- a/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll +++ b/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll @@ -862,16 +862,16 @@ module VariableCapture { class BasicBlock extends BasicBlocks::BasicBlock { Callable getEnclosingCallable() { result = this.getScope() } + + BasicBlock getASuccessor() { result = super.getASuccessor() } + + BasicBlock getImmediateDominator() { result = super.getImmediateDominator() } + + predicate inDominanceFrontier(BasicBlock df) { super.inDominanceFrontier(df) } } class ControlFlowNode = CfgNode; - BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { - result = bb.getImmediateDominator() - } - - BasicBlock getABasicBlockSuccessor(BasicBlock bb) { result = bb.getASuccessor() } - class CapturedVariable extends Variable { CapturedVariable() { this.isCaptured() } diff --git a/rust/ql/lib/codeql/rust/dataflow/internal/SsaImpl.qll b/rust/ql/lib/codeql/rust/dataflow/internal/SsaImpl.qll index 9b6d254dec1..6429055fccd 100644 --- a/rust/ql/lib/codeql/rust/dataflow/internal/SsaImpl.qll +++ b/rust/ql/lib/codeql/rust/dataflow/internal/SsaImpl.qll @@ -59,10 +59,6 @@ module SsaInput implements SsaImplCommon::InputSig { class ControlFlowNode = CfgNode; - BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { result = bb.getImmediateDominator() } - - BasicBlock getABasicBlockSuccessor(BasicBlock bb) { result = bb.getASuccessor() } - class SourceVariable = Variable; predicate variableWrite(BasicBlock bb, int i, SourceVariable v, boolean certain) { diff --git a/shared/dataflow/codeql/dataflow/VariableCapture.qll b/shared/dataflow/codeql/dataflow/VariableCapture.qll index 922391221a4..60a94bea351 100644 --- a/shared/dataflow/codeql/dataflow/VariableCapture.qll +++ b/shared/dataflow/codeql/dataflow/VariableCapture.qll @@ -30,6 +30,12 @@ signature module InputSig { /** Gets the location of this basic block. */ Location getLocation(); + + BasicBlock getASuccessor(); + + BasicBlock getImmediateDominator(); + + predicate inDominanceFrontier(BasicBlock df); } /** A control flow node. */ @@ -41,33 +47,8 @@ signature module InputSig { Location getLocation(); } - /** - * Gets the basic block that immediately dominates basic block `bb`, if any. - * - * That is, all paths reaching `bb` from some entry point basic block must go - * through the result. - * - * Example: - * - * ```csharp - * int M(string s) { - * if (s == null) - * throw new ArgumentNullException(nameof(s)); - * return s.Length; - * } - * ``` - * - * The basic block starting on line 2 is an immediate dominator of - * the basic block on line 4 (all paths from the entry point of `M` - * to `return s.Length;` must go through the null check. - */ - BasicBlock getImmediateBasicBlockDominator(BasicBlock bb); - - /** Gets an immediate successor of basic block `bb`, if any. */ - BasicBlock getABasicBlockSuccessor(BasicBlock bb); - /** Holds if `bb` is a control-flow entry point. */ - default predicate entryBlock(BasicBlock bb) { not exists(getImmediateBasicBlockDominator(bb)) } + default predicate entryBlock(BasicBlock bb) { not exists(bb.getImmediateDominator()) } /** A variable that is captured in a closure. */ class CapturedVariable { @@ -332,17 +313,17 @@ module Flow Input> implements OutputSig query predicate uniqueDominator(RelevantBasicBlock bb, string msg) { msg = "BasicBlock has multiple immediate dominators" and - 2 <= strictcount(getImmediateBasicBlockDominator(bb)) + 2 <= strictcount(bb.getImmediateDominator()) } query predicate localDominator(RelevantBasicBlock bb, string msg) { msg = "BasicBlock has non-local dominator" and - bb.getEnclosingCallable() != getImmediateBasicBlockDominator(bb).getEnclosingCallable() + bb.getEnclosingCallable() != bb.getImmediateDominator().getEnclosingCallable() } query predicate localSuccessor(RelevantBasicBlock bb, string msg) { msg = "BasicBlock has non-local successor" and - bb.getEnclosingCallable() != getABasicBlockSuccessor(bb).getEnclosingCallable() + bb.getEnclosingCallable() != bb.getASuccessor().getEnclosingCallable() } query predicate uniqueDefiningScope(CapturedVariable v, string msg) { @@ -690,14 +671,6 @@ module Flow Input> implements OutputSig final class ControlFlowNode = Input::ControlFlowNode; - BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { - result = Input::getImmediateBasicBlockDominator(bb) - } - - BasicBlock getABasicBlockSuccessor(BasicBlock bb) { - result = Input::getABasicBlockSuccessor(bb) - } - class SourceVariable = CaptureContainer; predicate variableWrite(BasicBlock bb, int i, SourceVariable cc, boolean certain) { diff --git a/shared/ssa/codeql/ssa/Ssa.qll b/shared/ssa/codeql/ssa/Ssa.qll index 2aa136ff719..df25142792d 100644 --- a/shared/ssa/codeql/ssa/Ssa.qll +++ b/shared/ssa/codeql/ssa/Ssa.qll @@ -26,6 +26,12 @@ signature module InputSig { /** Gets the location of this basic block. */ Location getLocation(); + + BasicBlock getASuccessor(); + + BasicBlock getImmediateDominator(); + + predicate inDominanceFrontier(BasicBlock df); } /** A control flow node. */ @@ -37,31 +43,6 @@ signature module InputSig { Location getLocation(); } - /** - * Gets the basic block that immediately dominates basic block `bb`, if any. - * - * That is, all paths reaching `bb` from some entry point basic block must go - * through the result. - * - * Example: - * - * ```csharp - * int M(string s) { - * if (s == null) - * throw new ArgumentNullException(nameof(s)); - * return s.Length; - * } - * ``` - * - * The basic block starting on line 2 is an immediate dominator of - * the basic block on line 4 (all paths from the entry point of `M` - * to `return s.Length;` must go through the null check. - */ - BasicBlock getImmediateBasicBlockDominator(BasicBlock bb); - - /** Gets an immediate successor of basic block `bb`, if any. */ - BasicBlock getABasicBlockSuccessor(BasicBlock bb); - /** A variable that can be SSA converted. */ class SourceVariable { /** Gets a textual representation of this variable. */ @@ -111,9 +92,7 @@ signature module InputSig { module Make Input> { private import Input - private BasicBlock getABasicBlockPredecessor(BasicBlock bb) { - getABasicBlockSuccessor(result) = bb - } + private BasicBlock getABasicBlockPredecessor(BasicBlock bb) { result.getASuccessor() = bb } /** * A classification of variable references into reads and @@ -237,9 +216,7 @@ module Make Input> { /** * Holds if source variable `v` is live at the end of basic block `bb`. */ - predicate liveAtExit(BasicBlock bb, SourceVariable v) { - liveAtEntry(getABasicBlockSuccessor(bb), v) - } + predicate liveAtExit(BasicBlock bb, SourceVariable v) { liveAtEntry(bb.getASuccessor(), v) } /** * Holds if variable `v` is live in basic block `bb` at rank `rnk`. @@ -270,25 +247,6 @@ module Make Input> { private import Liveness - /** - * Holds if `df` is in the dominance frontier of `bb`. - * - * This is equivalent to: - * - * ```ql - * bb = getImmediateBasicBlockDominator*(getABasicBlockPredecessor(df)) and - * not bb = getImmediateBasicBlockDominator+(df) - * ``` - */ - private predicate inDominanceFrontier(BasicBlock bb, BasicBlock df) { - bb = getABasicBlockPredecessor(df) and not bb = getImmediateBasicBlockDominator(df) - or - exists(BasicBlock prev | inDominanceFrontier(prev, df) | - bb = getImmediateBasicBlockDominator(prev) and - not bb = getImmediateBasicBlockDominator(df) - ) - } - /** * Holds if `bb` is in the dominance frontier of a block containing a * definition of `v`. @@ -297,7 +255,7 @@ module Make Input> { private predicate inDefDominanceFrontier(BasicBlock bb, SourceVariable v) { exists(BasicBlock defbb, Definition def | def.definesAt(v, defbb, _) and - inDominanceFrontier(defbb, bb) + defbb.inDominanceFrontier(bb) ) } @@ -307,7 +265,7 @@ module Make Input> { */ pragma[nomagic] private predicate inReadDominanceFrontier(BasicBlock bb, SourceVariable v) { - exists(BasicBlock readbb | inDominanceFrontier(readbb, bb) | + exists(BasicBlock readbb | readbb.inDominanceFrontier(bb) | ssaDefReachesRead(v, _, readbb, _) and variableRead(readbb, _, v, true) and not variableWrite(readbb, _, v, _) @@ -389,7 +347,7 @@ module Make Input> { */ pragma[nomagic] private predicate liveThrough(BasicBlock idom, BasicBlock bb, SourceVariable v) { - idom = getImmediateBasicBlockDominator(bb) and + idom = bb.getImmediateDominator() and liveAtExit(bb, v) and not any(Definition def).definesAt(v, bb, _) } @@ -439,7 +397,7 @@ module Make Input> { ssaDefReachesReadWithinBlock(v, def, bb, i) or ssaRef(bb, i, v, Read()) and - ssaDefReachesEndOfBlock(getImmediateBasicBlockDominator(bb), def, v) and + ssaDefReachesEndOfBlock(bb.getImmediateDominator(), def, v) and not ssaDefReachesReadWithinBlock(v, _, bb, i) } @@ -483,7 +441,7 @@ module Make Input> { */ pragma[nomagic] private predicate liveThrough(BasicBlock idom, BasicBlock bb, SourceVariable v) { - idom = getImmediateBasicBlockDominator(bb) and + idom = bb.getImmediateDominator() and liveAtExit(bb, v) and not ssaRef(bb, _, v, _) } @@ -517,7 +475,7 @@ module Make Input> { bb1 = bb2 and refRank(bb1, i1, v, _) + 1 = refRank(bb2, i2, v, Read()) or - refReachesEndOfBlock(bb1, i1, getImmediateBasicBlockDominator(bb2), v) and + refReachesEndOfBlock(bb1, i1, bb2.getImmediateDominator(), v) and 1 = refRank(bb2, i2, v, Read()) } @@ -808,12 +766,12 @@ module Make Input> { DefinitionExt def, SourceVariable v, BasicBlock bb1, BasicBlock bb2 ) { defOccursInBlock(def, bb1, v, _) and - bb2 = getABasicBlockSuccessor(bb1) + bb2 = bb1.getASuccessor() or exists(BasicBlock mid | varBlockReachesExt(def, v, bb1, mid) and ssaDefReachesThroughBlock(def, mid) and - bb2 = getABasicBlockSuccessor(mid) + bb2 = mid.getASuccessor() ) } @@ -943,7 +901,7 @@ module Make Input> { // the node. If two definitions dominate a node then one must dominate the // other, so therefore the definition of _closest_ is given by the dominator // tree. Thus, reaching definitions can be calculated in terms of dominance. - ssaDefReachesEndOfBlockExt0(getImmediateBasicBlockDominator(bb), def, pragma[only_bind_into](v)) and + ssaDefReachesEndOfBlockExt0(bb.getImmediateDominator(), def, pragma[only_bind_into](v)) and liveThroughExt(bb, pragma[only_bind_into](v)) } @@ -1150,7 +1108,7 @@ module Make Input> { predicate uncertainWriteDefinitionInput = SsaDefReachesNew::uncertainWriteDefinitionInput/2; /** Holds if `bb` is a control-flow exit point. */ - private predicate exitBlock(BasicBlock bb) { not exists(getABasicBlockSuccessor(bb)) } + private predicate exitBlock(BasicBlock bb) { not exists(bb.getASuccessor()) } /** * NB: If this predicate is exposed, it should be cached. @@ -1418,7 +1376,7 @@ module Make Input> { or ssaDefReachesRead(v, def, bb, i) and not SsaDefReachesNew::ssaDefReachesReadWithinBlock(v, def, bb, i) and - not def.definesAt(v, getImmediateBasicBlockDominator*(bb), _) + not def.definesAt(v, bb.getImmediateDominator*(), _) ) } @@ -1667,7 +1625,7 @@ module Make Input> { DfInput::keepAllPhiInputBackEdges() and exists(getAPhiInputDef(phi, input)) and phi.getBasicBlock() = bbPhi and - getImmediateBasicBlockDominator+(input) = bbPhi + input.getImmediateDominator+() = bbPhi ) } diff --git a/swift/ql/lib/codeql/swift/dataflow/Ssa.qll b/swift/ql/lib/codeql/swift/dataflow/Ssa.qll index ed75a06e534..e7dff4fe753 100644 --- a/swift/ql/lib/codeql/swift/dataflow/Ssa.qll +++ b/swift/ql/lib/codeql/swift/dataflow/Ssa.qll @@ -15,12 +15,6 @@ module Ssa { class ControlFlowNode = Cfg::ControlFlowNode; - BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { - result = bb.getImmediateDominator() - } - - BasicBlock getABasicBlockSuccessor(BasicBlock bb) { result = bb.getASuccessor() } - private newtype TSourceVariable = TNormalSourceVariable(VarDecl v) or TKeyPathSourceVariable(EntryNode entry) { entry.getScope() instanceof KeyPathExpr } diff --git a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll index 4849c5ac235..712d2282b41 100644 --- a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll +++ b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll @@ -897,16 +897,16 @@ private module CaptureInput implements VariableCapture::InputSig { Callable getEnclosingCallable() { result = super.getScope() } Location getLocation() { result = super.getLocation() } + + BasicBlock getASuccessor() { result = super.getASuccessor() } + + BasicBlock getImmediateDominator() { result = super.getImmediateDominator() } + + predicate inDominanceFrontier(BasicBlock df) { super.inDominanceFrontier(df) } } class ControlFlowNode = Cfg::ControlFlowNode; - BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { - result.(B::BasicBlock).immediatelyDominates(bb) - } - - BasicBlock getABasicBlockSuccessor(BasicBlock bb) { result = bb.(B::BasicBlock).getASuccessor() } - class CapturedVariable instanceof S::VarDecl { CapturedVariable() { any(S::CapturedDecl capturedDecl).getDecl() = this and