diff --git a/ruby/ql/consistency-queries/DataFlowConsistency.ql b/ruby/ql/consistency-queries/DataFlowConsistency.ql index 24766016cbb..4e0588d5d4a 100644 --- a/ruby/ql/consistency-queries/DataFlowConsistency.ql +++ b/ruby/ql/consistency-queries/DataFlowConsistency.ql @@ -28,7 +28,7 @@ private module Input implements InputSig { exists(CfgNodes::ExprCfgNode n | arg.argumentOf(call, _) and n = call.asCall() and - arg.asExpr().getASuccessor(any(SuccessorTypes::ConditionalSuccessor c)).getASuccessor*() = n and + arg.asExpr().getASuccessor(any(ConditionalSuccessor c)).getASuccessor*() = n and n.getASplit() instanceof Split::ConditionalCompletionSplit ) } diff --git a/ruby/ql/lib/codeql/ruby/controlflow/BasicBlocks.qll b/ruby/ql/lib/codeql/ruby/controlflow/BasicBlocks.qll index bd13fca2875..72d8360b24c 100644 --- a/ruby/ql/lib/codeql/ruby/controlflow/BasicBlocks.qll +++ b/ruby/ql/lib/codeql/ruby/controlflow/BasicBlocks.qll @@ -8,7 +8,6 @@ private import codeql.ruby.ast.internal.TreeSitter private import codeql.ruby.controlflow.ControlFlowGraph private import internal.ControlFlowGraphImpl as CfgImpl private import CfgNodes -private import SuccessorTypes private import CfgImpl::BasicBlocks as BasicBlocksImpl private import codeql.controlflow.BasicBlock as BB diff --git a/ruby/ql/lib/codeql/ruby/controlflow/ControlFlowGraph.qll b/ruby/ql/lib/codeql/ruby/controlflow/ControlFlowGraph.qll index 1d3632ba1c0..8a7abea3090 100644 --- a/ruby/ql/lib/codeql/ruby/controlflow/ControlFlowGraph.qll +++ b/ruby/ql/lib/codeql/ruby/controlflow/ControlFlowGraph.qll @@ -2,9 +2,9 @@ overlay[local] module; +import codeql.controlflow.SuccessorType private import codeql.ruby.AST private import codeql.ruby.controlflow.BasicBlocks -private import SuccessorTypes private import internal.ControlFlowGraphImpl as CfgImpl private import internal.Splitting as Splitting private import internal.Completion @@ -59,241 +59,6 @@ class CfgNode extends CfgImpl::Node { BasicBlock getBasicBlock() { result.getANode() = this } } -/** The type of a control flow successor. */ -class SuccessorType extends CfgImpl::TSuccessorType { - /** Gets a textual representation of successor type. */ - string toString() { none() } -} - -/** Provides different types of control flow successor types. */ -module SuccessorTypes { - /** A normal control flow successor. */ - class NormalSuccessor extends SuccessorType, CfgImpl::TSuccessorSuccessor { - final override string toString() { result = "successor" } - } - - /** - * A conditional control flow successor. Either a Boolean successor (`BooleanSuccessor`) - * or a matching successor (`MatchingSuccessor`) - */ - class ConditionalSuccessor extends SuccessorType { - boolean value; - - ConditionalSuccessor() { - this = CfgImpl::TBooleanSuccessor(value) or - this = CfgImpl::TMatchingSuccessor(value) - } - - /** Gets the Boolean value of this successor. */ - final boolean getValue() { result = value } - - override string toString() { result = this.getValue().toString() } - } - - /** - * A Boolean control flow successor. - * - * For example, in - * - * ```rb - * if x >= 0 - * puts "positive" - * else - * puts "negative" - * end - * ``` - * - * `x >= 0` has both a `true` successor and a `false` successor. - */ - class BooleanSuccessor extends ConditionalSuccessor, CfgImpl::TBooleanSuccessor { } - - /** - * A matching control flow successor. - * - * For example, this program fragment: - * - * ```rb - * case x - * when 1 then puts "one" - * else puts "not one" - * end - * ``` - * - * has a control flow graph containing matching successors: - * - * ``` - * x - * | - * 1 - * / \ - * / \ - * / \ - * / \ - * match non-match - * | | - * puts "one" puts "not one" - * ``` - */ - class MatchingSuccessor extends ConditionalSuccessor, CfgImpl::TMatchingSuccessor { - override string toString() { if value = true then result = "match" else result = "no-match" } - } - - /** - * A `return` control flow successor. - * - * Example: - * - * ```rb - * def sum(x,y) - * return x + y - * end - * ``` - * - * The exit node of `sum` is a `return` successor of the `return x + y` - * statement. - */ - class ReturnSuccessor extends SuccessorType, CfgImpl::TReturnSuccessor { - final override string toString() { result = "return" } - } - - /** - * A `break` control flow successor. - * - * Example: - * - * ```rb - * def m - * while x >= 0 - * x -= 1 - * if num > 100 - * break - * end - * end - * puts "done" - * end - * ``` - * - * The node `puts "done"` is `break` successor of the node `break`. - */ - class BreakSuccessor extends SuccessorType, CfgImpl::TBreakSuccessor { - final override string toString() { result = "break" } - } - - /** - * A `next` control flow successor. - * - * Example: - * - * ```rb - * def m - * while x >= 0 - * x -= 1 - * if num > 100 - * next - * end - * end - * puts "done" - * end - * ``` - * - * The node `x >= 0` is `next` successor of the node `next`. - */ - class NextSuccessor extends SuccessorType, CfgImpl::TNextSuccessor { - final override string toString() { result = "next" } - } - - /** - * A `redo` control flow successor. - * - * Example: - * - * Example: - * - * ```rb - * def m - * while x >= 0 - * x -= 1 - * if num > 100 - * redo - * end - * end - * puts "done" - * end - * ``` - * - * The node `x -= 1` is `redo` successor of the node `redo`. - */ - class RedoSuccessor extends SuccessorType, CfgImpl::TRedoSuccessor { - final override string toString() { result = "redo" } - } - - /** - * A `retry` control flow successor. - * - * Example: - * - * Example: - * - * ```rb - * def m - * begin - * puts "Retry" - * raise - * rescue - * retry - * end - * end - * ``` - * - * The node `puts "Retry"` is `retry` successor of the node `retry`. - */ - class RetrySuccessor extends SuccessorType, CfgImpl::TRetrySuccessor { - final override string toString() { result = "retry" } - } - - /** - * An exceptional control flow successor. - * - * Example: - * - * ```rb - * def m x - * if x > 2 - * raise "x > 2" - * end - * puts "x <= 2" - * end - * ``` - * - * The exit node of `m` is an exceptional successor of the node - * `raise "x > 2"`. - */ - class RaiseSuccessor extends SuccessorType, CfgImpl::TRaiseSuccessor { - final override string toString() { result = "raise" } - } - - /** - * An exit control flow successor. - * - * Example: - * - * ```rb - * def m x - * if x > 2 - * exit 1 - * end - * puts "x <= 2" - * end - * ``` - * - * The exit node of `m` is an exit successor of the node - * `exit 1`. - */ - class ExitSuccessor extends SuccessorType, CfgImpl::TExitSuccessor { - final override string toString() { result = "exit" } - } -} - class Split = Splitting::Split; /** Provides different kinds of control flow graph splittings. */ diff --git a/ruby/ql/lib/codeql/ruby/controlflow/internal/Completion.qll b/ruby/ql/lib/codeql/ruby/controlflow/internal/Completion.qll index b8edf83c20d..bcfe8f98d43 100644 --- a/ruby/ql/lib/codeql/ruby/controlflow/internal/Completion.qll +++ b/ruby/ql/lib/codeql/ruby/controlflow/internal/Completion.qll @@ -12,7 +12,6 @@ private import codeql.ruby.ast.internal.Control private import codeql.ruby.controlflow.ControlFlowGraph private import ControlFlowGraphImpl as CfgImpl private import NonReturning -private import SuccessorTypes private newtype TCompletion = TSimpleCompletion() or @@ -267,7 +266,7 @@ abstract private class NonNestedNormalCompletion extends NormalCompletion { } /** A simple (normal) completion. */ class SimpleCompletion extends NonNestedNormalCompletion, TSimpleCompletion { - override NormalSuccessor getAMatchingSuccessorType() { any() } + override DirectSuccessor getAMatchingSuccessorType() { any() } override string toString() { result = "simple" } } @@ -377,7 +376,7 @@ class NextCompletion extends Completion { this = TNestedCompletion(_, TNextCompletion(), _) } - override NextSuccessor getAMatchingSuccessorType() { any() } + override ContinueSuccessor getAMatchingSuccessorType() { any() } override string toString() { // `NestedCompletion` defines `toString()` for the other case @@ -431,7 +430,7 @@ class RaiseCompletion extends Completion { this = TNestedCompletion(_, TRaiseCompletion(), _) } - override RaiseSuccessor getAMatchingSuccessorType() { any() } + override ExceptionSuccessor getAMatchingSuccessorType() { any() } override string toString() { // `NestedCompletion` defines `toString()` for the other case diff --git a/ruby/ql/lib/codeql/ruby/controlflow/internal/ControlFlowGraphImpl.qll b/ruby/ql/lib/codeql/ruby/controlflow/internal/ControlFlowGraphImpl.qll index d0d418a839f..d45f6f19cdf 100644 --- a/ruby/ql/lib/codeql/ruby/controlflow/internal/ControlFlowGraphImpl.qll +++ b/ruby/ql/lib/codeql/ruby/controlflow/internal/ControlFlowGraphImpl.qll @@ -50,17 +50,13 @@ private module CfgInput implements CfgShared::InputSig { SuccessorType getAMatchingSuccessorType(Completion c) { result = c.getAMatchingSuccessorType() } - predicate successorTypeIsSimple(SuccessorType t) { - t instanceof Cfg::SuccessorTypes::NormalSuccessor - } + predicate successorTypeIsSimple(SuccessorType t) { t instanceof Cfg::DirectSuccessor } - predicate successorTypeIsCondition(SuccessorType t) { - t instanceof Cfg::SuccessorTypes::ConditionalSuccessor - } + predicate successorTypeIsCondition(SuccessorType t) { t instanceof Cfg::ConditionalSuccessor } predicate isAbnormalExitType(SuccessorType t) { - t instanceof Cfg::SuccessorTypes::RaiseSuccessor or - t instanceof Cfg::SuccessorTypes::ExitSuccessor + t instanceof Cfg::ExceptionSuccessor or + t instanceof Cfg::ExitSuccessor } private predicate id(Ruby::AstNode node1, Ruby::AstNode node2) { node1 = node2 } @@ -1528,21 +1524,3 @@ CfgScope getCfgScope(AstNode n) { pragma[only_bind_into](result) = getCfgScopeImpl(n0) ) } - -cached -private module Cached { - cached - newtype TSuccessorType = - TSuccessorSuccessor() or - TBooleanSuccessor(boolean b) { b in [false, true] } or - TMatchingSuccessor(boolean isMatch) { isMatch in [false, true] } or - TReturnSuccessor() or - TBreakSuccessor() or - TNextSuccessor() or - TRedoSuccessor() or - TRetrySuccessor() or - TRaiseSuccessor() or // TODO: Add exception type? - TExitSuccessor() -} - -import Cached diff --git a/ruby/ql/lib/codeql/ruby/controlflow/internal/Guards.qll b/ruby/ql/lib/codeql/ruby/controlflow/internal/Guards.qll index a60102e017c..387fb03dc9a 100644 --- a/ruby/ql/lib/codeql/ruby/controlflow/internal/Guards.qll +++ b/ruby/ql/lib/codeql/ruby/controlflow/internal/Guards.qll @@ -6,7 +6,7 @@ private import codeql.ruby.CFG /** Holds if the guard `guard` controls block `bb` upon evaluating to `branch`. */ pragma[nomagic] predicate guardControlsBlock(CfgNodes::AstCfgNode guard, BasicBlock bb, boolean branch) { - exists(ConditionBlock conditionBlock, SuccessorTypes::ConditionalSuccessor s | + exists(ConditionBlock conditionBlock, ConditionalSuccessor s | guard = conditionBlock.getLastNode() and s.getValue() = branch and conditionBlock.edgeDominates(bb, s) diff --git a/ruby/ql/lib/codeql/ruby/controlflow/internal/Splitting.qll b/ruby/ql/lib/codeql/ruby/controlflow/internal/Splitting.qll index 146d5927479..6a809292217 100644 --- a/ruby/ql/lib/codeql/ruby/controlflow/internal/Splitting.qll +++ b/ruby/ql/lib/codeql/ruby/controlflow/internal/Splitting.qll @@ -8,7 +8,6 @@ private import codeql.ruby.AST as Ast private import Completion as Comp private import Comp private import ControlFlowGraphImpl -private import SuccessorTypes private import codeql.ruby.controlflow.ControlFlowGraph cached @@ -123,16 +122,19 @@ module EnsureSplitting { * the `ensure` block must end with a `return` as well (provided that * the `ensure` block executes normally). */ - class EnsureSplitType extends SuccessorType { + class EnsureSplitType instanceof SuccessorType { EnsureSplitType() { not this instanceof ConditionalSuccessor } + /** Gets a textual representation of this successor type. */ + string toString() { result = super.toString() } + /** Holds if this split type matches entry into an `ensure` block with completion `c`. */ predicate isSplitForEntryCompletion(Completion c) { if c instanceof NormalCompletion then // If the entry into the `ensure` block completes with any normal completion, // it simply means normal execution after the `ensure` block - this instanceof NormalSuccessor + this instanceof DirectSuccessor else this = c.getAMatchingSuccessorType() } } @@ -195,7 +197,7 @@ module EnsureSplitting { int getNestLevel() { result = nestLevel } override string toString() { - if type instanceof NormalSuccessor + if type instanceof DirectSuccessor then result = "" else if nestLevel > 0 @@ -271,14 +273,14 @@ module EnsureSplitting { or not c instanceof NormalCompletion or - type instanceof NormalSuccessor + type instanceof DirectSuccessor ) else ( // `ensure` block can exit with inherited completion `c`, which must // match this split inherited = true and type = c.getAMatchingSuccessorType() and - not type instanceof NormalSuccessor + not type instanceof DirectSuccessor ) ) or @@ -308,7 +310,7 @@ module EnsureSplitting { exists(EnsureSplitImpl outer | outer.(EnsureSplit).getNestLevel() = super.getNestLevel() - 1 and outer.exit(pred, c, inherited) and - super.getType() instanceof NormalSuccessor and + super.getType() instanceof DirectSuccessor and inherited = true ) } diff --git a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll index 91408072ed7..9332aa43e52 100644 --- a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll +++ b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll @@ -153,7 +153,7 @@ module LocalFlow { exprTo = nodeTo.asExpr() and n.getReturningNode().getAstNode() instanceof BreakStmt and exprTo.getAstNode() instanceof Loop and - nodeTo.asExpr().getAPredecessor(any(SuccessorTypes::BreakSuccessor s)) = n.getReturningNode() + nodeTo.asExpr().getAPredecessor(any(BreakSuccessor s)) = n.getReturningNode() ) or nodeFrom.asExpr() = nodeTo.(ReturningStatementNode).getReturningNode().getReturnedValueNode() @@ -161,7 +161,7 @@ module LocalFlow { nodeTo.asExpr() = any(CfgNodes::ExprNodes::ForExprCfgNode for | exists(SuccessorType s | - not s instanceof SuccessorTypes::BreakSuccessor and + not s instanceof BreakSuccessor and exists(for.getAPredecessor(s)) ) and nodeFrom.asExpr() = for.getValue() @@ -2386,8 +2386,7 @@ module TypeInference { | m = resolveConstantReadAccess(pattern.getExpr()) and cb.getLastNode() = pattern and - cb.edgeDominates(read.getBasicBlock(), - any(SuccessorTypes::MatchingSuccessor match | match.getValue() = true)) and + cb.edgeDominates(read.getBasicBlock(), any(MatchingSuccessor match | match.getValue() = true)) and caseRead = def.getARead() and read = def.getARead() and case.getValue() = caseRead diff --git a/ruby/ql/lib/codeql/ruby/dataflow/internal/SsaImpl.qll b/ruby/ql/lib/codeql/ruby/dataflow/internal/SsaImpl.qll index af5745f6fd3..029d4c53060 100644 --- a/ruby/ql/lib/codeql/ruby/dataflow/internal/SsaImpl.qll +++ b/ruby/ql/lib/codeql/ruby/dataflow/internal/SsaImpl.qll @@ -487,7 +487,7 @@ private module DataFlowIntegrationInput implements Impl::DataFlowIntegrationInpu * from `bb1` to `bb2`. */ predicate hasValueBranchEdge(BasicBlock bb1, BasicBlock bb2, GuardValue branch) { - exists(Cfg::SuccessorTypes::ConditionalSuccessor s | + exists(Cfg::ConditionalSuccessor s | this.getBasicBlock() = bb1 and bb2 = bb1.getASuccessor(s) and s.getValue() = branch diff --git a/ruby/ql/test/library-tests/controlflow/graph/BasicBlocks.ql b/ruby/ql/test/library-tests/controlflow/graph/BasicBlocks.ql index 8db21dd4496..c99de9bc0a8 100644 --- a/ruby/ql/test/library-tests/controlflow/graph/BasicBlocks.ql +++ b/ruby/ql/test/library-tests/controlflow/graph/BasicBlocks.ql @@ -9,7 +9,7 @@ query predicate immediateDominator(BasicBlock bb1, BasicBlock bb2) { bb1.getImmediateDominator() = bb2 } -query predicate controls(ConditionBlock bb1, BasicBlock bb2, SuccessorTypes::ConditionalSuccessor t) { +query predicate controls(ConditionBlock bb1, BasicBlock bb2, ConditionalSuccessor t) { bb1.edgeDominates(bb2, t) } diff --git a/ruby/ql/test/library-tests/dataflow/barrier-guards/barrier-guards.ql b/ruby/ql/test/library-tests/dataflow/barrier-guards/barrier-guards.ql index 1bf7461e7fa..86609a73909 100644 --- a/ruby/ql/test/library-tests/dataflow/barrier-guards/barrier-guards.ql +++ b/ruby/ql/test/library-tests/dataflow/barrier-guards/barrier-guards.ql @@ -12,7 +12,7 @@ query predicate newStyleBarrierGuards(DataFlow::Node n) { n instanceof StringConstArrayInclusionCallBarrier } -query predicate controls(CfgNode condition, BasicBlock bb, SuccessorTypes::ConditionalSuccessor s) { +query predicate controls(CfgNode condition, BasicBlock bb, ConditionalSuccessor s) { exists(ConditionBlock cb | cb.edgeDominates(bb, s) and condition = cb.getLastNode()