diff --git a/changelog.d/7422-buffer-global-dispatch-arming.md b/changelog.d/7422-buffer-global-dispatch-arming.md new file mode 100644 index 0000000000..527c56cad9 --- /dev/null +++ b/changelog.d/7422-buffer-global-dispatch-arming.md @@ -0,0 +1 @@ +**Buffer statics reached through the global now dispatch (#6924).** `class MyBuf extends Buffer` inherited statics (`MyBuf.from`, `.alloc`, `.isBuffer`, `.concat`, …) and value-captured statics off the global (`const f = (Buffer as any).from`) read as functions but silently returned `undefined` when invoked, in any program without an explicit `buffer` import. The statics are BOUND_METHOD closures dispatching by name through the `"buffer.Buffer"` namespace, and the per-module devirtualization registry (#5256) is armed only by codegen-emitted `js_nm_install_buffer()` calls at *import* sites — the global-`Buffer` mint (`buffer_constructor_value()`) violated the registry's "bound export exists only after its module's install ran" rule. The mint now arms the bucket itself, mirroring its existing `install_native_module_vtable()` call. The new per-PR unit test suppresses the `cfg(test)` lazy install-all fallback (new RAII-guarded toggle) so the assertion cannot be vacuously green (sabotage-verified); gap-suite twin `test_gap_6924_extends_buffer_statics.ts` matches Node 26.5.1 byte-for-byte. diff --git a/crates/perry-runtime/src/object/native_module/callable_exports.rs b/crates/perry-runtime/src/object/native_module/callable_exports.rs index aad2f4e9ee..72ad9533e6 100644 --- a/crates/perry-runtime/src/object/native_module/callable_exports.rs +++ b/crates/perry-runtime/src/object/native_module/callable_exports.rs @@ -737,6 +737,18 @@ pub(crate) fn buffer_constructor_value() -> f64 { return f64::from_bits(cached); } + // #6924: the statics minted below are BOUND_METHOD closures that + // dispatch by name through the "buffer.Buffer" namespace, and that + // dispatch resolves via the per-module registry + // (`nm_dispatch_lookup`). The registry's soundness rule — a bound + // export exists only after its module's `js_nm_install_*` ran — is + // upheld by codegen for IMPORTED modules, but `Buffer` is a global: + // this mint runs with no `buffer` import anywhere, so nothing armed + // the bucket and every inherited/captured static (`MyBuf.from`, + // `const f = B.from`) silently returned `undefined`. Arm it at the + // mint, mirroring `install_native_module_vtable()` above. + super::super::native_module_registry::js_nm_install_buffer(); + let func_ptr = buffer_constructor_thunk as *const u8; let closure = crate::closure::js_closure_alloc(func_ptr, 0); if closure.is_null() { diff --git a/crates/perry-runtime/src/object/native_module_registry.rs b/crates/perry-runtime/src/object/native_module_registry.rs index f1eb5642c7..ab4e98eadc 100644 --- a/crates/perry-runtime/src/object/native_module_registry.rs +++ b/crates/perry-runtime/src/object/native_module_registry.rs @@ -138,6 +138,22 @@ fn nm_module_index(name: &str) -> Option { } } +/// Test-only: suppress the `cfg(test)` lazy `js_nm_install_all()` fallback in +/// the lookup fns below. The fallback exists so ordinary unit tests can +/// dispatch without the codegen-emitted install — but it makes any test that +/// asserts "path X armed bucket Y itself" vacuously green (the lookup self- +/// heals). A test proving an arming obligation flips this on (RAII-guarded) +/// so the registry state it observes is exactly what the code under test +/// produced. See `tests::buffer_constructor_mint_arms_the_buffer_dispatch_bucket`. +#[cfg(test)] +pub(crate) static NM_TEST_DISABLE_LAZY_INSTALL: std::sync::atomic::AtomicBool = + std::sync::atomic::AtomicBool::new(false); + +#[cfg(test)] +fn nm_lazy_install_enabled() -> bool { + !NM_TEST_DISABLE_LAZY_INSTALL.load(Ordering::Relaxed) +} + /// Look up the installed per-module dispatch fn for `name`. `None` if unknown or /// its `js_nm_install_()` was never emitted (module not statically imported). pub(crate) fn nm_dispatch_lookup(name: &str) -> Option { @@ -150,7 +166,7 @@ pub(crate) fn nm_dispatch_lookup(name: &str) -> Option { // `js_nm_install_()` that precedes use in real programs. Lazily // populate so tests exercise the real registry path. (Not in production.) #[cfg(test)] - { + if nm_lazy_install_enabled() { js_nm_install_all(); let p = NM_DISPATCH_REGISTRY[b as usize].load(Ordering::Relaxed); if !p.is_null() { @@ -637,7 +653,7 @@ pub(crate) fn nm_ctor_lookup(module: &str) -> Option { // See nm_dispatch_lookup: unit tests construct directly without the codegen // install; lazily populate so tests exercise the real registry. #[cfg(test)] - { + if nm_lazy_install_enabled() { js_nm_install_all(); let p = NM_CTOR_REGISTRY[b as usize].load(Ordering::Relaxed); if !p.is_null() { @@ -680,7 +696,7 @@ pub(crate) fn nm_attach_lookup(module: &str) -> Option { return Some(unsafe { std::mem::transmute::<*mut (), NmAttachFn>(p) }); } #[cfg(test)] - { + if nm_lazy_install_enabled() { js_nm_install_all(); let p = NM_ATTACH_REGISTRY[b as usize].load(Ordering::Relaxed); if !p.is_null() { @@ -695,3 +711,60 @@ pub(crate) fn nm_attach_lookup(module: &str) -> Option { fn nm_register_attach(b: NmBucket, f: NmAttachFn) { NM_ATTACH_REGISTRY[b as usize].store(f as *mut (), Ordering::Relaxed); } + +#[cfg(test)] +mod tests { + use super::*; + + /// #6924 acceptance: minting the global `Buffer` constructor value must arm + /// the buffer dispatch bucket ITSELF. `Buffer` is a global — a program that + /// never imports `buffer` still mints its bound statics (`Buffer.from` + /// captured as a value, `class MyBuf extends Buffer` inherited statics), + /// and those dispatch by name through `nm_dispatch_lookup("buffer.Buffer")`. + /// Before the fix nothing armed the bucket on that path, so every such call + /// silently returned `undefined`. + /// + /// The gap-suite twin (`test_gap_6924_extends_buffer_statics.ts`) covers + /// the end-to-end program shape but is tag-gated; this test is the per-PR + /// gate. Three resets make the assertion non-vacuous (sabotage-verified: + /// removing the mint's install call turns this red): + /// - the thread-local ctor cache, else a prior same-thread mint + /// early-returns before the install; + /// - the bucket slot, else another test's arming lingers; + /// - the `cfg(test)` lazy install-all (RAII-suppressed), else the + /// `nm_attach_lookup` the mint performs self-heals the whole registry + /// and the test cannot fail. + /// The brief null window on the process-global slot is unobservable under + /// CI's serial `RUST_TEST_THREADS=1` mode. + #[test] + fn buffer_constructor_mint_arms_the_buffer_dispatch_bucket() { + struct LazyInstallGuard; + impl Drop for LazyInstallGuard { + fn drop(&mut self) { + NM_TEST_DISABLE_LAZY_INSTALL.store(false, Ordering::Relaxed); + } + } + NM_TEST_DISABLE_LAZY_INSTALL.store(true, Ordering::Relaxed); + let _guard = LazyInstallGuard; + + super::super::native_module::BUFFER_CONSTRUCTOR_VALUE.with(|slot| slot.set(0)); + NM_DISPATCH_REGISTRY[NmBucket::Buffer as usize] + .store(std::ptr::null_mut(), Ordering::Relaxed); + + let ctor = super::super::native_module::buffer_constructor_value(); + assert_ne!( + ctor.to_bits(), + crate::value::TAG_UNDEFINED, + "Buffer constructor mint failed outright" + ); + + assert!( + !NM_DISPATCH_REGISTRY[NmBucket::Buffer as usize] + .load(Ordering::Relaxed) + .is_null(), + "buffer_constructor_value() must arm the buffer dispatch bucket: \ + its bound statics dispatch through nm_dispatch_lookup(\"buffer.Buffer\"), \ + and no import-emitted js_nm_install_buffer exists on the global-Buffer path" + ); + } +} diff --git a/test-files/test_gap_6924_extends_buffer_statics.ts b/test-files/test_gap_6924_extends_buffer_statics.ts new file mode 100644 index 0000000000..65dbc640f1 --- /dev/null +++ b/test-files/test_gap_6924_extends_buffer_statics.ts @@ -0,0 +1,29 @@ +// #6924: `class MyBuf extends Buffer` — inherited statics (`MyBuf.from`, +// `.alloc`, `.isBuffer`, `.concat`) must resolve as values AND dispatch when +// invoked. The statics are BOUND_METHOD closures dispatching by name through +// the "buffer.Buffer" namespace; the per-module dispatch bucket used to be +// armed only by an explicit `buffer` import, so a program that reached Buffer +// purely through the GLOBAL (the normal case) minted the bound statics with an +// unarmed registry and every call silently returned `undefined`. +// +// Validated byte-for-byte against `node --experimental-strip-types`. + +class MyBuf extends Buffer {} + +console.log("typeof from:", typeof (MyBuf as any).from); +console.log("from().length:", (MyBuf as any).from("ab").length); +console.log("from() bytes:", (MyBuf as any).from("ab")[0], (MyBuf as any).from("ab")[1]); +console.log("alloc:", (MyBuf as any).alloc(3).length); +console.log("isBuffer(buf):", (MyBuf as any).isBuffer(Buffer.from("x"))); +console.log("isBuffer(str):", (MyBuf as any).isBuffer("x")); +console.log( + "concat:", + (MyBuf as any).concat([Buffer.from("a"), Buffer.from("b")]).toString() +); + +// The same statics captured as VALUES off the global (the alias shape the +// subclass read reduces to) must also dispatch. +const B: any = Buffer; +const f = B.from; +console.log("captured from:", f("cd").toString()); +console.log("captured alloc:", B.alloc(2).length);