Bug 1242165 - Update DataView index handling. r=Waldo
authorTom Schuster <evilpies@gmail.com>
Thu, 28 Jul 2016 00:24:17 +0200
changeset 346988 450787142f41b76eac55038c62d544f43831cd06
parent 346987 5083d26c9c92a364f954be87a4aea2323950afc4
child 346989 b8deefd10b85ec0c4ce65599c8db2f9d9f04f5d4
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)
reviewersWaldo
bugs1242165
milestone50.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 1242165 - Update DataView index handling. r=Waldo
js/src/tests/ecma_6/DataView/get-set-index-range.js
js/src/tests/js1_8_5/extensions/dataview.js
js/src/vm/TypedArrayObject.cpp
js/src/vm/TypedArrayObject.h
new file mode 100644
--- /dev/null
+++ b/js/src/tests/ecma_6/DataView/get-set-index-range.js
@@ -0,0 +1,36 @@
+var buffer = new ArrayBuffer(2);
+var view = new DataView(buffer);
+
+function check(view) {
+    for (let fun of ['getInt8', 'setInt8', 'getInt16', 'setInt16']) {
+        assertThrowsInstanceOf(() => view[fun](-10), RangeError);
+        assertThrowsInstanceOf(() => view[fun](-Infinity), RangeError);
+        assertThrowsInstanceOf(() => view[fun](Infinity), RangeError);
+
+        assertThrowsInstanceOf(() => view[fun](Math.pow(2, 53)), RangeError);
+        assertThrowsInstanceOf(() => view[fun](Math.pow(2, 54)), RangeError);
+    }
+}
+
+check(view);
+
+for (let fun of ['getInt8', 'getInt16']) {
+    assertEq(view[fun](0), 0);
+    assertEq(view[fun](undefined), 0);
+    assertEq(view[fun](NaN), 0);
+}
+
+if ('detachArrayBuffer' in this) {
+    // ToIndex is called before detachment check, so we can tell the difference
+    // between a ToIndex failure and a real out of bounds failure.
+    detachArrayBuffer(buffer, 'same-data');
+
+    check(view);
+
+    assertThrowsInstanceOf(() => view.getInt8(0), TypeError);
+    assertThrowsInstanceOf(() => view.setInt8(0, 0), TypeError);
+    assertThrowsInstanceOf(() => view.getInt8(Math.pow(2, 53) - 1), TypeError);
+    assertThrowsInstanceOf(() => view.setInt8(Math.pow(2, 53) - 1, 0), TypeError);
+}
+
+reportCompare(0, 0, 'OK');
--- a/js/src/tests/js1_8_5/extensions/dataview.js
+++ b/js/src/tests/js1_8_5/extensions/dataview.js
@@ -722,28 +722,16 @@ function test() {
     checkThrow(() => view.getInt32(2147483649), RangeError);
     checkThrow(() => view.getUint32(2147483648), RangeError);
     checkThrow(() => view.getUint32(2147483649), RangeError);
     checkThrow(() => view.getFloat32(2147483648), RangeError);
     checkThrow(() => view.getFloat32(2147483649), RangeError);
     checkThrow(() => view.getFloat64(2147483648), RangeError);
     checkThrow(() => view.getFloat64(2147483649), RangeError);
 
-    // Test for wrong arguments passed to get methods
-    //
-    // See http://www.w3.org/TR/WebIDL/#es-operations and the step saying "If entry is null, throw a TypeError."
-    checkThrow(() => view.getInt8(), TypeError);
-    checkThrow(() => view.getUint8(), TypeError);
-    checkThrow(() => view.getInt16(), TypeError);
-    checkThrow(() => view.getUint16(), TypeError);
-    checkThrow(() => view.getInt32(), TypeError);
-    checkThrow(() => view.getUint32(), TypeError);
-    checkThrow(() => view.getFloat32(), TypeError);
-    checkThrow(() => view.getFloat64(), TypeError);
-
     // Test for wrong type of |this|
     checkThrow(() => view.getInt8.apply("dead", [0]), TypeError);
     checkThrow(() => view.getUint8.apply("puppies", [0]), TypeError);
     checkThrow(() => view.getInt16.apply("aren", [0]), TypeError);
     checkThrow(() => view.getUint16.apply("t", [0]), TypeError);
     checkThrow(() => view.getInt32.apply("much", [0]), TypeError);
     checkThrow(() => view.getUint32.apply("fun", [0]), TypeError);
     checkThrow(() => view.getFloat32.apply("(in", [0]), TypeError);
@@ -1508,34 +1496,16 @@ function test() {
     checkThrow(() => view.setUint8(2147483649, 1), RangeError);
     checkThrow(() => view.setInt16(2147483649, 1), RangeError);
     checkThrow(() => view.setUint16(2147483649, 1), RangeError);
     checkThrow(() => view.setInt32(2147483649, 1), RangeError);
     checkThrow(() => view.setUint32(2147483649, 1), RangeError);
     checkThrow(() => view.setFloat32(2147483649, 1), RangeError);
     checkThrow(() => view.setFloat64(2147483649, 1), RangeError);
 
-    // Test for wrong arguments passed to set methods
-    checkThrow(() => view.setInt8(), Error);
-    checkThrow(() => view.setUint8(), Error);
-    checkThrow(() => view.setInt16(), Error);
-    checkThrow(() => view.setUint16(), Error);
-    checkThrow(() => view.setInt32(), Error);
-    checkThrow(() => view.setUint32(), Error);
-    checkThrow(() => view.setFloat32(), Error);
-    checkThrow(() => view.setFloat64(), Error);
-    checkThrow(() => view.setInt8(1), Error);
-    checkThrow(() => view.setUint8(1), Error);
-    checkThrow(() => view.setInt16(1), Error);
-    checkThrow(() => view.setUint16(1), Error);
-    checkThrow(() => view.setInt32(1), Error);
-    checkThrow(() => view.setUint32(1), Error);
-    checkThrow(() => view.setFloat32(1), Error);
-    checkThrow(() => view.setFloat64(1), Error);
-
     // testAlignment
     var intArray1 = [0, 1, 2, 3, 100, 101, 102, 103, 128, 129, 130, 131, 252, 253, 254, 255];
     view = new DataView((new Uint8Array(intArray1)).buffer, 0);
     assertEq(view.getUint32(0, false), 0x00010203);
     view = new DataView((new Uint8Array(intArray1)).buffer, 1);
     assertEq(view.getUint32(0, false), 0x01020364);
     view = new DataView((new Uint8Array(intArray1)).buffer, 2);
     assertEq(view.getUint32(0, false), 0x02036465);
--- a/js/src/vm/TypedArrayObject.cpp
+++ b/js/src/vm/TypedArrayObject.cpp
@@ -1735,25 +1735,28 @@ DataViewObject::class_constructor(JSCont
 
     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)
+DataViewObject::getDataPointer(JSContext* cx, Handle<DataViewObject*> obj, double offset)
 {
+    MOZ_ASSERT(offset >= 0);
+
     const size_t TypeSize = sizeof(NativeType);
     if (offset > UINT32_MAX - TypeSize || offset + TypeSize > obj->byteLength()) {
         JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_ARG_INDEX_OUT_OF_RANGE, "1");
         return nullptr;
     }
 
-    return static_cast<uint8_t*>(obj->dataPointer()) + offset;
+    MOZ_ASSERT(offset < UINT32_MAX);
+    return static_cast<uint8_t*>(obj->dataPointer()) + uint32_t(offset);
 }
 
 static inline bool
 needToSwapBytes(bool littleEndian)
 {
 #if IS_LITTLE_ENDIAN
     return !littleEndian;
 #else
@@ -1820,43 +1823,70 @@ struct DataViewIO
         MOZ_ASSERT((reinterpret_cast<uintptr_t>(src) & (Min<size_t>(MOZ_ALIGNOF(void*), sizeof(DataType)) - 1)) == 0);
         ReadWriteType temp = *reinterpret_cast<const ReadWriteType*>(src);
         if (wantSwap)
             temp = swapBytes(temp);
         memcpy(unalignedBuffer, (void*) &temp, sizeof(ReadWriteType));
     }
 };
 
