[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}

src/builtins/accessors.cc

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

src/builtins/builtins-array.cc

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

src/builtins/builtins-typed-array.cc

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

src/objects/elements.h

@@ -66,7 +66,8 @@ 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 void SetLength(Handle<JSArray> holder, uint32_t new_length) = 0;
+  virtual Maybe<bool> 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
@@ -96,13 +97,13 @@ class ElementsAccessor {
       Handle<JSObject> receiver, KeyAccumulator* accumulator,
       AddKeyConversion convert) = 0;
 
-  virtual void TransitionElementsKind(Handle<JSObject> object,
+  virtual Maybe<bool> TransitionElementsKind(Handle<JSObject> object,
                                              Handle<Map> map) = 0;
-  virtual void GrowCapacityAndConvert(Handle<JSObject> object,
+  virtual Maybe<bool> 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 bool GrowCapacity(Handle<JSObject> object, uint32_t index) = 0;
+  virtual Maybe<bool> GrowCapacity(Handle<JSObject> object, uint32_t index) = 0;
 
   static void InitializeOncePerProcess();
   static void TearDown();
@@ -110,29 +111,31 @@ class ElementsAccessor {
   virtual void Set(Handle<JSObject> holder, InternalIndex entry,
                    Object value) = 0;
 
-  virtual void Add(Handle<JSObject> object, uint32_t index,
+  virtual Maybe<bool> 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 uint32_t Push(Handle<JSArray> receiver, BuiltinArguments* args,
+  virtual Maybe<uint32_t> Push(Handle<JSArray> receiver, BuiltinArguments* args,
                                uint32_t push_size) = 0;
 
-  virtual uint32_t Unshift(Handle<JSArray> receiver, BuiltinArguments* args,
+  virtual Maybe<uint32_t> Unshift(Handle<JSArray> receiver,
+                                  BuiltinArguments* args,
                                   uint32_t unshift_size) = 0;
 
-  virtual Handle<Object> Pop(Handle<JSArray> receiver) = 0;
+  virtual MaybeHandle<Object> Pop(Handle<JSArray> receiver) = 0;
 
-  virtual Handle<Object> Shift(Handle<JSArray> receiver) = 0;
+  virtual MaybeHandle<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 Object Fill(Handle<JSObject> receiver, Handle<Object> obj_value,
-                      size_t start, size_t end) = 0;
+  virtual MaybeHandle<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,7 +479,12 @@ Handle<JSObject> InnerAddElement(Isolate* isolate, Handle<JSArray> array,
                         field_type_string, NONE);
 
   JSObject::AddProperty(isolate, element, factory->value_string(), value, NONE);
-  JSObject::AddDataElement(array, index, element, 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();
+  }
   return element;
 }
 
@@ -1569,7 +1574,7 @@ std::vector<std::string> BestFitSupportedLocales(
 }
 
 // ecma262 #sec-createarrayfromlist
-Handle<JSArray> CreateArrayFromList(Isolate* isolate,
+MaybeHandle<JSArray> CreateArrayFromList(Isolate* isolate,
                                          std::vector<std::string> elements,
                                          PropertyAttributes attr) {
   Factory* factory = isolate->factory();
@@ -1584,10 +1589,11 @@ Handle<JSArray> CreateArrayFromList(Isolate* isolate,
     const std::string& part = elements[i];
     Handle<String> value =
         factory->NewStringFromUtf8(CStrVector(part.c_str())).ToHandleChecked();
-    JSObject::AddDataElement(array, i, value, attr);
+    MAYBE_RETURN(JSObject::AddDataElement(array, i, value, attr),
+                 MaybeHandle<JSArray>());
   }
   // 5. Return array.
-  return array;
+  return MaybeHandle<JSArray>(array);
 }
 
 // ECMA 402 9.2.9 SupportedLocales(availableLocales, requestedLocales, options)

src/objects/js-array.h

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

src/objects/js-objects.cc

@@ -4849,7 +4849,7 @@ static ElementsKind BestFittingFastElementsKind(JSObject object) {
 }
 
 // static
-void JSObject::AddDataElement(Handle<JSObject> object, uint32_t index,
+Maybe<bool> JSObject::AddDataElement(Handle<JSObject> object, uint32_t index,
                                      Handle<Object> value,
                                      PropertyAttributes attributes) {
   Isolate* isolate = object->GetIsolate();
@@ -4894,13 +4894,15 @@ void JSObject::AddDataElement(Handle<JSObject> object, uint32_t index,
   }
   to = GetMoreGeneralElementsKind(kind, to);
   ElementsAccessor* accessor = ElementsAccessor::ForKind(to);
-  accessor->Add(object, index, value, attributes, new_capacity);
+  MAYBE_RETURN(accessor->Add(object, index, value, attributes, new_capacity),
+               Nothing<bool>());
 
   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>
@@ -4967,7 +4969,15 @@ 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());
-    ElementsAccessor::ForKind(to_kind)->GrowCapacityAndConvert(object, c);
+    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();
+    }
   }
 }
 

src/objects/js-objects.h

@@ -440,9 +440,8 @@ class JSObject : public TorqueGeneratedJSObject<JSObject, JSReceiver> {
                           const char* name, Handle<Object> value,
                           PropertyAttributes attributes);
 
-  V8_EXPORT_PRIVATE static void AddDataElement(Handle<JSObject> receiver,
-                                               uint32_t index,
-                                               Handle<Object> value,
+  V8_EXPORT_PRIVATE static Maybe<bool> AddDataElement(
+      Handle<JSObject> receiver, uint32_t index, Handle<Object> value,
       PropertyAttributes attributes);
 
   // Extend the receiver with a single fast property appeared first in the

src/objects/objects.cc

@@ -2899,8 +2899,9 @@ Maybe<bool> Object::AddDataProperty(LookupIterator* it, Handle<Object> value,
     }
 
     Handle<JSObject> receiver_obj = Handle<JSObject>::cast(receiver);
-    JSObject::AddDataElement(receiver_obj, it->array_index(), value,
-                             attributes);
+    MAYBE_RETURN(JSObject::AddDataElement(receiver_obj, it->array_index(),
+                                          value, attributes),
+                 Nothing<bool>());
     JSObject::ValidateElements(*receiver_obj);
     return Just(true);
   } else {
@@ -3418,7 +3419,7 @@ Maybe<bool> JSArray::ArraySetLength(Isolate* isolate, Handle<JSArray> a,
     // (Not needed.)
   }
   // Most of steps 16 through 19 is implemented by JSArray::SetLength.
-  JSArray::SetLength(a, new_len);
+  MAYBE_RETURN(JSArray::SetLength(a, new_len), Nothing<bool>());
   // Steps 19d-ii, 20.
   if (!new_writable) {
     PropertyDescriptor readonly;
@@ -5102,13 +5103,13 @@ void JSArray::Initialize(Handle<JSArray> array, int capacity, int length) {
       array, length, capacity, INITIALIZE_ARRAY_ELEMENTS_WITH_HOLE);
 }
 
-void JSArray::SetLength(Handle<JSArray> array, uint32_t new_length) {
+Maybe<bool> 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);
   }
-  array->GetElementsAccessor()->SetLength(array, new_length);
+  return 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);
-  JSArray::SetLength(array, length);
+  MAYBE_RETURN(JSArray::SetLength(array, length), MaybeHandle<JSArray>());
   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);
-        JSArray::SetLength(js_array, length);
+        MAYBE_RETURN_NULL(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,7 +28,14 @@ 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();
-  ElementsAccessor::ForKind(to_kind)->TransitionElementsKind(object, to_map);
+  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();
+  }
   return *object;
 }
 
@@ -180,7 +187,11 @@ RUNTIME_FUNCTION(Runtime_GrowArrayElements) {
   uint32_t capacity = static_cast<uint32_t>(object->elements().length());
 
   if (index >= capacity) {
-    if (!object->GetElementsAccessor()->GrowCapacity(object, index)) {
+    bool has_grown;
+    MAYBE_ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
+        isolate, has_grown,
+        object->GetElementsAccessor()->GrowCapacity(object, index));
+    if (!has_grown) {
       return Smi::zero();
     }
   }

test/mjsunit/mjsunit.status

@@ -196,6 +196,7 @@
   '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],
@@ -534,6 +535,9 @@
   # 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);