perf

Author: hvitved

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

@@ -256,8 +256,9 @@ module VariableCapture {
   private predicate closureFlowStep(ControlFlow::Nodes::ExprNode e1, ControlFlow::Nodes::ExprNode e2) {
     e1 = LocalFlow::getALastEvalNode(e2)
     or
-    exists(Ssa::Definition def |
-      SsaFlow::Input::ssaDefAssigns(def.getAnUltimateDefinition(), e1) and
+    exists(Ssa::Definition def, AssignableDefinition adef |
+      LocalFlow::defAssigns(adef, _, e1) and
+      def.getAnUltimateDefinition().(Ssa::ExplicitDefinition).getADefinition() = adef and
       exists(def.getAReadAtNode(e2))
     )
   }
@@ -483,42 +484,7 @@ module VariableCapture {
 
 /** Provides logic related to SSA. */
 module SsaFlow {
-  module Input implements SsaImpl::Impl::DataFlowIntegrationInputSig {
-    private import csharp as Cs
-
-    class Expr extends ControlFlow::Node {
-      predicate hasCfgNode(ControlFlow::BasicBlock bb, int i) { this = bb.getNode(i) }
-    }
-
-    predicate ssaDefAssigns(SsaImpl::WriteDefinition def, Expr value) {
-      exists(AssignableDefinition adef, ControlFlow::Node cfnDef |
-        any(LocalFlow::LocalExprStepConfiguration conf).hasDefPath(_, value, adef, cfnDef) and
-        def.(Ssa::ExplicitDefinition).getADefinition() = adef and
-        def.(Ssa::ExplicitDefinition).getControlFlowNode() = cfnDef
-      )
-    }
-
-    class Parameter = Cs::Parameter;
-
-    predicate ssaDefInitializesParam(SsaImpl::WriteDefinition def, Parameter p) {
-      def.(Ssa::ImplicitParameterDefinition).getParameter() = p
-    }
-
-    /**
-     * Allows for flow into uncertain defintions that are not call definitions,
-     * as we, conservatively, consider such definitions to be certain.
-     */
-    predicate allowFlowIntoUncertainDef(SsaImpl::UncertainWriteDefinition def) {
-      def instanceof Ssa::ExplicitDefinition
-      or
-      def =
-        any(Ssa::ImplicitQualifierDefinition qdef |
-          allowFlowIntoUncertainDef(qdef.getQualifierDefinition())
-        )
-    }
-  }
-
-  module Impl = SsaImpl::Impl::DataFlowIntegration<Input>;
+  module Impl = SsaImpl::DataFlowIntegration;
 
   Impl::Node asNode(Node n) {
     n = TSsaNode(result)
@@ -532,47 +498,12 @@ module SsaFlow {
   }
 
   predicate localFlowStep(SsaImpl::DefinitionExt def, Node nodeFrom, Node nodeTo) {
-    Impl::localFlowStep(def, asNode(nodeFrom), asNode(nodeTo)) and
-    // 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
-    not Input::ssaDefAssigns(def, nodeFrom.(ExprNode).getControlFlowNode())
+    Impl::localFlowStep(def, asNode(nodeFrom), asNode(nodeTo))
   }
 
   predicate localMustFlowStep(SsaImpl::DefinitionExt def, Node nodeFrom, Node nodeTo) {
     Impl::localMustFlowStep(def, asNode(nodeFrom), asNode(nodeTo))
   }
-
-  module BarrierGuardsInput implements Impl::BarrierGuardsInputSig {
-    private import semmle.code.csharp.controlflow.BasicBlocks
-    private import semmle.code.csharp.controlflow.Guards as Guards
-
-    class Guard extends Guards::Guard {
-      predicate hasCfgNode(ControlFlow::BasicBlock bb, int i) {
-        this.getAControlFlowNode() = bb.getNode(i)
-      }
-    }
-
-    /** Holds if the guard `guard` controls block `bb` upon evaluating to `branch`. */
-    predicate guardControlsBlock(Guard guard, ControlFlow::BasicBlock bb, boolean branch) {
-      exists(ConditionBlock conditionBlock, ControlFlow::SuccessorTypes::ConditionalSuccessor s |
-        guard.getAControlFlowNode() = conditionBlock.getLastNode() and
-        s.getValue() = branch and
-        conditionBlock.controls(bb, s)
-      )
-    }
-
-    /** Gets an immediate conditional successor of basic block `bb`, if any. */
-    ControlFlow::BasicBlock getAConditionalBasicBlockSuccessor(
-      ControlFlow::BasicBlock bb, boolean branch
-    ) {
-      exists(ControlFlow::SuccessorTypes::ConditionalSuccessor s |
-        result = bb.getASuccessorByType(s) and
-        s.getValue() = branch
-      )
-    }
-  }
-
-  module BarrierGuardsImpl = Impl::BarrierGuards<BarrierGuardsInput>;
 }
 
 /** Provides predicates related to local data flow. */
@@ -1200,6 +1131,11 @@ private module Cached {
   newtype TDataFlowType =
     TGvnDataFlowType(Gvn::GvnType t) or
     TDelegateDataFlowType(Callable lambda) { lambdaCreationExpr(_, lambda) }
+
+  cached
+  Node getABarrierNode(Guard guard, Ssa::Definition def, boolean branch) {
+    SsaFlow::asNode(result) = SsaImpl::DataFlowIntegration::getABarrierNode(guard, def, branch)
+  }
 }
 
 import Cached

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

@@ -171,20 +171,21 @@ signature predicate guardChecksSig(Guard g, Expr e, AbstractValue v);
  * in data flow and taint tracking.
  */
 module BarrierGuard<guardChecksSig/3 guardChecks> {
-  private predicate guardChecksAdj(
-    SsaFlow::BarrierGuardsInput::Guard g, SsaFlow::Input::Expr e, boolean branch
-  ) {
+  pragma[nomagic]
+  private predicate guardChecksSsaDef(Guard g, Ssa::Definition def, boolean branch) {
     exists(AbstractValues::BooleanValue v |
-      guardChecks(g, e.getAstNode(), v) and
+      guardChecks(g, def.getARead(), v) and
       branch = v.getValue()
     )
   }
 
   /** Gets a node that is safely guarded by the given guard check. */
   pragma[nomagic]
   Node getABarrierNode() {
-    SsaFlow::asNode(result) =
-      SsaFlow::BarrierGuardsImpl::BarrierGuard<guardChecksAdj/3>::getABarrierNode()
+    exists(Guard g, Ssa::Definition def, boolean branch |
+      result = getABarrierNode(g, def, branch) and
+      guardChecksSsaDef(g, def, branch)
+    )
     or
     exists(Guard g, Expr e, AbstractValue v |
       guardChecks(g, e, v) and

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

@@ -977,6 +977,15 @@ private module Cached {
       outRefExitRead(bb, i, v)
     )
   }
+
+  private import TaintTrackingPrivate as TaintTrackingPrivate
+
+  cached
+  predicate forceCachingInSameStage() {
+    // needed in order to avoid recomputing SSA predicates in the `Integration` module
+    TaintTrackingPrivate::forceCachingInSameStage() and
+    DataFlowIntegration::localFlowStep(_, _, _)
+  }
 }
 
 import Cached
@@ -1034,3 +1043,65 @@ class PhiReadNode extends DefinitionExt, Impl::PhiReadNode {
     result = this.getSourceVariable().getEnclosingCallable()
   }
 }
+
+private module DataFlowIntegrationInput implements Impl::DataFlowIntegrationInputSig {
+  private import csharp as Cs
+  private import semmle.code.csharp.controlflow.BasicBlocks
+  private import semmle.code.csharp.controlflow.Guards as Guards
+
+  class Expr extends ControlFlow::Node {
+    predicate hasCfgNode(ControlFlow::BasicBlock bb, int i) { this = bb.getNode(i) }
+  }
+
+  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 = Cs::Parameter;
+
+  predicate ssaDefInitializesParam(WriteDefinition def, Parameter p) {
+    def.(Ssa::ImplicitParameterDefinition).getParameter() = p
+  }
+
+  /**
+   * Allows for flow into uncertain defintions that are not call definitions,
+   * as we, conservatively, consider such definitions to be certain.
+   */
+  predicate allowFlowIntoUncertainDef(UncertainWriteDefinition def) {
+    def instanceof Ssa::ExplicitDefinition
+    or
+    def =
+      any(Ssa::ImplicitQualifierDefinition qdef |
+        allowFlowIntoUncertainDef(qdef.getQualifierDefinition())
+      )
+  }
+
+  class Guard extends Guards::Guard {
+    predicate hasCfgNode(ControlFlow::BasicBlock bb, int i) {
+      this.getAControlFlowNode() = bb.getNode(i)
+    }
+  }
+
+  /** Holds if the guard `guard` controls block `bb` upon evaluating to `branch`. */
+  predicate guardControlsBlock(Guard guard, ControlFlow::BasicBlock bb, boolean branch) {
+    exists(ConditionBlock conditionBlock, ControlFlow::SuccessorTypes::ConditionalSuccessor s |
+      guard.getAControlFlowNode() = conditionBlock.getLastNode() and
+      s.getValue() = branch and
+      conditionBlock.controls(bb, s)
+    )
+  }
+
+  /** Gets an immediate conditional successor of basic block `bb`, if any. */
+  ControlFlow::BasicBlock getAConditionalBasicBlockSuccessor(
+    ControlFlow::BasicBlock bb, boolean branch
+  ) {
+    exists(ControlFlow::SuccessorTypes::ConditionalSuccessor s |
+      result = bb.getASuccessorByType(s) and
+      s.getValue() = branch
+    )
+  }
+}
+
+module DataFlowIntegration = Impl::DataFlowIntegration<DataFlowIntegrationInput>;

ruby/ql/lib/codeql/ruby/dataflow/SSA.qll

@@ -202,15 +202,32 @@ module Ssa {
     final VariableWriteAccessCfgNode getWriteAccess() { result = write }
 
     /**
-     * Holds if this SSA definition represents a direct assignment of `value`
-     * to the underlying variable.
+     * Holds if this SSA definition assigns `value` to the underlying variable.
+     *
+     * This is either a direct assignment, `x = value`, or an assignment via
+     * simple pattern matching
+     *
+     * ```rb
+     * case value
+     *  in Foo => x then ...
+     *  in y => then ...
+     * end
+     * ```
      */
     predicate assigns(CfgNodes::ExprCfgNode value) {
       exists(CfgNodes::ExprNodes::AssignExprCfgNode a, BasicBlock bb, int i |
         this.definesAt(_, bb, i) and
         a = bb.getNode(i) and
         value = a.getRhs()
       )
+      or
+      exists(CfgNodes::ExprNodes::CaseExprCfgNode case, CfgNodes::AstCfgNode pattern |
+        case.getValue() = value and
+        pattern = case.getBranch(_).(CfgNodes::ExprNodes::InClauseCfgNode).getPattern()
+      |
+        this.getWriteAccess() =
+          [pattern, pattern.(CfgNodes::ExprNodes::AsPatternCfgNode).getVariableAccess()]
+      )
     }
 
     final override string toString() { result = write.toString() }