+static bool
+ToIndex(JSContext* cx, HandleValue v, double* index)
+{
+    if (v.isUndefined()) {
+        *index = 0.0;
+        return true;
+    }
+
+    double integerIndex;
+    if (!ToInteger(cx, v, &integerIndex))
+        return false;
+
+    // Inlined version of ToLength.
+    // 1. Already an integer
+    // 2. Step eliminates < 0, +0 == -0 with SameValueZero
+    // 3/4. Limit to <= 2^53-1, so everything above should fail.
+    if (integerIndex < 0 || integerIndex > 9007199254740991) {
+        JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_BAD_INDEX);
+        return false;
+    }
+
+    *index = integerIndex;
+    return true;
+}
+
 template<typename NativeType>
 /* static */ bool
 DataViewObject::read(JSContext* cx, Handle<DataViewObject*> obj,
                      const CallArgs& args, NativeType* val, const char* method)
 {
-    if (args.length() < 1) {
-        JS_ReportErrorNumber(cx, GetErrorMessage, nullptr,
-                             JSMSG_MORE_ARGS_NEEDED, method, "0", "s");
+    // Steps 1-2. done by the caller
+    // Step 3. unnecessary assert
+
+    // Step 4.
+    double getIndex;
+    if (!ToIndex(cx, args.get(0), &getIndex))
         return false;
-    }
-
-    uint32_t offset;
-    if (!ToUint32(cx, args[0], &offset))
-        return false;
-
-    bool fromLittleEndian = args.length() >= 2 && ToBoolean(args[1]);
-
+
+    // Step 5.
+    bool isLittleEndian = args.length() >= 2 && ToBoolean(args[1]);
+
+    // Steps 6-7.
     if (obj->arrayBuffer().isDetached()) {
         JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_TYPED_ARRAY_DETACHED);
         return false;
     }
 
