fix(codegen): root the function across Func.prototype.x = <call> (#11635)

changelog.d/11639-func-proto-register-rooting.md

@@ -0,0 +1 @@
+- **fix(codegen): `F.prototype.x = <call>` no longer hands the runtime a from-space function (#11635).** `Expr::RegisterFunctionPrototypeMethod` lowered the function operand first and kept it in a register across the value's lowering. When the value was a call that collected (moment 2.31.0's `proto.toIsoString = deprecate(msg, fn)` at module init, with `Duration` a boxed function declaration), an evacuating minor moved the closure and `js_register_function_prototype_method` read the retired header in `synthetic_class_id_for_function`. Under a seeded schedule with the from-space quarantine that faulted (moment/parse_format: 22/200 seeds on main, 0/200 fixed); without it the method was registered against a stale address and instances never saw it (`TypeError: a is not a function` in the new witness, on every GC arm including the default one). The function and the value are now held in a `RootedGroup` and re-read below the window. The second hazard named in the issue, `FUNCTION_CLASS_IDS` being keyed by the closure's bits, was already handled by the class side-table scanner's rekey on evacuation; a new runtime test pins that through the real registration entry point (sabotaged: disabling the rekey turns it red). `gc_root_dominance_check.py --stale-registers` already reported the six moment uses; `--fatal-sinks` now ranks `js_register_function_prototype_method` / `js_get_function_prototype_method` as dereferencing sinks, so they no longer drop out of the fatal slice. New witness: `test-files/test_gap_gc_11635_func_proto_register_across_call.ts` (registered in `test-parity/gc_repsel_corpus.txt`).

crates/perry-codegen/src/expr/computed_store_rooting_tests.rs

@@ -992,3 +992,69 @@ fn growing_array_store_uses_the_reallocated_head_for_its_barrier() {
         "the realloc-path barrier must use {new_head}, returned by the grow helper; got `{barrier}`"
     );
 }
+
+/// #11635 — `F.prototype.x = f()` for a function declaration `F`
+/// (`Expr::RegisterFunctionPrototypeMethod`). `F` is evaluated first and was
+/// held in a register across the value's lowering, so an evacuating minor
+/// inside the value's call left `js_register_function_prototype_method`
+/// reading the retired closure (moment 2.31.0's `proto.toIsoString =
+/// deprecate(...)`). The function operand here is a call result — a value no
+/// local slot can re-derive — so only a temp root can carry it across the
+/// window. Its reload must sit below the allocating value and its store above.
+#[test]
+fn function_prototype_registration_roots_the_function_across_an_allocating_value() {
+    let make_func = Expr::Call {
+        callee: Box::new(Expr::LocalGet(1)),
+        args: Vec::new(),
+        type_args: Vec::new(),
+        byte_offset: 0,
+    };
+    let ir = compile_body_with_params(
+        "func_proto_register",
+        vec![param(1, "mk", Type::Any)],
+        vec![Stmt::Expr(Expr::RegisterFunctionPrototypeMethod {
+            func: Box::new(make_func),
+            method_name: "toIsoString".to_string(),
+            value: Box::new(allocating_value()),
+        })],
+    );
+    assert!(
+        calls(&ir, "js_register_function_prototype_method"),
+        "the registration arm was not reached:\n{ir}"
+    );
+    // The value operand is itself rooted across the registration call, so it
+    // reaches the call as a reload; the window is the value's ALLOCATION.
+    let alloc = ir
+        .lines()
+        .position(|l| l.contains("@js_object_alloc") && !l.trim_start().starts_with("declare"))
+        .unwrap_or_else(|| panic!("the allocating value was not emitted:\n{ir}"));
+    let func = call_operand_of(&ir, "js_register_function_prototype_method", 0);
+    let reload = producer_line(&ir, &func);
+    let reload_line = ir.lines().nth(reload).expect("producer line exists");
+    assert!(
+        reload_line.contains("load ptr addrspace(1), ptr "),
+        "the function operand ({func}) is not reloaded from a root slot:\n{ir}"
+    );
+    assert!(
+        reload > alloc,
+        "the function operand is reloaded at line {reload}, above the value's allocation at \
+         line {alloc}, so the register it names can be from-space:\n{ir}"
+    );
+    let slot = reload_line
+        .rsplit_once(", ptr ")
+        .map(|(_, tail)| tail.split(',').next().unwrap_or(tail).trim())
+        .expect("a root reload names its slot");
+    assert!(
+        ir.lines()
+            .take(alloc)
+            .any(|l| l.contains("store ptr addrspace(1)")
+                && !l.contains(" null,")
+                && l.rsplit_once(", ptr ").is_some_and(|(_, tail)| tail
+                    .split(',')
+                    .next()
+                    .unwrap_or(tail)
+                    .trim()
+                    == slot)),
+        "root slot {slot} has no store above the value's allocation:\n{ir}"
+    );
+}

crates/perry-codegen/src/expr/static_field_meta.rs

@@ -981,13 +981,26 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
         // `new <FuncRef>(args)` lowering below stamps the same id on
         // the instance so dispatch finds the method via the regular
         // `(*obj).class_id` walk.
+        //
+        // #11635: `func` is evaluated FIRST and is live across the lowering
+        // of `value`, which is routinely a call (`proto.toIsoString =
+        // deprecate(msg, fn)` in moment). Holding it in a register let an
+        // evacuating minor inside that call move the closure while the
+        // register kept its from-space address, and the runtime then read
+        // the retired closure header in `synthetic_class_id_for_function`.
+        // Root it across the window and re-read it below. `value` is
+        // rooted across the registration call too, because that call is a
+        // `Reenters` runtime entry and the value is the expression result.
         Expr::RegisterFunctionPrototypeMethod {
             func,
             method_name,
             value,
-        } => {
-            let func_double = lower_expr(ctx, func)?;
-            let val_double = lower_expr(ctx, value)?;
+        } => with_rooted_group(ctx, 2, |ctx, group| {
+            let protect_func = any_operand_may_collect(ctx, [value.as_ref()]);
+            let func_i = group.lower(ctx, func, protect_func)?;
+            let val_i = group.lower(ctx, value, true)?;
+            let func_double = group.reread(ctx, func_i)?;
+            let val_double = group.reread(ctx, val_i)?;
             let key_idx = ctx.strings.intern(method_name);
             let key_bytes_global = format!("@{}", ctx.strings.entry(key_idx).bytes_global);
             let key_len = ctx.strings.entry(key_idx).byte_len.to_string();
@@ -1001,8 +1014,8 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
                     (DOUBLE, &val_double),
                 ],
             );
-            Ok(val_double)
-        }
+            group.reread(ctx, val_i)
+        }),
         // Read side of #838 followup (b): `<funcDecl>.prototype.<name>`
         // (Ident or computed-string-literal form) lowered into a direct
         // lookup of the prototype-method side-table. Returns the closure

