Revert "[runtime] Add length check in ConvertElementsWithCapacity"

This reverts commit b271648e.

Reason for revert: New test fails: https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Linux%20-%20shared/42282/overview

Original change's description:
> [runtime] Add length check in ConvertElementsWithCapacity
>
> This also propagates the exception through all the users of
> ConvertElementsWithCapacity.
>
> Bug: chromium:1201626
> Change-Id: Ie44ba4327a4c3a20f1376477f45d3cd95d0da3b3
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2857961
> Commit-Queue: Victor Gomes <victorgomes@chromium.org>
> Reviewed-by: Toon Verwaest <verwaest@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#74412}

Bug: chromium:1201626
Change-Id: I764256e9d0dcc69ea3a2f3c77afaca73a910bb66
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2876861
Auto-Submit: Clemens Backes <clemensb@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#74414}

src/builtins/accessors.cc

@@ -203,12 +203,7 @@ void Accessors::ArrayLengthSetter(
     return;
   }
 
-  if (JSArray::SetLength(array, length).IsNothing()) {
-    // TODO(victorgomes): AccessorNameBooleanSetterCallback does not handle
-    // exceptions.
-    FATAL("Fatal JavaScript invalid array length %u", length);
-    UNREACHABLE();
-  }
+  JSArray::SetLength(array, length);
 
   uint32_t actual_new_len = 0;
   CHECK(array->length().ToArrayLength(&actual_new_len));

src/builtins/builtins-array.cc

@@ -173,8 +173,7 @@ V8_WARN_UNUSED_RESULT MaybeHandle<Object> SetLengthProperty(
     Handle<JSArray> array = Handle<JSArray>::cast(receiver);
     if (!JSArray::HasReadOnlyLength(array)) {
       DCHECK_LE(length, kMaxUInt32);
-      MAYBE_RETURN_NULL(
-          JSArray::SetLength(array, static_cast<uint32_t>(length)));
+      JSArray::SetLength(array, static_cast<uint32_t>(length));
       return receiver;
     }
   }
@@ -386,9 +385,7 @@ BUILTIN(ArrayPush) {
   }
 
   ElementsAccessor* accessor = array->GetElementsAccessor();
-  uint32_t new_length;
-  MAYBE_ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
-      isolate, new_length, accessor->Push(array, &args, to_add));
+  uint32_t new_length = accessor->Push(array, &args, to_add);
   return *isolate->factory()->NewNumberFromUint((new_length));
 }
 
@@ -471,8 +468,7 @@ BUILTIN(ArrayPop) {
   Handle<Object> result;
   if (IsJSArrayFastElementMovingAllowed(isolate, JSArray::cast(*receiver))) {
     // Fast Elements Path
-    ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
-        isolate, result, array->GetElementsAccessor()->Pop(array));
+    result = array->GetElementsAccessor()->Pop(array);
   } else {
     // Use Slow Lookup otherwise
     uint32_t new_length = len - 1;
@@ -487,9 +483,7 @@ BUILTIN(ArrayPop) {
                                 isolate->factory()->length_string(),
                                 Object::TypeOf(isolate, array), array));
     }
-    bool set_len_ok;
-    MAYBE_ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
-        isolate, set_len_ok, JSArray::SetLength(array, new_length));
+    JSArray::SetLength(array, new_length);
   }
 
   return *result;
@@ -601,8 +595,7 @@ BUILTIN(ArrayShift) {
 
   if (CanUseFastArrayShift(isolate, receiver)) {
     Handle<JSArray> array = Handle<JSArray>::cast(receiver);
-    RETURN_RESULT_OR_FAILURE(isolate,
-                             array->GetElementsAccessor()->Shift(array));
+    return *array->GetElementsAccessor()->Shift(array);
   }
 
   return GenericArrayShift(isolate, receiver, length);
@@ -630,9 +623,7 @@ BUILTIN(ArrayUnshift) {
   DCHECK(!JSArray::HasReadOnlyLength(array));
 
   ElementsAccessor* accessor = array->GetElementsAccessor();
-  uint32_t new_length;
-  MAYBE_ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
-      isolate, new_length, accessor->Unshift(array, &args, to_add));
+  int new_length = accessor->Unshift(array, &args, to_add);
   return Smi::FromInt(new_length);
 }
 

