Fix Array.protoype.slice bug in argument object handling

Bug: chromium:778574
Change-Id: I014b16b9deabab07ca7dfb662ea8cb0dbf9c8987
Reviewed-on: https://chromium-review.googlesource.com/738148
Commit-Queue: Daniel Clifford <danno@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48975}

src/builtins/builtins-array.cc

@@ -48,28 +48,6 @@ inline bool ClampedToInteger(Isolate* isolate, Object* object, int* out) {
   return false;
 }
 
-inline bool GetSloppyArgumentsLength(Isolate* isolate, Handle<JSObject> object,
-                                     int* out) {
-  Context* context = *isolate->native_context();
-  Map* map = object->map();
-  if (map != context->sloppy_arguments_map() &&
-      map != context->strict_arguments_map() &&
-      map != context->fast_aliased_arguments_map()) {
-    return false;
-  }
-  DCHECK(object->HasFastElements() || object->HasFastArgumentsElements());
-  Object* len_obj = object->InObjectPropertyAt(JSArgumentsObject::kLengthIndex);
-  if (!len_obj->IsSmi()) return false;
-  *out = Max(0, Smi::ToInt(len_obj));
-
-  FixedArray* parameters = FixedArray::cast(object->elements());
-  if (object->HasSloppyArgumentsElements()) {
-    FixedArray* arguments = FixedArray::cast(parameters->get(1));
-    return *out <= arguments->length();
-  }
-  return *out <= parameters->length();
-}
-
 inline bool IsJSArrayFastElementMovingAllowed(Isolate* isolate,
                                               JSArray* receiver) {
   return JSObject::PrototypeHasNoElements(isolate, receiver);
@@ -281,8 +259,8 @@ BUILTIN(ArraySlice) {
     }
     len = Smi::ToInt(array->length());
   } else if (receiver->IsJSObject() &&
-             GetSloppyArgumentsLength(isolate, Handle<JSObject>::cast(receiver),
-                                      &len)) {
+             JSSloppyArgumentsObject::GetSloppyArgumentsLength(
+                 isolate, Handle<JSObject>::cast(receiver), &len)) {
     // Array.prototype.slice.call(arguments, ...) is quite a common idiom
     // (notably more than 50% of invocations in Web apps).
     // Treat it in C++ as well.

src/objects/arguments-inl.h

@@ -45,6 +45,33 @@ void SloppyArgumentsElements::set_mapped_entry(uint32_t entry, Object* object) {
   set(entry + kParameterMapStart, object);
 }
 
+// TODO(danno): This shouldn't be inline here, but to defensively avoid
+// regressions associated with the fix for the bug 778574, it's staying that way
+// until the splice implementation in builtin-arrays.cc can be removed and this
+// function can be moved into runtime-arrays.cc near its other usage.
+bool JSSloppyArgumentsObject::GetSloppyArgumentsLength(Isolate* isolate,
+                                                       Handle<JSObject> object,
+                                                       int* out) {
+  Context* context = *isolate->native_context();
+  Map* map = object->map();
+  if (map != context->sloppy_arguments_map() &&
+      map != context->strict_arguments_map() &&
+      map != context->fast_aliased_arguments_map()) {
+    return false;
+  }
+  DCHECK(object->HasFastElements() || object->HasFastArgumentsElements());
+  Object* len_obj = object->InObjectPropertyAt(JSArgumentsObject::kLengthIndex);
+  if (!len_obj->IsSmi()) return false;
+  *out = Max(0, Smi::ToInt(len_obj));
+
+  FixedArray* parameters = FixedArray::cast(object->elements());
+  if (object->HasSloppyArgumentsElements()) {
+    FixedArray* arguments = FixedArray::cast(parameters->get(1));
+    return *out <= arguments->length();
+  }
+  return *out <= parameters->length();
+}
+
 }  // namespace internal
 }  // namespace v8
 

src/objects/arguments.h

@@ -42,6 +42,10 @@ class JSSloppyArgumentsObject : public JSArgumentsObject {
   // Indices of in-object properties.
   static const int kCalleeIndex = kLengthIndex + 1;
 
+  inline static bool GetSloppyArgumentsLength(Isolate* isolate,
+                                              Handle<JSObject> object,
+                                              int* out);
+
  private:
   DISALLOW_IMPLICIT_CONSTRUCTORS(JSSloppyArgumentsObject);
 };

src/runtime/runtime-array.cc

@@ -395,12 +395,11 @@ RUNTIME_FUNCTION(Runtime_TrySliceSimpleNonFastElements) {
       return Smi::FromInt(0);
     }
   } else {
-    Map* map = receiver->map();
-    Context* context = *isolate->native_context();
-    if (map != context->sloppy_arguments_map() &&
-        map != context->strict_arguments_map() &&
-        map != context->fast_aliased_arguments_map() &&
-        map != context->slow_aliased_arguments_map()) {
+    int len;
+    if (!receiver->IsJSObject() ||
+        !JSSloppyArgumentsObject::GetSloppyArgumentsLength(
+            isolate, Handle<JSObject>::cast(receiver), &len) ||
+        (length > static_cast<uint32_t>(len))) {
       return Smi::FromInt(0);
     }
   }

test/mjsunit/regress/regress-778574.js

+// Copyright 2017 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 () {
+  arguments.length = 7;
+  Array.prototype.slice.call(arguments);
+})();