Model aliasing effects for array loop operators (#2570)

Summary:
This PR guarantees the correctness of optimized `Array.map` operators, even in the face of aliasing effects. It does four things:

1. Trigger generic leaking if an array operator is re-specialized in the non-Instant Render use case.
2. Tracks aliasing effects created by specialized operators, and triggers leaking or materialization when needed to ensure correct behavior
3. Deactivates immediate transitive materialization following the use of specialized operators, instead deferring this to the leaking implementation. The leaking implementation reaches aliased objects via a new arg added to widened numeric arrays. The arg is an abstract value of kind "mayAliasSet" that is set to top, but whose may alias set is tracked. If leaking does not happen, then materialization is avoided.
4. It permits benign mutations in Instant Render, where the mutations do not cause references to non-final snapshots of the object.

Follow up:
- Model aliasing effects losslessly via widened objects: #2569
- Add support for `filter` and `reduce`

Resolves #2449
Pull Request resolved: https://github.com/facebook/prepack/pull/2570

Differential Revision: D10149117

Pulled By: sb98052

fbshipit-source-id: eb686982574c8ef868934472903c405f3d63bbed

src/intrinsics/ecma262/Array.js

@@ -243,7 +243,9 @@ export default function(realm: Realm): NativeFunctionValue {
           if (thisArg) {
             args.push(thisArg);
           }
-          possibleNestedOptimizedFunctions = [{ func: mapfn, thisValue: thisArg || realm.intrinsics.undefined }];
+          possibleNestedOptimizedFunctions = [
+            { func: mapfn, thisValue: thisArg || realm.intrinsics.undefined, kind: "map" },
+          ];
         }
         Leak.value(realm, items);
         return ArrayValue.createTemporalWithWidenedNumericProperty(

src/intrinsics/ecma262/ArrayPrototype.js

@@ -994,7 +994,9 @@ export default function(realm: Realm, obj: ObjectValue): void {
         args.push(thisArg);
       }
       invariant(callbackfn instanceof ECMAScriptSourceFunctionValue || callbackfn instanceof BoundFunctionValue);
-      let possibleNestedOptimizedFunctions = [{ func: callbackfn, thisValue: thisArg || realm.intrinsics.undefined }];
+      let possibleNestedOptimizedFunctions = [
+        { func: callbackfn, thisValue: thisArg || realm.intrinsics.undefined, kind: "map" },
+      ];
       return ArrayValue.createTemporalWithWidenedNumericProperty(
         realm,
         args,

src/methods/get.js

@@ -43,6 +43,7 @@ import invariant from "../invariant.js";
 import type { BabelNodeTemplateLiteral } from "@babel/types";
 import { createOperationDescriptor } from "../utils/generator.js";
 import { PropertyDescriptor, AbstractJoinedDescriptor } from "../descriptors.js";
+import { IsArrayIndex } from "./is.js";
 
 // ECMA262 7.3.22
 export function GetFunctionRealm(realm: Realm, obj: ObjectValue): Realm {
@@ -120,6 +121,28 @@ export function OrdinaryGet(
         propName = P;
       }
       invariant(Receiver instanceof ObjectValue || Receiver instanceof AbstractObjectValue);
+
+      if (IsArrayIndex(realm, P)) {
+        // Deal with aliasing effects
+        invariant(val.args.length === 1);
+        let aliasSet = val.args[0];
+
+        invariant(aliasSet instanceof AbstractValue && aliasSet.kind === "mayAliasSet");
+        for (let object of aliasSet.args) {
+          // This explicit handling of aliasing should become unnecessary
+          // when we unify arrays with widened numeric properties. We have effectively
+          // pushed this leaking decision as far out as we possibly can, for now.
+          // and objects with widened properties. TODO #2569.
+          invariant(object instanceof ObjectValue);
+          // TODO: Deal with nested Array.map, in which the following
+          // pessimistic leaking call may fail because object is not tracked
+          // for leaking
+          invariant(realm.createdObjectsTrackedForLeaks !== undefined);
+          invariant(realm.createdObjectsTrackedForLeaks.has(object));
+          Leak.value(realm, object);
+        }
+      }
+
       return GetFromArrayWithWidenedNumericProperty(realm, Receiver, propName);
     } else if (!propValue) {
       AbstractValue.reportIntrospectionError(val, "abstract computed property name");

src/types.js

@@ -402,7 +402,7 @@ export type LeakType = {
 
 export type MaterializeType = {
   materializeObject(realm: Realm, object: ObjectValue): void,
-  materializeObjectsTransitive(realm: Realm, value: FunctionValue): void,
+  computeReachableObjects(realm: Realm, value: Value): Set<ObjectValue>,
 };
 
 export type PropertiesType = {

src/utils/leak.js

@@ -610,7 +610,11 @@ export class LeakImplementation {
 export class MaterializeImplementation {
   // TODO: Understand relation to snapshots: #2441
   materializeObject(realm: Realm, val: ObjectValue): void {
-    materializeObject(realm, val);
+    if (realm.instantRender.enabled)
+      // Materialization leads to runtime code that mutates objects
+      // this is at best undesirable in InstantRender
+      val.makeFinal();
+    else materializeObject(realm, val);
   }
 
   // This routine materializes objects reachable from non-local bindings read
@@ -624,31 +628,15 @@ export class MaterializeImplementation {
   // - Previously havoced locations (#2446) should be reloaded, but are currently rejected.
   // - Specialization depends on the assumption that the Array op will only be used once.
   // First, we will enforce it: #2448. Later we will relax it: #2454
-  materializeObjectsTransitive(realm: Realm, outlinedFunction: FunctionValue): void {
+  computeReachableObjects(realm: Realm, rootValue: Value): Set<ObjectValue> {
     invariant(realm.isInPureScope());
-    let objectsToMaterialize: Set<ObjectValue> = new Set();
+
+    let reachableObjects: Set<ObjectValue> = new Set();
     let visitedValues: Set<Value> = new Set();
-    computeFromValue(outlinedFunction);
+    computeFromValue(rootValue);
 
-    let handleMaterialization: ObjectValue => void;
-    if (realm.instantRender.enabled) {
-      // We prevent mutations to objects so that non-final
-      // values cannot occur, and hence materialization is avoided.
-      // The values of properties needed are issued via object literals.
-      handleMaterialization = o => {
-        o.makeFinal();
-      };
-    } else {
-      handleMaterialization = o => {
-        if (!TestIntegrityLevel(realm, o, "frozen")) materializeObject(realm, o);
-      };
-    }
+    return reachableObjects;
 
-    for (let object of objectsToMaterialize) {
-      handleMaterialization(object);
-    }
-
-    return;
     function computeFromBindings(func: FunctionValue, nonLocalReadBindings: Set<string>): void {
       invariant(func instanceof ECMAScriptSourceFunctionValue);
       let environment = func.$Environment;
@@ -697,6 +685,7 @@ export class MaterializeImplementation {
       computeFromValue(value.$ProxyHandler);
     }
     function computeFromValue(value: Value): void {
+      invariant(value !== undefined);
       if (value.isIntrinsic() || value instanceof EmptyValue || value instanceof PrimitiveValue) {
         visit(value);
       } else if (value instanceof AbstractValue) {
@@ -758,7 +747,7 @@ export class MaterializeImplementation {
           );
           break;
       }
-      if (!objectsToMaterialize.has(value)) objectsToMaterialize.add(value);
+      if (!reachableObjects.has(value)) reachableObjects.add(value);
     }
     function computeFromDescriptor(descriptor: Descriptor): void {
       if (descriptor === undefined) {
@@ -803,7 +792,7 @@ export class MaterializeImplementation {
       computeFromObjectPrototype(obj);
     }
     function computeFromObjectPrototype(obj: ObjectValue) {
-      computeFromValue(obj.$Prototype);
+      if (obj.$Prototype !== undefined) computeFromValue(obj.$Prototype);
     }
     function computeFromFunctionValue(fn: FunctionValue) {
       computeFromObjectProperties(fn);
@@ -912,7 +901,7 @@ export class MaterializeImplementation {
     function notSupportedForTransitiveMaterialization() {
       let error = new CompilerDiagnostic(
         "Not supported for transitive materialization",
-        outlinedFunction.expressionLocation,
+        rootValue.expressionLocation,
         "PP0041",
         "FatalError"
       );

src/values/AbstractValue.js

@@ -48,6 +48,7 @@ export type AbstractValueKind =
   | "!=="
   | "rebuiltProperty"
   | "abstractConcreteUnion"
+  | "mayAliasSet"
   | "build function"
   | "widened property"
   | "widened numeric property"

src/values/ArrayValue.js

@@ -10,7 +10,7 @@
 /* @flow strict-local */
 
 import type { Effects, Realm } from "../realm.js";
-import type { PropertyKeyValue, Descriptor, ObjectKind } from "../types.js";
+import type { PropertyBinding, PropertyKeyValue, Descriptor, ObjectKind } from "../types.js";
 import {
   AbstractValue,
   BoundFunctionValue,
@@ -26,9 +26,15 @@ import { type OperationDescriptor } from "../utils/generator.js";
 import invariant from "../invariant.js";
 import { NestedOptimizedFunctionSideEffect } from "../errors.js";
 import { PropertyDescriptor } from "../descriptors.js";
+import { SimpleNormalCompletion } from "../completions.js";
 
+type ArrayNestedOptimizedFunctionType = "map" | "filter";
 type PossibleNestedOptimizedFunctions = [
-  { func: BoundFunctionValue | ECMAScriptSourceFunctionValue, thisValue: Value },
+  {
+    func: BoundFunctionValue | ECMAScriptSourceFunctionValue,
+    thisValue: Value,
+    kind: ArrayNestedOptimizedFunctionType,
+  },
 ];
 
 function evaluatePossibleNestedOptimizedFunctionsAndStoreEffects(
@@ -44,8 +50,21 @@ function evaluatePossibleNestedOptimizedFunctionsAndStoreEffects(
     }
     invariant(funcToModel instanceof ECMAScriptSourceFunctionValue);
 
-    if (realm.instantRender.enabled && realm.collectedNestedOptimizedFunctionEffects.has(funcToModel)) {
-      realm.instantRenderBailout("Array operators may only be optimized once", funcToModel.expressionLocation);
+    if (funcToModel.isCalledInMultipleContexts) return;
+
+    let previouslyComputedEffects = realm.collectedNestedOptimizedFunctionEffects.get(funcToModel);
+    if (previouslyComputedEffects !== undefined) {
+      if (realm.instantRender.enabled) {
+        realm.instantRenderBailout("Array operators may only be optimized once", funcToModel.expressionLocation);
+      } else {
+        // We currently do not support context-sensitive specialization,
+        // where the calls we specialize depend on the specialization context.
+        // TODO: #2454
+        // TODO: Implement context-sensitive specialization instead of giving up
+        funcToModel.isCalledInMultipleContexts = true;
+        Leak.value(realm, func);
+        return;
+      }
     }
 
     let funcCall = Utils.createModelledFunctionCall(realm, funcToModel, undefined, thisValue);
@@ -76,26 +95,6 @@ function evaluatePossibleNestedOptimizedFunctionsAndStoreEffects(
       throw e;
     }
 
-    // This is an incremental step from this list aimed to resolve a particular issue: #2452
-    //
-    // Assumptions:
-    // 1. We are here because the array op is pure, havocing of bindings is not needed.
-    // 2. Aliasing effects will lead to a fatal error. To be enforced: #2449
-    // 3. Indices of a widened array are not backed by locations
-    //
-    // Transitive materialization is needed to unblock this issue: #2405
-    //
-    // The bindings themselves do not have to materialize, since the values in them
-    // are used to optimize the nested optimized function. We compute the set of
-    // objects that are transitively reachable from read bindings and materialize them.
-
-    Materialize.materializeObjectsTransitive(realm, func);
-
-    // We assume that we do not have to materialize widened arrays because they are intrinsic.
-    // If somebody changes the underlying design in a major way, then materialization could be
-    // needed, and this check will fail.
-    invariant(abstractArrayValue.isIntrinsic());
-
     // Check if effects were pure then add them
     if (abstractArrayValue.nestedOptimizedFunctionEffects === undefined) {
       abstractArrayValue.nestedOptimizedFunctionEffects = new Map();
@@ -105,14 +104,91 @@ function evaluatePossibleNestedOptimizedFunctionsAndStoreEffects(
   }
 }
 
+/*
+  We track aliases explicitly, because we currently do not have the primitives to model objects created
+inside of the loop. TODO: Revisit when #2543 and subsequent modeling work
+lands. At that point, instead of of a mayAliasSet, we can return a widened
+abstract value.
+*/
+function modelUnknownPropertyOfSpecializedArray(
+  realm: Realm,
+  args: Array<Value>,
+  array: ArrayValue,
+  possibleNestedOptimizedFunctions: ?PossibleNestedOptimizedFunctions
+): PropertyBinding {
+  let sentinelProperty = {
+    key: undefined,
+    descriptor: new PropertyDescriptor({
+      writable: true,
+      enumerable: true,
+      configurable: true,
+    }),
+    object: array,
+  };
+
+  let mayAliasedObjects: Set<ObjectValue> = new Set();
+
+  if (realm.arrayNestedOptimizedFunctionsEnabled && possibleNestedOptimizedFunctions) {
+    invariant(possibleNestedOptimizedFunctions.length > 0);
+    if (possibleNestedOptimizedFunctions[0].kind === "map") {
+      for (let { func } of possibleNestedOptimizedFunctions) {
+        let funcToModel;
+        if (func instanceof BoundFunctionValue) {
+          funcToModel = func.$BoundTargetFunction;
+        } else {
+          funcToModel = func;
+        }
+        invariant(funcToModel instanceof ECMAScriptSourceFunctionValue);
+        if (array.nestedOptimizedFunctionEffects !== undefined) {
+          let effects = array.nestedOptimizedFunctionEffects.get(funcToModel);
+          if (effects !== undefined) {
+            invariant(effects.result instanceof SimpleNormalCompletion);
+            let reachableObjects = Materialize.computeReachableObjects(realm, effects.result.value);
+            for (let reachableObject of reachableObjects) {
+              if (!effects.createdObjects.has(reachableObject)) mayAliasedObjects.add(reachableObject);
+            }
+          }
+        }
+      }
+    }
+    // For filter, we just collect the may alias set of the mapped array
+    if (args.length > 0) {
+      let mappedArray = args[0];
+      if (ArrayValue.isIntrinsicAndHasWidenedNumericProperty(mappedArray)) {
+        invariant(mappedArray instanceof ArrayValue);
+        invariant(mappedArray.unknownProperty !== undefined);
+        invariant(mappedArray.unknownProperty.descriptor instanceof PropertyDescriptor);
+
+        let unknownPropertyValue = mappedArray.unknownProperty.descriptor.value;
+        invariant(unknownPropertyValue instanceof AbstractValue);
+
+        let aliasSet = unknownPropertyValue.args[0];
+        invariant(aliasSet instanceof AbstractValue && aliasSet.kind === "mayAliasSet");
+        for (let aliasedObject of aliasSet.args) {
+          invariant(aliasedObject instanceof ObjectValue);
+          mayAliasedObjects.add(aliasedObject);
+        }
+      }
+    }
+  }
+
+  let aliasSet = AbstractValue.createFromType(realm, Value, "mayAliasSet", [...mayAliasedObjects]);
+  sentinelProperty.descriptor.value = AbstractValue.createFromType(realm, Value, "widened numeric property", [
+    aliasSet,
+  ]);
+
+  return sentinelProperty;
+}
+
 function createArrayWithWidenedNumericProperty(
   realm: Realm,
+  args: Array<Value>,
   intrinsicName: string,
   possibleNestedOptimizedFunctions?: PossibleNestedOptimizedFunctions
 ): ArrayValue {
   let abstractArrayValue = new ArrayValue(realm, intrinsicName);
 
-  if (possibleNestedOptimizedFunctions !== undefined) {
+  if (possibleNestedOptimizedFunctions !== undefined && possibleNestedOptimizedFunctions.length > 0) {
     if (realm.arrayNestedOptimizedFunctionsEnabled && (!realm.react.enabled || realm.react.optimizeNestedFunctions)) {
       evaluatePossibleNestedOptimizedFunctionsAndStoreEffects(
         realm,
@@ -129,13 +205,12 @@ function createArrayWithWidenedNumericProperty(
     }
   }
   // Add unknownProperty so we manually handle this object property access
-  abstractArrayValue.unknownProperty = {
-    key: undefined,
-    descriptor: new PropertyDescriptor({
-      value: AbstractValue.createFromType(realm, Value, "widened numeric property"),
-    }),
-    object: abstractArrayValue,
-  };
+  abstractArrayValue.unknownProperty = modelUnknownPropertyOfSpecializedArray(
+    realm,
+    args,
+    abstractArrayValue,
+    possibleNestedOptimizedFunctions
+  );
   return abstractArrayValue;
 }
 
@@ -231,8 +306,10 @@ export default class ArrayValue extends ObjectValue {
     possibleNestedOptimizedFunctions?: PossibleNestedOptimizedFunctions
   ): ArrayValue {
     invariant(realm.generator !== undefined);
+
     let value = realm.generator.deriveConcreteObject(
-      intrinsicName => createArrayWithWidenedNumericProperty(realm, intrinsicName, possibleNestedOptimizedFunctions),
+      intrinsicName =>
+        createArrayWithWidenedNumericProperty(realm, args, intrinsicName, possibleNestedOptimizedFunctions),
       args,
       operationDescriptor,
       { isPure: true }

src/values/ECMAScriptFunctionValue.js

@@ -18,6 +18,7 @@ import type { BabelNodeSourceLocation } from "@babel/types";
 export default class ECMAScriptFunctionValue extends FunctionValue {
   constructor(realm: Realm, intrinsicName?: string) {
     super(realm, intrinsicName);
+    this.isCalledInMultipleContexts = false;
   }
 
   $ConstructorKind: "base" | "derived";
@@ -26,4 +27,5 @@ export default class ECMAScriptFunctionValue extends FunctionValue {
   $FunctionKind: "normal" | "classConstructor" | "generator";
   activeArguments: void | Map<BabelNodeSourceLocation, [number, Array<Value>]>;
   isSelfRecursive: boolean;
+  isCalledInMultipleContexts: boolean;
 }

test/error-handler/InstantRenderArrayOps4.js

@@ -1,21 +0,0 @@
-// instant render
-// expected errors:[{"location":{"start":{"line":15,"column":12},"end":{"line":15,"column":13},"source":"test/error-handler/InstantRenderArrayOps4.js"},"severity":"RecoverableError","errorCode":"PP0039"}]
-
-function f(c) {
-  var arr = Array.from(c);
-  let obj = { foo: 1 };
-
-  function op(x) {
-    return obj;
-  }
-
-  let mapped = arr.map(op);
-  let val = arr[0].foo;
-  let ret = mapped[0].foo;
-  obj.foo = 2; // Not allowed - requires materialization via mutations
-
-  return ret;
-}
-global.__optimize && __optimize(f);
-
-inspect = () => f([0]);

test/serializer/additional-functions/OptimizedArrayOpAliasing.js

@@ -0,0 +1,20 @@
+// arrayNestedOptimizedFunctionsEnabled
+
+function f(c) {
+  var arr = Array.from(c);
+  let obj = { foo: 1 };
+
+  function op(x) {
+    return obj;
+  }
+
+  let mapped = arr.map(op);
+  let val = arr[0].foo;
+  obj.foo = 2;
+  let ret = mapped[0].foo;
+
+  return ret;
+}
+global.__optimize && __optimize(f);
+
+inspect = () => f([0]);

test/serializer/additional-functions/OptimizedArrayOpAliasing2.js

@@ -0,0 +1,19 @@
+// arrayNestedOptimizedFunctionsEnabled
+// does not contain: return 42
+
+function f(c) {
+  var arr = Array.from(c);
+  let obj = { foo: 42 };
+
+  function op(x) {
+    return obj;
+  }
+
+  let mapped = arr.map(op);
+  mapped[0].foo = 0;
+
+  return obj.foo;
+}
+global.__optimize && __optimize(f);
+
+inspect = () => f([0]);

test/serializer/additional-functions/OptimizedArrayOpAliasing3.js

@@ -0,0 +1,24 @@
+// arrayNestedOptimizedFunctionsEnabled
+// does not contain: return 42
+
+function f(c) {
+  var arr = Array.from(c);
+  let obj = { foo: 42 };
+
+  function op1(x) {
+    return obj;
+  }
+
+  function op2(x) {
+    return x;
+  }
+
+  let mapped = arr.map(op1);
+  let mapped2 = mapped.map(op2);
+  mapped2[0].foo = 0;
+
+  return obj.foo;
+}
+global.__optimize && __optimize(f);
+
+inspect = () => f([0]);

test/serializer/additional-functions/OptimizedArrayOpNoDefaultMaterialization.js

@@ -0,0 +1,19 @@
+// arrayNestedOptimizedFunctionsEnabled
+// does not contain:foo = 1
+
+function f(c) {
+  var arr = Array.from(c);
+  let obj = { foo: 1 };
+
+  function op(x) {
+    return obj;
+  }
+
+  let mapped = arr.map(op);
+  obj.foo = 2;
+
+  return obj;
+}
+global.__optimize && __optimize(f);
+
+inspect = () => JSON.stringify(f([0]));

test/serializer/additional-functions/OptimizedArrayOpSpecialization.js

@@ -0,0 +1,18 @@
+// arrayNestedOptimizedFunctionsEnabled
+// does contain:42
+
+function f(c, g) {
+  var arr = Array.from(c);
+  let obj = { foo: 1 };
+
+  function op(x) {
+    return obj.foo + 41;
+  }
+
+  let mapped = arr.map(op);
+
+  return mapped;
+}
+global.__optimize && __optimize(f);
+
+inspect = () => f([0], () => {});

test/serializer/additional-functions/OptimizedArrayOpSpecializationBailout.js

@@ -0,0 +1,20 @@
+// arrayNestedOptimizedFunctionsEnabled
+// does not contain: 42
+
+function f(c) {
+  var arr = Array.from(c);
+  let x = 1;
+
+  function op(x) {
+    return x + 41;
+  }
+
+  let mapped = arr.map(op);
+  x = 2;
+  let mapped2 = mapped.map(op);
+
+  return mapped2;
+}
+global.__optimize && __optimize(f);
+
+inspect = () => f([0]);

test/serializer/additional-functions/OptimizedArrayOpSpecializationBailout2.js

@@ -0,0 +1,21 @@
+// arrayNestedOptimizedFunctionsEnabled
+// does not contain: 42
+
+function f(c) {
+  var arr = Array.from(c);
+  let x = 1;
+
+  function op(x) {
+    return x + 41;
+  }
+
+  let mapped = arr.map(op);
+  x = 2;
+  let mapped2 = mapped.map(op);
+  let mapped3 = mapped2.map(op);
+
+  return mapped3;
+}
+global.__optimize && __optimize(f);
+
+inspect = () => f([0]);

test/serializer/optimized-functions/InstantRenderArrayOpsBenignMutation.js

@@ -0,0 +1,18 @@
+// instant render
+
+function f(c) {
+  var arr = Array.from(c);
+  let obj = { foo: 1 };
+
+  function op(x) {
+    return obj;
+  }
+
+  let mapped = arr.map(op);
+  obj.foo = 2; // Allowed - the non-final value of obj is not referenced
+
+  return obj;
+}
+global.__optimize && __optimize(f);
+
+inspect = () => JSON.stringify(f([0]));

test/serializer/optimized-functions/TransitiveMaterializationViaFunctionCall.js

@@ -1,12 +1,11 @@
 // arrayNestedOptimizedFunctionsEnabled
 
-function f(c) {
+function f(c, b) {
   var arr = Array.from(c);
   let obj = { foo: 1 };
 
   function nested(x) {
-    let mapped_inner = arr.map(x => obj);
-    return mapped_inner[0];
+    return b ? x : undefined;
   }
 
   function op(x) {
@@ -23,4 +22,4 @@ function f(c) {
 
 global.__optimize && __optimize(f);
 
-inspect = () => f([0]);
+inspect = () => f([0], true);