Backed out changeset f2cf760a4668 (bug 1240984) for XPCShell failures
authorIris Hsiao <ihsiao@mozilla.com>
Thu, 21 Jul 2016 11:14:49 +0800
changeset 346021 56faf83bf0c3fa65d671c3334227923d9870eacd
parent 346020 c805161da79c471e266e8db70ca4e8b18b697e29
child 346022 439867de69353a878b0bf43b6d5d00e92d678304
push id6389
push userraliiev@mozilla.com
push dateMon, 19 Sep 2016 13:38:22 +0000
treeherdermozilla-beta@01d67bfe6c81 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1240984
milestone50.0a1
backs outf2cf760a466829efa2b6182fa2243211f73429eb
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Backed out changeset f2cf760a4668 (bug 1240984) for XPCShell failures
js/src/jit-test/tests/asm.js/testNeuter.js
js/src/jsfriendapi.h
js/src/vm/ArrayBufferObject.cpp
js/src/vm/ArrayBufferObject.h
js/src/vm/StructuredClone.cpp
--- a/js/src/jit-test/tests/asm.js/testNeuter.js
+++ b/js/src/jit-test/tests/asm.js/testNeuter.js
@@ -18,34 +18,32 @@ var m = asmCompile('stdlib', 'foreign', 
                        return i32[i>>2]|0
                    }
                    return {get:get, set:set}`);
 
 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";
                    var i32 = new stdlib.Int32Array(buffer);
                    var ffi = foreign.ffi;
                    function inner(i) {
                        i=i|0;
                        ffi();
                        return i32[i>>2]|0
                    }
                    return inner`);
 
 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);
--- a/js/src/jsfriendapi.h
+++ b/js/src/jsfriendapi.h
@@ -2102,17 +2102,22 @@ enum DetachDataDisposition {
     ChangeData,
     KeepData
 };
 
 /**
  * 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,
                      DetachDataDisposition changeData);
 
 /**
  * Check whether the obj is a detached ArrayBufferObject. Note that this may
  * return false if a security wrapper is encountered that denies the
--- a/js/src/vm/ArrayBufferObject.cpp
+++ b/js/src/vm/ArrayBufferObject.cpp
@@ -255,22 +255,24 @@ NoteViewBufferWasDetached(ArrayBufferVie
                           JSContext* cx)
 {
     view->notifyBufferDetached(newContents.data());
 
     // Notify compiled jit code that the base pointer has moved.
     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");
-    MOZ_ASSERT(buffer->hasDetachableContents(), "detaching the non-detachable");
+    if (buffer->isWasm()) {
+        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
     // expect their data to ever change.
     MOZ_ASSERT_IF(buffer->forInlineTypedObject(),
                   newContents.data() == buffer->dataPointer());
 
     // When detaching a buffer with typed object views, any jitcode accessing
@@ -307,16 +309,17 @@ ArrayBufferObject::detach(JSContext* cx,
         }
     }
 
     if (newContents.data() != buffer->dataPointer())
         buffer->setNewOwnedData(cx->runtime()->defaultFreeOp(), newContents);
 
     buffer->setByteLength(0);
     buffer->setIsDetached();
+    return true;
 }
 
 void
 ArrayBufferObject::setNewOwnedData(FreeOp* fop, BufferContents newContents)
 {
     if (ownsData()) {
         MOZ_ASSERT(newContents.data() != dataPointer());
         releaseData(fop);
@@ -721,49 +724,45 @@ bool
 ArrayBufferObject::createDataViewForThis(JSContext* cx, unsigned argc, Value* vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
     return CallNonGenericMethod<IsArrayBuffer, createDataViewForThisImpl>(cx, args);
 }
 
 /* static */ ArrayBufferObject::BufferContents
 ArrayBufferObject::stealContents(JSContext* cx, Handle<ArrayBufferObject*> buffer,
-                                 bool forceCopy)
+                                 bool hasStealableContents)
 {
-    if (buffer->isDetached()) {
-        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);
-    }
+    MOZ_ASSERT_IF(hasStealableContents, buffer->hasStealableContents());
 
     BufferContents oldContents(buffer->dataPointer(), buffer->bufferKind());
+    BufferContents newContents = AllocateArrayBufferContents(cx, buffer->byteLength());
+    if (!newContents)
+        return BufferContents::createPlain(nullptr);
 
