Merge remote-tracking branch 'upstream/master' into mergeback-20181130

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

@@ -70,6 +70,7 @@
 
 import javascript
 private import internal.FlowSteps
+private import internal.AccessPaths
 
 /**
  * A data flow tracking configuration for finding inter-procedural paths from
@@ -291,26 +292,12 @@ abstract class BarrierGuardNode extends DataFlow::Node {
       )
     )
     or
-    // 2) `nd` is a use of an SSA variable `ssa`, and dominated by a barrier for `ssa`
-    exists (SsaVariable ssa, BasicBlock bb |
-      nd = DataFlow::valueNode(ssa.getAUseIn(bb)) and
-      exists (ConditionGuardNode cond |
-        asExpr() = cond.getTest() and
-        blocks(cond.getOutcome(), ssa.getAUse()) and
-        cond.dominates(bb)
-      )
-    )
-    or
-    // 3) `nd` is a property access `ssa.p.q` on an SSA variable `ssa`, and dominated by
-    // a barrier for `ssa.p.q`
-    exists (SsaVariable ssa, string props, BasicBlock bb |
-      nd = DataFlow::valueNode(nestedPropAccessOnSsaVar(ssa, props)) and
-      bb = nd.getBasicBlock() |
-      exists (ConditionGuardNode cond |
-        asExpr() = cond.getTest() and
-        blocks(cond.getOutcome(), nestedPropAccessOnSsaVar(ssa, props)) and
-        cond.dominates(bb)
-      )
+    // 2) `nd` is an instance of an access path `p`, and dominated by a barrier for `p`
+    exists (AccessPath p, BasicBlock bb, ConditionGuardNode cond |
+      nd = DataFlow::valueNode(p.getAnInstanceIn(bb)) and
+      asExpr() = cond.getTest() and
+      blocks(cond.getOutcome(), p.getAnInstance()) and
+      cond.dominates(bb)
     )
   }
 
@@ -330,20 +317,6 @@ abstract class LabeledBarrierGuardNode extends BarrierGuardNode {
   abstract FlowLabel getALabel();
 }
 
-/**
- * Holds if `props` is a string of the form `p.q.r`, and the result is a property access
- * of the form `v.p.q.r`.
- */
-private DotExpr nestedPropAccessOnSsaVar(SsaVariable v, string props) {
-  exists (Expr base, string prop | result.accesses(base, prop) |
-    base = v.getAUse() and props = prop
-    or
-    exists (string prevProps |
-      base = nestedPropAccessOnSsaVar(v, prevProps) and
-      props = prevProps + "." + prop
-    )
-  )
-}
 
 /**
  * A data flow edge that should be added to all data flow configurations in

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

@@ -254,22 +254,18 @@ class AnalyzedFunction extends DataFlow::AnalyzedValueNode {
    * Gets a return value for a call to this function.
    */
   AbstractValue getAReturnValue() {
-    if astNode.isGenerator() or astNode.isAsync() then
-      result = TAbstractOtherObject()
-    else (
-      // explicit return value
-      result = astNode.getAReturnedExpr().analyze().getALocalValue()
+    // explicit return value
+    result = astNode.getAReturnedExpr().analyze().getALocalValue()
+    or
+    // implicit return value
+    (
+      // either because execution of the function may terminate normally
+      mayReturnImplicitly()
       or
-      // implicit return value
-      (
-        // either because execution of the function may terminate normally
-        mayReturnImplicitly()
-        or
-        // or because there is a bare `return;` statement
-        exists (ReturnStmt ret | ret = astNode.getAReturnStmt() | not exists(ret.getExpr()))
-      ) and
-      result = TAbstractUndefined()
-    )
+      // or because there is a bare `return;` statement
+      exists (ReturnStmt ret | ret = astNode.getAReturnStmt() | not exists(ret.getExpr()))
+    ) and
+    result = TAbstractUndefined()
   }
 
   /**
@@ -288,5 +284,26 @@ class AnalyzedFunction extends DataFlow::AnalyzedValueNode {
       not final instanceof ThrowStmt
     )
   }
-
+}
+
+/**
+ * Flow analysis for generator functions.
+ */
+private class AnalyzedGeneratorFunction extends AnalyzedFunction {
+  AnalyzedGeneratorFunction() { astNode.isGenerator() }
+
+  override AbstractValue getAReturnValue() {
+    result = TAbstractOtherObject()
+  }
+}
+
+/**
+ * Flow analysis for async functions.
+ */
+private class AnalyzedAsyncFunction extends AnalyzedFunction {
+  AnalyzedAsyncFunction() { astNode.isAsync() }
+
+  override AbstractValue getAReturnValue() {
+    result = TAbstractOtherObject()
+  }
 }

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

@@ -39,16 +39,7 @@ AbstractValue getAnInitialPropertyValue(DefiniteAbstractValue baseVal, string pr
   )
   or
   // class members
