diff --git a/ruby/ql/consistency-queries/CfgConsistency.ql b/ruby/ql/consistency-queries/CfgConsistency.ql index 297b81678ee..5af4f22fc26 100644 --- a/ruby/ql/consistency-queries/CfgConsistency.ql +++ b/ruby/ql/consistency-queries/CfgConsistency.ql @@ -11,9 +11,7 @@ import codeql.ruby.controlflow.internal.ControlFlowGraphImpl as CfgImpl query predicate nonPostOrderExpr(Expr e, string cls) { cls = e.getPrimaryQlClasses() and not exists(e.getDesugared()) and - not e instanceof BeginExpr and - not e instanceof Namespace and - not e instanceof Toplevel and + not e instanceof BodyStmt and exists(AstNode last, Completion c | CfgImpl::last(e, last, c) and last != e and diff --git a/ruby/ql/lib/codeql/ruby/controlflow/internal/ControlFlowGraphImpl.qll b/ruby/ql/lib/codeql/ruby/controlflow/internal/ControlFlowGraphImpl.qll index f564633bb00..63198a17cf7 100644 --- a/ruby/ql/lib/codeql/ruby/controlflow/internal/ControlFlowGraphImpl.qll +++ b/ruby/ql/lib/codeql/ruby/controlflow/internal/ControlFlowGraphImpl.qll @@ -100,24 +100,26 @@ private class EndBlockScope extends CfgScopeImpl, EndBlock { } } -private class BodyStmtCallableScope extends CfgScopeImpl, AstInternal::TBodyStmt, Callable { - final override predicate entry(AstNode first) { this.(Trees::BodyStmtTree).firstInner(first) } - - final override predicate exit(AstNode last, Completion c) { - this.(Trees::BodyStmtTree).lastInner(last, c) - } -} - -private class BraceBlockScope extends CfgScopeImpl, BraceBlock { +private class CallableScope extends CfgScopeImpl, Callable { final override predicate entry(AstNode first) { - first(this.(Trees::BraceBlockTree).getBodyChild(0, _), first) + first(this.(Trees::CallableTree).getBodyChild(0), first) } final override predicate exit(AstNode last, Completion c) { - last(this.(Trees::BraceBlockTree).getLastBodyChild(), last, c) + this.getBody().(Trees::BodyStmtTree).last(last, c) or - last(this.(Trees::BraceBlockTree).getBodyChild(_, _), last, c) and - not c instanceof NormalCompletion + exists(int i | + not exists(this.getBody()) and + last(this.(Trees::CallableTree).getBodyChild(i), last, c) and + not exists(this.(Trees::CallableTree).getBodyChild(i + 1)) + ) + or + exists(AstNode child | + child = this.(Trees::CallableTree).getBodyChild(_) and + not child = this.getBody() and + last(child, last, c) and + not c instanceof NormalCompletion + ) } } @@ -159,10 +161,6 @@ module Trees { } private class BeginTree extends BodyStmtTree instanceof BeginExpr { - final override predicate first(AstNode first) { this.firstInner(first) } - - final override predicate last(AstNode last, Completion c) { this.lastInner(last, c) } - final override predicate propagatesAbnormal(AstNode child) { none() } } @@ -196,16 +194,14 @@ module Trees { private class BlockParameterTree extends NonDefaultValueParameterTree instanceof BlockParameter { } - abstract class BodyStmtTree extends StmtSequenceTree instanceof BodyStmt { + class BodyStmtTree extends StmtSequenceTree instanceof BodyStmt { /** Gets a rescue clause in this block. */ final RescueClause getARescue() { result = super.getRescue(_) } /** Gets the `ensure` clause in this block, if any. */ final StmtSequence getEnsure() { result = super.getEnsure() } - override predicate first(AstNode first) { first = this } - - predicate firstInner(AstNode first) { + override predicate first(AstNode first) { first(this.getBodyChild(0, _), first) or not exists(this.getBodyChild(_, _)) and @@ -217,7 +213,7 @@ module Trees { ) } - predicate lastInner(AstNode last, Completion c) { + override predicate last(AstNode last, Completion c) { exists(boolean ensurable | last = this.getAnEnsurePredecessor(c, ensurable) | not super.hasEnsure() or @@ -387,27 +383,28 @@ module Trees { private class BooleanLiteralTree extends LeafTree instanceof BooleanLiteral { } - class BraceBlockTree extends StmtSequenceTree instanceof BraceBlock { - final override predicate propagatesAbnormal(AstNode child) { none() } - - final override AstNode getBodyChild(int i, boolean rescuable) { - result = super.getParameter(i) and rescuable = false + class BraceBlockTree extends CallableTree instanceof BraceBlock { + final override AstNode getBodyChild(int i) { + result = super.getParameter(i) or - result = super.getLocalVariable(i - super.getNumberOfParameters()) and rescuable = false + result = super.getLocalVariable(i - super.getNumberOfParameters()) or - result = - StmtSequenceTree.super - .getBodyChild(i - super.getNumberOfParameters() - count(super.getALocalVariable()), - rescuable) + result = super.getBody() and + i = super.getNumberOfParameters() + count(super.getALocalVariable()) } + } + + class CallableTree extends PostOrderTree instanceof Callable { + final override predicate propagatesAbnormal(AstNode child) { none() } override predicate first(AstNode first) { first = this } + abstract AstNode getBodyChild(int i); + override predicate succ(AstNode pred, AstNode succ, Completion c) { - // Normal left-to-right evaluation in the body exists(int i | - last(this.getBodyChild(i, _), pred, c) and - first(this.getBodyChild(i + 1, _), succ) and + last(this.getBodyChild(i), pred, c) and + first(this.getBodyChild(i + 1), succ) and c instanceof NormalCompletion ) } @@ -1016,20 +1013,16 @@ module Trees { final override predicate succ(AstNode pred, AstNode succ, Completion c) { none() } } - private class DoBlockTree extends BodyStmtTree instanceof DoBlock { + private class DoBlockTree extends CallableTree instanceof DoBlock { /** Gets the `i`th child in the body of this block. */ - final override AstNode getBodyChild(int i, boolean rescuable) { - result = super.getParameter(i) and rescuable = false + final override AstNode getBodyChild(int i) { + result = super.getParameter(i) or - result = super.getLocalVariable(i - super.getNumberOfParameters()) and rescuable = false + result = super.getLocalVariable(i - super.getNumberOfParameters()) or - result = - BodyStmtTree.super - .getBodyChild(i - super.getNumberOfParameters() - count(super.getALocalVariable()), - rescuable) + result = super.getBody() and + i = super.getNumberOfParameters() + count(super.getALocalVariable()) } - - override predicate propagatesAbnormal(AstNode child) { none() } } private class EmptyStatementTree extends LeafTree instanceof EmptyStmt { } @@ -1073,14 +1066,12 @@ module Trees { final override AstNode getAccessNode() { result = super.getDefiningAccess() } } - private class LambdaTree extends BodyStmtTree instanceof Lambda { - final override predicate propagatesAbnormal(AstNode child) { none() } - + private class LambdaTree extends CallableTree instanceof Lambda { /** Gets the `i`th child in the body of this block. */ - final override AstNode getBodyChild(int i, boolean rescuable) { - result = super.getParameter(i) and rescuable = false + final override AstNode getBodyChild(int i) { + result = super.getParameter(i) or - result = BodyStmtTree.super.getBodyChild(i - super.getNumberOfParameters(), rescuable) + result = super.getBody() and i = super.getNumberOfParameters() } } @@ -1151,14 +1142,12 @@ module Trees { private class MethodNameTree extends LeafTree instanceof MethodName, AstInternal::TTokenMethodName { } - private class MethodTree extends BodyStmtTree instanceof Method { - final override predicate propagatesAbnormal(AstNode child) { none() } - + private class MethodTree extends CallableTree instanceof Method { /** Gets the `i`th child in the body of this block. */ - final override AstNode getBodyChild(int i, boolean rescuable) { - result = super.getParameter(i) and rescuable = false + final override AstNode getBodyChild(int i) { + result = super.getParameter(i) or - result = BodyStmtTree.super.getBodyChild(i - super.getNumberOfParameters(), rescuable) + result = super.getBody() and i = super.getNumberOfParameters() } } @@ -1183,12 +1172,12 @@ module Trees { BodyStmtTree.super.succ(pred, succ, c) or pred = this and - this.firstInner(succ) and + super.first(succ) and c instanceof SimpleCompletion } final override predicate last(AstNode last, Completion c) { - this.lastInner(last, c) + super.last(last, c) or not exists(this.getAChild(_)) and last = this and @@ -1328,7 +1317,7 @@ module Trees { private class SingletonClassTree extends BodyStmtTree instanceof SingletonClass { final override predicate first(AstNode first) { - this.firstInner(first) + super.first(first) or not exists(this.getAChild(_)) and first = this @@ -1338,7 +1327,12 @@ module Trees { BodyStmtTree.super.succ(pred, succ, c) or succ = this and - this.lastInner(pred, c) + super.last(pred, c) + } + + final override predicate last(AstNode last, Completion c) { + last = this and + c.isValidFor(this) } /** Gets the `i`th child in the body of this block. */ @@ -1351,20 +1345,18 @@ module Trees { } } - private class SingletonMethodTree extends BodyStmtTree instanceof SingletonMethod { - final override predicate propagatesAbnormal(AstNode child) { none() } - + private class SingletonMethodTree extends CallableTree instanceof SingletonMethod { /** Gets the `i`th child in the body of this block. */ - final override AstNode getBodyChild(int i, boolean rescuable) { - result = super.getParameter(i) and rescuable = false + final override AstNode getBodyChild(int i) { + result = super.getParameter(i) or - result = BodyStmtTree.super.getBodyChild(i - super.getNumberOfParameters(), rescuable) + result = super.getBody() and i = super.getNumberOfParameters() } override predicate first(AstNode first) { first(super.getObject(), first) } override predicate succ(AstNode pred, AstNode succ, Completion c) { - BodyStmtTree.super.succ(pred, succ, c) + CallableTree.super.succ(pred, succ, c) or last(super.getObject(), pred, c) and succ = this and @@ -1443,10 +1435,6 @@ module Trees { or result = BodyStmtTree.super.getBodyChild(i - count(super.getABeginBlock()), rescuable) } - - final override predicate first(AstNode first) { super.firstInner(first) } - - final override predicate last(AstNode last, Completion c) { super.lastInner(last, c) } } private class UndefStmtTree extends StandardPreOrderTree instanceof UndefStmt { diff --git a/ruby/ql/lib/codeql/ruby/controlflow/internal/Splitting.qll b/ruby/ql/lib/codeql/ruby/controlflow/internal/Splitting.qll index 782315dc14c..737f450b4f2 100644 --- a/ruby/ql/lib/codeql/ruby/controlflow/internal/Splitting.qll +++ b/ruby/ql/lib/codeql/ruby/controlflow/internal/Splitting.qll @@ -246,7 +246,7 @@ module EnsureSplitting { private predicate exit0(AstNode pred, Trees::BodyStmtTree block, int nestLevel, Completion c) { this.appliesToPredecessor(pred) and nestLevel = block.getNestLevel() and - block.lastInner(pred, c) + block.last(pred, c) } /**