From 083acc82e7809b08720972468ff8c5e4feda6b39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 18 Aug 2026 09:29:29 +0200 Subject: [PATCH 1/4] fix(runtime): complete ObjectDefineProperties key semantics --- .../object/object_ops/define_properties.rs | 163 ++++++++++-------- .../test_gap_5901_define_properties_keys.ts | 47 +++++ 2 files changed, 134 insertions(+), 76 deletions(-) create mode 100644 test-files/test_gap_5901_define_properties_keys.ts diff --git a/crates/perry-runtime/src/object/object_ops/define_properties.rs b/crates/perry-runtime/src/object/object_ops/define_properties.rs index c95323e3cf..d5f9d57908 100644 --- a/crates/perry-runtime/src/object/object_ops/define_properties.rs +++ b/crates/perry-runtime/src/object/object_ops/define_properties.rs @@ -29,33 +29,53 @@ pub extern "C" fn js_object_define_properties(target: f64, descriptors: f64) -> if !target_is_class_ref && !target_is_handle && !unsafe { value_is_object_like(target) } { throw_object_type_error(b"Object.defineProperties called on non-object"); } + // ToObject(Properties) below may allocate a primitive wrapper. Root both + // inputs first so a moving collection cannot leave the target or a heap + // string primitive stale before the main algorithm starts. + let scope = crate::gc::RuntimeHandleScope::new(); + let target_handle = scope.root_nanbox_f64(target); + let descriptors_input_handle = scope.root_nanbox_f64(descriptors); // #2817: the properties bag must be coercible to an object. Node throws // `Cannot convert undefined or null to object` for null/undefined, and // primitives are boxed (no own enumerable keys → no-op). Match the nullish // case explicitly. - { + let descriptors = { + let descriptors = descriptors_input_handle.get_nanbox_f64(); let jv = crate::value::JSValue::from_bits(descriptors.to_bits()); if jv.is_undefined() || jv.is_null() { throw_object_type_error(b"Cannot convert undefined or null to object"); } - } + // ObjectDefineProperties step 2 is ToObject(Properties), not merely a + // nullish check. In particular, a non-empty string becomes a String + // exotic with enumerable index keys; reading its first descriptor + // value then fails ToPropertyDescriptor because that value is a + // primitive character. Other primitive wrappers have no enumerable + // own keys and are a no-op. Preserve class refs (INT32-tagged + // constructor objects) rather than boxing them as Numbers. + if super::super::class_ref_id(descriptors).is_some() + || crate::proxy::js_proxy_is_proxy(descriptors) != 0 + || unsafe { value_is_object_like(descriptors) } + { + descriptors + } else { + super::super::js_object_coerce(descriptors) + } + }; + let descriptors_handle = scope.root_nanbox_f64(descriptors); let desc_obj = unsafe { extract_obj_ptr(descriptors) }; - if desc_obj.is_null() || !is_valid_obj_ptr(desc_obj as *const u8) { - return target; + if crate::proxy::js_proxy_is_proxy(descriptors) == 0 + && (desc_obj.is_null() || !is_valid_obj_ptr(desc_obj as *const u8)) + { + return target_handle.get_nanbox_f64(); } // #7949: everything below spans allocations — `propertyIsEnumerable` and - // the `[[Get]]` can run user accessors, `str_from_value` coerces (and so - // allocates for every key shape except an already-heap string), and + // the `[[Get]]` can run user accessors, key coercion can allocate, and // `js_object_define_property` grows the target. The receiver, the - // properties bag, the own-names array and the collected key list are all + // properties bag, the own-keys array and the collected key list are all // rooted for the duration, and each is re-read out of its root after every // call that could have moved it. A bare `Vec` of keys is invisible to // every scanner, so under an evacuating collection the second loop used to // define properties under stale key strings. - let scope = crate::gc::RuntimeHandleScope::new(); - let target_handle = scope.root_nanbox_f64(target); - let descriptors_handle = scope.root_nanbox_f64(descriptors); - // Snapshot the descriptor object's own keys array. We collect into a // rooted list first so adding properties via `js_object_define_property` // (which can resize the target's keys_array) can't perturb iteration @@ -68,28 +88,36 @@ pub extern "C" fn js_object_define_properties(target: f64, descriptors: f64) -> // (so accessors on the properties bag run). Using the full own-key set is // wrong for native namespaces like `Math` (whose `E`/`PI`/... are // non-enumerable) and for any object with non-enumerable own props. - let names_handle = scope.root_nanbox_f64(js_object_get_own_property_names( - descriptors_handle.get_nanbox_f64(), - )); let mut keys = crate::gc::RootedValues::new(&scope); - let names_len = { - let names_arr = crate::value::js_nanbox_get_pointer(names_handle.get_nanbox_f64()) - as *const crate::array::ArrayHeader; - if names_arr.is_null() { - 0 - } else { - crate::array::js_array_length(names_arr) as usize - } + // A Proxy must observe exactly one [[OwnPropertyKeys]] call, and its + // returned string/Symbol order is used verbatim. Asking + // getOwnPropertyNames and getOwnPropertySymbols separately would fire the + // trap twice; the names helper also filters via [[GetOwnProperty]] before + // this algorithm can perform its own observable descriptor read. + let descriptors_is_proxy = + crate::proxy::js_proxy_is_proxy(descriptors_handle.get_nanbox_f64()) != 0; + let names = if descriptors_is_proxy { + crate::proxy::js_proxy_own_keys(descriptors_handle.get_nanbox_f64()) + } else { + js_object_get_own_property_names(descriptors_handle.get_nanbox_f64()) + }; + let names_handle = scope.root_nanbox_f64(names); + let names_arr = crate::value::js_nanbox_get_pointer(names_handle.get_nanbox_f64()) + as *const crate::array::ArrayHeader; + let names_len = if names_arr.is_null() { + 0 + } else { + crate::array::js_array_length(names_arr) as usize }; + const TAG_TRUE: u64 = 0x7FFC_0000_0000_0004; for i in 0..names_len { let names_arr = crate::value::js_nanbox_get_pointer(names_handle.get_nanbox_f64()) as *const crate::array::ArrayHeader; - let k = crate::array::js_array_get(names_arr, i as u32); - let k_handle = scope.root_nanbox_f64(f64::from_bits(k.bits())); + let k = crate::array::js_array_get_f64(names_arr, i as u32); + let k_handle = scope.root_nanbox_f64(k); // Skip non-enumerable own keys (spec step: descriptor must be // enumerable). `propertyIsEnumerable` returns false for absent or // non-enumerable keys. - const TAG_TRUE: u64 = 0x7FFC_0000_0000_0004; let enumerable = js_object_property_is_enumerable( descriptors_handle.get_nanbox_f64(), k_handle.get_nanbox_f64(), @@ -98,6 +126,34 @@ pub extern "C" fn js_object_define_properties(target: f64, descriptors: f64) -> keys.push(k_handle.get_nanbox_f64()); } } + // OrdinaryOwnPropertyKeys orders Symbols after all string keys. The Proxy + // arm above already received both kinds in one array, so only ordinary + // descriptor bags need the second source appended here. + if !descriptors_is_proxy { + let symbols = unsafe { + crate::symbol::js_object_get_own_property_symbols(descriptors_handle.get_nanbox_f64()) + }; + if symbols != 0 { + let symbols_handle = scope.root_raw_mut_ptr(symbols as *mut crate::array::ArrayHeader); + let symbols_len = crate::array::js_array_length( + symbols_handle.get_raw_const_ptr::(), + ); + for i in 0..symbols_len { + let symbol = crate::array::js_array_get_f64( + symbols_handle.get_raw_const_ptr::(), + i, + ); + let symbol_handle = scope.root_nanbox_f64(symbol); + let enumerable = js_object_property_is_enumerable( + descriptors_handle.get_nanbox_f64(), + symbol_handle.get_nanbox_f64(), + ); + if enumerable.to_bits() == TAG_TRUE { + keys.push(symbol_handle.get_nanbox_f64()); + } + } + } + } for i in 0..keys.len() { // Read the descriptor through `[[Get]]` so accessors on the properties // bag are honored, then ToPropertyDescriptor + DefinePropertyOrThrow. @@ -107,22 +163,13 @@ pub extern "C" fn js_object_define_properties(target: f64, descriptors: f64) -> // and may be ANY object — a Date, array, boxed primitive, class // instance, etc. `Object.create({}, new Date(0))` previously bit-cast the // Date's `DateCell` pointer to an `ObjectHeader` and segfaulted. The - // dynamic getter dispatches on the receiver's real type. - let key_str_handle = scope.root_nanbox_f64(box_string_ptr(str_from_value(keys.get(i)))); + // property-key getter dispatches on the receiver's real type and keeps + // Symbol keys intact. let descriptor = unsafe { - let key_str = unbox_string_ptr(key_str_handle.get_nanbox_f64()); - if key_str.is_null() { - f64::from_bits(crate::value::TAG_UNDEFINED) - } else { - let name_ptr = - (key_str as *const u8).add(std::mem::size_of::()); - let name_len = (*key_str).byte_len as usize; - crate::value::js_dynamic_object_get_property( - descriptors_handle.get_nanbox_f64(), - name_ptr as *const i8, - name_len, - ) - } + super::super::js_object_get_property_key( + descriptors_handle.get_nanbox_f64(), + keys.get(i), + ) }; let descriptor_handle = scope.root_nanbox_f64(descriptor); js_object_define_property( @@ -134,42 +181,6 @@ pub extern "C" fn js_object_define_properties(target: f64, descriptors: f64) -> target_handle.get_nanbox_f64() } -/// NaN-box a coerced key string so a `RuntimeHandleScope` can root it (the -/// handle stack rewrites STRING_TAG slots on evacuation). A null pointer boxes -/// as `undefined`, which [`unbox_string_ptr`] maps back to null. -fn box_string_ptr(ptr: *const crate::string::StringHeader) -> f64 { - if ptr.is_null() { - f64::from_bits(crate::value::TAG_UNDEFINED) - } else { - f64::from_bits(0x7FFF_0000_0000_0000 | (ptr as u64 & 0x0000_FFFF_FFFF_FFFF)) - } -} - -/// Inverse of [`box_string_ptr`]. Read this immediately before the use — the -/// pointer is a copy, and the next allocation can move the string. -fn unbox_string_ptr(value: f64) -> *const crate::string::StringHeader { - let bits = value.to_bits(); - if bits >> 48 == 0x7FFF { - (bits & 0x0000_FFFF_FFFF_FFFF) as *const crate::string::StringHeader - } else { - std::ptr::null() - } -} - -/// Coerce an arbitrary key value (f64 — usually a STRING_TAG NaN-box) to a -/// `*const StringHeader` for use with `js_object_get_field_by_name_f64`. -/// Returns null if the value isn't string-like. -fn str_from_value(v: f64) -> *const crate::string::StringHeader { - let bits = v.to_bits(); - let top = bits >> 48; - if top == 0x7FFF { - (bits & 0x0000_FFFF_FFFF_FFFF) as *const crate::string::StringHeader - } else { - // Try to coerce (handles number keys, etc.). - crate::builtins::js_string_coerce(v) as *const crate::string::StringHeader - } -} - /// `Object.setPrototypeOf(obj, proto)` — chalk's callable-with-getter-bag /// foundation. Perry's runtime bakes class IDs at allocation time (it /// walks `parent_class_id` for INT32-tagged class refs), so we cannot diff --git a/test-files/test_gap_5901_define_properties_keys.ts b/test-files/test_gap_5901_define_properties_keys.ts new file mode 100644 index 0000000000..895d395d7f --- /dev/null +++ b/test-files/test_gap_5901_define_properties_keys.ts @@ -0,0 +1,47 @@ +// #5901: ObjectDefineProperties must ToObject-box a primitive properties bag, +// collect String and Symbol keys in [[OwnPropertyKeys]] order, and perform the +// descriptor bag's observable [[GetOwnProperty]] / [[Get]] operations. + +function outcome(fn: () => void): string { + try { + fn(); + return "ok"; + } catch (error: any) { + return error.name; + } +} + +console.log("primitive-empty", outcome(() => Object.defineProperties({}, true as any))); +console.log("primitive-string", outcome(() => Object.defineProperties({}, "hello" as any))); +console.log("create-string", outcome(() => Object.create({}, "hello" as any))); + +const symbolKey = Symbol("descriptor"); +const symbolBag: any = {}; +symbolBag[symbolKey] = { value: 42, enumerable: true }; +const symbolTarget: any = {}; +Object.defineProperties(symbolTarget, symbolBag); +console.log("symbol", symbolTarget[symbolKey], Reflect.ownKeys(symbolTarget).length); +const hiddenSymbol = Symbol("hidden"); +Object.defineProperty(symbolBag, hiddenSymbol, { + value: { value: 99 }, + enumerable: false, +}); +const hiddenSymbolTarget: any = {}; +Object.defineProperties(hiddenSymbolTarget, symbolBag); +console.log("hidden-symbol", Reflect.ownKeys(hiddenSymbolTarget).length); + +const proxyLog: PropertyKey[] = []; +const proxyTarget: any = { 0: 1, foo: 2 }; +const proxySymbol = Symbol("proxy"); +proxyTarget[proxySymbol] = 3; +const proxyBag = new Proxy(proxyTarget, { + getOwnPropertyDescriptor(_target, key) { + proxyLog.push(key); + return undefined; + }, +}); +Object.defineProperties({}, proxyBag); +console.log( + "proxy-order", + proxyLog.map((key) => typeof key === "symbol" ? key.toString() : key).join("|"), +); From b75f723b48fda567e143c8fc4d045bd782ec3e03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 18 Aug 2026 09:31:45 +0200 Subject: [PATCH 2/4] docs(changelog): note ObjectDefineProperties fix --- changelog.d/8352-object-define-properties.md | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelog.d/8352-object-define-properties.md diff --git a/changelog.d/8352-object-define-properties.md b/changelog.d/8352-object-define-properties.md new file mode 100644 index 0000000000..bdb7c39a39 --- /dev/null +++ b/changelog.d/8352-object-define-properties.md @@ -0,0 +1,3 @@ +Fixed `Object.defineProperties` and the descriptor form of `Object.create` to +box primitive property bags, preserve enumerable symbol keys, and observe a +Proxy's single `ownKeys` result in specification order. From 67bdb117310a0139a57a2727941e491255b77a5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 18 Aug 2026 10:56:31 +0200 Subject: [PATCH 3/4] fix(runtime): satisfy raw handle gate --- .../src/object/object_ops/define_properties.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/crates/perry-runtime/src/object/object_ops/define_properties.rs b/crates/perry-runtime/src/object/object_ops/define_properties.rs index d5f9d57908..264ef47051 100644 --- a/crates/perry-runtime/src/object/object_ops/define_properties.rs +++ b/crates/perry-runtime/src/object/object_ops/define_properties.rs @@ -135,14 +135,13 @@ pub extern "C" fn js_object_define_properties(target: f64, descriptors: f64) -> }; if symbols != 0 { let symbols_handle = scope.root_raw_mut_ptr(symbols as *mut crate::array::ArrayHeader); - let symbols_len = crate::array::js_array_length( - symbols_handle.get_raw_const_ptr::(), - ); + let symbols_len = symbols_handle + .with_const_ptr::(crate::array::js_array_length); for i in 0..symbols_len { - let symbol = crate::array::js_array_get_f64( - symbols_handle.get_raw_const_ptr::(), - i, - ); + let symbol = + symbols_handle.with_const_ptr::(|symbols| { + crate::array::js_array_get_f64(symbols, i) + }); let symbol_handle = scope.root_nanbox_f64(symbol); let enumerable = js_object_property_is_enumerable( descriptors_handle.get_nanbox_f64(), From 084be0861e1be3439d639a8570b4d54e2e8bd8c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 18 Aug 2026 12:00:18 +0200 Subject: [PATCH 4/4] test(runtime): exercise proxy ownKeys ordering --- .../src/object/object_ops/define_properties.rs | 6 ++++-- test-files/test_gap_5901_define_properties_keys.ts | 4 ++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/crates/perry-runtime/src/object/object_ops/define_properties.rs b/crates/perry-runtime/src/object/object_ops/define_properties.rs index 264ef47051..8fb6bb9846 100644 --- a/crates/perry-runtime/src/object/object_ops/define_properties.rs +++ b/crates/perry-runtime/src/object/object_ops/define_properties.rs @@ -135,8 +135,10 @@ pub extern "C" fn js_object_define_properties(target: f64, descriptors: f64) -> }; if symbols != 0 { let symbols_handle = scope.root_raw_mut_ptr(symbols as *mut crate::array::ArrayHeader); - let symbols_len = symbols_handle - .with_const_ptr::(crate::array::js_array_length); + let symbols_len = + symbols_handle.with_const_ptr::(|symbols| { + crate::array::js_array_length(symbols) + }); for i in 0..symbols_len { let symbol = symbols_handle.with_const_ptr::(|symbols| { diff --git a/test-files/test_gap_5901_define_properties_keys.ts b/test-files/test_gap_5901_define_properties_keys.ts index 895d395d7f..638d08032b 100644 --- a/test-files/test_gap_5901_define_properties_keys.ts +++ b/test-files/test_gap_5901_define_properties_keys.ts @@ -35,6 +35,10 @@ const proxyTarget: any = { 0: 1, foo: 2 }; const proxySymbol = Symbol("proxy"); proxyTarget[proxySymbol] = 3; const proxyBag = new Proxy(proxyTarget, { + ownKeys() { + proxyLog.push("ownKeys"); + return [proxySymbol, "foo", "0"]; + }, getOwnPropertyDescriptor(_target, key) { proxyLog.push(key); return undefined;