[builtins] Array.prototype.concat should set length on return value

Per https://tc39.github.io/ecma262/#sec-array.prototype.concat, step 6.

Bug: v8:6707, v8:6708
Change-Id: Iad3eb94a3b5fe35e5ecd1b8632612a7f2f169434
Reviewed-on: https://chromium-review.googlesource.com/636695
Commit-Queue: Adam Klein <adamk@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47654}

src/builtins/builtins-array.cc

@@ -504,16 +504,17 @@ class ArrayConcatVisitor {
     return array;
   }
 
-  // Storage is either a FixedArray (if is_fixed_array()) or a JSReciever
-  // (otherwise)
-  Handle<FixedArray> storage_fixed_array() {
-    DCHECK(is_fixed_array());
-    DCHECK(has_simple_elements());
-    return Handle<FixedArray>::cast(storage_);
-  }
-  Handle<JSReceiver> storage_jsreceiver() {
+  MUST_USE_RESULT MaybeHandle<JSReceiver> ToJSReceiver() {
     DCHECK(!is_fixed_array());
-    return Handle<JSReceiver>::cast(storage_);
+    Handle<JSReceiver> result = Handle<JSReceiver>::cast(storage_);
+    Handle<Object> length =
+        isolate_->factory()->NewNumber(static_cast<double>(index_offset_));
+    RETURN_ON_EXCEPTION(
+        isolate_,
+        JSReceiver::SetProperty(result, isolate_->factory()->length_string(),
+                                length, STRICT),
+        JSReceiver);
+    return result;
   }
   bool has_simple_elements() const {
     return HasSimpleElementsField::decode(bit_field_);
@@ -568,6 +569,11 @@ class ArrayConcatVisitor {
     bit_field_ = ExceedsLimitField::update(bit_field_, exceeds);
   }
   bool is_fixed_array() const { return IsFixedArrayField::decode(bit_field_); }
+  Handle<FixedArray> storage_fixed_array() {
+    DCHECK(is_fixed_array());
+    DCHECK(has_simple_elements());
+    return Handle<FixedArray>::cast(storage_);
+  }
 
   Isolate* isolate_;
   Handle<Object> storage_;  // Always a global handle.
@@ -1124,7 +1130,7 @@ Object* Slow_ArrayConcat(BuiltinArguments* args, Handle<Object> species,
   if (is_array_species) {
     return *visitor.ToArray();
   } else {
-    return *visitor.storage_jsreceiver();
+    RETURN_RESULT_OR_FAILURE(isolate, visitor.ToJSReceiver());
   }
 }
 

test/mjsunit/es6/array-species.js

@@ -52,11 +52,10 @@ assertEquals(undefined,
              Array.prototype.map.call(new MyNonArray(), ()=>{}).length);
 assertEquals(undefined,
              Array.prototype.filter.call(new MyNonArray(), ()=>{}).length);
-assertEquals(undefined,
-             Array.prototype.concat.call(new MyNonArray(), ()=>{}).length);
-// slice and splice actually do explicitly define the length for some reason
+// slice, splice, and concat actually do explicitly define the length.
 assertEquals(0, Array.prototype.slice.call(new MyNonArray()).length);
 assertEquals(0, Array.prototype.splice.call(new MyNonArray()).length);
+assertEquals(1, Array.prototype.concat.call(new MyNonArray(), ()=>{}).length);
 
 // Cross-realm Arrays build same-realm arrays
 

test/mjsunit/regress/regress-6707.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.
+
+var a = [0, 1];
+a.constructor = {
+  [Symbol.species]: function(len) {
+    var arr = {length: 0};
+    Object.defineProperty(arr, "length", {writable: false});
+    return arr;
+  }
+}
+assertThrows(() => { Array.prototype.concat.call(a) }, TypeError);

test/mjsunit/regress/regress-6708.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.
+
+var a = [0, 1];
+a.constructor = {
+  [Symbol.species]: function(len) {
+    var arr = Array(20);
+    return arr;
+  }
+};
+assertEquals([0, 1], Array.prototype.concat.call(a));