Bug 1317382 - DataView constructor uses ToIndex in ES2017. r=Waldo
authorTom Schuster <evilpies@gmail.com>
Wed, 15 Feb 2017 23:24:44 +0100
changeset 343204 5a394ec7d209c5edd03760d056fe5fa8dcb9125d
parent 343203 b9ee8724079936ea2adab7d017ff7a3aab61c151
child 343205 7838f017fe4c83604dd7a93656b7a4b3360ecf2d
push id31372
push usercbook@mozilla.com
push dateThu, 16 Feb 2017 12:16:10 +0000
treeherdermozilla-central@2737f66ad6ac [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersWaldo
bugs1317382
milestone54.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 1317382 - DataView constructor uses ToIndex in ES2017. r=Waldo
js/src/builtin/DataViewObject.cpp
js/src/tests/jstests.list
--- a/js/src/builtin/DataViewObject.cpp
+++ b/js/src/builtin/DataViewObject.cpp
@@ -2,16 +2,17 @@
  * vim: set ts=8 sts=4 et sw=4 tw=99:
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "builtin/DataViewObject.h"
 
 #include "mozilla/Alignment.h"
+#include "mozilla/Casting.h"
 
 #include <string.h>
 
 #include "jsapi.h"
 #include "jsarray.h"
 #include "jscntxt.h"
 #include "jsnum.h"
 #include "jsobj.h"
@@ -31,16 +32,17 @@
 #include "gc/Nursery-inl.h"
 #include "gc/StoreBuffer-inl.h"
 #include "vm/ArrayBufferObject-inl.h"
 #include "vm/NativeObject-inl.h"
 
 using namespace js;
 using namespace js::gc;
 
+using mozilla::AssertedCast;
 using JS::CanonicalizeNaN;
 using JS::ToInt32;
 using JS::ToUint32;
 
 static NewObjectKind
 DataViewNewObjectKind(JSContext* cx, uint32_t byteLength, JSObject* proto)
 {
     if (!proto && byteLength >= TypedArrayObject::SINGLETON_BYTE_LENGTH)
@@ -134,81 +136,73 @@ DataViewObject::create(JSContext* cx, ui
     if (arrayBuffer->is<ArrayBufferObject>()) {
         if (!arrayBuffer->as<ArrayBufferObject>().addView(cx, &dvobj))
             return nullptr;
     }
 
     return &dvobj;
 }
 
+// ES2017 draft rev 931261ecef9b047b14daacf82884134da48dfe0f
+// 24.3.2.1 DataView (extracted part of the main algorithm)
 bool
 DataViewObject::getAndCheckConstructorArgs(JSContext* cx, HandleObject bufobj, const CallArgs& args,
                                            uint32_t* byteOffsetPtr, uint32_t* byteLengthPtr)
 {
+    // Step 3.
     if (!IsArrayBufferMaybeShared(bufobj)) {
         JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_NOT_EXPECTED_TYPE,
                                   "DataView", "ArrayBuffer", bufobj->getClass()->name);
         return false;
     }
-
     Rooted<ArrayBufferObjectMaybeShared*> buffer(cx, &AsArrayBufferMaybeShared(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_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_ARG_INDEX_OUT_OF_RANGE,
-                                      "1");
-            return false;
-        }
-    }
+    // Step 4.
+    uint64_t offset;
+    if (!ToIndex(cx, args.get(1), &offset))
+        return false;
 
+    // Step 5.
     if (buffer->isDetached()) {
         JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_TYPED_ARRAY_DETACHED);
         return false;
     }
 
