diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index c5951dd147a..a1936aa0dff 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -1653,9 +1653,14 @@ pub struct RuntimeHooks { /// `configureDebugger()` — everything `init()` does that names a /// `bun_runtime` type. Called once with the freshly-boxed VM AFTER /// `vm.global` / `vm.jsc_vm` are populated (spec VirtualMachine.zig:1313+); - /// returns the opaque per-VM runtime state pointer (or null). - pub init_runtime_state: - unsafe fn(vm: *mut VirtualMachine, opts: &mut InitOptions) -> RuntimeState, + /// returns the opaque per-VM runtime state pointer (or null). `Err` when + /// `Transpiler::init` fails (e.g. a deleted cwd → `getcwd` ENOENT); the + /// hook unwinds its own allocations, so [`VirtualMachine::init`] only has to + /// propagate the error (spec bubbles it via `try Transpiler.init(...)`). + pub init_runtime_state: unsafe fn( + vm: *mut VirtualMachine, + opts: &mut InitOptions, + ) -> Result, /// Reclaim the per-VM state boxed by `init_runtime_state`. Called from /// [`VirtualMachine::destroy`] (worker teardown) with the exact opaque /// pointer `init_runtime_state` returned (or null). The high tier @@ -2182,7 +2187,13 @@ impl VirtualMachine { // thread. Write through the raw `vm` ptr (not `vm_ref`) so no // `&mut VirtualMachine` is held live across the hook call — the // hook body itself dereferences `vm`. - unsafe { (*vm).runtime_state = (hooks.init_runtime_state)(vm, &mut opts) }; + // + // `?`: on `Err` (e.g. a deleted cwd → `getcwd` ENOENT out of + // `Transpiler::init`) the hook already unwound its own per-VM state, + // so abort `init` here — `vm.transpiler` was never written, and + // bailing out before the CLI reads it turns the old segfault into a + // clean error + non-zero exit (spec: `try Transpiler.init(...)`). + unsafe { (*vm).runtime_state = (hooks.init_runtime_state)(vm, &mut opts)? }; } // JSGlobalObject creation. Spec JSGlobalObject.zig:875 — the wrapper diff --git a/src/runtime/jsc_hooks.rs b/src/runtime/jsc_hooks.rs index c22b8034d3c..49d4e9610c2 100644 --- a/src/runtime/jsc_hooks.rs +++ b/src/runtime/jsc_hooks.rs @@ -267,13 +267,20 @@ unsafe fn ssl_ctx_cache_get_or_create( /// `configureDebugger()` — everything `VirtualMachine.init()` does that names /// a `bun_runtime` type. Spec VirtualMachine.zig:1313-1322. /// +/// Returns `Err` when `Transpiler::init` fails — most notably when the process +/// cwd was deleted, so `getcwd` yields `ENOENT` (spec `VirtualMachine.init` +/// bubbles the same error via `try Transpiler.init(...)`). The caller +/// propagates it out of `VirtualMachine::init`, and the CLI turns it into a +/// user-facing message + non-zero exit rather than reading a zeroed +/// `vm.transpiler`. +/// /// # Safety /// `vm` is the freshly-boxed unique VM on this thread, with `vm.global` / /// `vm.jsc_vm` already populated by `bun_jsc::VirtualMachine::init`. unsafe fn init_runtime_state( vm: *mut VirtualMachine, opts: &mut InitOptions, -) -> OpaqueRuntimeState { +) -> Result { // PORT NOTE: do NOT form `&mut *vm` here — the caller // (`VirtualMachine::init`) may still hold a `&mut VirtualMachine` to the // same allocation. Dereference per-field via the raw `vm` ptr if needed. @@ -331,11 +338,10 @@ unsafe fn init_runtime_state( .log .map(|p| p.as_ptr()) .unwrap_or(ptr::null_mut()); - // `bun_bundler::Transpiler::init` is now public (transpiler.rs); its body - // sub-gates the `BundleOptions::from_api` / `Resolver::init1` tail and - // returns `Err(Error::TODO)` until those surface, so the `Err` arm below - // is the live path for now. The `ptr::write` shape is load-bearing: do - // not replace with `(*vm).transpiler = ...` (drops zeroed bytes → UB). + // `bun_bundler::Transpiler::init` (transpiler.rs) returns `Ok` on the + // happy path; the `Err` arm below handles genuine failures (e.g. a deleted + // cwd → `getcwd` ENOENT). The `ptr::write` shape is load-bearing: do not + // replace with `(*vm).transpiler = ...` (drops zeroed bytes → UB). { use bun_options_types::schema::api; // Move (not clone) the caller's `TransformOptions` into the @@ -413,11 +419,27 @@ unsafe fn init_runtime_state( } Err(e) => { // Spec: `try Transpiler.init(...)` bubbles the error out of - // `VirtualMachine.init`. The hook signature has no error - // channel, so log + leave the field zeroed (validity-UB on - // first read — same failure mode as before this hook existed). - // TODO(b2): widen `init_runtime_state` return to `Result<_, Error>`. - bun_core::Output::err("Transpiler", "{}", format_args!("init failed: {e:?}")); + // `VirtualMachine.init` (VirtualMachine.zig:1241). The most + // common trigger is a deleted cwd → `getcwd` ENOENT + // (resolver/lib.rs). `vm.transpiler` was never written, so + // returning `Err` here leaves it as the zeroed bytes the low + // tier allocated — and the caller aborts `init` before anything + // reads the field, instead of surfacing it as a segfault. + // + // Unwind the per-VM state this hook set up before the + // `Transpiler::init` attempt: the `RuntimeState` box + its TLS + // slot (set above) and the thread-local AST stores that + // `Transpiler::init_in_place` `create()`d before it failed. + // Mirrors `deinit_runtime_state`, which is the teardown the + // `Ok` path would otherwise reach. + RUNTIME_STATE.with(|c| c.set(ptr::null_mut())); + // SAFETY: `state` is the unique `heap::into_raw` result from the + // top of this fn; the TLS slot was just nulled so no other live + // alias exists on this thread. + drop(unsafe { bun_core::heap::take(state.cast::()) }); + bun_ast::expr::data::Store::deinit(); + bun_ast::stmt::data::Store::deinit(); + return Err(e); } } } @@ -441,7 +463,7 @@ unsafe fn init_runtime_state( unsafe { configure_debugger(vm, &opts.debugger) }; } - state.cast() + Ok(state.cast()) } /// Spec VirtualMachine.zig:1335 `configureDebugger` — translate the CLI flag / @@ -627,25 +649,12 @@ unsafe fn load_preloads( // SAFETY: `vm.global` is set during `VirtualMachine::init` and outlives the VM. let global: *mut JSGlobalObject = unsafe { &*vm }.global; - // ── guard: zeroed transpiler ──────────────────────────────────────── - // `init_runtime_state` swallows `Transpiler::init`'s `Err` (logs + leaves - // `vm.transpiler` as zeroed bytes — see its `TODO(b2): widen return`). - // Spec VirtualMachine.zig:1240 uses `try Transpiler.init(...)`, so - // `loadPreloads` is unreachable with an invalid transpiler; in Rust we - // must check `fs.is_null()` to avoid null-deref UB on `--preload` until - // `Transpiler::init`'s gated tail un-gates and `init_runtime_state`'s - // return widens to `Result`. Fail loudly (PORTING.md §Forbidden: - // silent-no-op). - // SAFETY: per fn contract — reading the raw ptr field itself is fine; only - // the deref below would be UB on null. - if unsafe { &*vm }.transpiler.fs.is_null() { - bun_core::Output::err( - "preload", - "transpiler not initialized; ignoring --preload", - (), - ); - return Ok(ptr::null_mut()); - } + // `vm.transpiler` (hence `transpiler.fs`) is always initialized here: spec + // VirtualMachine.zig:1240 builds it with `try Transpiler.init(...)`, and the + // Rust port matches — `init_runtime_state` returns `Err` on `Transpiler::init` + // failure and `VirtualMachine::init` propagates it via `?`, so a VM that + // failed to build its transpiler never reaches `load_preloads` (this hook + // only runs via `reload_entry_point*`, which operate on an already-`Ok` VM). let top_level_dir: *const [u8] = Fs::FileSystem::get().top_level_dir; // Spec VirtualMachine.zig:2213 — `if (this.standalone_module_graph == null) // .read_only else .disable`. diff --git a/test/bundler/bun-build-compile.test.ts b/test/bundler/bun-build-compile.test.ts index 72d16e0c36f..3b19b938116 100644 --- a/test/bundler/bun-build-compile.test.ts +++ b/test/bundler/bun-build-compile.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "bun:test"; -import { bunEnv, bunExe, isArm64, isLinux, isMacOS, isMusl, isWindows, tempDir } from "harness"; +import { bunEnv, bunExe, isArm64, isLinux, isMacOS, isMusl, isPosix, isWindows, tempDir } from "harness"; import { chmodSync } from "node:fs"; import { join } from "path"; @@ -520,4 +520,62 @@ if (process.platform === "android") { }); } +// A standalone compiled binary bypasses `Arguments::parse` (no `--cwd`/global +// flags, no baked exec-argv), so `absolute_working_dir` stays unset and the +// FIRST `getcwd` of the whole startup is the one inside `Transpiler::init`. +// When the cwd has been deleted that `getcwd` fails with ENOENT; the bug was +// that the per-VM init hook swallowed the error and left `vm.transpiler` +// zeroed, so the next read (`configure_defines` → `run_env_loader`) hit a null +// deref and the binary crashed (the segfault users saw launching a compiled +// CLI from a directory that had been removed). It must instead exit cleanly +// with the ENOENT message. +// +// POSIX-only: a process can keep a deleted directory as its cwd until the last +// fd to it closes, whereas Windows refuses to remove a directory that is any +// process's cwd — so the scenario is unreachable there. The cwd has to be +// removed AFTER the process starts, which `Bun.spawn`'s `cwd` can't do, so a +// shell wrapper `cd`s in, `rmdir`s, then execs the binary (how a user hits it). +describe("compiled binary in a deleted cwd", () => { + test.if(isPosix)( + "exits cleanly instead of crashing", + async () => { + using dir = tempDir("build-compile-deleted-cwd", { + "app.js": `console.log("should-not-run");`, + }); + const outfile = join(String(dir), "app"); + + await using build = Bun.spawn({ + cmd: [bunExe(), "build", "--compile", join(String(dir), "app.js"), "--outfile", outfile], + env: bunEnv, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + const [, buildStderr, buildExit] = await Promise.all([build.stdout.text(), build.stderr.text(), build.exited]); + expect(buildStderr).not.toContain("error:"); + expect(buildExit).toBe(0); + + // A fresh directory to stand in and delete — NOT `dir`, which holds the + // compiled binary we still need to exec. + using cwdDir = tempDir("build-compile-gone-cwd", {}); + const gone = String(cwdDir); + + await using proc = Bun.spawn({ + cmd: ["/bin/sh", "-c", `cd "${gone}" && rmdir "${gone}" && exec "${outfile}"`], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + // The entry never runs (VM init aborts first), the ENOENT surfaces, and the + // process exits 1 — a crash would terminate via a signal, never exit 1. + expect(stdout).toBe(""); + expect(stderr).toContain("ENOENT"); + expect(exitCode).toBe(1); + }, + 60_000, + ); +}); + // file command test works well