Bug 1252270 - SIMD: Coerce non-numeric indexes to load/store function…

…s. r=lth

Follow the DataView functions and use Tonumber to coerce index arguments on the
load/store functions. Throw a RangeError when we see a non-integer index or a
number outside the range of the array.

See tc39/ecmascript_simd#328

MozReview-Commit-ID: IpHkfPyywU0

js/src/builtin/SIMD.cpp

@@ -1258,10 +1258,81 @@ Select(JSContext* cx, unsigned argc, Value* vp)
     return StoreResult<V>(cx, args, result);
 }
 
-template<class VElem, unsigned NumElem>
+// Get an integer array index from a function argument. Coerce if necessary.
+//
+// When a JS function argument represents an integer index into an array, it is
+// laundered like this:
+//
+//   1. numericIndex = ToNumber(argument)            (may throw TypeError)
+//   2. intIndex = ToInteger(numericIndex)
+//   3. if intIndex != numericIndex throw RangeError
+//
+// This function additionally bounds the range to the non-negative contiguous
+// integers:
+//
+//   4. if intIndex < 0 or intIndex > 2^53 throw RangeError
+//
+// Return true and set |*index| to the integer value if |argument| is a valid
+// array index argument. Otherwise report an TypeError or RangeError and return
+// false.
+//
+// The returned index will always be in the range 0 <= *index <= 2^53.
 static bool
