fix(runtime): own-key order + observable trap order for Object enumeration; array symbol-keyed properties (#5901)

changelog.d/7467-object-enumeration-order.md

@@ -0,0 +1,27 @@
+fix(runtime): own-property enumeration order + observable trap order for `Object` statics; array symbol-keyed properties (#5901, PR #7467)
+
+- Symbol keys now keep property-**creation** order across a data→accessor
+  `defineProperty` redefine, and for accessors installed between two data
+  installs: `set_symbol_accessor_property` leaves an order-preserving
+  placeholder in `SYMBOL_PROPERTIES` (value readers all consult the accessor
+  table first; `clone_symbol_entries_for_obj_ptr` filters placeholders for
+  the raw-entry consumers). test262:
+  `getOwnPropertySymbols/order-after-define-property`.
+- `Object.values` / `Object.entries` on a Proxy fire one `ownKeys` trap, then
+  interleave `getOwnPropertyDescriptor` + `get` per key per
+  EnumerableOwnPropertyNames, instead of batching all descriptor reads first.
+  test262: `values/observable-operations`, `entries/observable-operations`.
+- `Object.getOwnPropertyDescriptors` on a Proxy fires `ownKeys` once (the
+  generic string/symbol two-helper enumeration fired an observable second
+  trap) and reads descriptors in the trap result's verbatim key order.
+  test262: `getOwnPropertyDescriptors/observable-operations`.
+- Arrays support symbol-keyed properties: `arr[sym] = v` was silently
+  dropped (no symbol arm in `js_array_set_index_or_string`) and `arr[sym]`
+  hard-returned `undefined`; both now route through the symbol side table
+  like plain-object receivers.
+
+Validation: new sabotage-verified unit test
+(`symbol_keys_keep_creation_order_across_accessor_redefine`); perry-runtime
+`--lib` 1655/1655; test262 `built-ins/Object` slice 3141→3149 pass with only
+removals in the failure diff; `built-ins/Array` slice swept — remaining
+failures all predate the change (#5898 snapshot cross-check).

crates/perry-runtime/src/array/indexing.rs

@@ -1759,7 +1759,20 @@ pub extern "C" fn js_array_get_index_or_string(arr: *const ArrayHeader, idx: f64
     }
 
     if unsafe { crate::symbol::js_is_symbol(idx) } != 0 {
-        return f64::from_bits(crate::value::TAG_UNDEFINED);
+        // Symbol-keyed read on an array: `arr[sym] = v` stores into the
+        // symbol side table keyed by the header address (write arm in
+        // `js_array_set_index_or_string`), so read it back through the
+        // standard symbol getter — which also serves an accessor installed
+        // via `defineProperty(arr, sym, {get})`. This used to hard-return
+        // `undefined`, making every stored symbol property unreadable
+        // (test262 getOwnPropertySymbols/order-after-define-property,
+        // Array-receiver half).
+        return unsafe {
+            crate::symbol::js_object_get_symbol_property(
+                crate::value::js_nanbox_pointer(arr as i64),
+                idx,
+            )
+        };
     }
     // #6935: read-side sibling of `js_array_set_index_or_string` below —
     // `js_jsvalue_to_string` on an object key (`a[new Number(1)]`,
@@ -1848,13 +1861,34 @@ pub extern "C" fn js_array_set_index_or_string(
         }
         return arr_handle.get_raw_mut_ptr::<ArrayHeader>();
     }
+    // Symbol-keyed write: store through the symbol side table (keyed by the
+    // header address), exactly like a plain-object receiver. This arm used to
+    // be missing — a symbol key fell past the string fallback below (guarded
+    // `js_is_symbol == 0`) to the final bare return, so the write was
+    // silently DROPPED and `arr[sym]` / `getOwnPropertySymbols(arr)` saw
+    // nothing (test262 getOwnPropertySymbols/order-after-define-property,
+    // Array-receiver half).
+    if unsafe { crate::symbol::js_is_symbol(idx) } != 0 {
+        // The store can run a user setter (symbol accessor installed on the
+        // array), which can GC and evacuate the receiver.
+        let scope = crate::gc::RuntimeHandleScope::new();
+        let arr_handle = scope.root_raw_mut_ptr(arr);
+        unsafe {
+            crate::symbol::js_object_set_symbol_property(
+                crate::value::js_nanbox_pointer(arr as i64),
+                idx,
+                value,
+            );
+        }
+        return arr_handle.get_raw_mut_ptr::<ArrayHeader>();
+    }
     // Fallback for a NON-numeric key: a primitive (`a[null]`, `a[undefined]`,
     // `a[true]`, `a[10n]`) or a boxed object (`a[new Number(1)]`). Per
     // ToPropertyKey these become string property keys (or, for `10n`, the
     // canonical index "10"); `js_array_set_string_key` routes accordingly.
     // Arrays previously DROPPED these writes (plain objects handled them).
     // Restricted to `numeric.is_none()`: numeric keys (including non-integer
-    // finite floats) are handled above. Symbols stay symbol-keyed.
+    // finite floats) are handled above. Symbols are handled by the arm above.
     //
     // #6935: this is the boxed-object arm the doc comment above names, so
     // `js_jsvalue_to_string` here runs a USER `toString` / `valueOf` — allocate

crates/perry-runtime/src/object/descriptors.rs

@@ -1385,6 +1385,66 @@ pub extern "C" fn js_object_get_own_property_names(obj_value: f64) -> f64 {
     }
 }
 
+/// `Object.getOwnPropertyDescriptors` for a Proxy receiver: one `ownKeys`
+/// trap, then the per-key `getOwnPropertyDescriptor` trap reads in the trap's
+/// verbatim key order (strings and symbols interleaved as returned). Split out
+/// of the generic path so a proxy never observes the extra `ownKeys` the
+/// two-helper enumeration there would fire.
+unsafe fn proxy_get_own_property_descriptors(obj_value: f64) -> f64 {
+    const POINTER_TAG: u64 = 0x7FFD_0000_0000_0000;
+    // The per-key descriptor read runs a user trap that can GC, so the
+    // receiver, key list, result object, and per-iteration key/descriptor all
+    // live in handles (same discipline as the generic path below).
+    let scope = crate::gc::RuntimeHandleScope::new();
+    let obj_handle = scope.root_nanbox_f64(obj_value);
+    let keys_boxed = crate::proxy::js_proxy_own_keys(obj_value);
+    let keys_arr =
+        (keys_boxed.to_bits() & crate::value::POINTER_MASK) as *mut crate::array::ArrayHeader;
+    let keys_handle = scope.root_raw_mut_ptr(keys_arr);
+    let result_handle = scope.root_raw_mut_ptr(js_object_alloc(0, 0));
+    let key_handle = scope.root_nanbox_f64(f64::from_bits(crate::value::TAG_UNDEFINED));
+    let desc_handle = scope.root_nanbox_f64(f64::from_bits(crate::value::TAG_UNDEFINED));
+    let len =
+        crate::array::js_array_length(keys_handle.get_raw_const_ptr::<crate::array::ArrayHeader>());
+    for i in 0..len {
+        let key_val = crate::array::js_array_get(
+            keys_handle.get_raw_const_ptr::<crate::array::ArrayHeader>(),
+            i,
+        );
+        key_handle.set_nanbox_u64(key_val.bits());
+        let desc = js_object_get_own_property_descriptor(
+            obj_handle.get_nanbox_f64(),
+            key_handle.get_nanbox_f64(),
+        );
+        // Spec step: skip keys whose descriptor read comes back undefined
+        // (removed by the trap between key collection and this read).
+        if desc.to_bits() == crate::value::TAG_UNDEFINED {
+            continue;
+        }
+        desc_handle.set_nanbox_f64(desc);
+        if crate::symbol::js_is_symbol(key_handle.get_nanbox_f64()) != 0 {
+            let result_value = f64::from_bits(
+                (result_handle.get_raw_mut_ptr::<ObjectHeader>() as u64) | POINTER_TAG,
+            );
+            crate::symbol::js_object_set_symbol_property(
+                result_value,
+                key_handle.get_nanbox_f64(),
+                desc_handle.get_nanbox_f64(),
+            );
+        } else {
+            let key_str = crate::builtins::js_string_coerce(key_handle.get_nanbox_f64());
+            if !key_str.is_null() {
+                js_object_set_field_by_name(
+                    result_handle.get_raw_mut_ptr::<ObjectHeader>(),
+                    key_str,
+                    desc_handle.get_nanbox_f64(),
+                );
+            }
+        }
+    }
+    f64::from_bits((result_handle.get_raw_mut_ptr::<ObjectHeader>() as u64) | POINTER_TAG)
+}
+
 /// Object.getOwnPropertyDescriptors(obj) — returns a new object whose own
 /// property keys (the same set `Object.getOwnPropertyNames` reports, including
 /// non-enumerable keys and class-ref method names) each map to the property
@@ -1399,6 +1459,15 @@ pub extern "C" fn js_object_get_own_property_names(obj_value: f64) -> f64 {
 pub extern "C" fn js_object_get_own_property_descriptors(obj_value: f64) -> f64 {
     const POINTER_TAG: u64 = 0x7FFD_0000_0000_0000;
     unsafe {
+        // A Proxy receiver gets its own arm: the spec performs ONE
+        // [[OwnPropertyKeys]] (`ownKeys` trap), then a [[GetOwnProperty]]
+        // (`getOwnPropertyDescriptor` trap) per key. The generic path below
+        // enumerates string and symbol keys through two separate helpers,
+        // each firing its own `ownKeys` trap — an observably extra call
+        // (test262 getOwnPropertyDescriptors/observable-operations).
+        if crate::proxy::js_proxy_is_proxy(obj_value) != 0 {
+            return proxy_get_own_property_descriptors(obj_value);
+        }
         // Enumerate own keys exactly like Object.getOwnPropertyNames — this
         // handles class refs and plain objects, and includes non-enumerable
         // keys, matching the spec's [[OwnPropertyKeys]] string-key set.

crates/perry-runtime/src/object/field_get_set/enumeration.rs

@@ -468,23 +468,72 @@ fn for_each_string_char<F: FnMut(u32, f64)>(value: f64, mut emit: F) -> Option<u
 /// handle-band payload, not an address — either got dereferenced (SIGSEGV) or,
 /// once the handle-band guard rejected it, silently reported no properties.
 unsafe fn proxy_values_or_entries(value: f64, want_pairs: bool) -> *mut ArrayHeader {
-    let keys_boxed = crate::proxy::proxy_enum_own_keys(value);
+    // EnumerableOwnPropertyNames(O, value / key+value) on a Proxy: ONE
+    // `ownKeys` trap, then — per string key — `getOwnPropertyDescriptor`
+    // followed immediately by `get` when the descriptor is enumerable. The
+    // traps must interleave per key (test262 values/entries
+    // observable-operations); routing through `proxy_enum_own_keys` batched
+    // every descriptor read before the first `get`
+    // (|gOPD:a|gOPD:b|gOPD:c|get:a|…).
+    //
+    // Both trap calls run user code that can GC, so the receiver, the key
+    // list, the result array, and the per-iteration key/value all live in
+    // handles and are re-read after each call.
+    let scope = crate::gc::RuntimeHandleScope::new();
+    let recv_h = scope.root_nanbox_f64(value);
+    let keys_boxed = crate::proxy::js_proxy_own_keys(value);
     let keys_arr = (keys_boxed.to_bits() & crate::value::POINTER_MASK) as *mut ArrayHeader;
-    let len = crate::array::js_array_length(keys_arr);
-    let mut out = crate::array::js_array_alloc(len.max(1) as u32);
+    let keys_h = scope.root_raw_mut_ptr(keys_arr);
+    let len = crate::array::js_array_length(keys_h.get_raw_const_ptr::<ArrayHeader>());
+    let out_h = scope.root_raw_mut_ptr(crate::array::js_array_alloc(len.max(1) as u32));
+    let key_h = scope.root_nanbox_f64(f64::from_bits(crate::value::TAG_UNDEFINED));
+    // Allocated once and rewritten per iteration so an N-key proxy doesn't
+    // push N slots onto the handle stack (same discipline as
+    // `js_object_get_own_property_descriptors`).
+    let val_h = scope.root_nanbox_f64(f64::from_bits(crate::value::TAG_UNDEFINED));
     for i in 0..len {
-        let key = crate::array::js_array_get(keys_arr, i);
-        let val = crate::proxy::js_proxy_get(value, f64::from_bits(key.bits()));
+        let key = crate::array::js_array_get(keys_h.get_raw_const_ptr::<ArrayHeader>(), i);
+        if !key.is_any_string() {
+            continue; // symbol keys are excluded from values/entries
+        }
+        key_h.set_nanbox_u64(key.bits());
+        let desc = crate::proxy::js_reflect_get_own_property_descriptor(
+            recv_h.get_nanbox_f64(),
+            key_h.get_nanbox_f64(),
+        );
+        if desc.to_bits() == crate::value::TAG_UNDEFINED {
+            continue;
+        }
+        let desc_ptr = (desc.to_bits() & crate::value::POINTER_MASK) as *const ObjectHeader;
+        if desc_ptr.is_null() {
+            continue;
+        }
+        let ek = crate::string::js_string_from_bytes(b"enumerable".as_ptr(), 10);
+        if crate::value::js_is_truthy(crate::object::js_object_get_field_by_name_f64(desc_ptr, ek))
+            == 0
+        {
+            continue;
+        }
+        let val = crate::proxy::js_proxy_get(recv_h.get_nanbox_f64(), key_h.get_nanbox_f64());
+        val_h.set_nanbox_f64(val);
         if want_pairs {
             let pair = crate::array::js_array_alloc(2);
-            let pair = crate::array::js_array_push(pair, key);
-            let pair = crate::array::js_array_push_f64(pair, val);
-            out = crate::array::js_array_push(out, JSValue::array_ptr(pair));
+            let pair = crate::array::js_array_push_f64(pair, key_h.get_nanbox_f64());
+            let pair = crate::array::js_array_push_f64(pair, val_h.get_nanbox_f64());
+            let pushed = crate::array::js_array_push(
+                out_h.get_raw_mut_ptr::<ArrayHeader>(),
+                JSValue::array_ptr(pair),
+            );
+            out_h.set_raw_mut_ptr(pushed);
         } else {
-            out = crate::array::js_array_push_f64(out, val);
+            let pushed = crate::array::js_array_push_f64(
+                out_h.get_raw_mut_ptr::<ArrayHeader>(),
+                val_h.get_nanbox_f64(),
+            );
+            out_h.set_raw_mut_ptr(pushed);
         }
     }
-    out
+    out_h.get_raw_mut_ptr::<ArrayHeader>()
 }
 
 /// Tag-dispatching `Object.values(value)` — see [`js_object_keys_value`].

crates/perry-runtime/src/object/tests.rs

@@ -539,6 +539,85 @@ fn symbol_define_property_attrs_round_trip_descriptor() {
     }
 }
 
+#[test]
+fn symbol_keys_keep_creation_order_across_accessor_redefine() {
+    // `[[OwnPropertyKeys]]` reports symbol keys in property-CREATION order. A
+    // data→accessor redefine must not move the key to the end (test262
+    // getOwnPropertySymbols/order-after-define-property), and an accessor
+    // installed BETWEEN two data installs must enumerate at its install
+    // position — both rest on the order-preserving placeholder that
+    // `set_symbol_accessor_property` leaves in `SYMBOL_PROPERTIES`.
+    let _global = crate::gc::global_side_table_test_lock();
+    crate::symbol::test_clear_symbol_side_table_roots();
+    unsafe {
+        let own_symbol_order = |obj_value: f64| -> Vec<usize> {
+            let arr = crate::symbol::js_object_get_own_property_symbols(obj_value)
+                as *const crate::array::ArrayHeader;
+            assert!(!arr.is_null());
+            let n = crate::array::js_array_length(arr);
+            (0..n)
+                .map(|i| {
+                    (crate::array::js_array_get(arr, i).bits() & crate::value::POINTER_MASK)
+                        as usize
+                })
+                .collect()
+        };
+        let getter_descriptor = || -> f64 {
+            let getter = crate::closure::js_closure_alloc(closure_accessor_getter as *const u8, 0);
+            assert!(!getter.is_null());
+            let get_key = crate::string::js_string_from_bytes(b"get".as_ptr(), 3);
+            let descriptor = js_object_alloc(0, 0);
+            assert!(!descriptor.is_null());
+            js_object_set_field_by_name(
+                descriptor,
+                get_key,
+                crate::value::js_nanbox_pointer(getter as i64),
+            );
+            crate::value::js_nanbox_pointer(descriptor as i64)
+        };
+
+        // Data → accessor redefine keeps the key's position.
+        let obj = js_object_alloc(0, 0);
+        assert!(!obj.is_null());
+        let obj_value = crate::value::js_nanbox_pointer(obj as i64);
+        let sym_a = crate::symbol::js_symbol_new_empty();
+        let sym_b = crate::symbol::js_symbol_new_empty();
+        let a_ptr = crate::symbol::sym_key_from_f64(sym_a);
+        let b_ptr = crate::symbol::sym_key_from_f64(sym_b);
+        crate::symbol::js_object_set_symbol_property(obj_value, sym_a, 1.0);
+        crate::symbol::js_object_set_symbol_property(obj_value, sym_b, 2.0);
+        js_object_define_property(obj_value, sym_a, getter_descriptor());
+        assert_eq!(
+            own_symbol_order(obj_value),
+            vec![a_ptr, b_ptr],
+            "data→accessor redefine moved the key out of creation order"
+        );
+        // The placeholder must never serve as the value — the read goes
+        // through the accessor table and runs the getter.
+        let read = crate::symbol::js_object_get_symbol_property(obj_value, sym_a);
+        assert_eq!(read.to_bits(), 4.0f64.to_bits());
+
+        // Accessor installed between two data installs enumerates in place.
+        let obj2 = js_object_alloc(0, 0);
+        assert!(!obj2.is_null());
+        let obj2_value = crate::value::js_nanbox_pointer(obj2 as i64);
+        let sym_c = crate::symbol::js_symbol_new_empty();
+        let sym_d = crate::symbol::js_symbol_new_empty();
+        let sym_e = crate::symbol::js_symbol_new_empty();
+        let c_ptr = crate::symbol::sym_key_from_f64(sym_c);
+        let d_ptr = crate::symbol::sym_key_from_f64(sym_d);
+        let e_ptr = crate::symbol::sym_key_from_f64(sym_e);
+        crate::symbol::js_object_set_symbol_property(obj2_value, sym_c, 1.0);
+        js_object_define_property(obj2_value, sym_d, getter_descriptor());
+        crate::symbol::js_object_set_symbol_property(obj2_value, sym_e, 3.0);
+        assert_eq!(
+            own_symbol_order(obj2_value),
+            vec![c_ptr, d_ptr, e_ptr],
+            "interleaved accessor install enumerated out of creation order"
+        );
+    }
+}
+
 #[test]
 fn test_object_alloc_and_fields() {
     let obj = js_object_alloc(1, 3);

crates/perry-runtime/src/symbol/accessors.rs

@@ -46,11 +46,27 @@ pub(crate) unsafe fn set_symbol_accessor_property(
     }
     crate::symbol::note_symbol_key_installed(sym_key);
     {
+        // `SYMBOL_PROPERTIES` is the only insertion-ordered record of symbol
+        // property CREATION order, which `[[OwnPropertyKeys]]` must report
+        // (test262 getOwnPropertySymbols/order-after-define-property).
+        // Removing the data entry on a data→accessor redefine — or never
+        // adding one for a fresh accessor install — destroys that position,
+        // so the key re-enumerated at the end (or in creation-id order, which
+        // is not install order). Keep an order-preserving placeholder instead:
+        // same key, TAG_UNDEFINED value bits so the old data value stops
+        // being rooted. Readers never mistake it for a data value — get, set,
+        // gOPD and has-own all consult `SYMBOL_ACCESSOR_PROPERTIES` first,
+        // and `clone_symbol_entries_for_obj_ptr` filters accessor-keyed
+        // entries out for the raw-entry consumers (formatting, freeze/seal).
         let mut props = crate::gc::lock_gc_root_registry(&SYMBOL_PROPERTIES);
-        if let Some(map) = props.as_mut() {
-            if let Some(entries) = map.get_mut(&obj_key) {
-                entries.retain(|(key, _)| *key != sym_key);
-            }
+        if props.is_none() {
+            *props = Some(HashMap::new());
+        }
+        let entries = props.as_mut().unwrap().entry(obj_key).or_default();
+        if let Some(entry) = entries.iter_mut().find(|entry| entry.0 == sym_key) {
+            entry.1 = crate::value::TAG_UNDEFINED;
+        } else {
+            entries.push((sym_key, crate::value::TAG_UNDEFINED));
         }
     }
     {