[builtins] Fix ArrayPrototypeSlice

The bug was introduced in
https://chromium-review.googlesource.com/c/v8/v8/+/3147910 : We only
want the fast path when "start" is either missing or the number 0, not
when it's something which converts to 0.

Bug: chromium:1248704
Change-Id: I72bb8fa8a9b90a13aae216c6a8e16e7be54285fe
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3157948
Commit-Queue: Marja Hölttä <marja@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/main@{#76789}

src/builtins/array-slice.tq

@@ -150,12 +150,21 @@ ArrayPrototypeSlice(
   const end: JSAny = arguments[1];
   const relativeEnd: Number = end == Undefined ? len : ToInteger_Inline(end);
 
-  // Handle array cloning case if the receiver is a fast array. This logic
-  // should be in sync with ArrayPrototypeSlice (to a reasonable degree). This
-  // is because CloneFastJSArray produces arrays which are potentially COW. If
-  // there's a discrepancy, TF generates code which produces a COW array and
-  // then expects it to be non-COW (or the other way around) -> immediate deopt.
-  if (relativeStart == 0 && end == Undefined) {
+  // Handle array cloning case if the receiver is a fast array. In the case
+  // where relativeStart is 0 but start is not the SMI zero (e.g., start is an
+  // object whose valueOf returns 0) we must not call CloneFastJSArray. This is
+  // because CloneFastArray reloads the array length, and the ToInteger above
+  // might have called user code which changed it. Thus, calling
+  // CloneFastJSArray here is safe only if we know ToInteger didn't call user
+  // code.
+
+  // This logic should be in sync with ArrayPrototypeSlice (to a reasonable
+  // degree). This is because CloneFastJSArray produces arrays which are
+  // potentially COW. If there's a discrepancy, TF generates code which produces
+  // a COW array and then expects it to be non-COW (or the other way around) ->
+  // immediate deopt.
+  if ((start == Undefined || TaggedEqual(start, SmiConstant(0))) &&
+      end == Undefined) {
     typeswitch (receiver) {
       case (a: FastJSArrayForCopy): {
         return CloneFastJSArray(context, a);

test/mjsunit/regress/regress-crbug-1248704.js

+// Copyright 2021 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.
+
+let evil = {
+  valueOf: function () {
+    array.length = 1;
+  }
+};
+let array = [1, 2, 3];
+let newArray = array.slice(evil);
+assertEquals(3, newArray.length);