diff --git a/js/src/jit-test/tests/asm.js/testNeuter.js b/js/src/jit-test/tests/asm.js/testNeuter.js index ecd85b03de06..a6bcac12e182 100644 --- a/js/src/jit-test/tests/asm.js/testNeuter.js +++ b/js/src/jit-test/tests/asm.js/testNeuter.js @@ -23,10 +23,7 @@ var buffer = new ArrayBuffer(BUF_MIN); var {get, set} = asmLink(m, this, null, buffer); set(4, 42); assertEq(get(4), 42); -assertThrowsInstanceOf(() => detachArrayBuffer(buffer, "change-data"), - InternalError); -assertThrowsInstanceOf(() => detachArrayBuffer(buffer, "same-data"), - InternalError); +assertThrowsInstanceOf(() => detachArrayBuffer(buffer), Error); var m = asmCompile('stdlib', 'foreign', 'buffer', `"use asm"; @@ -42,8 +39,13 @@ var m = asmCompile('stdlib', 'foreign', 'buffer', var buffer = new ArrayBuffer(BUF_MIN); function ffi1() { - assertThrowsInstanceOf(() => detachArrayBuffer(buffer, "change-data"), - InternalError); + assertThrowsInstanceOf(() => detachArrayBuffer(buffer), Error); } 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 5609310bd582..40e0a9292962 100644 --- a/js/src/jsfriendapi.h +++ b/js/src/jsfriendapi.h @@ -2107,12 +2107,7 @@ enum DetachDataDisposition { * Detach an ArrayBuffer, causing all associated views to no longer refer to * the ArrayBuffer's original attached memory. * - * 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. + * The |changeData| argument is obsolete and ignored. */ 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 3437c0c98793..db52564c88c7 100644 --- a/js/src/vm/ArrayBufferObject.cpp +++ b/js/src/vm/ArrayBufferObject.cpp @@ -260,14 +260,12 @@ NoteViewBufferWasDetached(ArrayBufferViewObject* view, MarkObjectStateChange(cx, view); } -/* static */ bool +/* static */ void ArrayBufferObject::detach(JSContext* cx, Handle buffer, BufferContents newContents) { - if (buffer->isWasm()) { - JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_OUT_OF_MEMORY); - return false; - } + MOZ_ASSERT(!buffer->isWasm(), "WASM/asm.js ArrayBuffers cannot be detached"); + MOZ_ASSERT(buffer->hasDetachableContents(), "detaching the non-detachable"); // 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 @@ -314,7 +312,6 @@ ArrayBufferObject::detach(JSContext* cx, Handle buffer, buffer->setByteLength(0); buffer->setIsDetached(); - return true; } void @@ -729,35 +726,39 @@ ArrayBufferObject::createDataViewForThis(JSContext* cx, unsigned argc, Value* vp /* static */ ArrayBufferObject::BufferContents ArrayBufferObject::stealContents(JSContext* cx, Handle buffer, - bool hasStealableContents) + bool forceCopy) { - MOZ_ASSERT_IF(hasStealableContents, buffer->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); + } BufferContents oldContents(buffer->dataPointer(), buffer->bufferKind()); - BufferContents newContents = AllocateArrayBufferContents(cx, buffer->byteLength()); - if (!newContents) - return BufferContents::createPlain(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); - } + 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. return oldContents; } - // 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()); + // 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) return BufferContents::createPlain(nullptr); - } - return newContents; + + if (buffer->byteLength() > 0) + memcpy(contentsCopy.data(), oldContents.data(), buffer->byteLength()); + ArrayBufferObject::detach(cx, buffer, oldContents); + return contentsCopy; } /* static */ void @@ -1032,10 +1033,11 @@ 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(); @@ -1055,6 +1057,8 @@ 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. @@ -1081,7 +1085,6 @@ JSObject::is() const void ArrayBufferViewObject::notifyBufferDetached(void* newData) { - MOZ_ASSERT(newData != nullptr); if (is()) { as().notifyBufferDetached(newData); } else if (is()) { @@ -1190,20 +1193,21 @@ JS_DetachArrayBuffer(JSContext* cx, HandleObject obj, Rooted buffer(cx, &obj->as()); - 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->isDetached()) + return true; + + if (buffer->isWasm()) { + JS_ReportError(cx, "Cannot detach WASM ArrayBuffer"); + return false; } + if (!buffer->hasDetachableContents()) { + JS_ReportError(cx, "Cannot detach ArrayBuffer"); + return false; + } + + ArrayBufferObject::detach(cx, buffer, buffer->contents()); + return true; } @@ -1286,18 +1290,14 @@ 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. - // 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(); + // 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(); - return ArrayBufferObject::stealContents(cx, buffer, hasStealableContents).data(); + return ArrayBufferObject::stealContents(cx, buffer, forceCopy).data(); } JS_PUBLIC_API(JSObject*) diff --git a/js/src/vm/ArrayBufferObject.h b/js/src/vm/ArrayBufferObject.h index 31c44fdbebf8..fdcab71119a8 100644 --- a/js/src/vm/ArrayBufferObject.h +++ b/js/src/vm/ArrayBufferObject.h @@ -213,9 +213,6 @@ 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); @@ -246,21 +243,19 @@ 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 hasStealableContents); + bool forceCopy); - bool hasStealableContents() const { - // Inline elements strictly adhere to the corresponding buffer. - if (!ownsData()) + // Some types of buffer are simply not willing to be detached, eg Wasm + // buffers. + bool hasDetachableContents() const { + if (isDetached()) return false; - - // 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 isPlain() || isMapped(); } // Return whether the buffer is allocated by js_malloc and should be freed @@ -286,8 +281,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 MOZ_MUST_USE bool - detach(JSContext* cx, Handle buffer, BufferContents newContents); + static void detach(JSContext* cx, Handle buffer, + BufferContents newContents); private: void changeViewContents(JSContext* cx, ArrayBufferViewObject* view, @@ -325,6 +320,8 @@ 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); @@ -355,7 +352,6 @@ 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 c9b2df28b059..cadb44bd8d4c 100644 --- a/js/src/vm/StructuredClone.cpp +++ b/js/src/vm/StructuredClone.cpp @@ -1339,13 +1339,10 @@ 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, hasStealableContents); + ArrayBufferObject::stealContents(context(), arrayBuffer, false); if (!bufContents) - return false; // Destructor will clean up the already-transferred data. + return false; // already transferred data or non-transferable buffer type content = bufContents.data(); tag = SCTAG_TRANSFER_MAP_ARRAY_BUFFER;