From 1ded4df3fd6d78060a7b22428329c1f6ee69fb59 Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Thu, 27 Mar 2025 11:35:38 +0100 Subject: [PATCH 01/10] SSA: Add an alternative to ssaDefAssigns/ssaDefInitializesParam. --- shared/ssa/codeql/ssa/Ssa.qll | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/shared/ssa/codeql/ssa/Ssa.qll b/shared/ssa/codeql/ssa/Ssa.qll index 47578bee7b9..62dc4230573 100644 --- a/shared/ssa/codeql/ssa/Ssa.qll +++ b/shared/ssa/codeql/ssa/Ssa.qll @@ -1459,6 +1459,15 @@ module Make Input> { ) } + /** + * Holds if `def` has some form of input flow. For example, the right-hand + * side of an assignment or a parameter of an SSA entry definition. + * + * For such definitions, a flow step is added from a synthetic node + * representing the source to the definition. + */ + default predicate ssaDefHasSource(WriteDefinition def) { any() } + /** Holds if SSA definition `def` assigns `value` to the underlying variable. */ predicate ssaDefAssigns(WriteDefinition def, Expr value); @@ -1665,6 +1674,7 @@ module Make Input> { cached private newtype TNode = + TWriteDefSource(WriteDefinition def) { DfInput::ssaDefHasSource(def) } or TParamNode(DfInput::Parameter p) { exists(WriteDefinition def | DfInput::ssaDefInitializesParam(def, p)) } or @@ -1696,6 +1706,22 @@ module Make Input> { final class Node = NodeImpl; + /** A source of a write definition. */ + private class WriteDefSourceNodeImpl extends NodeImpl, TWriteDefSource { + private WriteDefinition def; + + WriteDefSourceNodeImpl() { this = TWriteDefSource(def) } + + /** Gets the underlying definition. */ + WriteDefinition getDefinition() { result = def } + + override string toString() { result = def.toString() } + + override Location getLocation() { result = def.getLocation() } + } + + final class WriteDefSourceNode = WriteDefSourceNodeImpl; + /** A parameter node. */ private class ParameterNodeImpl extends NodeImpl, TParamNode { private DfInput::Parameter p; @@ -1976,6 +2002,9 @@ module Make Input> { */ predicate localFlowStep(SourceVariable v, Node nodeFrom, Node nodeTo, boolean isUseStep) { exists(Definition def | + // Flow from write definition source into SSA definition + nodeFrom = TWriteDefSource(def) + or // Flow from assignment into SSA definition DfInput::ssaDefAssigns(def, nodeFrom.(ExprNode).getExpr()) or @@ -2012,6 +2041,9 @@ module Make Input> { /** Holds if the value of `nodeTo` is given by `nodeFrom`. */ predicate localMustFlowStep(SourceVariable v, Node nodeFrom, Node nodeTo) { exists(Definition def | + // Flow from write definition source into SSA definition + nodeFrom = TWriteDefSource(def) + or // Flow from assignment into SSA definition DfInput::ssaDefAssigns(def, nodeFrom.(ExprNode).getExpr()) or From 4c420c5bae8cf1b2d3ba392a3c08f4d75ba34f0f Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Thu, 27 Mar 2025 13:11:40 +0100 Subject: [PATCH 02/10] Java: Switch from ssaDefAssigns/ssaDefInitializesParam to ssaDefHasSource. --- .../java/dataflow/internal/DataFlowNodes.qll | 15 ++++++++++++++- .../code/java/dataflow/internal/SsaImpl.qll | 18 +++++------------- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowNodes.qll b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowNodes.qll index 7e1b10f58e3..7778f6ebc35 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowNodes.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowNodes.qll @@ -26,6 +26,14 @@ private predicate deadcode(Expr e) { module SsaFlow { module Impl = SsaImpl::DataFlowIntegration; + private predicate ssaDefAssigns(SsaExplicitUpdate def, Expr value) { + exists(VariableUpdate upd | upd = def.getDefiningExpr() | + value = upd.(VariableAssign).getSource() or + value = upd.(AssignOp) or + value = upd.(RecordBindingVariableExpr) + ) + } + Impl::Node asNode(Node n) { n = TSsaNode(result) or @@ -33,7 +41,12 @@ module SsaFlow { or result.(Impl::ExprPostUpdateNode).getExpr() = n.(PostUpdateNode).getPreUpdateNode().asExpr() or - TExplicitParameterNode(result.(Impl::ParameterNode).getParameter()) = n + exists(Parameter p | + n = TExplicitParameterNode(p) and + result.(Impl::WriteDefSourceNode).getDefinition().(SsaImplicitInit).isParameterDefinition(p) + ) + or + ssaDefAssigns(result.(Impl::WriteDefSourceNode).getDefinition(), n.asExpr()) } predicate localFlowStep(SsaSourceVariable v, Node nodeFrom, Node nodeTo, boolean isUseStep) { diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/SsaImpl.qll b/java/ql/lib/semmle/code/java/dataflow/internal/SsaImpl.qll index 573f1cb51ea..91750597e2b 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/SsaImpl.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/SsaImpl.qll @@ -649,21 +649,13 @@ private module DataFlowIntegrationInput implements Impl::DataFlowIntegrationInpu class Parameter = J::Parameter; - predicate ssaDefAssigns(Impl::WriteDefinition def, Expr value) { - exists(VariableUpdate upd | upd = def.(SsaExplicitUpdate).getDefiningExpr() | - value = upd.(VariableAssign).getSource() or - value = upd.(AssignOp) or - value = upd.(RecordBindingVariableExpr) - ) + predicate ssaDefHasSource(WriteDefinition def) { + def instanceof SsaExplicitUpdate or def.(SsaImplicitInit).isParameterDefinition(_) } - predicate ssaDefInitializesParam(Impl::WriteDefinition def, Parameter p) { - def.(SsaImplicitInit).getSourceVariable() = - any(SsaSourceVariable v | - v.getVariable() = p and - v.getEnclosingCallable() = p.getCallable() - ) - } + predicate ssaDefAssigns(Impl::WriteDefinition def, Expr value) { none() } + + predicate ssaDefInitializesParam(Impl::WriteDefinition def, Parameter p) { none() } predicate allowFlowIntoUncertainDef(UncertainWriteDefinition def) { def instanceof SsaUncertainImplicitUpdate From dafed9f465213683f31d7c81d5c520e469859417 Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Thu, 27 Mar 2025 13:12:17 +0100 Subject: [PATCH 03/10] Rust: Remove dead code. --- rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll | 6 ------ 1 file changed, 6 deletions(-) diff --git a/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll b/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll index d0c60dc8d51..f0f25d41264 100644 --- a/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll +++ b/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll @@ -172,10 +172,6 @@ predicate isArgumentForCall(ExprCfgNode arg, CallExprBaseCfgNode call, Parameter module SsaFlow { private module SsaFlow = SsaImpl::DataFlowIntegration; - private ParameterNode toParameterNode(ParamCfgNode p) { - result.(SourceParameterNode).getParameter() = p - } - /** Converts a control flow node into an SSA control flow node. */ SsaFlow::Node asNode(Node n) { n = TSsaNode(result) @@ -183,8 +179,6 @@ module SsaFlow { result.(SsaFlow::ExprNode).getExpr() = n.asExpr() or result.(SsaFlow::ExprPostUpdateNode).getExpr() = n.(PostUpdateNode).getPreUpdateNode().asExpr() - or - n = toParameterNode(result.(SsaFlow::ParameterNode).getParameter()) } predicate localFlowStep( From 8aedd63b9ef6c4ffda6fffe6b54278eb4ea212a4 Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Thu, 27 Mar 2025 13:12:34 +0100 Subject: [PATCH 04/10] Rust: Add ssaDefHasSource. --- rust/ql/lib/codeql/rust/dataflow/internal/SsaImpl.qll | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rust/ql/lib/codeql/rust/dataflow/internal/SsaImpl.qll b/rust/ql/lib/codeql/rust/dataflow/internal/SsaImpl.qll index 446a35ac68e..fe88826e055 100644 --- a/rust/ql/lib/codeql/rust/dataflow/internal/SsaImpl.qll +++ b/rust/ql/lib/codeql/rust/dataflow/internal/SsaImpl.qll @@ -340,6 +340,8 @@ private module DataFlowIntegrationInput implements Impl::DataFlowIntegrationInpu Expr getARead(Definition def) { result = Cached::getARead(def) } + predicate ssaDefHasSource(WriteDefinition def) { none() } // handled in `DataFlowImpl.qll` instead + /** Holds if SSA definition `def` assigns `value` to the underlying variable. */ predicate ssaDefAssigns(WriteDefinition def, Expr value) { none() // handled in `DataFlowImpl.qll` instead From 25297cb2b6650ce55f4f2d0b52c454ea8760f14c Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Thu, 27 Mar 2025 15:21:48 +0100 Subject: [PATCH 05/10] Ruby: Switch from ssaDefAssigns/ssaDefInitializesParam to WriteDefSourceNode. --- .../lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll | 7 ++++++- ruby/ql/lib/codeql/ruby/dataflow/internal/SsaImpl.qll | 6 ++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll index baf36bd9639..da0c0379717 100644 --- a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll +++ b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll @@ -108,7 +108,12 @@ module SsaFlow { or result.(Impl::ExprPostUpdateNode).getExpr() = n.(PostUpdateNode).getPreUpdateNode().asExpr() or - n = toParameterNode(result.(Impl::ParameterNode).getParameter()) + exists(SsaImpl::ParameterExt p | + n = toParameterNode(p) and + p.isInitializedBy(result.(Impl::WriteDefSourceNode).getDefinition()) + ) + or + result.(Impl::WriteDefSourceNode).getDefinition().(Ssa::WriteDefinition).assigns(n.asExpr()) } predicate localFlowStep( diff --git a/ruby/ql/lib/codeql/ruby/dataflow/internal/SsaImpl.qll b/ruby/ql/lib/codeql/ruby/dataflow/internal/SsaImpl.qll index 89677990b8b..57098b5bcdd 100644 --- a/ruby/ql/lib/codeql/ruby/dataflow/internal/SsaImpl.qll +++ b/ruby/ql/lib/codeql/ruby/dataflow/internal/SsaImpl.qll @@ -481,11 +481,9 @@ private module DataFlowIntegrationInput implements Impl::DataFlowIntegrationInpu Expr getARead(Definition def) { result = Cached::getARead(def) } - predicate ssaDefAssigns(WriteDefinition def, Expr value) { - def.(Ssa::WriteDefinition).assigns(value) - } + predicate ssaDefAssigns(WriteDefinition def, Expr value) { none() } - predicate ssaDefInitializesParam(WriteDefinition def, Parameter p) { p.isInitializedBy(def) } + predicate ssaDefInitializesParam(WriteDefinition def, Parameter p) { none() } class Guard extends Cfg::CfgNodes::AstCfgNode { /** From d8e14a6b5545aacad2c387468591f20568dc4711 Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Thu, 27 Mar 2025 15:24:20 +0100 Subject: [PATCH 06/10] JS: Add ssaDefHasSource. --- .../semmle/javascript/dataflow/internal/sharedlib/Ssa.qll | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/javascript/ql/lib/semmle/javascript/dataflow/internal/sharedlib/Ssa.qll b/javascript/ql/lib/semmle/javascript/dataflow/internal/sharedlib/Ssa.qll index cb5757ab30d..9de4be22f5b 100644 --- a/javascript/ql/lib/semmle/javascript/dataflow/internal/sharedlib/Ssa.qll +++ b/javascript/ql/lib/semmle/javascript/dataflow/internal/sharedlib/Ssa.qll @@ -56,6 +56,11 @@ module SsaDataflowInput implements DataFlowIntegrationInputSig { predicate hasCfgNode(js::BasicBlock bb, int i) { this = bb.getNode(i) } } + predicate ssaDefHasSource(WriteDefinition def) { + // This library only handles use-use flow after a post-update, there are no definitions, only uses. + none() + } + predicate ssaDefAssigns(WriteDefinition def, Expr value) { // This library only handles use-use flow after a post-update, there are no definitions, only uses. none() From 6e9ebca977d3a0aec52af14830271c0bb4a1ad96 Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Thu, 27 Mar 2025 15:32:11 +0100 Subject: [PATCH 07/10] C#: Switch from ssaDefAssigns/ssaDefInitializesParam to ssaDefHasSource. --- .../code/csharp/dataflow/internal/DataFlowPrivate.qll | 2 +- .../lib/semmle/code/csharp/dataflow/internal/SsaImpl.qll | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll index c5e915b0535..ff2bf709251 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll @@ -506,7 +506,7 @@ module SsaFlow { result.(Impl::ExprPostUpdateNode).getExpr() = n.(PostUpdateNode).getPreUpdateNode().(ExprNode).getControlFlowNode() or - result.(Impl::ParameterNode).getParameter() = n.(ExplicitParameterNode).getSsaDefinition() + result.(Impl::WriteDefSourceNode).getDefinition() = n.(ExplicitParameterNode).getSsaDefinition() } predicate localFlowStep(Ssa::SourceVariable v, Node nodeFrom, Node nodeTo, boolean isUseStep) { diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/SsaImpl.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/SsaImpl.qll index 420cf518e80..4ca00bfcc6e 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/SsaImpl.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/SsaImpl.qll @@ -1023,6 +1023,12 @@ private module DataFlowIntegrationInput implements Impl::DataFlowIntegrationInpu Expr getARead(Definition def) { exists(getAReadAtNode(def, result)) } + predicate ssaDefHasSource(WriteDefinition def) { + // exclude flow directly from RHS to SSA definition, as we instead want to + // go from RHS to matching assignable definition, and from there to SSA definition + def instanceof Ssa::ImplicitParameterDefinition + } + predicate ssaDefAssigns(WriteDefinition def, Expr value) { // exclude flow directly from RHS to SSA definition, as we instead want to // go from RHS to matching assingnable definition, and from there to SSA definition @@ -1031,7 +1037,7 @@ private module DataFlowIntegrationInput implements Impl::DataFlowIntegrationInpu class Parameter = Ssa::ImplicitParameterDefinition; - predicate ssaDefInitializesParam(WriteDefinition def, Parameter p) { def = p } + predicate ssaDefInitializesParam(WriteDefinition def, Parameter p) { none() } /** * Allows for flow into uncertain defintions that are not call definitions, From 308d15401fb73453b2463e00f6edc19273915a19 Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Thu, 27 Mar 2025 15:38:19 +0100 Subject: [PATCH 08/10] C++: Add ssaDefHasSource. --- .../lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll index 9cf8f8806a2..52b20739af2 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll @@ -977,6 +977,8 @@ private module DataFlowIntegrationInput implements SsaImpl::DataFlowIntegrationI ) } + predicate ssaDefHasSource(SsaImpl::WriteDefinition def) { none() } + predicate ssaDefAssigns(SsaImpl::WriteDefinition def, Expr value) { none() } class Parameter extends Void { From 5a986f532777643d3b0e111eac324c8c72babcdb Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Thu, 27 Mar 2025 16:03:00 +0100 Subject: [PATCH 09/10] SSA: Remove empty predicates and dead code. --- .../cpp/ir/dataflow/internal/SsaInternals.qll | 10 --- .../code/csharp/dataflow/internal/SsaImpl.qll | 10 --- .../code/java/dataflow/internal/SsaImpl.qll | 6 -- .../dataflow/internal/sharedlib/Ssa.qll | 12 ---- .../codeql/ruby/dataflow/internal/SsaImpl.qll | 6 -- .../codeql/rust/dataflow/internal/SsaImpl.qll | 12 ---- shared/ssa/codeql/ssa/Ssa.qll | 61 +------------------ 7 files changed, 3 insertions(+), 114 deletions(-) diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll index 52b20739af2..202d3fa32c8 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll @@ -956,8 +956,6 @@ class GlobalDef extends Definition { private module SsaImpl = SsaImplCommon::Make; private module DataFlowIntegrationInput implements SsaImpl::DataFlowIntegrationInputSig { - private import codeql.util.Void - class Expr extends Instruction { Expr() { exists(IRBlock bb, int i | @@ -979,14 +977,6 @@ private module DataFlowIntegrationInput implements SsaImpl::DataFlowIntegrationI predicate ssaDefHasSource(SsaImpl::WriteDefinition def) { none() } - predicate ssaDefAssigns(SsaImpl::WriteDefinition def, Expr value) { none() } - - class Parameter extends Void { - Location getLocation() { none() } - } - - predicate ssaDefInitializesParam(SsaImpl::WriteDefinition def, Parameter p) { none() } - predicate allowFlowIntoUncertainDef(SsaImpl::UncertainWriteDefinition def) { any() } private EdgeKind getConditionalEdge(boolean branch) { diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/SsaImpl.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/SsaImpl.qll index 4ca00bfcc6e..ad7a2aba911 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/SsaImpl.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/SsaImpl.qll @@ -1029,16 +1029,6 @@ private module DataFlowIntegrationInput implements Impl::DataFlowIntegrationInpu def instanceof Ssa::ImplicitParameterDefinition } - predicate ssaDefAssigns(WriteDefinition def, Expr value) { - // exclude flow directly from RHS to SSA definition, as we instead want to - // go from RHS to matching assingnable definition, and from there to SSA definition - none() - } - - class Parameter = Ssa::ImplicitParameterDefinition; - - predicate ssaDefInitializesParam(WriteDefinition def, Parameter p) { none() } - /** * Allows for flow into uncertain defintions that are not call definitions, * as we, conservatively, consider such definitions to be certain. diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/SsaImpl.qll b/java/ql/lib/semmle/code/java/dataflow/internal/SsaImpl.qll index 91750597e2b..b5a42a97569 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/SsaImpl.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/SsaImpl.qll @@ -647,16 +647,10 @@ private module DataFlowIntegrationInput implements Impl::DataFlowIntegrationInpu Expr getARead(Definition def) { result = getAUse(def) } - class Parameter = J::Parameter; - predicate ssaDefHasSource(WriteDefinition def) { def instanceof SsaExplicitUpdate or def.(SsaImplicitInit).isParameterDefinition(_) } - predicate ssaDefAssigns(Impl::WriteDefinition def, Expr value) { none() } - - predicate ssaDefInitializesParam(Impl::WriteDefinition def, Parameter p) { none() } - predicate allowFlowIntoUncertainDef(UncertainWriteDefinition def) { def instanceof SsaUncertainImplicitUpdate } diff --git a/javascript/ql/lib/semmle/javascript/dataflow/internal/sharedlib/Ssa.qll b/javascript/ql/lib/semmle/javascript/dataflow/internal/sharedlib/Ssa.qll index 9de4be22f5b..adc4a79dd04 100644 --- a/javascript/ql/lib/semmle/javascript/dataflow/internal/sharedlib/Ssa.qll +++ b/javascript/ql/lib/semmle/javascript/dataflow/internal/sharedlib/Ssa.qll @@ -61,18 +61,6 @@ module SsaDataflowInput implements DataFlowIntegrationInputSig { none() } - predicate ssaDefAssigns(WriteDefinition def, Expr value) { - // This library only handles use-use flow after a post-update, there are no definitions, only uses. - none() - } - - class Parameter = js::Parameter; - - predicate ssaDefInitializesParam(WriteDefinition def, Parameter p) { - // This library only handles use-use flow after a post-update, there are no definitions, only uses. - none() - } - cached Expr getARead(Definition def) { // Copied from implementation so we can cache it here diff --git a/ruby/ql/lib/codeql/ruby/dataflow/internal/SsaImpl.qll b/ruby/ql/lib/codeql/ruby/dataflow/internal/SsaImpl.qll index 57098b5bcdd..b34577602f7 100644 --- a/ruby/ql/lib/codeql/ruby/dataflow/internal/SsaImpl.qll +++ b/ruby/ql/lib/codeql/ruby/dataflow/internal/SsaImpl.qll @@ -473,18 +473,12 @@ class ParameterExt extends TParameterExt { private module DataFlowIntegrationInput implements Impl::DataFlowIntegrationInputSig { private import codeql.ruby.controlflow.internal.Guards as Guards - class Parameter = ParameterExt; - class Expr extends Cfg::CfgNodes::ExprCfgNode { predicate hasCfgNode(SsaInput::BasicBlock bb, int i) { this = bb.getNode(i) } } Expr getARead(Definition def) { result = Cached::getARead(def) } - predicate ssaDefAssigns(WriteDefinition def, Expr value) { none() } - - predicate ssaDefInitializesParam(WriteDefinition def, Parameter p) { none() } - class Guard extends Cfg::CfgNodes::AstCfgNode { /** * Holds if the control flow branching from `bb1` is dependent on this guard, diff --git a/rust/ql/lib/codeql/rust/dataflow/internal/SsaImpl.qll b/rust/ql/lib/codeql/rust/dataflow/internal/SsaImpl.qll index fe88826e055..dcfe4f0edaf 100644 --- a/rust/ql/lib/codeql/rust/dataflow/internal/SsaImpl.qll +++ b/rust/ql/lib/codeql/rust/dataflow/internal/SsaImpl.qll @@ -342,11 +342,6 @@ private module DataFlowIntegrationInput implements Impl::DataFlowIntegrationInpu predicate ssaDefHasSource(WriteDefinition def) { none() } // handled in `DataFlowImpl.qll` instead - /** Holds if SSA definition `def` assigns `value` to the underlying variable. */ - predicate ssaDefAssigns(WriteDefinition def, Expr value) { - none() // handled in `DataFlowImpl.qll` instead - } - private predicate isArg(CfgNodes::CallExprBaseCfgNode call, CfgNodes::ExprCfgNode e) { call.getArgument(_) = e or @@ -366,13 +361,6 @@ private module DataFlowIntegrationInput implements Impl::DataFlowIntegrationInpu ) } - class Parameter = CfgNodes::ParamBaseCfgNode; - - /** Holds if SSA definition `def` initializes parameter `p` at function entry. */ - predicate ssaDefInitializesParam(WriteDefinition def, Parameter p) { - none() // handled in `DataFlowImpl.qll` instead - } - class Guard extends CfgNodes::AstCfgNode { /** * Holds if the control flow branching from `bb1` is dependent on this guard, diff --git a/shared/ssa/codeql/ssa/Ssa.qll b/shared/ssa/codeql/ssa/Ssa.qll index 62dc4230573..0e70740576f 100644 --- a/shared/ssa/codeql/ssa/Ssa.qll +++ b/shared/ssa/codeql/ssa/Ssa.qll @@ -1468,21 +1468,6 @@ module Make Input> { */ default predicate ssaDefHasSource(WriteDefinition def) { any() } - /** Holds if SSA definition `def` assigns `value` to the underlying variable. */ - predicate ssaDefAssigns(WriteDefinition def, Expr value); - - /** A parameter. */ - class Parameter { - /** Gets a textual representation of this parameter. */ - string toString(); - - /** Gets the location of this parameter. */ - Location getLocation(); - } - - /** Holds if SSA definition `def` initializes parameter `p` at function entry. */ - predicate ssaDefInitializesParam(WriteDefinition def, Parameter p); - /** * Holds if flow should be allowed into uncertain SSA definition `def` from * previous definitions or reads. @@ -1675,17 +1660,7 @@ module Make Input> { cached private newtype TNode = TWriteDefSource(WriteDefinition def) { DfInput::ssaDefHasSource(def) } or - TParamNode(DfInput::Parameter p) { - exists(WriteDefinition def | DfInput::ssaDefInitializesParam(def, p)) - } or - TExprNode(DfInput::Expr e, Boolean isPost) { - e = DfInput::getARead(_) - or - exists(DefinitionExt def | - DfInput::ssaDefAssigns(def, e) and - isPost = false - ) - } or + TExprNode(DfInput::Expr e, Boolean isPost) { e = DfInput::getARead(_) } or TSsaDefinitionNode(DefinitionExt def) { not phiHasUniqNextNode(def) } or TSsaInputNode(SsaPhiExt phi, BasicBlock input) { relevantPhiInputNode(phi, input) } @@ -1722,22 +1697,6 @@ module Make Input> { final class WriteDefSourceNode = WriteDefSourceNodeImpl; - /** A parameter node. */ - private class ParameterNodeImpl extends NodeImpl, TParamNode { - private DfInput::Parameter p; - - ParameterNodeImpl() { this = TParamNode(p) } - - /** Gets the underlying parameter. */ - DfInput::Parameter getParameter() { result = p } - - override string toString() { result = p.toString() } - - override Location getLocation() { result = p.getLocation() } - } - - final class ParameterNode = ParameterNodeImpl; - /** A (post-update) expression node. */ abstract private class ExprNodePreOrPostImpl extends NodeImpl, TExprNode { DfInput::Expr e; @@ -2003,14 +1962,7 @@ module Make Input> { predicate localFlowStep(SourceVariable v, Node nodeFrom, Node nodeTo, boolean isUseStep) { exists(Definition def | // Flow from write definition source into SSA definition - nodeFrom = TWriteDefSource(def) - or - // Flow from assignment into SSA definition - DfInput::ssaDefAssigns(def, nodeFrom.(ExprNode).getExpr()) - or - // Flow from parameter into entry definition - DfInput::ssaDefInitializesParam(def, nodeFrom.(ParameterNode).getParameter()) - | + nodeFrom = TWriteDefSource(def) and isUseStep = false and if DfInput::includeWriteDefsInFlowStep() then @@ -2042,14 +1994,7 @@ module Make Input> { predicate localMustFlowStep(SourceVariable v, Node nodeFrom, Node nodeTo) { exists(Definition def | // Flow from write definition source into SSA definition - nodeFrom = TWriteDefSource(def) - or - // Flow from assignment into SSA definition - DfInput::ssaDefAssigns(def, nodeFrom.(ExprNode).getExpr()) - or - // Flow from parameter into entry definition - DfInput::ssaDefInitializesParam(def, nodeFrom.(ParameterNode).getParameter()) - | + nodeFrom = TWriteDefSource(def) and v = def.getSourceVariable() and if DfInput::includeWriteDefsInFlowStep() then nodeTo.(SsaDefinitionNode).getDefinition() = def From 0d1ac7789b13f3b9f4ed2cdecf5c08e89796572b Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Fri, 28 Mar 2025 13:27:56 +0100 Subject: [PATCH 10/10] SSA/Ruby: Address review comments. --- ruby/ql/lib/codeql/ruby/dataflow/internal/SsaImpl.qll | 4 ++++ shared/ssa/codeql/ssa/Ssa.qll | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/ruby/ql/lib/codeql/ruby/dataflow/internal/SsaImpl.qll b/ruby/ql/lib/codeql/ruby/dataflow/internal/SsaImpl.qll index b34577602f7..3c1da6f3013 100644 --- a/ruby/ql/lib/codeql/ruby/dataflow/internal/SsaImpl.qll +++ b/ruby/ql/lib/codeql/ruby/dataflow/internal/SsaImpl.qll @@ -479,6 +479,10 @@ private module DataFlowIntegrationInput implements Impl::DataFlowIntegrationInpu Expr getARead(Definition def) { result = Cached::getARead(def) } + predicate ssaDefHasSource(WriteDefinition def) { + any(ParameterExt p).isInitializedBy(def) or def.(Ssa::WriteDefinition).assigns(_) + } + class Guard extends Cfg::CfgNodes::AstCfgNode { /** * Holds if the control flow branching from `bb1` is dependent on this guard, diff --git a/shared/ssa/codeql/ssa/Ssa.qll b/shared/ssa/codeql/ssa/Ssa.qll index 0e70740576f..2bbdb6e2a47 100644 --- a/shared/ssa/codeql/ssa/Ssa.qll +++ b/shared/ssa/codeql/ssa/Ssa.qll @@ -1690,7 +1690,7 @@ module Make Input> { /** Gets the underlying definition. */ WriteDefinition getDefinition() { result = def } - override string toString() { result = def.toString() } + override string toString() { result = "[source] " + def.toString() } override Location getLocation() { result = def.getLocation() } }