Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions crates/perry-runtime/src/array/concat_reverse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down
5 changes: 5 additions & 0 deletions crates/perry-runtime/src/array/immutable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
7 changes: 6 additions & 1 deletion crates/perry-runtime/src/array/jsvalue_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
11 changes: 10 additions & 1 deletion crates/perry-runtime/src/array/push_pop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down
39 changes: 39 additions & 0 deletions crates/perry-runtime/src/array/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
86 changes: 86 additions & 0 deletions crates/perry/tests/string_append_heap_alias.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
Loading