From a90e80ff481a0689ba1a835f3dbcbf9a078076a5 Mon Sep 17 00:00:00 2001
From: Darius Mercadier <[email protected]>
Date: Fri, 26 Jun 2026 17:12:05 +0200
Subject: [PATCH] [turboshaft] Mark array initializing stores as initializing

With the right combination of loop unrolling and a bunch of other
optimizations kicking in in the perfect way, this missing annotation
can lead to the initializing stores being elided and the GC observing
uninitialized memory.

Fixed: 527385397
Change-Id: I581f8bf8b518a64d2a3427ab18746c1242ba6194
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/8005985
Reviewed-by: Nico Hartmann <[email protected]>
Auto-Submit: Darius Mercadier <[email protected]>
Commit-Queue: Nico Hartmann <[email protected]>
Cr-Commit-Position: refs/heads/main@{#108296}
---

diff --git a/src/compiler/turboshaft/assembler.h b/src/compiler/turboshaft/assembler.h
index 8c88ebd..07f4614 100644
--- a/src/compiler/turboshaft/assembler.h
+++ b/src/compiler/turboshaft/assembler.h
@@ -3384,15 +3384,19 @@
     return StoreElement(object, access, index, value, true);
   }
   template <typename Base>
-  void StoreNonArrayBufferElement(V<Base> object, const ElementAccess& access,
-                                  V<WordPtr> index, V<Any> value) {
-    return StoreElement(object, access, index, value, false);
+  void StoreNonArrayBufferElement(
+      V<Base> object, const ElementAccess& access, V<WordPtr> index,
+      V<Any> value, bool maybe_initializing_or_transitioning = false) {
+    return StoreElement(object, access, index, value, /*is_array_buffer*/ false,
+                        maybe_initializing_or_transitioning);
   }
 
   template <typename Class, typename T>
   void StoreElement(V<Class> object, const ElementAccessTS<Class, T>& access,
-                    ConstOrV<WordPtr> index, V<T> value) {
-    StoreElement(object, access, index, value, access.is_array_buffer_load);
+                    ConstOrV<WordPtr> index, V<T> value,
+                    bool maybe_initializing_or_transitioning = false) {
+    StoreElement(object, access, index, value, access.is_array_buffer_load,
+                 maybe_initializing_or_transitioning);
   }
 
   template <typename Class, typename T>
@@ -3400,7 +3404,8 @@
                          const ElementAccessTS<Class, T>& access,
                          ConstOrV<WordPtr> index, V<T> value) {
     StoreElement(object.object(), access, index, value,
-                 access.is_array_buffer_load);
+                 access.is_array_buffer_load,
+                 /*maybe_initializing_or_transitioning*/ true);
   }
 
   // TODO(nicohartmann): Remove `InitializeArrayBufferElement` once fully
@@ -3411,13 +3416,14 @@
                                     V<WordPtr> index, V<Any> value) {
     StoreArrayBufferElement(object.object(), access, index, value);
   }
-  // TODO(nicohartmann): Remove `InitializeNoneArrayBufferElement` once fully
+  // TODO(nicohartmann): Remove `InitializeNonArrayBufferElement` once fully
   // transitioned to `ElementAccess`.
   template <typename Base>
   void InitializeNonArrayBufferElement(Uninitialized<Base>& object,
                                        const ElementAccess& access,
                                        V<WordPtr> index, V<Any> value) {
-    StoreNonArrayBufferElement(object.object(), access, index, value);
+    StoreNonArrayBufferElement(object.object(), access, index, value,
+                               /*maybe_initializing_or_transitioning*/ true);
   }
 
 #if V8_STATIC_ROOTS_BOOL
@@ -5882,8 +5888,8 @@
   // instead of StoreElement.
   template <typename Base>
   void StoreElement(V<Base> object, const ElementAccess& access,
-                    ConstOrV<WordPtr> index, V<Any> value,
-                    bool is_array_buffer) {
+                    ConstOrV<WordPtr> index, V<Any> value, bool is_array_buffer,
+                    bool maybe_initializing_or_transitioning = false) {
     if constexpr (is_taggable_v<Base>) {
       DCHECK_EQ(access.base_is_tagged, BaseTaggedness::kTaggedBase);
     } else {
@@ -5895,7 +5901,8 @@
     MemoryRepresentation rep =
         MemoryRepresentation::FromMachineType(access.machine_type);
     Store(object, resolve(index), value, kind, rep, access.write_barrier_kind,
-          access.header_size, rep.SizeInBytesLog2());
+          access.header_size, rep.SizeInBytesLog2(),
+          maybe_initializing_or_transitioning);
   }
 
   // BranchAndBind should be called from GotoIf/GotoIfNot. It will insert a
