Fix Array.p.pop() for read-only length 0

Array.prototype.pop() must throw a TypeError whenever the array's
length is readonly; there is no exception to that when the length
is 0. This patch moves the length==0 special case after the read-
only length check in both fast paths (CSA and C++).

Fixed: v8:10908
Change-Id: I4a77439478cffeaf11022ff8beb78b0a907290d2
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2440576
Auto-Submit: Jakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70233}

src/builtins/builtins-array-gen.cc

@@ -246,11 +246,12 @@ TF_BUILTIN(ArrayPrototypePop, CodeStubAssembler) {
     TNode<JSArray> array_receiver = CAST(receiver);
     TNode<IntPtrT> length = SmiUntag(LoadFastJSArrayLength(array_receiver));
     Label return_undefined(this), fast_elements(this);
-    GotoIf(IntPtrEqual(length, IntPtrConstant(0)), &return_undefined);
 
     // 2) Ensure that the length is writable.
     EnsureArrayLengthWritable(context, LoadMap(array_receiver), &runtime);
 
+    GotoIf(IntPtrEqual(length, IntPtrConstant(0)), &return_undefined);
+
     // 3) Check that the elements backing store isn't copy-on-write.
     TNode<FixedArrayBase> elements = LoadElements(array_receiver);
     GotoIf(TaggedEqual(LoadMap(elements), FixedCOWArrayMapConstant()),

src/builtins/builtins-array.cc

@@ -457,11 +457,11 @@ BUILTIN(ArrayPop) {
   Handle<JSArray> array = Handle<JSArray>::cast(receiver);
 
   uint32_t len = static_cast<uint32_t>(array->length().Number());
-  if (len == 0) return ReadOnlyRoots(isolate).undefined_value();
 
   if (JSArray::HasReadOnlyLength(array)) {
     return GenericArrayPop(isolate, &args);
   }
+  if (len == 0) return ReadOnlyRoots(isolate).undefined_value();
 
   Handle<Object> result;
   if (IsJSArrayFastElementMovingAllowed(isolate, JSArray::cast(*receiver))) {

test/mjsunit/regress/regress-10908.js

+// Copyright 2020 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
+
+let a = [];
+Object.defineProperty(a, "length", {writable: false});
+function f() {
+  return a.pop();
+}
+assertThrows(f, TypeError, /Cannot assign to read only property 'length'/);
+
+%PrepareFunctionForOptimization(f);
+for (let i = 0; i < 3; i++) {
+  assertThrows(f, TypeError, /Cannot assign to read only property 'length'/);
+}
+%OptimizeFunctionOnNextCall(f);
+assertThrows(f, TypeError, /Cannot assign to read only property 'length'/);