Merge pull request #731 from xiemaisi/js/performance-fiddling

Approved by asger-semmle, esben-semmle

Author: semmle-qlci

javascript/ql/src/LanguageFeatures/InconsistentNew.ql

@@ -28,41 +28,20 @@ predicate guardsAgainstMissingNew(Function f) {
   )
 }
 
-/**
- * Holds if `callee` is a function that may be invoked at callsite `cs`,
- * where `imprecision` is a heuristic measure of how likely it is that `callee`
- * is only suggested as a potential callee due to imprecise analysis of global
- * variables and is not, in fact, a viable callee at all.
- */
-predicate calls(DataFlow::InvokeNode cs, Function callee, int imprecision) {
-  callee = cs.getACallee() and
-  (
-    // if global flow was used to derive the callee, we may be imprecise
-    if cs.isIndefinite("global")
-    then
-      // callees within the same file are probably genuine
-      callee.getFile() = cs.getFile() and imprecision = 0
-      or
-      // calls to global functions declared in an externs file are fairly
-      // safe as well
-      callee.inExternsFile() and imprecision = 1
-      or
-      // otherwise we make worst-case assumptions
-      imprecision = 2
-    else
-      // no global flow, so no imprecision
-      imprecision = 0
-  )
-}
-
 /**
  * Gets a function that may be invoked at `cs`, preferring callees that
  * are less likely to be derived due to analysis imprecision and excluding
  * whitelisted call sites and callees. Additionally, `isNew` is bound to
  * `true` if `cs` is a `new` expression, and to `false` otherwise.
  */
 Function getALikelyCallee(DataFlow::InvokeNode cs, boolean isNew) {
-  calls(cs, result, min(int p | calls(cs, _, p))) and
+  result = min(Function callee, int imprecision |
+      callee = cs.getACallee(imprecision)
+    |
+      callee
+      order by
+        imprecision
+    ) and
   not cs.isUncertain() and
   not whitelistedCall(cs) and
   not whitelistedCallee(result) and

javascript/ql/src/Statements/UselessConditional.ql

@@ -17,18 +17,27 @@ import semmle.javascript.RestrictedLocations
 import semmle.javascript.dataflow.Refinements
 import semmle.javascript.DefensiveProgramming
 
+/**
+ * Gets the unique definition of `v`.
+ *
+ * If `v` has no definitions or more than one, or if its single definition
+ * is a destructuring assignment, this predicate is undefined.
+ */
+VarDef getSingleDef(Variable v) {
+  strictcount(VarDef vd | vd.getAVariable() = v) = 1 and
+  result.getTarget() = v.getAReference()
+}
+
 /**
  * Holds if variable `v` looks like a symbolic constant, that is, it is assigned
  * exactly once, either in a `const` declaration or with a constant initializer.
  *
  * We do not consider conditionals to be useless if they check a symbolic constant.
  */
 predicate isSymbolicConstant(Variable v) {
-  // defined exactly once
-  count(VarDef vd | vd.getAVariable() = v) = 1 and
-  // the definition is either a `const` declaration or it assigns a constant to it
-  exists(VarDef vd | vd.getAVariable() = v and count(vd.getAVariable()) = 1 |
-    vd.(VariableDeclarator).getDeclStmt() instanceof ConstDeclStmt or
+  exists(VarDef vd | vd = getSingleDef(v) |
+    vd.(VariableDeclarator).getDeclStmt() instanceof ConstDeclStmt
+    or
     isConstant(vd.getSource())
   )
 }

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

@@ -78,21 +78,20 @@ class InvokeNode extends DataFlow::SourceNode {
   }
 
   /** Gets an abstract value representing possible callees of this call site. */
-  cached
-  AbstractValue getACalleeValue() { result = impl.getCalleeNode().analyze().getAValue() }
-
-  /** Gets a potential callee based on dataflow analysis results. */
-  private Function getACalleeFromDataflow() {
-    result = getACalleeValue().(AbstractCallable).getFunction()
-  }
+  AbstractValue getACalleeValue() { result = InvokeNode::getACalleeValue(this) }
 
   /** Gets a potential callee of this call site. */
-  Function getACallee() {
-    result = getACalleeFromDataflow()
-    or
-    not exists(getACalleeFromDataflow()) and
-    result = impl.(DataFlow::Impl::ExplicitInvokeNode).asExpr().(InvokeExpr).getResolvedCallee()
-  }
+  Function getACallee() { result = InvokeNode::getACallee(this) }
+
+  /**
+   * Gets a callee of this call site where `imprecision` is a heuristic measure of how
+   * likely it is that `callee` is only suggested as a potential callee due to
+   * imprecise analysis of global variables and is not, in fact, a viable callee at all.
+   *
+   * Callees with imprecision zero, in particular, have either been derived without
+   * considering global variables, or are calls to a global variable within the same file.
+   */
+  Function getACallee(int imprecision) { result = InvokeNode::getACallee(this, imprecision) }
 
   /**
    * Holds if the approximation of possible callees for this call site is
@@ -133,6 +132,60 @@ class InvokeNode extends DataFlow::SourceNode {
   predicate isUncertain() { isImprecise() or isIncomplete() }
 }
 
+/** Auxiliary module used to cache a few related predicates together. */
+cached
+private module InvokeNode {
+  /** Gets an abstract value representing possible callees of `invk`. */
+  cached
+  AbstractValue getACalleeValue(InvokeNode invk) {
+    result = invk.getCalleeNode().analyze().getAValue()
+  }
+
+  /** Gets a potential callee of `invk` based on dataflow analysis results. */
+  private Function getACalleeFromDataflow(InvokeNode invk) {
+    result = getACalleeValue(invk).(AbstractCallable).getFunction()
+  }
+
+  /** Gets a potential callee of `invk`. */
+  cached
+  Function getACallee(InvokeNode invk) {
+    result = getACalleeFromDataflow(invk)
+    or
+    not exists(getACalleeFromDataflow(invk)) and
+    result = invk.(DataFlow::Impl::ExplicitInvokeNode).asExpr().(InvokeExpr).getResolvedCallee()
+  }
+
+  /**
+   * Gets a callee of `invk` where `imprecision` is a heuristic measure of how
+   * likely it is that `callee` is only suggested as a potential callee due to
+   * imprecise analysis of global variables and is not, in fact, a viable callee at all.
+   *
+   * Callees with imprecision zero, in particular, have either been derived without
+   * considering global variables, or are calls to a global variable within the same file.
+   */
+  cached
+  Function getACallee(InvokeNode invk, int imprecision) {
+    result = getACallee(invk) and
+    (
+      // if global flow was used to derive the callee, we may be imprecise
+      if invk.isIndefinite("global")
+      then
+        // callees within the same file are probably genuine
+        result.getFile() = invk.getFile() and imprecision = 0
+        or
+        // calls to global functions declared in an externs file are fairly
+        // safe as well
+        result.inExternsFile() and imprecision = 1
+        or
+        // otherwise we make worst-case assumptions
+        imprecision = 2
+      else
+        // no global flow, so no imprecision
+        imprecision = 0
+    )
+  }
+}
+
 /** A data flow node corresponding to a function call without `new`. */
 class CallNode extends InvokeNode {
   override DataFlow::Impl::CallNodeDef impl;

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

@@ -20,12 +20,7 @@ predicate shouldTrackProperties(AbstractValue obj) {
 /**
  * Holds if `invk` may invoke `f`.
  */
-predicate calls(DataFlow::InvokeNode invk, Function f) {
-  if invk.isIndefinite("global")
-  then (
-    f = invk.getACallee() and f.getFile() = invk.getFile()
-  ) else f = invk.getACallee()
-}
+predicate calls(DataFlow::InvokeNode invk, Function f) { f = invk.getACallee(0) }
 
 /**
  * Holds if `invk` may invoke `f` indirectly through the given `callback` argument.