diff --git a/crates/perry-runtime/src/object/alloc.rs b/crates/perry-runtime/src/object/alloc.rs index 9661dfb7a..7a4188101 100644 --- a/crates/perry-runtime/src/object/alloc.rs +++ b/crates/perry-runtime/src/object/alloc.rs @@ -1060,17 +1060,42 @@ pub unsafe extern "C" fn js_object_assign_one(target_f64: f64, source_f64: f64) } } - // 2) Copy own symbol-keyed enumerable properties from source to target. - // The clone-then-iterate dance is non-negotiable — the inner - // `js_object_set_symbol_property` re-acquires SYMBOL_PROPERTIES' - // Mutex; holding the lock across the iteration would deadlock. - let entries = crate::symbol::clone_symbol_entries_for_obj_ptr(src_raw); - for (sym_ptr, value_bits) in entries { + // 2) Copy own symbol-keyed enumerable properties from source to target, + // in `[[OwnPropertyKeys]]` symbol order (after the string keys). Use the + // full own-symbol-key list — `clone_symbol_entries_for_obj_ptr` only + // surfaces symbols with a stored *value*, missing accessor-only symbols + // (`Object.defineProperty(o, sym, { get })`), so a symbol getter never + // ran during assign (test262 assign/strings-and-symbol-order). Snapshot + // the symbol pointers first: the inner `[[Get]]` / set re-acquire + // SYMBOL_PROPERTIES, so iterating a held snapshot avoids re-entrancy. + let sym_keys: Vec = { + let arr_raw = crate::symbol::js_object_get_own_property_symbols(source_f64); + let mut v = Vec::new(); + if arr_raw != 0 { + let arr = arr_raw as *const crate::array::ArrayHeader; + if !arr.is_null() { + let n = crate::array::js_array_length(arr); + for i in 0..n { + let sv = crate::array::js_array_get(arr, i); + let p = (sv.bits() & crate::value::POINTER_MASK) as usize; + if p != 0 { + v.push(p); + } + } + } + } + v + }; + for sym_ptr in sym_keys { if !crate::symbol::symbol_property_is_enumerable(src_raw, sym_ptr) { continue; } let sym_f64 = f64::from_bits(JSValue::pointer(sym_ptr as *const u8).bits()); - let value_f64 = f64::from_bits(value_bits); + // Read the source value through `[[Get]]`, not the raw side-table bits, + // so a symbol-keyed accessor's getter runs during `Object.assign` + // (test262 assign/strings-and-symbol-order). The earlier string-key + // copy already uses `[[Get]]` via `js_object_get_field_by_name`. + let value_f64 = crate::symbol::js_object_get_symbol_property(source_f64, sym_f64); // Strict `Set` semantics for symbol-keyed writes too. { let owner = tgt_raw; diff --git a/crates/perry-runtime/src/object/descriptors.rs b/crates/perry-runtime/src/object/descriptors.rs index 63e9f46c4..ecc5d26db 100644 --- a/crates/perry-runtime/src/object/descriptors.rs +++ b/crates/perry-runtime/src/object/descriptors.rs @@ -1241,13 +1241,43 @@ pub extern "C" fn js_object_get_own_property_descriptors(obj_value: f64) -> f64 let key_val = crate::array::js_array_get(names_arr, i as u32); let key_f64 = f64::from_bits(key_val.bits()); let desc = js_object_get_own_property_descriptor(obj_value, key_f64); + // Spec step: only add the entry when the descriptor is not + // undefined (the key was removed between key-collection and the + // descriptor read, e.g. by a Proxy trap). + if desc.to_bits() == crate::value::TAG_UNDEFINED { + continue; + } let key_str = crate::builtins::js_string_coerce(key_f64); if !key_str.is_null() { js_object_set_field_by_name(result, key_str, desc); } } } - f64::from_bits((result as u64) | POINTER_TAG) + + // [[OwnPropertyKeys]] includes symbol keys after the string keys, and + // `Object.getOwnPropertyDescriptors` must report a descriptor for each + // (including non-enumerable ones). `getOwnPropertyNames` above only + // covers the string subset, so enumerate the symbol keys separately and + // install each descriptor under its symbol key on the result object. + // (test262 getOwnPropertyDescriptors/symbols-included, order-after-*.) + let result_value = f64::from_bits((result as u64) | POINTER_TAG); + let sym_arr_raw = crate::symbol::js_object_get_own_property_symbols(obj_value); + if sym_arr_raw != 0 { + let sym_arr = sym_arr_raw as *const crate::array::ArrayHeader; + if !sym_arr.is_null() { + let slen = crate::array::js_array_length(sym_arr) as usize; + for i in 0..slen { + let sym_val = crate::array::js_array_get(sym_arr, i as u32); + let sym_f64 = f64::from_bits(sym_val.bits()); + let desc = js_object_get_own_property_descriptor(obj_value, sym_f64); + if desc.to_bits() == crate::value::TAG_UNDEFINED { + continue; + } + crate::symbol::js_object_set_symbol_property(result_value, sym_f64, desc); + } + } + } + result_value } } diff --git a/crates/perry-runtime/src/object/field_get_set.rs b/crates/perry-runtime/src/object/field_get_set.rs index 37f5a5045..cb4125b0e 100644 --- a/crates/perry-runtime/src/object/field_get_set.rs +++ b/crates/perry-runtime/src/object/field_get_set.rs @@ -1987,23 +1987,51 @@ pub extern "C" fn js_object_values(obj: *const ObjectHeader) -> *mut ArrayHeader None => j as u32, } }; - // #5046: skip keys a descriptor marks non-enumerable, like - // `js_object_keys` does. Cheap any-descriptor probe first so - // descriptor-free objects stay on the fast path. - let has_descriptors = - PROPERTY_DESCRIPTORS.with(|m| m.borrow().keys().any(|(ptr, _)| *ptr == obj as usize)); + // Snapshot the own key list before reading values, then read each + // through the name-keyed `[[Get]]` so own accessors fire and getter side + // effects don't perturb the key set (mirrors `js_object_entries`). + // + // Two correctness requirements drive this shape: + // * GC safety — a getter fired by `js_object_get_field_by_name` can + // delete a future key and allocate/GC before we visit it. A key kept + // only as a NaN-boxed pointer inside this Rust-heap `Vec` is not a + // stack-visible GC root, so it could dangle. We snapshot the owned + // key *bytes* and rematerialize the string at read time instead. + // * EnumerableOwnProperties — enumerability is determined per key at + // read time, not cached up front: an earlier getter can create a + // descriptor or flip a future key's enumerability, so we defer the + // `descriptor_marks_non_enumerable` check to the read phase. + let mut snapshot_keys: Vec> = Vec::with_capacity(count); + let mut key_buf = [0u8; crate::value::SHORT_STRING_MAX_LEN]; for j in 0..count { let i = pos(j); - if !keys.is_null() && i < crate::array::js_array_length(keys) { - let key_val = crate::array::js_array_get(keys, i); - if instance_private_key_hidden(obj, key_val) { - continue; - } - if has_descriptors && descriptor_marks_non_enumerable(obj, key_val) { - continue; - } + if keys.is_null() || i >= crate::array::js_array_length(keys) { + continue; + } + let key_val = crate::array::js_array_get(keys, i); + if instance_private_key_hidden(obj, key_val) { + continue; + } + if let Some(bytes) = crate::string::js_string_key_bytes(key_val, &mut key_buf) { + snapshot_keys.push(bytes.to_vec()); } - let value = js_object_get_field(obj as *mut ObjectHeader, i); + } + for key_bytes in snapshot_keys { + let key_str = + crate::string::js_string_from_bytes(key_bytes.as_ptr(), key_bytes.len() as u32); + if key_str.is_null() { + continue; + } + // Re-check own + enumerable at read time (a prior getter may have + // removed/hidden the key, or created a descriptor) — see + // `js_object_entries`. + if !super::own_key_present(obj as *mut ObjectHeader, key_str) { + continue; + } + if descriptor_marks_non_enumerable(obj, JSValue::string_ptr(key_str)) { + continue; + } + let value = js_object_get_field_by_name(obj as *const ObjectHeader, key_str); crate::array::js_array_push_f64(result, f64::from_bits(value.bits())); } @@ -2133,38 +2161,66 @@ pub extern "C" fn js_object_entries(obj: *const ObjectHeader) -> *mut ArrayHeade None => j as u32, } }; - // #5046: skip keys a descriptor marks non-enumerable, like - // `js_object_keys` does. Cheap any-descriptor probe first so - // descriptor-free objects stay on the fast path. - let has_descriptors = - PROPERTY_DESCRIPTORS.with(|m| m.borrow().keys().any(|(ptr, _)| *ptr == obj as usize)); + // Spec (EnumerableOwnProperties): the own key list is determined ONCE up + // front, then `[[Get]]` is invoked per key. A getter that adds, removes, + // or hides a future key during enumeration must not change the set of + // entries reported (test262 entries/getter-adding-key, + // getter-removing-future-key, getter-making-future-key-nonenumerable). + // + // Snapshot the own key *bytes* (not NaN-boxed pointers): a getter fired + // by `js_object_get_field_by_name` can delete a future key and + // allocate/GC before we visit it, and a key kept only inside this + // Rust-heap `Vec` is not a stack-visible GC root — it could dangle. + // Owning the bytes and rematerializing the string at read time sidesteps + // that. Enumerability is likewise re-evaluated per key in the read phase + // (an earlier getter can create a descriptor or flip a future key's + // enumerability), so we deliberately do NOT filter it during the snapshot. + let mut snapshot_keys: Vec> = Vec::with_capacity(count); + let mut key_buf = [0u8; crate::value::SHORT_STRING_MAX_LEN]; for j in 0..count { let i = pos(j); - if !keys.is_null() && i < crate::array::js_array_length(keys) { - let key_val = crate::array::js_array_get(keys, i); - if instance_private_key_hidden(obj, key_val) { - continue; - } - if has_descriptors && descriptor_marks_non_enumerable(obj, key_val) { - continue; - } + if keys.is_null() || i >= crate::array::js_array_length(keys) { + continue; } - // Create a pair array [key, value] - let pair = crate::array::js_array_alloc(2); + let key_val = crate::array::js_array_get(keys, i); + if instance_private_key_hidden(obj, key_val) { + continue; + } + if let Some(bytes) = crate::string::js_string_key_bytes(key_val, &mut key_buf) { + snapshot_keys.push(bytes.to_vec()); + } + } - // Get the key (from keys array — already validated non-null - // when count came from there). - if !keys.is_null() && i < crate::array::js_array_length(keys) { - let key = crate::array::js_array_get_f64(keys, i); - crate::array::js_array_push_f64(pair, key); - } else { - crate::array::js_array_push_f64(pair, 0.0); + for key_bytes in snapshot_keys { + let key_str = + crate::string::js_string_from_bytes(key_bytes.as_ptr(), key_bytes.len() as u32); + if key_str.is_null() { + continue; + } + // Spec EnumerableOwnProperties re-reads `[[GetOwnProperty]]` per key + // and skips it when the descriptor is now undefined or no longer + // enumerable — a getter earlier in the loop may have deleted or + // hidden a key that was in the initial snapshot (test262 + // entries/getter-removing-future-key, getter-making-future-key- + // nonenumerable). + if !super::own_key_present(obj as *mut ObjectHeader, key_str) { + continue; } + if descriptor_marks_non_enumerable(obj, JSValue::string_ptr(key_str)) { + continue; + } + // Create a pair array [key, value]. + let pair = crate::array::js_array_alloc(2); + crate::array::js_array_push_f64( + pair, + f64::from_bits(JSValue::string_ptr(key_str).bits()), + ); - // Read the value. `js_object_get_field` handles the - // inline-vs-overflow split internally (inline if - // i < field_count, overflow_get otherwise). - let value = js_object_get_field(obj as *mut ObjectHeader, i); + // Read the value through the name-keyed `[[Get]]`, which fires an + // own accessor's getter (the raw index-based field read returned the + // empty data slot for accessor-defined properties — test262 + // entries/getter-adding-key expected the getter's "B"). + let value = js_object_get_field_by_name(obj as *const ObjectHeader, key_str); crate::array::js_array_push_f64(pair, f64::from_bits(value.bits())); // Push the pair to result (NaN-box the array pointer) diff --git a/crates/perry-runtime/src/symbol.rs b/crates/perry-runtime/src/symbol.rs index 40d971bd6..ef3eae293 100644 --- a/crates/perry-runtime/src/symbol.rs +++ b/crates/perry-runtime/src/symbol.rs @@ -2313,6 +2313,10 @@ pub unsafe extern "C" fn js_object_get_own_property_symbols(obj_f64: f64) -> i64 .cloned() .unwrap_or_default(); drop(guard); + // `entries[..data_len]` are the data-valued symbol properties from + // `SYMBOL_PROPERTIES`, already in their true insertion order. Everything + // appended after `data_len` is an accessor-only symbol. + let data_len = entries.len(); for sym_key in accessors::owner_symbol_accessor_keys(obj_key) { if !entries.iter().any(|(existing, _)| *existing == sym_key) { entries.push((sym_key, 0)); @@ -2321,6 +2325,25 @@ pub unsafe extern "C" fn js_object_get_own_property_symbols(obj_f64: f64) -> i64 if entries.is_empty() { return crate::array::js_array_alloc(0) as i64; } + // `[[OwnPropertyKeys]]` reports symbol keys in property-creation order. + // Data-valued symbols already arrive in insertion order, so we must NOT + // reorder them (an unconditional sort by creation id would reorder e.g. + // `obj[b]=…; obj[a]=…` when `a` was created before `b`). Accessor-only + // symbols, however, are appended from a HashMap (`owner_symbol_accessor_keys`) + // in nondeterministic order, so a `defineProperty(o, sym, {get})` pair came + // out unstable (test262 assign/strings-and-symbol-order, + // getOwnPropertyDescriptors/order-after-define-property). Sort ONLY that + // appended accessor-only tail by the symbol's monotonic creation id (the + // convention the class-ref symbol path already uses), leaving the data-symbol + // insertion order intact. + entries[data_len..].sort_by_key(|(sym_ptr_usize, _)| { + let ptr = *sym_ptr_usize as *const SymbolHeader; + if ptr.is_null() { + u64::MAX + } else { + (*ptr).id + } + }); let mut arr = crate::array::js_array_alloc(entries.len() as u32); for (sym_ptr_usize, _val_bits) in entries.iter() { // Re-NaN-box each symbol pointer with POINTER_TAG so the array