src/builtins/builtins-typed-array.cc

@@ -154,8 +154,7 @@ BUILTIN(TypedArrayPrototypeFill) {
   DCHECK_LE(end, len);
   DCHECK_LE(count, len);
 
-  RETURN_RESULT_OR_FAILURE(isolate, ElementsAccessor::ForKind(kind)->Fill(
-                                        array, obj_value, start, end));
+  return ElementsAccessor::ForKind(kind)->Fill(array, obj_value, start, end);
 }
 
 BUILTIN(TypedArrayPrototypeIncludes) {

src/objects/elements.h

@@ -66,8 +66,7 @@ class ElementsAccessor {
   // changing array sizes as defined in EcmaScript 5.1 15.4.5.2, i.e. array that
   // have non-deletable elements can only be shrunk to the size of highest
   // element that is non-deletable.
-  virtual Maybe<bool> SetLength(Handle<JSArray> holder,
-                                uint32_t new_length) = 0;
+  virtual void SetLength(Handle<JSArray> holder, uint32_t new_length) = 0;
 
   // Copy all indices that have elements from |object| into the given
   // KeyAccumulator. For Dictionary-based element-kinds we filter out elements
@@ -97,13 +96,13 @@ class ElementsAccessor {
       Handle<JSObject> receiver, KeyAccumulator* accumulator,
       AddKeyConversion convert) = 0;
 
-  virtual Maybe<bool> TransitionElementsKind(Handle<JSObject> object,
-                                             Handle<Map> map) = 0;
-  virtual Maybe<bool> GrowCapacityAndConvert(Handle<JSObject> object,
-                                             uint32_t capacity) = 0;
+  virtual void TransitionElementsKind(Handle<JSObject> object,
+                                      Handle<Map> map) = 0;
+  virtual void GrowCapacityAndConvert(Handle<JSObject> object,
+                                      uint32_t capacity) = 0;
   // Unlike GrowCapacityAndConvert do not attempt to convert the backing store
   // and simply return false in this case.
-  virtual Maybe<bool> GrowCapacity(Handle<JSObject> object, uint32_t index) = 0;
+  virtual bool GrowCapacity(Handle<JSObject> object, uint32_t index) = 0;
 
   static void InitializeOncePerProcess();
   static void TearDown();
@@ -111,31 +110,29 @@ class ElementsAccessor {
   virtual void Set(Handle<JSObject> holder, InternalIndex entry,
                    Object value) = 0;
 
-  virtual Maybe<bool> Add(Handle<JSObject> object, uint32_t index,
-                          Handle<Object> value, PropertyAttributes attributes,
-                          uint32_t new_capacity) = 0;
+  virtual void Add(Handle<JSObject> object, uint32_t index,
+                   Handle<Object> value, PropertyAttributes attributes,
+                   uint32_t new_capacity) = 0;
 
   static Handle<JSArray> Concat(Isolate* isolate, BuiltinArguments* args,
                                 uint32_t concat_size, uint32_t result_length);
 
-  virtual Maybe<uint32_t> Push(Handle<JSArray> receiver, BuiltinArguments* args,
-                               uint32_t push_size) = 0;
+  virtual uint32_t Push(Handle<JSArray> receiver, BuiltinArguments* args,
+                        uint32_t push_size) = 0;
 
-  virtual Maybe<uint32_t> Unshift(Handle<JSArray> receiver,
-                                  BuiltinArguments* args,
-                                  uint32_t unshift_size) = 0;
+  virtual uint32_t Unshift(Handle<JSArray> receiver, BuiltinArguments* args,
+                           uint32_t unshift_size) = 0;
 
-  virtual MaybeHandle<Object> Pop(Handle<JSArray> receiver) = 0;
+  virtual Handle<Object> Pop(Handle<JSArray> receiver) = 0;
 
-  virtual MaybeHandle<Object> Shift(Handle<JSArray> receiver) = 0;
+  virtual Handle<Object> Shift(Handle<JSArray> receiver) = 0;
 
   virtual Handle<NumberDictionary> Normalize(Handle<JSObject> object) = 0;
 
   virtual size_t GetCapacity(JSObject holder, FixedArrayBase backing_store) = 0;
 
-  virtual MaybeHandle<Object> Fill(Handle<JSObject> receiver,
-                                   Handle<Object> obj_value, size_t start,
-                                   size_t end) = 0;
+  virtual Object Fill(Handle<JSObject> receiver, Handle<Object> obj_value,
+                      size_t start, size_t end) = 0;
 
   // Check an Object's own elements for an element (using SameValueZero
   // semantics)

src/objects/intl-objects.cc

@@ -479,12 +479,7 @@ Handle<JSObject> InnerAddElement(Isolate* isolate, Handle<JSArray> array,
                         field_type_string, NONE);
 
   JSObject::AddProperty(isolate, element, factory->value_string(), value, NONE);
-  // TODO(victorgomes): Temporarily forcing a fatal error here in case of
-  // overflow, until Intl::AddElement can handle exceptions.
-  if (JSObject::AddDataElement(array, index, element, NONE).IsNothing()) {
-    FATAL("Fatal JavaScript invalid array size when adding element");
-    UNREACHABLE();
-  }
+  JSObject::AddDataElement(array, index, element, NONE);
   return element;
 }
 
@@ -1574,9 +1569,9 @@ std::vector<std::string> BestFitSupportedLocales(
 }
 
 // ecma262 #sec-createarrayfromlist
-MaybeHandle<JSArray> CreateArrayFromList(Isolate* isolate,
-                                         std::vector<std::string> elements,
-                                         PropertyAttributes attr) {
+Handle<JSArray> CreateArrayFromList(Isolate* isolate,
+                                    std::vector<std::string> elements,
+                                    PropertyAttributes attr) {
   Factory* factory = isolate->factory();
   // Let array be ! ArrayCreate(0).
   Handle<JSArray> array = factory->NewJSArray(0);
@@ -1589,11 +1584,10 @@ MaybeHandle<JSArray> CreateArrayFromList(Isolate* isolate,
     const std::string& part = elements[i];
     Handle<String> value =
         factory->NewStringFromUtf8(CStrVector(part.c_str())).ToHandleChecked();
-    MAYBE_RETURN(JSObject::AddDataElement(array, i, value, attr),
-                 MaybeHandle<JSArray>());
+    JSObject::AddDataElement(array, i, value, attr);
   }
   // 5. Return array.
-  return MaybeHandle<JSArray>(array);
+  return array;
 }
 
 // ECMA 402 9.2.9 SupportedLocales(availableLocales, requestedLocales, options)

src/objects/js-array.h

@@ -58,8 +58,8 @@ class JSArray : public JSObject {
   // Initializes the array to a certain length.
   inline bool AllowsSetLength();
 
-  V8_EXPORT_PRIVATE static Maybe<bool> SetLength(Handle<JSArray> array,
-                                                 uint32_t length);
+  V8_EXPORT_PRIVATE static void SetLength(Handle<JSArray> array,
+                                          uint32_t length);
 
   // Set the content of the array to the content of storage.
   static inline void SetContent(Handle<JSArray> array,

src/objects/js-objects.cc

@@ -4849,9 +4849,9 @@ static ElementsKind BestFittingFastElementsKind(JSObject object) {
 }
 
 // static
-Maybe<bool> JSObject::AddDataElement(Handle<JSObject> object, uint32_t index,
-                                     Handle<Object> value,
-                                     PropertyAttributes attributes) {
+void JSObject::AddDataElement(Handle<JSObject> object, uint32_t index,
+                              Handle<Object> value,
+                              PropertyAttributes attributes) {
   Isolate* isolate = object->GetIsolate();
 
   DCHECK(object->map(isolate).is_extensible());
@@ -4894,15 +4894,13 @@ Maybe<bool> JSObject::AddDataElement(Handle<JSObject> object, uint32_t index,
   }
   to = GetMoreGeneralElementsKind(kind, to);
   ElementsAccessor* accessor = ElementsAccessor::ForKind(to);
-  MAYBE_RETURN(accessor->Add(object, index, value, attributes, new_capacity),
-               Nothing<bool>());
+  accessor->Add(object, index, value, attributes, new_capacity);
 
   if (object->IsJSArray(isolate) && index >= old_length) {
     Handle<Object> new_length =
         isolate->factory()->NewNumberFromUint(index + 1);
     JSArray::cast(*object).set_length(*new_length);
   }
-  return Just(true);
 }
 
 template <AllocationSiteUpdateMode update_or_check>
@@ -4969,15 +4967,7 @@ void JSObject::TransitionElementsKind(Handle<JSObject> object,
     DCHECK((IsSmiElementsKind(from_kind) && IsDoubleElementsKind(to_kind)) ||
            (IsDoubleElementsKind(from_kind) && IsObjectElementsKind(to_kind)));
     uint32_t c = static_cast<uint32_t>(object->elements().length());
-    if (ElementsAccessor::ForKind(to_kind)
-            ->GrowCapacityAndConvert(object, c)
-            .IsNothing()) {
-      // TODO(victorgomes): Temporarily forcing a fatal error here in case of
-      // overflow, until all users of TransitionElementsKind can handle
-      // exceptions.
-      FATAL("Fatal JavaScript invalid array size transitioning elements kind.");
-      UNREACHABLE();
-    }
+    ElementsAccessor::ForKind(to_kind)->GrowCapacityAndConvert(object, c);
   }
 }
 

src/objects/js-objects.h

@@ -440,9 +440,10 @@ class JSObject : public TorqueGeneratedJSObject<JSObject, JSReceiver> {
                           const char* name, Handle<Object> value,
                           PropertyAttributes attributes);
 
-  V8_EXPORT_PRIVATE static Maybe<bool> AddDataElement(
-      Handle<JSObject> receiver, uint32_t index, Handle<Object> value,
-      PropertyAttributes attributes);
+  V8_EXPORT_PRIVATE static void AddDataElement(Handle<JSObject> receiver,
+                                               uint32_t index,
+                                               Handle<Object> value,
+                                               PropertyAttributes attributes);
 
   // Extend the receiver with a single fast property appeared first in the
   // passed map. This also extends the property backing store if necessary.

src/objects/objects.cc

@@ -2899,9 +2899,8 @@ Maybe<bool> Object::AddDataProperty(LookupIterator* it, Handle<Object> value,
     }
 
     Handle<JSObject> receiver_obj = Handle<JSObject>::cast(receiver);
-    MAYBE_RETURN(JSObject::AddDataElement(receiver_obj, it->array_index(),
-                                          value, attributes),
-                 Nothing<bool>());
+    JSObject::AddDataElement(receiver_obj, it->array_index(), value,
+                             attributes);
     JSObject::ValidateElements(*receiver_obj);
     return Just(true);
   } else {
@@ -3419,7 +3418,7 @@ Maybe<bool> JSArray::ArraySetLength(Isolate* isolate, Handle<JSArray> a,
     // (Not needed.)
   }
   // Most of steps 16 through 19 is implemented by JSArray::SetLength.
-  MAYBE_RETURN(JSArray::SetLength(a, new_len), Nothing<bool>());
+  JSArray::SetLength(a, new_len);
   // Steps 19d-ii, 20.
   if (!new_writable) {
     PropertyDescriptor readonly;
@@ -5103,13 +5102,13 @@ void JSArray::Initialize(Handle<JSArray> array, int capacity, int length) {
       array, length, capacity, INITIALIZE_ARRAY_ELEMENTS_WITH_HOLE);
 }
 
-Maybe<bool> JSArray::SetLength(Handle<JSArray> array, uint32_t new_length) {
+void JSArray::SetLength(Handle<JSArray> array, uint32_t new_length) {
   // We should never end in here with a pixel or external array.
   DCHECK(array->AllowsSetLength());
   if (array->SetLengthWouldNormalize(new_length)) {
     JSObject::NormalizeElements(array);
   }
-  return array->GetElementsAccessor()->SetLength(array, new_length);
+  array->GetElementsAccessor()->SetLength(array, new_length);
 }
 
 // ES6: 9.5.2 [[SetPrototypeOf]] (V)

src/objects/value-serializer.cc

@@ -1548,7 +1548,7 @@ MaybeHandle<JSArray> ValueDeserializer::ReadSparseJSArray() {
   HandleScope scope(isolate_);
   Handle<JSArray> array =
       isolate_->factory()->NewJSArray(0, TERMINAL_FAST_ELEMENTS_KIND);
-  MAYBE_RETURN(JSArray::SetLength(array, length), MaybeHandle<JSArray>());
+  JSArray::SetLength(array, length);
   AddObjectWithID(id, array);
 
   uint32_t num_properties;
@@ -2321,7 +2321,7 @@ ValueDeserializer::ReadObjectUsingEntireBufferForLegacyFormat() {
 
         Handle<JSArray> js_array =
             isolate_->factory()->NewJSArray(0, TERMINAL_FAST_ELEMENTS_KIND);
-        MAYBE_RETURN_NULL(JSArray::SetLength(js_array, length));
+        JSArray::SetLength(js_array, length);
         size_t begin_properties =
             stack.size() - 2 * static_cast<size_t>(num_properties);
         if (num_properties &&

src/runtime/runtime-array.cc

@@ -28,14 +28,7 @@ RUNTIME_FUNCTION(Runtime_TransitionElementsKind) {
   CONVERT_ARG_HANDLE_CHECKED(JSObject, object, 0);
   CONVERT_ARG_HANDLE_CHECKED(Map, to_map, 1);
   ElementsKind to_kind = to_map->elements_kind();
-  if (ElementsAccessor::ForKind(to_kind)
-          ->TransitionElementsKind(object, to_map)
-          .IsNothing()) {
-    // TODO(victorgomes): EffectControlLinearizer::LowerTransitionElementsKind
-    // does not handle exceptions.
-    FATAL("Fatal JavaScript invalid array size");
-    UNREACHABLE();
-  }
+  ElementsAccessor::ForKind(to_kind)->TransitionElementsKind(object, to_map);
   return *object;
 }
 
@@ -187,11 +180,7 @@ RUNTIME_FUNCTION(Runtime_GrowArrayElements) {
   uint32_t capacity = static_cast<uint32_t>(object->elements().length());
 
   if (index >= capacity) {
-    bool has_grown;
-    MAYBE_ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
-        isolate, has_grown,
-        object->GetElementsAccessor()->GrowCapacity(object, index));
-    if (!has_grown) {
+    if (!object->GetElementsAccessor()->GrowCapacity(object, index)) {
       return Smi::zero();
     }
   }

test/mjsunit/mjsunit.status

@@ -196,7 +196,6 @@
   'regress/regress-crbug-808192': [SKIP],
   'regress/regress-crbug-941743': [SKIP],
   'regress/regress-create-exception': [SKIP],
-  'regress/regress-crbug-1201626': [SKIP],
 
   # These tests run out of stack space in debug mode.
   'big-array-literal': [SKIP],
@@ -535,9 +534,6 @@
   # The failed allocation causes an asan/msan/tsan error
   'es6/typedarray-construct-offset-not-smi': [SKIP],
 
-  # Skip slow tests.
-  'regress/regress-crbug-1201626': [SKIP],
-
   # Exception thrown during bootstrapping on ASAN builds, see issue 4236.
   'regress/regress-1132': [SKIP],
 

test/mjsunit/regress/regress-crbug-1201626.js

-// Copyright 2021 the V8 project authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-function main() {
-  let a = []
-  a.length = 4294967295.0;
-  function f() {
-    a.length = 0;
-    return -1
-  }
-  const len = {valueOf:f};
-  a.fill(1.1,0,len);
-}
-
-assertThrows(() => main(), RangeError);