Ssa: Update qltests including consistency checks

csharp/ql/consistency-queries/SsaConsistency.ql

@@ -3,22 +3,6 @@ import semmle.code.csharp.dataflow.internal.SsaImpl as Impl
 import Impl::Consistency
 import Ssa
 
-class MyRelevantDefinition extends RelevantDefinition, Ssa::Definition {
-  override predicate hasLocationInfo(
-    string filepath, int startline, int startcolumn, int endline, int endcolumn
-  ) {
-    this.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
-  }
-}
-
-class MyRelevantDefinitionExt extends RelevantDefinitionExt, Impl::DefinitionExt {
-  override predicate hasLocationInfo(
-    string filepath, int startline, int startcolumn, int endline, int endcolumn
-  ) {
-    this.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
-  }
-}
-
 query predicate localDeclWithSsaDef(LocalVariableDeclExpr d) {
   // Local variables in C# must be initialized before every use, so uninitialized
   // local variables should not have an SSA definition, as that would imply that

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

@@ -60,12 +60,6 @@ class PhiNode = Impl::PhiNode;
 
 module Consistency = Impl::Consistency;
 
-module ExposedForTestingOnly {
-  predicate ssaDefReachesReadExt = Impl::ssaDefReachesReadExt/4;
-
-  predicate phiHasInputFromBlockExt = Impl::phiHasInputFromBlockExt/3;
-}
-
 /**
  * Holds if the `i`th node of basic block `bb` reads source variable `v`.
  */
@@ -967,13 +961,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);
@@ -1000,9 +994,9 @@ private module Cached {
 
 import Cached
 
-private string getSplitString(DefinitionExt def) {
+private string getSplitString(Definition def) {
   exists(ControlFlow::BasicBlock bb, int i, ControlFlow::Node cfn |
-    def.definesAt(_, bb, i, _) and
+    def.definesAt(_, bb, i) and
     result = cfn.(ControlFlow::Nodes::ElementNode).getSplitsString()
   |
     cfn = bb.getNode(i)
@@ -1012,48 +1006,13 @@ private string getSplitString(DefinitionExt def) {
   )
 }
 
-string getToStringPrefix(DefinitionExt def) {
+string getToStringPrefix(Definition def) {
   result = "[" + getSplitString(def) + "] "
   or
   not exists(getSplitString(def)) and
   result = ""
 }
 
-/**
- * An extended static single assignment (SSA) definition.
- *
- * This is either a normal SSA definition (`Definition`) or a
- * phi-read node (`PhiReadNode`).
- *
- * Only intended for internal use.
- */
-class DefinitionExt extends Impl::DefinitionExt {
-  override string toString() { result = this.(Ssa::Definition).toString() }
-
-  /** Gets the location of this definition. */
-  override Location getLocation() { result = this.(Ssa::Definition).getLocation() }
-
-  /** Gets the enclosing callable of this definition. */
-  Callable getEnclosingCallable() { result = this.(Ssa::Definition).getEnclosingCallable() }
-}
-
-/**
- * A phi-read node.
- *
- * Only intended for internal use.
- */
-class PhiReadNode extends DefinitionExt, Impl::PhiReadNode {
-  override string toString() {
-    result = getToStringPrefix(this) + "SSA phi read(" + this.getSourceVariable() + ")"
-  }
-
-  override Location getLocation() { result = this.getBasicBlock().getLocation() }
-
-  override Callable getEnclosingCallable() {
-    result = this.getSourceVariable().getEnclosingCallable()
-  }
-}
-
 private module DataFlowIntegrationInput implements Impl::DataFlowIntegrationInputSig {
   private import csharp as Cs
   private import semmle.code.csharp.controlflow.BasicBlocks

csharp/ql/test/library-tests/dataflow/ssa/SSAPhiRead.expected

@@ -6,7 +6,7 @@ phiReadNode
 | Test.cs:25:16:25:16 | SSA phi read(x) | Test.cs:8:13:8:13 | x |
 | Test.cs:90:9:97:9 | SSA phi read(x) | Test.cs:78:13:78:13 | x |
 | Test.cs:99:9:99:15 | SSA phi read(x) | Test.cs:78:13:78:13 | x |
-phiReadNodeRead
+phiReadNodeFirstRead
 | DefUse.cs:80:30:80:31 | SSA phi read(this.Field2) | DefUse.cs:63:9:63:14 | this.Field2 | DefUse.cs:80:37:80:42 | access to field Field2 |
 | Fields.cs:63:16:63:28 | SSA phi read(this.LoopField) | Fields.cs:65:24:65:32 | this.LoopField | Fields.cs:65:24:65:32 | access to field LoopField |
 | Patterns.cs:20:9:38:9 | SSA phi read(o) | Patterns.cs:7:16:7:16 | o | Patterns.cs:20:17:20:17 | access to local variable o |
@@ -15,16 +15,21 @@ phiReadNodeRead
 | Test.cs:90:9:97:9 | SSA phi read(x) | Test.cs:78:13:78:13 | x | Test.cs:92:17:92:17 | access to local variable x |
 | Test.cs:90:9:97:9 | SSA phi read(x) | Test.cs:78:13:78:13 | x | Test.cs:96:17:96:17 | access to local variable x |
 | Test.cs:99:9:99:15 | SSA phi read(x) | Test.cs:78:13:78:13 | x | Test.cs:99:13:99:13 | access to local variable x |
-| Test.cs:99:9:99:15 | SSA phi read(x) | Test.cs:78:13:78:13 | x | Test.cs:104:17:104:17 | access to local variable x |
 phiReadInput
-| DefUse.cs:80:30:80:31 | SSA phi read(this.Field2) | DefUse.cs:63:9:63:18 | SSA def(this.Field2) |
-| DefUse.cs:80:30:80:31 | SSA phi read(this.Field2) | DefUse.cs:80:30:80:31 | SSA phi read(this.Field2) |
+| DefUse.cs:80:30:80:31 | SSA phi read(this.Field2) | DefUse.cs:64:13:64:18 | SSA read(this.Field2) |
+| DefUse.cs:80:30:80:31 | SSA phi read(this.Field2) | DefUse.cs:80:37:80:42 | SSA read(this.Field2) |
 | Fields.cs:63:16:63:28 | SSA phi read(this.LoopField) | Fields.cs:61:17:61:17 | SSA entry def(this.LoopField) |
-| Fields.cs:63:16:63:28 | SSA phi read(this.LoopField) | Fields.cs:63:16:63:28 | SSA phi read(this.LoopField) |
-| Patterns.cs:20:9:38:9 | SSA phi read(o) | Patterns.cs:7:16:7:23 | SSA def(o) |
+| Fields.cs:63:16:63:28 | SSA phi read(this.LoopField) | Fields.cs:65:24:65:32 | SSA read(this.LoopField) |
+| Patterns.cs:20:9:38:9 | SSA phi read(o) | Patterns.cs:8:13:8:13 | SSA read(o) |
+| Patterns.cs:20:9:38:9 | SSA phi read(o) | Patterns.cs:12:18:12:18 | SSA read(o) |
+| Patterns.cs:20:9:38:9 | SSA phi read(o) | Patterns.cs:16:18:16:18 | SSA read(o) |
 | Properties.cs:63:16:63:16 | SSA phi read(this.LoopProp) | Properties.cs:61:17:61:17 | SSA entry def(this.LoopProp) |
-| Properties.cs:63:16:63:16 | SSA phi read(this.LoopProp) | Properties.cs:63:16:63:16 | SSA phi read(this.LoopProp) |
+| Properties.cs:63:16:63:16 | SSA phi read(this.LoopProp) | Properties.cs:65:24:65:31 | SSA read(this.LoopProp) |
 | Test.cs:25:16:25:16 | SSA phi read(x) | Test.cs:24:9:24:15 | SSA phi(x) |
-| Test.cs:25:16:25:16 | SSA phi read(x) | Test.cs:25:16:25:16 | SSA phi read(x) |
+| Test.cs:25:16:25:16 | SSA phi read(x) | Test.cs:25:16:25:16 | SSA read(x) |
 | Test.cs:90:9:97:9 | SSA phi read(x) | Test.cs:78:13:78:17 | SSA def(x) |
+| Test.cs:90:9:97:9 | SSA phi read(x) | Test.cs:82:17:82:17 | SSA read(x) |
+| Test.cs:90:9:97:9 | SSA phi read(x) | Test.cs:86:17:86:17 | SSA read(x) |
 | Test.cs:99:9:99:15 | SSA phi read(x) | Test.cs:90:9:97:9 | SSA phi read(x) |
+| Test.cs:99:9:99:15 | SSA phi read(x) | Test.cs:92:17:92:17 | SSA read(x) |
+| Test.cs:99:9:99:15 | SSA phi read(x) | Test.cs:96:17:96:17 | SSA read(x) |

csharp/ql/test/library-tests/dataflow/ssa/SSAPhiRead.ql

@@ -1,17 +1,21 @@
 import csharp
 import semmle.code.csharp.dataflow.internal.SsaImpl
-import ExposedForTestingOnly
+import Impl::TestAdjacentRefs as RefTest
 
-query predicate phiReadNode(PhiReadNode phi, Ssa::SourceVariable v) { phi.getSourceVariable() = v }
+query predicate phiReadNode(RefTest::Ref phi, Ssa::SourceVariable v) {
+  phi.isPhiRead() and phi.getSourceVariable() = v
+}
 
-query predicate phiReadNodeRead(PhiReadNode phi, Ssa::SourceVariable v, ControlFlow::Node read) {
-  phi.getSourceVariable() = v and
-  exists(ControlFlow::BasicBlock bb, int i |
-    ssaDefReachesReadExt(v, phi, bb, i) and
+query predicate phiReadNodeFirstRead(RefTest::Ref phi, Ssa::SourceVariable v, ControlFlow::Node read) {
+  exists(RefTest::Ref r, ControlFlow::BasicBlock bb, int i |
+    phi.isPhiRead() and
+    RefTest::adjacentRefRead(phi, r) and
+    r.accessAt(bb, i, v) and
     read = bb.getNode(i)
   )
 }
 
-query predicate phiReadInput(PhiReadNode phi, DefinitionExt inp) {
-  phiHasInputFromBlockExt(phi, inp, _)
+query predicate phiReadInput(RefTest::Ref phi, RefTest::Ref inp) {
+  phi.isPhiRead() and
+  RefTest::adjacentRefPhi(inp, phi)
 }

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/consistency-queries/SsaConsistency.ql

@@ -1,19 +1,3 @@
 import codeql.ruby.dataflow.SSA
 import codeql.ruby.dataflow.internal.SsaImpl
 import Consistency
-
-class MyRelevantDefinition extends RelevantDefinition, Ssa::Definition {
-  override predicate hasLocationInfo(
-    string filepath, int startline, int startcolumn, int endline, int endcolumn
-  ) {
-    this.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
-  }
-}
-
-class MyRelevantDefinitionExt extends RelevantDefinitionExt, DefinitionExt {
-  override predicate hasLocationInfo(
-    string filepath, int startline, int startcolumn, int endline, int endcolumn
-  ) {
-    this.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
-  }
-}

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