C#/Java/Ruby/Rust/SSA: Replace DefinitionExt with SourceVariable in d…

…ata flow integration predicates.

csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll

@@ -456,9 +456,9 @@ module VariableCapture {
     Flow::clearsContent(asClosureNode(node), getCapturedVariableContent(c))
   }
 
-  class CapturedSsaDefinitionExt extends SsaImpl::DefinitionExt {
-    CapturedSsaDefinitionExt() {
-      this.getSourceVariable().getAssignable() = any(CapturedVariable v).asLocalScopeVariable()
+  class CapturedSsaSourceVariable extends Ssa::SourceVariable {
+    CapturedSsaSourceVariable() {
+      this.getAssignable() = any(CapturedVariable v).asLocalScopeVariable()
     }
   }
 
@@ -509,12 +509,12 @@ module SsaFlow {
     result.(Impl::ParameterNode).getParameter() = n.(ExplicitParameterNode).getSsaDefinition()
   }
 
-  predicate localFlowStep(SsaImpl::DefinitionExt def, Node nodeFrom, Node nodeTo, boolean isUseStep) {
-    Impl::localFlowStep(def, asNode(nodeFrom), asNode(nodeTo), isUseStep)
+  predicate localFlowStep(Ssa::SourceVariable v, Node nodeFrom, Node nodeTo, boolean isUseStep) {
+    Impl::localFlowStep(v, asNode(nodeFrom), asNode(nodeTo), isUseStep)
   }
 
-  predicate localMustFlowStep(SsaImpl::DefinitionExt def, Node nodeFrom, Node nodeTo) {
-    Impl::localMustFlowStep(def, asNode(nodeFrom), asNode(nodeTo))
+  predicate localMustFlowStep(Ssa::SourceVariable v, Node nodeFrom, Node nodeTo) {
+    Impl::localMustFlowStep(v, asNode(nodeFrom), asNode(nodeTo))
   }
 }
 
@@ -644,12 +644,10 @@ module LocalFlow {
   }
 
   /**
-   * Holds if the source variable of SSA definition `def` is an instance field.
+   * Holds if the source variable `v` is an instance field.
    */
-  predicate usesInstanceField(SsaImpl::DefinitionExt def) {
-    exists(Ssa::SourceVariables::FieldOrPropSourceVariable fp | fp = def.getSourceVariable() |
-      not fp.getAssignable().(Modifiable).isStatic()
-    )
+  predicate isInstanceField(Ssa::SourceVariables::FieldOrPropSourceVariable v) {
+    not v.getAssignable().(Modifiable).isStatic()
   }
 
   predicate localFlowStepCommon(Node nodeFrom, Node nodeTo) {
@@ -749,10 +747,10 @@ predicate simpleLocalFlowStep(Node nodeFrom, Node nodeTo, string model) {
   (
     LocalFlow::localFlowStepCommon(nodeFrom, nodeTo)
     or
-    exists(SsaImpl::DefinitionExt def, boolean isUseStep |
-      SsaFlow::localFlowStep(def, nodeFrom, nodeTo, isUseStep) and
-      not LocalFlow::usesInstanceField(def) and
-      not def instanceof VariableCapture::CapturedSsaDefinitionExt
+    exists(Ssa::SourceVariable v, boolean isUseStep |
+      SsaFlow::localFlowStep(v, nodeFrom, nodeTo, isUseStep) and
+      not LocalFlow::isInstanceField(v) and
+      not v instanceof VariableCapture::CapturedSsaSourceVariable
     |
       isUseStep = false
       or
@@ -3007,13 +3005,13 @@ private predicate delegateCreationStep(Node nodeFrom, Node nodeTo) {
 
 /** Extra data-flow steps needed for lambda flow analysis. */
 predicate additionalLambdaFlowStep(Node nodeFrom, Node nodeTo, boolean preservesValue) {
-  exists(SsaImpl::DefinitionExt def |
-    SsaFlow::localFlowStep(def, nodeFrom, nodeTo, _) and
+  exists(Ssa::SourceVariable v |
+    SsaFlow::localFlowStep(v, nodeFrom, nodeTo, _) and
     preservesValue = true
   |
-    LocalFlow::usesInstanceField(def)
+    LocalFlow::isInstanceField(v)
     or
-    def instanceof VariableCapture::CapturedSsaDefinitionExt
+    v instanceof VariableCapture::CapturedSsaSourceVariable
   )
   or
   delegateCreationStep(nodeFrom, nodeTo) and

csharp/ql/lib/semmle/code/csharp/dataflow/internal/SsaImpl.qll

@@ -967,13 +967,13 @@ private module Cached {
     import DataFlowIntegrationImpl
 
     cached
-    predicate localFlowStep(DefinitionExt def, Node nodeFrom, Node nodeTo, boolean isUseStep) {
-      DataFlowIntegrationImpl::localFlowStep(def, nodeFrom, nodeTo, isUseStep)
+    predicate localFlowStep(Ssa::SourceVariable v, Node nodeFrom, Node nodeTo, boolean isUseStep) {
+      DataFlowIntegrationImpl::localFlowStep(v, nodeFrom, nodeTo, isUseStep)
     }
 
     cached
-    predicate localMustFlowStep(DefinitionExt def, Node nodeFrom, Node nodeTo) {
-      DataFlowIntegrationImpl::localMustFlowStep(def, nodeFrom, nodeTo)
+    predicate localMustFlowStep(Ssa::SourceVariable v, Node nodeFrom, Node nodeTo) {
+      DataFlowIntegrationImpl::localMustFlowStep(v, nodeFrom, nodeTo)
     }
 
     signature predicate guardChecksSig(Guards::Guard g, Expr e, Guards::AbstractValue v);

java/ql/lib/semmle/code/java/dataflow/internal/DataFlowNodes.qll

@@ -36,14 +36,12 @@ module SsaFlow {
     TExplicitParameterNode(result.(Impl::ParameterNode).getParameter()) = n
   }
 
-  predicate localFlowStep(
-    SsaImpl::Impl::DefinitionExt def, Node nodeFrom, Node nodeTo, boolean isUseStep
-  ) {
-    Impl::localFlowStep(def, asNode(nodeFrom), asNode(nodeTo), isUseStep)
+  predicate localFlowStep(SsaSourceVariable v, Node nodeFrom, Node nodeTo, boolean isUseStep) {
+    Impl::localFlowStep(v, asNode(nodeFrom), asNode(nodeTo), isUseStep)
   }
 
-  predicate localMustFlowStep(SsaImpl::Impl::DefinitionExt def, Node nodeFrom, Node nodeTo) {
-    Impl::localMustFlowStep(def, asNode(nodeFrom), asNode(nodeTo))
+  predicate localMustFlowStep(Node nodeFrom, Node nodeTo) {
+    Impl::localMustFlowStep(_, asNode(nodeFrom), asNode(nodeTo))
   }
 }
 

java/ql/lib/semmle/code/java/dataflow/internal/DataFlowUtil.qll

@@ -168,7 +168,7 @@ predicate localMustFlowStep(Node node1, Node node2) {
       node2.(ImplicitInstanceAccess).getInstanceAccess().(OwnInstanceAccess).getEnclosingCallable()
   )
   or
-  SsaFlow::localMustFlowStep(_, node1, node2)
+  SsaFlow::localMustFlowStep(node1, node2)
   or
   node2.asExpr().(CastingExpr).getExpr() = node1.asExpr()
   or

java/ql/lib/semmle/code/java/dataflow/internal/SsaImpl.qll

@@ -544,15 +544,13 @@ private module Cached {
     import DataFlowIntegrationImpl
 
     cached
-    predicate localFlowStep(Impl::DefinitionExt def, Node nodeFrom, Node nodeTo, boolean isUseStep) {
-      not def instanceof UntrackedDef and
-      DataFlowIntegrationImpl::localFlowStep(def, nodeFrom, nodeTo, isUseStep)
+    predicate localFlowStep(TrackedVar v, Node nodeFrom, Node nodeTo, boolean isUseStep) {
+      DataFlowIntegrationImpl::localFlowStep(v, nodeFrom, nodeTo, isUseStep)
     }
 
     cached
-    predicate localMustFlowStep(Impl::DefinitionExt def, Node nodeFrom, Node nodeTo) {
-      not def instanceof UntrackedDef and
-      DataFlowIntegrationImpl::localMustFlowStep(def, nodeFrom, nodeTo)
+    predicate localMustFlowStep(TrackedVar v, Node nodeFrom, Node nodeTo) {
+      DataFlowIntegrationImpl::localMustFlowStep(v, nodeFrom, nodeTo)
     }
 
     signature predicate guardChecksSig(Guards::Guard g, Expr e, boolean branch);

ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll

@@ -111,12 +111,14 @@ module SsaFlow {
     n = toParameterNode(result.(Impl::ParameterNode).getParameter())
   }
 
-  predicate localFlowStep(SsaImpl::DefinitionExt def, Node nodeFrom, Node nodeTo, boolean isUseStep) {
-    Impl::localFlowStep(def, asNode(nodeFrom), asNode(nodeTo), isUseStep)
+  predicate localFlowStep(
+    SsaImpl::SsaInput::SourceVariable v, Node nodeFrom, Node nodeTo, boolean isUseStep
+  ) {
+    Impl::localFlowStep(v, asNode(nodeFrom), asNode(nodeTo), isUseStep)
   }
 
-  predicate localMustFlowStep(SsaImpl::DefinitionExt def, Node nodeFrom, Node nodeTo) {
-    Impl::localMustFlowStep(def, asNode(nodeFrom), asNode(nodeTo))
+  predicate localMustFlowStep(Node nodeFrom, Node nodeTo) {
+    Impl::localMustFlowStep(_, asNode(nodeFrom), asNode(nodeTo))
   }
 }
 
@@ -175,7 +177,7 @@ module LocalFlow {
   }
 
   predicate localMustFlowStep(Node node1, Node node2) {
-    SsaFlow::localMustFlowStep(_, node1, node2)
+    SsaFlow::localMustFlowStep(node1, node2)
     or
     node1.asExpr() = node2.asExpr().(CfgNodes::ExprNodes::AssignExprCfgNode).getRhs()
     or
@@ -525,10 +527,10 @@ private module Cached {
     (
       LocalFlow::localFlowStepCommon(nodeFrom, nodeTo)
       or
-      exists(SsaImpl::DefinitionExt def, boolean isUseStep |
-        SsaFlow::localFlowStep(def, nodeFrom, nodeTo, isUseStep) and
+      exists(SsaImpl::SsaInput::SourceVariable v, boolean isUseStep |
+        SsaFlow::localFlowStep(v, nodeFrom, nodeTo, isUseStep) and
         // captured variables are handled by the shared `VariableCapture` library
-        not def instanceof VariableCapture::CapturedSsaDefinitionExt
+        not v instanceof VariableCapture::CapturedVariable
       |
         isUseStep = false
         or

ruby/ql/lib/codeql/ruby/dataflow/internal/SsaImpl.qll

@@ -387,13 +387,13 @@ private module Cached {
     import DataFlowIntegrationImpl
 
     cached
-    predicate localFlowStep(DefinitionExt def, Node nodeFrom, Node nodeTo, boolean isUseStep) {
-      DataFlowIntegrationImpl::localFlowStep(def, nodeFrom, nodeTo, isUseStep)
+    predicate localFlowStep(SsaInput::SourceVariable v, Node nodeFrom, Node nodeTo, boolean isUseStep) {
+      DataFlowIntegrationImpl::localFlowStep(v, nodeFrom, nodeTo, isUseStep)
     }
 
     cached
-    predicate localMustFlowStep(DefinitionExt def, Node nodeFrom, Node nodeTo) {
-      DataFlowIntegrationImpl::localMustFlowStep(def, nodeFrom, nodeTo)
+    predicate localMustFlowStep(SsaInput::SourceVariable v, Node nodeFrom, Node nodeTo) {
+      DataFlowIntegrationImpl::localMustFlowStep(v, nodeFrom, nodeTo)
     }
 
     signature predicate guardChecksSig(Cfg::CfgNodes::AstCfgNode g, Cfg::CfgNode e, boolean branch);

rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll

@@ -608,12 +608,14 @@ module SsaFlow {
     n = toParameterNode(result.(SsaFlow::ParameterNode).getParameter())
   }
 
-  predicate localFlowStep(SsaImpl::DefinitionExt def, Node nodeFrom, Node nodeTo, boolean isUseStep) {
-    SsaFlow::localFlowStep(def, asNode(nodeFrom), asNode(nodeTo), isUseStep)
+  predicate localFlowStep(
+    SsaImpl::SsaInput::SourceVariable v, Node nodeFrom, Node nodeTo, boolean isUseStep
+  ) {
+    SsaFlow::localFlowStep(v, asNode(nodeFrom), asNode(nodeTo), isUseStep)
   }
 
-  predicate localMustFlowStep(SsaImpl::DefinitionExt def, Node nodeFrom, Node nodeTo) {
-    SsaFlow::localMustFlowStep(def, asNode(nodeFrom), asNode(nodeTo))
+  predicate localMustFlowStep(Node nodeFrom, Node nodeTo) {
+    SsaFlow::localMustFlowStep(_, asNode(nodeFrom), asNode(nodeTo))
   }
 }
 
@@ -1205,9 +1207,9 @@ module RustDataFlow implements InputSig<Location> {
     (
       LocalFlow::localFlowStepCommon(nodeFrom, nodeTo)
       or
-      exists(SsaImpl::DefinitionExt def, boolean isUseStep |
-        SsaFlow::localFlowStep(def, nodeFrom, nodeTo, isUseStep) and
-        not def instanceof VariableCapture::CapturedSsaDefinitionExt
+      exists(SsaImpl::SsaInput::SourceVariable v, boolean isUseStep |
+        SsaFlow::localFlowStep(v, nodeFrom, nodeTo, isUseStep) and
+        not v instanceof VariableCapture::CapturedVariable
       |
         isUseStep = false
         or
@@ -1514,7 +1516,7 @@ module RustDataFlow implements InputSig<Location> {
    * must also apply to `node1`.
    */
   predicate localMustFlowStep(Node node1, Node node2) {
-    SsaFlow::localMustFlowStep(_, node1, node2)
+    SsaFlow::localMustFlowStep(node1, node2)
     or
     FlowSummaryImpl::Private::Steps::summaryLocalMustFlowStep(node1
           .(Node::FlowSummaryNode)
@@ -1688,10 +1690,6 @@ module VariableCapture {
   predicate clearsContent(Node node, CapturedVariableContent c) {
     Flow::clearsContent(asClosureNode(node), c.getVariable())
   }
-
-  class CapturedSsaDefinitionExt extends SsaImpl::DefinitionExt {
-    CapturedSsaDefinitionExt() { this.getSourceVariable() instanceof CapturedVariable }
-  }
 }
 
 import MakeImpl<Location, RustDataFlow>

rust/ql/lib/codeql/rust/dataflow/internal/SsaImpl.qll

@@ -303,13 +303,15 @@ private module Cached {
     import DataFlowIntegrationImpl
 
     cached
-    predicate localFlowStep(DefinitionExt def, Node nodeFrom, Node nodeTo, boolean isUseStep) {
-      DataFlowIntegrationImpl::localFlowStep(def, nodeFrom, nodeTo, isUseStep)
+    predicate localFlowStep(
+      SsaInput::SourceVariable v, Node nodeFrom, Node nodeTo, boolean isUseStep
+    ) {
+      DataFlowIntegrationImpl::localFlowStep(v, nodeFrom, nodeTo, isUseStep)
     }
 
     cached
-    predicate localMustFlowStep(DefinitionExt def, Node nodeFrom, Node nodeTo) {
-      DataFlowIntegrationImpl::localMustFlowStep(def, nodeFrom, nodeTo)
+    predicate localMustFlowStep(SsaInput::SourceVariable v, Node nodeFrom, Node nodeTo) {
+      DataFlowIntegrationImpl::localMustFlowStep(v, nodeFrom, nodeTo)
     }
 
     signature predicate guardChecksSig(CfgNodes::AstCfgNode g, Cfg::CfgNode e, boolean branch);

shared/ssa/codeql/ssa/Ssa.qll

@@ -1765,17 +1765,17 @@ module Make<LocationSig Location, InputSig<Location> Input> {
      * Holds if `nodeFrom` corresponds to the reference to `v` at index `i` in
      * `bb`. The boolean `isUseStep` indicates whether `nodeFrom` is an actual
      * read. If it is false then `nodeFrom` may be any of the following: an
-     * uncertain write, a certain write, a phi, or a phi read. `def` is the SSA
-     * definition that is read/defined at `nodeFrom`.
+     * uncertain write, a certain write, a phi, or a phi read.
      */
     private predicate flowOutOf(
-      DefinitionExt def, Node nodeFrom, SourceVariable v, BasicBlock bb, int i, boolean isUseStep
+      Node nodeFrom, SourceVariable v, BasicBlock bb, int i, boolean isUseStep
     ) {
-      nodeFrom.(SsaDefinitionExtNodeImpl).getDefExt() = def and
-      def.definesAt(v, bb, i, _) and
-      isUseStep = false
+      exists(DefinitionExt def |
+        nodeFrom.(SsaDefinitionExtNodeImpl).getDefExt() = def and
+        def.definesAt(v, bb, i, _) and
+        isUseStep = false
+      )
       or
-      ssaDefReachesReadExt(v, def, bb, i) and
       [nodeFrom, nodeFrom.(ExprPostUpdateNode).getPreUpdateNode()].(ReadNode).readsAt(bb, i, v) and
       isUseStep = true
     }
@@ -1786,27 +1786,29 @@ module Make<LocationSig Location, InputSig<Location> Input> {
      * `isUseStep` is `true` when `nodeFrom` is a (post-update) read node and
      * `nodeTo` is a read node or phi (read) node.
      */
-    predicate localFlowStep(DefinitionExt def, Node nodeFrom, Node nodeTo, boolean isUseStep) {
-      (
+    predicate localFlowStep(SourceVariable v, Node nodeFrom, Node nodeTo, boolean isUseStep) {
+      exists(Definition def |
         // 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())
-      ) and
-      nodeTo.(SsaDefinitionNode).getDefinition() = def and
-      isUseStep = false
+      |
+        nodeTo.(SsaDefinitionNode).getDefinition() = def and
+        v = def.getSourceVariable() and
+        isUseStep = false
+      )
       or
       // Flow from definition/read to next read
-      exists(SourceVariable v, BasicBlock bb1, int i1, BasicBlock bb2, int i2 |
-        flowOutOf(def, nodeFrom, v, bb1, i1, isUseStep) and
+      exists(BasicBlock bb1, int i1, BasicBlock bb2, int i2 |
+        flowOutOf(nodeFrom, v, bb1, i1, isUseStep) and
         AdjacentSsaRefs::adjacentRefRead(bb1, i1, bb2, i2, v) and
         nodeTo.(ReadNode).readsAt(bb2, i2, v)
       )
       or
       // Flow from definition/read to next uncertain write
-      exists(SourceVariable v, BasicBlock bb1, int i1, BasicBlock bb2, int i2 |
-        flowOutOf(def, nodeFrom, v, bb1, i1, isUseStep) and
+      exists(BasicBlock bb1, int i1, BasicBlock bb2, int i2 |
+        flowOutOf(nodeFrom, v, bb1, i1, isUseStep) and
         AdjacentSsaRefs::adjacentRefRead(bb1, i1, bb2, i2, v) and
         exists(UncertainWriteDefinition def2 |
           DfInput::allowFlowIntoUncertainDef(def2) and
@@ -1816,36 +1818,41 @@ module Make<LocationSig Location, InputSig<Location> Input> {
       )
       or
       // Flow from definition/read to phi input
-      exists(
-        SourceVariable v, BasicBlock bb, int i, BasicBlock input, BasicBlock bbPhi,
-        DefinitionExt phi
-      |
-        flowOutOf(def, nodeFrom, v, bb, i, isUseStep) and
+      exists(BasicBlock bb, int i, BasicBlock input, BasicBlock bbPhi, DefinitionExt phi |
+        flowOutOf(nodeFrom, v, bb, i, isUseStep) and
         AdjacentSsaRefs::adjacentRefPhi(bb, i, input, bbPhi, v) and
         nodeTo = TSsaInputNode(phi, input) and
         phi.definesAt(v, bbPhi, -1, _)
       )
       or
       // Flow from input node to def
-      nodeTo.(SsaDefinitionExtNodeImpl).getDefExt() = def and
-      def = nodeFrom.(SsaInputNodeImpl).getPhi() and
-      isUseStep = false
+      exists(DefinitionExt def |
+        nodeTo.(SsaDefinitionExtNodeImpl).getDefExt() = def and
+        def = nodeFrom.(SsaInputNodeImpl).getPhi() and
+        v = def.getSourceVariable() and
+        isUseStep = false
+      )
     }
 
     /** Holds if the value of `nodeTo` is given by `nodeFrom`. */
-    predicate localMustFlowStep(DefinitionExt def, Node nodeFrom, Node nodeTo) {
-      (
+    predicate localMustFlowStep(SourceVariable v, Node nodeFrom, Node nodeTo) {
+      exists(Definition def |
         // 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())
-      ) and
-      nodeTo.(SsaDefinitionNode).getDefinition() = def
+      |
+        nodeTo.(SsaDefinitionNode).getDefinition() = def and
+        v = def.getSourceVariable()
+      )
       or
       // Flow from SSA definition to read
-      nodeFrom.(SsaDefinitionExtNodeImpl).getDefExt() = def and
-      nodeTo.(ExprNode).getExpr() = DfInput::getARead(def)
+      exists(DefinitionExt def |
+        nodeFrom.(SsaDefinitionExtNodeImpl).getDefExt() = def and
+        nodeTo.(ExprNode).getExpr() = DfInput::getARead(def) and
+        v = def.getSourceVariable()
+      )
     }
 
     /**