Title: [285117] trunk
Revision
285117
Author
rmoris...@apple.com
Date
2021-11-01 11:01:21 -0700 (Mon, 01 Nov 2021)

Log Message

JSGenericTypedArrayView<Adaptor>::set crashes if the length + objectOffset is > UINT32_MAX
https://bugs.webkit.org/show_bug.cgi?id=231975
rdar://84402043

Reviewed by Yusuke Suzuki.

JSTests:

- regress-84402043 is the testcase that revealed the problem.
- typed-array-set-large(-offset) test the same function, in the typed-array to typed-array case
- typed-array-large-slice tests the only caller that passes a non-0 objectOffset, and found other issues with it
- typed-array-large-oob-eventually-not.js is just another test of the Wasm4GB change that I had forgotten to commit

* stress/regress-84402043.js: Added.
* stress/typed-array-large-oob-eventually-not.js: Added.
(test):
* stress/typed-array-large-slice.js: Added.
(expect):
* stress/typed-array-set-large-offset.js: Added.
* stress/typed-array-set-large.js: Added.

Source/_javascript_Core:

UINT_MAX (and anything above it) is not a valid array index, so we cannot use JSObject::get(JSGlobalObject*, unsigned) with an index that big.
This was pointed by Yusuke in his review of my recent patch that introduced the problem (https://bugs.webkit.org/show_bug.cgi?id=229353#c21),
but I misunderstood the code and thought we could never get values that big at that point, thus only putting a RELEASE_ASSERT.
In this patch I instead apply his original suggestion to have a first loop using the (fast) JSObject::get(),
and a second loop for any large indices, using a slower but safe code path.

I also fixed an unrelated bug I noticed in Clobberize/AbstractInterpreter while testing the rest of the patch:
they were not aware that NewTypedArray can take a Int52RepUse child.

Finally, while trying to properly test this change, I discovered that genericTypedArrayViewProtoFuncSlice
(which is the only caller of JSGenericTypedArrayView<Adaptor>::set which passes it a non-0 objectOffset)
was still using unsigned everywhere instead of size_t, and that the same was true of all other functions in the same file.
So I fixed it in the same patch.

* dfg/DFGAbstractInterpreterInlines.h:
(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
* dfg/DFGClobberize.h:
(JSC::DFG::clobberize):
* runtime/JSArrayBufferConstructor.cpp:
(JSC::JSGenericArrayBufferConstructor<sharingMode>::constructImpl):
* runtime/JSGenericTypedArrayViewConstructorInlines.h:
(JSC::constructGenericTypedArrayViewImpl):
* runtime/JSGenericTypedArrayViewInlines.h:
(JSC::JSGenericTypedArrayView<Adaptor>::set):
* runtime/JSGenericTypedArrayViewPrototypeFunctions.h:
(JSC::argumentClampedIndexFromStartOrEnd):
(JSC::genericTypedArrayViewProtoFuncSet):
(JSC::genericTypedArrayViewProtoFuncCopyWithin):
(JSC::genericTypedArrayViewProtoFuncIncludes):
(JSC::genericTypedArrayViewProtoFuncIndexOf):
(JSC::genericTypedArrayViewProtoFuncJoin):
(JSC::genericTypedArrayViewProtoFuncFill):
(JSC::genericTypedArrayViewProtoFuncLastIndexOf):
(JSC::genericTypedArrayViewProtoFuncSlice):
(JSC::genericTypedArrayViewPrivateFuncSubarrayCreate):

Modified Paths

Added Paths

Diff

Modified: trunk/JSTests/ChangeLog (285116 => 285117)


--- trunk/JSTests/ChangeLog	2021-11-01 17:50:16 UTC (rev 285116)
+++ trunk/JSTests/ChangeLog	2021-11-01 18:01:21 UTC (rev 285117)
@@ -1,3 +1,24 @@
+2021-11-01  Robin Morisset  <rmoris...@apple.com>
+
+        JSGenericTypedArrayView<Adaptor>::set crashes if the length + objectOffset is > UINT32_MAX
+        https://bugs.webkit.org/show_bug.cgi?id=231975
+        rdar://84402043
+
+        Reviewed by Yusuke Suzuki.
+
+        - regress-84402043 is the testcase that revealed the problem.
+        - typed-array-set-large(-offset) test the same function, in the typed-array to typed-array case
+        - typed-array-large-slice tests the only caller that passes a non-0 objectOffset, and found other issues with it
+        - typed-array-large-oob-eventually-not.js is just another test of the Wasm4GB change that I had forgotten to commit
+
+        * stress/regress-84402043.js: Added.
+        * stress/typed-array-large-oob-eventually-not.js: Added.
+        (test):
+        * stress/typed-array-large-slice.js: Added.
+        (expect):
+        * stress/typed-array-set-large-offset.js: Added.
+        * stress/typed-array-set-large.js: Added.
+
 2021-10-29  Yusuke Suzuki  <ysuz...@apple.com>
 
         Unreviewed, reduce # of iterations in stress/validate-int-52-ai-state.js due to Debug timeout

Added: trunk/JSTests/stress/regress-84402043.js (0 => 285117)


--- trunk/JSTests/stress/regress-84402043.js	                        (rev 0)
+++ trunk/JSTests/stress/regress-84402043.js	2021-11-01 18:01:21 UTC (rev 285117)
@@ -0,0 +1,4 @@
+//@skip if $memoryLimited
+//@requireOptions("--watchdog=1000", "--watchdog-exception-ok")
+
+new Uint8Array({length: 2**32});

Added: trunk/JSTests/stress/typed-array-large-oob-eventually-not.js (0 => 285117)


--- trunk/JSTests/stress/typed-array-large-oob-eventually-not.js	                        (rev 0)
+++ trunk/JSTests/stress/typed-array-large-oob-eventually-not.js	2021-11-01 18:01:21 UTC (rev 285117)
@@ -0,0 +1,29 @@
+//@ skip
+// This tests takes >4s even in release mode on an M1 MBP, so I'd rather avoid running it on EWS by default.
+
+let _oneGiga_ = 1024 * 1024 * 1024;
+
+function test(array, actualLength, string)
+{
+    for (var i = 0; i < 1000000; ++i) {
+        var index = actualLength + 10;
+        var value = 42;
+        array[index] = value;
+        var result = array[index];
+        if (result != undefined)
+            throw ("Expected " + value + " but got " + result + " in case " + string);
+    }
+    var value = 42;
+    var index = 10;
+    array[index] = value;
+    var result = array[index]
+    if (result != value)
+        throw ("In out-of-bounds case, expected undefined but got " + result + " in case " + string);
+}
+
+let threeGigs = 3 * oneGiga;
+let fourGigs = 4 * oneGiga;
+
+test(new Int8Array(threeGigs), threeGigs, "Int8Array/3GB");
+test(new Uint8Array(fourGigs), fourGigs, "Uint8Array/4GB");
+test(new Uint8ClampedArray(threeGigs), threeGigs, "Uint8ClampedArray/3GB");

Added: trunk/JSTests/stress/typed-array-large-slice.js (0 => 285117)


--- trunk/JSTests/stress/typed-array-large-slice.js	                        (rev 0)
+++ trunk/JSTests/stress/typed-array-large-slice.js	2021-11-01 18:01:21 UTC (rev 285117)
@@ -0,0 +1,35 @@
+//@ skip if $memoryLimited or ($architecture != "arm64" && $architecture != "x86-64")
+
+let giga = 1024 * 1024 * 1024;
+
+let array = new Int8Array(4 * giga);
+array[0] = 1;
+array[giga] = 2;
+array[2 * giga] = 3;
+array[3 * giga] = 4;
+
+function expect(base, index, expected, string)
+{
+    let result = base [index]
+    if (result != expected)
+        throw "Expected " + expected + " but got " + result + " while testing " + string;
+}
+
+let slice0 = array.slice(giga);
+expect(slice0, 0, 2, "slice0");
+expect(slice0, giga, 3, "slice0");
+expect(slice0, 2*giga, 4, "slice0");
+
+let subslice0 = slice0.slice(giga);
+expect(subslice0, 0, 3, "subslice0");
+expect(subslice0, giga, 4, "subslice0");
+
+let slice1 = array.slice(giga, 2 * giga);
+expect(slice1, 0, 2, "slice1");
+
+let slice2 = array.slice(3 * giga);
+expect(slice2, 0, 4, "slice2");
+
+let slice3 = array.slice(4 * giga);
+let subSlice3 = slice3.slice(0);
+

Added: trunk/JSTests/stress/typed-array-set-large-offset.js (0 => 285117)


--- trunk/JSTests/stress/typed-array-set-large-offset.js	                        (rev 0)
+++ trunk/JSTests/stress/typed-array-set-large-offset.js	2021-11-01 18:01:21 UTC (rev 285117)
@@ -0,0 +1,28 @@
+//@skip
+// This tests takes >4s even in release mode on an M1 MBP, so I'd rather avoid running it on EWS by default.
+
+let giga = 1024 * 1024 * 1024;
+let sourceLength = giga;
+let destinationLength = 4 * giga;
+let offset = 3 * giga;
+
+var source = new Uint8ClampedArray(sourceLength);
+for (var i = 0; i < 100; ++i)
+    source[i] = i & 0x3F;
+
+var destination = new Int8Array(destinationLength);
+destination.set(source, offset);
+
+// Before the offset
+let value = destination[42];
+if (value !== 0)
+    throw "At index " + 42 + ", expected 0 but got " + value;
+
+// After the offset
+for (var i = 0; i < 100; ++i) {
+    let index = offset + i;
+    let value = destination[index];
+    let expectedValue = (index - offset) & 0x3F;
+    if (value != expectedValue)
+        throw "At index " + index + ", expected " + expectedValue + " but got " + value;
+}

Added: trunk/JSTests/stress/typed-array-set-large.js (0 => 285117)


--- trunk/JSTests/stress/typed-array-set-large.js	                        (rev 0)
+++ trunk/JSTests/stress/typed-array-set-large.js	2021-11-01 18:01:21 UTC (rev 285117)
@@ -0,0 +1,41 @@
+//@skip
+// This tests takes >4s even in release mode on an M1 MBP, so I'd rather avoid running it on EWS by default.
+
+let giga = 1024 * 1024 * 1024;
+let sourceLength = 2 * giga;
+let destinationLength = 3 * giga;
+let offset = giga;
+
+var source = new Uint8ClampedArray(sourceLength);
+for (var i = 0; i < 100; ++i)
+    source[i] = i & 0x3F;
+for (var i = 0; i < 100; ++i) {
+    let index = giga + i;
+    source[index] = index & 0x3F
+}
+
+var destination = new Int8Array(destinationLength);
+destination.set(source, offset);
+
+// Before the offset
+let value = destination[42];
+if (value !== 0)
+    throw "At index " + 42 + ", expected 0 but got " + value;
+
+// After the offset but before INT32_MAX
+for (var i = 0; i < 100; ++i) {
+    let index = offset + i;
+    let value = destination[index];
+    let expectedValue = (index - offset) & 0x3F;
+    if (value != expectedValue)
+        throw "At index " + index + ", expected " + expectedValue + " but got " + value;
+}
+
+// After the offset and greater than INT32_MAX
+for (var i = 0; i < 100; ++i) {
+    let index = offset + giga + i;
+    let value = destination[index];
+    let expectedValue = (index - offset) & 0x3F;
+    if (value != expectedValue)
+        throw "At index " + index + ", expected " + expectedValue + " but got " + value;
+}

Modified: trunk/Source/_javascript_Core/ChangeLog (285116 => 285117)


--- trunk/Source/_javascript_Core/ChangeLog	2021-11-01 17:50:16 UTC (rev 285116)
+++ trunk/Source/_javascript_Core/ChangeLog	2021-11-01 18:01:21 UTC (rev 285117)
@@ -1,3 +1,47 @@
+2021-11-01  Robin Morisset  <rmoris...@apple.com>
+
+        JSGenericTypedArrayView<Adaptor>::set crashes if the length + objectOffset is > UINT32_MAX
+        https://bugs.webkit.org/show_bug.cgi?id=231975
+        rdar://84402043
+
+        Reviewed by Yusuke Suzuki.
+
+        UINT_MAX (and anything above it) is not a valid array index, so we cannot use JSObject::get(JSGlobalObject*, unsigned) with an index that big.
+        This was pointed by Yusuke in his review of my recent patch that introduced the problem (https://bugs.webkit.org/show_bug.cgi?id=229353#c21),
+        but I misunderstood the code and thought we could never get values that big at that point, thus only putting a RELEASE_ASSERT.
+        In this patch I instead apply his original suggestion to have a first loop using the (fast) JSObject::get(),
+        and a second loop for any large indices, using a slower but safe code path.
+
+        I also fixed an unrelated bug I noticed in Clobberize/AbstractInterpreter while testing the rest of the patch:
+        they were not aware that NewTypedArray can take a Int52RepUse child.
+
+        Finally, while trying to properly test this change, I discovered that genericTypedArrayViewProtoFuncSlice
+        (which is the only caller of JSGenericTypedArrayView<Adaptor>::set which passes it a non-0 objectOffset)
+        was still using unsigned everywhere instead of size_t, and that the same was true of all other functions in the same file.
+        So I fixed it in the same patch.
+
+        * dfg/DFGAbstractInterpreterInlines.h:
+        (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
+        * dfg/DFGClobberize.h:
+        (JSC::DFG::clobberize):
+        * runtime/JSArrayBufferConstructor.cpp:
+        (JSC::JSGenericArrayBufferConstructor<sharingMode>::constructImpl):
+        * runtime/JSGenericTypedArrayViewConstructorInlines.h:
+        (JSC::constructGenericTypedArrayViewImpl):
+        * runtime/JSGenericTypedArrayViewInlines.h:
+        (JSC::JSGenericTypedArrayView<Adaptor>::set):
+        * runtime/JSGenericTypedArrayViewPrototypeFunctions.h:
+        (JSC::argumentClampedIndexFromStartOrEnd):
+        (JSC::genericTypedArrayViewProtoFuncSet):
+        (JSC::genericTypedArrayViewProtoFuncCopyWithin):
+        (JSC::genericTypedArrayViewProtoFuncIncludes):
+        (JSC::genericTypedArrayViewProtoFuncIndexOf):
+        (JSC::genericTypedArrayViewProtoFuncJoin):
+        (JSC::genericTypedArrayViewProtoFuncFill):
+        (JSC::genericTypedArrayViewProtoFuncLastIndexOf):
+        (JSC::genericTypedArrayViewProtoFuncSlice):
+        (JSC::genericTypedArrayViewPrivateFuncSubarrayCreate):
+
 2021-10-31  Cameron McCormack  <hey...@apple.com>
 
         Update Web IDL links to new URL

Modified: trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h (285116 => 285117)


--- trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h	2021-11-01 17:50:16 UTC (rev 285116)
+++ trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h	2021-11-01 18:01:21 UTC (rev 285117)
@@ -2958,6 +2958,7 @@
     case NewTypedArray:
         switch (node->child1().useKind()) {
         case Int32Use:
+        case Int52RepUse:
             break;
         case UntypedUse:
             clobberWorld();

Modified: trunk/Source/_javascript_Core/dfg/DFGClobberize.h (285116 => 285117)


--- trunk/Source/_javascript_Core/dfg/DFGClobberize.h	2021-11-01 17:50:16 UTC (rev 285116)
+++ trunk/Source/_javascript_Core/dfg/DFGClobberize.h	2021-11-01 18:01:21 UTC (rev 285117)
@@ -1552,6 +1552,7 @@
     case NewTypedArray:
         switch (node->child1().useKind()) {
         case Int32Use:
+        case Int52RepUse:
             read(HeapObjectCount);
             write(HeapObjectCount);
             return;

Modified: trunk/Source/_javascript_Core/runtime/JSArrayBufferConstructor.cpp (285116 => 285117)


--- trunk/Source/_javascript_Core/runtime/JSArrayBufferConstructor.cpp	2021-11-01 17:50:16 UTC (rev 285116)
+++ trunk/Source/_javascript_Core/runtime/JSArrayBufferConstructor.cpp	2021-11-01 18:01:21 UTC (rev 285117)
@@ -81,9 +81,9 @@
     Structure* structure = JSC_GET_DERIVED_STRUCTURE(vm, arrayBufferStructureWithSharingMode<sharingMode>, newTarget, callFrame->jsCallee());
     RETURN_IF_EXCEPTION(scope, { });
 
-    unsigned length;
+    size_t length;
     if (callFrame->argumentCount()) {
-        length = callFrame->uncheckedArgument(0).toIndex(globalObject, "length");
+        length = callFrame->uncheckedArgument(0).toTypedArrayIndex(globalObject, "length");
         RETURN_IF_EXCEPTION(scope, encodedJSValue());
     } else {
         // Although the documentation doesn't say so, it is in fact correct to say

Modified: trunk/Source/_javascript_Core/runtime/JSGenericTypedArrayViewConstructorInlines.h (285116 => 285117)


--- trunk/Source/_javascript_Core/runtime/JSGenericTypedArrayViewConstructorInlines.h	2021-11-01 17:50:16 UTC (rev 285116)
+++ trunk/Source/_javascript_Core/runtime/JSGenericTypedArrayViewConstructorInlines.h	2021-11-01 18:01:21 UTC (rev 285117)
@@ -278,7 +278,7 @@
     size_t offset = 0;
     std::optional<size_t> length = std::nullopt;
     if (jsDynamicCast<JSArrayBuffer*>(vm, firstValue) && argCount > 1) {
-        offset = callFrame->uncheckedArgument(1).toIndex(globalObject, "byteOffset");
+        offset = callFrame->uncheckedArgument(1).toTypedArrayIndex(globalObject, "byteOffset");
         RETURN_IF_EXCEPTION(scope, encodedJSValue());
 
         if (argCount > 2) {

Modified: trunk/Source/_javascript_Core/runtime/JSGenericTypedArrayViewInlines.h (285116 => 285117)


--- trunk/Source/_javascript_Core/runtime/JSGenericTypedArrayViewInlines.h	2021-11-01 17:50:16 UTC (rev 285116)
+++ trunk/Source/_javascript_Core/runtime/JSGenericTypedArrayViewInlines.h	2021-11-01 18:01:21 UTC (rev 285117)
@@ -313,13 +313,8 @@
         if (!success)
             return false;
 
-        // Verify that we won't ever call object->get() with an index of UINT_MAX or more
-        RELEASE_ASSERT(isSumSmallerThanOrEqual(static_cast<uint64_t>(length), static_cast<uint64_t>(objectOffset), static_cast<uint64_t>(std::numeric_limits<unsigned>::max())));
-        // We could optimize this case. But right now, we don't.
-        for (size_t i = 0; i < length; ++i) {
-            JSValue value = object->get(globalObject, static_cast<unsigned>(i + objectOffset));
-            RETURN_IF_EXCEPTION(scope, false);
-            bool success = setIndex(globalObject, offset + i, value);
+        auto trySetIndex = [&](size_t index, JSValue value) -> bool {
+            bool success = setIndex(globalObject, index, value);
             EXCEPTION_ASSERT(!scope.exception() || !success);
             if (!success) {
                 if (isDetached())
@@ -326,7 +321,26 @@
                     throwTypeError(globalObject, scope, typedArrayBufferHasBeenDetachedErrorMessage);
                 return false;
             }
+            return true;
+        };
+        // It is not valid to ever call object->get() with an index of more than MAX_ARRAY_INDEX.
+        // So we iterate in the optimized loop up to MAX_ARRAY_INDEX, then if there is anything to do beyond this, we rely on slower code.
+        size_t safeUnadjustedLength = std::min(length, static_cast<size_t>(MAX_ARRAY_INDEX) + 1);
+        size_t safeLength = objectOffset <= safeUnadjustedLength ? safeUnadjustedLength - objectOffset : 0;
+        for (size_t i = 0; i < safeLength; ++i) {
+            ASSERT(i + objectOffset <= MAX_ARRAY_INDEX);
+            JSValue value = object->get(globalObject, static_cast<unsigned>(i + objectOffset));
+            RETURN_IF_EXCEPTION(scope, false);
+            if (!trySetIndex(offset + i, value))
+                return false;
         }
+        for (size_t i = safeLength; i < length; ++i) {
+            Identifier ident = Identifier::from(vm, static_cast<uint64_t>(i + objectOffset));
+            JSValue value = object->get(globalObject, ident);
+            RETURN_IF_EXCEPTION(scope, false);
+            if (!trySetIndex(offset + i, value))
+                return false;
+        }
         return true;
     } }
     

Modified: trunk/Source/_javascript_Core/runtime/JSGenericTypedArrayViewPrototypeFunctions.h (285116 => 285117)


--- trunk/Source/_javascript_Core/runtime/JSGenericTypedArrayViewPrototypeFunctions.h	2021-11-01 17:50:16 UTC (rev 285116)
+++ trunk/Source/_javascript_Core/runtime/JSGenericTypedArrayViewPrototypeFunctions.h	2021-11-01 18:01:21 UTC (rev 285117)
@@ -86,7 +86,7 @@
     return nullptr;
 }
 
-inline unsigned argumentClampedIndexFromStartOrEnd(JSGlobalObject* globalObject, JSValue value, unsigned length, unsigned undefinedValue = 0)
+inline size_t argumentClampedIndexFromStartOrEnd(JSGlobalObject* globalObject, JSValue value, size_t length, size_t undefinedValue = 0)
 {
     if (value.isUndefined())
         return undefinedValue;
@@ -94,9 +94,9 @@
     double indexDouble = value.toIntegerOrInfinity(globalObject);
     if (indexDouble < 0) {
         indexDouble += length;
-        return indexDouble < 0 ? 0 : static_cast<unsigned>(indexDouble);
+        return indexDouble < 0 ? 0 : static_cast<size_t>(indexDouble);
     }
-    return indexDouble > length ? length : static_cast<unsigned>(indexDouble);
+    return indexDouble > length ? length : static_cast<size_t>(indexDouble);
 }
 
 template<typename ViewClass>
@@ -110,13 +110,16 @@
     if (UNLIKELY(!callFrame->argumentCount()))
         return throwVMTypeError(globalObject, scope, "Expected at least one argument"_s);
 
-    unsigned offset;
+    size_t offset;
     if (callFrame->argumentCount() >= 2) {
         double offsetNumber = callFrame->uncheckedArgument(1).toIntegerOrInfinity(globalObject);
         RETURN_IF_EXCEPTION(scope, encodedJSValue());
         if (UNLIKELY(offsetNumber < 0))
             return throwVMRangeError(globalObject, scope, "Offset should not be negative");
-        offset = static_cast<unsigned>(std::min(offsetNumber, static_cast<double>(std::numeric_limits<unsigned>::max())));
+        if (offsetNumber <= maxSafeInteger() && offsetNumber <= static_cast<double>(std::numeric_limits<size_t>::max()))
+            offset = offsetNumber;
+        else
+            offset = std::numeric_limits<size_t>::max();
     } else
         offset = 0;
 
@@ -126,7 +129,7 @@
     JSObject* sourceArray = callFrame->uncheckedArgument(0).toObject(globalObject);
     RETURN_IF_EXCEPTION(scope, { });
 
-    unsigned length;
+    size_t length;
     if (isTypedView(sourceArray->classInfo(vm)->typedArrayStorageType)) {
         JSArrayBufferView* sourceView = jsCast<JSArrayBufferView*>(sourceArray);
         if (UNLIKELY(sourceView->isDetached()))
@@ -136,7 +139,7 @@
     } else {
         JSValue lengthValue = sourceArray->get(globalObject, vm.propertyNames->length);
         RETURN_IF_EXCEPTION(scope, encodedJSValue());
-        length = lengthValue.toUInt32(globalObject);
+        length = lengthValue.toLength(globalObject);
     }
 
     RETURN_IF_EXCEPTION(scope, encodedJSValue());
@@ -156,18 +159,20 @@
     if (thisObject->isDetached())
         return throwVMTypeError(globalObject, scope, typedArrayBufferHasBeenDetachedErrorMessage);
 
-    long length = thisObject->length();
-    long to = argumentClampedIndexFromStartOrEnd(globalObject, callFrame->argument(0), length);
+    size_t length = thisObject->length();
+    size_t to = argumentClampedIndexFromStartOrEnd(globalObject, callFrame->argument(0), length);
     RETURN_IF_EXCEPTION(scope, encodedJSValue());
-    long from = argumentClampedIndexFromStartOrEnd(globalObject, callFrame->argument(1), length);
+    size_t from = argumentClampedIndexFromStartOrEnd(globalObject, callFrame->argument(1), length);
     RETURN_IF_EXCEPTION(scope, encodedJSValue());
-    long final = argumentClampedIndexFromStartOrEnd(globalObject, callFrame->argument(2), length, length);
+    size_t final = argumentClampedIndexFromStartOrEnd(globalObject, callFrame->argument(2), length, length);
     RETURN_IF_EXCEPTION(scope, encodedJSValue());
 
     if (final < from)
         return JSValue::encode(callFrame->thisValue());
 
-    long count = std::min(length - std::max(to, from), final - from);
+    ASSERT(to <= length);
+    ASSERT(from <= length);
+    size_t count = std::min(length - std::max(to, from), final - from);
 
     if (thisObject->isDetached())
         return throwVMTypeError(globalObject, scope, typedArrayBufferHasBeenDetachedErrorMessage);
@@ -187,7 +192,7 @@
     if (thisObject->isDetached())
         return throwVMTypeError(globalObject, scope, typedArrayBufferHasBeenDetachedErrorMessage);
 
-    unsigned length = thisObject->length();
+    size_t length = thisObject->length();
 
     if (!length)
         return JSValue::encode(jsBoolean(false));
@@ -194,7 +199,7 @@
 
     JSValue valueToFind = callFrame->argument(0);
 
-    unsigned index = argumentClampedIndexFromStartOrEnd(globalObject, callFrame->argument(1), length);
+    size_t index = argumentClampedIndexFromStartOrEnd(globalObject, callFrame->argument(1), length);
     RETURN_IF_EXCEPTION(scope, encodedJSValue());
 
     if (thisObject->isDetached())
@@ -233,13 +238,13 @@
     if (thisObject->isDetached())
         return throwVMTypeError(globalObject, scope, typedArrayBufferHasBeenDetachedErrorMessage);
 
-    unsigned length = thisObject->length();
+    size_t length = thisObject->length();
 
     if (!length)
         return JSValue::encode(jsNumber(-1));
 
     JSValue valueToFind = callFrame->argument(0);
-    unsigned index = argumentClampedIndexFromStartOrEnd(globalObject, callFrame->argument(1), length);
+    size_t index = argumentClampedIndexFromStartOrEnd(globalObject, callFrame->argument(1), length);
     RETURN_IF_EXCEPTION(scope, encodedJSValue());
 
     if (thisObject->isDetached())
@@ -269,12 +274,12 @@
     if (thisObject->isDetached())
         return throwVMTypeError(globalObject, scope, typedArrayBufferHasBeenDetachedErrorMessage);
 
-    unsigned length = thisObject->length();
+    size_t length = thisObject->length();
     auto joinWithSeparator = [&] (StringView separator) -> EncodedJSValue {
         JSStringJoiner joiner(globalObject, separator, length);
         RETURN_IF_EXCEPTION(scope, { });
         if (!thisObject->isDetached()) {
-            for (unsigned i = 0; i < length; i++) {
+            for (size_t i = 0; i < length; i++) {
                 JSValue value;
                 if constexpr (ViewClass::Adaptor::canConvertToJSQuickly)
                     value = thisObject->getIndexQuickly(i);
@@ -287,7 +292,7 @@
                 RETURN_IF_EXCEPTION(scope, { });
             }
         } else {
-            for (unsigned i = 0; i < length; i++)
+            for (size_t i = 0; i < length; i++)
                 joiner.appendEmptyString();
         }
         RELEASE_AND_RETURN(scope, JSValue::encode(joiner.join(globalObject)));
@@ -317,19 +322,19 @@
     if (thisObject->isDetached())
         return throwVMTypeError(globalObject, scope, typedArrayBufferHasBeenDetachedErrorMessage);
 
-    unsigned length = thisObject->length();
+    size_t length = thisObject->length();
     auto nativeValue = ViewClass::toAdaptorNativeFromValue(globalObject, callFrame->argument(0));
     RETURN_IF_EXCEPTION(scope, { });
 
-    unsigned start = argumentClampedIndexFromStartOrEnd(globalObject, callFrame->argument(1), length, 0);
+    size_t start = argumentClampedIndexFromStartOrEnd(globalObject, callFrame->argument(1), length, 0);
     RETURN_IF_EXCEPTION(scope, { });
-    unsigned end = argumentClampedIndexFromStartOrEnd(globalObject, callFrame->argument(2), length, length);
+    size_t end = argumentClampedIndexFromStartOrEnd(globalObject, callFrame->argument(2), length, length);
     RETURN_IF_EXCEPTION(scope, { });
 
     if (thisObject->isDetached())
         return throwVMTypeError(globalObject, scope, typedArrayBufferHasBeenDetachedErrorMessage);
 
-    for (unsigned index = start; index < end; ++index)
+    for (size_t index = start; index < end; ++index)
         thisObject->setIndexQuicklyToNativeValue(index, nativeValue);
 
     return JSValue::encode(thisObject);
@@ -345,7 +350,7 @@
     if (thisObject->isDetached())
         return throwVMTypeError(globalObject, scope, typedArrayBufferHasBeenDetachedErrorMessage);
 
-    unsigned length = thisObject->length();
+    size_t length = thisObject->length();
 
     if (!length)
         return JSValue::encode(jsNumber(-1));
@@ -352,7 +357,7 @@
 
     JSValue valueToFind = callFrame->argument(0);
 
-    int index = length - 1;
+    size_t index = length - 1;
     if (callFrame->argumentCount() >= 2) {
         JSValue fromValue = callFrame->uncheckedArgument(1);
         double fromDouble = fromValue.toIntegerOrInfinity(globalObject);
@@ -363,7 +368,7 @@
                 return JSValue::encode(jsNumber(-1));
         }
         if (fromDouble < length)
-            index = static_cast<unsigned>(fromDouble);
+            index = static_cast<size_t>(fromDouble);
     }
 
     if (thisObject->isDetached())
@@ -377,10 +382,14 @@
     scope.assertNoExceptionExceptTermination();
     RELEASE_ASSERT(!thisObject->isDetached());
 
-    for (; index >= 0; --index) {
+    // We always have at least one iteration, since we checked that length is different from 0 earlier.
+    do {
         if (array[index] == targetOption)
             return JSValue::encode(jsNumber(index));
-    }
+        if (!index)
+            break;
+        --index;
+    } while (true);
 
     return JSValue::encode(jsNumber(-1));
 }
@@ -463,11 +472,11 @@
     if (thisObject->isDetached())
         return throwVMTypeError(globalObject, scope, typedArrayBufferHasBeenDetachedErrorMessage);
 
-    unsigned thisLength = thisObject->length();
+    size_t thisLength = thisObject->length();
 
-    unsigned begin = argumentClampedIndexFromStartOrEnd(globalObject, callFrame->argument(0), thisLength);
+    size_t begin = argumentClampedIndexFromStartOrEnd(globalObject, callFrame->argument(0), thisLength);
     RETURN_IF_EXCEPTION(scope, encodedJSValue());
-    unsigned end = argumentClampedIndexFromStartOrEnd(globalObject, callFrame->argument(1), thisLength, thisLength);
+    size_t end = argumentClampedIndexFromStartOrEnd(globalObject, callFrame->argument(1), thisLength, thisLength);
     RETURN_IF_EXCEPTION(scope, encodedJSValue());
 
     if (thisObject->isDetached())
@@ -477,7 +486,7 @@
     end = std::max(begin, end);
 
     ASSERT(end >= begin);
-    unsigned length = end - begin;
+    size_t length = end - begin;
 
     MarkedArgumentBuffer args;
     args.append(jsNumber(length));
@@ -569,7 +578,7 @@
         return throwVMTypeError(globalObject, scope, typedArrayBufferHasBeenDetachedErrorMessage);
 
     // Get the length here; later assert that the length didn't change.
-    unsigned thisLength = thisObject->length();
+    size_t thisLength = thisObject->length();
 
     // I would assert that the arguments are integers here but that's not true since
     // https://tc39.github.io/ecma262/#sec-tointeger allows the result of the operation
@@ -576,9 +585,9 @@
     // to be +/- Infinity and -0.
     ASSERT(callFrame->argument(0).isNumber());
     ASSERT(callFrame->argument(1).isUndefined() || callFrame->argument(1).isNumber());
-    unsigned begin = argumentClampedIndexFromStartOrEnd(globalObject, callFrame->argument(0), thisLength);
+    size_t begin = argumentClampedIndexFromStartOrEnd(globalObject, callFrame->argument(0), thisLength);
     scope.assertNoException();
-    unsigned end = argumentClampedIndexFromStartOrEnd(globalObject, callFrame->argument(1), thisLength, thisLength);
+    size_t end = argumentClampedIndexFromStartOrEnd(globalObject, callFrame->argument(1), thisLength, thisLength);
     scope.assertNoException();
 
     RELEASE_ASSERT(!thisObject->isDetached());
@@ -587,8 +596,8 @@
     end = std::max(begin, end);
 
     ASSERT(end >= begin);
-    unsigned offset = begin;
-    unsigned length = end - begin;
+    size_t offset = begin;
+    size_t length = end - begin;
 
     RefPtr<ArrayBuffer> arrayBuffer = thisObject->possiblySharedBuffer();
     if (UNLIKELY(!arrayBuffer)) {
@@ -597,7 +606,7 @@
     }
     RELEASE_ASSERT(thisLength == thisObject->length());
 
-    unsigned newByteOffset = thisObject->byteOffset() + offset * ViewClass::elementSize;
+    size_t newByteOffset = thisObject->byteOffset() + offset * ViewClass::elementSize;
 
     JSObject* defaultConstructor = globalObject->typedArrayConstructor(ViewClass::TypedArrayStorageType);
     JSValue species = callFrame->uncheckedArgument(2);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to