Fix Array.toReversed to create properties for holes

Array.toReversed always creates properties even for holes, but the
optimization paths for HOLEY_* arrays did not respect the spec. This CL
fixes the fast paths to set `undefined` value instead of the hole.

Bug: chromium:1395672
Change-Id: I51584829caf312a1864f93928315782bb120ee14
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4081689
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Choongwoo Han <choongwoo.han@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#84766}

src/builtins/array-to-reversed.tq

@@ -3,13 +3,14 @@
 // found in the LICENSE file.
 
 namespace array {
-macro FastArrayToReversed<Elements : type extends FixedArrayBase, T: type>(
-    implicit context: Context)(
-    kind: constexpr ElementsKind, elements: FixedArrayBase,
-    length: Smi): JSArray {
+
+macro FastPackedDoubleArrayToReversed(implicit context: Context)(
+    elements: FixedDoubleArray, length: Smi): JSArray {
   // 3. Let A be ? ArrayCreate(𝔽(len)).
-  const copy: FixedArrayBase = AllocateFixedArray(
-      kind, SmiUntag(length), AllocationFlag::kAllowLargeObjectAllocation);
+  const copy: FixedDoubleArray =
+      UnsafeCast<FixedDoubleArray>(AllocateFixedArray(
+          ElementsKind::PACKED_DOUBLE_ELEMENTS, SmiUntag(length),
+          AllocationFlag::kAllowLargeObjectAllocation));
 
   // 4. Let k be 0.
   let k: Smi = 0;
@@ -21,10 +22,52 @@ macro FastArrayToReversed<Elements : type extends FixedArrayBase, T: type>(
     const from = length - k - 1;
 
     // c. Let fromValue be ? Get(O, from).
-    const fromValue: T = LoadElement<Elements, T>(elements, from);
+    const fromValue: float64 =
+        elements.floats[from].Value() otherwise unreachable;
 
     // d. Perform ! CreateDataPropertyOrThrow(A, Pk, fromValue).
-    StoreElement<Elements>(copy, k, fromValue);
+    StoreFixedDoubleArrayElement(copy, k, fromValue);
+
+    // e. Set k to k + 1.
+    ++k;
+  }
+
+  // 6. Return A.
+  const map: Map = LoadJSArrayElementsMap(
+      ElementsKind::PACKED_DOUBLE_ELEMENTS, LoadNativeContext(context));
+  return NewJSArray(map, copy);
+}
+
+macro FastArrayToReversed<FromElements : type extends FixedArrayBase>(
+    implicit context: Context)(
+    kind: constexpr ElementsKind, elements: FromElements, length: Smi,
+    initializeArray: constexpr bool): JSArray {
+  // 3. Let A be ? ArrayCreate(𝔽(len)).
+  const copy: FixedArrayBase = AllocateFixedArray(
+      kind, SmiUntag(length), AllocationFlag::kAllowLargeObjectAllocation);
+
+  // Reversing HOLEY_DOUBLE_ELEMENTS array may allocate heap numbers.
+  // We need to initialize the array to avoid running GC with garbage values.
+  if (initializeArray) {
+    dcheck(Is<FixedArray>(copy));
+    FillFixedArrayWithSmiZero(
+        kind, UnsafeCast<FixedArray>(copy), 0, SmiUntag(length));
+  }
+
+  // 4. Let k be 0.
+  let k: Smi = 0;
+
+  // 5. Repeat, while k < len,
+  while (k < length) {
+    // a. Let from be ! ToString(𝔽(len - k - 1)).
+    // b. Let Pk be ! ToString(𝔽(k)).
+    const from = length - k - 1;
+
+    // c. Let fromValue be ? Get(O, from).
+    const fromValue: Object = LoadElementOrUndefined(elements, from);
+
+    // d. Perform ! CreateDataPropertyOrThrow(A, Pk, fromValue).
+    StoreElement<FixedArray>(copy, k, fromValue);
 
     // e. Set k to k + 1.
     ++k;
@@ -43,27 +86,29 @@ macro TryFastArrayToReversed(implicit context: Context)(receiver: JSAny):
 
   const kind: ElementsKind = array.map.elements_kind;
   if (kind == ElementsKind::PACKED_SMI_ELEMENTS) {
-    return FastArrayToReversed<FixedArray, Object>(
-        ElementsKind::PACKED_SMI_ELEMENTS, array.elements, array.length);
+    return FastArrayToReversed<FixedArray>(
+        ElementsKind::PACKED_SMI_ELEMENTS,
+        UnsafeCast<FixedArray>(array.elements), array.length, false);
   } else if (kind == ElementsKind::PACKED_ELEMENTS) {
-    return FastArrayToReversed<FixedArray, Object>(
-        ElementsKind::PACKED_ELEMENTS, array.elements, array.length);
+    return FastArrayToReversed<FixedArray>(
+        ElementsKind::PACKED_ELEMENTS, UnsafeCast<FixedArray>(array.elements),
+        array.length, false);
   } else if (kind == ElementsKind::PACKED_DOUBLE_ELEMENTS) {
-    return FastArrayToReversed<FixedDoubleArray, float64_or_hole>(
-        ElementsKind::PACKED_DOUBLE_ELEMENTS, array.elements, array.length);
+    return FastPackedDoubleArrayToReversed(
+        UnsafeCast<FixedDoubleArray>(array.elements), array.length);
   } else {
     if (!IsPrototypeInitialArrayPrototype(array.map)) goto Slow;
     if (IsNoElementsProtectorCellInvalid()) goto Slow;
 
-    if (kind == ElementsKind::HOLEY_SMI_ELEMENTS) {
-      return FastArrayToReversed<FixedArray, Object>(
-          ElementsKind::HOLEY_SMI_ELEMENTS, array.elements, array.length);
-    } else if (kind == ElementsKind::HOLEY_ELEMENTS) {
-      return FastArrayToReversed<FixedArray, Object>(
-          ElementsKind::HOLEY_ELEMENTS, array.elements, array.length);
+    if (kind == ElementsKind::HOLEY_SMI_ELEMENTS ||
+        kind == ElementsKind::HOLEY_ELEMENTS) {
+      return FastArrayToReversed<FixedArray>(
+          ElementsKind::PACKED_ELEMENTS, UnsafeCast<FixedArray>(array.elements),
+          array.length, false);
     } else if (kind == ElementsKind::HOLEY_DOUBLE_ELEMENTS) {
-      return FastArrayToReversed<FixedDoubleArray, float64_or_hole>(
-          ElementsKind::HOLEY_DOUBLE_ELEMENTS, array.elements, array.length);
+      return FastArrayToReversed<FixedDoubleArray>(
+          ElementsKind::PACKED_ELEMENTS,
+          UnsafeCast<FixedDoubleArray>(array.elements), array.length, true);
     }
 
     goto Slow;

test/mjsunit/harmony/array-to-reversed.js

@@ -35,7 +35,9 @@ assertEquals("toReversed", Array.prototype.toReversed.name);
   let a = [1,,3,4];
   let r = a.toReversed();
   assertEquals([1,,3,4], a);
-  assertEquals([4,3,,1], r);
+  assertEquals([4,3,undefined,1], r);
+  assertFalse(a.hasOwnProperty(1));
+  assertTrue(r.hasOwnProperty(2));
   assertFalse(a === r);
 })();
 
@@ -43,7 +45,9 @@ assertEquals("toReversed", Array.prototype.toReversed.name);
   let a = [1.1,,3.3,4.4];
   let r = a.toReversed();
   assertEquals([1.1,,3.3,4.4], a);
-  assertEquals([4.4,3.3,,1.1], r);
+  assertEquals([4.4,3.3,undefined,1.1], r);
+  assertFalse(a.hasOwnProperty(1));
+  assertTrue(r.hasOwnProperty(2));
   assertFalse(a === r);
 })();
 
@@ -51,7 +55,9 @@ assertEquals("toReversed", Array.prototype.toReversed.name);
   let a = [true,false,,1,42.42];
   let r = a.toReversed();
   assertEquals([true,false,,1,42.42], a);
-  assertEquals([42.42,1,,false,true], r);
+  assertEquals([42.42,1,undefined,false,true], r);
+  assertFalse(a.hasOwnProperty(2));
+  assertTrue(r.hasOwnProperty(2));
   assertFalse(a === r);
 })();