JS: Fix perf issue from overriding isIncomplete

Author: asgerf

javascript/ql/src/semmle/javascript/dataflow/DataFlow.qll

@@ -22,6 +22,7 @@ import javascript
 private import internal.CallGraphs
 private import internal.FlowSteps as FlowSteps
 private import internal.DataFlowNode
+private import internal.AnalyzedParameters
 
 module DataFlow {
   /**
@@ -1527,9 +1528,13 @@ module DataFlow {
       e instanceof TaggedTemplateExpr
       or
       e instanceof Parameter and
-      not localArgumentPassing(_, e)
+      not localArgumentPassing(_, e) and
+      not isAnalyzedParameter(e) and
+      not e.(Parameter).isRestParameter()
     )
     or
+    nd.(AnalyzedParameter).hasIncompleteness(cause)
+    or
     nd.asExpr() instanceof ExternalModuleReference and
     cause = "import"
     or

javascript/ql/src/semmle/javascript/dataflow/internal/AnalyzedParameters.qll

@@ -0,0 +1,50 @@
+private import javascript
+private import VariableTypeInference
+
+/**
+ * Holds if `p` is analyzed precisely by the type inference.
+ */
+pragma[nomagic]
+predicate isAnalyzedParameter(Parameter p) {
+  exists(FunctionWithAnalyzedParameters f, int parmIdx | p = f.getParameter(parmIdx) |
+    // we cannot track flow into rest parameters
+    not p.(Parameter).isRestParameter()
+  )
+}
+
+/**
+ * A parameter whose value is propagated interprocedurally.
+ */
+class AnalyzedParameter extends AnalyzedValueNode {
+  override Parameter astNode;
+
+  AnalyzedParameter() { isAnalyzedParameter(astNode) }
+
+  FunctionWithAnalyzedParameters getFunction() { astNode = result.getAParameter() }
+
+  override AbstractValue getALocalValue() {
+    exists(DataFlow::AnalyzedNode pred |
+      getFunction().argumentPassing(astNode, pred.asExpr()) and
+      result = pred.getALocalValue()
+    )
+    or
+    not getFunction().mayReceiveArgument(astNode) and
+    result = TAbstractUndefined()
+    or
+    result = astNode.getDefault().analyze().getALocalValue()
+  }
+
+  /**
+   * Whether this node should be considered incomplete with the given cause.
+   *
+   * For performance reasons, this is not an override of `isIncomplete`, but is
+   * explicitly included in that predicate.
+   */
+  predicate hasIncompleteness(DataFlow::Incompleteness cause) {
+    getFunction().isIncomplete(cause)
+    or
+    not getFunction().argumentPassing(astNode, _) and
+    getFunction().mayReceiveArgument(astNode) and
+    cause = "call"
+  }
+}

javascript/ql/src/semmle/javascript/dataflow/internal/VariableTypeInference.qll

@@ -6,6 +6,7 @@
 
 private import javascript
 private import AbstractValuesImpl
+private import AnalyzedParameters
 private import semmle.javascript.dataflow.InferredTypes
 private import semmle.javascript.dataflow.Refinements
 
@@ -140,41 +141,6 @@ class AnalyzedVarDef extends VarDef {
   TopLevel getTopLevel() { result = this.(ASTNode).getTopLevel() }
 }
 
-private predicate isAnalyzedParameter(Parameter p) {
-  exists(FunctionWithAnalyzedParameters f, int parmIdx | p = f.getParameter(parmIdx) |
-    // we cannot track flow into rest parameters
-    not p.(Parameter).isRestParameter()
-  )
-}
-
-private class AnalyzedParameter extends AnalyzedValueNode {
-  override Parameter astNode;
-
-  AnalyzedParameter() { isAnalyzedParameter(astNode) }
-
-  FunctionWithAnalyzedParameters getFunction() { astNode = result.getAParameter() }
-
-  override AbstractValue getALocalValue() {
-    exists(DataFlow::AnalyzedNode pred |
-      getFunction().argumentPassing(astNode, pred.asExpr()) and
-      result = pred.getALocalValue()
-    )
-    or
-    not getFunction().mayReceiveArgument(astNode) and
-    result = TAbstractUndefined()
-    or
-    result = astNode.getDefault().analyze().getALocalValue()
-  }
-
-  override predicate isIncomplete(DataFlow::Incompleteness cause) {
-    getFunction().isIncomplete(cause)
-    or
-    not getFunction().argumentPassing(astNode, _) and
-    getFunction().mayReceiveArgument(astNode) and
-    cause = "call"
-  }
-}
-
 /**
  * Flow analysis for simple parameters of selected functions.
  */
@@ -193,8 +159,6 @@ private class AnalyzedRestParameter extends AnalyzedValueNode {
   AnalyzedRestParameter() { astNode.(Parameter).isRestParameter() }
 
   override AbstractValue getALocalValue() { result = TAbstractOtherObject() }
-
-  override predicate isIncomplete(DataFlow::Incompleteness cause) { none() }
 }
 
 /**
@@ -679,7 +643,7 @@ abstract class FunctionWithAnalyzedParameters extends Function {
    * Holds if `p` is a parameter of this function and `arg` is
    * the corresponding argument.
    */
-  abstract predicate argumentPassing(SimpleParameter p, Expr arg);
+  abstract predicate argumentPassing(Parameter p, Expr arg);
 
   /**
    * Holds if `p` is a parameter of this function that may receive a value from an argument.
@@ -699,7 +663,7 @@ abstract private class CallWithAnalyzedParameters extends FunctionWithAnalyzedPa
    */
   abstract DataFlow::InvokeNode getAnInvocation();
 
-  override predicate argumentPassing(SimpleParameter p, Expr arg) {
+  override predicate argumentPassing(Parameter p, Expr arg) {
     exists(DataFlow::InvokeNode invk, int argIdx | invk = getAnInvocation() |
       p = getParameter(argIdx) and
       not p.isRestParameter() and

javascript/ql/test/library-tests/DataFlow/incomplete.expected

@@ -4,7 +4,6 @@
 | eval.js:3:3:3:16 | eval("x = 23") | call |
 | eval.js:3:3:3:16 | exceptional return of eval("x = 23") | call |
 | sources.js:1:1:1:12 | exceptional return of new (x => x) | call |
-| sources.js:1:6:1:6 | x | call |
 | sources.js:1:6:1:11 | exceptional return of anonymous function | call |
 | sources.js:3:1:5:6 | exceptional return of (functi ... \\n})(23) | call |
 | sources.js:3:2:5:1 | exceptional return of anonymous function | call |
@@ -20,7 +19,6 @@
 | tst2.ts:8:3:8:5 | A.x | heap |
 | tst2.ts:11:11:11:13 | A.x | heap |
 | tst2.ts:13:26:13:29 | List | global |
-| tst2.ts:13:39:13:38 | args | call |
 | tst2.ts:13:39:13:38 | exceptional return of default constructor of class StringList | call |
 | tst2.ts:13:39:13:38 | exceptional return of super(...args) | call |
 | tst2.ts:13:39:13:38 | super | call |