diff --git a/changelog.d/7815-constructor-receiver-rooting.md b/changelog.d/7815-constructor-receiver-rooting.md new file mode 100644 index 0000000000..94f0c7bd92 --- /dev/null +++ b/changelog.d/7815-constructor-receiver-rooting.md @@ -0,0 +1,13 @@ +**Three constructors now root their freshly-allocated receiver across a later ToString coercion** (#6949 shape b). + +The shape: `js_object_alloc` into a raw Rust local, then `js_string_coerce` (or another allocating call) further down, then writes through that local. `js_string_coerce` returns without allocating only for an already-heap `STRING_TAG` value; every other shape allocates — an SSO string materialises, a number/bool/null/BigInt builds its stringification, a `POINTER_TAG` object runs a user `toString`/`valueOf` — and any of those can collect and **evacuate**. A raw local is neither a GC root nor a shadow slot. + +* **`messaging.rs` `js_broadcast_channel_new`** — `obj` allocated, then the channel `name` coerced, then eight `set_field`/`install_method` writes through `obj`. +* **`builtins/formatting/boxed_primitives.rs` `js_boxed_string_new`** — `obj` allocated, then *both* branches allocate (`js_string_from_bytes` for `new String()`, `js_string_coerce` otherwise), then the payload registration, the two `install_string_wrapper_*` calls and the prototype attach all use `obj`. +* **`disposable.rs` `js_suppressed_error_new`** — needed more than a rebind, for two reasons worth recording. Its `set_nonenum` closure captures `obj` **by value**, so a single re-read after the coercion would leave every property write using the address captured at definition time. And `object_set_static_prototype(obj as usize, …)` keys a **side table** on the address, so a stale one does not fault — it files the prototype under an address nothing looks up, and `instanceof SuppressedError` quietly stops resolving. The handle is therefore re-read at every use rather than once. + +Same `RuntimeHandleScope` idiom #6943 established, and the same honest caveat as #7811 (shape a): **no failing witness.** A fixture driving all of these with non-string arguments matches Node exactly on both arms. The window needs the pointee to move during that specific coercion, and this family is documented as invisible to runtime probes at the moment of collection; the justification is the repo's own rooting invariant — a raw heap pointer held across a call that can allocate is a defect regardless of whether today's allocator layout exposes it. + +**One site from the issue's shape-(b) list is deliberately not here.** `object/class_registry/construct.rs`'s rebound-`RegExp` arm (where the coerced `pattern` spans the `flags` coercion) is a real instance and the fix is written, but that file sits at 1999 lines against the 2000-line CI cap, so any addition trips `check_file_size.sh`. PR #7779 already restructures that file for #7524; this site should land on top of it rather than fight the cap twice in two PRs. + +Verified: `cargo test -p perry-runtime --lib` 2051 passed / 0 failed; `test_gap_regexp` 2/2, `test_gap_disposable` 1/1, `test_gap_string` 5/5, `test_gap_error` 2/2; fmt and file-size clean. diff --git a/crates/perry-runtime/src/builtins/formatting/boxed_primitives.rs b/crates/perry-runtime/src/builtins/formatting/boxed_primitives.rs index ffc2cb974a..389d2e3032 100644 --- a/crates/perry-runtime/src/builtins/formatting/boxed_primitives.rs +++ b/crates/perry-runtime/src/builtins/formatting/boxed_primitives.rs @@ -299,6 +299,15 @@ pub extern "C" fn js_boxed_number_new(value: f64) -> f64 { #[no_mangle] pub extern "C" fn js_boxed_string_new(value: f64, has_arg: i32) -> f64 { let obj = crate::object::js_object_alloc(CLASS_ID_BOXED_STRING, 0); + // #6949(b): both branches below allocate — `js_string_from_bytes` for the + // empty-string case and `js_string_coerce` otherwise, the latter running a + // user `toString`/`valueOf` for a POINTER_TAG value — so either can collect + // and EVACUATE while `obj` sits in a raw Rust local. Every use below + // (`register_boxed_primitive_payload`, the two `install_string_wrapper_*` + // calls, `attach_boxed_primitive_prototype`, and the returned NaN-box) + // dereferences or keys on it. + let scope = crate::gc::RuntimeHandleScope::new(); + let obj_handle = scope.root_raw_mut_ptr(obj); // `new String()` (no args) is spec'd to box "", not "undefined". let ptr = if has_arg == 0 { crate::string::js_string_from_bytes(std::ptr::null(), 0) @@ -309,6 +318,7 @@ pub extern "C" fn js_boxed_string_new(value: f64, has_arg: i32) -> f64 { } js_string_coerce(value) }; + let obj = obj_handle.get_raw_mut_ptr::(); let boxed = f64::from_bits(crate::value::JSValue::string_ptr(ptr).bits()); register_boxed_primitive_payload(obj, boxed); install_string_wrapper_indices(obj, ptr); diff --git a/crates/perry-runtime/src/disposable.rs b/crates/perry-runtime/src/disposable.rs index effbfb4e72..725e7ef496 100644 --- a/crates/perry-runtime/src/disposable.rs +++ b/crates/perry-runtime/src/disposable.rs @@ -461,8 +461,25 @@ pub extern "C" fn js_suppressed_error_new(error: f64, suppressed: f64, message: // properties { writable:true, enumerable:false, configurable:true }. The // `name` default ("SuppressedError") lives on `SuppressedError.prototype`, // so it is *not* set as an own property here. + // #6949(b): `obj` is a raw Rust local — neither a GC root nor a shadow slot + // — and everything below it allocates: `js_string_from_bytes` per key, + // `js_object_set_field_by_name` when the object grows, and + // `js_string_coerce` on the message. Any of those can collect and EVACUATE. + // + // A single rebind after the coercion would not be enough here for two + // reasons: the closure captures `obj` BY VALUE, so every `set_nonenum` call + // would keep using the address captured at definition time; and + // `object_set_static_prototype` at the end keys a SIDE TABLE on + // `obj as usize`, so a stale address does not fault — it files the + // prototype under an address nothing will look up, and `instanceof + // SuppressedError` quietly stops resolving. + // + // So root once and re-read at every use, which is what the handle gives. + let scope = crate::gc::RuntimeHandleScope::new(); + let obj_handle = scope.root_raw_mut_ptr(obj); let set_nonenum = |key: &str, value: f64| { let key_ptr = js_string_from_bytes(key.as_ptr(), key.len() as u32); + let obj = obj_handle.get_raw_mut_ptr::(); js_object_set_field_by_name(obj, key_ptr, value); crate::object::set_property_attrs( obj as usize, @@ -484,11 +501,13 @@ pub extern "C" fn js_suppressed_error_new(error: f64, suppressed: f64, message: }; set_nonenum("message", message_val); } + let obj = obj_handle.get_raw_mut_ptr::(); let result = js_nanbox_pointer(obj as i64); // Link the instance to `SuppressedError.prototype` so `name`/`message` // defaults and `instanceof SuppressedError` resolve through the chain. let proto = crate::object::builtin_prototype_value("SuppressedError"); if proto.to_bits() != TAG_UNDEFINED && js_nanbox_get_pointer(proto) != 0 { + let obj = obj_handle.get_raw_mut_ptr::(); crate::object::prototype_chain::object_set_static_prototype(obj as usize, proto.to_bits()); } result diff --git a/crates/perry-runtime/src/messaging.rs b/crates/perry-runtime/src/messaging.rs index b4df478f40..a5d1ff74ba 100644 --- a/crates/perry-runtime/src/messaging.rs +++ b/crates/perry-runtime/src/messaging.rs @@ -606,7 +606,15 @@ pub extern "C" fn js_broadcast_channel_new(name: f64) -> f64 { "constructor", get_global_constructor("BroadcastChannel"), ); + // #6949(b): `js_string_coerce` allocates for every shape except an + // already-heap STRING_TAG value, so it can collect and EVACUATE — and + // `obj`, allocated a few lines up, is a raw Rust local: neither a GC root + // nor a shadow slot. Every `set_field`/`install_method` below writes + // through it. Root it across the coercion and re-read. + let scope = crate::gc::RuntimeHandleScope::new(); + let obj_handle = scope.root_raw_mut_ptr(obj); let name_ptr = crate::builtins::js_string_coerce(name); + let obj = obj_handle.get_raw_mut_ptr::(); let name_value = f64::from_bits(JSValue::string_ptr(name_ptr).bits()); set_field(obj, "name", name_value); install_method(obj, "close", noop0 as *const u8, 0);