fix(object): symbol keys + accessor [[Get]] in descriptor/enumeration engine

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<usize> = {
+        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;

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
     }
 }
 

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<u8>> = 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<u8>> = 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)

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