Java: Update data-flow caching

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

@@ -4,71 +4,48 @@ private import semmle.code.java.dataflow.FlowSummary
 private import semmle.code.java.dataflow.TypeFlow
 private import DataFlowPrivate
 private import FlowSummaryImpl as FlowSummaryImpl
+private import DataFlowImplCommon as DataFlowImplCommon
 
 cached
-private module Cached {
-  cached
-  newtype TNode =
-    TExprNode(Expr e) {
-      not e.getType() instanceof VoidType and
-      not e.getParent*() instanceof Annotation
-    } or
-    TExplicitParameterNode(Parameter p) {
-      exists(p.getCallable().getBody()) or p.getCallable() instanceof SummarizedCallable
-    } or
-    TImplicitVarargsArray(Call c) {
-      c.getCallee().isVarargs() and
-      not exists(Argument arg | arg.getCall() = c and arg.isExplicitVarargsArray())
-    } or
-    TInstanceParameterNode(Callable c) {
-      (exists(c.getBody()) or c instanceof SummarizedCallable) and
-      not c.isStatic()
-    } or
-    TImplicitInstanceAccess(InstanceAccessExt ia) { not ia.isExplicit(_) } or
-    TMallocNode(ClassInstanceExpr cie) or
-    TExplicitExprPostUpdate(Expr e) {
-      explicitInstanceArgument(_, e)
-      or
-      e instanceof Argument and not e.getType() instanceof ImmutableType
-      or
-      exists(FieldAccess fa | fa.getField() instanceof InstanceField and e = fa.getQualifier())
-      or
-      exists(ArrayAccess aa | e = aa.getArray())
-    } or
-    TImplicitExprPostUpdate(InstanceAccessExt ia) {
-      implicitInstanceArgument(_, ia)
-      or
-      exists(FieldAccess fa |
-        fa.getField() instanceof InstanceField and ia.isImplicitFieldQualifier(fa)
-      )
-    } or
-    TSummaryInternalNode(SummarizedCallable c, FlowSummaryImpl::Private::SummaryNodeState state) {
-      FlowSummaryImpl::Private::summaryNodeRange(c, state)
-    }
-
-  cached
-  predicate summaryOutNodeCached(DataFlowCall c, Node out) {
-    FlowSummaryImpl::Private::summaryOutNode(c, out, _)
+newtype TNode =
+  TExprNode(Expr e) {
+    DataFlowImplCommon::forceCachingInSameStage() and
+    not e.getType() instanceof VoidType and
+    not e.getParent*() instanceof Annotation
+  } or
+  TExplicitParameterNode(Parameter p) {
+    exists(p.getCallable().getBody()) or p.getCallable() instanceof SummarizedCallable
+  } or
+  TImplicitVarargsArray(Call c) {
+    c.getCallee().isVarargs() and
+    not exists(Argument arg | arg.getCall() = c and arg.isExplicitVarargsArray())
+  } or
+  TInstanceParameterNode(Callable c) {
+    (exists(c.getBody()) or c instanceof SummarizedCallable) and
+    not c.isStatic()
+  } or
+  TImplicitInstanceAccess(InstanceAccessExt ia) { not ia.isExplicit(_) } or
+  TMallocNode(ClassInstanceExpr cie) or
+  TExplicitExprPostUpdate(Expr e) {
+    explicitInstanceArgument(_, e)
+    or
+    e instanceof Argument and not e.getType() instanceof ImmutableType
+    or
+    exists(FieldAccess fa | fa.getField() instanceof InstanceField and e = fa.getQualifier())
+    or
+    exists(ArrayAccess aa | e = aa.getArray())
+  } or
+  TImplicitExprPostUpdate(InstanceAccessExt ia) {
+    implicitInstanceArgument(_, ia)
+    or
+    exists(FieldAccess fa |
+      fa.getField() instanceof InstanceField and ia.isImplicitFieldQualifier(fa)
+    )
+  } or
+  TSummaryInternalNode(SummarizedCallable c, FlowSummaryImpl::Private::SummaryNodeState state) {
+    FlowSummaryImpl::Private::summaryNodeRange(c, state)
   }
 
-  cached
-  predicate summaryArgumentNodeCached(DataFlowCall c, Node arg, int i) {
-    FlowSummaryImpl::Private::summaryArgumentNode(c, arg, i)
-  }
-
-  cached
-  predicate summaryPostUpdateNodeCached(Node post, ParameterNode pre) {
-    FlowSummaryImpl::Private::summaryPostUpdateNode(post, pre)
-  }
-
-  cached
-  predicate summaryReturnNodeCached(Node ret) {
-    FlowSummaryImpl::Private::summaryReturnNode(ret, _)
-  }
-}
-
-private import Cached
-
 private predicate explicitInstanceArgument(Call call, Expr instarg) {
   call instanceof MethodAccess and
   instarg = call.getQualifier() and
@@ -404,13 +381,15 @@ module Private {
     override string toString() { result = "[summary] " + state + " in " + c }
 
     /** Holds if this summary node is the `i`th argument of `call`. */
-    predicate isArgumentOf(DataFlowCall call, int i) { summaryArgumentNodeCached(call, this, i) }
+    predicate isArgumentOf(DataFlowCall call, int i) {
+      FlowSummaryImpl::Private::summaryArgumentNode(call, this, i)
+    }
 
     /** Holds if this summary node is a return node. */
-    predicate isReturn() { summaryReturnNodeCached(this) }
+    predicate isReturn() { FlowSummaryImpl::Private::summaryReturnNode(this, _) }
 
     /** Holds if this summary node is an out node for `call`. */
-    predicate isOut(DataFlowCall call) { summaryOutNodeCached(call, this) }
+    predicate isOut(DataFlowCall call) { FlowSummaryImpl::Private::summaryOutNode(call, this, _) }
   }
 
   SummaryNode getSummaryNode(SummarizedCallable c, FlowSummaryImpl::Private::SummaryNodeState state) {
@@ -439,7 +418,7 @@ private class MallocNode extends Node, TMallocNode {
 private class SummaryPostUpdateNode extends SummaryNode, PostUpdateNode {
   private Node pre;
 
-  SummaryPostUpdateNode() { summaryPostUpdateNodeCached(this, pre) }
+  SummaryPostUpdateNode() { FlowSummaryImpl::Private::summaryPostUpdateNode(this, pre) }
 
   override Node getPreUpdateNode() { result = pre }
 }

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

@@ -284,7 +284,6 @@ private class ConstantBooleanArgumentNode extends ArgumentNode, ExprNode {
 /**
  * Holds if the node `n` is unreachable when the call context is `call`.
  */
-cached
 predicate isUnreachableInCall(Node n, DataFlowCall call) {
   exists(
     ExplicitParameterNode paramNode, ConstantBooleanArgumentNode arg, SsaImplicitInit param,

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

@@ -11,6 +11,7 @@ private import semmle.code.java.dataflow.FlowSteps
 private import semmle.code.java.dataflow.FlowSummary
 import semmle.code.java.dataflow.InstanceAccess
 private import FlowSummaryImpl as FlowSummaryImpl
+private import TaintTrackingUtil as TaintTrackingUtil
 import DataFlowNodes::Public
 
 /** Holds if `n` is an access to an unqualified `this` at `cfgnode`. */
@@ -112,6 +113,7 @@ predicate localFlowStep(Node node1, Node node2) {
  */
 cached
 predicate simpleLocalFlowStep(Node node1, Node node2) {
+  TaintTrackingUtil::forceCachingInSameStage() and
   // Variable flow steps through adjacent def-use and use-use pairs.
   exists(SsaExplicitUpdate upd |
     upd.getDefiningExpr().(VariableAssign).getSource() = node1.asExpr() or

java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll

@@ -29,55 +29,69 @@ predicate localExprTaint(Expr src, Expr sink) {
   localTaint(DataFlow::exprNode(src), DataFlow::exprNode(sink))
 }
 
-/**
- * Holds if taint can flow in one local step from `src` to `sink`.
- */
-predicate localTaintStep(DataFlow::Node src, DataFlow::Node sink) {
-  DataFlow::localFlowStep(src, sink) or
-  localAdditionalTaintStep(src, sink) or
-  // Simple flow through library code is included in the exposed local
-  // step relation, even though flow is technically inter-procedural
-  FlowSummaryImpl::Private::Steps::summaryThroughStep(src, sink, false)
+cached
+private module Cached {
+  private import DataFlowImplCommon as DataFlowImplCommon
+
+  cached
+  predicate forceCachingInSameStage() { DataFlowImplCommon::forceCachingInSameStage() }
+
+  /**
+   * Holds if taint can flow in one local step from `src` to `sink`.
+   */
+  cached
+  predicate localTaintStep(DataFlow::Node src, DataFlow::Node sink) {
+    DataFlow::localFlowStep(src, sink) or
+    localAdditionalTaintStep(src, sink) or
+    // Simple flow through library code is included in the exposed local
+    // step relation, even though flow is technically inter-procedural
+    FlowSummaryImpl::Private::Steps::summaryThroughStep(src, sink, false)
+  }
+
+  /**
+   * Holds if taint can flow in one local step from `src` to `sink` excluding
+   * local data flow steps. That is, `src` and `sink` are likely to represent
+   * different objects.
+   */
+  cached
+  predicate localAdditionalTaintStep(DataFlow::Node src, DataFlow::Node sink) {
+    localAdditionalTaintExprStep(src.asExpr(), sink.asExpr())
+    or
+    localAdditionalTaintUpdateStep(src.asExpr(),
+      sink.(DataFlow::PostUpdateNode).getPreUpdateNode().asExpr())
+    or
+    exists(Argument arg |
+      src.asExpr() = arg and
+      arg.isVararg() and
+      sink.(DataFlow::ImplicitVarargsArray).getCall() = arg.getCall()
+    )
+    or
+    FlowSummaryImpl::Private::Steps::summaryLocalStep(src, sink, false)
+  }
+
+  /**
+   * Holds if the additional step from `src` to `sink` should be included in all
+   * global taint flow configurations.
+   */
+  cached
+  predicate defaultAdditionalTaintStep(DataFlow::Node src, DataFlow::Node sink) {
+    localAdditionalTaintStep(src, sink) or
+    any(AdditionalTaintStep a).step(src, sink)
+  }
+
+  /**
+   * Holds if `node` should be a sanitizer in all global taint flow configurations
+   * but not in local taint.
+   */
+  cached
+  predicate defaultTaintSanitizer(DataFlow::Node node) {
+    // Ignore paths through test code.
+    node.getEnclosingCallable().getDeclaringType() instanceof NonSecurityTestClass or
+    node.asExpr() instanceof ValidatedVariableAccess
+  }
 }
 
-/**
- * Holds if taint can flow in one local step from `src` to `sink` excluding
- * local data flow steps. That is, `src` and `sink` are likely to represent
- * different objects.
- */
-predicate localAdditionalTaintStep(DataFlow::Node src, DataFlow::Node sink) {
-  localAdditionalTaintExprStep(src.asExpr(), sink.asExpr())
-  or
-  localAdditionalTaintUpdateStep(src.asExpr(),
-    sink.(DataFlow::PostUpdateNode).getPreUpdateNode().asExpr())
-  or
-  exists(Argument arg |
-    src.asExpr() = arg and
-    arg.isVararg() and
-    sink.(DataFlow::ImplicitVarargsArray).getCall() = arg.getCall()
-  )
-  or
-  FlowSummaryImpl::Private::Steps::summaryLocalStep(src, sink, false)
-}
-
-/**
- * Holds if the additional step from `src` to `sink` should be included in all
- * global taint flow configurations.
- */
-predicate defaultAdditionalTaintStep(DataFlow::Node src, DataFlow::Node sink) {
-  localAdditionalTaintStep(src, sink) or
-  any(AdditionalTaintStep a).step(src, sink)
-}
-
-/**
- * Holds if `node` should be a sanitizer in all global taint flow configurations
- * but not in local taint.
- */
-predicate defaultTaintSanitizer(DataFlow::Node node) {
-  // Ignore paths through test code.
-  node.getEnclosingCallable().getDeclaringType() instanceof NonSecurityTestClass or
-  node.asExpr() instanceof ValidatedVariableAccess
-}
+import Cached
 
 /**
  * Holds if taint can flow in one local step from `src` to `sink` excluding