-    if (args.length() > 1) {
-        if (byteOffset > byteLength) {
-            JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_ARG_INDEX_OUT_OF_RANGE,
-                                      "1");
+    // Step 6.
+    uint32_t bufferByteLength = buffer->byteLength();
+
+    // Step 7.
+    if (offset > bufferByteLength) {
+        JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_ARG_INDEX_OUT_OF_RANGE, "1");
+        return false;
+    }
+    MOZ_ASSERT(offset <= INT32_MAX);
+
+    // Step 8.a
+    uint64_t viewByteLength = bufferByteLength - offset;
+    if (args.hasDefined(2)) {
+        // Step 9.a.
+        if (!ToIndex(cx, args.get(2), &viewByteLength))
+            return false;
+
+
+        MOZ_ASSERT(offset + viewByteLength >= offset,
+                   "can't overflow: both numbers are less than DOUBLE_INTEGRAL_PRECISION_LIMIT");
+
+        // Step 9.b.
+        if (offset + viewByteLength > bufferByteLength) {
+            JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
+                                      JSMSG_ARG_INDEX_OUT_OF_RANGE, "2");
             return false;
         }
-
-        if (args.get(2).isUndefined()) {
-            byteLength -= byteOffset;
-        } else {
-            if (!ToUint32(cx, args[2], &byteLength))
-                return false;
-            if (byteLength > INT32_MAX) {
-                JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
-                                          JSMSG_ARG_INDEX_OUT_OF_RANGE, "2");
-                return false;
-            }
+    }
+    MOZ_ASSERT(viewByteLength <= INT32_MAX);
 
-            MOZ_ASSERT(byteOffset + byteLength >= byteOffset,
-                       "can't overflow: both numbers are less than INT32_MAX");
-            if (byteOffset + byteLength > buffer->byteLength()) {
-                JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
-                                          JSMSG_ARG_INDEX_OUT_OF_RANGE, "1");
-                return false;
-            }
-        }
-    }
-
-    /* The sum of these cannot overflow a uint32_t */
-    MOZ_ASSERT(byteOffset <= INT32_MAX);
-    MOZ_ASSERT(byteLength <= INT32_MAX);
-
-
-    *byteOffsetPtr = byteOffset;
-    *byteLengthPtr = byteLength;
-
+    *byteOffsetPtr = AssertedCast<uint32_t>(offset);
+    *byteLengthPtr = AssertedCast<uint32_t>(viewByteLength);
     return true;
 }
 
 bool
 DataViewObject::constructSameCompartment(JSContext* cx, HandleObject bufobj, const CallArgs& args)
 {
     MOZ_ASSERT(args.isConstructing());
     assertSameCompartment(cx, bufobj);
--- a/js/src/tests/jstests.list
+++ b/js/src/tests/jstests.list
@@ -411,23 +411,16 @@ skip script test262/built-ins/RegExp/pro
 
 # https://bugzilla.mozilla.org/show_bug.cgi?id=1317395
 skip script test262/built-ins/ArrayBuffer/prototype/byteLength/detached-buffer.js
 
 # https://bugzilla.mozilla.org/show_bug.cgi?id=1317394
 skip script test262/built-ins/DataView/prototype/byteOffset/detached-buffer.js
 skip script test262/built-ins/DataView/prototype/byteLength/detached-buffer.js
 
-# https://bugzilla.mozilla.org/show_bug.cgi?id=1317382
-skip script test262/built-ins/DataView/byteoffset-is-negative-throws.js
-skip script test262/built-ins/DataView/excessive-byteoffset-throws.js
-skip script test262/built-ins/DataView/excessive-bytelength-throws.js
-skip script test262/built-ins/DataView/negative-byteoffset-throws.js
-skip script test262/built-ins/DataView/negative-bytelength-throws.js
-
 # https://bugzilla.mozilla.org/show_bug.cgi?id=1317391
 skip script test262/language/statements/class/subclass/class-definition-null-proto-missing-return-override.js
 skip script test262/language/statements/class/subclass/class-definition-null-proto-this.js
 
 # https://bugzilla.mozilla.org/show_bug.cgi?id=1108941
 skip script test262/language/expressions/tagged-template/cache-differing-expressions.js
 skip script test262/language/expressions/tagged-template/cache-differing-expressions-eval.js
 skip script test262/language/expressions/tagged-template/cache-differing-expressions-new-function.js