Backed out changeset f2cf760a4668 (bug 1240984) for XPCShell failures

js/src/jit-test/tests/asm.js/testNeuter.js

@@ -23,7 +23,10 @@ var buffer = new ArrayBuffer(BUF_MIN);
 var {get, set} = asmLink(m, this, null, buffer);
 set(4, 42);
 assertEq(get(4), 42);
-assertThrowsInstanceOf(() => detachArrayBuffer(buffer), Error);
+assertThrowsInstanceOf(() => detachArrayBuffer(buffer, "change-data"),
+                       InternalError);
+assertThrowsInstanceOf(() => detachArrayBuffer(buffer, "same-data"),
+                       InternalError);
 
 var m = asmCompile('stdlib', 'foreign', 'buffer',
                   `"use asm";
@@ -39,13 +42,8 @@ var m = asmCompile('stdlib', 'foreign', 'buffer',
 var buffer = new ArrayBuffer(BUF_MIN);
 function ffi1()
 {
-    assertThrowsInstanceOf(() => detachArrayBuffer(buffer), Error);
+    assertThrowsInstanceOf(() => detachArrayBuffer(buffer, "change-data"),
+                           InternalError);
 }
 
 var inner = asmLink(m, this, {ffi: ffi1}, buffer);
-
-function ffi2()
-{
-    assertThrowsInstanceOf(() => serialize([], [buffer]), Error);
-}
-var inner2 = asmLink(m, this, {ffi: ffi2}, buffer);

js/src/jsfriendapi.h

@@ -2107,7 +2107,12 @@ enum DetachDataDisposition {
  * Detach an ArrayBuffer, causing all associated views to no longer refer to
  * the ArrayBuffer's original attached memory.
  *
- * The |changeData| argument is obsolete and ignored.
+ * The |changeData| argument is a hint to inform internal behavior with respect
+ * to the ArrayBuffer's internal pointer to associated data.  |ChangeData|
+ * attempts to set the internal pointer to fresh memory of the same size as the
+ * original memory; |KeepData| attempts to preserve the original pointer, even
+ * while the ArrayBuffer appears observably detached.  There is no guarantee
+ * this parameter is respected -- it's only a hint.
  */
 extern JS_FRIEND_API(bool)
 JS_DetachArrayBuffer(JSContext* cx, JS::HandleObject obj,

js/src/vm/ArrayBufferObject.cpp

@@ -260,12 +260,14 @@ NoteViewBufferWasDetached(ArrayBufferViewObject* view,
     MarkObjectStateChange(cx, view);
 }
 
-/* static */ void
+/* static */ bool
 ArrayBufferObject::detach(JSContext* cx, Handle<ArrayBufferObject*> buffer,
                           BufferContents newContents)
 {
-    MOZ_ASSERT(!buffer->isWasm(), "WASM/asm.js ArrayBuffers cannot be detached");
+    if (buffer->isWasm()) {
-    MOZ_ASSERT(buffer->hasDetachableContents(), "detaching the non-detachable");
+        JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_OUT_OF_MEMORY);
+        return false;
+    }
 
     // When detaching buffers where we don't know all views, the new data must
     // match the old data. All missing views are typed objects, which do not
@@ -312,6 +314,7 @@ ArrayBufferObject::detach(JSContext* cx, Handle<ArrayBufferObject*> buffer,
 
     buffer->setByteLength(0);
     buffer->setIsDetached();
+    return true;
 }
 
 void
@@ -726,39 +729,35 @@ ArrayBufferObject::createDataViewForThis(JSContext* cx, unsigned argc, Value* vp
 
 /* static */ ArrayBufferObject::BufferContents
 ArrayBufferObject::stealContents(JSContext* cx, Handle<ArrayBufferObject*> buffer,
-                                 bool forceCopy)
+                                 bool hasStealableContents)
 {
-    if (buffer->isDetached()) {
+    MOZ_ASSERT_IF(hasStealableContents, buffer->hasStealableContents());
-        JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_TYPED_ARRAY_DETACHED);
-        return BufferContents::createPlain(nullptr);
-    }
-    if (!buffer->hasDetachableContents()) {
-        JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_TYPED_ARRAY_BAD_ARGS);
-        return BufferContents::createPlain(nullptr);
-    }
 
     BufferContents oldContents(buffer->dataPointer(), buffer->bufferKind());
+    BufferContents newContents = AllocateArrayBufferContents(cx, buffer->byteLength());
+    if (!newContents)
+        return BufferContents::createPlain(nullptr);
 
-    if (!forceCopy && buffer->ownsData()) {
+    if (hasStealableContents) {
-        // Return the old contents and reset the detached buffer's data
+        // Return the old contents and give the detached buffer a pointer to
-        // pointer. This pointer should never be accessed.
+        // freshly allocated memory that we will never write to and should
-        auto newContents = BufferContents::createPlain(nullptr);
+        // never get committed.
-        buffer->setOwnsData(DoesntOwnData); // Do not free the stolen data.
+        buffer->setOwnsData(DoesntOwnData);
-        ArrayBufferObject::detach(cx, buffer, newContents);
+        if (!ArrayBufferObject::detach(cx, buffer, newContents)) {
-        buffer->setOwnsData(DoesntOwnData); // Do not free the nullptr.
+            js_free(newContents.data());
+            return BufferContents::createPlain(nullptr);
+        }
         return oldContents;
     }
 
-    // Create a new chunk of memory to return since we cannot remove the
+    // Create a new chunk of memory to return since we cannot steal the
-    // existing contents pointer from the buffer.
+    // existing contents away from the buffer.
-    BufferContents contentsCopy = AllocateArrayBufferContents(cx, buffer->byteLength());
+    memcpy(newContents.data(), oldContents.data(), buffer->byteLength());
-    if (!contentsCopy)
+    if (!ArrayBufferObject::detach(cx, buffer, oldContents)) {
+        js_free(newContents.data());
         return BufferContents::createPlain(nullptr);
-
+    }
-    if (buffer->byteLength() > 0)
+    return newContents;
-        memcpy(contentsCopy.data(), oldContents.data(), buffer->byteLength());
-    ArrayBufferObject::detach(cx, buffer, oldContents);
-    return contentsCopy;
 }
 
 /* static */ void
@@ -1033,11 +1032,10 @@ ArrayBufferViewObject::trace(JSTracer* trc, JSObject* objArg)
         if (IsArrayBuffer(&bufSlot.toObject())) {
             ArrayBufferObject& buf = AsArrayBuffer(MaybeForwarded(&bufSlot.toObject()));
             uint32_t offset = uint32_t(obj->getFixedSlot(TypedArrayObject::BYTEOFFSET_SLOT).toInt32());
+            MOZ_ASSERT(buf.dataPointer() != nullptr);
             MOZ_ASSERT(offset <= INT32_MAX);
 
             if (buf.forInlineTypedObject()) {
-                MOZ_ASSERT(buf.dataPointer() != nullptr);
-
                 // The data is inline with an InlineTypedObject associated with the
                 // buffer. Get a new address for the typed object if it moved.
                 JSObject* view = buf.firstView();
@@ -1057,8 +1055,6 @@ ArrayBufferViewObject::trace(JSTracer* trc, JSObject* objArg)
                 trc->runtime()->gc.nursery.maybeSetForwardingPointer(trc, srcData, dstData,
                                                                      /* direct = */ false);
             } else {
-                MOZ_ASSERT_IF(buf.dataPointer() == nullptr, offset == 0);
-
                 // The data may or may not be inline with the buffer. The buffer
                 // can only move during a compacting GC, in which case its
                 // objectMoved hook has already updated the buffer's data pointer.
@@ -1085,6 +1081,7 @@ JSObject::is<js::ArrayBufferObjectMaybeShared>() const
 void
 ArrayBufferViewObject::notifyBufferDetached(void* newData)
 {
+    MOZ_ASSERT(newData != nullptr);
     if (is<DataViewObject>()) {
         as<DataViewObject>().notifyBufferDetached(newData);
     } else if (is<TypedArrayObject>()) {
@@ -1193,21 +1190,20 @@ JS_DetachArrayBuffer(JSContext* cx, HandleObject obj,
 
     Rooted<ArrayBufferObject*> buffer(cx, &obj->as<ArrayBufferObject>());
 
-    if (buffer->isDetached())
+    if (changeData == ChangeData && buffer->hasStealableContents()) {
-        return true;
+        ArrayBufferObject::BufferContents newContents =
-
+            AllocateArrayBufferContents(cx, buffer->byteLength());
-    if (buffer->isWasm()) {
+        if (!newContents)
-        JS_ReportError(cx, "Cannot detach WASM ArrayBuffer");
+            return false;
-        return false;
+        if (!ArrayBufferObject::detach(cx, buffer, newContents)) {
+            js_free(newContents.data());
+            return false;
+        }
+    } else {
+        if (!ArrayBufferObject::detach(cx, buffer, buffer->contents()))
+            return false;
     }
 
-    if (!buffer->hasDetachableContents()) {
-        JS_ReportError(cx, "Cannot detach ArrayBuffer");
-        return false;
-    }
-
-    ArrayBufferObject::detach(cx, buffer, buffer->contents());
-
     return true;
 }
 
@@ -1290,14 +1286,18 @@ JS_StealArrayBufferContents(JSContext* cx, HandleObject objArg)
     }
 
     Rooted<ArrayBufferObject*> buffer(cx, &obj->as<ArrayBufferObject>());
+    if (buffer->isDetached()) {
+        JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_TYPED_ARRAY_DETACHED);
+        return nullptr;
+    }
 
-    // The caller assumes that a plain malloc'd buffer is returned. ownsData is
+    // The caller assumes that a plain malloc'd buffer is returned.
-    // true for (owned) mapped buffers, so we must additionally require that
+    // hasStealableContents is true for mapped buffers, so we must additionally
-    // the buffer is plain. In the future, we could consider returning
+    // require that the buffer is plain. In the future, we could consider
-    // something that handles releasing the memory.
+    // returning something that handles releasing the memory.
-    bool forceCopy = !buffer->ownsData() || !buffer->hasMallocedContents();
+    bool hasStealableContents = buffer->hasStealableContents() && buffer->hasMallocedContents();
 
-    return ArrayBufferObject::stealContents(cx, buffer, forceCopy).data();
+    return ArrayBufferObject::stealContents(cx, buffer, hasStealableContents).data();
 }
 
 JS_PUBLIC_API(JSObject*)

js/src/vm/ArrayBufferObject.h

@@ -213,6 +213,9 @@ class ArrayBufferObject : public ArrayBufferObjectMaybeShared
     static bool fun_slice(JSContext* cx, unsigned argc, Value* vp);
 
     static bool fun_isView(JSContext* cx, unsigned argc, Value* vp);
+#ifdef NIGHTLY_BUILD
+    static bool fun_transfer(JSContext* cx, unsigned argc, Value* vp);
+#endif
 
     static bool fun_species(JSContext* cx, unsigned argc, Value* vp);
 
@@ -243,19 +246,21 @@ class ArrayBufferObject : public ArrayBufferObjectMaybeShared
     static void trace(JSTracer* trc, JSObject* obj);
     static void objectMoved(JSObject* obj, const JSObject* old);
 
-    // Detach the ArrayBuffer and return its contents (if possible), or
-    // possibly a copy of its contents (if forceCopy is passed or the buffer
-    // does not own the data).
     static BufferContents stealContents(JSContext* cx,
                                         Handle<ArrayBufferObject*> buffer,
-                                        bool forceCopy);
+                                        bool hasStealableContents);
 
-    // Some types of buffer are simply not willing to be detached, eg Wasm
+    bool hasStealableContents() const {
-    // buffers.
+        // Inline elements strictly adhere to the corresponding buffer.
-    bool hasDetachableContents() const {
+        if (!ownsData())
-        if (isDetached())
             return false;
-        return isPlain() || isMapped();
+
+        // Detached contents aren't transferrable because we want a detached
+        // buffer's contents to be backed by zeroed memory equal in length to
+        // the original buffer contents.  Transferring these contents would
+        // allocate new ones based on the current byteLength, which is 0 for a
+        // detached buffer -- not the original byteLength.
+        return !isDetached();
     }
 
     // Return whether the buffer is allocated by js_malloc and should be freed
@@ -281,8 +286,8 @@ class ArrayBufferObject : public ArrayBufferObjectMaybeShared
 
     // Detach this buffer from its original memory.  (This necessarily makes
     // views of this buffer unusable for modifying that original memory.)
-    static void detach(JSContext* cx, Handle<ArrayBufferObject*> buffer,
+    static MOZ_MUST_USE bool
-                       BufferContents newContents);
+    detach(JSContext* cx, Handle<ArrayBufferObject*> buffer, BufferContents newContents);
 
   private:
     void changeViewContents(JSContext* cx, ArrayBufferViewObject* view,
@@ -320,8 +325,6 @@ class ArrayBufferObject : public ArrayBufferObjectMaybeShared
     bool isMapped() const { return bufferKind() == MAPPED; }
     bool isDetached() const { return flags() & DETACHED; }
 
-    bool ownsData() const { return flags() & OWNS_DATA; }
-
     static ArrayBufferObject* createForWasm(JSContext* cx, uint32_t numBytes, bool signalsForOOB);
     static bool prepareForAsmJS(JSContext* cx, Handle<ArrayBufferObject*> buffer, bool signalsForOOB);
 
@@ -352,6 +355,7 @@ class ArrayBufferObject : public ArrayBufferObjectMaybeShared
     uint32_t flags() const;
     void setFlags(uint32_t flags);
 
+    bool ownsData() const { return flags() & OWNS_DATA; }
     void setOwnsData(OwnsState owns) {
         setFlags(owns ? (flags() | OWNS_DATA) : (flags() & ~OWNS_DATA));
     }

js/src/vm/StructuredClone.cpp

@@ -1339,10 +1339,13 @@ JSStructuredCloneWriter::transferOwnership()
 
             // Structured cloning currently only has optimizations for mapped
             // and malloc'd buffers, not asm.js-ified buffers.
+            bool hasStealableContents = arrayBuffer->hasStealableContents() &&
+                                        (arrayBuffer->isMapped() || arrayBuffer->hasMallocedContents());
+
             ArrayBufferObject::BufferContents bufContents =
-                ArrayBufferObject::stealContents(context(), arrayBuffer, false);
+                ArrayBufferObject::stealContents(context(), arrayBuffer, hasStealableContents);
             if (!bufContents)
-                return false; // already transferred data or non-transferable buffer type
+                return false; // Destructor will clean up the already-transferred data.
 
             content = bufContents.data();
             tag = SCTAG_TRANSFER_MAP_ARRAY_BUFFER;