[runtime] TryFastArrayFill can throw exception

Bug: chromium:1206994, chromium:1206754, chromium:1206822
Change-Id: I8ccd501c5a918613fad59afdd65ca499ee57d7a1
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2882805
Commit-Queue: Victor Gomes <victorgomes@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#74500}

src/builtins/builtins-array.cc

@@ -208,16 +208,16 @@ V8_WARN_UNUSED_RESULT Object GenericArrayFill(Isolate* isolate,
   return *receiver;
 }
 
-V8_WARN_UNUSED_RESULT bool TryFastArrayFill(
+V8_WARN_UNUSED_RESULT Maybe<bool> TryFastArrayFill(
     Isolate* isolate, BuiltinArguments* args, Handle<JSReceiver> receiver,
     Handle<Object> value, double start_index, double end_index) {
   // If indices are too large, use generic path since they are stored as
   // properties, not in the element backing store.
-  if (end_index > kMaxUInt32) return false;
-  if (!receiver->IsJSObject()) return false;
+  if (end_index > kMaxUInt32) return Just(false);
+  if (!receiver->IsJSObject()) return Just(false);
 
   if (!EnsureJSArrayWithWritableFastElements(isolate, receiver, args, 1, 1)) {
-    return false;
+    return Just(false);
   }
 
   Handle<JSArray> array = Handle<JSArray>::cast(receiver);
@@ -241,14 +241,14 @@ V8_WARN_UNUSED_RESULT bool TryFastArrayFill(
   CHECK(DoubleToUint32IfEqualToSelf(end_index, &end));
 
   ElementsAccessor* accessor = array->GetElementsAccessor();
-  accessor->Fill(array, value, start, end);
-  return true;
+  RETURN_ON_EXCEPTION_VALUE(isolate, accessor->Fill(array, value, start, end),
+                            Nothing<bool>());
+  return Just(true);
 }
 }  // namespace
 
 BUILTIN(ArrayPrototypeFill) {
   HandleScope scope(isolate);
-
   if (isolate->debug_execution_mode() == DebugInfo::kSideEffects) {
     if (!isolate->debug()->PerformSideEffectCheckForObject(args.receiver())) {
       return ReadOnlyRoots(isolate).exception();
@@ -293,10 +293,12 @@ BUILTIN(ArrayPrototypeFill) {
 
   Handle<Object> value = args.atOrUndefined(isolate, 1);
 
-  if (TryFastArrayFill(isolate, &args, receiver, value, start_index,
-                       end_index)) {
-    return *receiver;
-  }
+  bool success;
+  MAYBE_ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
+      isolate, success,
+      TryFastArrayFill(isolate, &args, receiver, value, start_index,
+                       end_index));
+  if (success) return *receiver;
   return GenericArrayFill(isolate, receiver, value, start_index, end_index);
 }
 

src/objects/elements.h

@@ -66,7 +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,
+  V8_WARN_UNUSED_RESULT virtual Maybe<bool> SetLength(Handle<JSArray> holder,
                                                       uint32_t new_length) = 0;
 
   // Copy all indices that have elements from |object| into the given
@@ -97,13 +97,14 @@ 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;
+  V8_WARN_UNUSED_RESULT virtual Maybe<bool> TransitionElementsKind(
+      Handle<JSObject> object, Handle<Map> map) = 0;
+  V8_WARN_UNUSED_RESULT 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 Maybe<bool> GrowCapacity(Handle<JSObject> object, uint32_t index) = 0;
+  V8_WARN_UNUSED_RESULT virtual Maybe<bool> GrowCapacity(
+      Handle<JSObject> object, uint32_t index) = 0;
 
   static void InitializeOncePerProcess();
   static void TearDown();
@@ -111,30 +112,35 @@ 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,
+  V8_WARN_UNUSED_RESULT 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 Maybe<uint32_t> Push(Handle<JSArray> receiver, BuiltinArguments* args,
+  V8_WARN_UNUSED_RESULT virtual Maybe<uint32_t> Push(Handle<JSArray> receiver,
+                                                     BuiltinArguments* args,
                                                      uint32_t push_size) = 0;
 
-  virtual Maybe<uint32_t> Unshift(Handle<JSArray> receiver,
-                                  BuiltinArguments* args,
+  V8_WARN_UNUSED_RESULT virtual Maybe<uint32_t> Unshift(
+      Handle<JSArray> receiver, BuiltinArguments* args,
       uint32_t unshift_size) = 0;
 
-  virtual MaybeHandle<Object> Pop(Handle<JSArray> receiver) = 0;
+  V8_WARN_UNUSED_RESULT virtual MaybeHandle<Object> Pop(
+      Handle<JSArray> receiver) = 0;
 
-  virtual MaybeHandle<Object> Shift(Handle<JSArray> receiver) = 0;
+  V8_WARN_UNUSED_RESULT 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 MaybeHandle<Object> Fill(Handle<JSObject> receiver,
-                                   Handle<Object> obj_value, size_t start,
+  V8_WARN_UNUSED_RESULT 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

test/cctest/heap/test-heap.cc

@@ -5781,7 +5781,7 @@ TEST(Regress598319) {
         v8::HandleScope scope(CcTest::isolate());
         Handle<JSArray> js_array = isolate->factory()->NewJSArrayWithElements(
             Handle<FixedArray>(arr.get(), isolate));
-        js_array->GetElementsAccessor()->Shift(js_array);
+        js_array->GetElementsAccessor()->Shift(js_array).Check();
       }
       break;
     }