Bug 1055472 - Part 15: Make the DataView constructor properly subclassable. (r=jorendorff, r=bhackett)
authorEric Faust <efaustbmo@gmail.com>
Fri, 13 Nov 2015 18:22:22 -0800
changeset 309844 c2573c84ff61692634696bcf72b7b6403e61d4af
parent 309843 1e0c29a6d05886f9001513753aa0875654c13393
child 309845 79b47f5f715a0647ebdba664acc7544bf5ca1761
push id5513
push userraliiev@mozilla.com
push dateMon, 25 Jan 2016 13:55:34 +0000
treeherdermozilla-beta@5ee97dd05b5c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjorendorff, bhackett
bugs1055472
milestone45.0a1
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
Bug 1055472 - Part 15: Make the DataView constructor properly subclassable. (r=jorendorff, r=bhackett)
js/src/tests/ecma_6/Class/extendBuiltinConstructors.js
js/src/tests/ecma_6/DataView/detach-after-construction.js
js/src/vm/ArrayBufferObject.cpp
js/src/vm/TypedArrayObject.cpp
js/src/vm/TypedArrayObject.h
--- a/js/src/tests/ecma_6/Class/extendBuiltinConstructors.js
+++ b/js/src/tests/ecma_6/Class/extendBuiltinConstructors.js
@@ -51,16 +51,18 @@ testBuiltin(RegExp);
 testBuiltin(RegExp, /Regexp Argument/);
 testBuiltin(RegExp, "String Argument");
 testBuiltin(Map);
 testBuiltin(Set);
 testBuiltin(WeakMap);
 testBuiltin(WeakSet);
 testBuiltin(ArrayBuffer);
 testBuiltinTypedArrays();