crates/perry-runtime/src/gc/tests/copying_side_tables.rs

@@ -80,6 +80,80 @@ fn test_copying_minor_rewrites_class_side_table_values_and_function_keys() {
     );
 }
 
+/// #11635: the synthetic class id a `Func.prototype.x = fn` registration
+/// allocates is keyed by the function's NaN-boxed bits in
+/// `FUNCTION_CLASS_IDS`. When the function moves in a copying minor, the key
+/// must follow it: a second registration and a `new Func()` made through the
+/// POST-move address must land on the SAME id, or `F.prototype` methods
+/// registered before the move are split from instances created after it.
+///
+/// Driven through the real entry point (`js_register_function_prototype_method`)
+/// rather than a seeded key, and asserts the subject moved, so a green run
+/// cannot be one where nothing was relocated.
+#[test]
+fn test_function_prototype_registration_class_id_survives_a_move() {
+    let _guard = CopyingNurseryTestGuard::new(1);
+    crate::object::test_clear_class_side_table_roots();
+    gc_register_mutable_root_scanner(crate::object::scan_class_side_table_roots_mut);
+
+    let func = crate::arena::arena_alloc_gc(
+        std::mem::size_of::<crate::closure::ClosureHeader>(),
+        std::mem::align_of::<crate::closure::ClosureHeader>(),
+        GC_TYPE_CLOSURE,
+    ) as usize;
+    unsafe { init_test_closure(func as *mut u8) };
+    js_shadow_slot_set(0, ptr_bits(func));
+    let before = young_leaf();
+    let before_cid = unsafe {
+        crate::object::js_register_function_prototype_method(
+            f64::from_bits(ptr_bits(func)),
+            b"before".as_ptr(),
+            6,
+            f64::from_bits(string_bits(before)),
+        )
+    };
+    assert_ne!(
+        before_cid, 0,
+        "a verified closure must get a synthetic class id"
+    );
+
+    let _ = gc_collect_minor();
+
+    let func_after_bits = js_shadow_slot_get(0);
+    assert_ne!(
+        func_after_bits,
+        ptr_bits(func),
+        "the function must MOVE, or this test proves nothing"
+    );
+    assert!(crate::arena::pointer_in_nursery(
+        (func_after_bits & POINTER_MASK) as usize
+    ));
+
+    let after = young_leaf();
+    let after_cid = unsafe {
+        crate::object::js_register_function_prototype_method(
+            f64::from_bits(func_after_bits),
+            b"after".as_ptr(),
+            5,
+            f64::from_bits(string_bits(after)),
+        )
+    };
+    assert_eq!(
+        after_cid, before_cid,
+        "a registration through the moved function must reuse its class id"
+    );
+    assert_eq!(
+        crate::object::synthetic_class_id_for_function(f64::from_bits(func_after_bits)),
+        before_cid,
+        "`new F()` through the moved function must stamp the same class id"
+    );
+    assert_eq!(
+        crate::object::test_class_prototype_method_root_bits(before_cid, "before") & TAG_MASK,
+        STRING_TAG,
+        "the method registered before the move must stay on the same class"
+    );
+}
+
 #[test]
 fn test_copying_minor_rewrites_symbol_side_table_roots_and_lookups() {
     let _guard = CopyingNurseryTestGuard::new(1);

scripts/addr_class_ratchet_baseline.txt

@@ -28,7 +28,7 @@ handle-floor | crates/perry-ext-http/src/lib.rs | 2
 handle-floor | crates/perry-runtime/src/array/alloc.rs | 2
 handle-floor | crates/perry-runtime/src/array/concat_reverse.rs | 1
 handle-floor | crates/perry-runtime/src/array/flat_clone.rs | 3
-handle-floor | crates/perry-runtime/src/array/generic.rs | 4
+handle-floor | crates/perry-runtime/src/array/generic.rs | 3
 handle-floor | crates/perry-runtime/src/array/header.rs | 3
 handle-floor | crates/perry-runtime/src/array/indexing.rs | 2
 handle-floor | crates/perry-runtime/src/array/indexing_keyed.rs | 1
@@ -60,11 +60,10 @@ handle-floor | crates/perry-runtime/src/builtins/formatting/typed_array_equality
 handle-floor | crates/perry-runtime/src/builtins/globals.rs | 9
 handle-floor | crates/perry-runtime/src/builtins/numbers.rs | 2
 handle-floor | crates/perry-runtime/src/builtins/table.rs | 1
-handle-floor | crates/perry-runtime/src/child_process/registry.rs | 1
 handle-floor | crates/perry-runtime/src/child_process/v8_serde.rs | 1
 handle-floor | crates/perry-runtime/src/closure/dispatch/validate.rs | 1
 handle-floor | crates/perry-runtime/src/cluster.rs | 1
-handle-floor | crates/perry-runtime/src/date.rs | 3
+handle-floor | crates/perry-runtime/src/date.rs | 1
 handle-floor | crates/perry-runtime/src/dgram.rs | 1
 handle-floor | crates/perry-runtime/src/dns.rs | 4
 handle-floor | crates/perry-runtime/src/exception.rs | 2
@@ -178,7 +177,7 @@ handle-floor | crates/perry-runtime/src/typed_feedback.rs | 3
 handle-floor | crates/perry-runtime/src/typed_feedback/guards.rs | 1
 handle-floor | crates/perry-runtime/src/typedarray/access.rs | 5
 handle-floor | crates/perry-runtime/src/typedarray/construct.rs | 4
-handle-floor | crates/perry-runtime/src/typedarray/mod.rs | 5
+handle-floor | crates/perry-runtime/src/typedarray/mod.rs | 4
 handle-floor | crates/perry-runtime/src/typedarray_props.rs | 2
 handle-floor | crates/perry-runtime/src/url/abort.rs | 1
 handle-floor | crates/perry-runtime/src/url/search_params.rs | 1

scripts/gc_root_dominance_check.py

@@ -2833,7 +2833,11 @@ def rewritten_load_kind(text):
     # A stale RegExpHeader* is dereferenced immediately by both of these —
     # this is #7154's residual, and it faulted rather than merely answering
     # wrong, so it belongs in the fatal ranking and not just the raw count.
-    r"regexp_test|regexp_exec|regexp_match\w*|regexp_replace\w*"
+    r"regexp_test|regexp_exec|regexp_match\w*|regexp_replace\w*|"
+    # Both read the function's closure header (`is_callable_function_value`)
+    # to key its synthetic class id. A stale function register here faulted
+    # at moment's module init (#11635), so it ranks as fatal too.
+    r"register_function_prototype_method|get_function_prototype_method"
     r")$"
 )
 

