JS: Improve performance of trackUseNode

javascript/ql/lib/semmle/javascript/ApiGraphs.qll

@@ -1041,7 +1041,7 @@ module API {
         )
         or
         // property reads
-        exists(DataFlow::SourceNode src, DataFlow::SourceNode pred, string propDesc |
+        exists(RawSourceNode src, RawSourceNode pred, string propDesc |
           use(base, src) and
           pred = trackUseNode(src, false, 0, propDesc) and
           propertyRead(pred, propDesc, lbl, ref) and
@@ -1050,9 +1050,7 @@ module API {
           (base instanceof TNonModuleDef or base instanceof TUse)
         )
         or
-        exists(DataFlow::SourceNode src, DataFlow::SourceNode pred |
-          use(base, src) and pred = trackUseNode(src)
-        |
+        exists(RawSourceNode src, RawSourceNode pred | use(base, src) and pred = trackUseNode(src) |
           lbl = Label::instance() and
           ref = pred.getAnInstantiation()
           or
@@ -1253,6 +1251,35 @@ module API {
 
     private import semmle.javascript.dataflow.TypeTracking
 
+    /**
+     * A version of `DataFlow::SourceNode` without a charpred, to avoid redundant type checks that the
+     * optimizer is not yet able to remove. Should only be used in cases where we know only `SourceNode` values
+     * will appear.
+     */
+    private class RawSourceNode extends DataFlow::Node {
+      predicate flowsTo(DataFlow::Node node) { this.(DataFlow::SourceNode).flowsTo(node) }
+
+      predicate flowsToExpr(Expr expr) { this.(DataFlow::SourceNode).flowsToExpr(expr) }
+
+      DataFlow::Node getALocalUse() { result = this.(DataFlow::SourceNode).getALocalUse() }
+
+      DataFlow::PropRead getAPropertyRead(string prop) {
+        result = this.(DataFlow::SourceNode).getAPropertyRead(prop)
+      }
+
+      DataFlow::PropWrite getAPropertyWrite() {
+        result = this.(DataFlow::SourceNode).getAPropertyWrite()
+      }
+
+      DataFlow::InvokeNode getAnInvocation() {
+        result = this.(DataFlow::SourceNode).getAnInvocation()
+      }
+
+      DataFlow::NewNode getAnInstantiation() {
+        result = this.(DataFlow::SourceNode).getAnInstantiation()
+      }
+    }
+
     /**
      * Gets a data-flow node to which `nd`, which is a use of an API-graph node, flows.
      *
@@ -1266,72 +1293,112 @@ module API {
      *   - `prop`: if non-empty, the flow is only guaranteed to preserve the value of this property,
      *     and not necessarily the entire object.
      */
-    private DataFlow::SourceNode trackUseNode(
-      DataFlow::SourceNode nd, boolean promisified, int boundArgs, string prop,
-      DataFlow::TypeTracker t
+    pragma[noopt]
+    private RawSourceNode trackUseNode(
+      RawSourceNode nd, boolean promisified, int boundArgs, string prop, DataFlow::TypeTracker t
     ) {
-      t.start() and
       use(_, nd) and
+      t.start() and
       result = nd and
       promisified = false and
       boundArgs = 0 and
       prop = ""
       or
-      exists(Promisify::PromisifyCall promisify |
-        trackUseNode(nd, false, boundArgs, prop, t.continue()).flowsTo(promisify.getArgument(0)) and
+      exists(RawSourceNode prev, boolean prevPromisified, int prevBoundArgs, string prevProp |
+        prev = trackUseNode(nd, prevPromisified, prevBoundArgs, prevProp, t)
+      |
+        promisificationBigStep(prev, result) and
+        validPromisificationState(prevPromisified, prevProp) and
         promisified = true and
-        prop = "" and
-        result = promisify
-      )
-      or
-      exists(DataFlow::PartialInvokeNode pin, DataFlow::Node pred, int predBoundArgs |
-        trackUseNode(nd, promisified, predBoundArgs, prop, t.continue()).flowsTo(pred) and
-        prop = "" and
-        result = pin.getBoundFunction(pred, boundArgs - predBoundArgs) and
-        boundArgs in [0 .. 10]
-      )
+        boundArgs = prevBoundArgs and
+        prop = prevProp
+        or
+        exists(int b |
+          partialInvocationBigStep(prev, result, b) and
+          boundArgs = prevBoundArgs + b and
+          promisified = prevPromisified and
+          prop = prevProp
+        )
+        or
+        loadStoreBigStep(prev, result, prop) and
+        validLoadStoreBigState(prevPromisified, prevBoundArgs) and
+        promisified = prevPromisified and
+        boundArgs = prevBoundArgs and
+        prevProp = [prop, ""]
+      ) and
+      t.end() // 't' must be a valid ending point for the above cases (i.e. not inside a content)
       or
-      exists(DataFlow::SourceNode mid |
+      exists(RawSourceNode mid |
         mid = trackUseNode(nd, promisified, boundArgs, prop, t) and
-        AdditionalUseStep::step(pragma[only_bind_out](mid), result)
+        AdditionalUseStep::step(mid, result)
       )
       or
-      exists(DataFlow::Node pred, string preprop |
-        trackUseNode(nd, promisified, boundArgs, preprop, t.continue()).flowsTo(pred) and
-        promisified = false and
-        boundArgs = 0 and
-        SharedTypeTrackingStep::loadStoreStep(pred, result, prop)
+      exists(RawSourceNode prev, DataFlow::TypeTracker prevT |
+        prev = trackUseNode(nd, promisified, boundArgs, prop, prevT)
       |
-        prop = preprop
+        exists(StepSummary summary |
+          restrictedBigStep(prev, result, summary) and
+          t = prevT.append(summary)
+        )
         or
-        preprop = ""
+        t.hasCall() = false and
+        returnBigStep(prev, result) and
+        t = prevT
       )
-      or
-      t = useStep(nd, promisified, boundArgs, prop, result)
     }
 
-    /**
-     * Holds if `nd`, which is a use of an API-graph node, flows in zero or more potentially
-     * inter-procedural steps to some intermediate node, and then from that intermediate node to
-     * `res` in one step. The entire flow is described by the resulting `TypeTracker`.
-     *
-     * This predicate exists solely to enforce a better join order in `trackUseNode` above.
-     */
-    pragma[noopt]
-    private DataFlow::TypeTracker useStep(
-      DataFlow::Node nd, boolean promisified, int boundArgs, string prop, DataFlow::Node res
+    pragma[nomagic]
+    private predicate validPromisificationState(boolean promisified, string prop) {
+      promisified = false and
+      prop = ""
+    }
+
+    pragma[nomagic]
+    private predicate validLoadStoreBigState(boolean promisified, int boundArgs) {
+      promisified = false and boundArgs = 0
+    }
+
+    pragma[nomagic]
+    private predicate promisificationBigStep(DataFlow::SourceNode node1, DataFlow::SourceNode node2) {
+      exists(Promisify::PromisifyCall promisify |
+        node1 = promisify.getArgument(0).getALocalSource() and
+        node2 = promisify
+      )
+    }
+
+    pragma[nomagic]
+    private predicate partialInvocationBigStep(
+      DataFlow::SourceNode node1, DataFlow::SourceNode node2, int boundArgs
+    ) {
+      node2 = any(DataFlow::PartialInvokeNode pin).getBoundFunction(node1.getALocalUse(), boundArgs)
+    }
+
+    pragma[nomagic]
+    private predicate loadStoreBigStep(
+      DataFlow::SourceNode node1, DataFlow::SourceNode node2, string prop
     ) {
-      exists(DataFlow::TypeTracker t, StepSummary summary, DataFlow::SourceNode prev |
-        prev = trackUseNode(nd, promisified, boundArgs, prop, t) and
-        StepSummary::step(prev, res, summary) and
-        result = t.append(summary) and
+      SharedTypeTrackingStep::loadStoreStep(node1.getALocalUse(), node2, prop)
+    }
+
+    pragma[nomagic]
+    private predicate restrictedBigStep(DataFlow::Node node1, DataFlow::Node node2, StepSummary step) {
+      StepSummary::step(node1, node2, step) and
+      not (
         // Block argument-passing into 'this' when it determines the call target
-        not summary = CallReceiverStep()
+        step = CallReceiverStep()
+        or
+        // Special-case return to avoid large fan-out
+        step = ReturnStep()
       )
     }
 
-    private DataFlow::SourceNode trackUseNode(
-      DataFlow::SourceNode nd, boolean promisified, int boundArgs, string prop
+    pragma[nomagic]
+    private predicate returnBigStep(DataFlow::SourceNode node1, DataFlow::Node node2) {
+      StepSummary::step(node1, node2, ReturnStep())
+    }
+
+    private RawSourceNode trackUseNode(
+      RawSourceNode nd, boolean promisified, int boundArgs, string prop
     ) {
       result = trackUseNode(nd, promisified, boundArgs, prop, DataFlow::TypeTracker::end())
     }
@@ -1340,9 +1407,7 @@ module API {
      * Gets a node that is inter-procedurally reachable from `nd`, which is a use of some node.
      */
     cached
-    DataFlow::SourceNode trackUseNode(DataFlow::SourceNode nd) {
-      result = trackUseNode(nd, false, 0, "")
-    }
+    RawSourceNode trackUseNode(RawSourceNode nd) { result = trackUseNode(nd, false, 0, "") }
 
     private DataFlow::SourceNode trackDefNode(DataFlow::Node nd, DataFlow::TypeBackTracker t) {
       t.start() and

javascript/ql/lib/semmle/javascript/dataflow/internal/StepSummary.qll

@@ -67,7 +67,13 @@ private module Cached {
    */
   cached
   predicate step(DataFlow::SourceNode pred, DataFlow::SourceNode succ, StepSummary summary) {
-    exists(DataFlow::Node mid | pred.flowsTo(mid) | StepSummary::smallstep(mid, succ, summary))
+    exists(DataFlow::Node mid |
+      pred.flowsTo(mid) and
+      StepSummary::smallstep(mid, succ, summary) and
+      (pred = mid or not mid instanceof DataFlow::SourceNode)
+    )
+    or
+    pred.flowsTo(succ) and summary = LevelStep() and pred != succ
   }
 
   pragma[nomagic]