From 139627c5751304731adcb702dba8632dd2a33d93 Mon Sep 17 00:00:00 2001 From: Ralph Kuepper Date: Mon, 24 Aug 2026 18:33:46 +0200 Subject: [PATCH] fix(runtime): honor exotic indices in Array shift --- crates/perry-runtime/src/array/push_pop.rs | 84 ++++++++++++- .../tests/issue_5898_array_shift_exotic.rs | 118 ++++++++++++++++++ 2 files changed, 201 insertions(+), 1 deletion(-) create mode 100644 crates/perry/tests/issue_5898_array_shift_exotic.rs diff --git a/crates/perry-runtime/src/array/push_pop.rs b/crates/perry-runtime/src/array/push_pop.rs index 65e31deeb4..78b76b889a 100644 --- a/crates/perry-runtime/src/array/push_pop.rs +++ b/crates/perry-runtime/src/array/push_pop.rs @@ -1110,8 +1110,19 @@ pub extern "C" fn js_array_shift_f64(arr: *mut ArrayHeader) -> f64 { return TAG_UNDEFINED_F64; } + // A raw memmove is only equivalent to Shift when every observable + // indexed operation is an ordinary dense-array access. Indexed + // descriptors and prototype properties require the specified live + // HasProperty/Get/Set/Delete order; their accessors can also freeze the + // receiver or make `length` non-writable before the final length Set. + if crate::array::array_iteration_is_exotic(arr) { + return shift_array_spec_path(arr); + } + + // `TAG_HOLE` is an internal storage sentinel. Even on the dense path, + // Get(O, "0") must expose it as `undefined`. + let value = crate::array::js_array_get_f64(arr, 0); let elements_ptr = (arr as *mut u8).add(std::mem::size_of::()) as *mut f64; - let value = *elements_ptr; // Shift all elements down // GC_STORE_AUDIT(BARRIERED): shift memmove is followed by layout/barrier rebuild. @@ -1122,6 +1133,77 @@ pub extern "C" fn js_array_shift_f64(arr: *mut ArrayHeader) -> f64 { } } +/// ECMA-262 Array.prototype.shift for a real array whose indexed operations +/// are observable. The loop keeps the original length while consulting live +/// presence and values, and roots both the receiver and carried values across +/// accessors which may allocate or move either one. +unsafe fn shift_array_spec_path(arr: *mut ArrayHeader) -> f64 { + let scope = crate::gc::RuntimeHandleScope::new(); + let arr_handle = scope.root_raw_mut_ptr(arr); + let first_handle = scope.root_nanbox_f64(f64::from_bits(crate::value::TAG_UNDEFINED)); + let from_value_handle = scope.root_nanbox_f64(f64::from_bits(crate::value::TAG_UNDEFINED)); + let len = (*arr).length; + + let (first, _) = arr_handle.across_mut::(|| { + arr_handle.with_mut_ptr(|current| crate::array::array_spec_get(current, 0)) + }); + first_handle.set_nanbox_f64(first); + + for from in 1..len { + let from_present = arr_handle.with_mut_ptr::(|current| { + crate::array::array_spec_has_index(current, from) + }); + let to = from - 1; + if from_present { + let (value, _) = arr_handle.across_mut::(|| { + arr_handle.with_mut_ptr(|current| crate::array::array_spec_get(current, from)) + }); + from_value_handle.set_nanbox_f64(value); + shift_array_spec_set(&arr_handle, to, &from_value_handle); + } else { + shift_array_spec_delete(&arr_handle, to); + } + } + + shift_array_spec_delete(&arr_handle, len - 1); + + // Set(O, "length", len - 1, true) occurs after every indexed operation. + // Re-read the receiver state because any getter/setter above may have + // frozen it or replaced `length` with a non-writable descriptor. + arr_handle.with_mut_ptr::(|current| { + let current = clean_arr_ptr_mut(current); + if array_is_frozen(current) { + throw_frozen_array_mutation(); + } + guard_writable_length(current); + (*current).length = len - 1; + rebuild_array_layout(current); + }); + first_handle.get_nanbox_f64() +} + +fn shift_array_spec_set( + arr_handle: &crate::gc::RuntimeHandle<'_>, + index: u32, + value_handle: &crate::gc::RuntimeHandle<'_>, +) { + let _ = arr_handle.across_mut::(|| { + let value = value_handle.get_nanbox_f64(); + arr_handle.with_mut_ptr(|current| { + crate::array::js_array_set_f64_extend(current, index, value); + }); + }); +} + +fn shift_array_spec_delete(arr_handle: &crate::gc::RuntimeHandle<'_>, index: u32) { + let (deleted, _) = arr_handle.across_mut::(|| { + arr_handle.with_mut_ptr(|current| crate::array::js_array_delete(current, index)) + }); + if deleted == 0 { + throw_cannot_delete_array_index(index); + } +} + /// Unshift an element to the beginning of an array, growing if needed /// Returns a pointer to the (possibly reallocated) array #[no_mangle] diff --git a/crates/perry/tests/issue_5898_array_shift_exotic.rs b/crates/perry/tests/issue_5898_array_shift_exotic.rs new file mode 100644 index 0000000000..42e39d3672 --- /dev/null +++ b/crates/perry/tests/issue_5898_array_shift_exotic.rs @@ -0,0 +1,118 @@ +//! Regression coverage for the Array.prototype.shift exotic-index cluster in +//! #5898. Shift must use live ordinary-property operations and observe indexed +//! getter side effects before setting the final array length. + +use std::path::PathBuf; +use std::process::Command; + +fn perry_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_perry")) +} + +#[test] +fn shift_observes_inherited_indices_holes_and_length_side_effects() { + let dir = tempfile::tempdir().expect("tempdir"); + let entry = dir.path().join("main.ts"); + let output = dir.path().join("main_bin"); + let runtime_dir = perry_bin() + .parent() + .expect("perry binary directory") + .to_path_buf(); + std::fs::write( + &entry, + r#" +(Array.prototype as any)[1] = 1; +const inherited = [0]; +inherited.length = 2; +console.log("inherited", inherited.shift(), inherited[0], inherited[1]); +delete (Array.prototype as any)[1]; + +const holey: any[] = []; +holey[0] = 0; +holey[3] = 3; +console.log("holey-first", holey.shift(), holey.length, holey[0], holey[2]); +holey.length = 1; +console.log("holey-second", holey.shift(), holey.length); + +const frozen: any[] = new Array(1); +let frozenGetterCalls = 0; +Object.defineProperty(Array.prototype, "0", { + configurable: true, + get() { + Object.freeze(frozen); + frozenGetterCalls++; + } +}); +try { + frozen.shift(); + console.log("frozen no throw"); +} catch (error) { + console.log("frozen", error instanceof TypeError, frozen.length, frozenGetterCalls); +} +delete (Array.prototype as any)[0]; + +const readonlyLength: any[] = new Array(1); +let readonlyGetterCalls = 0; +Object.defineProperty(Array.prototype, "0", { + configurable: true, + get() { + Object.defineProperty(readonlyLength, "length", { writable: false }); + readonlyGetterCalls++; + } +}); +try { + readonlyLength.shift(); + console.log("readonly no throw"); +} catch (error) { + console.log( + "readonly", + error instanceof TypeError, + readonlyLength.length, + readonlyGetterCalls + ); +} +delete (Array.prototype as any)[0]; +"#, + ) + .expect("write entry"); + + let compile = Command::new(perry_bin()) + .current_dir(dir.path()) + .arg("compile") + .arg(&entry) + .arg("-o") + .arg(&output) + .arg("--no-cache") + .env("PERRY_LIB_DIR", &runtime_dir) + .env("PERRY_NO_AUTO_OPTIMIZE", "1") + .env("PERRY_RS4GC", "0") + .output() + .expect("run perry compile"); + assert!( + compile.status.success(), + "perry compile failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&compile.stdout), + String::from_utf8_lossy(&compile.stderr) + ); + + let run = Command::new(&output) + .current_dir(dir.path()) + .output() + .expect("run compiled binary"); + assert!( + run.status.success(), + "compiled binary failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&run.stdout), + String::from_utf8_lossy(&run.stderr) + ); + assert_eq!( + String::from_utf8_lossy(&run.stdout), + concat!( + "inherited 0 1 1\n", + "holey-first 0 3 undefined 3\n", + "holey-second undefined 0\n", + "frozen true 1 1\n", + "readonly true 1 1\n", + ) + ); +}