Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 1 addition & 30 deletions ruby/ql/consistency-queries/CfgConsistency.ql
Original file line number Diff line number Diff line change
@@ -1,31 +1,2 @@
import codeql.ruby.controlflow.internal.ControlFlowGraphImpl::Consistency as Consistency
import Consistency
import codeql.ruby.AST
import codeql.ruby.CFG
import codeql.ruby.controlflow.internal.Completion
import codeql.ruby.controlflow.internal.ControlFlowGraphImpl as CfgImpl

/**
* All `Expr` nodes are `PostOrderTree`s
*/
query predicate nonPostOrderExpr(Expr e, string cls) {
cls = e.getPrimaryQlClasses() and
not exists(e.getDesugared()) and
not e instanceof BodyStmt and
exists(AstNode last, Completion c |
CfgImpl::last(e, last, c) and
last != e and
c instanceof NormalCompletion
)
}

query predicate scopeNoFirst(CfgScope scope) {
Consistency::scopeNoFirst(scope) and
not scope = any(StmtSequence seq | not exists(seq.getAStmt())) and
not scope =
any(Callable c |
not exists(c.getAParameter()) and
not c.getBody().hasEnsure() and
not exists(c.getBody().getARescue())
)
}
import ControlFlow::Consistency
11 changes: 0 additions & 11 deletions ruby/ql/consistency-queries/DataFlowConsistency.ql
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,6 @@ private module Input implements InputSig<Location, RubyDataFlow> {
not isNonConstantExpr(n.asExpr())
}

predicate multipleArgumentCallExclude(ArgumentNode arg, DataFlowCall call) {
// An argument such as `x` in `if not x then ...` has two successors (and hence
// two calls); one for each Boolean outcome of `x`.
exists(CfgNodes::ExprCfgNode n |
arg.argumentOf(call, _) and
n = call.asCall() and
arg.asExpr().getASuccessor(any(ConditionalSuccessor c)).getASuccessor*() = n and
n.getASplit() instanceof Split::ConditionalCompletionSplit
)
}