-    if (!forceCopy && buffer->ownsData()) {
-        // Return the old contents and reset the detached buffer's data
-        // pointer. This pointer should never be accessed.
-        auto newContents = BufferContents::createPlain(nullptr);
-        buffer->setOwnsData(DoesntOwnData); // Do not free the stolen data.
-        ArrayBufferObject::detach(cx, buffer, newContents);
-        buffer->setOwnsData(DoesntOwnData); // Do not free the nullptr.
+    if (hasStealableContents) {
+        // Return the old contents and give the detached buffer a pointer to
+        // freshly allocated memory that we will never write to and should
+        // never get committed.
+        buffer->setOwnsData(DoesntOwnData);
+        if (!ArrayBufferObject::detach(cx, buffer, newContents)) {
+            js_free(newContents.data());
+            return BufferContents::createPlain(nullptr);
+        }
         return oldContents;
     }
 
-    // Create a new chunk of memory to return since we cannot remove the
-    // existing contents pointer from the buffer.
-    BufferContents contentsCopy = AllocateArrayBufferContents(cx, buffer->byteLength());
-    if (!contentsCopy)
+    // Create a new chunk of memory to return since we cannot steal the
+    // existing contents away from the buffer.
+    memcpy(newContents.data(), oldContents.data(), buffer->byteLength());
+    if (!ArrayBufferObject::detach(cx, buffer, oldContents)) {
+        js_free(newContents.data());
         return BufferContents::createPlain(nullptr);
-
-    if (buffer->byteLength() > 0)
-        memcpy(contentsCopy.data(), oldContents.data(), buffer->byteLength());
-    ArrayBufferObject::detach(cx, buffer, oldContents);
-    return contentsCopy;
+    }
+    return newContents;
 }
 
 /* static */ void
 ArrayBufferObject::addSizeOfExcludingThis(JSObject* obj, mozilla::MallocSizeOf mallocSizeOf,
                                           JS::ClassInfo* info)
 {
     ArrayBufferObject& buffer = AsArrayBuffer(obj);
 
@@ -1028,21 +1027,20 @@ ArrayBufferViewObject::trace(JSTracer* t
     HeapSlot& bufSlot = obj->getFixedSlotRef(TypedArrayObject::BUFFER_SLOT);
     TraceEdge(trc, &bufSlot, "typedarray.buffer");
 
     // Update obj's data pointer if it moved.
     if (bufSlot.isObject()) {
         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();
 
                 // Mark the object to move it into the tenured space.
                 TraceManuallyBarrieredEdge(trc, &view, "typed array nursery owner");
                 MOZ_ASSERT(view->is<InlineTypedObject>());
                 MOZ_ASSERT(view != obj);
@@ -1052,18 +1050,16 @@ ArrayBufferViewObject::trace(JSTracer* t
                 obj->setPrivateUnbarriered(dstData);
 
                 // We can't use a direct forwarding pointer here, as there might
                 // not be enough bytes available, and other views might have data
                 // pointers whose forwarding pointers would overlap this one.
                 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.
                 obj->initPrivate(buf.dataPointer() + offset);
             }
         }
     }
 }
