From 3d953696087349b0fb7e3fc629405d6ea2310f7f Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Thu, 3 Oct 2024 10:26:37 +0200 Subject: [PATCH 1/4] Shared `ConditionalSplitting` implementation --- .../internal/ControlFlowGraphImpl.qll | 22 +- .../rust/controlflow/internal/Splitting.qll | 90 ++++---- shared/controlflow/codeql/controlflow/Cfg.qll | 196 ++++++++++++++++-- shared/util/codeql/util/Void.qll | 10 + 4 files changed, 254 insertions(+), 64 deletions(-) create mode 100644 shared/util/codeql/util/Void.qll diff --git a/rust/ql/lib/codeql/rust/controlflow/internal/ControlFlowGraphImpl.qll b/rust/ql/lib/codeql/rust/controlflow/internal/ControlFlowGraphImpl.qll index 6ade39203c96..8bb27c3d9910 100644 --- a/rust/ql/lib/codeql/rust/controlflow/internal/ControlFlowGraphImpl.qll +++ b/rust/ql/lib/codeql/rust/controlflow/internal/ControlFlowGraphImpl.qll @@ -7,7 +7,6 @@ private import codeql.rust.controlflow.ControlFlowGraph as Cfg private module CfgInput implements InputSig { private import rust as Rust private import Completion as C - private import Splitting as S class AstNode = Rust::AstNode; @@ -24,10 +23,6 @@ private module CfgInput implements InputSig { CfgScope getCfgScope(AstNode n) { result = Scope::scopeOfAst(n) } - class SplitKindBase = S::TSplitKind; - - class Split = S::Split; - class SuccessorType = Cfg::SuccessorType; /** Gets a successor type that matches completion `c`. */ @@ -51,7 +46,22 @@ private module CfgInput implements InputSig { predicate scopeLast(CfgScope scope, AstNode last, Completion c) { scope.scopeLast(last, c) } } -private module CfgImpl = Make; +private module CfgSplittingInput implements SplittingInputSig { + private import Splitting as S + + class SplitKindBase = S::TSplitKind; + + class Split = S::Split; +} + +private module ConditionalCompletionSplittingInput implements + ConditionalCompletionSplittingInputSig +{ + import Splitting::ConditionalCompletionSplitting::ConditionalCompletionSplittingInput +} + +private module CfgImpl = + MakeWithSplitting; import CfgImpl diff --git a/rust/ql/lib/codeql/rust/controlflow/internal/Splitting.qll b/rust/ql/lib/codeql/rust/controlflow/internal/Splitting.qll index 10965649caa5..ae00e3e0d7a5 100644 --- a/rust/ql/lib/codeql/rust/controlflow/internal/Splitting.qll +++ b/rust/ql/lib/codeql/rust/controlflow/internal/Splitting.qll @@ -21,7 +21,7 @@ abstract private class Split_ extends TSplit { final class Split = Split_; -private module ConditionalCompletionSplitting { +module ConditionalCompletionSplitting { /** * A split for conditional completions. For example, in * @@ -39,10 +39,12 @@ private module ConditionalCompletionSplitting { ConditionalCompletionSplit() { this = TConditionalCompletionSplit(completion) } + ConditionalCompletion getCompletion() { result = completion } + override string toString() { result = completion.toString() } } - private class ConditionalCompletionSplitKind extends SplitKind, TConditionalCompletionSplitKind { + private class ConditionalCompletionSplitKind_ extends SplitKind, TConditionalCompletionSplitKind { override int getListOrder() { result = 0 } override predicate isEnabled(AstNode cfe) { this.appliesTo(cfe) } @@ -50,66 +52,64 @@ private module ConditionalCompletionSplitting { override string toString() { result = "ConditionalCompletion" } } - private class ConditionalCompletionSplitImpl extends SplitImpl instanceof ConditionalCompletionSplit - { - ConditionalCompletion completion; + module ConditionalCompletionSplittingInput { + private import Completion as Comp - ConditionalCompletionSplitImpl() { this = TConditionalCompletionSplit(completion) } + class ConditionalCompletion = Comp::ConditionalCompletion; - override ConditionalCompletionSplitKind getKind() { any() } + class ConditionalCompletionSplitKind extends ConditionalCompletionSplitKind_, TSplitKind { } - override predicate hasEntry(AstNode pred, AstNode succ, Completion c) { - succ(pred, succ, c) and - last(succ, _, completion) and + class ConditionalCompletionSplit = ConditionalCompletionSplitting::ConditionalCompletionSplit; + + bindingset[parent, parentCompletion] + predicate condPropagateExpr( + AstNode parent, ConditionalCompletion parentCompletion, AstNode child, + ConditionalCompletion childCompletion + ) { + child = parent.(LogicalNotExpr).getExpr() and + childCompletion.getDual() = parentCompletion + or + childCompletion = parentCompletion and ( - last(succ.(LogicalNotExpr).getExpr(), pred, c) and - completion.(BooleanCompletion).getDual() = c - or - last(succ.(BinaryLogicalOperation).getAnOperand(), pred, c) and - completion = c + child = parent.(BinaryLogicalOperation).getAnOperand() or - succ = - any(IfExpr ie | - last([ie.getThen(), ie.getElse()], pred, c) and - completion = c - ) + parent = any(IfExpr ie | child = [ie.getThen(), ie.getElse()]) or - last(succ.(MatchExpr).getAnArm().getExpr(), pred, c) and - completion = c + child = parent.(MatchExpr).getAnArm().getExpr() or - last(succ.(BlockExpr).getStmtList().getTailExpr(), pred, c) and - completion = c + child = parent.(BlockExpr).getStmtList().getTailExpr() ) - or - succ(pred, succ, c) and - last(succ.(BreakExpr).getExpr(), pred, c) and - exists(AstNode target | - succ(succ, target, _) and - last(target, _, completion) - ) and - completion = c } + } - override predicate hasEntryScope(CfgScope scope, AstNode first) { none() } - - override predicate hasExit(AstNode pred, AstNode succ, Completion c) { - this.appliesTo(pred) and - succ(pred, succ, c) and - if c instanceof ConditionalCompletion - then completion = c - else not this.hasSuccessor(pred, succ, c) + private class ConditionalCompletionSplitImpl extends SplitImplementations::ConditionalCompletionSplitting::ConditionalCompletionSplitImpl + { + /** + * Gets a `break` expression whose target can have a Boolean completion that + * matches this split. + */ + private BreakExpr getABreakExpr(Expr target) { + target = result.getTarget() and + last(target, _, this.getCompletion()) } - override predicate hasExitScope(CfgScope scope, AstNode last, Completion c) { - this.appliesTo(last) and - scope.scopeLast(last, c) and - if c instanceof ConditionalCompletion then completion = c else any() + override predicate hasEntry(AstNode pred, AstNode succ, Completion c) { + super.hasEntry(pred, succ, c) + or + succ(pred, succ, c) and + last(succ.(BreakExpr).getExpr(), pred, c) and + succ = this.getABreakExpr(_) and + c = this.getCompletion() } override predicate hasSuccessor(AstNode pred, AstNode succ, Completion c) { + super.hasSuccessor(pred, succ, c) + or this.appliesTo(pred) and succ(pred, succ, c) and - not c instanceof ConditionalCompletion + pred = this.getABreakExpr(succ) } } + + int getNextListOrder() { result = 1 } } diff --git a/shared/controlflow/codeql/controlflow/Cfg.qll b/shared/controlflow/codeql/controlflow/Cfg.qll index 7f1eb54b2b02..be05a5695a95 100644 --- a/shared/controlflow/codeql/controlflow/Cfg.qll +++ b/shared/controlflow/codeql/controlflow/Cfg.qll @@ -5,6 +5,7 @@ private import codeql.util.Location private import codeql.util.FileSystem +private import codeql.util.Void /** Provides the language-specific input specification. */ signature module InputSig { @@ -56,18 +57,6 @@ signature module InputSig { /** Holds if `scope` is exited when `last` finishes with completion `c`. */ predicate scopeLast(CfgScope scope, AstNode last, Completion c); - /** Gets the maximum number of splits allowed for a given node. */ - default int maxSplits() { result = 5 } - - /** The base class of `SplitKind`. */ - class SplitKindBase; - - /** A split. */ - class Split { - /** Gets a textual representation of this split. */ - string toString(); - } - /** A type of a control flow successor. */ class SuccessorType { /** Gets a textual representation of this successor type. */ @@ -90,6 +79,56 @@ signature module InputSig { predicate isAbnormalExitType(SuccessorType t); } +/** Provides input needed for CFG splitting. */ +signature module SplittingInputSig Input> { + /** Gets the maximum number of splits allowed for a given node. */ + default int maxSplits() { result = 5 } + + /** The base class of `SplitKind`. */ + class SplitKindBase; + + /** A split. */ + class Split { + /** Gets a textual representation of this split. */ + string toString(); + } +} + +/** Provides input needed for `ConditionalCompletionSplitting`. */ +signature module ConditionalCompletionSplittingInputSig< + LocationSig Location, InputSig Input, SplittingInputSig SplittingInput> +{ + /** A conditional control-flow completion. */ + class ConditionalCompletion extends Input::Completion; + + /** A split kind for `ConditionalCompletionSplitting`. */ + class ConditionalCompletionSplitKind extends SplittingInput::SplitKindBase; + + /** The user-facing split class. */ + class ConditionalCompletionSplit extends SplittingInput::Split { + /** Gets the completion recorded in this split. */ + ConditionalCompletion getCompletion(); + } + + /** + * Holds if `child` is a sub expression of `parent`, and whenever a last node + * of `child` (normally `child` itself) has `parent` as a successor with label + * `childCompletion`, then edges out of `parent` must have label + * `parentCompletion`. + * + * For example, for an expression `!x`, when `child = x` has conditional + * completion `c` then `parent = !x` must have the dual completion of `c`. + * + * Similarly, for an expression `x && y`, when `child = {x, y}` has conditional + * completion `c`, then `parent = x && y` must have the same completion of `c`. + */ + bindingset[parent, parentCompletion] + predicate condPropagateExpr( + Input::AstNode parent, ConditionalCompletion parentCompletion, Input::AstNode child, + ConditionalCompletion childCompletion + ); +} + /** * Provides a shared interface for constructing control-flow graphs (CFGs) from * abstract syntax trees (ASTs). @@ -122,8 +161,14 @@ signature module InputSig { * loop break will be caught up by its surrounding loop and turned into a normal * completion. */ -module Make Input> { +module MakeWithSplitting< + LocationSig Location, // + InputSig Input, // + SplittingInputSig SplittingInput, // + ConditionalCompletionSplittingInputSig ConditionalCompletionSplittingInput> +{ private import Input + private import SplittingInput final private class AstNodeFinal = AstNode; @@ -1133,6 +1178,71 @@ module Make Input> { final class AstCfgNode = AstCfgNodeImpl; + /** Provides logic for common CFG split implementations that can be reused across languages. */ + module SplitImplementations { + /** + * Provides an implementation of splitting for conditional completions. + * + * For example, in + * + * ```rust + * if x && !y { + * // ... + * } + * ``` + * + * we record whether `x`, `y`, and `!y` evaluate to `true` or `false`, and restrict + * the edges out of `!y` and `x && !y` accordingly. + */ + module ConditionalCompletionSplitting { + private import ConditionalCompletionSplittingInput + + final private class ConditionalCompletionSplitFinal = + ConditionalCompletionSplittingInput::ConditionalCompletionSplit; + + /** A split for conditional completions. */ + class ConditionalCompletionSplitImpl extends SplitImpl, ConditionalCompletionSplitFinal { + ConditionalCompletion completion; + + ConditionalCompletionSplitImpl() { completion = this.getCompletion() } + + override SplitKind getKind() { result instanceof ConditionalCompletionSplitKind } + + override predicate hasEntry(AstNode pred, AstNode succ, Completion c) { + exists(AstNode child, AstNode parent | + last(parent, succ, completion) and + condPropagateExpr(parent, completion, child, c) and + succ(pred, succ, c) and + last(child, pred, c) + ) + } + + override predicate hasEntryScope(CfgScope scope, AstNode first) { none() } + + override predicate hasExit(AstNode pred, AstNode succ, Completion c) { + this.appliesTo(pred) and + succ(pred, succ, c) and + if c instanceof ConditionalCompletion + then completion = c + else not this.hasSuccessor(pred, succ, c) + } + + override predicate hasExitScope(CfgScope scope, AstNode last, Completion c) { + this.appliesTo(last) and + scopeLast(scope, last, c) and + if c instanceof ConditionalCompletion then completion = c else any() + } + + override predicate hasSuccessor(AstNode pred, AstNode succ, Completion c) { + this.appliesTo(pred) and + succ(pred, succ, c) and + not c instanceof ConditionalCompletion and + completionIsNormal(c) + } + } + } + } + /** A node to be included in the output of `TestOutput`. */ signature class RelevantNodeSig extends Node; @@ -1152,6 +1262,9 @@ module Make Input> { ) } + /** + * Provides logic for representing a CFG as a [Mermaid diagram](https://mermaid.js.org/). + */ module Mermaid { private string nodeId(RelevantNode n) { result = @@ -1200,6 +1313,7 @@ module Make Input> { ) } + /** Holds if the Mermaid representation is `s`. */ query predicate mermaid(string s) { s = "flowchart TD\n" + nodes() + "\n\n" + edges() } } } @@ -1277,6 +1391,7 @@ module Make Input> { import Output::Mermaid + /** Holds if `pred` -> `succ` is an edge in the CFG. */ query predicate edges(RelevantNode pred, RelevantNode succ, string attr, string val) { attr = "semmle.label" and Output::edges(pred, succ, val) @@ -1397,3 +1512,58 @@ module Make Input> { query predicate scopeNoFirst(CfgScope scope) { not scopeFirst(scope, _) } } } + +/** Provides a disabled `SplittingInputSig` implementation. */ +module NoSplittingInput Input> implements + SplittingInputSig +{ + int maxSplits() { result = 0 } + + class SplitKindBase = Void; + + class Split = Void; +} + +/** Provides a disabled `ConditionalCompletionSplittingInputSig` implementation. */ +module NoConditionalCompletionSplittingInput< + LocationSig Location, InputSig Input, SplittingInputSig SplittingInput> + implements ConditionalCompletionSplittingInputSig +{ + final private class Completion = Input::Completion; + + class ConditionalCompletion extends Completion { + ConditionalCompletion() { none() } + } + + final private class SplitKindBase = SplittingInput::SplitKindBase; + + class ConditionalCompletionSplitKind extends SplitKindBase { + ConditionalCompletionSplitKind() { none() } + + /** Gets a textual representation of this split kind. */ + string toString() { none() } + } + + final private class Split = SplittingInput::Split; + + class ConditionalCompletionSplit extends Split { + ConditionalCompletion getCompletion() { none() } + } + + predicate condPropagateExpr( + Input::AstNode parent, ConditionalCompletion parentCompletion, Input::AstNode child, + ConditionalCompletion childCompletion + ) { + none() + } +} + +/** Same as `MakeWithSplitting`, but without CFG splitting. */ +module Make Input> { + private module SplittingInput = NoSplittingInput; + + private module ConditionalCompletionSplittingInput = + NoConditionalCompletionSplittingInput; + + import MakeWithSplitting +} diff --git a/shared/util/codeql/util/Void.qll b/shared/util/codeql/util/Void.qll new file mode 100644 index 000000000000..886687b54602 --- /dev/null +++ b/shared/util/codeql/util/Void.qll @@ -0,0 +1,10 @@ +/** Provides the empty `Void` class. */ + +/** The empty void type. */ +private newtype TVoid = TMkVoid() { none() } + +/** The trivial empty type. */ +final class Void extends TVoid { + /** Gets a textual representation of this element. */ + string toString() { result = "dummy" } +} From 5d925d36d334e2378bc03e5c2784b5c05eaae361 Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Thu, 3 Oct 2024 15:11:43 +0200 Subject: [PATCH 2/4] C#: Adopt shared `ConditionalCompletionSplitting` implementation --- .../internal/ControlFlowGraphImpl.qll | 21 +++- .../csharp/controlflow/internal/Splitting.qll | 114 +++++++----------- 2 files changed, 61 insertions(+), 74 deletions(-) diff --git a/csharp/ql/lib/semmle/code/csharp/controlflow/internal/ControlFlowGraphImpl.qll b/csharp/ql/lib/semmle/code/csharp/controlflow/internal/ControlFlowGraphImpl.qll index 74383240f5e3..463ed260067c 100644 --- a/csharp/ql/lib/semmle/code/csharp/controlflow/internal/ControlFlowGraphImpl.qll +++ b/csharp/ql/lib/semmle/code/csharp/controlflow/internal/ControlFlowGraphImpl.qll @@ -53,7 +53,6 @@ private predicate idOf(AstNode x, int y) = equivalenceRelation(id/2)(x, y) private module CfgInput implements CfgShared::InputSig { private import ControlFlowGraphImpl as Impl private import Completion as Comp - private import Splitting as Splitting private import SuccessorType as ST private import semmle.code.csharp.Caching @@ -80,10 +79,6 @@ private module CfgInput implements CfgShared::InputSig { Impl::scopeLast(scope, last, c) } - class SplitKindBase = Splitting::TSplitKind; - - class Split = Splitting::Split; - class SuccessorType = ST::SuccessorType; SuccessorType getAMatchingSuccessorType(Completion c) { result = c.getAMatchingSuccessorType() } @@ -102,7 +97,21 @@ private module CfgInput implements CfgShared::InputSig { } } -import CfgShared::Make +private module CfgSplittingInput implements CfgShared::SplittingInputSig { + private import Splitting as S + + class SplitKindBase = S::TSplitKind; + + class Split = S::Split; +} + +private module ConditionalCompletionSplittingInput implements + CfgShared::ConditionalCompletionSplittingInputSig +{ + import Splitting::ConditionalCompletionSplitting::ConditionalCompletionSplittingInput +} + +import CfgShared::MakeWithSplitting /** * A compilation. diff --git a/csharp/ql/lib/semmle/code/csharp/controlflow/internal/Splitting.qll b/csharp/ql/lib/semmle/code/csharp/controlflow/internal/Splitting.qll index 2fc5ffe4afd6..b6efdfcf1eac 100644 --- a/csharp/ql/lib/semmle/code/csharp/controlflow/internal/Splitting.qll +++ b/csharp/ql/lib/semmle/code/csharp/controlflow/internal/Splitting.qll @@ -5,7 +5,8 @@ */ import csharp -private import Completion +private import Completion as Comp +private import Comp private import ControlFlowGraphImpl private import semmle.code.csharp.controlflow.ControlFlowGraph::ControlFlow as Cfg private import semmle.code.csharp.controlflow.internal.PreSsa @@ -260,10 +261,12 @@ module ConditionalCompletionSplitting { ConditionalCompletionSplit() { this = TConditionalCompletionSplit(completion) } + ConditionalCompletion getCompletion() { result = completion } + override string toString() { result = completion.toString() } } - private class ConditionalCompletionSplitKind extends SplitKind, TConditionalCompletionSplitKind { + private class ConditionalCompletionSplitKind_ extends SplitKind, TConditionalCompletionSplitKind { override int getListOrder() { result = InitializerSplitting::getNextListOrder() } override predicate isEnabled(AstNode cfe) { this.appliesTo(cfe) } @@ -271,89 +274,64 @@ module ConditionalCompletionSplitting { override string toString() { result = "ConditionalCompletion" } } - int getNextListOrder() { result = InitializerSplitting::getNextListOrder() + 1 } + module ConditionalCompletionSplittingInput { + private import Completion as Comp - private class ConditionalCompletionSplitImpl extends SplitImpl instanceof ConditionalCompletionSplit - { - ConditionalCompletion completion; + class ConditionalCompletion = Comp::ConditionalCompletion; - ConditionalCompletionSplitImpl() { this = TConditionalCompletionSplit(completion) } + class ConditionalCompletionSplitKind extends ConditionalCompletionSplitKind_, TSplitKind { } - override ConditionalCompletionSplitKind getKind() { any() } + class ConditionalCompletionSplit = ConditionalCompletionSplitting::ConditionalCompletionSplit; - override predicate hasEntry(AstNode pred, AstNode succ, Completion c) { - succ(pred, succ, c) and - last(succ, _, completion) and + bindingset[parent, parentCompletion] + predicate condPropagateExpr( + AstNode parent, ConditionalCompletion parentCompletion, AstNode child, + ConditionalCompletion childCompletion + ) { + child = parent.(LogicalNotExpr).getOperand() and + childCompletion.getDual() = parentCompletion + or + childCompletion = parentCompletion and ( - last(succ.(LogicalNotExpr).getOperand(), pred, c) and - completion.(BooleanCompletion).getDual() = c + child = parent.(LogicalAndExpr).getAnOperand() or - last(succ.(LogicalAndExpr).getAnOperand(), pred, c) and - completion = c + child = parent.(LogicalOrExpr).getAnOperand() or - last(succ.(LogicalOrExpr).getAnOperand(), pred, c) and - completion = c + parent = any(ConditionalExpr ce | child = [ce.getThen(), ce.getElse()]) or - succ = - any(ConditionalExpr ce | - last([ce.getThen(), ce.getElse()], pred, c) and - completion = c - ) + child = parent.(SwitchExpr).getACase() or - succ = + child = parent.(SwitchCaseExpr).getBody() + or + parent = any(NullCoalescingExpr nce | - exists(Expr operand | - last(operand, pred, c) and - completion = c - | - if c instanceof NullnessCompletion - then operand = nce.getRightOperand() - else operand = nce.getAnOperand() - ) + if childCompletion instanceof NullnessCompletion + then child = nce.getRightOperand() + else child = nce.getAnOperand() ) + ) + or + child = parent.(NotPatternExpr).getPattern() and + childCompletion.getDual() = parentCompletion + or + child = parent.(IsExpr).getPattern() and + parentCompletion.(BooleanCompletion).getValue() = + childCompletion.(MatchingCompletion).getValue() + or + childCompletion = parentCompletion and + ( + child = parent.(AndPatternExpr).getAnOperand() or - last(succ.(SwitchExpr).getACase(), pred, c) and - completion = c - or - last(succ.(SwitchCaseExpr).getBody(), pred, c) and - completion = c - or - last(succ.(NotPatternExpr).getPattern(), pred, c) and - completion.(MatchingCompletion).getDual() = c - or - last(succ.(IsExpr).getPattern(), pred, c) and - completion.(BooleanCompletion).getValue() = c.(MatchingCompletion).getValue() - or - last(succ.(AndPatternExpr).getAnOperand(), pred, c) and - completion = c - or - last(succ.(OrPatternExpr).getAnOperand(), pred, c) and - completion = c + child = parent.(OrPatternExpr).getAnOperand() or - last(succ.(RecursivePatternExpr).getAChildExpr(), pred, c) and - completion = c + child = parent.(RecursivePatternExpr).getAChildExpr() or - last(succ.(PropertyPatternExpr).getPattern(_), pred, c) and - completion = c + child = parent.(PropertyPatternExpr).getPattern(_) ) } - - override predicate hasEntryScope(CfgScope scope, AstNode first) { none() } - - override predicate hasExit(AstNode pred, AstNode succ, Completion c) { - this.appliesTo(pred) and - succ(pred, succ, c) and - if c instanceof ConditionalCompletion then completion = c else any() - } - - override predicate hasExitScope(CfgScope scope, AstNode last, Completion c) { - this.appliesTo(last) and - scopeLast(scope, last, c) and - if c instanceof ConditionalCompletion then completion = c else any() - } - - override predicate hasSuccessor(AstNode pred, AstNode succ, Completion c) { none() } } + + int getNextListOrder() { result = InitializerSplitting::getNextListOrder() + 1 } } module AssertionSplitting { From 3a098d7449a1f19a91737fce2722dc16cbb1e16d Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Thu, 3 Oct 2024 21:24:18 +0200 Subject: [PATCH 3/4] Ruby: Adopt shared `ConditionalCompletionSplitting` implementation --- .../internal/ControlFlowGraphImpl.qll | 21 ++++-- .../ruby/controlflow/internal/Splitting.qll | 73 +++++++++---------- 2 files changed, 49 insertions(+), 45 deletions(-) diff --git a/ruby/ql/lib/codeql/ruby/controlflow/internal/ControlFlowGraphImpl.qll b/ruby/ql/lib/codeql/ruby/controlflow/internal/ControlFlowGraphImpl.qll index 0aa93e573ebe..643decb560b9 100644 --- a/ruby/ql/lib/codeql/ruby/controlflow/internal/ControlFlowGraphImpl.qll +++ b/ruby/ql/lib/codeql/ruby/controlflow/internal/ControlFlowGraphImpl.qll @@ -22,7 +22,6 @@ class AstNode extends Ast::AstNode { private module CfgInput implements CfgShared::InputSig { private import ControlFlowGraphImpl as Impl private import Completion as Comp - private import Splitting as Splitting private import codeql.ruby.CFG as Cfg class AstNode = Impl::AstNode; @@ -45,10 +44,6 @@ private module CfgInput implements CfgShared::InputSig { scope.(Impl::CfgScopeImpl).exit(last, c) } - class SplitKindBase = Splitting::TSplitKind; - - class Split = Splitting::Split; - class SuccessorType = Cfg::SuccessorType; SuccessorType getAMatchingSuccessorType(Completion c) { result = c.getAMatchingSuccessorType() } @@ -67,7 +62,21 @@ private module CfgInput implements CfgShared::InputSig { } } -import CfgShared::Make +private module CfgSplittingInput implements CfgShared::SplittingInputSig { + private import Splitting as S + + class SplitKindBase = S::TSplitKind; + + class Split = S::Split; +} + +private module ConditionalCompletionSplittingInput implements + CfgShared::ConditionalCompletionSplittingInputSig +{ + import Splitting::ConditionalCompletionSplitting::ConditionalCompletionSplittingInput +} + +import CfgShared::MakeWithSplitting abstract class CfgScopeImpl extends AstNode { abstract predicate entry(AstNode first); diff --git a/ruby/ql/lib/codeql/ruby/controlflow/internal/Splitting.qll b/ruby/ql/lib/codeql/ruby/controlflow/internal/Splitting.qll index d8cfb9230d8b..5331a3d26b64 100644 --- a/ruby/ql/lib/codeql/ruby/controlflow/internal/Splitting.qll +++ b/ruby/ql/lib/codeql/ruby/controlflow/internal/Splitting.qll @@ -3,7 +3,8 @@ */ private import codeql.ruby.AST as Ast -private import Completion +private import Completion as Comp +private import Comp private import ControlFlowGraphImpl private import SuccessorTypes private import codeql.ruby.controlflow.ControlFlowGraph @@ -31,7 +32,7 @@ class Split extends TSplit { string toString() { none() } } -private module ConditionalCompletionSplitting { +module ConditionalCompletionSplitting { /** * A split for conditional completions. For example, in * @@ -51,10 +52,12 @@ private module ConditionalCompletionSplitting { ConditionalCompletionSplit() { this = TConditionalCompletionSplit(completion) } + ConditionalCompletion getCompletion() { result = completion } + override string toString() { result = completion.toString() } } - private class ConditionalCompletionSplitKind extends SplitKind, TConditionalCompletionSplitKind { + private class ConditionalCompletionSplitKind_ extends SplitKind, TConditionalCompletionSplitKind { override int getListOrder() { result = 0 } override predicate isEnabled(AstNode n) { this.appliesTo(n) } @@ -62,56 +65,48 @@ private module ConditionalCompletionSplitting { override string toString() { result = "ConditionalCompletion" } } - int getNextListOrder() { result = 1 } + module ConditionalCompletionSplittingInput { + private import Completion as Comp - private class ConditionalCompletionSplitImpl extends SplitImpl instanceof ConditionalCompletionSplit - { - ConditionalCompletion completion; + class ConditionalCompletion = Comp::ConditionalCompletion; - ConditionalCompletionSplitImpl() { this = TConditionalCompletionSplit(completion) } + class ConditionalCompletionSplitKind extends ConditionalCompletionSplitKind_, TSplitKind { } - override ConditionalCompletionSplitKind getKind() { any() } + class ConditionalCompletionSplit = ConditionalCompletionSplitting::ConditionalCompletionSplit; - override predicate hasEntry(AstNode pred, AstNode succ, Completion c) { - succ(pred, succ, c) and - last(succ, _, completion) and + bindingset[parent, parentCompletion] + predicate condPropagateExpr( + AstNode parent, ConditionalCompletion parentCompletion, AstNode child, + ConditionalCompletion childCompletion + ) { + child = parent.(Ast::NotExpr).getOperand() and + childCompletion.(BooleanCompletion).getDual() = parentCompletion + or + childCompletion = parentCompletion and ( - last(succ.(Ast::NotExpr).getOperand(), pred, c) and - completion.(BooleanCompletion).getDual() = c - or - last(succ.(Ast::LogicalAndExpr).getAnOperand(), pred, c) and - completion = c + child = parent.(Ast::LogicalAndExpr).getAnOperand() or - last(succ.(Ast::LogicalOrExpr).getAnOperand(), pred, c) and - completion = c + child = parent.(Ast::LogicalOrExpr).getAnOperand() or - last(succ.(Ast::StmtSequence).getLastStmt(), pred, c) and - completion = c + child = parent.(Ast::StmtSequence).getLastStmt() or - last(succ.(Ast::ConditionalExpr).getBranch(_), pred, c) and - completion = c + child = parent.(Ast::ConditionalExpr).getBranch(_) ) - or - succ(pred, succ, c) and - succ instanceof Ast::WhenClause and - completion = c } + } - override predicate hasEntryScope(CfgScope scope, AstNode succ) { none() } + int getNextListOrder() { result = 1 } - override predicate hasExit(AstNode pred, AstNode succ, Completion c) { - this.appliesTo(pred) and + private class ConditionalCompletionSplitImpl extends SplitImplementations::ConditionalCompletionSplitting::ConditionalCompletionSplitImpl + { + override predicate hasEntry(AstNode pred, AstNode succ, Completion c) { + super.hasEntry(pred, succ, c) + or + // a non-standard case is needed for `when` clauses succ(pred, succ, c) and - if c instanceof ConditionalCompletion then completion = c else any() - } - - override predicate hasExitScope(CfgScope scope, AstNode last, Completion c) { - this.appliesTo(last) and - succExit(scope, last, c) and - if c instanceof ConditionalCompletion then completion = c else any() + succ instanceof Ast::WhenClause and + c = this.getCompletion() } - - override predicate hasSuccessor(AstNode pred, AstNode succ, Completion c) { none() } } } From bdb793ba92fb1d7766375c261bf0ba1ae40c7bcc Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Fri, 4 Oct 2024 10:13:40 +0200 Subject: [PATCH 4/4] Swift: Adopt shared `ConditionalCompletionSplitting` implementation --- .../internal/ControlFlowGraphImplSpecific.qll | 21 ++++-- .../swift/controlflow/internal/Splitting.qll | 72 ++++++++----------- 2 files changed, 47 insertions(+), 46 deletions(-) diff --git a/swift/ql/lib/codeql/swift/controlflow/internal/ControlFlowGraphImplSpecific.qll b/swift/ql/lib/codeql/swift/controlflow/internal/ControlFlowGraphImplSpecific.qll index 07d3a0842c9b..362537068a63 100644 --- a/swift/ql/lib/codeql/swift/controlflow/internal/ControlFlowGraphImplSpecific.qll +++ b/swift/ql/lib/codeql/swift/controlflow/internal/ControlFlowGraphImplSpecific.qll @@ -46,10 +46,6 @@ module CfgInput implements InputSig { CfgScope getCfgScope(AstNode n) { result = scopeOfAst(n.asAstNode()) } - class SplitKindBase = Splitting::TSplitKind; - - class Split = Splitting::Split; - class SuccessorType = Cfg::SuccessorType; /** Gets a successor type that matches completion `c`. */ @@ -88,4 +84,19 @@ module CfgInput implements InputSig { } } -module CfgImpl = Make; +private module CfgSplittingInput implements SplittingInputSig { + private import Splitting as S + + class SplitKindBase = S::TSplitKind; + + class Split = S::Split; +} + +private module ConditionalCompletionSplittingInput implements + ConditionalCompletionSplittingInputSig +{ + import Splitting::ConditionalCompletionSplitting::ConditionalCompletionSplittingInput +} + +module CfgImpl = + MakeWithSplitting; diff --git a/swift/ql/lib/codeql/swift/controlflow/internal/Splitting.qll b/swift/ql/lib/codeql/swift/controlflow/internal/Splitting.qll index dcc34ceb46b6..ddf2f01a8d41 100644 --- a/swift/ql/lib/codeql/swift/controlflow/internal/Splitting.qll +++ b/swift/ql/lib/codeql/swift/controlflow/internal/Splitting.qll @@ -26,7 +26,7 @@ class Split extends TSplit { string toString() { none() } } -private module ConditionalCompletionSplitting { +module ConditionalCompletionSplitting { /** A split for conditional completions. */ class ConditionalCompletionSplit extends Split, TConditionalCompletionSplit { ConditionalCompletion completion; @@ -38,7 +38,7 @@ private module ConditionalCompletionSplitting { override string toString() { result = completion.toString() } } - private class ConditionalCompletionSplitKind extends SplitKind, TConditionalCompletionSplitKind { + private class ConditionalCompletionSplitKind_ extends SplitKind, TConditionalCompletionSplitKind { override int getListOrder() { result = 0 } override predicate isEnabled(ControlFlowElement n) { this.appliesTo(n) } @@ -46,54 +46,44 @@ private module ConditionalCompletionSplitting { override string toString() { result = "ConditionalCompletion" } } - private class ConditionalCompletionSplitImpl extends SplitImpl instanceof ConditionalCompletionSplit - { - override ConditionalCompletionSplitKind getKind() { any() } + module ConditionalCompletionSplittingInput { + private import Completion as Comp - override predicate hasEntry(ControlFlowElement pred, ControlFlowElement succ, Completion c) { - succ(pred, succ, c) and - last(succ, _, super.getCompletion()) and + class ConditionalCompletion = Comp::ConditionalCompletion; + + class ConditionalCompletionSplitKind extends ConditionalCompletionSplitKind_, TSplitKind { } + + class ConditionalCompletionSplit = ConditionalCompletionSplitting::ConditionalCompletionSplit; + + bindingset[parent, parentCompletion] + private predicate condPropagateAstExpr( + AstNode parent, ConditionalCompletion parentCompletion, AstNode child, + ConditionalCompletion childCompletion + ) { + child = parent.(NotExpr).getOperand().getFullyConverted() and + childCompletion.(BooleanCompletion).getDual() = parentCompletion + or + childCompletion = parentCompletion and ( - astLast(succ.asAstNode().(NotExpr).getOperand().getFullyConverted(), pred, c) and - super.getCompletion().(BooleanCompletion).getDual() = c + child = parent.(LogicalAndExpr).getAnOperand().getFullyConverted() or - astLast(succ.asAstNode().(LogicalAndExpr).getAnOperand().getFullyConverted(), pred, c) and - super.getCompletion() = c + child = parent.(LogicalOrExpr).getAnOperand().getFullyConverted() or - astLast(succ.asAstNode().(LogicalOrExpr).getAnOperand().getFullyConverted(), pred, c) and - super.getCompletion() = c + child = parent.(IfExpr).getBranch(_).getFullyConverted() or - succ.asAstNode() = - any(IfExpr ce | - astLast(ce.getBranch(_).getFullyConverted(), pred, c) and - super.getCompletion() = c - ) - or - exists(Expr e, Exprs::Conversions::ConversionOrIdentityTree conv | - succ.asAstNode() = conv.getAst() and - conv.convertsFrom(e) and - astLast(e, pred, c) and - super.getCompletion() = c + exists(Exprs::Conversions::ConversionOrIdentityTree conv | + parent = conv.getAst() and + conv.convertsFrom(child) ) ) } - override predicate hasEntryScope(CfgInput::CfgScope scope, ControlFlowElement succ) { none() } - - override predicate hasExit(ControlFlowElement pred, ControlFlowElement succ, Completion c) { - this.appliesTo(pred) and - succ(pred, succ, c) and - if c instanceof ConditionalCompletion then super.getCompletion() = c else any() - } - - override predicate hasExitScope(CfgInput::CfgScope scope, ControlFlowElement last, Completion c) { - this.appliesTo(last) and - succExit(scope, last, c) and - if c instanceof ConditionalCompletion then super.getCompletion() = c else any() - } - - override predicate hasSuccessor(ControlFlowElement pred, ControlFlowElement succ, Completion c) { - none() + bindingset[parent, parentCompletion] + predicate condPropagateExpr( + ControlFlowElement parent, ConditionalCompletion parentCompletion, ControlFlowElement child, + ConditionalCompletion childCompletion + ) { + condPropagateAstExpr(parent.asAstNode(), parentCompletion, child.asAstNode(), childCompletion) } } }