diff --git a/changelog.d/9339-array-prototype-define-property-index.md b/changelog.d/9339-array-prototype-define-property-index.md new file mode 100644 index 0000000000..947fd5338a --- /dev/null +++ b/changelog.d/9339-array-prototype-define-property-index.md @@ -0,0 +1,8 @@ +### fix(runtime): honor prototype index descriptors installed with defineProperty + +Indexed array assignments now observe accessors and non-writable data +properties installed on `Array.prototype` or `Object.prototype` through +`Object.defineProperty`, `Object.defineProperties`, or +`Reflect.defineProperty`. Descriptor installation raises the same prototype +invalidation latch as a plain indexed write, and the strict store fallback now +walks the default prototype chain before creating an own element. Fixes #9249. diff --git a/crates/perry-runtime/src/array/indexing.rs b/crates/perry-runtime/src/array/indexing.rs index acec2003d0..f08344312a 100644 --- a/crates/perry-runtime/src/array/indexing.rs +++ b/crates/perry-runtime/src/array/indexing.rs @@ -1228,20 +1228,24 @@ fn js_array_set_f64_extend_strict_impl( return js_array_set_f64_extend(arr, index, value); } - // #9220: only a retargeted array with no own index pays the inherited - // [[Set]] walk. `array_custom_prototype` is the #9219 classification shared - // with reads/HasProperty and deliberately returns None for a Proxy + // #9220 / #9249: only a prototype-sensitive array with no own index pays + // the inherited [[Set]] walk. This includes both a retargeted receiver and + // the default chain after an index is installed on `Array.prototype` or + // `Object.prototype`. `array_custom_prototype` is the #9219 classification + // shared with reads/HasProperty and deliberately returns None for a Proxy // prototype, whose dedicated dispatch must remain single-shot. Existing // own elements have already had every applicable dense lane above; the // fallback still needs the ownership check for descriptor/restricted // shapes that correctly declined those lanes. - // The process latch leads for the same reason it does in the read/HasProperty - // twin (`generic::real_array_uses_recorded_spec_path`): recording a - // prototype on ANY array sets it, so a clear latch proves this array cannot - // have one and the side-table probe is skipped entirely. + // + // The process latches lead for the same reason they do in the + // read/HasProperty twin: when all are clear, no inherited indexed property + // can intercept this store and the side-table probe is skipped entirely. if !prototype_already_checked - && crate::object::prototype_chain::array_static_proto_recorded() - && unsafe { array_custom_prototype(clean).is_some() } + && (array_prototype_has_index_flag() + || object_prototype_has_index_flag() + || (crate::object::prototype_chain::array_static_proto_recorded() + && unsafe { array_custom_prototype(clean).is_some() })) && unsafe { !array_has_own_index(clean, index) } { return array_spec_set(clean, index, value); diff --git a/crates/perry-runtime/src/array/mod.rs b/crates/perry-runtime/src/array/mod.rs index 74285f8582..5fb029c72a 100644 --- a/crates/perry-runtime/src/array/mod.rs +++ b/crates/perry-runtime/src/array/mod.rs @@ -153,9 +153,9 @@ pub use self::indexing::{ pub(crate) use self::indexing_support::test_keys_array_slot_fallbacks; pub(crate) use self::indexing_support::{ array_proto_iterator_modified, invalidate_array_index_fast_path, - keys_array_len_capped_to_capacity, keys_array_slot, note_array_proto_iterator_write, - note_object_prototype_index_write, object_prototype_has_index_flag, - PERRY_ARRAY_INDEX_FAST_PATH_INVALIDATED, + keys_array_len_capped_to_capacity, keys_array_slot, note_array_index_write, + note_array_proto_iterator_write, note_object_prototype_index_write, + object_prototype_has_index_flag, PERRY_ARRAY_INDEX_FAST_PATH_INVALIDATED, }; pub use self::is_array::js_array_is_array; pub(crate) use self::iter_methods::throw_reduce_of_empty; diff --git a/crates/perry-runtime/src/object/array_object_ops.rs b/crates/perry-runtime/src/object/array_object_ops.rs index 46ff0a1a9a..b39f4f520d 100644 --- a/crates/perry-runtime/src/object/array_object_ops.rs +++ b/crates/perry-runtime/src/object/array_object_ops.rs @@ -460,6 +460,13 @@ pub(crate) unsafe fn define_array_property( } if let Some(index) = super::canonical_array_index(key_name) { + // `Object.defineProperty(Array.prototype, i, descriptor)` installs an + // inherited index without passing through any array element-write + // helper. Raise the same sticky latch those helpers do so generated + // stores decline their own-slot fast paths and perform the inherited + // descriptor walk. `Object.defineProperties` and + // `Reflect.defineProperty` both funnel through this branch. + crate::array::note_array_index_write(current_arr() as usize); let exists = super::has_own_helpers::array_own_key_present(current_arr(), current_key()); // Array exotic `[[DefineOwnProperty]]` (ECMA-262 10.4.2.1) step 3.b: a diff --git a/crates/perry/tests/issue_9249_array_prototype_define_property.rs b/crates/perry/tests/issue_9249_array_prototype_define_property.rs new file mode 100644 index 0000000000..7abc78182f --- /dev/null +++ b/crates/perry/tests/issue_9249_array_prototype_define_property.rs @@ -0,0 +1,132 @@ +//! Regression coverage for #9249: indexed accessors installed on +//! `Array.prototype` through the descriptor APIs must invalidate array-store +//! fast paths just like a plain indexed assignment to the prototype does. + +use std::path::PathBuf; +use std::process::Command; + +fn perry_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_perry")) +} + +fn compile_and_run(source: &str, expected: &str, label: &str) { + let dir = tempfile::tempdir().expect("tempdir"); + let entry = dir.path().join(format!("{label}.ts")); + let output = dir.path().join(format!("{label}_bin")); + std::fs::write(&entry, source).expect("write entry"); + + let compile = Command::new(perry_bin()) + .current_dir(dir.path()) + .arg("compile") + .arg(&entry) + .arg("-o") + .arg(&output) + .arg("--no-cache") + .output() + .expect("run perry compile"); + assert!( + compile.status.success(), + "perry compile failed for {label}\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 for {label}\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&run.stdout), + String::from_utf8_lossy(&run.stderr) + ); + assert_eq!( + String::from_utf8_lossy(&run.stdout), + expected, + "{label} output must match Node\nstderr:\n{}", + String::from_utf8_lossy(&run.stderr) + ); +} + +#[test] +fn define_property_array_prototype_index_setter_intercepts_numeric_store() { + compile_and_run( + r#" +let hits = 0; +Object.defineProperty(Array.prototype, 7, { + set(value) { hits++; }, + get() { return "P"; }, + configurable: true +}); +const nums = [1, 2, 3]; +nums[7] = 42; +console.log(hits, nums.length, nums[7]); +"#, + "1 3 P\n", + "define_property", + ); +} + +#[test] +fn define_properties_array_prototype_index_setter_intercepts_boolean_store() { + compile_and_run( + r#" +let hits = 0; +const descriptors: any = {}; +descriptors[9] = { + set(value) { hits++; }, + get() { return "P"; }, + configurable: true +}; +Object.defineProperties(Array.prototype, descriptors); +const flags = [true, false]; +flags[9] = false; +console.log(hits, flags.length, flags[9]); +"#, + "1 2 P\n", + "define_properties", + ); +} + +#[test] +fn define_property_object_prototype_index_setter_intercepts_array_store() { + compile_and_run( + r#" +let hits = 0; +Object.defineProperty(Object.prototype, 5, { + set(value) { hits++; }, + get() { return "P"; }, + configurable: true +}); +const values = [1]; +values[5] = 99; +console.log(hits, values.length, values[5]); +"#, + "1 1 P\n", + "object_prototype", + ); +} + +#[test] +fn reflect_define_property_non_writable_prototype_index_blocks_array_store() { + compile_and_run( + r#" +Reflect.defineProperty(Array.prototype, 11, { + value: "P", + writable: false, + configurable: true +}); +const values = [1]; +let result = "no error"; +try { + values[11] = 99; +} catch (error) { + result = error.name; +} +console.log(result, values.length, values[11]); +"#, + "TypeError 1 P\n", + "reflect_define_property", + ); +}