[array] Consistently throw TypeError for zero-length arrays

This CL fixes a bug that allowed calls to Array.p.shift on
zero-length arrays where the 'length' is read-only without throwing
a TypeError.

R=bmeurer@chromium.org, jgruber@chromium.org

Bug: chromium:882233
Change-Id: Ib129ab4c4f4f233e7bb553effa77539badfbe26e
Reviewed-on: https://chromium-review.googlesource.com/1215164Reviewed-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@{#55746}

src/builtins/builtins-array-gen.cc

@@ -1524,15 +1524,20 @@ TF_BUILTIN(ArrayPrototypeShift, CodeStubAssembler) {
   {
     TNode<JSArray> array_receiver = CAST(receiver);
     CSA_ASSERT(this, TaggedIsPositiveSmi(LoadJSArrayLength(array_receiver)));
+
+    // 2) Ensure that the length is writable.
+    //    This check needs to happen before the check for length zero.
+    //    The spec requires a "SetProperty(array, 'length', 0)" call when
+    //    the length is zero. This must throw an exception in the case of a
+    //    read-only length.
+    EnsureArrayLengthWritable(LoadMap(array_receiver), &runtime);
+
     Node* length =
         LoadAndUntagObjectField(array_receiver, JSArray::kLengthOffset);
     Label return_undefined(this), fast_elements_tagged(this),
         fast_elements_smi(this);
     GotoIf(IntPtrEqual(length, IntPtrConstant(0)), &return_undefined);
 
-    // 2) Ensure that the length is writable.
-    EnsureArrayLengthWritable(LoadMap(array_receiver), &runtime);
-
     // 3) Check that the elements backing store isn't copy-on-write.
     Node* elements = LoadElements(array_receiver);
     GotoIf(WordEqual(LoadMap(elements),

test/mjsunit/regress/regress-crbug-882233-1.js

+// Copyright 2018 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.
+
+// Intended to test bug [882233] on CSA fast-path.
+
+let array = [];
+Object.defineProperty(array, 'length', {writable: false});
+
+assertEquals(array.length, 0);
+assertThrows(() => array.shift(), TypeError);
+
+let object = { length: 0 };
+Object.defineProperty(object, 'length', {writable: false});
+
+assertEquals(object.length, 0);
+assertThrows(() => Array.prototype.shift.call(object));

test/mjsunit/regress/regress-crbug-882233-2.js

+// Copyright 2018 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.
+
+// Flags: --allow-natives-syntax --opt
+
+// Intended to test bug [882233] on TF inlined (js-call-reducer) path.
+
+function shift_array() {
+  let array = [];
+  Object.defineProperty(array, 'length', {writable : false});
+  return array.shift();
+}
+
+assertThrows(shift_array);
+assertThrows(shift_array);
+%OptimizeFunctionOnNextCall(shift_array);
+assertThrows(shift_array);
+assertOptimized(shift_array);
+
+
+function shift_object() {
+  let object = { length: 0 };
+  Object.defineProperty(object, 'length', {writable : false});
+  return object.shift();
+}
+
+assertThrows(shift_object);
+assertThrows(shift_object);
+%OptimizeFunctionOnNextCall(shift_object);
+assertThrows(shift_object);
+assertOptimized(shift_object);