Merge pull request github#5725 from hvitved/csharp/dataflow/performance

C#: Various data-flow performance tweaks

Author: hvitved

csharp/ql/src/semmle/code/csharp/Caching.qll

@@ -49,6 +49,7 @@ module Stages {
 
   cached
   module DataFlowStage {
+    private import semmle.code.csharp.dataflow.internal.DataFlowDispatch
     private import semmle.code.csharp.dataflow.internal.DataFlowPrivate
     private import semmle.code.csharp.dataflow.internal.DataFlowImplCommon
     private import semmle.code.csharp.dataflow.internal.TaintTrackingPrivate
@@ -78,6 +79,8 @@ module Stages {
       or
       exists(CallContext cc)
       or
+      exists(any(DataFlowCall c).getEnclosingCallable())
+      or
       forceCachingInSameStageRev()
     }
   }

csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowDispatch.qll

@@ -4,6 +4,7 @@ private import dotnet
 private import DataFlowPublic
 private import DataFlowPrivate
 private import FlowSummaryImpl as FlowSummaryImpl
+private import semmle.code.csharp.Caching
 private import semmle.code.csharp.dataflow.FlowSummary
 private import semmle.code.csharp.dispatch.Dispatch
 private import semmle.code.csharp.frameworks.system.Collections
@@ -69,8 +70,6 @@ private predicate transitiveCapturedCallTarget(ControlFlow::Nodes::ElementNode c
 
 cached
 private module Cached {
-  private import semmle.code.csharp.Caching
-
   cached
   newtype TReturnKind =
     TNormalReturnKind() { Stages::DataFlowStage::forceCachingInSameStage() } or
@@ -247,6 +246,7 @@ abstract class DataFlowCall extends TDataFlowCall {
   abstract DataFlow::Node getNode();
 
   /** Gets the enclosing callable of this call. */
+  cached
   abstract DataFlowCallable getEnclosingCallable();
 
   /** Gets the underlying expression, if any. */
@@ -280,7 +280,10 @@ class NonDelegateDataFlowCall extends DataFlowCall, TNonDelegateCall {
 
   override DataFlow::ExprNode getNode() { result.getControlFlowNode() = cfn }
 
-  override DataFlowCallable getEnclosingCallable() { result = cfn.getEnclosingCallable() }
+  override DataFlowCallable getEnclosingCallable() {
+    Stages::DataFlowStage::forceCachingInSameStage() and
+    result = cfn.getEnclosingCallable()
+  }
 
   override string toString() { result = cfn.toString() }
 

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

@@ -21,9 +21,11 @@ private import semmle.code.csharp.frameworks.system.threading.Tasks
 
 abstract class NodeImpl extends Node {
   /** Do not call: use `getEnclosingCallable()` instead. */
+  cached
   abstract DataFlowCallable getEnclosingCallableImpl();
 
   /** Do not call: use `getType()` instead. */
+  cached
   abstract DotNet::Type getTypeImpl();
 
   /** Gets the type of this node used for type pruning. */
@@ -39,27 +41,39 @@ abstract class NodeImpl extends Node {
   }
 
   /** Do not call: use `getControlFlowNode()` instead. */
+  cached
   abstract ControlFlow::Node getControlFlowNodeImpl();
 
   /** Do not call: use `getLocation()` instead. */
+  cached
   abstract Location getLocationImpl();
 
   /** Do not call: use `toString()` instead. */
+  cached
   abstract string toStringImpl();
 }
 
 private class ExprNodeImpl extends ExprNode, NodeImpl {
   override DataFlowCallable getEnclosingCallableImpl() {
+    Stages::DataFlowStage::forceCachingInSameStage() and
     result = this.getExpr().getEnclosingCallable()
   }
 
-  override DotNet::Type getTypeImpl() { result = this.getExpr().getType() }
+  override DotNet::Type getTypeImpl() {
+    Stages::DataFlowStage::forceCachingInSameStage() and
+    result = this.getExpr().getType()
+  }
 
-  override ControlFlow::Nodes::ElementNode getControlFlowNodeImpl() { this = TExprNode(result) }
+  override ControlFlow::Nodes::ElementNode getControlFlowNodeImpl() {
+    Stages::DataFlowStage::forceCachingInSameStage() and this = TExprNode(result)
+  }
 
-  override Location getLocationImpl() { result = this.getExpr().getLocation() }
+  override Location getLocationImpl() {
+    Stages::DataFlowStage::forceCachingInSameStage() and result = this.getExpr().getLocation()
+  }
 
   override string toStringImpl() {
+    Stages::DataFlowStage::forceCachingInSameStage() and
     result = this.getControlFlowNode().toString()
     or
     exists(CIL::Expr e |
@@ -967,6 +981,16 @@ private module Cached {
     or
     n.asExpr() = any(WithExpr we).getInitializer()
   }
+
+  cached
+  predicate parameterNode(Node n, DataFlowCallable c, int i) {
+    n.(ParameterNodeImpl).isParameterOf(c, i)
+  }
+
+  cached
+  predicate argumentNode(Node n, DataFlowCall call, int pos) {
+    n.(ArgumentNodeImpl).argumentOf(call, pos)
+  }
 }
 
 import Cached
@@ -992,8 +1016,6 @@ class SsaDefinitionNode extends NodeImpl, TSsaDefinitionNode {
 }
 
 abstract class ParameterNodeImpl extends NodeImpl {
-  abstract DotNet::Parameter getParameter();
-
   abstract predicate isParameterOf(DataFlowCallable c, int i);
 }
 
@@ -1010,11 +1032,9 @@ private module ParameterNodes {
     /** Gets the SSA definition corresponding to this parameter, if any. */
     Ssa::ExplicitDefinition getSsaDefinition() {
       result.getADefinition().(AssignableDefinitions::ImplicitParameterDefinition).getParameter() =
-        this.getParameter()
+        parameter
     }
 
-    override DotNet::Parameter getParameter() { result = parameter }
-
     override predicate isParameterOf(DataFlowCallable c, int i) { c.getParameter(i) = parameter }
 
     override DataFlowCallable getEnclosingCallableImpl() { result = parameter.getCallable() }
@@ -1037,8 +1057,6 @@ private module ParameterNodes {
     /** Gets the callable containing this implicit instance parameter. */
     Callable getCallable() { result = callable }
 
-    override DotNet::Parameter getParameter() { none() }
-
     override predicate isParameterOf(DataFlowCallable c, int pos) { callable = c and pos = -1 }
 
     override DataFlowCallable getEnclosingCallableImpl() { result = callable }
@@ -1113,8 +1131,6 @@ private module ParameterNodes {
     /** Gets the captured variable that this implicit parameter models. */
     LocalScopeVariable getVariable() { result = def.getVariable() }
 
-    override DotNet::Parameter getParameter() { none() }
-
     override predicate isParameterOf(DataFlowCallable c, int i) {
       i = getParameterPosition(def) and
       c = this.getEnclosingCallable()
@@ -1125,13 +1141,15 @@ private module ParameterNodes {
 import ParameterNodes
 
 /** A data-flow node that represents a call argument. */
-abstract class ArgumentNode extends Node {
+class ArgumentNode extends Node {
+  ArgumentNode() { argumentNode(this, _, _) }
+
   /** Holds if this argument occurs at the given position in the given call. */
-  cached
-  abstract predicate argumentOf(DataFlowCall call, int pos);
+  final predicate argumentOf(DataFlowCall call, int pos) { argumentNode(this, call, pos) }
+}
 
-  /** Gets the call in which this node is an argument. */
-  final DataFlowCall getCall() { this.argumentOf(result, _) }
+abstract private class ArgumentNodeImpl extends Node {
+  abstract predicate argumentOf(DataFlowCall call, int pos);
 }
 
 private module ArgumentNodes {
@@ -1149,15 +1167,14 @@ private module ArgumentNodes {
   }
 
   /** A data-flow node that represents an explicit call argument. */
-  class ExplicitArgumentNode extends ArgumentNode {
+  class ExplicitArgumentNode extends ArgumentNodeImpl {
     ExplicitArgumentNode() {
       this.asExpr() instanceof Argument
       or
       this.asExpr() = any(CIL::Call call).getAnArgument()
     }
 
     override predicate argumentOf(DataFlowCall call, int pos) {
-      Stages::DataFlowStage::forceCachingInSameStage() and
       exists(ArgumentConfiguration x, Expr c, Argument arg |
         arg = this.asExpr() and
         c = call.getExpr() and
@@ -1189,7 +1206,8 @@ private module ArgumentNodes {
    * }                                }
    * ```
    */
-  class ImplicitCapturedArgumentNode extends ArgumentNode, NodeImpl, TImplicitCapturedArgumentNode {
+  class ImplicitCapturedArgumentNode extends ArgumentNodeImpl, NodeImpl,
+    TImplicitCapturedArgumentNode {
     private LocalScopeVariable v;
     private ControlFlow::Nodes::ElementNode cfn;
 
@@ -1231,7 +1249,7 @@ private module ArgumentNodes {
    * A node that corresponds to the value of an object creation (`new C()`) before
    * the constructor has run.
    */
-  class MallocNode extends ArgumentNode, NodeImpl, TMallocNode {
+  class MallocNode extends ArgumentNodeImpl, NodeImpl, TMallocNode {
     private ControlFlow::Nodes::ElementNode cfn;
 
     MallocNode() { this = TMallocNode(cfn) }
@@ -1266,7 +1284,7 @@ private module ArgumentNodes {
    * and that argument is itself a compatible array, for example
    * `Foo(new[] { "a", "b", "c" })`.
    */
-  class ParamsArgumentNode extends ArgumentNode, NodeImpl, TParamsArgumentNode {
+  class ParamsArgumentNode extends ArgumentNodeImpl, NodeImpl, TParamsArgumentNode {
     private ControlFlow::Node callCfn;
 
     ParamsArgumentNode() { this = TParamsArgumentNode(callCfn) }
@@ -1291,7 +1309,7 @@ private module ArgumentNodes {
     override string toStringImpl() { result = "[implicit array creation] " + callCfn }
   }
 
-  private class SummaryArgumentNode extends SummaryNode, ArgumentNode {
+  private class SummaryArgumentNode extends SummaryNode, ArgumentNodeImpl {
     private DataFlowCall c;
     private int i;
 
@@ -1324,10 +1342,7 @@ private module ReturnNodes {
       )
     }
 
-    override NormalReturnKind getKind() {
-      any(DotNet::Callable c).canReturn(this.getExpr()) and
-      exists(result)
-    }
+    override NormalReturnKind getKind() { exists(result) }
   }
 
   /**
@@ -1744,7 +1759,10 @@ class DataFlowType extends Gvn::GvnType {
 }
 
 /** Gets the type of `n` used for type pruning. */
-DataFlowType getNodeType(NodeImpl n) { result = n.getDataFlowType() }
+pragma[inline]
+Gvn::GvnType getNodeType(NodeImpl n) {
+  pragma[only_bind_into](result) = pragma[only_bind_out](n).getDataFlowType()
+}
 
 /** Gets a string representation of a `DataFlowType`. */
 string ppReprType(DataFlowType t) { result = t.toString() }
@@ -1819,7 +1837,8 @@ private module PostUpdateNodes {
    * Such a node acts as both a post-update node for the `MallocNode`, as well as
    * a pre-update node for the `ObjectCreationNode`.
    */
-  class ObjectInitializerNode extends PostUpdateNode, NodeImpl, ArgumentNode, TObjectInitializerNode {
+  class ObjectInitializerNode extends PostUpdateNode, NodeImpl, ArgumentNodeImpl,
+    TObjectInitializerNode {
     private ObjectCreation oc;
     private ControlFlow::Nodes::ElementNode cfn;
 

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

@@ -3,7 +3,6 @@ private import cil
 private import dotnet
 private import DataFlowDispatch
 private import DataFlowPrivate
-private import semmle.code.csharp.Caching
 private import semmle.code.csharp.controlflow.Guards
 private import semmle.code.csharp.Unification
 
@@ -38,38 +37,21 @@ class Node extends TNode {
   }
 
   /** Gets the type of this node. */
-  cached
-  final DotNet::Type getType() {
-    Stages::DataFlowStage::forceCachingInSameStage() and result = this.(NodeImpl).getTypeImpl()
-  }
+  final DotNet::Type getType() { result = this.(NodeImpl).getTypeImpl() }
 
   /** Gets the enclosing callable of this node. */
-  cached
   final DataFlowCallable getEnclosingCallable() {
-    Stages::DataFlowStage::forceCachingInSameStage() and
     result = this.(NodeImpl).getEnclosingCallableImpl()
   }
 
   /** Gets the control flow node corresponding to this node, if any. */
-  cached
-  final ControlFlow::Node getControlFlowNode() {
-    Stages::DataFlowStage::forceCachingInSameStage() and
-    result = this.(NodeImpl).getControlFlowNodeImpl()
-  }
+  final ControlFlow::Node getControlFlowNode() { result = this.(NodeImpl).getControlFlowNodeImpl() }
 
   /** Gets a textual representation of this node. */
-  cached
-  final string toString() {
-    Stages::DataFlowStage::forceCachingInSameStage() and
-    result = this.(NodeImpl).toStringImpl()
-  }
+  final string toString() { result = this.(NodeImpl).toStringImpl() }
 
   /** Gets the location of this node. */
-  cached
-  final Location getLocation() {
-    Stages::DataFlowStage::forceCachingInSameStage() and
-    result = this.(NodeImpl).getLocationImpl()
-  }
+  final Location getLocation() { result = this.(NodeImpl).getLocationImpl() }
 
   /**
    * Holds if this element is at the specified location.
@@ -81,7 +63,7 @@ class Node extends TNode {
   predicate hasLocationInfo(
     string filepath, int startline, int startcolumn, int endline, int endcolumn
   ) {
-    getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
+    this.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
   }
 }
 
@@ -117,18 +99,18 @@ class ExprNode extends Node {
  * flow graph.
  */
 class ParameterNode extends Node {
-  private ParameterNodeImpl p;
-
-  ParameterNode() { this = p }
+  ParameterNode() { parameterNode(this, _, _) }
 
   /** Gets the parameter corresponding to this node, if any. */
-  DotNet::Parameter getParameter() { result = p.getParameter() }
+  DotNet::Parameter getParameter() {
+    exists(DataFlowCallable c, int i | this.isParameterOf(c, i) and result = c.getParameter(i))
+  }
 
   /**
    * Holds if this node is the parameter of callable `c` at the specified
    * (zero-based) position.
    */
-  predicate isParameterOf(DataFlowCallable c, int i) { p.isParameterOf(c, i) }
+  predicate isParameterOf(DataFlowCallable c, int i) { parameterNode(this, c, i) }
 }
 
 /** A definition, viewed as a node in a data flow graph. */
@@ -166,6 +148,7 @@ predicate localFlowStep = localFlowStepImpl/2;
  * Holds if data flows from `source` to `sink` in zero or more local
  * (intra-procedural) steps.
  */
+pragma[inline]
 predicate localFlow(Node source, Node sink) { localFlowStep*(source, sink) }
 
 /**