[objects] Add JSObject::kMaxElementIndex and JSArray::kMaxArrayLength

The available constants are now:

 JSObject {
  kMaxElementCount = kMaxUInt32,
  kMaxElementIndex = kMaxElementCount - 1,
 }

 JSArray {
  kMaxArrayLength = JSObject::kMaxElementCount,
  kMaxArrayIndex = JSObject::kMaxElementIndex,
 }

I also updated the codebase to use the new constants.

Change-Id: I3142f9ff9627c9acb1d4493729b490150fdcdf50
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2712755
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Auto-Submit: Jakob Gruber <jgruber@chromium.org>
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#73006}

src/builtins/array-join.tq

@@ -3,8 +3,12 @@
 // found in the LICENSE file.
 
 namespace array {
+
 type LoadJoinElementFn = builtin(Context, JSReceiver, uintptr) => JSAny;
 
+const kMaxArrayLength:
+    constexpr uint32 generates 'JSArray::kMaxArrayLength';
+
 // Fast C call to write a fixed array (see Buffer.fixedArray) to a single
 // string.
 extern macro
@@ -555,8 +559,9 @@ ArrayPrototypeJoin(
 
   // Only handle valid array lengths. Although the spec allows larger
   // values, this matches historical V8 behavior.
-  if (len > kMaxArrayIndex + 1)
+  if (len > kMaxArrayLength) {
     ThrowTypeError(MessageTemplate::kInvalidArrayLength);
+  }
 
   return CycleProtectedArrayJoin<JSArray>(
       false, o, len, separator, Undefined, Undefined);
@@ -576,8 +581,9 @@ transitioning javascript builtin ArrayPrototypeToLocaleString(
 
   // Only handle valid array lengths. Although the spec allows larger
   // values, this matches historical V8 behavior.
-  if (len > kMaxArrayIndex + 1)
+  if (len > kMaxArrayLength) {
     ThrowTypeError(MessageTemplate::kInvalidArrayLength);
+  }
 
   return CycleProtectedArrayJoin<JSArray>(true, o, len, ',', locales, options);
 }

src/builtins/base.tq

@@ -415,8 +415,6 @@ extern enum PropertyAttributes extends int31 {
   ...
 }
 
-const kMaxArrayIndex:
-    constexpr uint32 generates 'JSArray::kMaxArrayIndex';
 const kArrayBufferMaxByteLength:
     constexpr uintptr generates 'JSArrayBuffer::kMaxByteLength';
 const kTypedArrayMaxLength:

src/builtins/builtins-array.cc

@@ -333,7 +333,7 @@ V8_WARN_UNUSED_RESULT Object GenericArrayPush(Isolate* isolate,
     Handle<Object> element = args->at(i + 1);
 
     // b. Perform ? Set(O, ! ToString(len), E, true).
-    if (length <= static_cast<double>(JSArray::kMaxArrayIndex)) {
+    if (length <= JSObject::kMaxElementIndex) {
       RETURN_FAILURE_ON_EXCEPTION(
           isolate, Object::SetElement(isolate, receiver, length, element,
                                       ShouldThrow::kThrowOnError));
@@ -662,7 +662,7 @@ class ArrayConcatVisitor {
   V8_WARN_UNUSED_RESULT bool visit(uint32_t i, Handle<Object> elm) {
     uint32_t index = index_offset_ + i;
 
-    if (i >= JSObject::kMaxElementCount - index_offset_) {
+    if (i > JSArray::kMaxArrayIndex - index_offset_) {
       set_exceeds_array_limit(true);
       // Exception hasn't been thrown at this point. Return true to
       // break out, and caller will throw. !visit would imply that
@@ -707,8 +707,8 @@ class ArrayConcatVisitor {
   uint32_t index_offset() const { return index_offset_; }
 
   void increase_index_offset(uint32_t delta) {
-    if (JSObject::kMaxElementCount - index_offset_ < delta) {
-      index_offset_ = JSObject::kMaxElementCount;
+    if (JSArray::kMaxArrayLength - index_offset_ < delta) {
+      index_offset_ = JSArray::kMaxArrayLength;
     } else {
       index_offset_ += delta;
     }
@@ -813,7 +813,7 @@ class ArrayConcatVisitor {
   Isolate* isolate_;
   Handle<Object> storage_;  // Always a global handle.
   // Index after last seen index. Always less than or equal to
-  // JSObject::kMaxElementCount.
+  // JSArray::kMaxArrayLength.
   uint32_t index_offset_;
   uint32_t bit_field_;
 };
@@ -1249,14 +1249,14 @@ Object Slow_ArrayConcat(BuiltinArguments* args, Handle<Object> species,
       length_estimate = 1;
       element_estimate = 1;
     }
-    // Avoid overflows by capping at kMaxElementCount.
-    if (JSObject::kMaxElementCount - estimate_result_length < length_estimate) {
-      estimate_result_length = JSObject::kMaxElementCount;
+    // Avoid overflows by capping at kMaxArrayLength.
+    if (JSArray::kMaxArrayLength - estimate_result_length < length_estimate) {
+      estimate_result_length = JSArray::kMaxArrayLength;
     } else {
       estimate_result_length += length_estimate;
     }
-    if (JSObject::kMaxElementCount - estimate_nof < element_estimate) {
-      estimate_nof = JSObject::kMaxElementCount;
+    if (JSArray::kMaxArrayLength - estimate_nof < element_estimate) {
+      estimate_nof = JSArray::kMaxArrayLength;
     } else {
       estimate_nof += element_estimate;
     }

src/codegen/code-stub-assembler.cc

@@ -9284,7 +9284,7 @@ void CodeStubAssembler::TryLookupElement(
   {
     // Negative and too-large keys must be converted to property names.
     if (Is64()) {
-      GotoIf(UintPtrLessThan(IntPtrConstant(JSArray::kMaxArrayIndex),
+      GotoIf(UintPtrLessThan(IntPtrConstant(JSObject::kMaxElementIndex),
                              intptr_index),
              if_bailout);
     } else {
@@ -9323,7 +9323,7 @@ void CodeStubAssembler::TryLookupElement(
     // Positive OOB indices mean "not found", negative indices and indices
     // out of array index range must be converted to property names.
     if (Is64()) {
-      GotoIf(UintPtrLessThan(IntPtrConstant(JSArray::kMaxArrayIndex),
+      GotoIf(UintPtrLessThan(IntPtrConstant(JSObject::kMaxElementIndex),
                              intptr_index),
              if_bailout);
     } else {

src/compiler/js-native-context-specialization.cc

@@ -1951,8 +1951,10 @@ Reduction JSNativeContextSpecialization::ReduceElementLoadFromHeapConstant(
   // Check whether we're accessing a known element on the {receiver} and can
   // constant-fold the load.
   NumberMatcher mkey(key);
-  if (mkey.IsInteger() && mkey.IsInRange(0.0, kMaxUInt32 - 1.0)) {
-    uint32_t index = static_cast<uint32_t>(mkey.ResolvedValue());
+  if (mkey.IsInteger() &&
+      mkey.IsInRange(0.0, static_cast<double>(JSObject::kMaxElementIndex))) {
+    STATIC_ASSERT(JSObject::kMaxElementIndex <= kMaxUInt32);
+    const uint32_t index = static_cast<uint32_t>(mkey.ResolvedValue());
     base::Optional<ObjectRef> element;
 
     if (receiver_ref.IsJSObject()) {

src/ic/accessor-assembler.cc

@@ -392,7 +392,7 @@ void AccessorAssembler::HandleLoadICSmiHandlerCase(
       if (Is64()) {
         GotoIfNot(
             UintPtrLessThanOrEqual(var_intptr_index.value(),
-                                   IntPtrConstant(JSArray::kMaxArrayIndex)),
+                                   IntPtrConstant(JSObject::kMaxElementIndex)),
             miss);
       } else {
         GotoIf(IntPtrLessThan(var_intptr_index.value(), IntPtrConstant(0)),
@@ -2137,7 +2137,7 @@ void AccessorAssembler::EmitElementLoad(
     {
       Comment("dictionary elements");
       if (Is64()) {
-        GotoIf(UintPtrLessThan(IntPtrConstant(JSArray::kMaxArrayIndex),
+        GotoIf(UintPtrLessThan(IntPtrConstant(JSObject::kMaxElementIndex),
                                intptr_index),
                out_of_bounds);
       } else {
@@ -2328,11 +2328,11 @@ void AccessorAssembler::GenericElementLoad(
     // without ever checking the prototype chain.
     GotoIf(IsJSTypedArrayInstanceType(lookup_start_object_instance_type),
            &return_undefined);
-    // Positive OOB indices within JSArray index range are effectively the same
+    // Positive OOB indices within elements index range are effectively the same
     // as hole loads. Larger keys and negative keys are named loads.
     if (Is64()) {
       Branch(UintPtrLessThanOrEqual(index,
-                                    IntPtrConstant(JSArray::kMaxArrayIndex)),
+                                    IntPtrConstant(JSObject::kMaxElementIndex)),
              &if_element_hole, slow);
     } else {
       Branch(IntPtrLessThan(index, IntPtrConstant(0)), slow, &if_element_hole);

src/ic/ic.cc

@@ -1301,10 +1301,14 @@ bool IntPtrKeyToSize(intptr_t index, Handle<HeapObject> receiver, size_t* out) {
     return false;
   }
 #if V8_HOST_ARCH_64_BIT
-  // On 32-bit platforms, any intptr_t is less than kMaxArrayIndex.
-  if (index > JSArray::kMaxArrayIndex && !receiver->IsJSTypedArray()) {
+  if (index > JSObject::kMaxElementIndex && !receiver->IsJSTypedArray()) {
     return false;
   }
+#else
+  // On 32-bit platforms, any intptr_t is less than kMaxElementIndex.
+  STATIC_ASSERT(
+      static_cast<double>(std::numeric_limits<decltype(index)>::max()) <=
+      static_cast<double>(JSObject::kMaxElementIndex));
 #endif
   *out = static_cast<size_t>(index);
   return true;

src/ic/keyed-store-generic.cc

@@ -555,8 +555,8 @@ void KeyedStoreGenericAssembler::EmitGenericElementStore(
 
   // Out-of-capacity accesses (index >= capacity) jump here. Additionally,
   // an ElementsKind transition might be necessary.
-  // The index can also be negative or larger than kMaxArrayIndex at this point!
-  // Jump to the runtime in that case to convert it to a named property.
+  // The index can also be negative or larger than kMaxElementIndex at this
+  // point! Jump to the runtime in that case to convert it to a named property.
   BIND(&if_grow);
   {
     Comment("Grow backing store");

src/objects/js-array.h

@@ -126,8 +126,15 @@ class JSArray : public JSObject {
   // Max. number of elements being copied in Array builtins.
   static const int kMaxCopyElements = 100;
 
+  // Valid array indices range from +0 <= i < 2^32 - 1 (kMaxUInt32).
+  static constexpr uint32_t kMaxArrayLength = JSObject::kMaxElementCount;
+  static constexpr uint32_t kMaxArrayIndex = JSObject::kMaxElementIndex;
+  STATIC_ASSERT(kMaxArrayLength == kMaxUInt32);
+  STATIC_ASSERT(kMaxArrayIndex == kMaxUInt32 - 1);
+
   // This constant is somewhat arbitrary. Any large enough value would work.
-  static const uint32_t kMaxFastArrayLength = 32 * 1024 * 1024;
+  static constexpr uint32_t kMaxFastArrayLength = 32 * 1024 * 1024;
+  STATIC_ASSERT(kMaxFastArrayLength <= kMaxArrayLength);
 
   // Min. stack size for detecting an Array.prototype.join() call cycle.
   static const uint32_t kMinJoinStackSize = 2;
@@ -137,9 +144,6 @@ class JSArray : public JSObject {
        AllocationMemento::kSize) >>
       kDoubleSizeLog2;
 
-  // Valid array indices range from +0 <= i < 2^32 - 1 (kMaxUInt32).
-  static const uint32_t kMaxArrayIndex = kMaxUInt32 - 1;
-
   OBJECT_CONSTRUCTORS(JSArray, JSObject);
 };
 

src/objects/js-objects.h

@@ -745,7 +745,8 @@ class JSObject : public TorqueGeneratedJSObject<JSObject, JSReceiver> {
 
   // Maximal number of elements (numbered 0 .. kMaxElementCount - 1).
   // Also maximal value of JSArray's length property.
-  static const uint32_t kMaxElementCount = 0xffffffffu;
+  static constexpr uint32_t kMaxElementCount = kMaxUInt32;
+  static constexpr uint32_t kMaxElementIndex = kMaxElementCount - 1;
 
   // Constants for heuristics controlling conversion of fast elements
   // to slow elements.

src/objects/lookup-inl.h

@@ -73,7 +73,7 @@ LookupIterator::LookupIterator(Isolate* isolate, Handle<Object> receiver,
   if (IsElement()) {
     // If we're not looking at a TypedArray, we will need the key represented
     // as an internalized string.
-    if (index_ > JSArray::kMaxArrayIndex &&
+    if (index_ > JSObject::kMaxElementIndex &&
         !lookup_start_object->IsJSTypedArray()) {
       if (name_.is_null()) {
         name_ = isolate->factory()->SizeToString(index_);
@@ -106,7 +106,9 @@ LookupIterator::LookupIterator(Isolate* isolate, Handle<Object> receiver,
 LookupIterator::Key::Key(Isolate* isolate, double index) {
   DCHECK_EQ(index, static_cast<uint64_t>(index));
 #if V8_TARGET_ARCH_32_BIT
-  if (index <= JSArray::kMaxArrayIndex) {
+  if (index <= JSObject::kMaxElementIndex) {
+    STATIC_ASSERT(JSObject::kMaxElementIndex <=
+                  std::numeric_limits<size_t>::max());
     index_ = static_cast<size_t>(index);
   } else {
     index_ = LookupIterator::kInvalidIndex;
@@ -165,7 +167,7 @@ Handle<Name> LookupIterator::GetName() {
 }
 
 bool LookupIterator::IsElement(JSReceiver object) const {
-  return index_ <= JSArray::kMaxArrayIndex ||
+  return index_ <= JSObject::kMaxElementIndex ||
          (index_ != kInvalidIndex && object.map().has_typed_array_elements());
 }
 
@@ -270,7 +272,7 @@ Handle<T> LookupIterator::GetStoreTarget() const {
 
 template <bool is_element>
 InterceptorInfo LookupIterator::GetInterceptor(JSObject holder) const {
-  if (is_element && index_ <= JSArray::kMaxArrayIndex) {
+  if (is_element && index_ <= JSObject::kMaxElementIndex) {
     return holder.GetIndexedInterceptor(isolate_);
   } else {
     return holder.GetNamedInterceptor(isolate_);

src/objects/lookup.cc

@@ -1101,7 +1101,7 @@ namespace {
 template <bool is_element>
 bool HasInterceptor(Map map, size_t index) {
   if (is_element) {
-    if (index > JSArray::kMaxArrayIndex) {
+    if (index > JSObject::kMaxElementIndex) {
       // There is currently no way to install interceptors on an object with
       // typed array elements.
       DCHECK(!map.has_typed_array_elements());

src/runtime/runtime-object.cc

@@ -324,7 +324,7 @@ RUNTIME_FUNCTION(Runtime_ObjectHasOwnProperty) {
 
     Map map = js_obj->map();
     if (!map.IsJSGlobalProxyMap() &&
-        (key.is_element() && key.index() <= JSArray::kMaxArrayIndex
+        (key.is_element() && key.index() <= JSObject::kMaxElementIndex
              ? !map.has_indexed_interceptor()
              : !map.has_named_interceptor())) {
       return ReadOnlyRoots(isolate).false_value();