Ruby: Unmerge SSA and data flow stages

Author: hvitved

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

@@ -83,7 +83,15 @@ SsaImpl::DefinitionExt getParameterDef(NamedParameter p) {
 
 /** Provides logic related to SSA. */
 private module SsaFlow {
-  module Impl = SsaImpl::DataFlowIntegration;
+  private module Impl = SsaImpl::DataFlowIntegration;
+
+  private ParameterNodeImpl toParameterNode(SsaImpl::ParameterExt p) {
+    result = TNormalParameterNode(p.asParameter())
+    or
+    result = TSelfMethodParameterNode(p.asMethodSelf())
+    or
+    result = TSelfToplevelParameterNode(p.asToplevelSelf())
+  }
 
   Impl::Node asNode(Node n) {
     n = TSsaNode(result)
@@ -92,11 +100,11 @@ private module SsaFlow {
     or
     result.(Impl::ExprPostUpdateNode).getExpr() = n.(PostUpdateNode).getPreUpdateNode().asExpr()
     or
-    n = result.(Impl::ParameterNode).getParameter().toParameterNode()
+    n = toParameterNode(result.(Impl::ParameterNode).getParameter())
   }
 
-  predicate localFlowStep(SsaImpl::DefinitionExt def, Node nodeFrom, Node nodeTo) {
-    Impl::localFlowStep(def, asNode(nodeFrom), asNode(nodeTo))
+  predicate localFlowStep(SsaImpl::DefinitionExt def, Node nodeFrom, Node nodeTo, boolean isUseStep) {
+    Impl::localFlowStep(def, asNode(nodeFrom), asNode(nodeTo), isUseStep)
   }
 
   predicate localMustFlowStep(SsaImpl::DefinitionExt def, Node nodeFrom, Node nodeTo) {
@@ -436,7 +444,7 @@ private module Cached {
   newtype TNode =
     TExprNode(CfgNodes::ExprCfgNode n) or
     TReturningNode(CfgNodes::ReturningCfgNode n) { exists(n.getReturnedValueNode()) } or
-    TSsaNode(SsaFlow::Impl::SsaNode node) or
+    TSsaNode(SsaImpl::DataFlowIntegration::SsaNode node) or
     TCapturedVariableNode(VariableCapture::CapturedVariable v) or
     TNormalParameterNode(SsaImpl::NormalParameter p) or
     TSelfMethodParameterNode(MethodBase m) or
@@ -507,10 +515,13 @@ private module Cached {
     (
       LocalFlow::localFlowStepCommon(nodeFrom, nodeTo)
       or
-      exists(SsaImpl::DefinitionExt def |
-        SsaFlow::localFlowStep(def, nodeFrom, nodeTo) and
+      exists(SsaImpl::DefinitionExt def, boolean isUseStep |
+        SsaFlow::localFlowStep(def, nodeFrom, nodeTo, isUseStep) and
         // captured variables are handled by the shared `VariableCapture` library
-        not def instanceof VariableCapture::CapturedSsaDefinitionExt and
+        not def instanceof VariableCapture::CapturedSsaDefinitionExt
+      |
+        isUseStep = false
+        or
         not FlowSummaryImpl::Private::Steps::prohibitsUseUseFlow(nodeFrom, _)
       )
       or
@@ -526,7 +537,7 @@ private module Cached {
   predicate localFlowStepImpl(Node nodeFrom, Node nodeTo) {
     LocalFlow::localFlowStepCommon(nodeFrom, nodeTo)
     or
-    SsaFlow::localFlowStep(_, nodeFrom, nodeTo)
+    SsaFlow::localFlowStep(_, nodeFrom, nodeTo, _)
     or
     // Simple flow through library code is included in the exposed local
     // step relation, even though flow is technically inter-procedural
@@ -540,7 +551,7 @@ private module Cached {
   predicate localFlowStepTypeTracker(Node nodeFrom, Node nodeTo) {
     LocalFlow::localFlowStepCommon(nodeFrom, nodeTo)
     or
-    SsaFlow::localFlowStep(_, nodeFrom, nodeTo)
+    SsaFlow::localFlowStep(_, nodeFrom, nodeTo, _)
     or
     VariableCapture::flowInsensitiveStep(nodeFrom, nodeTo)
     or
@@ -745,7 +756,7 @@ predicate nodeIsHidden(Node n) {
 
 /** An SSA node. */
 abstract class SsaNode extends NodeImpl, TSsaNode {
-  SsaFlow::Impl::SsaNode node;
+  SsaImpl::DataFlowIntegration::SsaNode node;
   SsaImpl::DefinitionExt def;
 
   SsaNode() {
@@ -765,7 +776,7 @@ abstract class SsaNode extends NodeImpl, TSsaNode {
 
 /** An (extended) SSA definition, viewed as a node in a data flow graph. */
 class SsaDefinitionExtNode extends SsaNode {
-  override SsaFlow::Impl::SsaDefinitionExtNode node;
+  override SsaImpl::DataFlowIntegration::SsaDefinitionExtNode node;
 
   /** Gets the underlying variable. */
   Variable getVariable() { result = def.getSourceVariable() }
@@ -827,7 +838,7 @@ class SsaDefinitionNodeImpl extends SsaDefinitionExtNode {
  * both inputs into the phi read node after the outer condition are guarded.
  */
 class SsaInputNode extends SsaNode {
-  override SsaFlow::Impl::SsaInputNode node;
+  override SsaImpl::DataFlowIntegration::SsaInputNode node;
 
   override predicate isHidden() { any() }
 

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

@@ -5,7 +5,6 @@ private import codeql.ruby.controlflow.internal.ControlFlowGraphImpl as ControlF
 private import codeql.ruby.dataflow.SSA
 private import codeql.ruby.ast.Variable
 private import Cfg::CfgNodes::ExprNodes
-private import DataFlowPrivate
 
 module SsaInput implements SsaImplCommon::InputSig<Location> {
   private import codeql.ruby.controlflow.ControlFlowGraph as Cfg
@@ -421,13 +420,24 @@ private module Cached {
     Impl::uncertainWriteDefinitionInput(def, result)
   }
 
-  private import TaintTrackingPrivate as TaintTrackingPrivate
-
   cached
-  predicate forceCachingInSameStage() {
-    // needed in order to avoid recomputing SSA predicates in the `Integration` module
-    DataFlowIntegration::localFlowStep(_, _, _) and
-    TaintTrackingPrivate::forceCachingInSameStage()
+  module DataFlowIntegration {
+    import DataFlowIntegrationImpl
+
+    cached
+    predicate localFlowStep(DefinitionExt def, Node nodeFrom, Node nodeTo, boolean isUseStep) {
+      DataFlowIntegrationImpl::localFlowStep(def, nodeFrom, nodeTo, isUseStep)
+    }
+
+    cached
+    predicate localMustFlowStep(DefinitionExt def, Node nodeFrom, Node nodeTo) {
+      DataFlowIntegrationImpl::localMustFlowStep(def, nodeFrom, nodeTo)
+    }
+
+    cached
+    Node getABarrierNode(Cfg::CfgNodes::AstCfgNode guard, Ssa::Definition def, boolean branch) {
+      result = DataFlowIntegrationImpl::getABarrierNode(guard, def, branch)
+    }
   }
 
   bindingset[def1, def2]
@@ -517,14 +527,6 @@ class ParameterExt extends TParameterExt {
 
   Toplevel asToplevelSelf() { this = TSelfToplevelParameter(result) }
 
-  ParameterNodeImpl toParameterNode() {
-    result = TNormalParameterNode(this.asParameter())
-    or
-    result = TSelfMethodParameterNode(this.asMethodSelf())
-    or
-    result = TSelfToplevelParameterNode(this.asToplevelSelf())
-  }
-
   predicate isInitializedBy(WriteDefinition def) {
     def = getParameterDef(this.asParameter())
     or
@@ -584,4 +586,4 @@ private module DataFlowIntegrationInput implements Impl::DataFlowIntegrationInpu
   }
 }
 
-module DataFlowIntegration = Impl::DataFlowIntegration<DataFlowIntegrationInput>;
+private module DataFlowIntegrationImpl = Impl::DataFlowIntegration<DataFlowIntegrationInput>;