Skip to content
Closed
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
8 changes: 8 additions & 0 deletions changelog.d/9339-array-prototype-define-property-index.md
Original file line number Diff line number Diff line change
@@ -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.
110 changes: 50 additions & 60 deletions crates/perry-runtime/src/array/indexing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use std::sync::atomic::Ordering;

#[path = "indexing_keyed.rs"]
mod keyed;
pub(crate) use keyed::js_array_set_index_or_string_inherited_strict;
pub use keyed::{
js_array_get_index_or_string, js_array_set_index_or_string,
js_array_set_index_or_string_strict, js_array_set_string_key,
Expand All @@ -23,62 +24,14 @@ const MAX_DENSE_ARRAY_GROW_LENGTH: u32 = 1_000_000;
/// benchmark for 6 hours (Regression Check, v0.5.1129–v0.5.1150).
const DENSE_ARRAY_GAP_LIMIT: u32 = 1024;

#[inline]
pub(crate) fn invalidate_array_index_fast_path() {
PERRY_ARRAY_INDEX_FAST_PATH_INVALIDATED.store(1, Ordering::Relaxed);
}

#[cfg(test)]
thread_local! {
static STRICT_DENSE_POINTER_OVERWRITE_HITS: std::cell::Cell<u64> = const {
std::cell::Cell::new(0)
};
}

#[cfg(test)]
pub(crate) fn test_strict_dense_pointer_overwrite_hits() -> u64 {
STRICT_DENSE_POINTER_OVERWRITE_HITS.with(std::cell::Cell::get)
}

// Test-only entry counter for `js_array_get_f64`, the JS-facing element
// accessor. A runtime walk that reaches for it PER ELEMENT is paying the whole
// gauntlet (forward-resolution, Map/Set/typed-array/buffer registry probes,
// descriptor gate, hole translation) for what is a raw slot read, so tests that
// assert "this walk no longer uses the element accessor" count it rather than
// timing it. Same shape as the hit counter above.
#[cfg(test)]
thread_local! {
static ELEMENT_ACCESSOR_CALLS: std::cell::Cell<u64> = const { std::cell::Cell::new(0) };
}

#[cfg(test)]
pub(crate) fn test_element_accessor_calls() -> u64 {
ELEMENT_ACCESSOR_CALLS.with(std::cell::Cell::get)
}

// The two strict-dense store helpers live in `strict_dense_test_helpers`
// (2000-line cap). Re-exported by name so `super::indexing::…` paths in the
// existing test modules keep resolving — a glob would not propagate.
// Strict-dense helpers and counters live in `strict_dense_test_helpers`
// (2000-line cap). Re-exported by name so existing test paths keep resolving.
#[cfg(test)]
pub(crate) use super::strict_dense_test_helpers::{
test_strict_dense_number_store, test_strict_dense_pointer_overwrite,
test_element_accessor_calls, test_strict_dense_number_store,
test_strict_dense_pointer_overwrite, test_strict_dense_pointer_overwrite_hits,
};

pub(crate) fn object_prototype_has_index_flag() -> bool {
OBJECT_PROTO_HAS_INDEX.load(Ordering::Relaxed)
}

/// Record (if `arr` is `Array.prototype`) that the prototype now carries an
/// indexed property, so subsequent out-of-bounds reads consult it. Called from
/// the array element-write paths; cheap (two relaxed atomic loads + compare).
#[inline]
pub(crate) fn note_array_index_write(arr: usize) {
if !ARRAY_PROTO_HAS_INDEX.load(Ordering::Relaxed) && arr != 0 && arr == array_prototype_addr() {
ARRAY_PROTO_HAS_INDEX.store(true, Ordering::Relaxed);
invalidate_array_index_fast_path();
}
}

/// Out-of-bounds element read fallback: `Array.prototype[index]` when the
/// prototype has indexed properties (see `ARRAY_PROTO_HAS_INDEX`). Returns the
/// inherited value, or `undefined` if absent. Skipped entirely when the
Expand Down Expand Up @@ -535,10 +488,11 @@ pub(crate) fn array_spec_set(arr: *mut ArrayHeader, index: u32, value: f64) -> *

unsafe {
if array_has_own_index(arr_handle.get_raw_mut_ptr::<ArrayHeader>(), index) {
return js_array_set_f64_extend_strict(
return js_array_set_f64_extend_strict_impl(
arr_handle.get_raw_mut_ptr::<ArrayHeader>(),
index,
value_handle.get_nanbox_f64(),
true,
);
}

Expand Down Expand Up @@ -598,10 +552,11 @@ pub(crate) fn array_spec_set(arr: *mut ArrayHeader, index: u32, value: f64) -> *
}
}

js_array_set_f64_extend_strict(
js_array_set_f64_extend_strict_impl(
arr_handle.get_raw_mut_ptr::<ArrayHeader>(),
index,
value_handle.get_nanbox_f64(),
true,
)
}
}
Expand Down Expand Up @@ -926,7 +881,7 @@ pub extern "C" fn js_array_numeric_get_f64_unboxed(arr: *mut ArrayHeader, index:
pub extern "C" fn js_array_get_f64(arr: *const ArrayHeader, index: u32) -> f64 {
const TAG_UNDEFINED_F64: f64 = f64::from_bits(0x7FFC_0000_0000_0001u64);
#[cfg(test)]
ELEMENT_ACCESSOR_CALLS.with(|c| c.set(c.get().wrapping_add(1)));
super::strict_dense_test_helpers::note_element_accessor_call();

// Issue #179 Phase 5: lazy fast path — must run BEFORE
// `clean_arr_ptr` because that helper force-materializes a lazy
Expand Down Expand Up @@ -1502,12 +1457,22 @@ pub(crate) unsafe fn try_strict_dense_number_store(
} else {
value_bits
};
let slot = super::header::array_elements_ptr(arr).add(index as usize);
// An in-range hole is not an own property. Once any prototype-index
// condition invalidates generated stores, decline this numeric lane so the
// inherited setter/non-writable descriptor walk can run. Existing numeric
// slots remain safe to overwrite without consulting the prototype chain.
let may_have_holes = flags & crate::gc::GC_ARRAY_RAW_F64_LAYOUT == 0
|| flags & crate::gc::GC_ARRAY_RAW_F64_HOLES != 0;
if PERRY_ARRAY_INDEX_FAST_PATH_INVALIDATED.load(Ordering::Relaxed) != 0
&& may_have_holes
&& ptr::read(slot) == crate::value::TAG_HOLE
{
return None;
}
// GC_STORE_AUDIT(POINTER_FREE): a number never holds a heap pointer, and
// the receiver's layout was proved pointer-free or tag-scanned above.
ptr::write(
super::header::array_elements_ptr(arr).add(index as usize),
store_bits,
);
ptr::write(slot, store_bits);
Some(arr)
}

Expand Down Expand Up @@ -1614,6 +1579,19 @@ pub extern "C" fn js_array_set_f64_extend_strict(
arr: *mut ArrayHeader,
index: u32,
value: f64,
) -> *mut ArrayHeader {
js_array_set_f64_extend_strict_impl(arr, index, value, false)
}

/// Strict indexed assignment after optionally completing the inherited
/// descriptor walk. `prototype_already_checked` is true only for the callback
/// from [`array_spec_set`], preventing a writable inherited data property from
/// recursing when that walk proceeds to create the receiver's own element.
fn js_array_set_f64_extend_strict_impl(
arr: *mut ArrayHeader,
index: u32,
value: f64,
prototype_already_checked: bool,
) -> *mut ArrayHeader {
// Two exact fast lanes, each storing only what the general path below
// would store and declining every shape it cannot prove. The plain-number
Expand Down Expand Up @@ -1644,6 +1622,18 @@ pub extern "C" fn js_array_set_f64_extend_strict(
return js_array_set_f64_extend(arr, index, value);
}

// #9249: a missing own index must perform the inherited [[Set]] walk after
// descriptor APIs install an index on the default Array/Object prototype
// chain. Keep this narrower than #9220's custom-prototype routing, which is
// intentionally absent after #9345. Existing own elements have already had
// every applicable dense lane above.
if !prototype_already_checked
&& (array_prototype_has_index_flag() || object_prototype_has_index_flag())
&& unsafe { !array_has_own_index(clean, index) }
{
return array_spec_set(clean, index, value);
}

// SAFETY: the clean above resolved this exact live plain-array head. The
// guard performs no Perry allocation/safepoint, so the proof remains live
// for the store core.
Expand Down Expand Up @@ -1781,7 +1771,7 @@ pub(crate) fn try_strict_dense_index_set(
&& old_bits & pointer_mask != 0
{
#[cfg(test)]
STRICT_DENSE_POINTER_OVERWRITE_HITS.with(|hits| hits.set(hits.get().wrapping_add(1)));
super::strict_dense_test_helpers::note_strict_dense_pointer_overwrite_hit();
// GC_STORE_AUDIT(BARRIERED): old and new are constructively
// pointer-bearing, so the slot mask is unchanged. Maintain the
// independent element proof and the mandatory generational/SATB
Expand Down
33 changes: 33 additions & 0 deletions crates/perry-runtime/src/array/indexing_keyed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,39 @@ pub extern "C" fn js_array_set_index_or_string_strict(
js_array_set_index_or_string(arr, idx, value)
}

/// Preserve the typed-feedback fallback's established non-strict behavior for
/// own elements while applying strict inherited `[[Set]]` semantics to a
/// missing canonical index after the default prototype chain was invalidated.
/// This keeps unrelated guard failures (for example, an existing element on a
/// frozen array) on their original path while making prototype descriptors
/// observable for holes and out-of-bounds stores.
pub(crate) fn js_array_set_index_or_string_inherited_strict(
arr: *mut ArrayHeader,
idx: f64,
value: f64,
) -> *mut ArrayHeader {
if arr.is_null() || (!array_prototype_has_index_flag() && !object_prototype_has_index_flag()) {
return js_array_set_index_or_string(arr, idx, value);
}

// An SSO key can materialize a StringHeader while it is classified, so
// keep both the receiver and stored value live across classification.
let scope = crate::gc::RuntimeHandleScope::new();
let arr_handle = scope.root_raw_mut_ptr(arr);
let value_handle = scope.root_nanbox_f64(value);
let (index, arr) = arr_handle.across_mut::<ArrayHeader, _>(|| canonical_index_of_set_key(idx));
let value = value_handle.get_nanbox_f64();

if let Some(index) = index {
let clean = clean_arr_ptr_mut(arr);
if !clean.is_null() && unsafe { !array_has_own_index(clean, index) } {
return array_spec_set(clean, index, value);
}
}

js_array_set_index_or_string(arr, idx, value)
}

/// The canonical array index (`0..2^32-1`) a dynamic `arr[key] = v` key targets,
/// or `None` for a non-index key. Numbers use the array-index boundary; string
/// keys are parsed via their ToString so `arr["3"]` on a frozen array throws
Expand Down
7 changes: 4 additions & 3 deletions crates/perry-runtime/src/array/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ pub use self::immutable::{
pub(crate) use self::indexing::{
array_has_own_index, array_iteration_is_exotic, array_iteration_is_exotic_resolved,
array_prototype_has_index_flag, array_spec_get, array_spec_has_index, array_spec_set,
js_array_set_index_or_string_inherited_strict,
};
pub use self::indexing::{
js_array_get_element, js_array_get_element_f64, js_array_get_f64, js_array_get_f64_unchecked,
Expand All @@ -152,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;
Expand Down
25 changes: 25 additions & 0 deletions crates/perry-runtime/src/array/strict_dense_test_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,31 @@
use super::indexing::{try_strict_dense_number_store, try_strict_dense_pointer_overwrite};
use super::*;

thread_local! {
static STRICT_DENSE_POINTER_OVERWRITE_HITS: std::cell::Cell<u64> = const {
std::cell::Cell::new(0)
};
static ELEMENT_ACCESSOR_CALLS: std::cell::Cell<u64> = const { std::cell::Cell::new(0) };
}

pub(crate) fn note_strict_dense_pointer_overwrite_hit() {
STRICT_DENSE_POINTER_OVERWRITE_HITS.with(|hits| hits.set(hits.get().wrapping_add(1)));
}

pub(crate) fn test_strict_dense_pointer_overwrite_hits() -> u64 {
STRICT_DENSE_POINTER_OVERWRITE_HITS.with(std::cell::Cell::get)
}

// Entry counter for `js_array_get_f64`, the JS-facing element accessor. Tests
// use it to prove internal walks avoid the full per-element resolver gauntlet.
pub(crate) fn note_element_accessor_call() {
ELEMENT_ACCESSOR_CALLS.with(|calls| calls.set(calls.get().wrapping_add(1)));
}

pub(crate) fn test_element_accessor_calls() -> u64 {
ELEMENT_ACCESSOR_CALLS.with(std::cell::Cell::get)
}

pub(crate) fn test_strict_dense_pointer_overwrite(
arr: *mut ArrayHeader,
index: u32,
Expand Down
7 changes: 7 additions & 0 deletions crates/perry-runtime/src/object/array_object_ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 7 additions & 1 deletion crates/perry-runtime/src/typed_feedback.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2598,7 +2598,13 @@ pub extern "C" fn js_typed_feedback_array_index_set_fallback_boxed(
(raw_addr as *const u8).sub(crate::gc::GC_HEADER_SIZE) as *const crate::gc::GcHeader;
match (*gc_header).obj_type {
crate::gc::GC_TYPE_ARRAY | crate::gc::GC_TYPE_LAZY_ARRAY => {
let new_arr = crate::array::js_array_set_index_or_string(
// #9249: this is the cold continuation of a source-level
// assignment after the inline guard rejects a prototype-
// sensitive array. Apply strict Set semantics only to a
// missing canonical index whose default prototype chain may
// carry a descriptor; preserve every other fallback's
// established behavior.
let new_arr = crate::array::js_array_set_index_or_string_inherited_strict(
raw_addr as *mut ArrayHeader,
index,
value,
Expand Down
Loading
Loading