test-files/test_gap_gc_11635_func_proto_register_across_call.ts

@@ -0,0 +1,78 @@
+// #11635: `F.prototype.x = <call>` must not hold F in a register across the
+// call.
+//
+// HIR lowers `F.prototype.x = v` (and the aliased `var proto = F.prototype;
+// proto.x = v`) for a function declaration F into a runtime registration keyed
+// by F's closure. Codegen evaluated F first, then the value, then called the
+// registration with F's register. When the value is a call that collects, an
+// evacuating minor moves F while the register keeps its old address, and the
+// registration reads the retired closure. moment 2.31.0 does exactly this at
+// module init: `proto.toIsoString = deprecate(msg, toISOString$1)` with
+// `proto = Duration.prototype`, where Duration is a function declaration
+// inside the UMD factory. Under a seeded GC schedule the process faulted in
+// `synthetic_class_id_for_function`; without the from-space quarantine the
+// method could be registered under the stale address, so instances never saw
+// it.
+//
+// The shape is kept: F is declared inside a factory and captured by nested
+// functions (so it is a boxed heap closure that can move), the prototype is
+// aliased, and each value comes from a
+// helper that allocates enough garbage for a collection to land inside it.
+//
+// Output must be byte-identical to node.
+
+function churn(rounds: number): number {
+  let n = 0;
+  for (let r = 0; r < rounds; r++) {
+    const a: any[] = new Array(16);
+    for (let j = 0; j < 16; j++) a[j] = { j, r, s: "v" + j };
+    n += a.length;
+  }
+  return n;
+}
+
+let churned = 0;
+
+function factory(): any {
+  const tag = "D";
+  function Duration(this: any, v: number) {
+    this.v = v;
+  }
+  function deprecate(msg: string, fn: (this: any) => string): (this: any) => string {
+    churned += churn(20000);
+    return function (this: any) {
+      return msg + ":" + fn.call(this);
+    };
+  }
+  function show(this: any) {
+    return tag + this.v;
+  }
+  // Nested functions that capture Duration, as moment's do: that is what
+  // puts Duration in a box whose read is not re-derivable from a root.
+  function isDuration(o: any): boolean {
+    return o instanceof Duration;
+  }
+  function make(v: number): any {
+    return new (Duration as any)(v);
+  }
+  const proto = Duration.prototype;
+  proto.a = deprecate("a", show);
+  proto.b = deprecate("b", show);
+  Duration.prototype.c = deprecate("c", show);
+  proto.d = deprecate("d", show);
+  (Duration as any).isDuration = isDuration;
+  (Duration as any).make = make;
+  return Duration;
+}
+
+const D = factory();
+churn(20000);
+const out: string[] = [];
+for (let i = 0; i < 3; i++) {
+  const d = new D(i);
+  out.push(d.a(), d.b(), d.c(), d.d());
+  out.push(String(d instanceof D), typeof D.prototype.a, typeof d.d);
+  out.push(String(D.isDuration(d)), D.make(i + 10).c());
+}
+console.log(out.join(" "));
+console.log("churned", churned > 0);

test-parity/gc_repsel_corpus.txt

@@ -900,3 +900,8 @@ test_gap_gc_template_coerce_join
 # so the collector never traced the slot. claude-code `-p` read a tool
 # record's `prompt` method back as a plain object.
 test_gap_gc_11559_spill_headroom_store
+# `F.prototype.x = <call>` held F (a boxed function declaration) in a register
+# across the value's call, so an evacuating minor inside it left the
+# registration reading the retired closure. moment 2.31.0 module init under a
+# seeded schedule (#11635).
+test_gap_gc_11635_func_proto_register_across_call