-TypedArrayFromArgs(JSContext* cx, const CallArgs& args,
-                   MutableHandleObject typedArray, int32_t* byteStart)
+ArgumentToIntegerIndex(JSContext* cx, JS::HandleValue v, uint64_t* index)
+{
+    // Fast common case.
+    if (v.isInt32()) {
+        int32_t i = v.toInt32();
+        if (i >= 0) {
+            *index = i;
+            return true;
+        }
+    }
+
+    // Slow case. Use ToNumber() to coerce. This may throw a TypeError.
+    double d;
+    if (!ToNumber(cx, v, &d))
+        return false;
+
+    // Check that |d| is an integer in the valid range.
+    //
+    // Not all floating point integers fit in the range of a uint64_t, so we
+    // need a rough range check before the real range check in our caller. We
+    // could limit indexes to UINT64_MAX, but this would mean that our callers
+    // have to be very careful about integer overflow. The contiguous integer
+    // floating point numbers end at 2^53, so make that our upper limit. If we
+    // ever support arrays with more than 2^53 elements, this will need to
+    // change.
+    //
+    // Reject infinities, NaNs, and numbers outside the contiguous integer range
+    // with a RangeError.
+
+    // Write relation so NaNs throw a RangeError.
+    if (!(0 <= d && d <= (uint64_t(1) << 53))) {
+        JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_BAD_INDEX);
+        return false;
+    }
+
+    // Check that d is an integer, throw a RangeError if not.
+    // Note that this conversion could invoke undefined behaviour without the
+    // range check above.
+    uint64_t i(d);
+    if (d != double(i)) {
+        JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_BAD_INDEX);
+        return false;
+    }
+
+    *index = i;
+    return true;
+}
+
+// Look for arguments (ta, idx) where ta is a TypedArray and idx is a
+// non-negative integer.
+// Check that accessBytes can be accessed starting from index idx in the array.
+// Return the array handle in typedArray and idx converted to a byte offset in byteStart.
+static bool
+TypedArrayFromArgs(JSContext* cx, const CallArgs& args, uint32_t accessBytes,
+                   MutableHandleObject typedArray, size_t* byteStart)
 {
     if (!args[0].isObject())
         return ErrorBadArgs(cx);
@@ -1272,18 +1343,19 @@ TypedArrayFromArgs(JSContext* cx, const CallArgs& args,
 
     typedArray.set(&argobj);
 
-    int32_t index;
-    if (!ToInt32(cx, args[1], &index))
+    uint64_t index;
+    if (!ArgumentToIntegerIndex(cx, args[1], &index))
         return false;
 
-    *byteStart = index * typedArray->as<TypedArrayObject>().bytesPerElement();
-    if (*byteStart < 0 || (uint32_t(*byteStart) + NumElem * sizeof(VElem)) >
-                          typedArray->as<TypedArrayObject>().byteLength())
-    {
+    // Do the range check in 64 bits even when size_t is 32 bits.
+    // This can't overflow because index <= 2^53.
+    uint64_t bytes = index * typedArray->as<TypedArrayObject>().bytesPerElement();
+    if ((bytes + accessBytes) > typedArray->as<TypedArrayObject>().byteLength()) {
         // Keep in sync with AsmJS OnOutOfBounds function.
         JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_BAD_INDEX);
         return false;
     }
+    *byteStart = bytes;
 
     return true;
 }
@@ -1298,9 +1370,9 @@ Load(JSContext* cx, unsigned argc, Value* vp)
     if (args.length() != 2)
         return ErrorBadArgs(cx);
 
-    int32_t byteStart;
+    size_t byteStart;
     RootedObject typedArray(cx);
-    if (!TypedArrayFromArgs<Elem, NumElem>(cx, args, &typedArray, &byteStart))
+    if (!TypedArrayFromArgs(cx, args, sizeof(Elem) * NumElem, &typedArray, &byteStart))
         return false;
 
     Rooted<TypeDescr*> typeDescr(cx, GetTypeDescr<V>(cx));
@@ -1330,9 +1402,9 @@ Store(JSContext* cx, unsigned argc, Value* vp)
     if (args.length() != 3)
         return ErrorBadArgs(cx);
 
-    int32_t byteStart;
+    size_t byteStart;
     RootedObject typedArray(cx);
-    if (!TypedArrayFromArgs<Elem, NumElem>(cx, args, &typedArray, &byteStart))
+    if (!TypedArrayFromArgs(cx, args, sizeof(Elem) * NumElem, &typedArray, &byteStart))
         return false;
 
     if (!IsVectorObject<V>(args[2]))

js/src/tests/ecma_7/SIMD/load.js

@@ -126,7 +126,18 @@ function testLoad(kind, TA) {
         assertThrowsInstanceOf(() => SIMD[kind].load(), TypeError);
         assertThrowsInstanceOf(() => SIMD[kind].load(ta), TypeError);
         assertThrowsInstanceOf(() => SIMD[kind].load("hello", 0), TypeError);
+        // Indexes must be integers, there is no rounding.
+        assertThrowsInstanceOf(() => SIMD[kind].load(ta, 1.5), RangeError);
         assertThrowsInstanceOf(() => SIMD[kind].load(ta, -1), RangeError);
+        assertThrowsInstanceOf(() => SIMD[kind].load(ta, "hello"), RangeError);
+        assertThrowsInstanceOf(() => SIMD[kind].load(ta, NaN), RangeError);
+        // Try to trip up the bounds checking. Int32 is enough for everybody.
+        assertThrowsInstanceOf(() => SIMD[kind].load(ta, 0x100000000), RangeError);
+        assertThrowsInstanceOf(() => SIMD[kind].load(ta, 0x80000000), RangeError);
+        assertThrowsInstanceOf(() => SIMD[kind].load(ta, 0x40000000), RangeError);
+        assertThrowsInstanceOf(() => SIMD[kind].load(ta, 0x20000000), RangeError);
+        assertThrowsInstanceOf(() => SIMD[kind].load(ta, (1<<30) * (1<<23) - 1), RangeError);
+        assertThrowsInstanceOf(() => SIMD[kind].load(ta, (1<<30) * (1<<23)), RangeError);
 
         // Valid and invalid reads
         var C = MakeComparator(kind, ta);
@@ -169,13 +180,15 @@ function testLoad(kind, TA) {
             assertThrowsInstanceOf(() => SIMD[kind].load2(ta, lastValidArgLoad2 + 1), RangeError);
             assertThrowsInstanceOf(() => SIMD[kind].load3(ta, lastValidArgLoad3 + 1), RangeError);
         }
+
+        // Indexes are coerced with ToNumber. Try some strings that
+        // CanonicalNumericIndexString() would reject.
+        C.load("1.0e0");
+        C.load(" 2");
     }
 
     if (lanes == 4) {
-        // Test ToInt32 behavior
-        var v = SIMD[kind].load(TA, 12.5);
-        assertEqX4(v, [12, 13, 14, 15]);
-
+        // Test ToNumber behavior.
         var obj = {
             valueOf: function() { return 12 }
         }