diff --git a/crates/perry-runtime/src/array/concat_reverse.rs b/crates/perry-runtime/src/array/concat_reverse.rs index 5bb539b0a..768781a9c 100644 --- a/crates/perry-runtime/src/array/concat_reverse.rs +++ b/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(); diff --git a/crates/perry-runtime/src/array/immutable.rs b/crates/perry-runtime/src/array/immutable.rs index 9d8f957b3..5f4a9d53d 100644 --- a/crates/perry-runtime/src/array/immutable.rs +++ b/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); diff --git a/crates/perry-runtime/src/array/jsvalue_api.rs b/crates/perry-runtime/src/array/jsvalue_api.rs index 741264afa..5073bfc1e 100644 --- a/crates/perry-runtime/src/array/jsvalue_api.rs +++ b/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); } diff --git a/crates/perry-runtime/src/array/push_pop.rs b/crates/perry-runtime/src/array/push_pop.rs index b60eeba46..7a462e112 100644 --- a/crates/perry-runtime/src/array/push_pop.rs +++ b/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; diff --git a/crates/perry-runtime/src/array/tests.rs b/crates/perry-runtime/src/array/tests.rs index c1d4a6603..abf59aa21 100644 --- a/crates/perry-runtime/src/array/tests.rs +++ b/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(); diff --git a/crates/perry/tests/string_append_heap_alias.rs b/crates/perry/tests/string_append_heap_alias.rs index 01fc3a2e3..7948421a7 100644 --- a/crates/perry/tests/string_append_heap_alias.rs +++ b/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).