-    uint8_t* data = DataViewObject::getDataPointer<NativeType>(cx, obj, offset);
+    // Steps 8-12.
+    uint8_t* data = DataViewObject::getDataPointer<NativeType>(cx, obj, getIndex);
     if (!data)
         return false;
 
-    DataViewIO<NativeType>::fromBuffer(val, data, needToSwapBytes(fromLittleEndian));
+    // Step 13.
+    DataViewIO<NativeType>::fromBuffer(val, data, needToSwapBytes(isLittleEndian));
     return true;
 }
 
 template <typename NativeType>
 static inline bool
 WebIDLCast(JSContext* cx, HandleValue value, NativeType* out)
 {
     int32_t temp;
@@ -1887,48 +1917,51 @@ WebIDLCast<double>(JSContext* cx, Handle
     return ToNumber(cx, value, out);
 }
 
 template<typename NativeType>
 /* static */ bool
 DataViewObject::write(JSContext* cx, Handle<DataViewObject*> obj,
                       const CallArgs& args, const char* method)
 {
-    if (args.length() < 2) {
-        JS_ReportErrorNumber(cx, GetErrorMessage, nullptr,
-                             JSMSG_MORE_ARGS_NEEDED, method, "1", "");
+    // Steps 1-2. done by the caller
+    // Step 3. unnecessary assert
+
+    // Step 4.
+    double getIndex;
+    if (!ToIndex(cx, args.get(0), &getIndex))
         return false;
-    }
-
-    uint32_t offset;
-    if (!ToUint32(cx, args[0], &offset))
-        return false;
-
+
+    // Step 5. Should just call ToNumber (unobservable)
     NativeType value;
-    if (!WebIDLCast(cx, args[1], &value))
+    if (!WebIDLCast(cx, args.get(1), &value))
         return false;
 
 #ifdef JS_MORE_DETERMINISTIC
     // See the comment in ElementSpecific::doubleToNative.
     if (TypeIsFloatingPoint<NativeType>())
         value = JS::CanonicalizeNaN(value);
 #endif
 
-    bool toLittleEndian = args.length() >= 3 && ToBoolean(args[2]);
-
+    // Step 6.
+    bool isLittleEndian = args.length() >= 3 && ToBoolean(args[2]);
+
+    // Steps 7-8.
     if (obj->arrayBuffer().isDetached()) {
         JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_TYPED_ARRAY_DETACHED);
         return false;
     }
 
-    uint8_t* data = DataViewObject::getDataPointer<NativeType>(cx, obj, offset);
+    // Steps 9-13.
+    uint8_t* data = DataViewObject::getDataPointer<NativeType>(cx, obj, getIndex);
     if (!data)
         return false;
 
-    DataViewIO<NativeType>::toBuffer(data, &value, needToSwapBytes(toLittleEndian));
+    // Step 14.
+    DataViewIO<NativeType>::toBuffer(data, &value, needToSwapBytes(isLittleEndian));
     return true;
 }
 
 bool
 DataViewObject::getInt8Impl(JSContext* cx, const CallArgs& args)
 {
     MOZ_ASSERT(is(args.thisv()));
 
@@ -2587,30 +2620,30 @@ const Class DataViewObject::class_ = {
     JSCLASS_HAS_RESERVED_SLOTS(TypedArrayObject::RESERVED_SLOTS) |
     JSCLASS_HAS_CACHED_PROTO(JSProto_DataView),
     &DataViewObjectClassOps
 };
 
 const JSFunctionSpec DataViewObject::jsfuncs[] = {
     JS_FN("getInt8",    DataViewObject::fun_getInt8,      1,0),
     JS_FN("getUint8",   DataViewObject::fun_getUint8,     1,0),
-    JS_FN("getInt16",   DataViewObject::fun_getInt16,     2,0),
-    JS_FN("getUint16",  DataViewObject::fun_getUint16,    2,0),
-    JS_FN("getInt32",   DataViewObject::fun_getInt32,     2,0),
-    JS_FN("getUint32",  DataViewObject::fun_getUint32,    2,0),
-    JS_FN("getFloat32", DataViewObject::fun_getFloat32,   2,0),
-    JS_FN("getFloat64", DataViewObject::fun_getFloat64,   2,0),
+    JS_FN("getInt16",   DataViewObject::fun_getInt16,     1,0),
+    JS_FN("getUint16",  DataViewObject::fun_getUint16,    1,0),
+    JS_FN("getInt32",   DataViewObject::fun_getInt32,     1,0),
+    JS_FN("getUint32",  DataViewObject::fun_getUint32,    1,0),
+    JS_FN("getFloat32", DataViewObject::fun_getFloat32,   1,0),
+    JS_FN("getFloat64", DataViewObject::fun_getFloat64,   1,0),
     JS_FN("setInt8",    DataViewObject::fun_setInt8,      2,0),
     JS_FN("setUint8",   DataViewObject::fun_setUint8,     2,0),
-    JS_FN("setInt16",   DataViewObject::fun_setInt16,     3,0),
-    JS_FN("setUint16",  DataViewObject::fun_setUint16,    3,0),
-    JS_FN("setInt32",   DataViewObject::fun_setInt32,     3,0),
-    JS_FN("setUint32",  DataViewObject::fun_setUint32,    3,0),
-    JS_FN("setFloat32", DataViewObject::fun_setFloat32,   3,0),
-    JS_FN("setFloat64", DataViewObject::fun_setFloat64,   3,0),
+    JS_FN("setInt16",   DataViewObject::fun_setInt16,     2,0),
+    JS_FN("setUint16",  DataViewObject::fun_setUint16,    2,0),
+    JS_FN("setInt32",   DataViewObject::fun_setInt32,     2,0),
+    JS_FN("setUint32",  DataViewObject::fun_setUint32,    2,0),
+    JS_FN("setFloat32", DataViewObject::fun_setFloat32,   2,0),
+    JS_FN("setFloat64", DataViewObject::fun_setFloat64,   2,0),
     JS_FS_END
 };
 
 template<Value ValueGetter(DataViewObject* view)>
 bool
 DataViewObject::getterImpl(JSContext* cx, const CallArgs& args)
 {
     args.rval().set(ValueGetter(&args.thisv().toObject().as<DataViewObject>()));
--- a/js/src/vm/TypedArrayObject.h
+++ b/js/src/vm/TypedArrayObject.h
@@ -419,17 +419,17 @@ class DataViewObject : public NativeObje
     static const Class protoClass;
 
     static bool is(HandleValue v) {
         return v.isObject() && v.toObject().hasClass(&class_);
     }
 
     template <typename NativeType>
     static uint8_t*
-    getDataPointer(JSContext* cx, Handle<DataViewObject*> obj, uint32_t offset);
+    getDataPointer(JSContext* cx, Handle<DataViewObject*> obj, double offset);
 
     template<Value ValueGetter(DataViewObject* view)>
     static bool
     getterImpl(JSContext* cx, const CallArgs& args);
 
     template<Value ValueGetter(DataViewObject* view)>
     static bool
     getter(JSContext* cx, unsigned argc, Value* vp);