diff --git a/src/compiler/turboshaft/machine-lowering-reducer-inl.h b/src/compiler/turboshaft/machine-lowering-reducer-inl.h
index 27b0e78..ec9e63f 100644
--- a/src/compiler/turboshaft/machine-lowering-reducer-inl.h
+++ b/src/compiler/turboshaft/machine-lowering-reducer-inl.h
@@ -2060,19 +2060,17 @@
                        AccessBuilder::ForFixedArrayLengthPadding(),
                        __ Word32Constant(0));
 #endif
-    // TODO(nicohartmann@): Should finish initialization only after all elements
-    // have been initialized.
-    auto array = __ FinishInitialization(std::move(uninitialized_array));
 
     ScopedVar<WordPtr> index(this, 0);
 
     WHILE(__ UintPtrLessThan(index, length)) {
-      __ StoreNonArrayBufferElement(array, access, index, the_hole_value);
+      __ InitializeNonArrayBufferElement(uninitialized_array, access, index,
+                                         the_hole_value);
       // Advance the {index}.
       index = __ WordPtrAdd(index, 1);
     }
 
-    GOTO(done, array);
+    GOTO(done, __ FinishInitialization(std::move(uninitialized_array)));
 
     BIND(done, result);
     return result;
diff --git a/test/mjsunit/turboshaft/regress-527385397.js b/test/mjsunit/turboshaft/regress-527385397.js
new file mode 100644
index 0000000..e8d929f
--- /dev/null
+++ b/test/mjsunit/turboshaft/regress-527385397.js
@@ -0,0 +1,67 @@
+// Copyright 2026 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
+
+// {glob} will be used to publish the allocation to the GC. Its {x} and {y}
+// field must not be at the same offset than `arr[2]` below. And its {y} field
+// must be a Tagged field (rather than Float64) so that it forces tagging when
+// we store to it, which can trigger a GC. (the "offset_16" field is at offset
+// 16 and is needed to that the "y" field isn't).
+let glob = { x : 42, offset_16 : "skipping offset 16", y : "abc" };
+
+function foo(x, ...rest) {
+  // Turboshaft can recognize rotations in xor trees, which Turbofan cannot. As
+  // a result, {lhs} and {rhs} below will not be recognized as being equal by
+  // Turbofan, but Turboshaft will realize that it's the case...
+  let lhs = (x << 2) ^ x ^ (x >>> 30);
+  let rhs = (x >>> 30) ^ x ^ (x << 2);
+  // ... which means that {cond} will be constant-folded to true but only in
+  // Turboshaft...
+  let cond = lhs == rhs;
+  // ... and len will thus be a Phi for Turbofan but 3 for Turboshaft.
+  let len = cond ? 3 : 4;
+
+  // Note that length 3 is the only one that works because:
+  //
+  //   - Only loops with length 3 or less are fully unrolled
+  //
+  //   - `new Array` does 2 allocation: the FixedArray backing store and the
+  //     JSArray, and to initialize the latter it does 4 stores, at offsets 0
+  //     (Map), 4 (properties_or_hash), 8 (elements == the FixedArra backing
+  //     store) and 12 (length), and it's important that none of those have the
+  //     same offset as the final `arr[2] = 42` (which will be at offset 16).
+
+  // Allocating fixed-sized array. This will be lowered to a NewArray, which
+  // itself will be lowered to a loop with 3 iterations, which will then be
+  // unrolled.
+  let arr = new Array(len);
+
+  // Publishing the array for the GC...
+  glob.x = arr;
+  // ... so that it's seen by the GC here.
+  %MajorGCForCompilerTesting();
+
+  // Storing at offset 2 in {arr}. This should not store-store eliminate with
+  // the initializing store since there is a GC in the middle.
+  arr[2] = 42;
+
+  // Making sure that {arr} escapes.
+  return arr;
+}
+
+// We need to warmup feedback with a non-smi int32 in order to avoid that `x<<2`
+// is lowered to `(x>>1)<<2` because of the untagging, since it would then be
+// optimized to `x<<1`, and the rotation would thus not be matched anymore since
+// `x<<1^x>>30` cannot be optimized to a rotation. With HeapNumber feedback, the
+// untagging will handle both Smi and HeapNumber and thus return a Phi, so
+// Turboshaft won't be able to optimize `x>>1<<2` to `x<<1` anymore.
+let non_smi_int32 = 0x80000000;
+
+%PrepareFunctionForOptimization(foo);
+foo(non_smi_int32);
+foo(42);
+
+%OptimizeFunctionOnNextCall(foo);
+foo(42);

