fix(runtime): #5552 demote unique strings in remaining array insert paths (unshift / fill / with / from_jsvalue) (#5567)

* fix(runtime): #5552 demote unique strings in remaining array insert paths

Follow-up to #5533 (object fields) and #5548 (the array store paths it
enumerated). A uniquely-owned (refcount==1) heap string written into an array
element aliases that slot, so a later in-place `s += x` on the source local
(`js_string_append`'s refcount==1 fast path) rewrites the stored element and
corrupts it. #5548 fixed the push / set / from_values / splice-insert paths; the
sibling insert/replace paths below do the same raw element write without the
demote.

Apply the same tag-checked `js_string_addref_if_heap_string` (no-op for SSO /
non-string, idempotent) before the element write at each:

- `js_array_unshift_f64` (covers `js_array_unshift_jsvalue` transitively) and the
  per-item loop in `js_array_unshift_variadic`.
- `js_array_fill` / `js_array_fill_range` (demote once before the fill loop — the
  source aliases every filled slot) and `js_array_fill_generic` (its
  object-receiver loop writes `value` into each index directly; the array
  receiver delegates to the two above, so the extra demote is idempotent).
- `js_array_with` (the replacement value stored into the new array's slot; the
  cloned elements are already shared).
- `js_array_from_jsvalue` (mixed-type literal construction — the JSValue sibling
  of the already-covered `js_array_from_values`).

Internal reshuffles (sort, splice tail shift, slice copy, copyWithin) only move
values already stored in an array — already shared — so no demote is needed.

Tests: `string_append_heap_alias.rs` gains compile-run regressions for unshift /
fill / with, each confirmed to fail without the demote. `js_array_from_jsvalue`
is not emitted by codegen from any TypeScript source, so it gets a runtime unit
test (`array/tests.rs`) instead, also confirmed to fail without the demote.

Refs #5533, #5548.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(runtime): restore GC_STORE_AUDIT marker on unshift_variadic insert write

The #5552 demote line pushed the ptr::write past the proximity window of the
existing GC_STORE_AUDIT(BARRIERED) marker, failing the lint job's GC store-site
inventory check. Re-annotate the insert write directly.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Ralph <ralph@skelpo.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: 3 people

crates/perry-runtime/src/array/concat_reverse.rs

@@ -347,6 +347,11 @@ fn reverse_throw_type_error(message: &[u8]) -> ! {
 /// `value`. Returns the same array pointer.
 #[no_mangle]
 pub extern "C" fn js_array_fill(arr: *mut ArrayHeader, value: f64) -> *mut ArrayHeader {
+    // #5552: `fill` writes the same source into every slot — a uniquely-owned
+    // (refcount==1) string would alias the source AND every filled slot to each
+    // other, so a later `s += x` corrupts them all. Demote once to shared (no-op
+    // for SSO / non-string; mirrors `js_array_push_f64`, #5548).
+    crate::string::js_string_addref_if_heap_string(value);
     let arr = clean_arr_ptr_mut(arr);
     if arr.is_null() {
         return arr;
@@ -388,6 +393,9 @@ pub extern "C" fn js_array_fill_range(
     start: f64,
     end: f64,
 ) -> *mut ArrayHeader {
+    // #5552: demote a uniquely-owned source string once before it fills the
+    // range (no-op for SSO / non-string). See `js_array_fill`.
+    crate::string::js_string_addref_if_heap_string(value);
     let arr = clean_arr_ptr_mut(arr);
     if arr.is_null() {
         return arr;
@@ -472,6 +480,11 @@ pub extern "C" fn js_array_fill_generic(
     has_end: i32,
     end: f64,
 ) -> f64 {
+    // #5552: demote a uniquely-owned source string once before any slot write.
+    // The array receiver delegates to `js_array_fill`/`_range` (which also
+    // demote — idempotent), but the generic object-receiver loop below writes
+    // `value` into every index directly, so the demote must happen here too.
+    crate::string::js_string_addref_if_heap_string(value);
     let receiver_value = JSValue::from_bits(receiver.to_bits());
     if receiver_value.is_null() || receiver_value.is_undefined() {
         throw_fill_nullish_receiver();

crates/perry-runtime/src/array/immutable.rs

@@ -234,6 +234,11 @@ pub extern "C" fn js_array_with(
     index: f64,
     value: f64,
 ) -> *mut ArrayHeader {
+    // #5552: the replacement `value` is stored into the new array's slot — demote
+    // a uniquely-owned (refcount==1) string to shared so a later `s += x` on the
+    // source local can't corrupt it. No-op for SSO / non-string. The cloned
+    // elements come from `arr` and are already shared. (Mirrors #5548.)
+    crate::string::js_string_addref_if_heap_string(value);
     let arr = clean_arr_ptr(arr);
     if arr.is_null() {
         return js_array_alloc(0);

crates/perry-runtime/src/array/jsvalue_api.rs

@@ -36,8 +36,13 @@ pub extern "C" fn js_array_from_jsvalue(elements: *const u64, count: u32) -> *mu
         // Each u64 contains NaN-boxed JSValue bits, store as f64 bits
         for i in 0..count as usize {
             let bits = *elements.add(i);
+            let value = f64::from_bits(bits);
+            // #5552: demote a uniquely-owned source string before it aliases its
+            // slot — the JSValue sibling of the already-covered
+            // `js_array_from_values` (#5548). No-op for SSO / non-string.
+            crate::string::js_string_addref_if_heap_string(value);
             // GC_STORE_AUDIT(BARRIERED): JSValue array initialization is followed by layout/barrier rebuild.
-            ptr::write(arr_elements.add(i), f64::from_bits(bits));
+            ptr::write(arr_elements.add(i), value);
         }
         rebuild_array_layout(arr);
     }

crates/perry-runtime/src/array/push_pop.rs

@@ -496,6 +496,11 @@ pub extern "C" fn js_array_shift_f64(arr: *mut ArrayHeader) -> f64 {
 /// Returns a pointer to the (possibly reallocated) array
 #[no_mangle]
 pub extern "C" fn js_array_unshift_f64(arr: *mut ArrayHeader, value: f64) -> *mut ArrayHeader {
+    // #5552: a uniquely-owned (refcount==1) string unshifted to the front aliases
+    // the new slot — demote it to shared so a later `s += x` on the source local
+    // allocates fresh instead of mutating the stored element. No-op for SSO /
+    // non-string (mirrors `js_array_push_f64`, #5548).
+    crate::string::js_string_addref_if_heap_string(value);
     let arr = clean_arr_ptr_mut(arr);
     if arr.is_null() {
         return js_array_alloc(0);
@@ -590,8 +595,12 @@ pub extern "C" fn js_array_unshift_variadic(
         // Shift existing elements up by `n`.
         // GC_STORE_AUDIT(BARRIERED): memmove + new slots followed by layout/barrier rebuild.
         ptr::copy(elements_ptr, elements_ptr.add(n), length as usize);
-        // Write items in source order at the front.
+        // Write items in source order at the front. #5552: demote each
+        // uniquely-owned string before it aliases its slot (no-op for SSO /
+        // non-string).
         for (i, v) in item_vec.into_iter().enumerate() {
+            crate::string::js_string_addref_if_heap_string(v);
+            // GC_STORE_AUDIT(BARRIERED): inserted slots are followed by the layout/barrier rebuild below.
             ptr::write(elements_ptr.add(i), v);
         }
         (*arr).length = length + n as u32;

crates/perry-runtime/src/array/tests.rs

@@ -739,6 +739,45 @@ fn test_array_from_jsvalue_int32_rebuild_canonicalizes_raw_slots() {
     assert_ne!(js_array_get_f64_unchecked(arr, 1).to_bits(), elements[1]);
 }
 
+/// #5552: `js_array_from_jsvalue` (the JSValue sibling of `js_array_from_values`,
+/// used for mixed-type array construction) must demote a uniquely-owned
+/// (refcount==1) heap string before it aliases an element slot — otherwise a
+/// later in-place `js_string_append` on the source rewrites the stored element.
+/// Codegen never emits this symbol today, so this can only be exercised at the
+/// runtime level (no compiled-TS regression test can reach it).
+#[test]
+fn test_array_from_jsvalue_demotes_unique_string_against_inplace_append() {
+    // A uniquely-owned heap string with spare capacity, so `js_string_append`
+    // takes its in-place (refcount==1) fast path.
+    let init = b"prefix_init";
+    let s = crate::string::js_string_from_bytes_with_capacity(init.as_ptr(), init.len() as u32, 64);
+    unsafe {
+        (*s).refcount = 1;
+    }
+    let s_bits = crate::value::js_nanbox_string(s as i64).to_bits();
+
+    // Construct a mixed-type array via the JSValue path.
+    let elements = [s_bits, int32_jsvalue_bits(1)];
+    let arr = js_array_from_jsvalue(elements.as_ptr(), elements.len() as u32);
+
+    // Grow the source in place. With the demote, `s` is now shared (refcount==0)
+    // so this allocates fresh and leaves the stored element untouched; without
+    // it, the in-place append corrupts arr[0].
+    let more = crate::string::js_string_from_bytes(b"_more".as_ptr(), 5);
+    let grown = crate::string::js_string_append(s, more);
+
+    let stored_bits = js_array_get_jsvalue(arr, 0);
+    let stored_ptr = (stored_bits & crate::value::POINTER_MASK) as *const crate::StringHeader;
+    let stored = crate::string::string_as_str(stored_ptr);
+    assert_eq!(
+        stored, "prefix_init",
+        "string stored via js_array_from_jsvalue must not be corrupted by a later in-place append"
+    );
+    // Sanity: the source itself did grow (the append happened).
+    let grown_str = crate::string::string_as_str(grown as *const crate::StringHeader);
+    assert_eq!(grown_str, "prefix_init_more");
+}
+
 #[test]
 fn test_nonnumeric_append_downgrades_raw_f64_and_preserves_payload() {
     let bool_bits = crate::value::JSValue::bool(true).bits();

crates/perry/tests/string_append_heap_alias.rs

@@ -265,6 +265,92 @@ console.log("a0=" + a[0] + " s=" + s);
     );
 }
 
+// #5552: sibling array insert/replace paths that #5548 left uncovered —
+// `unshift`, `fill`, `with`, and mixed-type literal construction
+// (`js_array_from_jsvalue`). Each does a raw write of the source value into a
+// slot without the demote; the same snapshot-then-grow shape corrupts the
+// stored element. Each fails without the matching runtime demote.
+
+/// `a.unshift(s)` — front insertion (`js_array_unshift_f64`).
+#[test]
+fn unique_string_unshifted_into_array_is_not_corrupted() {
+    let dir = tempfile::tempdir().expect("tempdir");
+    let entry = dir.path().join("main.ts");
+    std::fs::write(
+        &entry,
+        r#"
+let s = "prefix";   // non-SSO, shared literal
+s += "_init";       // append on shared -> fresh heap string, refcount==1
+const a = ["x"];
+a.unshift(s);       // front insert -> must demote s to shared
+s += "_more";       // refcount==1 in-place append -> must NOT corrupt a[0]
+console.log("a0=" + a[0] + " s=" + s);
+"#,
+    )
+    .expect("write entry");
+    let out = compile_and_run(dir.path(), &entry);
+    assert!(
+        out.contains("a0=prefix_init s=prefix_init_more"),
+        "string unshifted into an array must not be corrupted by a later += (got: {out:?})"
+    );
+}
+
+/// `a.fill(s)` — fills every slot with the same source (`js_array_fill`). The
+/// source aliases the source local AND every filled slot, so a later `+=` must
+/// not corrupt any of them.
+#[test]
+fn unique_string_filled_into_array_is_not_corrupted() {
+    let dir = tempfile::tempdir().expect("tempdir");
+    let entry = dir.path().join("main.ts");
+    std::fs::write(
+        &entry,
+        r#"
+let s = "prefix";
+s += "_init";
+const a = ["x", "y", "z"];
+a.fill(s);          // fill all slots -> must demote s to shared
+s += "_more";
+console.log("a0=" + a[0] + " a2=" + a[2] + " s=" + s);
+"#,
+    )
+    .expect("write entry");
+    let out = compile_and_run(dir.path(), &entry);
+    assert!(
+        out.contains("a0=prefix_init a2=prefix_init s=prefix_init_more"),
+        "string filled into an array must not be corrupted by a later += (got: {out:?})"
+    );
+}
+
+/// `a.with(i, s)` — immutable replace stores the source into the new array's
+/// slot (`js_array_with`).
+#[test]
+fn unique_string_stored_via_with_is_not_corrupted() {
+    let dir = tempfile::tempdir().expect("tempdir");
+    let entry = dir.path().join("main.ts");
+    std::fs::write(
+        &entry,
+        r#"
+let s = "prefix";
+s += "_init";
+const a = ["placeholder"].with(0, s);  // replace -> must demote s to shared
+s += "_more";
+console.log("a0=" + a[0] + " s=" + s);
+"#,
+    )
+    .expect("write entry");
+    let out = compile_and_run(dir.path(), &entry);
+    assert!(
+        out.contains("a0=prefix_init s=prefix_init_more"),
+        "string stored via arr.with() must not be corrupted by a later += (got: {out:?})"
+    );
+}
+
+// NOTE: the mixed-type construction path `js_array_from_jsvalue` (#5552) is not
+// emitted by codegen from any TypeScript source, so it has no compiled-TS
+// regression test here. Its demote is covered by the runtime unit test
+// `test_array_from_jsvalue_demotes_unique_string_against_inplace_append` in
+// crates/perry-runtime/src/array/tests.rs.
+
 /// The snapshot-then-grow shape end to end: store the latest value into a heap
 /// field each step, then keep growing the source. Every stored snapshot must
 /// retain the value it had when stored (no in-place rewrite).