[array] Move Array.p.push fall-back from JS to C++

This CL extends the existing ArrayPush C++ builtin with a generic
slow-path that replaces the JavaScript fall-back.

Bug: v8:7624
Change-Id: I1e8431601e8a872f3c5afba5d486f37fd5781d60
Reviewed-on: https://chromium-review.googlesource.com/1126922
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Simon Zünd <szuend@google.com>
Cr-Commit-Position: refs/heads/master@{#54282}

src/builtins/builtins-array.cc

@@ -141,30 +141,95 @@ V8_WARN_UNUSED_RESULT static Object* CallJsIntrinsic(
       isolate,
       Execution::Call(isolate, function, args.receiver(), argc, argv.start()));
 }
+
+V8_WARN_UNUSED_RESULT Object* GenericArrayPush(Isolate* isolate,
+                                               BuiltinArguments* args) {
+  // 1. Let O be ? ToObject(this value).
+  Handle<JSReceiver> receiver;
+  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
+      isolate, receiver, Object::ToObject(isolate, args->receiver()));
+
+  // 2. Let len be ? ToLength(? Get(O, "length")).
+  Handle<Object> raw_length_number;
+  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
+      isolate, raw_length_number,
+      Object::GetLengthFromArrayLike(isolate, Handle<Object>::cast(receiver)));
+
+  // 3. Let args be a List whose elements are, in left to right order,
+  //    the arguments that were passed to this function invocation.
+  // 4. Let arg_count be the number of elements in args.
+  int arg_count = args->length() - 1;
+
+  // 5. If len + arg_count > 2^53-1, throw a TypeError exception.
+  double length = raw_length_number->Number();
+  if (arg_count > kMaxSafeInteger - length) {
+    THROW_NEW_ERROR_RETURN_FAILURE(
+        isolate, NewTypeError(MessageTemplate::kPushPastSafeLength,
+                              isolate->factory()->NewNumberFromInt(arg_count),
+                              raw_length_number));
+  }
+
+  // 6. Repeat, while args is not empty.
+  for (int i = 0; i < arg_count; ++i) {
+    // a. Remove the first element from args and let E be the value of the
+    //    element.
+    Handle<Object> element = args->at(i + 1);
+
+    // b. Perform ? Set(O, ! ToString(len), E, true).
+    if (length <= static_cast<double>(JSArray::kMaxArrayIndex)) {
+      RETURN_FAILURE_ON_EXCEPTION(
+          isolate, Object::SetElement(isolate, receiver, length, element,
+                                      LanguageMode::kStrict));
+    } else {
+      bool success;
+      LookupIterator it = LookupIterator::PropertyOrElement(
+          isolate, receiver, isolate->factory()->NewNumber(length), &success);
+      // Must succeed since we always pass a valid key.
+      DCHECK(success);
+      MAYBE_RETURN(Object::SetProperty(&it, element, LanguageMode::kStrict,
+                                       Object::MAY_BE_STORE_FROM_KEYED),
+                   ReadOnlyRoots(isolate).exception());
+    }
+
+    // c. Let len be len+1.
+    ++length;
+  }
+
+  // 7. Perform ? Set(O, "length", len, true).
+  Handle<Object> final_length = isolate->factory()->NewNumber(length);
+  RETURN_FAILURE_ON_EXCEPTION(
+      isolate,
+      Object::SetProperty(receiver, isolate->factory()->length_string(),
+                          final_length, LanguageMode::kStrict));
+
+  // 8. Return len.
+  return *final_length;
+}
 }  // namespace
 
 BUILTIN(ArrayPush) {
   HandleScope scope(isolate);
   Handle<Object> receiver = args.receiver();
   if (!EnsureJSArrayWithWritableFastElements(isolate, receiver, &args, 1)) {
-    return CallJsIntrinsic(isolate, isolate->array_push(), args);
+    return GenericArrayPush(isolate, &args);
   }
+
   // Fast Elements Path
   int to_add = args.length() - 1;
   Handle<JSArray> array = Handle<JSArray>::cast(receiver);
-  int len = Smi::ToInt(array->length());
+  uint32_t len = static_cast<uint32_t>(array->length()->Number());
-  if (to_add == 0) return Smi::FromInt(len);
+  if (to_add == 0) return *isolate->factory()->NewNumberFromUint(len);
 
   // Currently fixed arrays cannot grow too big, so we should never hit this.
   DCHECK_LE(to_add, Smi::kMaxValue - Smi::ToInt(array->length()));
 
   if (JSArray::HasReadOnlyLength(array)) {
-    return CallJsIntrinsic(isolate, isolate->array_push(), args);
+    return GenericArrayPush(isolate, &args);
   }
 
   ElementsAccessor* accessor = array->GetElementsAccessor();
-  int new_length = accessor->Push(array, &args, to_add);
+  uint32_t new_length = accessor->Push(array, &args, to_add);
-  return Smi::FromInt(new_length);
+  return *isolate->factory()->NewNumberFromUint((new_length));
 }
 
 BUILTIN(ArrayPop) {

src/contexts.h

@@ -72,7 +72,6 @@ enum ContextLookupFlags {
 
 #define NATIVE_CONTEXT_IMPORTED_FIELDS(V)                                 \
   V(ARRAY_POP_INDEX, JSFunction, array_pop)                               \
-  V(ARRAY_PUSH_INDEX, JSFunction, array_push)                             \
   V(ARRAY_SHIFT_INDEX, JSFunction, array_shift)                           \
   V(ARRAY_SPLICE_INDEX, JSFunction, array_splice)                         \
   V(ARRAY_UNSHIFT_INDEX, JSFunction, array_unshift)                       \

src/js/array.js

@@ -409,28 +409,6 @@ function ArrayPopFallback() {
 }
 
 
-// Appends the arguments to the end of the array and returns the new
-// length of the array. See ECMA-262, section 15.4.4.7.
-function ArrayPushFallback() {
-  var array = TO_OBJECT(this);
-  var n = TO_LENGTH(array.length);
-  var m = arguments.length;
-
-  // Subtract n from kMaxSafeInteger rather than testing m + n >
-  // kMaxSafeInteger. n may already be kMaxSafeInteger. In that case adding
-  // e.g., 1 would not be safe.
-  if (m > kMaxSafeInteger - n) throw %make_type_error(kPushPastSafeLength, m, n);
-
-  for (var i = 0; i < m; i++) {
-    array[i+n] = arguments[i];
-  }
-
-  var new_length = n + m;
-  array.length = new_length;
-  return new_length;
-}
-
-
 // For implementing reverse() on large, sparse arrays.
 function SparseReverse(array, len) {
   var keys = GetSortedArrayKeys(array, %GetArrayKeys(array, len));
@@ -1060,7 +1038,6 @@ utils.Export(function(to) {
   "array_values_iterator", ArrayValues,
   // Fallback implementations of Array builtins.
   "array_pop", ArrayPopFallback,
-  "array_push", ArrayPushFallback,
   "array_shift", ArrayShiftFallback,
   "array_splice", ArraySpliceFallback,
   "array_unshift", ArrayUnshiftFallback,

src/objects/js-array.h

@@ -94,6 +94,9 @@ 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;
+
  private:
   DISALLOW_IMPLICIT_CONSTRUCTORS(JSArray);
 };