@@ -1080,16 +1076,17 @@ bool
 JSObject::is<js::ArrayBufferObjectMaybeShared>() const
 {
     return is<ArrayBufferObject>() || is<SharedArrayBufferObject>();
 }
 
 void
 ArrayBufferViewObject::notifyBufferDetached(void* newData)
 {
+    MOZ_ASSERT(newData != nullptr);
     if (is<DataViewObject>()) {
         as<DataViewObject>().notifyBufferDetached(newData);
     } else if (is<TypedArrayObject>()) {
         if (as<TypedArrayObject>().isSharedMemory())
             return;
         as<TypedArrayObject>().notifyBufferDetached(newData);
     } else {
         as<OutlineTypedObject>().notifyBufferDetached(newData);
@@ -1188,31 +1185,30 @@ JS_DetachArrayBuffer(JSContext* cx, Hand
 {
     if (!obj->is<ArrayBufferObject>()) {
         JS_ReportError(cx, "ArrayBuffer object required");
         return false;
     }
 
     Rooted<ArrayBufferObject*> buffer(cx, &obj->as<ArrayBufferObject>());
 
-    if (buffer->isDetached())
-        return true;
-
-    if (buffer->isWasm()) {
-        JS_ReportError(cx, "Cannot detach WASM ArrayBuffer");
-        return false;
+    if (changeData == ChangeData && buffer->hasStealableContents()) {
+        ArrayBufferObject::BufferContents newContents =
+            AllocateArrayBufferContents(cx, buffer->byteLength());
+        if (!newContents)
+            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;
 }
 
 JS_FRIEND_API(bool)
 JS_IsDetachedArrayBufferObject(JSObject* obj)
 {
     obj = CheckedUnwrap(obj);
     if (!obj)
@@ -1285,24 +1281,28 @@ JS_StealArrayBufferContents(JSContext* c
         return nullptr;
 
     if (!obj->is<ArrayBufferObject>()) {
         JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_TYPED_ARRAY_BAD_ARGS);
         return nullptr;
     }
 
     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
-    // true for (owned) mapped buffers, so we must additionally require that
-    // the buffer is plain. In the future, we could consider returning
-    // something that handles releasing the memory.
-    bool forceCopy = !buffer->ownsData() || !buffer->hasMallocedContents();
+    // The caller assumes that a plain malloc'd buffer is returned.
+    // hasStealableContents is true for mapped buffers, so we must additionally
+    // require that the buffer is plain. In the future, we could consider
+    // returning something that handles releasing the memory.
+    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_NewMappedArrayBufferWithContents(JSContext* cx, size_t nbytes, void* data)
 {
     MOZ_ASSERT(data);
     ArrayBufferObject::BufferContents contents =
         ArrayBufferObject::BufferContents::create<ArrayBufferObject::MAPPED>(data);
--- a/js/src/vm/ArrayBufferObject.h
+++ b/js/src/vm/ArrayBufferObject.h
@@ -208,16 +208,19 @@ class ArrayBufferObject : public ArrayBu
     static const JSFunctionSpec jsstaticfuncs[];
     static const JSPropertySpec jsstaticprops[];
 
     static bool byteLengthGetter(JSContext* cx, unsigned argc, Value* vp);
 
     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);
 
     static bool class_constructor(JSContext* cx, unsigned argc, Value* vp);
 
     static ArrayBufferObject* create(JSContext* cx, uint32_t nbytes,
                                      BufferContents contents,
                                      OwnsState ownsState = OwnsData,
@@ -238,29 +241,31 @@ class ArrayBufferObject : public ArrayBu
 
     static void copyData(Handle<ArrayBufferObject*> toBuffer,
                          Handle<ArrayBufferObject*> fromBuffer,
                          uint32_t fromIndex, uint32_t count);
 
     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);
+
+    bool hasStealableContents() const {
+        // Inline elements strictly adhere to the corresponding buffer.
+        if (!ownsData())
+            return false;
 
-    // Some types of buffer are simply not willing to be detached, eg Wasm
-    // buffers.
-    bool hasDetachableContents() const {
-        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
     // with js_free.
     bool hasMallocedContents() const {
         return (ownsData() && isPlain()) || isWasmMalloced();
     }
 
@@ -276,18 +281,18 @@ class ArrayBufferObject : public ArrayBu
 
     bool addView(JSContext* cx, JSObject* view);
 
     void setNewOwnedData(FreeOp* fop, BufferContents newContents);
     void changeContents(JSContext* cx, BufferContents newContents);
 
     // 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,
-                       BufferContents newContents);
+    static MOZ_MUST_USE bool
+    detach(JSContext* cx, Handle<ArrayBufferObject*> buffer, BufferContents newContents);
 
   private:
     void changeViewContents(JSContext* cx, ArrayBufferViewObject* view,
                             uint8_t* oldDataPointer, BufferContents newContents);
     void setFirstView(ArrayBufferViewObject* view);
 
     uint8_t* inlineDataPointer() const;
 
@@ -315,18 +320,16 @@ class ArrayBufferObject : public ArrayBu
     BufferKind bufferKind() const { return BufferKind(flags() & BUFFER_KIND_MASK); }
     bool isPlain() const { return bufferKind() == PLAIN; }
     bool isWasmMapped() const { return bufferKind() == WASM_MAPPED; }
     bool isWasmMalloced() const { return bufferKind() == WASM_MALLOCED; }
     bool isWasm() const { return isWasmMapped() || isWasmMalloced(); }
     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);
 
     static void finalize(FreeOp* fop, JSObject* obj);
 
     static BufferContents createMappedContents(int fd, size_t offset, size_t length);
 
     static size_t offsetOfFlagsSlot() {
@@ -347,16 +350,17 @@ class ArrayBufferObject : public ArrayBu
 
   protected:
     void setDataPointer(BufferContents contents, OwnsState ownsState);
     void setByteLength(uint32_t length);
 
     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));
     }
 
     bool hasTypedObjectViews() const { return flags() & TYPED_OBJECT_VIEWS; }
 
     void setIsWasmMalloced() { setFlags((flags() & ~KIND_MASK) | WASM_MALLOCED); }
     void setIsDetached() { setFlags(flags() | DETACHED); }
--- a/js/src/vm/StructuredClone.cpp
+++ b/js/src/vm/StructuredClone.cpp
@@ -1334,20 +1334,23 @@ JSStructuredCloneWriter::transferOwnersh
         if (cls == ESClass::ArrayBuffer) {
             // The current setup of the array buffer inheritance hierarchy doesn't
             // lend itself well to generic manipulation via proxies.
             Rooted<ArrayBufferObject*> arrayBuffer(context(), &CheckedUnwrap(obj)->as<ArrayBufferObject>());
             size_t nbytes = arrayBuffer->byteLength();
 
             // 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;
             if (bufContents.kind() == ArrayBufferObject::MAPPED)
                 ownership = JS::SCTAG_TMO_MAPPED_DATA;
             else
                 ownership = JS::SCTAG_TMO_ALLOC_DATA;
             extraData = nbytes;