From 91d5171d904f7bcfddbb768bff2febb4983b1b0d Mon Sep 17 00:00:00 2001 From: Simon Friis Vindum Date: Mon, 9 Sep 2024 17:43:03 +0200 Subject: [PATCH 1/4] Add base setup for control flow graph construction --- .../codeql/rust/controlflow/BasicBlocks.qll | 262 ++++++++++++++++++ .../rust/controlflow/ControlFlowGraph.qll | 36 +++ .../rust/controlflow/internal/Completion.qll | 84 ++++++ .../internal/ControlFlowGraphImpl.qll | 1 + .../internal/ControlFlowGraphImplSpecific.qll | 63 +++++ .../rust/controlflow/internal/PrintCFG.ql | 51 ++++ .../rust/controlflow/internal/Scope.qll | 36 +++ .../rust/controlflow/internal/Splitting.qll | 17 ++ .../controlflow/internal/SuccessorType.qll | 44 +++ .../library-tests/controlflow/CFG.expected | 0 rust/ql/test/library-tests/controlflow/CFG.ql | 13 + .../ql/test/library-tests/controlflow/test.rs | 7 + 12 files changed, 614 insertions(+) create mode 100644 rust/ql/lib/codeql/rust/controlflow/BasicBlocks.qll create mode 100644 rust/ql/lib/codeql/rust/controlflow/ControlFlowGraph.qll create mode 100644 rust/ql/lib/codeql/rust/controlflow/internal/Completion.qll create mode 100644 rust/ql/lib/codeql/rust/controlflow/internal/ControlFlowGraphImpl.qll create mode 100644 rust/ql/lib/codeql/rust/controlflow/internal/ControlFlowGraphImplSpecific.qll create mode 100644 rust/ql/lib/codeql/rust/controlflow/internal/PrintCFG.ql create mode 100644 rust/ql/lib/codeql/rust/controlflow/internal/Scope.qll create mode 100644 rust/ql/lib/codeql/rust/controlflow/internal/Splitting.qll create mode 100644 rust/ql/lib/codeql/rust/controlflow/internal/SuccessorType.qll create mode 100644 rust/ql/test/library-tests/controlflow/CFG.expected create mode 100644 rust/ql/test/library-tests/controlflow/CFG.ql create mode 100644 rust/ql/test/library-tests/controlflow/test.rs diff --git a/rust/ql/lib/codeql/rust/controlflow/BasicBlocks.qll b/rust/ql/lib/codeql/rust/controlflow/BasicBlocks.qll new file mode 100644 index 000000000000..2f3703f387cb --- /dev/null +++ b/rust/ql/lib/codeql/rust/controlflow/BasicBlocks.qll @@ -0,0 +1,262 @@ +private import rust +private import ControlFlowGraph +private import internal.SuccessorType +private import SuccessorTypes +private import internal.ControlFlowGraphImpl as Impl +private import codeql.rust.generated.Raw +private import codeql.rust.generated.Synth + +/** + * A basic block, that is, a maximal straight-line sequence of control flow nodes + * without branches or joins. + */ +class BasicBlock extends TBasicBlockStart { + /** Gets the scope of this basic block. */ + CfgScope getScope() { result = this.getAPredecessor().getScope() } + + /** Gets an immediate successor of this basic block, if any. */ + BasicBlock getASuccessor() { result = this.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) + } + + /** Gets an immediate predecessor of this basic block, if any. */ + BasicBlock getAPredecessor() { result.getASuccessor() = this } + + /** Gets an immediate predecessor of this basic block of a given type, if any. */ + BasicBlock getAPredecessor(SuccessorType t) { result.getASuccessor(t) = this } + + /** Gets the control flow node at a specific (zero-indexed) position in this basic block. */ + CfgNode getNode(int pos) { bbIndex(this.getFirstNode(), result, pos) } + + /** Gets a control flow node in this basic block. */ + CfgNode getANode() { result = this.getNode(_) } + + /** Gets the first control flow node in this basic block. */ + CfgNode getFirstNode() { this = TBasicBlockStart(result) } + + /** Gets the last control flow node in this basic block. */ + CfgNode getLastNode() { result = this.getNode(this.length() - 1) } + + /** Gets the length of this basic block. */ + int length() { result = strictcount(this.getANode()) } + + predicate immediatelyDominates(BasicBlock bb) { bbIDominates(this, bb) } + + predicate strictlyDominates(BasicBlock bb) { bbIDominates+(this, bb) } + + predicate dominates(BasicBlock bb) { + bb = this or + this.strictlyDominates(bb) + } + + predicate inDominanceFrontier(BasicBlock df) { + this.dominatesPredecessor(df) and + not this.strictlyDominates(df) + } + + /** + * Holds if this basic block dominates a predecessor of `df`. + */ + private predicate dominatesPredecessor(BasicBlock df) { this.dominates(df.getAPredecessor()) } + + BasicBlock getImmediateDominator() { bbIDominates(result, this) } + + predicate strictlyPostDominates(BasicBlock bb) { bbIPostDominates+(this, bb) } + + predicate postDominates(BasicBlock bb) { + this.strictlyPostDominates(bb) or + this = bb + } + + /** Holds if this basic block is in a loop in the control flow graph. */ + predicate inLoop() { this.getASuccessor+() = this } + + /** Gets a textual representation of this basic block. */ + string toString() { result = this.getFirstNode().toString() } + + /** Gets the location of this basic block. */ + Location getLocation() { result = this.getFirstNode().getLocation() } +} + +cached +private module Cached { + /** Internal representation of basic blocks. */ + cached + newtype TBasicBlock = TBasicBlockStart(CfgNode cfn) { startsBB(cfn) } + + /** Holds if `cfn` starts a new basic block. */ + private predicate startsBB(CfgNode cfn) { + not exists(cfn.getAPredecessor()) and exists(cfn.getASuccessor()) + or + cfn.isJoin() + or + cfn.getAPredecessor().isBranch() + } + + /** + * Holds if `succ` is a control flow successor of `pred` within + * the same basic block. + */ + private predicate intraBBSucc(CfgNode pred, CfgNode succ) { + succ = pred.getASuccessor() and + not startsBB(succ) + } + + /** + * Holds if `cfn` is the `i`th node in basic block `bb`. + * + * In other words, `i` is the shortest distance from a node `bb` + * that starts a basic block to `cfn` along the `intraBBSucc` relation. + */ + cached + predicate bbIndex(CfgNode bbStart, CfgNode cfn, int i) = + shortestDistances(startsBB/1, intraBBSucc/2)(bbStart, cfn, i) + + /** + * Holds if the first node of basic block `succ` is a control flow + * successor of the last node of basic block `pred`. + */ + private predicate succBB(BasicBlock pred, BasicBlock succ) { succ = pred.getASuccessor() } + + /** Holds if `dom` is an immediate dominator of `bb`. */ + cached + predicate bbIDominates(BasicBlock dom, BasicBlock bb) = + idominance(entryBB/1, succBB/2)(_, dom, bb) + + /** Holds if `pred` is a basic block predecessor of `succ`. */ + private predicate predBB(BasicBlock succ, BasicBlock pred) { succBB(pred, succ) } + + /** Holds if `bb` is an exit basic block that represents normal exit. */ + private predicate normalExitBB(BasicBlock bb) { + bb.getANode().(Impl::AnnotatedExitNode).isNormal() + } + + /** Holds if `dom` is an immediate post-dominator of `bb`. */ + cached + predicate bbIPostDominates(BasicBlock dom, BasicBlock bb) = + idominance(normalExitBB/1, predBB/2)(_, dom, bb) + + /** + * Gets the `i`th predecessor of join block `jb`, with respect to some + * arbitrary order. + */ + cached + JoinBlockPredecessor getJoinBlockPredecessor(JoinBlock jb, int i) { + result = + rank[i + 1](JoinBlockPredecessor jbp | + jbp = jb.getAPredecessor() + | + jbp order by JoinBlockPredecessors::getId(jbp), JoinBlockPredecessors::getSplitString(jbp) + ) + } +} + +private import Cached + +/** Holds if `bb` is an entry basic block. */ +private predicate entryBB(BasicBlock bb) { bb.getFirstNode() instanceof Impl::EntryNode } + +/** + * An entry basic block, that is, a basic block whose first node is + * an entry node. + */ +class EntryBasicBlock extends BasicBlock { + EntryBasicBlock() { entryBB(this) } + + override CfgScope getScope() { + this.getFirstNode() = any(Impl::EntryNode node | node.getScope() = result) + } +} + +/** + * An annotated exit basic block, that is, a basic block whose last node is + * an annotated exit node. + */ +class AnnotatedExitBasicBlock extends BasicBlock { + private boolean normal; + + AnnotatedExitBasicBlock() { + exists(Impl::AnnotatedExitNode n | + n = this.getANode() and + if n.isNormal() then normal = true else normal = false + ) + } + + /** Holds if this block represent a normal exit. */ + final predicate isNormal() { normal = true } +} + +/** + * An exit basic block, that is, a basic block whose last node is + * an exit node. + */ +class ExitBasicBlock extends BasicBlock { + ExitBasicBlock() { this.getLastNode() instanceof Impl::ExitNode } +} + +private module JoinBlockPredecessors { + private predicate id(Raw::AstNode x, Raw::AstNode y) { x = y } + + private predicate idOfDbAstNode(Raw::AstNode x, int y) = equivalenceRelation(id/2)(x, y) + + // TODO: does not work if fresh ipa entities (`ipa: on:`) turn out to be first of the block + private predicate idOf(AstNode x, int y) { idOfDbAstNode(Synth::convertAstNodeToRaw(x), y) } + + int getId(JoinBlockPredecessor jbp) { + idOf(jbp.getFirstNode().(Impl::AstCfgNode).getAstNode(), result) + or + idOf(jbp.(EntryBasicBlock).getScope(), result) + } + + string getSplitString(JoinBlockPredecessor jbp) { + result = jbp.getFirstNode().(Impl::AstCfgNode).getSplitsString() + or + not exists(jbp.getFirstNode().(Impl::AstCfgNode).getSplitsString()) and + result = "" + } +} + +/** A basic block with more than one predecessor. */ +class JoinBlock extends BasicBlock { + JoinBlock() { this.getFirstNode().isJoin() } + + /** + * Gets the `i`th predecessor of this join block, with respect to some + * arbitrary order. + */ + JoinBlockPredecessor getJoinBlockPredecessor(int i) { result = getJoinBlockPredecessor(this, i) } +} + +/** A basic block that is an immediate predecessor of a join block. */ +class JoinBlockPredecessor extends BasicBlock { + JoinBlockPredecessor() { this.getASuccessor() instanceof JoinBlock } +} + +/** A basic block that terminates in a condition, splitting the subsequent control flow. */ +class ConditionBlock extends BasicBlock { + ConditionBlock() { this.getLastNode().isCondition() } + + /** + * Holds if basic block `succ` is immediately controlled by this basic + * block with conditional value `s`. That is, `succ` is an immediate + * successor of this block, and `succ` can only be reached from + * the callable entry point by going via the `s` edge out of this basic block. + */ + pragma[nomagic] + predicate immediatelyControls(BasicBlock succ, SuccessorType s) { + succ = this.getASuccessor(s) and + forall(BasicBlock pred | pred = succ.getAPredecessor() and pred != this | succ.dominates(pred)) + } + + /** + * Holds if basic block `controlled` is controlled by this basic block with + * conditional value `s`. That is, `controlled` can only be reached from + * the callable entry point by going via the `s` edge out of this basic block. + */ + predicate controls(BasicBlock controlled, BooleanSuccessor s) { + exists(BasicBlock succ | this.immediatelyControls(succ, s) | succ.dominates(controlled)) + } +} diff --git a/rust/ql/lib/codeql/rust/controlflow/ControlFlowGraph.qll b/rust/ql/lib/codeql/rust/controlflow/ControlFlowGraph.qll new file mode 100644 index 000000000000..dce404aff96d --- /dev/null +++ b/rust/ql/lib/codeql/rust/controlflow/ControlFlowGraph.qll @@ -0,0 +1,36 @@ +/** Provides classes representing the control flow graph. */ + +private import rust +private import internal.ControlFlowGraphImpl +private import internal.Completion +private import internal.SuccessorType +private import codeql.rust.controlflow.BasicBlocks +import internal.Scope + +/** + * A control flow node. + * + * A control flow node is a node in the control flow graph (CFG). There is a + * many-to-one relationship between CFG nodes and AST nodes. + * + * Only nodes that can be reached from an entry point are included in the CFG. + */ +class CfgNode extends Node { + /** Gets the file of this control flow node. */ + final File getFile() { result = this.getLocation().getFile() } + + /** Gets a successor node of a given type, if any. */ + final CfgNode getASuccessor(SuccessorType t) { result = super.getASuccessor(t) } + + /** Gets an immediate successor, if any. */ + final CfgNode getASuccessor() { result = this.getASuccessor(_) } + + /** Gets an immediate predecessor node of a given flow type, if any. */ + final CfgNode getAPredecessor(SuccessorType t) { result.getASuccessor(t) = this } + + /** Gets an immediate predecessor, if any. */ + final CfgNode getAPredecessor() { result = this.getAPredecessor(_) } + + /** Gets the basic block that this control flow node belongs to. */ + BasicBlock getBasicBlock() { result.getANode() = this } +} diff --git a/rust/ql/lib/codeql/rust/controlflow/internal/Completion.qll b/rust/ql/lib/codeql/rust/controlflow/internal/Completion.qll new file mode 100644 index 000000000000..b00a4047c1fb --- /dev/null +++ b/rust/ql/lib/codeql/rust/controlflow/internal/Completion.qll @@ -0,0 +1,84 @@ +private import rust +private import codeql.rust.controlflow.ControlFlowGraph +private import SuccessorType +private import SuccessorTypes + +private newtype TCompletion = + TSimpleCompletion() or + TBooleanCompletion(boolean b) { b in [false, true] } or + TReturnCompletion() + +/** A completion of a statement or an expression. */ +abstract class Completion extends TCompletion { + /** Gets a textual representation of this completion. */ + abstract string toString(); + + predicate isValidForSpecific(AstNode e) { none() } + + predicate isValidFor(AstNode e) { this.isValidForSpecific(e) } + + /** Gets a successor type that matches this completion. */ + abstract SuccessorType getAMatchingSuccessorType(); +} + +/** + * A completion that represents normal evaluation of a statement or an + * expression. + */ +abstract class NormalCompletion extends Completion { } + +/** A simple (normal) completion. */ +class SimpleCompletion extends NormalCompletion, TSimpleCompletion { + override NormalSuccessor getAMatchingSuccessorType() { any() } + + // `SimpleCompletion` is the "default" completion type, thus it is valid for + // any node where there isn't another more specific completion type. + override predicate isValidFor(AstNode e) { not any(Completion c).isValidForSpecific(e) } + + override string toString() { result = "simple" } +} + +/** + * A completion that represents evaluation of an expression, whose value + * determines the successor. + */ +abstract class ConditionalCompletion extends NormalCompletion { + boolean value; + + bindingset[value] + ConditionalCompletion() { any() } + + /** Gets the Boolean value of this conditional completion. */ + final boolean getValue() { result = value } + + /** Gets the dual completion. */ + abstract ConditionalCompletion getDual(); +} + +/** + * A completion that represents evaluation of an expression + * with a Boolean value. + */ +class BooleanCompletion extends ConditionalCompletion, TBooleanCompletion { + BooleanCompletion() { this = TBooleanCompletion(value) } + + override predicate isValidForSpecific(AstNode e) { e = any(If c).getCondition() } + + /** Gets the dual Boolean completion. */ + override BooleanCompletion getDual() { result = TBooleanCompletion(value.booleanNot()) } + + override BooleanSuccessor getAMatchingSuccessorType() { result.getValue() = value } + + override string toString() { result = "boolean(" + value + ")" } +} + +/** + * A completion that represents a return. + */ +class ReturnCompletion extends TReturnCompletion, Completion { + override ReturnSuccessor getAMatchingSuccessorType() { any() } + + override predicate isValidForSpecific(AstNode e) { e instanceof Return } + + override string toString() { result = "return" } +} diff --git a/rust/ql/lib/codeql/rust/controlflow/internal/ControlFlowGraphImpl.qll b/rust/ql/lib/codeql/rust/controlflow/internal/ControlFlowGraphImpl.qll new file mode 100644 index 000000000000..386dd92d498b --- /dev/null +++ b/rust/ql/lib/codeql/rust/controlflow/internal/ControlFlowGraphImpl.qll @@ -0,0 +1 @@ +import ControlFlowGraphImplSpecific::CfgImpl diff --git a/rust/ql/lib/codeql/rust/controlflow/internal/ControlFlowGraphImplSpecific.qll b/rust/ql/lib/codeql/rust/controlflow/internal/ControlFlowGraphImplSpecific.qll new file mode 100644 index 000000000000..854bbcc7d73f --- /dev/null +++ b/rust/ql/lib/codeql/rust/controlflow/internal/ControlFlowGraphImplSpecific.qll @@ -0,0 +1,63 @@ +import codeql.controlflow.Cfg +import rust as Rust +private import SuccessorType as ST +private import Scope as Scope + +module CfgInput implements InputSig { + private import Completion as C + private import Splitting as S + + class AstNode = Rust::AstNode; + + class Completion = C::Completion; + + predicate completionIsNormal(Completion c) { c instanceof C::NormalCompletion } + + predicate completionIsSimple(Completion c) { c instanceof C::SimpleCompletion } + + predicate completionIsValidFor(Completion c, AstNode e) { c.isValidFor(e) } + + /** An AST node with an associated control-flow graph. */ + class CfgScope = Scope::CfgScope; + + CfgScope getCfgScope(AstNode n) { result = Scope::scopeOfAst(n) } + + class SplitKindBase = S::TSplitKind; + + class Split = S::Split; + + class SuccessorType = ST::SuccessorType; + + /** Gets a successor type that matches completion `c`. */ + SuccessorType getAMatchingSuccessorType(Completion c) { result = c.getAMatchingSuccessorType() } + + /** + * Hold if `c` represents simple (normal) evaluation of a statement or an expression. + */ + predicate successorTypeIsSimple(SuccessorType t) { + t instanceof ST::SuccessorTypes::NormalSuccessor + } + + /** Holds if `t` is an abnormal exit type out of a CFG scope. */ + predicate isAbnormalExitType(SuccessorType t) { none() } + + /** Hold if `t` represents a conditional successor type. */ + predicate successorTypeIsCondition(SuccessorType t) { + t instanceof ST::SuccessorTypes::BooleanSuccessor + } + + /** Gets the maximum number of splits allowed for a given node. */ + int maxSplits() { result = 0 } + + /** Holds if `first` is first executed when entering `scope`. */ + predicate scopeFirst(CfgScope scope, AstNode first) { + scope.(CfgImpl::ControlFlowTree).first(first) + } + + /** Holds if `scope` is exited when `last` finishes with completion `c`. */ + predicate scopeLast(CfgScope scope, AstNode last, Completion c) { + scope.(CfgImpl::ControlFlowTree).last(last, c) + } +} + +module CfgImpl = Make; diff --git a/rust/ql/lib/codeql/rust/controlflow/internal/PrintCFG.ql b/rust/ql/lib/codeql/rust/controlflow/internal/PrintCFG.ql new file mode 100644 index 000000000000..146126453883 --- /dev/null +++ b/rust/ql/lib/codeql/rust/controlflow/internal/PrintCFG.ql @@ -0,0 +1,51 @@ +/** + * @name Print CFG + * @description Produces a representation of a file's Control Flow Graph. + * This query is used by the VS Code extension. + * @id rust/print-cfg + * @kind graph + * @tags ide-contextual-queries/print-cfg + */ + +private import codeql.rust.elements.File +private import codeql.rust.controlflow.ControlFlowGraph +private import codeql.rust.controlflow.internal.ControlFlowGraphImpl as Impl +private import codeql.rust.controlflow.internal.ControlFlowGraphImplSpecific + +/** + * Gets the source file to generate a CFG from. + */ +external string selectedSourceFile(); + +private predicate selectedSourceFileAlias = selectedSourceFile/0; + +/** + * Gets the source line to generate a CFG from. + */ +external int selectedSourceLine(); + +private predicate selectedSourceLineAlias = selectedSourceLine/0; + +/** + * Gets the source column to generate a CFG from. + */ +external int selectedSourceColumn(); + +private predicate selectedSourceColumnAlias = selectedSourceColumn/0; + +module ViewCfgQueryInput implements Impl::ViewCfgQueryInputSig { + predicate selectedSourceFile = selectedSourceFileAlias/0; + + predicate selectedSourceLine = selectedSourceLineAlias/0; + + predicate selectedSourceColumn = selectedSourceColumnAlias/0; + + predicate cfgScopeSpan( + CfgInput::CfgScope scope, File file, int startLine, int startColumn, int endLine, int endColumn + ) { + file = scope.getFile() and + scope.getLocation().hasLocationInfo(_, startLine, startColumn, endLine, endColumn) + } +} + +import Impl::ViewCfgQuery diff --git a/rust/ql/lib/codeql/rust/controlflow/internal/Scope.qll b/rust/ql/lib/codeql/rust/controlflow/internal/Scope.qll new file mode 100644 index 000000000000..99d9cad978f9 --- /dev/null +++ b/rust/ql/lib/codeql/rust/controlflow/internal/Scope.qll @@ -0,0 +1,36 @@ +private import rust +private import Completion +private import codeql.rust.generated.ParentChild + +abstract class CfgScope extends AstNode { } + +class FunctionScope extends CfgScope, Function { } + +cached +private module Cached { + /** + * Gets the immediate parent of a non-`AstNode` element `e`. + * + * We restrict `e` to be a non-`AstNode` to skip past non-`AstNode` in + * the transitive closure computation in `getParentOfAst`. This is + * necessary because the parent of an `AstNode` is not necessarily an `AstNode`. + */ + private Element getParentOfAstStep(Element e) { + not e instanceof AstNode and + result = getImmediateParent(e) + } + + /** Gets the nearest enclosing parent of `ast` that is an `AstNode`. */ + cached + AstNode getParentOfAst(AstNode ast) { result = getParentOfAstStep*(getImmediateParent(ast)) } +} + +/** Gets the enclosing scope of a node */ +cached +AstNode scopeOfAst(AstNode n) { + exists(AstNode p | p = getParentOfAst(n) | + if p instanceof CfgScope then p = result else result = scopeOfAst(p) + ) +} + +import Cached diff --git a/rust/ql/lib/codeql/rust/controlflow/internal/Splitting.qll b/rust/ql/lib/codeql/rust/controlflow/internal/Splitting.qll new file mode 100644 index 000000000000..b4f0a060be6d --- /dev/null +++ b/rust/ql/lib/codeql/rust/controlflow/internal/Splitting.qll @@ -0,0 +1,17 @@ +cached +private module Cached { + // Not using CFG splitting, so the following are just placeholder types. + cached + newtype TSplitKind = TSplitKindUnit() + + cached + newtype TSplit = TSplitUnit() +} + +import Cached + +/** A split for a control flow node. */ +class Split extends TSplit { + /** Gets a textual representation of this split. */ + string toString() { none() } +} diff --git a/rust/ql/lib/codeql/rust/controlflow/internal/SuccessorType.qll b/rust/ql/lib/codeql/rust/controlflow/internal/SuccessorType.qll new file mode 100644 index 000000000000..255ac8c79655 --- /dev/null +++ b/rust/ql/lib/codeql/rust/controlflow/internal/SuccessorType.qll @@ -0,0 +1,44 @@ +cached +newtype TSuccessorType = + TSuccessorSuccessor() or + TBooleanSuccessor(boolean b) { b in [false, true] } or + TReturnSuccessor() + +/** The type of a control flow successor. */ +class SuccessorType extends 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, TSuccessorSuccessor { + final override string toString() { result = "successor" } + } + + /** A conditional control flow successor. */ + abstract class ConditionalSuccessor extends SuccessorType { + boolean value; + + bindingset[value] + ConditionalSuccessor() { any() } + + /** Gets the Boolean value of this successor. */ + final boolean getValue() { result = value } + + override string toString() { result = this.getValue().toString() } + } + + /** A Boolean control flow successor. */ + class BooleanSuccessor extends ConditionalSuccessor, TBooleanSuccessor { + BooleanSuccessor() { this = TBooleanSuccessor(value) } + } + + /** + * A `return` control flow successor. + */ + class ReturnSuccessor extends SuccessorType, TReturnSuccessor { + final override string toString() { result = "return" } + } +} diff --git a/rust/ql/test/library-tests/controlflow/CFG.expected b/rust/ql/test/library-tests/controlflow/CFG.expected new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/rust/ql/test/library-tests/controlflow/CFG.ql b/rust/ql/test/library-tests/controlflow/CFG.ql new file mode 100644 index 000000000000..b46fa460b845 --- /dev/null +++ b/rust/ql/test/library-tests/controlflow/CFG.ql @@ -0,0 +1,13 @@ +/** + * @kind graph + * @id rust/controlflow/cfg + */ + +import rust +import codeql.rust.controlflow.ControlFlowGraph + +class MyRelevantNode extends CfgNode { + string getOrderDisambiguation() { result = "" } +} + +import codeql.rust.controlflow.internal.ControlFlowGraphImpl::TestOutput diff --git a/rust/ql/test/library-tests/controlflow/test.rs b/rust/ql/test/library-tests/controlflow/test.rs new file mode 100644 index 000000000000..ecb0ecc98b4b --- /dev/null +++ b/rust/ql/test/library-tests/controlflow/test.rs @@ -0,0 +1,7 @@ +fn main() { + if "foo" == "bar" { + println!("foobar"); + } else { + println!("baz") + } +} From 6d972bea2c84c07c31902caabb8408bfe4542245 Mon Sep 17 00:00:00 2001 From: Simon Friis Vindum Date: Tue, 10 Sep 2024 10:03:01 +0200 Subject: [PATCH 2/4] Rust: Add a few control flow tree classes --- .../rust/controlflow/internal/Completion.qll | 14 +++++- .../internal/ControlFlowGraphImpl.qll | 46 +++++++++++++++++++ .../internal/ControlFlowGraphImplSpecific.qll | 6 +-- .../controlflow/internal/SuccessorType.qll | 4 +- 4 files changed, 64 insertions(+), 6 deletions(-) diff --git a/rust/ql/lib/codeql/rust/controlflow/internal/Completion.qll b/rust/ql/lib/codeql/rust/controlflow/internal/Completion.qll index b00a4047c1fb..ecf688f03931 100644 --- a/rust/ql/lib/codeql/rust/controlflow/internal/Completion.qll +++ b/rust/ql/lib/codeql/rust/controlflow/internal/Completion.qll @@ -1,11 +1,12 @@ -private import rust +private import codeql.util.Boolean private import codeql.rust.controlflow.ControlFlowGraph +private import rust private import SuccessorType private import SuccessorTypes private newtype TCompletion = TSimpleCompletion() or - TBooleanCompletion(boolean b) { b in [false, true] } or + TBooleanCompletion(Boolean b) or TReturnCompletion() /** A completion of a statement or an expression. */ @@ -82,3 +83,12 @@ class ReturnCompletion extends TReturnCompletion, Completion { override string toString() { result = "return" } } + +/** Hold if `c` represents normal evaluation of a statement or an expression. */ +predicate completionIsNormal(Completion c) { c instanceof NormalCompletion } + +/** Hold if `c` represents simple and normal evaluation of a statement or an expression. */ +predicate completionIsSimple(Completion c) { c instanceof SimpleCompletion } + +/** Holds if `c` is a valid completion for `n`. */ +predicate completionIsValidFor(Completion c, AstNode n) { c.isValidFor(n) } diff --git a/rust/ql/lib/codeql/rust/controlflow/internal/ControlFlowGraphImpl.qll b/rust/ql/lib/codeql/rust/controlflow/internal/ControlFlowGraphImpl.qll index 386dd92d498b..5586368a7d61 100644 --- a/rust/ql/lib/codeql/rust/controlflow/internal/ControlFlowGraphImpl.qll +++ b/rust/ql/lib/codeql/rust/controlflow/internal/ControlFlowGraphImpl.qll @@ -1 +1,47 @@ +private import rust import ControlFlowGraphImplSpecific::CfgImpl +import Completion + +class CallTree extends StandardPostOrderTree instanceof Call { + override ControlFlowTree getChildNode(int i) { result = super.getArg(i) } +} + +class BinaryOpTree extends StandardPostOrderTree instanceof BinaryOp { + override ControlFlowTree getChildNode(int i) { + i = 0 and result = super.getLhs() + or + i = 1 and result = super.getRhs() + } +} + +class IfTree extends PostOrderTree instanceof If { + override predicate first(AstNode node) { first(super.getCondition(), node) } + + override predicate propagatesAbnormal(AstNode child) { none() } + + override predicate succ(AstNode pred, AstNode succ, Completion c) { + // Edges from the condition to each branch + last(super.getCondition(), pred, c) and + ( + first(super.getThen(), succ) and c.(BooleanCompletion).getValue() = true + or + first(super.getElse(), succ) and c.(BooleanCompletion).getValue() = false + ) + or + // An edge from the then branch to the last node + last(super.getThen(), pred, c) and + succ = this and + completionIsSimple(c) + or + // An edge from the else branch to the last node + last(super.getElse(), pred, c) and + succ = this and + completionIsSimple(c) + } +} + +class LetTree extends StandardPostOrderTree instanceof Let { + override ControlFlowTree getChildNode(int i) { i = 0 and result = super.getExpr() } +} + +class LiteralTree extends LeafTree instanceof Literal { } diff --git a/rust/ql/lib/codeql/rust/controlflow/internal/ControlFlowGraphImplSpecific.qll b/rust/ql/lib/codeql/rust/controlflow/internal/ControlFlowGraphImplSpecific.qll index 854bbcc7d73f..9a6f615dab42 100644 --- a/rust/ql/lib/codeql/rust/controlflow/internal/ControlFlowGraphImplSpecific.qll +++ b/rust/ql/lib/codeql/rust/controlflow/internal/ControlFlowGraphImplSpecific.qll @@ -11,11 +11,11 @@ module CfgInput implements InputSig { class Completion = C::Completion; - predicate completionIsNormal(Completion c) { c instanceof C::NormalCompletion } + predicate completionIsNormal = C::completionIsNormal/1; - predicate completionIsSimple(Completion c) { c instanceof C::SimpleCompletion } + predicate completionIsSimple = C::completionIsSimple/1; - predicate completionIsValidFor(Completion c, AstNode e) { c.isValidFor(e) } + predicate completionIsValidFor = C::completionIsValidFor/2; /** An AST node with an associated control-flow graph. */ class CfgScope = Scope::CfgScope; diff --git a/rust/ql/lib/codeql/rust/controlflow/internal/SuccessorType.qll b/rust/ql/lib/codeql/rust/controlflow/internal/SuccessorType.qll index 255ac8c79655..6a58482348a6 100644 --- a/rust/ql/lib/codeql/rust/controlflow/internal/SuccessorType.qll +++ b/rust/ql/lib/codeql/rust/controlflow/internal/SuccessorType.qll @@ -1,7 +1,9 @@ +private import codeql.util.Boolean + cached newtype TSuccessorType = TSuccessorSuccessor() or - TBooleanSuccessor(boolean b) { b in [false, true] } or + TBooleanSuccessor(Boolean b) or TReturnSuccessor() /** The type of a control flow successor. */ From 809d04052836ebc601c99d8731b5d7fee7da93d4 Mon Sep 17 00:00:00 2001 From: Simon Friis Vindum Date: Wed, 11 Sep 2024 09:37:39 +0200 Subject: [PATCH 3/4] Make more classes private and final --- .../codeql/rust/controlflow/BasicBlocks.qll | 17 ++-- .../rust/controlflow/ControlFlowGraph.qll | 16 ++-- .../rust/controlflow/internal/Completion.qll | 5 +- .../internal/ControlFlowGraphImpl.qll | 96 +++++++++++++++++-- .../internal/ControlFlowGraphImplSpecific.qll | 63 ------------ .../internal/{PrintCFG.ql => PrintCfg.ql} | 2 +- .../rust/controlflow/internal/Scope.qll | 32 +++---- .../controlflow/internal/SuccessorType.qll | 65 +++++++------ .../{CFG.expected => Cfg.expected} | 0 .../controlflow/{CFG.ql => Cfg.ql} | 1 - 10 files changed, 154 insertions(+), 143 deletions(-) delete mode 100644 rust/ql/lib/codeql/rust/controlflow/internal/ControlFlowGraphImplSpecific.qll rename rust/ql/lib/codeql/rust/controlflow/internal/{PrintCFG.ql => PrintCfg.ql} (94%) rename rust/ql/test/library-tests/controlflow/{CFG.expected => Cfg.expected} (100%) rename rust/ql/test/library-tests/controlflow/{CFG.ql => Cfg.ql} (94%) diff --git a/rust/ql/lib/codeql/rust/controlflow/BasicBlocks.qll b/rust/ql/lib/codeql/rust/controlflow/BasicBlocks.qll index 2f3703f387cb..d5af73f2ba0f 100644 --- a/rust/ql/lib/codeql/rust/controlflow/BasicBlocks.qll +++ b/rust/ql/lib/codeql/rust/controlflow/BasicBlocks.qll @@ -1,16 +1,17 @@ private import rust private import ControlFlowGraph private import internal.SuccessorType -private import SuccessorTypes private import internal.ControlFlowGraphImpl as Impl private import codeql.rust.generated.Raw private import codeql.rust.generated.Synth +final class BasicBlock = BasicBlockImpl; + /** * A basic block, that is, a maximal straight-line sequence of control flow nodes * without branches or joins. */ -class BasicBlock extends TBasicBlockStart { +private class BasicBlockImpl extends TBasicBlockStart { /** Gets the scope of this basic block. */ CfgScope getScope() { result = this.getAPredecessor().getScope() } @@ -163,7 +164,7 @@ private predicate entryBB(BasicBlock bb) { bb.getFirstNode() instanceof Impl::En * An entry basic block, that is, a basic block whose first node is * an entry node. */ -class EntryBasicBlock extends BasicBlock { +class EntryBasicBlock extends BasicBlockImpl { EntryBasicBlock() { entryBB(this) } override CfgScope getScope() { @@ -175,7 +176,7 @@ class EntryBasicBlock extends BasicBlock { * An annotated exit basic block, that is, a basic block whose last node is * an annotated exit node. */ -class AnnotatedExitBasicBlock extends BasicBlock { +class AnnotatedExitBasicBlock extends BasicBlockImpl { private boolean normal; AnnotatedExitBasicBlock() { @@ -193,7 +194,7 @@ class AnnotatedExitBasicBlock extends BasicBlock { * An exit basic block, that is, a basic block whose last node is * an exit node. */ -class ExitBasicBlock extends BasicBlock { +class ExitBasicBlock extends BasicBlockImpl { ExitBasicBlock() { this.getLastNode() instanceof Impl::ExitNode } } @@ -220,7 +221,7 @@ private module JoinBlockPredecessors { } /** A basic block with more than one predecessor. */ -class JoinBlock extends BasicBlock { +class JoinBlock extends BasicBlockImpl { JoinBlock() { this.getFirstNode().isJoin() } /** @@ -231,12 +232,12 @@ class JoinBlock extends BasicBlock { } /** A basic block that is an immediate predecessor of a join block. */ -class JoinBlockPredecessor extends BasicBlock { +class JoinBlockPredecessor extends BasicBlockImpl { JoinBlockPredecessor() { this.getASuccessor() instanceof JoinBlock } } /** A basic block that terminates in a condition, splitting the subsequent control flow. */ -class ConditionBlock extends BasicBlock { +class ConditionBlock extends BasicBlockImpl { ConditionBlock() { this.getLastNode().isCondition() } /** diff --git a/rust/ql/lib/codeql/rust/controlflow/ControlFlowGraph.qll b/rust/ql/lib/codeql/rust/controlflow/ControlFlowGraph.qll index dce404aff96d..1934b5ea633a 100644 --- a/rust/ql/lib/codeql/rust/controlflow/ControlFlowGraph.qll +++ b/rust/ql/lib/codeql/rust/controlflow/ControlFlowGraph.qll @@ -5,7 +5,9 @@ private import internal.ControlFlowGraphImpl private import internal.Completion private import internal.SuccessorType private import codeql.rust.controlflow.BasicBlocks -import internal.Scope +private import internal.Scope as Scope + +final class CfgScope = Scope::CfgScope; /** * A control flow node. @@ -15,21 +17,21 @@ import internal.Scope * * Only nodes that can be reached from an entry point are included in the CFG. */ -class CfgNode extends Node { +final class CfgNode extends Node { /** Gets the file of this control flow node. */ - final File getFile() { result = this.getLocation().getFile() } + File getFile() { result = this.getLocation().getFile() } /** Gets a successor node of a given type, if any. */ - final CfgNode getASuccessor(SuccessorType t) { result = super.getASuccessor(t) } + CfgNode getASuccessor(SuccessorType t) { result = super.getASuccessor(t) } /** Gets an immediate successor, if any. */ - final CfgNode getASuccessor() { result = this.getASuccessor(_) } + CfgNode getASuccessor() { result = this.getASuccessor(_) } /** Gets an immediate predecessor node of a given flow type, if any. */ - final CfgNode getAPredecessor(SuccessorType t) { result.getASuccessor(t) = this } + CfgNode getAPredecessor(SuccessorType t) { result.getASuccessor(t) = this } /** Gets an immediate predecessor, if any. */ - final CfgNode getAPredecessor() { result = this.getAPredecessor(_) } + CfgNode getAPredecessor() { result = this.getAPredecessor(_) } /** Gets the basic block that this control flow node belongs to. */ BasicBlock getBasicBlock() { result.getANode() = this } diff --git a/rust/ql/lib/codeql/rust/controlflow/internal/Completion.qll b/rust/ql/lib/codeql/rust/controlflow/internal/Completion.qll index ecf688f03931..088866f7ea92 100644 --- a/rust/ql/lib/codeql/rust/controlflow/internal/Completion.qll +++ b/rust/ql/lib/codeql/rust/controlflow/internal/Completion.qll @@ -2,7 +2,6 @@ private import codeql.util.Boolean private import codeql.rust.controlflow.ControlFlowGraph private import rust private import SuccessorType -private import SuccessorTypes private newtype TCompletion = TSimpleCompletion() or @@ -63,7 +62,7 @@ abstract class ConditionalCompletion extends NormalCompletion { class BooleanCompletion extends ConditionalCompletion, TBooleanCompletion { BooleanCompletion() { this = TBooleanCompletion(value) } - override predicate isValidForSpecific(AstNode e) { e = any(If c).getCondition() } + override predicate isValidForSpecific(AstNode e) { e = any(IfExpr c).getCondition() } /** Gets the dual Boolean completion. */ override BooleanCompletion getDual() { result = TBooleanCompletion(value.booleanNot()) } @@ -79,7 +78,7 @@ class BooleanCompletion extends ConditionalCompletion, TBooleanCompletion { class ReturnCompletion extends TReturnCompletion, Completion { override ReturnSuccessor getAMatchingSuccessorType() { any() } - override predicate isValidForSpecific(AstNode e) { e instanceof Return } + override predicate isValidForSpecific(AstNode e) { e instanceof ReturnExpr } override string toString() { result = "return" } } diff --git a/rust/ql/lib/codeql/rust/controlflow/internal/ControlFlowGraphImpl.qll b/rust/ql/lib/codeql/rust/controlflow/internal/ControlFlowGraphImpl.qll index 5586368a7d61..7e866a0d36a6 100644 --- a/rust/ql/lib/codeql/rust/controlflow/internal/ControlFlowGraphImpl.qll +++ b/rust/ql/lib/codeql/rust/controlflow/internal/ControlFlowGraphImpl.qll @@ -1,12 +1,88 @@ private import rust -import ControlFlowGraphImplSpecific::CfgImpl +import codeql.controlflow.Cfg import Completion +import codeql.controlflow.Cfg +private import SuccessorType as ST +private import Scope as Scope -class CallTree extends StandardPostOrderTree instanceof Call { +module CfgInput implements InputSig { + private import rust as Rust + private import Completion as C + private import Splitting as S + + class AstNode = Rust::AstNode; + + class Completion = C::Completion; + + predicate completionIsNormal = C::completionIsNormal/1; + + predicate completionIsSimple = C::completionIsSimple/1; + + predicate completionIsValidFor = C::completionIsValidFor/2; + + /** An AST node with an associated control-flow graph. */ + class CfgScope = Scope::CfgScope; + + CfgScope getCfgScope(AstNode n) { result = Scope::scopeOfAst(n) } + + class SplitKindBase = S::TSplitKind; + + class Split = S::Split; + + class SuccessorType = ST::SuccessorType; + + /** Gets a successor type that matches completion `c`. */ + SuccessorType getAMatchingSuccessorType(Completion c) { result = c.getAMatchingSuccessorType() } + + /** + * Hold if `c` represents simple (normal) evaluation of a statement or an expression. + */ + predicate successorTypeIsSimple(SuccessorType t) { t instanceof ST::NormalSuccessor } + + /** Holds if `t` is an abnormal exit type out of a CFG scope. */ + predicate isAbnormalExitType(SuccessorType t) { none() } + + /** Hold if `t` represents a conditional successor type. */ + predicate successorTypeIsCondition(SuccessorType t) { t instanceof ST::BooleanSuccessor } + + /** Gets the maximum number of splits allowed for a given node. */ + int maxSplits() { result = 0 } + + /** Holds if `first` is first executed when entering `scope`. */ + predicate scopeFirst(CfgScope scope, AstNode first) { + scope.(CfgImpl::ControlFlowTree).first(first) + } + + /** Holds if `scope` is exited when `last` finishes with completion `c`. */ + predicate scopeLast(CfgScope scope, AstNode last, Completion c) { + scope.(CfgImpl::ControlFlowTree).last(last, c) + } +} + +module CfgImpl = Make; + +import CfgImpl + +class FunctionTree extends LeafTree instanceof Function { } + +class BlockExprTree extends StandardPostOrderTree instanceof BlockExpr { + override ControlFlowTree getChildNode(int i) { + result = super.getStatement(i) + or + exists(int last | + last + 1 = i and + exists(super.getStatement(last)) and + not exists(super.getStatement(last + 1)) and + result = super.getTail() + ) + } +} + +class CallExprTree extends StandardPostOrderTree instanceof CallExpr { override ControlFlowTree getChildNode(int i) { result = super.getArg(i) } } -class BinaryOpTree extends StandardPostOrderTree instanceof BinaryOp { +class BinaryOpExprTree extends StandardPostOrderTree instanceof BinaryOpExpr { override ControlFlowTree getChildNode(int i) { i = 0 and result = super.getLhs() or @@ -14,10 +90,12 @@ class BinaryOpTree extends StandardPostOrderTree instanceof BinaryOp { } } -class IfTree extends PostOrderTree instanceof If { +class IfExprTree extends PostOrderTree instanceof IfExpr { override predicate first(AstNode node) { first(super.getCondition(), node) } - override predicate propagatesAbnormal(AstNode child) { none() } + override predicate propagatesAbnormal(AstNode child) { + child = [super.getCondition(), super.getThen(), super.getElse()] + } override predicate succ(AstNode pred, AstNode succ, Completion c) { // Edges from the condition to each branch @@ -31,17 +109,17 @@ class IfTree extends PostOrderTree instanceof If { // An edge from the then branch to the last node last(super.getThen(), pred, c) and succ = this and - completionIsSimple(c) + completionIsNormal(c) or // An edge from the else branch to the last node last(super.getElse(), pred, c) and succ = this and - completionIsSimple(c) + completionIsNormal(c) } } -class LetTree extends StandardPostOrderTree instanceof Let { +class LetExprTree extends StandardPostOrderTree instanceof LetExpr { override ControlFlowTree getChildNode(int i) { i = 0 and result = super.getExpr() } } -class LiteralTree extends LeafTree instanceof Literal { } +class LiteralExprTree extends LeafTree instanceof LiteralExpr { } diff --git a/rust/ql/lib/codeql/rust/controlflow/internal/ControlFlowGraphImplSpecific.qll b/rust/ql/lib/codeql/rust/controlflow/internal/ControlFlowGraphImplSpecific.qll deleted file mode 100644 index 9a6f615dab42..000000000000 --- a/rust/ql/lib/codeql/rust/controlflow/internal/ControlFlowGraphImplSpecific.qll +++ /dev/null @@ -1,63 +0,0 @@ -import codeql.controlflow.Cfg -import rust as Rust -private import SuccessorType as ST -private import Scope as Scope - -module CfgInput implements InputSig { - private import Completion as C - private import Splitting as S - - class AstNode = Rust::AstNode; - - class Completion = C::Completion; - - predicate completionIsNormal = C::completionIsNormal/1; - - predicate completionIsSimple = C::completionIsSimple/1; - - predicate completionIsValidFor = C::completionIsValidFor/2; - - /** An AST node with an associated control-flow graph. */ - class CfgScope = Scope::CfgScope; - - CfgScope getCfgScope(AstNode n) { result = Scope::scopeOfAst(n) } - - class SplitKindBase = S::TSplitKind; - - class Split = S::Split; - - class SuccessorType = ST::SuccessorType; - - /** Gets a successor type that matches completion `c`. */ - SuccessorType getAMatchingSuccessorType(Completion c) { result = c.getAMatchingSuccessorType() } - - /** - * Hold if `c` represents simple (normal) evaluation of a statement or an expression. - */ - predicate successorTypeIsSimple(SuccessorType t) { - t instanceof ST::SuccessorTypes::NormalSuccessor - } - - /** Holds if `t` is an abnormal exit type out of a CFG scope. */ - predicate isAbnormalExitType(SuccessorType t) { none() } - - /** Hold if `t` represents a conditional successor type. */ - predicate successorTypeIsCondition(SuccessorType t) { - t instanceof ST::SuccessorTypes::BooleanSuccessor - } - - /** Gets the maximum number of splits allowed for a given node. */ - int maxSplits() { result = 0 } - - /** Holds if `first` is first executed when entering `scope`. */ - predicate scopeFirst(CfgScope scope, AstNode first) { - scope.(CfgImpl::ControlFlowTree).first(first) - } - - /** Holds if `scope` is exited when `last` finishes with completion `c`. */ - predicate scopeLast(CfgScope scope, AstNode last, Completion c) { - scope.(CfgImpl::ControlFlowTree).last(last, c) - } -} - -module CfgImpl = Make; diff --git a/rust/ql/lib/codeql/rust/controlflow/internal/PrintCFG.ql b/rust/ql/lib/codeql/rust/controlflow/internal/PrintCfg.ql similarity index 94% rename from rust/ql/lib/codeql/rust/controlflow/internal/PrintCFG.ql rename to rust/ql/lib/codeql/rust/controlflow/internal/PrintCfg.ql index 146126453883..a5b76afb7580 100644 --- a/rust/ql/lib/codeql/rust/controlflow/internal/PrintCFG.ql +++ b/rust/ql/lib/codeql/rust/controlflow/internal/PrintCfg.ql @@ -33,7 +33,7 @@ external int selectedSourceColumn(); private predicate selectedSourceColumnAlias = selectedSourceColumn/0; -module ViewCfgQueryInput implements Impl::ViewCfgQueryInputSig { +private module ViewCfgQueryInput implements Impl::ViewCfgQueryInputSig { predicate selectedSourceFile = selectedSourceFileAlias/0; predicate selectedSourceLine = selectedSourceLineAlias/0; diff --git a/rust/ql/lib/codeql/rust/controlflow/internal/Scope.qll b/rust/ql/lib/codeql/rust/controlflow/internal/Scope.qll index 99d9cad978f9..c0729f853f77 100644 --- a/rust/ql/lib/codeql/rust/controlflow/internal/Scope.qll +++ b/rust/ql/lib/codeql/rust/controlflow/internal/Scope.qll @@ -6,23 +6,21 @@ abstract class CfgScope extends AstNode { } class FunctionScope extends CfgScope, Function { } -cached -private module Cached { - /** - * Gets the immediate parent of a non-`AstNode` element `e`. - * - * We restrict `e` to be a non-`AstNode` to skip past non-`AstNode` in - * the transitive closure computation in `getParentOfAst`. This is - * necessary because the parent of an `AstNode` is not necessarily an `AstNode`. - */ - private Element getParentOfAstStep(Element e) { - not e instanceof AstNode and - result = getImmediateParent(e) - } +/** + * Gets the immediate parent of a non-`AstNode` element `e`. + * + * We restrict `e` to be a non-`AstNode` to skip past non-`AstNode` in + * the transitive closure computation in `getParentOfAst`. This is + * necessary because the parent of an `AstNode` is not necessarily an `AstNode`. + */ +private Element getParentOfAstStep(Element e) { + not e instanceof AstNode and + result = getImmediateParent(e) +} - /** Gets the nearest enclosing parent of `ast` that is an `AstNode`. */ - cached - AstNode getParentOfAst(AstNode ast) { result = getParentOfAstStep*(getImmediateParent(ast)) } +/** Gets the nearest enclosing parent of `ast` that is an `AstNode`. */ +private AstNode getParentOfAst(AstNode ast) { + result = getParentOfAstStep*(getImmediateParent(ast)) } /** Gets the enclosing scope of a node */ @@ -32,5 +30,3 @@ AstNode scopeOfAst(AstNode n) { if p instanceof CfgScope then p = result else result = scopeOfAst(p) ) } - -import Cached diff --git a/rust/ql/lib/codeql/rust/controlflow/internal/SuccessorType.qll b/rust/ql/lib/codeql/rust/controlflow/internal/SuccessorType.qll index 6a58482348a6..5ea12fc8a5c5 100644 --- a/rust/ql/lib/codeql/rust/controlflow/internal/SuccessorType.qll +++ b/rust/ql/lib/codeql/rust/controlflow/internal/SuccessorType.qll @@ -7,40 +7,39 @@ newtype TSuccessorType = TReturnSuccessor() /** The type of a control flow successor. */ -class SuccessorType extends TSuccessorType { +abstract private class SuccessorTypeImpl extends TSuccessorType { /** Gets a textual representation of successor type. */ - string toString() { none() } + abstract string toString(); } -/** Provides different types of control flow successor types. */ -module SuccessorTypes { - /** A normal control flow successor. */ - class NormalSuccessor extends SuccessorType, TSuccessorSuccessor { - final override string toString() { result = "successor" } - } - - /** A conditional control flow successor. */ - abstract class ConditionalSuccessor extends SuccessorType { - boolean value; - - bindingset[value] - ConditionalSuccessor() { any() } - - /** Gets the Boolean value of this successor. */ - final boolean getValue() { result = value } - - override string toString() { result = this.getValue().toString() } - } - - /** A Boolean control flow successor. */ - class BooleanSuccessor extends ConditionalSuccessor, TBooleanSuccessor { - BooleanSuccessor() { this = TBooleanSuccessor(value) } - } - - /** - * A `return` control flow successor. - */ - class ReturnSuccessor extends SuccessorType, TReturnSuccessor { - final override string toString() { result = "return" } - } +final class SuccessorType = SuccessorTypeImpl; + +/** A normal control flow successor. */ +final class NormalSuccessor extends SuccessorTypeImpl, TSuccessorSuccessor { + final override string toString() { result = "successor" } +} + +/** A conditional control flow successor. */ +abstract private class ConditionalSuccessor extends SuccessorTypeImpl { + boolean value; + + bindingset[value] + ConditionalSuccessor() { any() } + + /** Gets the Boolean value of this successor. */ + final boolean getValue() { result = value } + + override string toString() { result = this.getValue().toString() } +} + +/** A Boolean control flow successor. */ +final class BooleanSuccessor extends ConditionalSuccessor, TBooleanSuccessor { + BooleanSuccessor() { this = TBooleanSuccessor(value) } +} + +/** + * A `return` control flow successor. + */ +final class ReturnSuccessor extends SuccessorTypeImpl, TReturnSuccessor { + final override string toString() { result = "return" } } diff --git a/rust/ql/test/library-tests/controlflow/CFG.expected b/rust/ql/test/library-tests/controlflow/Cfg.expected similarity index 100% rename from rust/ql/test/library-tests/controlflow/CFG.expected rename to rust/ql/test/library-tests/controlflow/Cfg.expected diff --git a/rust/ql/test/library-tests/controlflow/CFG.ql b/rust/ql/test/library-tests/controlflow/Cfg.ql similarity index 94% rename from rust/ql/test/library-tests/controlflow/CFG.ql rename to rust/ql/test/library-tests/controlflow/Cfg.ql index b46fa460b845..e23a3122b370 100644 --- a/rust/ql/test/library-tests/controlflow/CFG.ql +++ b/rust/ql/test/library-tests/controlflow/Cfg.ql @@ -1,5 +1,4 @@ /** - * @kind graph * @id rust/controlflow/cfg */ From 857edb791c680f438c79696b2d387998efb98727 Mon Sep 17 00:00:00 2001 From: Simon Friis Vindum Date: Wed, 11 Sep 2024 11:16:06 +0200 Subject: [PATCH 4/4] Rust: Fix control flow tree for function and block expression --- .../rust/controlflow/ControlFlowGraph.qll | 2 +- .../internal/ControlFlowGraphImpl.qll | 18 ++-- .../rust/controlflow/internal/PrintCfg.ql | 9 +- .../library-tests/controlflow/Cfg.expected | 92 +++++++++++++++++++ rust/ql/test/library-tests/controlflow/Cfg.ql | 3 + .../ql/test/library-tests/controlflow/test.rs | 15 ++- 6 files changed, 123 insertions(+), 16 deletions(-) diff --git a/rust/ql/lib/codeql/rust/controlflow/ControlFlowGraph.qll b/rust/ql/lib/codeql/rust/controlflow/ControlFlowGraph.qll index 1934b5ea633a..e8e6e5f98c9d 100644 --- a/rust/ql/lib/codeql/rust/controlflow/ControlFlowGraph.qll +++ b/rust/ql/lib/codeql/rust/controlflow/ControlFlowGraph.qll @@ -4,8 +4,8 @@ private import rust private import internal.ControlFlowGraphImpl private import internal.Completion private import internal.SuccessorType -private import codeql.rust.controlflow.BasicBlocks private import internal.Scope as Scope +private import BasicBlocks final class CfgScope = Scope::CfgScope; diff --git a/rust/ql/lib/codeql/rust/controlflow/internal/ControlFlowGraphImpl.qll b/rust/ql/lib/codeql/rust/controlflow/internal/ControlFlowGraphImpl.qll index 7e866a0d36a6..1370362bf0f0 100644 --- a/rust/ql/lib/codeql/rust/controlflow/internal/ControlFlowGraphImpl.qll +++ b/rust/ql/lib/codeql/rust/controlflow/internal/ControlFlowGraphImpl.qll @@ -63,18 +63,17 @@ module CfgImpl = Make; import CfgImpl -class FunctionTree extends LeafTree instanceof Function { } +class FunctionTree extends StandardPostOrderTree instanceof Function { + override ControlFlowTree getChildNode(int i) { i = 0 and result = super.getBody() } +} class BlockExprTree extends StandardPostOrderTree instanceof BlockExpr { override ControlFlowTree getChildNode(int i) { result = super.getStatement(i) or - exists(int last | - last + 1 = i and - exists(super.getStatement(last)) and - not exists(super.getStatement(last + 1)) and - result = super.getTail() - ) + not exists(super.getStatement(i)) and + (exists(super.getStatement(i - 1)) or i = 0) and + result = super.getTail() } } @@ -123,3 +122,8 @@ class LetExprTree extends StandardPostOrderTree instanceof LetExpr { } class LiteralExprTree extends LeafTree instanceof LiteralExpr { } + +class PathExprTree extends LeafTree instanceof PathExpr { } + +// A leaf tree for unimplemented nodes in the AST. +class UnimplementedTree extends LeafTree instanceof Unimplemented { } diff --git a/rust/ql/lib/codeql/rust/controlflow/internal/PrintCfg.ql b/rust/ql/lib/codeql/rust/controlflow/internal/PrintCfg.ql index a5b76afb7580..d4db94ca22ae 100644 --- a/rust/ql/lib/codeql/rust/controlflow/internal/PrintCfg.ql +++ b/rust/ql/lib/codeql/rust/controlflow/internal/PrintCfg.ql @@ -8,9 +8,8 @@ */ private import codeql.rust.elements.File +private import codeql.rust.controlflow.internal.ControlFlowGraphImpl private import codeql.rust.controlflow.ControlFlowGraph -private import codeql.rust.controlflow.internal.ControlFlowGraphImpl as Impl -private import codeql.rust.controlflow.internal.ControlFlowGraphImplSpecific /** * Gets the source file to generate a CFG from. @@ -33,7 +32,7 @@ external int selectedSourceColumn(); private predicate selectedSourceColumnAlias = selectedSourceColumn/0; -private module ViewCfgQueryInput implements Impl::ViewCfgQueryInputSig { +private module ViewCfgQueryInput implements ViewCfgQueryInputSig { predicate selectedSourceFile = selectedSourceFileAlias/0; predicate selectedSourceLine = selectedSourceLineAlias/0; @@ -41,11 +40,11 @@ private module ViewCfgQueryInput implements Impl::ViewCfgQueryInputSig { predicate selectedSourceColumn = selectedSourceColumnAlias/0; predicate cfgScopeSpan( - CfgInput::CfgScope scope, File file, int startLine, int startColumn, int endLine, int endColumn + CfgScope scope, File file, int startLine, int startColumn, int endLine, int endColumn ) { file = scope.getFile() and scope.getLocation().hasLocationInfo(_, startLine, startColumn, endLine, endColumn) } } -import Impl::ViewCfgQuery +import ViewCfgQuery diff --git a/rust/ql/test/library-tests/controlflow/Cfg.expected b/rust/ql/test/library-tests/controlflow/Cfg.expected index e69de29bb2d1..57cbfe8c0011 100644 --- a/rust/ql/test/library-tests/controlflow/Cfg.expected +++ b/rust/ql/test/library-tests/controlflow/Cfg.expected @@ -0,0 +1,92 @@ +nodes +| test.rs:1:1:7:1 | enter main | semmle.order | 1 | +| test.rs:1:1:7:1 | exit main | semmle.order | 2 | +| test.rs:1:1:7:1 | exit main (normal) | semmle.order | 3 | +| test.rs:1:1:7:1 | main | semmle.order | 4 | +| test.rs:1:18:7:1 | BlockExpr | semmle.order | 5 | +| test.rs:2:5:6:5 | IfExpr | semmle.order | 6 | +| test.rs:2:8:2:12 | LiteralExpr | semmle.order | 7 | +| test.rs:2:8:2:21 | BinaryOpExpr | semmle.order | 8 | +| test.rs:2:17:2:21 | LiteralExpr | semmle.order | 9 | +| test.rs:2:23:4:5 | BlockExpr | semmle.order | 10 | +| test.rs:3:9:3:20 | CallExpr | semmle.order | 11 | +| test.rs:3:19:3:19 | LiteralExpr | semmle.order | 12 | +| test.rs:4:12:6:5 | BlockExpr | semmle.order | 13 | +| test.rs:5:9:5:20 | CallExpr | semmle.order | 14 | +| test.rs:5:19:5:19 | LiteralExpr | semmle.order | 15 | +| test.rs:9:1:16:1 | decrement | semmle.order | 16 | +| test.rs:9:1:16:1 | enter decrement | semmle.order | 17 | +| test.rs:9:1:16:1 | exit decrement | semmle.order | 18 | +| test.rs:9:1:16:1 | exit decrement (normal) | semmle.order | 19 | +| test.rs:9:29:16:1 | BlockExpr | semmle.order | 20 | +| test.rs:11:5:15:5 | IfExpr | semmle.order | 21 | +| test.rs:11:8:11:8 | PathExpr | semmle.order | 22 | +| test.rs:11:8:11:13 | BinaryOpExpr | semmle.order | 23 | +| test.rs:11:13:11:13 | LiteralExpr | semmle.order | 24 | +| test.rs:11:15:13:5 | BlockExpr | semmle.order | 25 | +| test.rs:12:9:12:9 | LiteralExpr | semmle.order | 26 | +| test.rs:13:12:15:5 | BlockExpr | semmle.order | 27 | +| test.rs:14:9:14:9 | PathExpr | semmle.order | 28 | +| test.rs:14:9:14:13 | BinaryOpExpr | semmle.order | 29 | +| test.rs:14:13:14:13 | LiteralExpr | semmle.order | 30 | +edges +| test.rs:1:1:7:1 | enter main | test.rs:2:8:2:12 | LiteralExpr | semmle.label | | +| test.rs:1:1:7:1 | enter main | test.rs:2:8:2:12 | LiteralExpr | semmle.order | 1 | +| test.rs:1:1:7:1 | exit main (normal) | test.rs:1:1:7:1 | exit main | semmle.label | | +| test.rs:1:1:7:1 | exit main (normal) | test.rs:1:1:7:1 | exit main | semmle.order | 1 | +| test.rs:1:1:7:1 | main | test.rs:1:1:7:1 | exit main (normal) | semmle.label | | +| test.rs:1:1:7:1 | main | test.rs:1:1:7:1 | exit main (normal) | semmle.order | 1 | +| test.rs:1:18:7:1 | BlockExpr | test.rs:1:1:7:1 | main | semmle.label | | +| test.rs:1:18:7:1 | BlockExpr | test.rs:1:1:7:1 | main | semmle.order | 1 | +| test.rs:2:5:6:5 | IfExpr | test.rs:1:18:7:1 | BlockExpr | semmle.label | | +| test.rs:2:5:6:5 | IfExpr | test.rs:1:18:7:1 | BlockExpr | semmle.order | 1 | +| test.rs:2:8:2:12 | LiteralExpr | test.rs:2:17:2:21 | LiteralExpr | semmle.label | | +| test.rs:2:8:2:12 | LiteralExpr | test.rs:2:17:2:21 | LiteralExpr | semmle.order | 1 | +| test.rs:2:8:2:21 | BinaryOpExpr | test.rs:3:19:3:19 | LiteralExpr | semmle.label | true | +| test.rs:2:8:2:21 | BinaryOpExpr | test.rs:3:19:3:19 | LiteralExpr | semmle.order | 1 | +| test.rs:2:8:2:21 | BinaryOpExpr | test.rs:5:19:5:19 | LiteralExpr | semmle.label | false | +| test.rs:2:8:2:21 | BinaryOpExpr | test.rs:5:19:5:19 | LiteralExpr | semmle.order | 2 | +| test.rs:2:17:2:21 | LiteralExpr | test.rs:2:8:2:21 | BinaryOpExpr | semmle.label | | +| test.rs:2:17:2:21 | LiteralExpr | test.rs:2:8:2:21 | BinaryOpExpr | semmle.order | 1 | +| test.rs:2:23:4:5 | BlockExpr | test.rs:2:5:6:5 | IfExpr | semmle.label | | +| test.rs:2:23:4:5 | BlockExpr | test.rs:2:5:6:5 | IfExpr | semmle.order | 1 | +| test.rs:3:9:3:20 | CallExpr | test.rs:2:23:4:5 | BlockExpr | semmle.label | | +| test.rs:3:9:3:20 | CallExpr | test.rs:2:23:4:5 | BlockExpr | semmle.order | 1 | +| test.rs:3:19:3:19 | LiteralExpr | test.rs:3:9:3:20 | CallExpr | semmle.label | | +| test.rs:3:19:3:19 | LiteralExpr | test.rs:3:9:3:20 | CallExpr | semmle.order | 1 | +| test.rs:4:12:6:5 | BlockExpr | test.rs:2:5:6:5 | IfExpr | semmle.label | | +| test.rs:4:12:6:5 | BlockExpr | test.rs:2:5:6:5 | IfExpr | semmle.order | 1 | +| test.rs:5:9:5:20 | CallExpr | test.rs:4:12:6:5 | BlockExpr | semmle.label | | +| test.rs:5:9:5:20 | CallExpr | test.rs:4:12:6:5 | BlockExpr | semmle.order | 1 | +| test.rs:5:19:5:19 | LiteralExpr | test.rs:5:9:5:20 | CallExpr | semmle.label | | +| test.rs:5:19:5:19 | LiteralExpr | test.rs:5:9:5:20 | CallExpr | semmle.order | 1 | +| test.rs:9:1:16:1 | decrement | test.rs:9:1:16:1 | exit decrement (normal) | semmle.label | | +| test.rs:9:1:16:1 | decrement | test.rs:9:1:16:1 | exit decrement (normal) | semmle.order | 1 | +| test.rs:9:1:16:1 | enter decrement | test.rs:11:8:11:8 | PathExpr | semmle.label | | +| test.rs:9:1:16:1 | enter decrement | test.rs:11:8:11:8 | PathExpr | semmle.order | 1 | +| test.rs:9:1:16:1 | exit decrement (normal) | test.rs:9:1:16:1 | exit decrement | semmle.label | | +| test.rs:9:1:16:1 | exit decrement (normal) | test.rs:9:1:16:1 | exit decrement | semmle.order | 1 | +| test.rs:9:29:16:1 | BlockExpr | test.rs:9:1:16:1 | decrement | semmle.label | | +| test.rs:9:29:16:1 | BlockExpr | test.rs:9:1:16:1 | decrement | semmle.order | 1 | +| test.rs:11:5:15:5 | IfExpr | test.rs:9:29:16:1 | BlockExpr | semmle.label | | +| test.rs:11:5:15:5 | IfExpr | test.rs:9:29:16:1 | BlockExpr | semmle.order | 1 | +| test.rs:11:8:11:8 | PathExpr | test.rs:11:13:11:13 | LiteralExpr | semmle.label | | +| test.rs:11:8:11:8 | PathExpr | test.rs:11:13:11:13 | LiteralExpr | semmle.order | 1 | +| test.rs:11:8:11:13 | BinaryOpExpr | test.rs:12:9:12:9 | LiteralExpr | semmle.label | true | +| test.rs:11:8:11:13 | BinaryOpExpr | test.rs:12:9:12:9 | LiteralExpr | semmle.order | 1 | +| test.rs:11:8:11:13 | BinaryOpExpr | test.rs:14:9:14:9 | PathExpr | semmle.label | false | +| test.rs:11:8:11:13 | BinaryOpExpr | test.rs:14:9:14:9 | PathExpr | semmle.order | 2 | +| test.rs:11:13:11:13 | LiteralExpr | test.rs:11:8:11:13 | BinaryOpExpr | semmle.label | | +| test.rs:11:13:11:13 | LiteralExpr | test.rs:11:8:11:13 | BinaryOpExpr | semmle.order | 1 | +| test.rs:11:15:13:5 | BlockExpr | test.rs:11:5:15:5 | IfExpr | semmle.label | | +| test.rs:11:15:13:5 | BlockExpr | test.rs:11:5:15:5 | IfExpr | semmle.order | 1 | +| test.rs:12:9:12:9 | LiteralExpr | test.rs:11:15:13:5 | BlockExpr | semmle.label | | +| test.rs:12:9:12:9 | LiteralExpr | test.rs:11:15:13:5 | BlockExpr | semmle.order | 1 | +| test.rs:13:12:15:5 | BlockExpr | test.rs:11:5:15:5 | IfExpr | semmle.label | | +| test.rs:13:12:15:5 | BlockExpr | test.rs:11:5:15:5 | IfExpr | semmle.order | 1 | +| test.rs:14:9:14:9 | PathExpr | test.rs:14:13:14:13 | LiteralExpr | semmle.label | | +| test.rs:14:9:14:9 | PathExpr | test.rs:14:13:14:13 | LiteralExpr | semmle.order | 1 | +| test.rs:14:9:14:13 | BinaryOpExpr | test.rs:13:12:15:5 | BlockExpr | semmle.label | | +| test.rs:14:9:14:13 | BinaryOpExpr | test.rs:13:12:15:5 | BlockExpr | semmle.order | 1 | +| test.rs:14:13:14:13 | LiteralExpr | test.rs:14:9:14:13 | BinaryOpExpr | semmle.label | | +| test.rs:14:13:14:13 | LiteralExpr | test.rs:14:9:14:13 | BinaryOpExpr | semmle.order | 1 | diff --git a/rust/ql/test/library-tests/controlflow/Cfg.ql b/rust/ql/test/library-tests/controlflow/Cfg.ql index e23a3122b370..7792c6c9cc1e 100644 --- a/rust/ql/test/library-tests/controlflow/Cfg.ql +++ b/rust/ql/test/library-tests/controlflow/Cfg.ql @@ -4,8 +4,11 @@ import rust import codeql.rust.controlflow.ControlFlowGraph +import TestUtils class MyRelevantNode extends CfgNode { + MyRelevantNode() { toBeTested(this.getScope()) } + string getOrderDisambiguation() { result = "" } } diff --git a/rust/ql/test/library-tests/controlflow/test.rs b/rust/ql/test/library-tests/controlflow/test.rs index ecb0ecc98b4b..f60a227c16c8 100644 --- a/rust/ql/test/library-tests/controlflow/test.rs +++ b/rust/ql/test/library-tests/controlflow/test.rs @@ -1,7 +1,16 @@ -fn main() { +fn main() -> i64 { if "foo" == "bar" { - println!("foobar"); + decrement(0) } else { - println!("baz") + decrement(5) + } +} + +fn decrement(n: i64) -> i64 { + 12; + if n == 0 { + 0 + } else { + n - 1 } }