diff --git a/js/src/jit-test/tests/asm.js/testNeuter.js b/js/src/jit-test/tests/asm.js/testNeuter.js index a6bcac12e182..ecd85b03de06 100644 --- a/js/src/jit-test/tests/asm.js/testNeuter.js +++ b/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); diff --git a/js/src/jsfriendapi.h b/js/src/jsfriendapi.h index 40e0a9292962..5609310bd582 100644 --- a/js/src/jsfriendapi.h +++ b/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, diff --git a/js/src/vm/ArrayBufferObject.cpp b/js/src/vm/ArrayBufferObject.cpp index db52564c88c7..3437c0c98793 100644 --- a/js/src/vm/ArrayBufferObject.cpp +++ b/js/src/vm/ArrayBufferObject.cpp @@ -260,12 +260,14 @@ NoteViewBufferWasDetached(ArrayBufferViewObject* view, MarkObjectStateChange(cx, view); } -/* static */ void +/* static */ bool ArrayBufferObject::detach(JSContext* cx, Handle 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 @@ -312,6 +314,7 @@ ArrayBufferObject::detach(JSContext* cx, Handle 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 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 @@ -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() const void ArrayBufferViewObject::notifyBufferDetached(void* newData) { + MOZ_ASSERT(newData != nullptr); if (is()) { as().notifyBufferDetached(newData); } else if (is()) { @@ -1193,21 +1190,20 @@ JS_DetachArrayBuffer(JSContext* cx, HandleObject obj, Rooted buffer(cx, &obj->as()); - 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; } @@ -1290,14 +1286,18 @@ JS_StealArrayBufferContents(JSContext* cx, HandleObject objArg) } Rooted buffer(cx, &obj->as()); + 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*) diff --git a/js/src/vm/ArrayBufferObject.h b/js/src/vm/ArrayBufferObject.h index fdcab71119a8..31c44fdbebf8 100644 --- a/js/src/vm/ArrayBufferObject.h +++ b/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 buffer, - bool forceCopy); + bool hasStealableContents); - // Some types of buffer are simply not willing to be detached, eg Wasm - // buffers. - bool hasDetachableContents() const { - if (isDetached()) + bool hasStealableContents() const { + // Inline elements strictly adhere to the corresponding buffer. + if (!ownsData()) 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 buffer, - BufferContents newContents); + static MOZ_MUST_USE bool + detach(JSContext* cx, Handle 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 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)); } diff --git a/js/src/vm/StructuredClone.cpp b/js/src/vm/StructuredClone.cpp index cadb44bd8d4c..c9b2df28b059 100644 --- a/js/src/vm/StructuredClone.cpp +++ b/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;