-  exists (ClassDefinition c, DataFlow::AnalyzedNode init, MemberDefinition m |
-    m = c.getMember(propertyName) and
-    not m instanceof AccessorMethodDefinition and
-    init = m.getInit().analyze() and
-    result = init.getALocalValue() |
-    if m.isStatic() then
-      baseVal = TAbstractClass(c)
-    else
-      baseVal = AbstractInstance::of(c)
-  )
+  result = getAnInitialMemberValue(getMember(baseVal, propertyName))
   or
   // object properties
   exists (ValueProperty p |
@@ -63,6 +54,30 @@ AbstractValue getAnInitialPropertyValue(DefiniteAbstractValue baseVal, string pr
   result = TAbstractInstance(baseVal)
 }
 
+/**
+ * Gets a class member definition that we abstractly represent as a property of `baseVal`
+ * with the given `name`.
+ */
+private MemberDefinition getMember(DefiniteAbstractValue baseVal, string name) {
+  exists (ClassDefinition c | result = c.getMember(name) |
+    if result.isStatic() then
+      baseVal = TAbstractClass(c)
+    else
+      baseVal = AbstractInstance::of(c)
+  )
+}
+
+/**
+ * Gets an abstract representation of the initial value of member definition `m`.
+ *
+ * For (non-accessor) methods, this is the abstract function corresponding to the
+ * method. For fields, it is an abstract representation of their initial value(s).
+ */
+private AbstractValue getAnInitialMemberValue(MemberDefinition m) {
+  not m instanceof AccessorMethodDefinition and
+  result = m.getInit().analyze().getALocalValue()
+}
+
 /**
  * Holds if `baseVal` is an abstract value whose properties we track for the purposes
  * of `getALocalValue`.

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

@@ -0,0 +1,117 @@
+/**
+ * Provides classes for reasoning about certain property accesses as access paths.
+ *
+ * Access paths represent SSA variables with zero or more property accesses applied to them.
+ * Each property name in the access must either be constant or itself be a use of an SSA
+ * variable.
+ *
+ * If `x` and `y` are SSA variables, then `x`, `x.p`, `x.p["q"]`, `x[y]` and `x[y].q` are
+ * access paths, but `x[x.y]` is not an access path.
+ *
+ * Access paths can be used to identify expressions that have the same value, disregarding
+ * any heap modifications. In general, expressions that are instances of the same access
+ * path are not guaranteed to evaluate to the same value nor do all expressions that evaluate
+ * to the same value have the same access paths, so access paths are neither sound nor
+ * complete as an approximation of expression semantics.
+ */
+
+import javascript
+
+/**
+ * A representation of a property name that is either statically known or is
+ * the value of an SSA variable.
+ */
+private newtype PropertyName =
+  StaticPropertyName(string name) {
+    exists (PropAccess pa | name = pa.getPropertyName())
+  }
+  or
+  DynamicPropertyName(SsaVariable var) {
+    exists (PropAccess pa | pa.getPropertyNameExpr() = var.getAUse())
+  }
+
+/**
+ * Gets the representation of the property name of `pacc`, if any.
+ */
+private PropertyName getPropertyName(PropAccess pacc) {
+  result = StaticPropertyName(pacc.getPropertyName())
+  or
+  exists (SsaVariable var |
+    pacc.getPropertyNameExpr() = var.getAUse() and
+    result = DynamicPropertyName(var)
+  )
+}
+
+/**
+ * A representation of a (nested) property access on an SSA variable
+ * where each property name is either constant or itself an SSA variable.
+ */
+private newtype TAccessPath =
+  MkRoot(SsaVariable var)
+  or
+  MkAccessStep(AccessPath base, PropertyName name) {
+    exists (PropAccess pacc |
+      pacc.getBase() = base.getAnInstance() and
+      getPropertyName(pacc) = name
+    )
+  }
+
+/**
+ * A representation of a (nested) property access on an SSA variable
+ * where each property name is either constant or itself an SSA variable.
+ */
+class AccessPath extends TAccessPath {
+  /**
+   * Gets an expression in `bb` represented by this access path.
+   */
+  Expr getAnInstanceIn(BasicBlock bb) {
+    exists (SsaVariable var |
+      this = MkRoot(var) and
+      result = var.getAUseIn(bb)
+    )
+    or
+    exists (PropertyName name |
+      result = getABaseInstanceIn(bb, name) and
+      getPropertyName(result) = name
+    )
+  }
+
+  /**
+   * Gets a property access in `bb` whose base is represented by the
+   * base of this access path, and where `name` is bound to the last
+   * component of this access path.
+   *
+   * This is an auxiliary predicate that's needed to enforce a better
+   * join order in `getAnInstanceIn` above.
+   */
+  pragma[noinline]
+  private PropAccess getABaseInstanceIn(BasicBlock bb, PropertyName name) {
+    exists (AccessPath base | this = MkAccessStep(base, name) |
+      result.getBase() = base.getAnInstanceIn(bb)
+    )
+  }
+
+  /**
+   * Gets an expression represented by this access path.
+   */
+  Expr getAnInstance() {
+    result = getAnInstanceIn(_)
+  }
+
+  /**
+   * Gets a textual representation of this access path.
+   */
+  string toString() {
+    exists (SsaVariable var | this = MkRoot(var) |
+      result = var.getSourceVariable().getName()
+    )
+    or
+    exists (AccessPath base, PropertyName name, string rest |
+      rest = "." + any(string s | name = StaticPropertyName(s))
+      or
+      rest = "[" + any(SsaVariable var | name = DynamicPropertyName(var)).getSourceVariable().getName() + "]" |
+      result = base.toString() + rest and
+      this = MkAccessStep(base, name)
+    )
+  }
+}