From 44bbabd65e9fc81701f51993c2261886ab9cd960 Mon Sep 17 00:00:00 2001 From: Steve Fink Date: Tue, 15 Oct 2013 17:26:19 -0700 Subject: [PATCH] Bug 861925 - Steal and neuter ArrayBuffers at end of structured clone, r=jorendorff --HG-- extra : rebase_source : 90ef9de8673dc50e81810a8cdcc86c8a8dbbc7bb --- js/public/StructuredClone.h | 4 +- js/src/js.msg | 2 +- js/src/jsapi.h | 16 +- .../js1_8_5/extensions/clone-transferables.js | 87 +++++++ js/src/tests/js1_8_5/extensions/shell.js | 13 + js/src/vm/StructuredClone.cpp | 238 +++++++++++++----- 6 files changed, 289 insertions(+), 71 deletions(-) create mode 100644 js/src/tests/js1_8_5/extensions/clone-transferables.js diff --git a/js/public/StructuredClone.h b/js/public/StructuredClone.h index 1095f483f4d..2ad3b769d17 100644 --- a/js/public/StructuredClone.h +++ b/js/public/StructuredClone.h @@ -48,7 +48,9 @@ typedef bool (*WriteStructuredCloneOp)(JSContext *cx, JSStructuredCloneWriter *w // with error set to one of the JS_SCERR_* values. typedef void (*StructuredCloneErrorOp)(JSContext *cx, uint32_t errorid); -// The maximum supported structured-clone serialization format version. +// The maximum supported structured-clone serialization format version. Note +// that this does not need to be bumped for Transferable-only changes, since +// they are never saved to persistent storage. #define JS_STRUCTURED_CLONE_VERSION 2 struct JSStructuredCloneCallbacks { diff --git a/js/src/js.msg b/js/src/js.msg index 1b3a1062e35..20eb96d1721 100644 --- a/js/src/js.msg +++ b/js/src/js.msg @@ -234,7 +234,7 @@ MSG_DEF(JSMSG_UNUSED180, 180, 0, JSEXN_NONE, "") MSG_DEF(JSMSG_UNUSED181, 181, 0, JSEXN_NONE, "") MSG_DEF(JSMSG_BAD_GENERATOR_SEND, 182, 1, JSEXN_TYPEERR, "attempt to send {0} to newborn generator") MSG_DEF(JSMSG_SC_NOT_TRANSFERABLE, 183, 0, JSEXN_TYPEERR, "invalid transferable array for structured clone") -MSG_DEF(JSMSG_UNUSED184, 184, 0, JSEXN_NONE, "") +MSG_DEF(JSMSG_SC_DUP_TRANSFERABLE, 184, 0, JSEXN_TYPEERR, "duplicate transferable for structured clone") MSG_DEF(JSMSG_CANT_REPORT_AS_NON_EXTENSIBLE, 185, 0, JSEXN_TYPEERR, "proxy can't report an extensible object as non-extensible") MSG_DEF(JSMSG_UNUSED186, 186, 0, JSEXN_NONE, "") MSG_DEF(JSMSG_UNUSED187, 187, 0, JSEXN_NONE, "") diff --git a/js/src/jsapi.h b/js/src/jsapi.h index cb2aed18575..e4ed8c5d831 100644 --- a/js/src/jsapi.h +++ b/js/src/jsapi.h @@ -224,6 +224,12 @@ class AutoArrayRooter : private AutoGCRooter { template class AutoVectorRooter : protected AutoGCRooter { + typedef js::Vector VectorImpl; + VectorImpl vector; + + /* Prevent overwriting of inline elements in vector. */ + js::SkipRoot vectorRoot; + public: explicit AutoVectorRooter(JSContext *cx, ptrdiff_t tag MOZ_GUARD_OBJECT_NOTIFIER_PARAM) @@ -240,6 +246,7 @@ class AutoVectorRooter : protected AutoGCRooter } typedef T ElementType; + typedef typename VectorImpl::Range Range; size_t length() const { return vector.length(); } bool empty() const { return vector.empty(); } @@ -299,6 +306,8 @@ class AutoVectorRooter : protected AutoGCRooter const T *end() const { return vector.end(); } T *end() { return vector.end(); } + Range all() { return vector.all(); } + const T &back() const { return vector.back(); } friend void AutoGCRooter::trace(JSTracer *trc); @@ -310,12 +319,6 @@ class AutoVectorRooter : protected AutoGCRooter memset(t, 0, sizeof(T)); } - typedef js::Vector VectorImpl; - VectorImpl vector; - - /* Prevent overwriting of inline elements in vector. */ - js::SkipRoot vectorRoot; - MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER }; @@ -335,6 +338,7 @@ class AutoHashMapRooter : protected AutoGCRooter typedef Key KeyType; typedef Value ValueType; + typedef typename HashMapImpl::Entry Entry; typedef typename HashMapImpl::Lookup Lookup; typedef typename HashMapImpl::Ptr Ptr; typedef typename HashMapImpl::AddPtr AddPtr; diff --git a/js/src/tests/js1_8_5/extensions/clone-transferables.js b/js/src/tests/js1_8_5/extensions/clone-transferables.js new file mode 100644 index 00000000000..18eac7feabe --- /dev/null +++ b/js/src/tests/js1_8_5/extensions/clone-transferables.js @@ -0,0 +1,87 @@ +// |reftest| skip-if(!xulRuntime.shell) +// Any copyright is dedicated to the Public Domain. +// http://creativecommons.org/licenses/publicdomain/ + +function test() { + // Note: -8 and -200 will trigger asm.js link failures because 8 and 200 + // bytes are below the minimum allowed size, and the buffer will not + // actually be converted to an asm.js buffer. + for (var size of [0, 8, 16, 200, 1000, 4096, -8, -200, -8192, -65536]) { + var buffer_ctor = (size < 0) ? AsmJSArrayBuffer : ArrayBuffer; + size = Math.abs(size); + + var old = buffer_ctor(size); + var copy = deserialize(serialize(old, [old])); + assertEq(old.byteLength, 0); + assertEq(copy.byteLength, size); + + var constructors = [ Int8Array, + Uint8Array, + Int16Array, + Uint16Array, + Int32Array, + Uint32Array, + Float32Array, + Float64Array, + Uint8ClampedArray ]; + + for (var ctor of constructors) { + var buf = buffer_ctor(size); + var old_arr = ctor(buf); + assertEq(buf.byteLength, size); + assertEq(buf, old_arr.buffer); + assertEq(old_arr.length, size / old_arr.BYTES_PER_ELEMENT); + + var copy_arr = deserialize(serialize(old_arr, [ buf ])); + assertEq(buf.byteLength, 0, "donor array buffer should be neutered"); + assertEq(old_arr.length, 0, "donor typed array should be neutered"); + assertEq(copy_arr.buffer.byteLength == size, true); + assertEq(copy_arr.length, size / old_arr.BYTES_PER_ELEMENT); + + buf = null; + old_arr = null; + gc(); // Tickle the ArrayBuffer -> view management + } + + for (var ctor of constructors) { + var buf = buffer_ctor(size); + var old_arr = ctor(buf); + var dv = DataView(buf); // Second view + var copy_arr = deserialize(serialize(old_arr, [ buf ])); + assertEq(buf.byteLength, 0, "donor array buffer should be neutered"); + assertEq(old_arr.length, 0, "donor typed array should be neutered"); + assertEq(dv.byteLength, 0, "all views of donor array buffer should be neutered"); + + buf = null; + old_arr = null; + gc(); // Tickle the ArrayBuffer -> view management + } + + // Mutate the buffer during the clone operation. The modifications should be visible. + if (size >= 4) { + old = buffer_ctor(size); + var view = Int32Array(old); + view[0] = 1; + var mutator = { get foo() { view[0] = 2; } }; + var copy = deserialize(serialize([ old, mutator ], [old])); + var viewCopy = Int32Array(copy[0]); + assertEq(view.length, 0); // Neutered + assertEq(viewCopy[0], 2); + } + + // Neuter the buffer during the clone operation. Should throw an exception. + if (size >= 4) { + old = buffer_ctor(size); + var mutator = { + get foo() { + deserialize(serialize(old, [old])); + } + }; + // The throw is not yet implemented, bug 919259. + //var copy = deserialize(serialize([ old, mutator ], [old])); + } + } +} + +test(); +reportCompare(0, 0, 'ok'); diff --git a/js/src/tests/js1_8_5/extensions/shell.js b/js/src/tests/js1_8_5/extensions/shell.js index e4b4493486f..ad9fa67eff6 100644 --- a/js/src/tests/js1_8_5/extensions/shell.js +++ b/js/src/tests/js1_8_5/extensions/shell.js @@ -197,3 +197,16 @@ function referencesVia(from, edge, to) { print(e); return false; } + +// Note that AsmJS ArrayBuffers have a minimum size, currently 4096 bytes. If a +// smaller size is given, a regular ArrayBuffer will be returned instead. +function AsmJSArrayBuffer(size) { + var ab = new ArrayBuffer(size); + (new Function('global', 'foreign', 'buffer', '' + +' "use asm";' + +' var i32 = new global.Int32Array(buffer);' + +' function g() {};' + +' return g;' + +''))(this,null,ab); + return ab; +} diff --git a/js/src/vm/StructuredClone.cpp b/js/src/vm/StructuredClone.cpp index 1661d4a2b61..3a1ae254e51 100644 --- a/js/src/vm/StructuredClone.cpp +++ b/js/src/vm/StructuredClone.cpp @@ -68,8 +68,8 @@ enum StructuredDataType { SCTAG_STRING_OBJECT, SCTAG_NUMBER_OBJECT, SCTAG_BACK_REFERENCE_OBJECT, - SCTAG_TRANSFER_MAP_HEADER, - SCTAG_TRANSFER_MAP, + SCTAG_DO_NOT_USE_1, + SCTAG_DO_NOT_USE_2, SCTAG_TYPED_ARRAY_OBJECT, SCTAG_TYPED_ARRAY_V1_MIN = 0xFFFF0100, SCTAG_TYPED_ARRAY_V1_INT8 = SCTAG_TYPED_ARRAY_V1_MIN + ScalarTypeRepresentation::TYPE_INT8, @@ -82,12 +82,37 @@ enum StructuredDataType { SCTAG_TYPED_ARRAY_V1_FLOAT64 = SCTAG_TYPED_ARRAY_V1_MIN + ScalarTypeRepresentation::TYPE_FLOAT64, SCTAG_TYPED_ARRAY_V1_UINT8_CLAMPED = SCTAG_TYPED_ARRAY_V1_MIN + ScalarTypeRepresentation::TYPE_UINT8_CLAMPED, SCTAG_TYPED_ARRAY_V1_MAX = SCTAG_TYPED_ARRAY_V1_MIN + ScalarTypeRepresentation::TYPE_MAX - 1, + + /* + * Define a separate range of numbers for Transferable-only tags, since + * they are not used for persistent clone buffers and therefore do not + * require bumping JS_STRUCTURED_CLONE_VERSION. + */ + SCTAG_TRANSFER_MAP_HEADER = 0xFFFF0200, + SCTAG_TRANSFER_MAP_ENTRY, + SCTAG_END_OF_BUILTIN_TYPES }; +// Data associated with an SCTAG_TRANSFER_MAP_HEADER that tells whether the +// contents have been read out yet or not. enum TransferableMapHeader { - SCTAG_TM_NOT_MARKED = 0, - SCTAG_TM_MARKED + SCTAG_TM_UNREAD = 0, + SCTAG_TM_TRANSFERRED +}; + +enum TransferableObjectType { + // Transferable data has not been filled in yet + SCTAG_TM_UNFILLED = 0, + + // Structured clone buffer does not yet own the data + SCTAG_TM_UNOWNED = 1, + + // All values at least this large are owned by the clone buffer + SCTAG_TM_FIRST_OWNED = 2, + + // Data is a pointer that can be freed + SCTAG_TM_ALLOC_DATA = 2, }; namespace js { @@ -95,6 +120,7 @@ namespace js { struct SCOutput { public: explicit SCOutput(JSContext *cx); + ~SCOutput(); JSContext *context() const { return cx; } @@ -110,7 +136,8 @@ struct SCOutput { bool extractBuffer(uint64_t **datap, size_t *sizep); - uint64_t count() { return buf.length(); } + uint64_t count() const { return buf.length(); } + uint64_t *rawBuffer() { return buf.begin(); } private: JSContext *cx; @@ -136,6 +163,12 @@ struct SCInput { bool replace(uint64_t u); bool replacePair(uint32_t tag, uint32_t data); + uint64_t *tell() const { return point; } + void seek(uint64_t *pos) { + JS_ASSERT(pos <= end); + point = pos; + } + template bool readArray(T *p, size_t nelems); @@ -206,8 +239,7 @@ struct JSStructuredCloneWriter { memory(out.context()), callbacks(cb), closure(cbClosure), transferable(out.context(), tVal), transferableObjects(out.context()) { } - bool init() { return transferableObjects.init() && parseTransferable() && - memory.init() && writeTransferMap(); } + bool init() { return parseTransferable() && memory.init() && writeTransferMap(); } bool write(const js::Value &v); @@ -228,6 +260,7 @@ struct JSStructuredCloneWriter { bool parseTransferable(); void reportErrorTransferable(); + bool transferOwnership(); inline void checkStack(); @@ -260,7 +293,7 @@ struct JSStructuredCloneWriter { // List of transferable objects JS::RootedValue transferable; - js::AutoObjectHashSet transferableObjects; + JS::AutoObjectVector transferableObjects; friend bool JS_WriteTypedArray(JSStructuredCloneWriter *w, JS::Value v); }; @@ -297,35 +330,47 @@ ReadStructuredClone(JSContext *cx, uint64_t *data, size_t nbytes, Value *vp, return r.read(vp); } -bool -ClearStructuredClone(const uint64_t *data, size_t nbytes) +// This may acquire new ways of discarding transfer map entries as new +// Transferables are implemented. +static void +DiscardEntry(uint32_t mapEntryDescriptor, const uint64_t *ptr) { - const uint64_t *point = data; - const uint64_t *end = data + nbytes / 8; + JS_ASSERT(mapEntryDescriptor == SCTAG_TM_ALLOC_DATA); + uint64_t u = LittleEndian::readUint64(ptr); + js_free(reinterpret_cast(u)); +} + +static void +Discard(const uint64_t *begin, const uint64_t *end) +{ + const uint64_t *point = begin; uint64_t u = LittleEndian::readUint64(point++); uint32_t tag = uint32_t(u >> 32); - if (tag == SCTAG_TRANSFER_MAP_HEADER) { - if ((TransferableMapHeader)uint32_t(u) == SCTAG_TM_NOT_MARKED) { - while (point != end) { - uint64_t u = LittleEndian::readUint64(point++); - uint32_t tag = uint32_t(u >> 32); - if (tag == SCTAG_TRANSFER_MAP) { - u = LittleEndian::readUint64(point++); - js_free(reinterpret_cast(u)); - } else { - // The only things in the transfer map should be - // SCTAG_TRANSFER_MAP tags paired with pointers. If we find - // any other tag, we've walked off the end of the transfer - // map. - break; - } - } + if (tag != SCTAG_TRANSFER_MAP_HEADER) + return; + + if (TransferableMapHeader(u) == SCTAG_TM_TRANSFERRED) + return; + + uint64_t numTransferables = LittleEndian::readUint64(point++); + while (numTransferables--) { + uint64_t u = LittleEndian::readUint64(point++); + JS_ASSERT(uint32_t(u >> 32) == SCTAG_TRANSFER_MAP_ENTRY); + uint32_t mapEntryDescriptor = uint32_t(u); + if (mapEntryDescriptor >= SCTAG_TM_FIRST_OWNED) { + DiscardEntry(mapEntryDescriptor, point); + point += 2; // Pointer and userdata } } +} - js_free((void *)data); - return true; +static void +ClearStructuredClone(const uint64_t *data, size_t nbytes) +{ + JS_ASSERT(nbytes % 8 == 0); + Discard(data, data + nbytes / 8); + js_free(const_cast(data)); } bool @@ -336,15 +381,14 @@ StructuredCloneHasTransferObjects(const uint64_t *data, size_t nbytes, bool *has if (data) { uint64_t u = LittleEndian::readUint64(data); uint32_t tag = uint32_t(u >> 32); - if (tag == SCTAG_TRANSFER_MAP_HEADER) { + if (tag == SCTAG_TRANSFER_MAP_HEADER) *hasTransferable = true; - } } return true; } -} +} /* namespace js */ static inline uint64_t PairToUInt64(uint32_t tag, uint32_t data) @@ -509,6 +553,12 @@ SCInput::readPtr(void **p) SCOutput::SCOutput(JSContext *cx) : cx(cx), buf(cx) {} +SCOutput::~SCOutput() +{ + // Free any transferable data left lying around in the buffer + Discard(rawBuffer(), rawBuffer() + count()); +} + bool SCOutput::write(uint64_t u) { @@ -636,7 +686,7 @@ JS_STATIC_ASSERT(JSString::MAX_LENGTH < UINT32_MAX); bool JSStructuredCloneWriter::parseTransferable() { - transferableObjects.clear(); + MOZ_ASSERT(transferableObjects.empty(), "parseTransferable called with stale data"); if (JSVAL_IS_NULL(transferable) || JSVAL_IS_VOID(transferable)) return true; @@ -672,7 +722,7 @@ JSStructuredCloneWriter::parseTransferable() JSObject* tObj = CheckedUnwrap(&v.toObject()); if (!tObj) { - JS_ReportError(context(), "Permission denied to access object"); + JS_ReportErrorNumber(context(), js_GetErrorMessage, NULL, JSMSG_UNWRAP_DENIED); return false; } if (!tObj->is()) { @@ -680,13 +730,13 @@ JSStructuredCloneWriter::parseTransferable() return false; } - // No duplicate: - if (transferableObjects.has(tObj)) { - reportErrorTransferable(); + // No duplicates allowed + if (std::find(transferableObjects.begin(), transferableObjects.end(), tObj) != transferableObjects.end()) { + JS_ReportErrorNumber(context(), js_GetErrorMessage, NULL, JSMSG_SC_DUP_TRANSFERABLE); return false; } - if (!transferableObjects.putNew(tObj)) + if (!transferableObjects.append(tObj)) return false; } @@ -904,25 +954,67 @@ JSStructuredCloneWriter::writeTransferMap() if (transferableObjects.empty()) return true; - if (!out.writePair(SCTAG_TRANSFER_MAP_HEADER, (uint32_t)SCTAG_TM_NOT_MARKED)) + if (!out.writePair(SCTAG_TRANSFER_MAP_HEADER, (uint32_t)SCTAG_TM_UNREAD)) return false; - for (HashSet::Range r = transferableObjects.all(); - !r.empty(); r.popFront()) { - JSObject *obj = r.front(); + if (!out.write(transferableObjects.length())) + return false; + + for (JS::AutoObjectVector::Range tr = transferableObjects.all(); + !tr.empty(); tr.popFront()) + { + JSObject *obj = tr.front(); if (!memory.put(obj, memory.count())) return false; + // Emit a placeholder pointer. We will steal the data and neuter the + // transferable later. + if (!out.writePair(SCTAG_TRANSFER_MAP_ENTRY, SCTAG_TM_UNFILLED) || + !out.writePtr(NULL) || + !out.write(0)) + { + return false; + } + } + + return true; +} + +bool +JSStructuredCloneWriter::transferOwnership() +{ + if (transferableObjects.empty()) + return true; + + // Walk along the transferables and the transfer map at the same time, + // grabbing out pointers from the transferables and stuffing them into the + // transfer map. + uint64_t *point = out.rawBuffer(); + JS_ASSERT(uint32_t(LittleEndian::readUint64(point) >> 32) == SCTAG_TRANSFER_MAP_HEADER); + point++; + JS_ASSERT(LittleEndian::readUint64(point) == transferableObjects.length()); + point++; + + for (JS::AutoObjectVector::Range tr = transferableObjects.all(); + !tr.empty(); + tr.popFront()) + { void *content; uint8_t *data; - if (!JS_StealArrayBufferContents(context(), obj, &content, &data)) - return false; + if (!JS_StealArrayBufferContents(context(), tr.front(), &content, &data)) + return false; // Destructor will clean up the already-transferred data - if (!out.writePair(SCTAG_TRANSFER_MAP, 0) || !out.writePtr(content)) - return false; + MOZ_ASSERT(uint32_t(LittleEndian::readUint64(point) >> 32) == SCTAG_TRANSFER_MAP_ENTRY); + LittleEndian::writeUint64(point++, PairToUInt64(SCTAG_TRANSFER_MAP_ENTRY, SCTAG_TM_ALLOC_DATA)); + LittleEndian::writeUint64(point++, reinterpret_cast(content)); + LittleEndian::writeUint64(point++, 0); } + JS_ASSERT(point <= out.rawBuffer() + out.count()); + JS_ASSERT_IF(point < out.rawBuffer() + out.count(), + uint32_t(LittleEndian::readUint64(point) >> 32) != SCTAG_TRANSFER_MAP_ENTRY); + return true; } @@ -969,8 +1061,7 @@ JSStructuredCloneWriter::write(const Value &v) } memory.clear(); - - return true; + return transferOwnership(); } bool @@ -1296,7 +1387,7 @@ JSStructuredCloneReader::startRead(Value *vp) "invalid input"); return false; - case SCTAG_TRANSFER_MAP: + case SCTAG_TRANSFER_MAP_ENTRY: // A map cannot be here but just at the beginning of the buffer. JS_ReportErrorNumber(context(), js_GetErrorMessage, nullptr, JSMSG_SC_BAD_SERIALIZED_DATA, @@ -1382,37 +1473,57 @@ JSStructuredCloneReader::readId(jsid *idp) bool JSStructuredCloneReader::readTransferMap() { + uint64_t *headerPos = in.tell(); + uint32_t tag, data; if (!in.getPair(&tag, &data)) return false; - if (tag != SCTAG_TRANSFER_MAP_HEADER || - (TransferableMapHeader)data == SCTAG_TM_MARKED) + if (tag != SCTAG_TRANSFER_MAP_HEADER || TransferableMapHeader(data) == SCTAG_TM_TRANSFERRED) return true; - if (!in.replacePair(SCTAG_TRANSFER_MAP_HEADER, SCTAG_TM_MARKED)) + uint64_t numTransferables; + MOZ_ALWAYS_TRUE(in.readPair(&tag, &data)); + if (!in.read(&numTransferables)) return false; - if (!in.readPair(&tag, &data)) - return false; + for (uint64_t i = 0; i < numTransferables; i++) { + uint64_t *pos = in.tell(); - while (1) { - if (!in.getPair(&tag, &data)) + if (!in.readPair(&tag, &data)) return false; - - if (tag != SCTAG_TRANSFER_MAP) - break; + JS_ASSERT(tag == SCTAG_TRANSFER_MAP_ENTRY); + JS_ASSERT(data == SCTAG_TM_ALLOC_DATA); void *content; + if (!in.readPtr(&content)) + return false; - if (!in.readPair(&tag, &data) || !in.readPtr(&content)) + uint64_t userdata; + if (!in.read(&userdata)) return false; JSObject *obj = JS_NewArrayBufferWithContents(context(), content); - if (!obj || !allObjs.append(ObjectValue(*obj))) + if (!obj) + return false; + + // Rewind to the SCTAG_TRANSFER_MAP_ENTRY and mark this entry as unowned by + // the input buffer. + uint64_t *next = in.tell(); + in.seek(pos); + MOZ_ALWAYS_TRUE(in.replacePair(SCTAG_TRANSFER_MAP_ENTRY, SCTAG_TM_UNOWNED)); + in.seek(next); + + if (!allObjs.append(ObjectValue(*obj))) return false; } + // Mark the whole transfer map as consumed + uint64_t *endPos = in.tell(); + in.seek(headerPos); + MOZ_ALWAYS_TRUE(in.replacePair(SCTAG_TRANSFER_MAP_HEADER, SCTAG_TM_TRANSFERRED)); + in.seek(endPos); + return true; } @@ -1487,7 +1598,8 @@ JS_WriteStructuredClone(JSContext *cx, JS::Value valueArg, uint64_t **bufp, size JS_PUBLIC_API(bool) JS_ClearStructuredClone(const uint64_t *data, size_t nbytes) { - return ClearStructuredClone(data, nbytes); + ClearStructuredClone(data, nbytes); + return true; } JS_PUBLIC_API(bool)