diff --git a/test/mjsunit/turboshaft/regress-527385397.js b/test/mjsunit/turboshaft/regress-527385397.js
new file mode 100644
index 0000000..e8d929f
--- /dev/null
+++ b/test/mjsunit/turboshaft/regress-527385397.js
@@ -0,0 +1,67 @@
+// Copyright 2026 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
+
+// {glob} will be used to publish the allocation to the GC. Its {x} and {y}
+// field must not be at the same offset than `arr[2]` below. And its {y} field
+// must be a Tagged field (rather than Float64) so that it forces tagging when
+// we store to it, which can trigger a GC. (the "offset_16" field is at offset
+// 16 and is needed to that the "y" field isn't).
+let glob = { x : 42, offset_16 : "skipping offset 16", y : "abc" };
+
+function foo(x, ...rest) {
+  // Turboshaft can recognize rotations in xor trees, which Turbofan cannot. As
+  // a result, {lhs} and {rhs} below will not be recognized as being equal by
+  // Turbofan, but Turboshaft will realize that it's the case...
+  let lhs = (x << 2) ^ x ^ (x >>> 30);
+  let rhs = (x >>> 30) ^ x ^ (x << 2);
+  // ... which means that {cond} will be constant-folded to true but only in
+  // Turboshaft...
+  let cond = lhs == rhs;
+  // ... and len will thus be a Phi for Turbofan but 3 for Turboshaft.
+  let len = cond ? 3 : 4;
+
+  // Note that length 3 is the only one that works because:
+  //
+  //   - Only loops with length 3 or less are fully unrolled
+  //
+  //   - `new Array` does 2 allocation: the FixedArray backing store and the
+  //     JSArray, and to initialize the latter it does 4 stores, at offsets 0
+  //     (Map), 4 (properties_or_hash), 8 (elements == the FixedArra backing
+  //     store) and 12 (length), and it's important that none of those have the
+  //     same offset as the final `arr[2] = 42` (which will be at offset 16).
+
+  // Allocating fixed-sized array. This will be lowered to a NewArray, which
+  // itself will be lowered to a loop with 3 iterations, which will then be
+  // unrolled.
+  let arr = new Array(len);
+
+  // Publishing the array for the GC...
+  glob.x = arr;
+  // ... so that it's seen by the GC here.
+  %MajorGCForCompilerTesting();
+
+  // Storing at offset 2 in {arr}. This should not store-store eliminate with
+  // the initializing store since there is a GC in the middle.
+  arr[2] = 42;
+
+  // Making sure that {arr} escapes.
+  return arr;
+}
+
+// We need to warmup feedback with a non-smi int32 in order to avoid that `x<<2`
+// is lowered to `(x>>1)<<2` because of the untagging, since it would then be
+// optimized to `x<<1`, and the rotation would thus not be matched anymore since
+// `x<<1^x>>30` cannot be optimized to a rotation. With HeapNumber feedback, the
+// untagging will handle both Smi and HeapNumber and thus return a Phi, so
+// Turboshaft won't be able to optimize `x>>1<<2` to `x<<1` anymore.
+let non_smi_int32 = 0x80000000;
+
+%PrepareFunctionForOptimization(foo);
+foo(non_smi_int32);
+foo(42);
+
+%OptimizeFunctionOnNextCall(foo);
+foo(42);