predicate uniqueTypeExclude(Node n) {
n =
any(DataFlow::CallNode call |
Expand Down
14 changes: 14 additions & 0 deletions ruby/ql/lib/change-notes/2026-08-24-cfg-swap.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
---
category: breaking
---
* The Ruby control flow graph implementation has been completely replaced. This
affects a number of queries slightly. The CFG now includes additional nodes
to more accurately represent certain constructs. This also means that any
existing code that implicitly relies on very specific details about the CFG
may need to be updated. The CFG no longer uses splitting, which means that
AST nodes now have a unique CFG node representation. In particular,
`ControlFlowNode.getAstNode` has changed its meaning. The AST-to-CFG mapping
remains one-to-many, but now for a different reason. It used to be because of
splitting, but now it's because of additional "helper" CFG nodes. To get the
(now canonical) CFG node for a given AST node, use
`Stmt.getControlFlowNode()` instead.
1 change: 0 additions & 1 deletion ruby/ql/lib/codeql/ruby/CFG.qll
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,3 @@
import codeql.Locations
import controlflow.ControlFlowGraph
import controlflow.CfgNodes as CfgNodes
import controlflow.BasicBlocks
2 changes: 1 addition & 1 deletion ruby/ql/lib/codeql/ruby/ast/Call.qll
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ class Call extends Expr instanceof CallImpl {
TCfgScope(result) = viableCallableLambda(c, _)
)
or
result = getTarget(TNormalCall(this.getAControlFlowNode()))
result = getTarget(TNormalCall(this.getControlFlowNode()))
}

override AstNode getAChild(string pred) {
Expand Down
15 changes: 9 additions & 6 deletions ruby/ql/lib/codeql/ruby/ast/Statement.qll
Original file line number Diff line number Diff line change
Expand Up @@ -6,22 +6,25 @@ private import codeql.ruby.CFG
private import internal.AST
private import internal.TreeSitter
private import internal.Variable
private import codeql.ruby.controlflow.internal.ControlFlowGraphImpl as CfgImpl

/**
* A statement.
*
* This is the root QL class for all statements.
*/
class Stmt extends AstNode, TStmt {
/** Gets a control-flow node for this statement, if any. */
CfgNodes::AstCfgNode getAControlFlowNode() { result.getAstNode() = this }
/** Gets the control-flow node for this statement, if any. */
ControlFlowNode getControlFlowNode() { result.injects(this) }

/** Gets a control-flow entry node for this statement, if any */
AstNode getAControlFlowEntryNode() { result = CfgImpl::getAControlFlowEntryNode(this) }
/**
* DEPRECATED: Use `getControlFlowNode()` instead.
*
* Gets a control-flow node for this statement, if any.
*/
deprecated CfgNodes::AstCfgNode getAControlFlowNode() { result.getAstNode() = this }

/** Gets the control-flow scope of this statement, if any. */
CfgScope getCfgScope() { result = CfgImpl::getCfgScope(this) }
CfgScope getCfgScope() { result = getEnclosingCallable(this) }

/** Gets the enclosing callable, if any. */
Callable getEnclosingCallable() { result = this.getCfgScope() }
Expand Down
20 changes: 10 additions & 10 deletions ruby/ql/lib/codeql/ruby/ast/internal/Constant.qll
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ private module Propagation {
or
isIntExpr(e.(ConstantReadAccess).getValue(), i)
or
forex(ExprCfgNode n | n = e.getAControlFlowNode() | isInt(n, i))
isInt(e.getControlFlowNode(), i)
}

predicate isFloat(ExprCfgNode e, float f) {
Expand Down Expand Up @@ -153,7 +153,7 @@ private module Propagation {
or
isFloatExpr(e.(ConstantReadAccess).getValue(), f)
or
forex(ExprCfgNode n | n = e.getAControlFlowNode() | isFloat(n, f))
isFloat(e.getControlFlowNode(), f)
}

predicate isRational(ExprCfgNode e, int numerator, int denominator) {
Expand All @@ -175,7 +175,7 @@ private module Propagation {
or
isRationalExpr(e.(ConstantReadAccess).getValue(), numerator, denominator)
or
forex(ExprCfgNode n | n = e.getAControlFlowNode() | isRational(n, numerator, denominator))
isRational(e.getControlFlowNode(), numerator, denominator)
}

predicate isComplex(ExprCfgNode e, float real, float imaginary) {
Expand All @@ -197,7 +197,7 @@ private module Propagation {
or
isComplexExpr(e.(ConstantReadAccess).getValue(), real, imaginary)
or
forex(ExprCfgNode n | n = e.getAControlFlowNode() | isComplex(n, real, imaginary))
isComplex(e.getControlFlowNode(), real, imaginary)
}

overlay[local]
Expand Down Expand Up @@ -309,7 +309,7 @@ private module Propagation {
or
isStringExpr(e.(ConstantReadAccess).getValue(), s)
or
forex(ExprCfgNode n | n = e.getAControlFlowNode() | isString(n, s))
isString(e.getControlFlowNode(), s)
}

predicate isSymbol(ExprCfgNode e, string s) {
Expand All @@ -334,7 +334,7 @@ private module Propagation {
or
isSymbolExpr(e.(ConstantReadAccess).getValue(), s)
or
forex(ExprCfgNode n | n = e.getAControlFlowNode() | isSymbol(n, s))
isSymbol(e.getControlFlowNode(), s)
}

predicate isRegExp(ExprCfgNode e, string s, string flags) {
Expand All @@ -359,7 +359,7 @@ private module Propagation {
or
isRegExpExpr(e.(ConstantReadAccess).getValue(), s, flags)
or
forex(ExprCfgNode n | n = e.getAControlFlowNode() | isRegExp(n, s, flags))
isRegExp(e.getControlFlowNode(), s, flags)
}

predicate isBoolean(ExprCfgNode e, boolean b) {
Expand All @@ -381,7 +381,7 @@ private module Propagation {
or
isBooleanExpr(e.(ConstantReadAccess).getValue(), b)
or
forex(ExprCfgNode n | n = e.getAControlFlowNode() | isBoolean(n, b))
isBoolean(e.getControlFlowNode(), b)
}

predicate isNil(ExprCfgNode e) {
Expand All @@ -403,7 +403,7 @@ private module Propagation {
or
isNilExpr(e.(ConstantReadAccess).getValue())
or
forex(ExprCfgNode n | n = e.getAControlFlowNode() | isNil(n))
isNil(e.getControlFlowNode())
}
}

Expand Down Expand Up @@ -566,7 +566,7 @@ private predicate isArrayExpr(Expr e, ArrayLiteralCfgNode arr) {
// control flow paths.
// Note(hmac): I don't think this is necessary, as `getSource` will not return
// results if the source is a phi node.
forex(ExprCfgNode n | n = e.getAControlFlowNode() | isArrayConstant(n, arr))
isArrayConstant(e.getControlFlowNode(), arr)
or
// if `e` is an array, then `e.freeze` is also an array
e.(MethodCall).getMethodName() = "freeze" and
Expand Down
4 changes: 4 additions & 0 deletions ruby/ql/lib/codeql/ruby/ast/internal/Synthesis.qll
Original file line number Diff line number Diff line change
Expand Up @@ -833,6 +833,10 @@ private module AssignOperationDesugar {
)
)
}

final override predicate excludeFromControlFlowTree(AstNode n) {
n = any(ScopeResolutionAssignOperation sao).getLeftOperand()
}
}

/** An assignment operation where the left-hand side is a method call. */
Expand Down
Loading
Loading