diff --git a/changelog.d/7461-layer1-migrate-urlnew.md b/changelog.d/7461-layer1-migrate-urlnew.md new file mode 100644 index 0000000000..5b737521cb --- /dev/null +++ b/changelog.d/7461-layer1-migrate-urlnew.md @@ -0,0 +1 @@ +- **`new URL(input, base)` is the first lowering migrated onto the Layer 1 rooting API, and the move closed a window the hand-fix left.** `FnCtx` has no interior mutability, so #7459's borrow-carrying handle cannot be built on it — the shape that works against a `&mut`-only emitter is the combinator the runtime already settled on for layer 3: `call_rooted` emits the collecting call and roots its result in one step, so no unrooted register ever exists to be held across a later collection point. Migrating the path revealed that #7453's hand-written fix left `base_ptr` live and unrooted across `js_gc_temp_root_get` — a small window, but one nobody spotted in review, which is the argument for an API over a checklist. 11/11 URL gap tests byte-identical to the Node oracle; clean under forced evacuation. One lowering: this starts the migration, it does not finish Layer 1. (#7461) diff --git a/crates/perry-codegen/src/expr/url_main.rs b/crates/perry-codegen/src/expr/url_main.rs index de38fef4c4..d20d528054 100644 --- a/crates/perry-codegen/src/expr/url_main.rs +++ b/crates/perry-codegen/src/expr/url_main.rs @@ -54,15 +54,25 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { // would root an already-stale pointer. let url_slot = super::temp_root::temp_root_push_i64(ctx, &url_ptr); let base_v = lower_expr(ctx, base)?; - let base_ptr = ctx - .block() - .call(I64, "js_url_coerce_string", &[(DOUBLE, &base_v)]); + // Layer 1 migration (#7459): `call_rooted` emits the collecting + // call and roots its result in one step, so no unrooted + // register for `base_ptr` ever exists to be held across a later + // collection point. The window that made #7453 a bug is not + // expressible here. + let base_slot = crate::rooting::call_rooted( + ctx, + I64, + "js_url_coerce_string", + &[(DOUBLE, &base_v)], + ); let url_ptr = super::temp_root::temp_root_get_i64(ctx, &url_slot); + let base_ptr = base_slot.read(ctx); let obj = ctx.block().call( I64, "js_url_new_with_base", &[(I64, &url_ptr), (I64, &base_ptr)], ); + base_slot.release(ctx); super::temp_root::temp_root_truncate(ctx, &url_slot); obj } else { diff --git a/crates/perry-codegen/src/rooting.rs b/crates/perry-codegen/src/rooting.rs index a35cedc68d..d77575fd5d 100644 --- a/crates/perry-codegen/src/rooting.rs +++ b/crates/perry-codegen/src/rooting.rs @@ -238,3 +238,67 @@ mod tests { assert_eq!(len.0, "%r0"); } } + +// --------------------------------------------------------------------------- +// Applying the design to the REAL emitter. +// +// `FnCtx` has no interior mutability -- `ctx.block()` needs `&mut` -- so the +// borrow-carrying `Raw` above cannot be built on it directly: `root(self)` +// would need a second borrow while the handle still holds the first (the same +// E0499 the RFC's own API hits, see `Raw`'s doc). +// +// The shape that DOES work against a `&mut`-only emitter is the combinator, and +// it is the same one the runtime settled on for layer 3 (`RuntimeHandle:: +// across_*`): never hand out an unrooted handle at all. `call_rooted` emits the +// collecting call and roots its result in one step, so there is no window in +// which an unrooted register exists to be misused, and `read` re-reads through +// the slot every time. +// +// This is weaker than the borrow formulation -- it prevents the bug rather than +// detecting attempts to write it -- but it needs no emitter rewrite, which is +// what makes it migratable one call site at a time. +// --------------------------------------------------------------------------- + +use crate::expr::FnCtx; +use crate::types::{I32, I64}; + +/// A slot holding a GC-managed pointer for the duration of a lowering. +#[derive(Debug, Clone)] +pub struct RootedSlot { + idx: String, +} + +impl RootedSlot { + /// Re-read the slot. Called afresh at every use: the returned register is + /// only valid until the next emission that can collect, and re-reading is + /// cheaper than reasoning about whether one has happened. + pub fn read(&self, ctx: &mut FnCtx<'_>) -> String { + ctx.block() + .call(I64, "js_gc_temp_root_get", &[(I32, &self.idx)]) + } + + /// Release the slot. Call after the last [`RootedSlot::read`]. + pub fn release(self, ctx: &mut FnCtx<'_>) { + ctx.block() + .call_void("js_gc_temp_root_truncate", &[(I32, &self.idx)]); + } +} + +/// Emit a call that can collect and root its result in one step. +/// +/// The point is what this function does NOT return: an unrooted register. A +/// caller cannot hold the result across a later collection point because it +/// never has the result -- only a slot -- which is what makes the #7453 shape +/// unwritable here rather than merely reviewable. +pub fn call_rooted( + ctx: &mut FnCtx<'_>, + ret_ty: crate::types::LlvmType, + callee: &str, + args: &[(crate::types::LlvmType, &str)], +) -> RootedSlot { + let reg = ctx.block().call(ret_ty, callee, args); + let idx = ctx + .block() + .call(I32, "js_gc_temp_root_push", &[(I64, ®)]); + RootedSlot { idx } +}