Bug 1502889 - Revert TypedArray [[Set]] to previous behavior if not receiver. r=jorendorff, a=RyanVM
authorTom Schuster <evilpies@gmail.com>
Tue, 30 Oct 2018 22:01:52 +0000
changeset 500949 831cb67404bdcb5ef9b9a4ed68702054803b81a0
parent 500948 20548d7c4ba4545b3158b58f50e4c96041be2d9a
child 500950 bf07e6c3ea338ef4e0c4ce6742ec4117f322b4e5
push id1864
push userffxbld-merge
push dateMon, 03 Dec 2018 15:51:40 +0000
treeherdermozilla-release@f040763d99ad [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjorendorff, RyanVM
bugs1502889
milestone64.0
Bug 1502889 - Revert TypedArray [[Set]] to previous behavior if not receiver. r=jorendorff, a=RyanVM Differential Revision: https://phabricator.services.mozilla.com/D10116
js/src/jit-test/tests/cacheir/typedarray-set.js
js/src/jit-test/tests/proxy/testDirectProxySetArray4.js
js/src/jit/CacheIR.cpp
js/src/tests/non262/TypedArray/set-with-receiver.js
js/src/vm/NativeObject.cpp
deleted file mode 100644
--- a/js/src/jit-test/tests/cacheir/typedarray-set.js
+++ /dev/null
@@ -1,29 +0,0 @@
-// Based on work by André Bargull
-
-function f() {
-    var x = [1,2,3];
-    x[3] = 0xff;
-
-    // Should have been defined on typed array.
-    assertEq(x.length, 3);
-    assertEq(x[3], -1);
-
-    x[3] = 0;
-}
-
-Object.setPrototypeOf(Array.prototype, new Int8Array(4));
-f();
-f();
-
-function g() {
-    var x = [1,2,3,4];
-    x[4] = 0xff;
-
-    // OOB [[Set]] should have been ignored
-    assertEq(x.length, 4);
-    assertEq(x[4], undefined);
-}
-
-Object.setPrototypeOf(Array.prototype, new Int8Array(4));
-g();
-g();
--- a/js/src/jit-test/tests/proxy/testDirectProxySetArray4.js
+++ b/js/src/jit-test/tests/proxy/testDirectProxySetArray4.js
@@ -16,8 +16,9 @@ function test(arr) {
     });
     var hits = 0;
     p[0] = "ponies";
     assertEq(hits, 1);
     assertEq(arr[0], 123);
 }
 
 test([123]);