+testBuiltin(DataView, new ArrayBuffer());
+testBuiltin(DataView, new (newGlobal().ArrayBuffer)());
 
 `;
 
 if (classesEnabled())
     eval(test);
 
 if (typeof reportCompare === 'function')
     reportCompare(0,0,"OK");
new file mode 100644
--- /dev/null
+++ b/js/src/tests/ecma_6/DataView/detach-after-construction.js
@@ -0,0 +1,13 @@
+// |reftest| skip-if(!xulRuntime.shell)
+
+for (var neuterArg of ['change-data', 'same-data']) {
+    var buf = new ArrayBuffer([1,2]);
+    var bufView = new DataView(buf);
+
+    neuter(buf, neuterArg);
+
+    assertThrowsInstanceOf(()=>bufView.getInt8(0), TypeError);
+}
+
+if (typeof reportCompare === 'function')
+    reportCompare(0,0,"OK");
--- a/js/src/vm/ArrayBufferObject.cpp
+++ b/js/src/vm/ArrayBufferObject.cpp
@@ -894,31 +894,35 @@ ArrayBufferObject::createSlice(JSContext
 
 bool
 ArrayBufferObject::createDataViewForThisImpl(JSContext* cx, const CallArgs& args)
 {
     MOZ_ASSERT(IsArrayBuffer(args.thisv()));
 
     /*
      * This method is only called for |DataView(alienBuf, ...)| which calls
-     * this as |createDataViewForThis.call(alienBuf, ..., DataView.prototype)|,
-     * ergo there must be at least two arguments.
+     * this as |createDataViewForThis.call(alienBuf, byteOffset, byteLength,
+     *                                     DataView.prototype)|,
+     * ergo there must be exactly 3 arguments.
      */
-    MOZ_ASSERT(args.length() >= 2);
+    MOZ_ASSERT(args.length() == 3);
 
-    Rooted<JSObject*> proto(cx, &args[args.length() - 1].toObject());
-
-    Rooted<JSObject*> buffer(cx, &args.thisv().toObject());
+    uint32_t byteOffset = args[0].toPrivateUint32();
+    uint32_t byteLength = args[1].toPrivateUint32();
+    Rooted<ArrayBufferObject*> buffer(cx, &args.thisv().toObject().as<ArrayBufferObject>());
 
     /*
      * Pop off the passed-along prototype and delegate to normal DataViewObject
      * construction.
      */
-    CallArgs frobbedArgs = CallArgsFromVp(args.length() - 1, args.base());
-    return DataViewObject::construct(cx, buffer, frobbedArgs, proto);
+    JSObject* obj = DataViewObject::create(cx, byteOffset, byteLength, buffer, &args[2].toObject());
+    if (!obj)
+        return false;
+    args.rval().setObject(*obj);
+    return true;
 }
 
 bool
 ArrayBufferObject::createDataViewForThis(JSContext* cx, unsigned argc, Value* vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
     return CallNonGenericMethod<IsArrayBuffer, createDataViewForThisImpl>(cx, args);
 }
--- a/js/src/vm/TypedArrayObject.cpp
+++ b/js/src/vm/TypedArrayObject.cpp
@@ -1013,47 +1013,44 @@ DataViewNewObjectKind(JSContext* cx, uin
         return SingletonObject;
     jsbytecode* pc;
     JSScript* script = cx->currentScript(&pc);
     if (script && ObjectGroup::useSingletonForAllocationSite(script, pc, &DataViewObject::class_))
         return SingletonObject;
     return GenericObject;
 }
 
-inline DataViewObject*
+DataViewObject*
 DataViewObject::create(JSContext* cx, uint32_t byteOffset, uint32_t byteLength,
                        Handle<ArrayBufferObject*> arrayBuffer, JSObject* protoArg)
 {
     MOZ_ASSERT(byteOffset <= INT32_MAX);
     MOZ_ASSERT(byteLength <= INT32_MAX);
     MOZ_ASSERT(byteOffset + byteLength < UINT32_MAX);
     MOZ_ASSERT(!arrayBuffer || !arrayBuffer->is<SharedArrayBufferObject>());
 
     RootedObject proto(cx, protoArg);
     RootedObject obj(cx);
 
     NewObjectKind newKind = DataViewNewObjectKind(cx, byteLength, proto);
-    obj = NewBuiltinClassInstance(cx, &class_, newKind);
+    obj = NewObjectWithClassProto(cx, &class_, proto, newKind);
     if (!obj)
         return nullptr;
 
-    if (proto) {
-        ObjectGroup* group = ObjectGroup::defaultNewGroup(cx, &class_, TaggedProto(proto));
-        if (!group)
-            return nullptr;
-        obj->setGroup(group);
-    } else if (byteLength >= TypedArrayObject::SINGLETON_BYTE_LENGTH) {
-        MOZ_ASSERT(obj->isSingleton());
-    } else {
-        jsbytecode* pc;
-        RootedScript script(cx, cx->currentScript(&pc));
-        if (script && !ObjectGroup::setAllocationSiteObjectGroup(cx, script, pc, obj,
-                                                                 newKind == SingletonObject))
-        {
-            return nullptr;
+    if (!proto) {
+        if (byteLength >= TypedArrayObject::SINGLETON_BYTE_LENGTH) {
+            MOZ_ASSERT(obj->isSingleton());
+        } else {
+            jsbytecode* pc;
+            RootedScript script(cx, cx->currentScript(&pc));
+            if (script && !ObjectGroup::setAllocationSiteObjectGroup(cx, script, pc, obj,
+                                                                     newKind == SingletonObject))
+            {
+                return nullptr;
+            }
         }
     }
 
     // Caller should have established these preconditions, and no
     // (non-self-hosted) JS code has had an opportunity to run so nothing can
     // have invalidated them.
     MOZ_ASSERT(byteOffset <= arrayBuffer->byteLength());
     MOZ_ASSERT(byteOffset + byteLength <= arrayBuffer->byteLength());
@@ -1074,106 +1071,183 @@ DataViewObject::create(JSContext* cx, ui
 
     if (!arrayBuffer->addView(cx, &dvobj))
         return nullptr;
 
     return &dvobj;
 }
 
 bool
-DataViewObject::construct(JSContext* cx, JSObject* bufobj, const CallArgs& args, HandleObject proto)
+DataViewObject::getAndCheckConstructorArgs(JSContext* cx, JSObject* bufobj, const CallArgs& args,
+                                           uint32_t* byteOffsetPtr, uint32_t* byteLengthPtr)
 {
     if (!IsArrayBuffer(bufobj)) {
         JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_NOT_EXPECTED_TYPE,
                              "DataView", "ArrayBuffer", bufobj->getClass()->name);
         return false;
     }
 
     Rooted<ArrayBufferObject*> buffer(cx, &AsArrayBuffer(bufobj));
     uint32_t byteOffset = 0;
     uint32_t byteLength = buffer->byteLength();
 
     if (args.length() > 1) {
         if (!ToUint32(cx, args[1], &byteOffset))
             return false;
         if (byteOffset > INT32_MAX) {
-            JS_ReportErrorNumber(cx, GetErrorMessage, nullptr,
-                                 JSMSG_ARG_INDEX_OUT_OF_RANGE, "1");
+            JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_ARG_INDEX_OUT_OF_RANGE, "1");
+            return false;
+        }
+    }
+
+    if (buffer->isNeutered()) {
+        JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_TYPED_ARRAY_DETACHED);
+        return false;
+    }
+
+    if (args.length() > 1) {
+        if (byteOffset > byteLength) {
+            JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_ARG_INDEX_OUT_OF_RANGE, "1");
             return false;
         }
 
-        if (!args.get(2).isUndefined()) {
+        if (args.get(2).isUndefined()) {
+            byteLength -= byteOffset;
+        } else {
             if (!ToUint32(cx, args[2], &byteLength))
                 return false;
             if (byteLength > INT32_MAX) {
                 JS_ReportErrorNumber(cx, GetErrorMessage, nullptr,
-                                     JSMSG_ARG_INDEX_OUT_OF_RANGE, "2");
+                                        JSMSG_ARG_INDEX_OUT_OF_RANGE, "2");
                 return false;
             }
-        } else {
-            uint32_t bufferLength = buffer->byteLength();
 
-            if (byteOffset > bufferLength) {
+            MOZ_ASSERT(byteOffset + byteLength >= byteOffset,
+                       "can't overflow: both numbers are less than INT32_MAX");
+            if (byteOffset + byteLength > buffer->byteLength()) {
                 JS_ReportErrorNumber(cx, GetErrorMessage, nullptr,
                                      JSMSG_ARG_INDEX_OUT_OF_RANGE, "1");
                 return false;
             }
-
-            byteLength = bufferLength - byteOffset;
         }
     }
 
     /* The sum of these cannot overflow a uint32_t */
     MOZ_ASSERT(byteOffset <= INT32_MAX);
     MOZ_ASSERT(byteLength <= INT32_MAX);
 
-    if (byteOffset + byteLength > buffer->byteLength()) {
-        JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_ARG_INDEX_OUT_OF_RANGE, "1");
+
+    *byteOffsetPtr = byteOffset;
+    *byteLengthPtr = byteLength;
+
+    return true;
+}
+
+bool
+DataViewObject::constructSameCompartment(JSContext* cx, HandleObject bufobj, const CallArgs& args)
+{
+    MOZ_ASSERT(args.isConstructing());
+    assertSameCompartment(cx, bufobj);
+
+    uint32_t byteOffset, byteLength;
+    if (!getAndCheckConstructorArgs(cx, bufobj, args, &byteOffset, &byteLength))
         return false;
-    }
 
+    RootedObject proto(cx);
+    RootedObject newTarget(cx, &args.newTarget().toObject());
+    if (!GetPrototypeFromConstructor(cx, newTarget, &proto))
+        return false;
+
+    Rooted<ArrayBufferObject*> buffer(cx, &AsArrayBuffer(bufobj));
     JSObject* obj = DataViewObject::create(cx, byteOffset, byteLength, buffer, proto);
     if (!obj)
         return false;
     args.rval().setObject(*obj);
     return true;
 }
 
+// Create a DataView object in another compartment.
+//
+// ES6 supports creating a DataView in global A (using global A's DataView
+// constructor) backed by an ArrayBuffer created in global B.
+//
+// Our DataViewObject implementation doesn't support a DataView in
+// compartment A backed by an ArrayBuffer in compartment B. So in this case,
+// we create the DataView in B (!) and return a cross-compartment wrapper.
+//
+// Extra twist: the spec says the new DataView's [[Prototype]] must be
+// A's DataView.prototype. So even though we're creating the DataView in B,
+// its [[Prototype]] must be (a cross-compartment wrapper for) the
+// DataView.prototype in A.
+//
+// As if this were not confusing enough, the way we actually do this is also
+// tricky. We call compartment A's createDataViewForThis method, passing it
+// bufobj as `this`. That calls ArrayBufferObject::createDataViewForThis(),
+// which uses CallNonGenericMethod to switch to compartment B so that
+// the new DataView is created there.
+bool
+DataViewObject::constructWrapped(JSContext* cx, HandleObject bufobj, const CallArgs& args)
+{
+    MOZ_ASSERT(args.isConstructing());
+    MOZ_ASSERT(bufobj->is<WrapperObject>());
+
+    JSObject* unwrapped = CheckedUnwrap(bufobj);
+    if (!unwrapped) {
+        JS_ReportError(cx, "Permission denied to access object");
+        return false;
+    }
+
+    // NB: This entails the IsArrayBuffer check
+    uint32_t byteOffset, byteLength;
+    if (!getAndCheckConstructorArgs(cx, unwrapped, args, &byteOffset, &byteLength))
+        return false;
+
+    // Make sure to get the [[Prototype]] for the created view from this
+    // compartment.
+    RootedObject proto(cx);
+    RootedObject newTarget(cx, &args.newTarget().toObject());
+    if (!GetPrototypeFromConstructor(cx, newTarget, &proto))
+        return false;
+
+    Rooted<GlobalObject*> global(cx, cx->compartment()->maybeGlobal());
+    if (!proto) {
+        proto = global->getOrCreateDataViewPrototype(cx);
+        if (!proto)
+            return false;
+    }
+
+    InvokeArgs args2(cx);
+    if (!args2.init(3))
+        return false;
+    args2.setCallee(global->createDataViewForThis());
+    args2.setThis(ObjectValue(*bufobj));
+    args2[0].set(PrivateUint32Value(byteOffset));
+    args2[1].set(PrivateUint32Value(byteLength));
+    args2[2].setObject(*proto);
+    if (!Invoke(cx, args2))
+        return false;
+    args.rval().set(args2.rval());
+    return true;
+}
+
 bool
 DataViewObject::class_constructor(JSContext* cx, unsigned argc, Value* vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
 
     if (!ThrowIfNotConstructing(cx, args, "DataView"))
         return false;
 
     RootedObject bufobj(cx);
     if (!GetFirstArgumentAsObject(cx, args, "DataView constructor", &bufobj))
         return false;
 
-    if (bufobj->is<WrapperObject>() && IsArrayBuffer(UncheckedUnwrap(bufobj))) {
-        Rooted<GlobalObject*> global(cx, cx->compartment()->maybeGlobal());
-        Rooted<JSObject*> proto(cx, global->getOrCreateDataViewPrototype(cx));
-        if (!proto)
-            return false;
-
-        InvokeArgs args2(cx);
-        if (!args2.init(args.length() + 1))
-            return false;
-        args2.setCallee(global->createDataViewForThis());
-        args2.setThis(ObjectValue(*bufobj));
-        PodCopy(args2.array(), args.array(), args.length());
-        args2[args.length()].setObject(*proto);
-        if (!Invoke(cx, args2))
-            return false;
-        args.rval().set(args2.rval());
-        return true;
-    }
-
-    return construct(cx, bufobj, args, nullptr);
+    if (bufobj->is<WrapperObject>())
+        return constructWrapped(cx, bufobj, args);
+    return constructSameCompartment(cx, bufobj, args);
 }
 
 template <typename NativeType>
 /* static */ uint8_t*
 DataViewObject::getDataPointer(JSContext* cx, Handle<DataViewObject*> obj, uint32_t offset)
 {
     const size_t TypeSize = sizeof(NativeType);
     if (offset > UINT32_MAX - TypeSize || offset + TypeSize > obj->byteLength()) {
@@ -1270,16 +1344,21 @@ DataViewObject::read(JSContext* cx, Hand
     }
 
     uint32_t offset;
     if (!ToUint32(cx, args[0], &offset))
         return false;
 
     bool fromLittleEndian = args.length() >= 2 && ToBoolean(args[1]);
 
+    if (obj->arrayBuffer().isNeutered()) {
+        JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_TYPED_ARRAY_DETACHED);
+        return false;
+    }
+
     uint8_t* data = DataViewObject::getDataPointer<NativeType>(cx, obj, offset);
     if (!data)
         return false;
 
     DataViewIO<NativeType>::fromBuffer(val, data, needToSwapBytes(fromLittleEndian));
     return true;
 }
 
@@ -1331,16 +1410,21 @@ DataViewObject::write(JSContext* cx, Han
         return false;
 
     NativeType value;
     if (!WebIDLCast(cx, args[1], &value))
         return false;
 
     bool toLittleEndian = args.length() >= 3 && ToBoolean(args[2]);
 
+    if (obj->arrayBuffer().isNeutered()) {
+        JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_TYPED_ARRAY_DETACHED);
+        return false;
+    }
+
     uint8_t* data = DataViewObject::getDataPointer<NativeType>(cx, obj, offset);
     if (!data)
         return false;
 
     DataViewIO<NativeType>::toBuffer(data, &value, needToSwapBytes(toLittleEndian));
     return true;
 }
 
--- a/js/src/vm/TypedArrayObject.h
+++ b/js/src/vm/TypedArrayObject.h
@@ -379,16 +379,26 @@ class DataViewObject : public NativeObje
     template<Value ValueGetter(DataViewObject* view)>
     static bool
     getter(JSContext* cx, unsigned argc, Value* vp);
 
     template<Value ValueGetter(DataViewObject* view)>
     static bool
     defineGetter(JSContext* cx, PropertyName* name, HandleNativeObject proto);
 
+    static bool getAndCheckConstructorArgs(JSContext* cx, JSObject* bufobj, const CallArgs& args,
+                                           uint32_t *byteOffset, uint32_t* byteLength);
+    static bool constructSameCompartment(JSContext* cx, HandleObject bufobj, const CallArgs& args);
+    static bool constructWrapped(JSContext* cx, HandleObject bufobj, const CallArgs& args);
+
+    friend bool ArrayBufferObject::createDataViewForThisImpl(JSContext* cx, const CallArgs& args);
+    static DataViewObject*
+    create(JSContext* cx, uint32_t byteOffset, uint32_t byteLength,
+           Handle<ArrayBufferObject*> arrayBuffer, JSObject* proto);
+
   public:
     static const Class class_;
 
     static Value byteOffsetValue(DataViewObject* view) {
         Value v = view->getFixedSlot(TypedArrayObject::BYTEOFFSET_SLOT);
         MOZ_ASSERT(v.toInt32() >= 0);
         return v;
     }
@@ -415,23 +425,16 @@ class DataViewObject : public NativeObje
         return bufferValue(const_cast<DataViewObject*>(this)).toObject().as<ArrayBufferObject>();
     }
 
     void* dataPointer() const {
         return getPrivate();
     }
 
     static bool class_constructor(JSContext* cx, unsigned argc, Value* vp);
-    static bool constructWithProto(JSContext* cx, unsigned argc, Value* vp);
-    static bool construct(JSContext* cx, JSObject* bufobj, const CallArgs& args,
-                          HandleObject proto);
-
-    static inline DataViewObject*
-    create(JSContext* cx, uint32_t byteOffset, uint32_t byteLength,
-           Handle<ArrayBufferObject*> arrayBuffer, JSObject* proto);
 
     static bool getInt8Impl(JSContext* cx, const CallArgs& args);
     static bool fun_getInt8(JSContext* cx, unsigned argc, Value* vp);
 
     static bool getUint8Impl(JSContext* cx, const CallArgs& args);
     static bool fun_getUint8(JSContext* cx, unsigned argc, Value* vp);
 
     static bool getInt16Impl(JSContext* cx, const CallArgs& args);