[builtins] Don't use ToSmiLength in TypedArray constructors

... and reimplement TryNumberToUintPtr.

Bug: v8:4153
Change-Id: I3b683b6a41ebf49229aee4ceea4910e94d35ccca
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1876817
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#64566}

src/builtins/base.tq

@@ -1849,8 +1849,6 @@ extern transitioning macro ToInteger_Inline(
     Context, JSAny, constexpr ToIntegerTruncationMode): Number;
 extern transitioning macro ToLength_Inline(Context, JSAny): Number;
 extern transitioning macro ToNumber_Inline(Context, JSAny): Number;
-extern transitioning macro ToSmiLength(implicit context: Context)(JSAny):
-    PositiveSmi labels IfRangeError;
 extern transitioning macro ToString_Inline(Context, JSAny): String;
 extern transitioning macro ToThisString(implicit context: Context)(
     JSAny, String): String;
@@ -2187,6 +2185,11 @@ extern operator '!' macro IsFalse(Boolean): bool;
 extern operator '.instanceType' macro LoadInstanceType(HeapObject):
     InstanceType;
 
+operator '.length_uintptr' macro LoadJSArrayLengthAsUintPtr(array: JSArray):
+    uintptr {
+  return Convert<uintptr>(array.length);
+}
+
 extern operator '.length_intptr' macro LoadStringLengthAsWord(String): intptr;
 operator '.length_uintptr' macro LoadStringLengthAsUintPtr(s: String): uintptr {
   return Unsigned(s.length_intptr);
@@ -2797,8 +2800,6 @@ extern macro TruncateFloat64ToFloat32(float64): float32;
 extern macro TruncateHeapNumberValueToWord32(HeapNumber): int32;
 extern macro LoadJSArrayElementsMap(constexpr ElementsKind, NativeContext): Map;
 extern macro LoadJSArrayElementsMap(ElementsKind, NativeContext): Map;
-extern macro ChangeNonnegativeNumberToUintPtr(Number): uintptr;
-extern macro TryNumberToUintPtr(Number): uintptr labels IfNegative;
 extern macro NumberConstant(constexpr float64): Number;
 extern macro NumberConstant(constexpr int32): Number;
 extern macro NumberConstant(constexpr uint32): Number;
@@ -3012,7 +3013,7 @@ Convert<float64, Number>(n: Number): float64 {
   return ChangeNumberToFloat64(n);
 }
 Convert<uintptr, Number>(n: Number): uintptr {
-  return ChangeNonnegativeNumberToUintPtr(n);
+  return ChangeUintPtrNumberToUintPtr(n);
 }
 Convert<float64, float32>(f: float32): float64 {
   return ChangeFloat32ToFloat64(f);
@@ -3606,48 +3607,103 @@ macro SameValue(a: JSAny, b: JSAny): bool {
   BranchIfSameValue(a, b) otherwise return true, return false;
 }
 
-// https://tc39.github.io/ecma262/#sec-toindex
-@export
-transitioning macro ToIndex(implicit context: Context)(value: JSAny):
-    uintptr labels IfRangeError {
-  if (value == Undefined) return 0;
-  const indexNumber = ToInteger_Inline(context, value, kTruncateMinusZero);
-  return ToIndex(indexNumber) otherwise IfRangeError;
-}
-
-// https://tc39.github.io/ecma262/#sec-toindex
-// Same as the version above but for Number arguments.
-macro ToIndex(indexNumber: Number): uintptr labels IfRangeError {
-  typeswitch (indexNumber) {
-    case (indexSmi: Smi): {
-      if (indexSmi < 0) goto IfRangeError;
-      const index: uintptr = Unsigned(Convert<intptr>(indexSmi));
+// TODO(tebbi): Define enum here once they appear in Torque.
+//
+// The value is a SafeInteger that fits into uintptr range, so no bounds checks
+// are necessary.
+const kModeValueIsSafeIntegerUintPtr: constexpr int31 = 0;
+// The value is a SafeInteger that may not fit into uintptr range, so only
+// uintptr bounds check is necessary.
+const kModeValueIsSafeInteger: constexpr int31 = 1;
+// The value is can be whatever non-NaN number, all checks are necessary.
+const kModeValueIsAnyNumber: constexpr int31 = 2;
+
+macro TryNumberToUintPtr(valueNumber: Number, kMode: constexpr int31):
+    uintptr labels IfLessThanZero, IfUIntPtrOverflow, IfSafeIntegerOverflow {
+  typeswitch (valueNumber) {
+    case (valueSmi: Smi): {
+      if (kMode == kModeValueIsAnyNumber) {
+        if (valueSmi < 0) goto IfLessThanZero;
+      } else {
+        assert(valueSmi >= 0);
+      }
+      const value: uintptr = Unsigned(Convert<intptr>(valueSmi));
       // Positive Smi values definitely fit into both [0, kMaxSafeInteger] and
       // [0, kMaxUintPtr] ranges.
-      return index;
+      return value;
     }
-    case (indexHeapNumber: HeapNumber): {
-      assert(IsNumberNormalized(indexHeapNumber));
-      const indexDouble: float64 = Convert<float64>(indexHeapNumber);
-      // NaNs must already be handled by ToIndex() version above accepting
-      // JSAny indices.
-      assert(!Float64IsNaN(indexDouble));
-      if (indexDouble < 0) goto IfRangeError;
+    case (valueHeapNumber: HeapNumber): {
+      assert(IsNumberNormalized(valueHeapNumber));
+      const valueDouble: float64 = Convert<float64>(valueHeapNumber);
+      // NaNs must be handled outside.
+      assert(!Float64IsNaN(valueDouble));
+      if (kMode == kModeValueIsAnyNumber) {
+        if (valueDouble < 0) goto IfLessThanZero;
+      } else {
+        assert(valueDouble >= 0);
+      }
 
       if constexpr (Is64()) {
-        if (indexDouble > kMaxSafeInteger) goto IfRangeError;
+        // On 64-bit architectures uintptr range is bigger than safe integer
+        // range.
+        if (kMode == kModeValueIsAnyNumber) {
+          if (valueDouble > kMaxSafeInteger) goto IfSafeIntegerOverflow;
+        } else {
+          assert(valueDouble <= kMaxSafeInteger);
+        }
       } else {
-        // On 32-bit architectures not all safe integers fit into uintptr but
-        // callers handle this case the same way as the safe integer range
-        // overflow case so we don't need special handling for the values in
-        // (kMaxUInt32, kMaxSafeInteger] range.
-        if (indexDouble > kMaxUInt32Double) goto IfRangeError;
+        // On 32-bit architectures uintptr range is smaller than safe integer
+        // range.
+        if (kMode == kModeValueIsAnyNumber ||
+            kMode == kModeValueIsSafeInteger) {
+          if (valueDouble > kMaxUInt32Double) goto IfUIntPtrOverflow;
+        } else {
+          assert(valueDouble <= kMaxUInt32Double);
+        }
       }
-      return ChangeFloat64ToUintPtr(indexDouble);
+      return ChangeFloat64ToUintPtr(valueDouble);
     }
   }
 }
 
+@export
+macro ChangeUintPtrNumberToUintPtr(value: Number): uintptr {
+  try {
+    return TryNumberToUintPtr(value, kModeValueIsSafeIntegerUintPtr)
+        otherwise InvalidValue, InvalidValue, InvalidValue;
+  }
+  label InvalidValue {
+    unreachable;
+  }
+}
+
+macro ChangeSafeIntegerNumberToUintPtr(value: Number):
+    uintptr labels IfUIntPtrOverflow {
+  try {
+    return TryNumberToUintPtr(value, kModeValueIsSafeInteger)
+        otherwise InvalidValue, IfUIntPtrOverflow, InvalidValue;
+  }
+  label InvalidValue {
+    unreachable;
+  }
+}
+
+// https://tc39.github.io/ecma262/#sec-toindex
+// Unlike ToIndex from the spec this implementation triggers IfRangeError if
+// the result is bigger than min(kMaxUIntPtr, kMaxSafeInteger).
+// We can do this because all callers do a range checks against uintptr length
+// anyway and throw a RangeError in case of out-of-bounds index.
+@export
+transitioning macro ToIndex(implicit context: Context)(value: JSAny):
+    uintptr labels IfRangeError {
+  if (value == Undefined) return 0;
+  const indexNumber = ToInteger_Inline(context, value, kTruncateMinusZero);
+  // Less than 0 case, uintptr range overflow and safe integer range overflow
+  // imply IfRangeError.
+  return TryNumberToUintPtr(indexNumber, kModeValueIsAnyNumber)
+      otherwise IfRangeError, IfRangeError, IfRangeError;
+}
+
 transitioning macro GetLengthProperty(implicit context: Context)(o: JSAny):
     Number {
   try {

src/builtins/typed-array-createtypedarray.tq

@@ -72,9 +72,9 @@ namespace typed_array {
   transitioning macro TypedArrayInitialize(implicit context: Context)(
       initialize: constexpr bool, map: Map, length: uintptr,
       elementsInfo: typed_array::TypedArrayElementsInfo,
-      bufferConstructor: JSReceiver): JSTypedArray {
+      bufferConstructor: JSReceiver): JSTypedArray labels IfRangeError {
     const byteLength = elementsInfo.CalculateByteLength(length)
-        otherwise ThrowRangeError(kInvalidArrayBufferLength);
+        otherwise IfRangeError;
     const byteLengthNum = Convert<Number>(byteLength);
     const defaultConstructor = GetArrayBufferFunction();
     const byteOffset: uintptr = 0;
@@ -119,81 +119,85 @@ namespace typed_array {
   // 22.2.4.2 TypedArray ( length )
   // ES #sec-typedarray-length
   transitioning macro ConstructByLength(implicit context: Context)(
-      map: Map, length: JSAny,
+      map: Map, lengthObj: JSAny,
       elementsInfo: typed_array::TypedArrayElementsInfo): JSTypedArray {
-    const convertedLength: Number =
-        ToInteger_Inline(context, length, kTruncateMinusZero);
-    // The maximum length of a TypedArray is MaxSmi().
-    // Note: this is not per spec, but rather a constraint of our current
-    // representation (which uses Smis).
-    // TODO(7881): support larger-than-smi typed array lengths
-    const positiveLength: PositiveSmi = Cast<PositiveSmi>(convertedLength)
-        otherwise ThrowRangeError(kInvalidTypedArrayLength, length);
-    const defaultConstructor: Constructor = GetArrayBufferFunction();
-    const initialize: constexpr bool = true;
-    return TypedArrayInitialize(
-        initialize, map, Convert<uintptr>(positiveLength), elementsInfo,
-        defaultConstructor);
+    try {
+      const length: uintptr = ToIndex(lengthObj) otherwise RangeError;
+      const defaultConstructor: Constructor = GetArrayBufferFunction();
+      const initialize: constexpr bool = true;
+      return TypedArrayInitialize(
+          initialize, map, length, elementsInfo, defaultConstructor)
+          otherwise RangeError;
+    }
+    label RangeError deferred {
+      ThrowRangeError(kInvalidTypedArrayLength, lengthObj);
+    }
   }
 
   // 22.2.4.4 TypedArray ( object )
   // ES #sec-typedarray-object
   transitioning macro ConstructByArrayLike(implicit context: Context)(
-      map: Map, arrayLike: HeapObject, initialLength: JSAny,
+      map: Map, arrayLike: HeapObject, length: uintptr,
       elementsInfo: typed_array::TypedArrayElementsInfo,
       bufferConstructor: JSReceiver): JSTypedArray {
-    // The caller has looked up length on arrayLike, which is observable.
-    const length: PositiveSmi = ToSmiLength(initialLength)
-        otherwise ThrowRangeError(kInvalidTypedArrayLength, initialLength);
-    const initialize: constexpr bool = false;
-    const typedArray = TypedArrayInitialize(
-        initialize, map, Convert<uintptr>(length), elementsInfo,
-        bufferConstructor);
-
     try {
-      const src: JSTypedArray = Cast<JSTypedArray>(arrayLike) otherwise IfSlow;
-
-      if (IsDetachedBuffer(src.buffer)) {
-        ThrowTypeError(kDetachedOperation, 'Construct');
-
-      } else if (src.elements_kind != elementsInfo.kind) {
-        goto IfSlow;
-
-      } else if (length > 0) {
-        const byteLength = typedArray.byte_length;
-        assert(byteLength <= kArrayBufferMaxByteLength);
-        typed_array::CallCMemcpy(typedArray.data_ptr, src.data_ptr, byteLength);
+      const initialize: constexpr bool = false;
+      const typedArray = TypedArrayInitialize(
+          initialize, map, length, elementsInfo, bufferConstructor)
+          otherwise RangeError;
+
+      try {
+        const src: JSTypedArray =
+            Cast<JSTypedArray>(arrayLike) otherwise IfSlow;
+
+        if (IsDetachedBuffer(src.buffer)) {
+          ThrowTypeError(kDetachedOperation, 'Construct');
+
+        } else if (src.elements_kind != elementsInfo.kind) {
+          goto IfSlow;
+
+        } else if (length > 0) {
+          const byteLength = typedArray.byte_length;
+          assert(byteLength <= kArrayBufferMaxByteLength);
+          typed_array::CallCMemcpy(
+              typedArray.data_ptr, src.data_ptr, byteLength);
+        }
       }
-    }
-    label IfSlow deferred {
-      if (length > 0) {
-        TypedArrayCopyElements(context, typedArray, arrayLike, length);
+      label IfSlow deferred {
+        if (length > 0) {
+          TypedArrayCopyElements(
+              context, typedArray, arrayLike, Convert<Number>(length));
+        }
       }
+      return typedArray;
+    }
+    label RangeError deferred {
+      ThrowRangeError(kInvalidTypedArrayLength, Convert<Number>(length));
     }
-    return typedArray;
   }
 
   // 22.2.4.4 TypedArray ( object )
   // ES #sec-typedarray-object
   transitioning macro ConstructByIterable(implicit context: Context)(
       iterable: JSReceiver, iteratorFn: Callable): never
-      labels IfConstructByArrayLike(JSArray, Number, JSReceiver) {
+      labels IfConstructByArrayLike(JSArray, uintptr, JSReceiver) {
     const array: JSArray =
         IterableToListMayPreserveHoles(context, iterable, iteratorFn);
-    goto IfConstructByArrayLike(array, array.length, GetArrayBufferFunction());
+    // Max JSArray length is a valid JSTypedArray length so we just use it.
+    goto IfConstructByArrayLike(
+        array, array.length_uintptr, GetArrayBufferFunction());
   }
 
   // 22.2.4.3 TypedArray ( typedArray )
   // ES #sec-typedarray-typedarray
   transitioning macro ConstructByTypedArray(implicit context: Context)(
       srcTypedArray: JSTypedArray): never
-      labels IfConstructByArrayLike(JSTypedArray, Number, JSReceiver) {
+      labels IfConstructByArrayLike(JSTypedArray, uintptr, JSReceiver) {
     let bufferConstructor: JSReceiver = GetArrayBufferFunction();
     const srcBuffer: JSArrayBuffer = srcTypedArray.buffer;
     // TODO(petermarshall): Throw on detached typedArray.
-    // TODO(v8:4156): Update this to support huge TypedArrays.
-    let length =
-        IsDetachedBuffer(srcBuffer) ? 0 : Convert<Number>(srcTypedArray.length);
+    let length: uintptr =
+        IsDetachedBuffer(srcBuffer) ? 0 : srcTypedArray.length;
 
     // The spec requires that constructing a typed array using a SAB-backed
     // typed array use the ArrayBuffer constructor, not the species constructor.
@@ -255,7 +259,7 @@ namespace typed_array {
       } else {
         // a. Let newByteLength be newLength × elementSize.
         newByteLength = elementsInfo.CalculateByteLength(newLength)
-            otherwise IfInvalidByteLength;
+            otherwise IfInvalidLength;
 
         // b. If offset + newByteLength > bufferByteLength, throw a RangeError
         // exception.
@@ -271,9 +275,6 @@ namespace typed_array {
     label IfInvalidAlignment(problemString: String) deferred {
       ThrowInvalidTypedArrayAlignment(map, problemString);
     }
-    label IfInvalidByteLength deferred {
-      ThrowRangeError(kInvalidArrayBufferLength);
-    }
     label IfInvalidLength deferred {
       ThrowRangeError(kInvalidTypedArrayLength, length);
     }
@@ -284,7 +285,7 @@ namespace typed_array {
 
   transitioning macro ConstructByJSReceiver(implicit context:
                                                 Context)(obj: JSReceiver): never
-      labels IfConstructByArrayLike(JSReceiver, Number, JSReceiver) {
+      labels IfConstructByArrayLike(JSReceiver, uintptr, JSReceiver) {
     try {
       const iteratorMethod: Object =
           GetIteratorMethod(obj) otherwise IfIteratorUndefined;
@@ -295,11 +296,15 @@ namespace typed_array {
     }
     label IfIteratorUndefined {
       const lengthObj: JSAny = GetProperty(obj, kLengthString);
-      const length: Smi = ToSmiLength(lengthObj)
-          otherwise goto IfInvalidLength(lengthObj);
+      const lengthNumber: Number = ToLength_Inline(context, lengthObj);
+      // Throw RangeError here if the length does not fit in uintptr because
+      // such a length will not pass bounds checks in ConstructByArrayLike()
+      // anyway.
+      const length: uintptr = ChangeSafeIntegerNumberToUintPtr(lengthNumber)
+          otherwise goto IfInvalidLength(lengthNumber);
       goto IfConstructByArrayLike(obj, length, GetArrayBufferFunction());
     }
-    label IfInvalidLength(length: Object) {
+    label IfInvalidLength(length: Number) {
       ThrowRangeError(kInvalidTypedArrayLength, length);
     }
   }
@@ -344,7 +349,7 @@ namespace typed_array {
       return ConstructByLength(map, length, elementsInfo);
     }
     label IfConstructByArrayLike(
-        arrayLike: JSReceiver, length: Number, bufferConstructor: JSReceiver) {
+        arrayLike: JSReceiver, length: uintptr, bufferConstructor: JSReceiver) {
       return ConstructByArrayLike(
           map, arrayLike, length, elementsInfo, bufferConstructor);
     }

src/codegen/code-stub-assembler.cc

@@ -1647,11 +1647,6 @@ TNode<HeapObject> CodeStubAssembler::LoadSlowProperties(
       [=] { return CAST(properties); });
 }
 
-TNode<Number> CodeStubAssembler::LoadJSArrayLength(SloppyTNode<JSArray> array) {
-  CSA_ASSERT(this, IsJSArray(array));
-  return CAST(LoadObjectField(array, JSArray::kLengthOffset));
-}
-
 TNode<Object> CodeStubAssembler::LoadJSArgumentsObjectWithLength(
     SloppyTNode<JSArgumentsObjectWithLength> array) {
   return LoadObjectField(array, JSArgumentsObjectWithLength::kLengthOffset);
@@ -5723,37 +5718,6 @@ TNode<Float64T> CodeStubAssembler::ChangeNumberToFloat64(TNode<Number> value) {
   return result.value();
 }
 
-TNode<UintPtrT> CodeStubAssembler::TryNumberToUintPtr(TNode<Number> value,
-                                                      Label* if_negative) {
-  TVARIABLE(UintPtrT, result);
-  Label done(this, &result);
-  Branch(
-      TaggedIsSmi(value),
-      [&] {
-        TNode<Smi> value_smi = CAST(value);
-        if (if_negative == nullptr) {
-          CSA_SLOW_ASSERT(this,
-                          SmiGreaterThanOrEqual(value_smi, SmiConstant(0)));
-        } else {
-          GotoIfNot(TaggedIsPositiveSmi(value), if_negative);
-        }
-        result = UncheckedCast<UintPtrT>(SmiToIntPtr(value_smi));
-        Goto(&done);
-      },
-      [&] {
-        TNode<HeapNumber> value_hn = CAST(value);
-        TNode<Float64T> value = LoadHeapNumberValue(value_hn);
-        if (if_negative != nullptr) {
-          GotoIf(Float64LessThan(value, Float64Constant(0.0)), if_negative);
-        }
-        result = ChangeFloat64ToUintPtr(value);
-        Goto(&done);
-      });
-
-  BIND(&done);
-  return result.value();
-}
-
 TNode<WordT> CodeStubAssembler::TimesSystemPointerSize(
     SloppyTNode<WordT> value) {
   return WordShl(value, kSystemPointerSizeLog2);
@@ -9105,7 +9069,8 @@ TNode<Object> CodeStubAssembler::CallGetterIfAccessor(
       GotoIfNot(IsLengthString(
                     LoadObjectField(accessor_info, AccessorInfo::kNameOffset)),
                 if_bailout);
-      var_value.Bind(LoadJSArrayLength(receiver));
+      TNode<JSArray> array = CAST(receiver);
+      var_value.Bind(LoadJSArrayLength(array));
       Goto(&done);
     }
 
@@ -10355,7 +10320,7 @@ void CodeStubAssembler::EmitElementStore(Node* object, Node* key, Node* value,
          IsNonextensibleElementsKind(elements_kind));
 
   Node* length = SelectImpl(
-      IsJSArray(object), [=]() { return LoadJSArrayLength(object); },
+      IsJSArray(object), [=]() { return LoadJSArrayLength(CAST(object)); },
       [=]() { return LoadFixedArrayBaseLength(elements); },
       MachineRepresentation::kTagged);
   length = TaggedToParameter(length, parameter_mode);
@@ -13353,7 +13318,7 @@ void CodeStubAssembler::CheckPrototypeEnumCache(Node* receiver,
 
     // It might still be an empty JSArray.
     GotoIfNot(IsJSArrayMap(object_map), if_slow);
-    TNode<Number> object_length = LoadJSArrayLength(object);
+    TNode<Number> object_length = LoadJSArrayLength(CAST(object));
     Branch(TaggedEqual(object_length, SmiConstant(0)), &if_no_elements,
            if_slow);
 

src/codegen/code-stub-assembler.h

@@ -1127,8 +1127,6 @@ class V8_EXPORT_PRIVATE CodeStubAssembler
   // Load the length of a JSArray instance.
   TNode<Object> LoadJSArgumentsObjectWithLength(
       SloppyTNode<JSArgumentsObjectWithLength> array);
-  // Load the length of a JSArray instance.
-  TNode<Number> LoadJSArrayLength(SloppyTNode<JSArray> array);
   // Load the length of a fast JSArray instance. Returns a positive Smi.
   TNode<Smi> LoadFastJSArrayLength(SloppyTNode<JSArray> array);
   // Load the length of a fixed array base instance.
@@ -2320,10 +2318,6 @@ class V8_EXPORT_PRIVATE CodeStubAssembler
   TNode<Number> ChangeUintPtrToTagged(TNode<UintPtrT> value);
   TNode<Uint32T> ChangeNumberToUint32(TNode<Number> value);
   TNode<Float64T> ChangeNumberToFloat64(TNode<Number> value);
-  TNode<UintPtrT> TryNumberToUintPtr(TNode<Number> value, Label* if_negative);
-  TNode<UintPtrT> ChangeNonnegativeNumberToUintPtr(TNode<Number> value) {
-    return TryNumberToUintPtr(value, nullptr);
-  }
 
   void TaggedToNumeric(Node* context, Node* value, Label* done,
                        Variable* var_numeric);

test/cctest/test-code-stub-assembler.cc

@@ -338,9 +338,9 @@ TEST(ConvertToRelativeIndex) {
     TNode<Number> length_number = m.CAST(m.Parameter(1));
     TNode<Number> expected_relative_index = m.CAST(m.Parameter(2));
 
-    TNode<UintPtrT> length = m.ChangeNonnegativeNumberToUintPtr(length_number);
+    TNode<UintPtrT> length = m.ChangeUintPtrNumberToUintPtr(length_number);
     TNode<UintPtrT> expected =
-        m.ChangeNonnegativeNumberToUintPtr(expected_relative_index);
+        m.ChangeUintPtrNumberToUintPtr(expected_relative_index);
 
     TNode<UintPtrT> result = m.ConvertToRelativeIndex(index, length);