+test(new Int32Array([123]));
--- a/js/src/jit/CacheIR.cpp
+++ b/js/src/jit/CacheIR.cpp
@@ -3965,21 +3965,16 @@ CanAttachAddElement(NativeObject* obj, b
         if (!proto) {
             break;
         }
 
         if (!proto->isNative()) {
             return false;
         }
 
-        // TypedArrayObjects [[Set]] has special behavior.
-        if (proto->is<TypedArrayObject>()) {
-            return false;
-        }
-
         // We have to make sure the proto has no non-writable (frozen) elements
         // because we're not allowed to shadow them. There are a few cases to
         // consider:
         //
         // * If the proto is extensible, its Shape will change when it's made
         //   non-extensible.
         //
         // * If the proto is already non-extensible, no new elements will be
--- a/js/src/tests/non262/TypedArray/set-with-receiver.js
+++ b/js/src/tests/non262/TypedArray/set-with-receiver.js
@@ -1,33 +1,28 @@
 for (var constructor of anyTypedArrayConstructors) {
-    var receiver = new Proxy({}, {
-        getOwnPropertyDescriptor(p) {
-            throw "fail";
-        },
-
-        defineProperty() {
-            throw "fail";
-        }
-    });
+    var receiver = {};
 
     var ta = new constructor(1);
     assertEq(Reflect.set(ta, 0, 47, receiver), true);
-    assertEq(ta[0], 47);
+    assertEq(ta[0], 0);
+    assertEq(receiver[0], 47);
 
     // Out-of-bounds
-    assertEq(Reflect.set(ta, 10, 47, receiver), false);
+    assertEq(Reflect.set(ta, 10, 47, receiver), true);
     assertEq(ta[10], undefined);
+    assertEq(receiver[10], 47);
 
     // Detached
     if (typeof detachArrayBuffer === "function" &&
         !isSharedConstructor(constructor))
     {
         detachArrayBuffer(ta.buffer)
 
         assertEq(ta[0], undefined);
-        assertEq(Reflect.set(ta, 0, 47, receiver), false);
+        assertEq(Reflect.set(ta, 0, 42, receiver), true);
         assertEq(ta[0], undefined);
+        assertEq(receiver[0], 42);
     }
 }
 
 if (typeof reportCompare === "function")
     reportCompare(true, true);
--- a/js/src/vm/NativeObject.cpp
+++ b/js/src/vm/NativeObject.cpp
@@ -2035,36 +2035,58 @@ js::NativeDefineDataProperty(JSContext* 
 }
 
 static bool
 DefineNonexistentProperty(JSContext* cx, HandleNativeObject obj, HandleId id,
                           HandleValue v, ObjectOpResult& result)
 {
     // Optimized NativeDefineProperty() version for known absent properties.
 
-#ifdef DEBUG
-    // Indexed properties of typed arrays should have been handled by SetTypedArrayElement.
-    uint64_t index;
-    MOZ_ASSERT_IF(obj->is<TypedArrayObject>(), !IsTypedArrayIndex(id, &index));
-#endif
-
     // Dispense with custom behavior of exotic native objects first.
     if (obj->is<ArrayObject>()) {
         // Array's length property is non-configurable, so we shouldn't
         // encounter it in this function.
         MOZ_ASSERT(id != NameToId(cx->names().length));
 
         // 9.4.2.1 step 3. Don't extend a fixed-length array.
         uint32_t index;
         if (IdIsIndex(id, &index)) {
             if (WouldDefinePastNonwritableLength(&obj->as<ArrayObject>(), index)) {
                 return result.fail(JSMSG_CANT_DEFINE_PAST_ARRAY_LENGTH);
             }
         }
-    }  else if (obj->is<ArgumentsObject>()) {
+    } else if (obj->is<TypedArrayObject>()) {
+        // 9.4.5.5 step 2. Indexed properties of typed arrays are special.
+        uint64_t index;
+        if (IsTypedArrayIndex(id, &index)) {
+            // This method is only called for non-existent properties, which
+            // means any absent indexed property must be out of range.
+            MOZ_ASSERT(index >= obj->as<TypedArrayObject>().length());
+
+            // Steps 1-2 are enforced by the caller.
+
+            // Step 3.
+            // We still need to call ToNumber, because of its possible side
+            // effects.
+            double d;
+            if (!ToNumber(cx, v, &d)) {
+                return false;
+            }
+
+            // Steps 4-5.
+            // ToNumber may have detached the array buffer.
+            if (obj->as<TypedArrayObject>().hasDetachedBuffer()) {
+                return result.failSoft(JSMSG_TYPED_ARRAY_DETACHED);
+            }
+
+            // Steps 6-9.
+            // We (wrongly) ignore out of range defines.
+            return result.failSoft(JSMSG_BAD_INDEX);
+        }
+    } else if (obj->is<ArgumentsObject>()) {
         // If this method is called with either |length| or |@@iterator|, the
         // property was previously deleted and hence should already be marked
         // as overridden.
         MOZ_ASSERT_IF(id == NameToId(cx->names().length),
                       obj->as<ArgumentsObject>().hasOverriddenLength());
         MOZ_ASSERT_IF(JSID_IS_SYMBOL(id) && JSID_TO_SYMBOL(id) == cx->wellKnownSymbols().iterator,
                       obj->as<ArgumentsObject>().hasOverriddenIterator());
 
@@ -2866,26 +2888,31 @@ SetDenseElement(JSContext* cx, HandleNat
  * dense or typed array element (i.e. not actually a pointer to a Shape).
  */
 static bool
 SetExistingProperty(JSContext* cx, HandleId id, HandleValue v, HandleValue receiver,
                     HandleNativeObject pobj, Handle<PropertyResult> prop, ObjectOpResult& result)
 {
     // Step 5 for dense elements.
     if (prop.isDenseOrTypedArrayElement()) {
-        MOZ_ASSERT(!pobj->is<TypedArrayObject>());
-
         // Step 5.a.
         if (pobj->denseElementsAreFrozen()) {
             return result.fail(JSMSG_READ_ONLY);
         }
 
         // Pure optimization for the common case:
         if (receiver.isObject() && pobj == &receiver.toObject()) {
-            return SetDenseElement(cx, pobj, JSID_TO_INT(id), v, result);
+            uint32_t index = JSID_TO_INT(id);
+
+            if (pobj->is<TypedArrayObject>()) {
+                Rooted<TypedArrayObject*> tobj(cx, &pobj->as<TypedArrayObject>());
+                return SetTypedArrayElement(cx, tobj, index, v, result);
+            }
+
+            return SetDenseElement(cx, pobj, index, v, result);
         }
 
         // Steps 5.b-f.
         return SetPropertyByDefining(cx, id, v, receiver, result);
     }
 
     // Step 5 for all other properties.
     RootedShape shape(cx, prop.shape());
@@ -2941,27 +2968,16 @@ js::NativeSetProperty(JSContext* cx, Han
     // implementation, but unfortunately not similar enough to common up.)
     for (;;) {
         // Steps 2-3. ('done' is a SpiderMonkey-specific thing, used below.)
         bool done;
         if (!LookupOwnPropertyInline<CanGC>(cx, pobj, id, &prop, &done)) {
             return false;
         }
 
-        if (pobj->is<TypedArrayObject>()) {
-            uint64_t index;
-            if (IsTypedArrayIndex(id, &index)) {
-                Rooted<TypedArrayObject*> tobj(cx, &pobj->as<TypedArrayObject>());
-                return SetTypedArrayElement(cx, tobj, index, v, result);
-            }
-
-            // This case should have been handled.
-            MOZ_ASSERT(!prop.isDenseOrTypedArrayElement());
-        }
-
         if (prop) {
             // Steps 5-6.
             return SetExistingProperty(cx, id, v, receiver, pobj, prop, result);
         }
 
         // Steps 4.a-b. The check for 'done' on this next line is tricky.
         // done can be true in exactly these unlikely-sounding cases:
         // - We're looking up an element, and pobj is a TypedArray that