Fixed multiple ordering bugs involving unsafe a-temporal abstract values

src/evaluators/BinaryExpression.js

@@ -199,7 +199,14 @@ export function computeBinary(
       try {
         // generate error if binary operation might throw or have side effects
         resultType = getPureBinaryOperationResultType(realm, op, lval, rval, lloc, rloc);
-        return AbstractValue.createFromBinaryOp(realm, op, lval, rval, loc);
+        let result = AbstractValue.createFromBinaryOp(realm, op, lval, rval, loc);
+
+        // "in" and "instanceof" can throw and therefore must result in temporals
+        if (result instanceof AbstractValue && (op === "in" || op === "instanceof")) {
+          invariant(realm.generator !== undefined);
+          result = realm.generator.deriveAbstract(result.types, result.values, result.args, result.operationDescriptor);
+        }
+        return result;
       } catch (x) {
         if (x instanceof FatalError) {
           // There is no need to revert any effects, because the above operation is pure.

src/intrinsics/ecma262/JSON.js

@@ -317,6 +317,8 @@ const JSONParse = buildExpressionTemplate(JSONParseStr);
 function InternalJSONClone(realm: Realm, val: Value): Value {
   if (val instanceof AbstractValue) {
     if (val instanceof AbstractObjectValue) {
+      // These may need to use the temporal variants of createFromTemplate
+      // See #2327
       let strVal = AbstractValue.createFromTemplate(realm, JSONStringify, StringValue, [val], JSONStringifyStr);
       let obVal = AbstractValue.createFromTemplate(realm, JSONParse, ObjectValue, [strVal], JSONParseStr);
       obVal.values = new ValuesDomain(new Set([InternalCloneObject(realm, val.getTemplate())]));

src/intrinsics/ecma262/Math.js

@@ -167,6 +167,9 @@ export default function(realm: Realm): ObjectValue {
           let template = buildExpressionTemplate(templateSource);
           buildMathTemplates.set(name, (r = { template, templateSource }));
         }
+        /* Functions in the Math module cannot throw, so if we ever end up supporting these calls on AbstractValues,
+        using createFromTemplate instead of createTemporalFromTemplate should be safe.
+        This makes the function amenable to CSE, which is a good thing for Math.* */
         return AbstractValue.createFromTemplate(realm, r.template, NumberValue, args, r.templateSource);
       }
 

src/intrinsics/ecma262/StringPrototype.js

@@ -577,7 +577,10 @@ export default function(realm: Realm, obj: ObjectValue): ObjectValue {
     let O = RequireObjectCoercible(realm, context);
 
     if (O instanceof AbstractValue && O.getType() === StringValue) {
-      return AbstractValue.createFromTemplate(realm, sliceTemplate, StringValue, [O, start, end], sliceTemplateSrc);
+      return AbstractValue.createTemporalFromTemplate(realm, sliceTemplate, StringValue, [O, start, end], {
+        isPure: true,
+        skipInvariant: true,
+      });
     }
 
     // 2. Let S be ? ToString(O).
@@ -611,13 +614,10 @@ export default function(realm: Realm, obj: ObjectValue): ObjectValue {
     let O = RequireObjectCoercible(realm, context);
 
     if (O instanceof AbstractValue && O.getType() === StringValue) {
-      return AbstractValue.createFromTemplate(
-        realm,
-        splitTemplate,
-        ObjectValue,
-        [O, separator, limit],
-        splitTemplateSrc
-      );
+      return AbstractValue.createTemporalFromTemplate(realm, splitTemplate, ObjectValue, [O, separator, limit], {
+        isPure: true,
+        skipInvariant: true,
+      });
     }
 
     // 2. If separator is neither undefined nor null, then

src/methods/get.js

@@ -344,7 +344,10 @@ export function OrdinaryGetPartial(
   Receiver: Value
 ): Value {
   if (Receiver instanceof AbstractValue && Receiver.getType() === StringValue && P === "length") {
-    return AbstractValue.createFromTemplate(realm, lengthTemplate, NumberValue, [Receiver], lengthTemplateSrc);
+    return AbstractValue.createTemporalFromTemplate(realm, lengthTemplate, NumberValue, [Receiver], {
+      isPure: true,
+      skipInvariant: true,
+    });
   }
 
   if (!(P instanceof AbstractValue)) return O.$Get(P, Receiver);

src/values/AbstractValue.js

@@ -711,7 +711,13 @@ export default class AbstractValue extends Value {
   /* Note that the template is parameterized by the names A, B, C and so on.
      When the abstract value is serialized, the serialized operations are substituted
      for the corresponding parameters and the resulting template is parsed into an AST subtree
-     that is incorporated into the AST produced by the serializer. */
+     that is incorporated into the AST produced by the serializer.
+
+     NOTE: Only call this function if you can guarantee that the template you pass to it will not
+     generate code that is a-temporal. Code is a-temporal if it cannot throw or cause side effects.
+     If there is a possibility that it might exhibit this behavior, then use the safer
+     createTemporalFromTemplate call instead. */
+
   static createFromTemplate(
     realm: Realm,
     template: PreludeGenerator => ({}) => BabelNodeExpression,

test/serializer/abstract/Issue2327InBinop.js

@@ -0,0 +1,11 @@
+// expected RecoverableError: PP0004
+
+let o = global.__abstractOrNull ? __abstractOrNull("object", "undefined") : undefined;
+let s = true;
+
+if (o != undefined) s = "foobar" in o;
+if (o != undefined) s = "foobar" in o;
+
+global.s = s;
+
+inspect = () => s; // Should be true

test/serializer/abstract/Issue2327InstanceOfBinop.js

@@ -0,0 +1,11 @@
+// expected RecoverableError: PP0004
+
+let o = global.__abstractOrNull ? __abstractOrNull("object", "undefined") : undefined;
+let s = true;
+
+if (o != undefined) s = "foobar" instanceof o;
+if (o != undefined) s = "foobar" instanceof o;
+
+global.s = s;
+
+inspect = () => s; // Should be true

test/serializer/abstract/Issue2327StringLength.js

@@ -0,0 +1,9 @@
+let o = global.__abstractOrNull ? __abstractOrNull("string", "undefined") : undefined;
+let s;
+
+if (o != undefined) s = 5 + o.length;
+if (o != undefined) s = 7 + o.length;
+
+global.s = s;
+
+inspect = () => s;

test/serializer/abstract/Issue2327StringSlice.js

@@ -0,0 +1,9 @@
+let o = global.__abstractOrNull ? __abstractOrNull("string", "undefined") : undefined;
+let s = "ok";
+
+if (o != undefined) s = "X" + o.slice(3);
+if (o != undefined) s = "Y" + o.slice(3);
+
+global.s = s;
+
+inspect = () => s;

test/serializer/abstract/Issue2327StringSplit.js

@@ -0,0 +1,9 @@
+let o = global.__abstractOrNull ? __abstractOrNull("string", "undefined") : undefined;
+let s = "ok";
+
+if (o != undefined) s = o.split();
+if (o != undefined) s = o.split();
+
+global.s = s;
+
+inspect = () => s;