Skip to content

Commit 2017304

Browse files
resolver: restore package_manager.log after resolve to avoid dangling stack pointer (#31020)
## What `resolveMaybeNeedsTrailingSlash` swaps `vm.log` / `resolver.log` to a stack-local `Log` for the duration of `_resolve`, then restores them via a drop guard. The Zig original also swaps and restores `transpiler.linker.log` and `resolver.package_manager.log`; the Rust port had those behind a `TODO(b2-cycle)` and only handled `vm.log` + `resolver.log`. When auto-install is enabled and the resolver lazily creates the `PackageManager` during `_resolve`, `Resolver::get_package_manager` seeds `pm.log` from `resolver.log` — which at that point is the **stack-local** `Log`. Because the restore guard never touched `pm.log`, it was left pointing into a dead stack frame after the function returned. The next resolve at a different stack depth that routes through the auto-install task runner dereferenced that stale pointer in `Log::add_error_fmt`, tripping ASAN's `stack-use-after-scope` (or segfaulting / executing garbage in release builds). Stack at the fault: ``` #0 bun_ast::Log::add_formatted_msg #1 bun_ast::Log::add_error_fmt #2 bun_install::…::run_tasks #7 bun_install::…::enqueue_dependency_to_root #9 bun_resolver::Resolver::enqueue_dependency_to_resolve #14 bun_resolver::Resolver::resolve_and_auto_install #15 bun_jsc::VirtualMachine::_resolve #16 bun_jsc::VirtualMachine::resolve_maybe_needs_trailing_slash::<true> ``` ## Fix Swap and restore `linker.log` and (when present) `package_manager.log` in both copies of the resolve log guard (`VirtualMachine::resolve_maybe_needs_trailing_slash` and `jsc_hooks::resolve_hook`), matching `VirtualMachine.zig`. The restore re-checks `resolver.package_manager` at drop time so a PM that was lazily created during `_resolve` is also pointed back at the VM log. Also adds the missing `<cassert>` include in `wtf-bindings.cpp`, which stopped being pulled in transitively. ## Repro ```js // run from an empty dir with // BUN_CONFIG_INSTALL=fallback BUN_CONFIG_REGISTRY=http://127.0.0.1:1 const realm = new ShadowRealm(); const variants = [ () => realm.importValue("pkg-not-found-a", "x"), () => (() => realm.importValue("pkg-not-found-b", "x"))(), () => (() => (() => realm.importValue("pkg-not-found-c", "x"))())(), () => import("pkg-not-found-f"), ]; for (let i = 0; i < 100; i++) for (const v of variants) try { v()?.catch?.(() => {}); } catch {} ``` Segfaults on `main`, clean after this change. Fixes #14432 Fixes #22407 --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
1 parent 2f15816 commit 2017304

3 files changed

Lines changed: 93 additions & 26 deletions

File tree

src/jsc/VirtualMachine.rs

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4259,8 +4259,12 @@ impl VirtualMachine {
42594259
let mut log = bun_ast::Log::default();
42604260
jsc_vm.log = NonNull::new(&raw mut log);
42614261
jsc_vm.transpiler.resolver.log = NonNull::from(&mut log);
4262-
// TODO(b2-cycle): `transpiler.linker.log` / `resolver.package_manager.log`
4263-
// — gated bundler fields.
4262+
jsc_vm.transpiler.linker.log = &raw mut log;
4263+
if let Some(pm) = jsc_vm.transpiler.resolver.package_manager {
4264+
// SAFETY: the `dyn AutoInstaller` is always `PackageManager`
4265+
// (sole impl — see `VirtualMachine::package_manager`).
4266+
unsafe { (*pm.cast::<bun_install::PackageManager>().as_ptr()).log = &raw mut log };
4267+
}
42644268
// PORT NOTE: Zig `defer { restore old_log }` — fires on every exit
42654269
// (including `?` from `ResolveMessage::create` below), so the VM's
42664270
// `log` cannot be left pointing at the dropped stack `log`. Hand-roll
@@ -4278,6 +4282,17 @@ impl VirtualMachine {
42784282
let jsc_vm = self.vm.get().as_mut();
42794283
jsc_vm.log = Some(self.old_log);
42804284
jsc_vm.transpiler.resolver.log = self.old_log;
4285+
jsc_vm.transpiler.linker.log = self.old_log.as_ptr();
4286+
// `_resolve` may have lazily created the PM with
4287+
// `pm.log = resolver.log` (our stack `log`), so restore even
4288+
// if it was `None` when we swapped.
4289+
if let Some(pm) = jsc_vm.transpiler.resolver.package_manager {
4290+
// SAFETY: sole `dyn AutoInstaller` impl is `PackageManager`.
4291+
unsafe {
4292+
(*pm.cast::<bun_install::PackageManager>().as_ptr()).log =
4293+
self.old_log.as_ptr();
4294+
}
4295+
}
42814296
}
42824297
}
42834298
let _restore = RestoreLog {

src/runtime/jsc_hooks.rs

Lines changed: 17 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -2238,31 +2238,18 @@ fn transpile_source_code_inner(
22382238
let args_log_nn = core::ptr::NonNull::new(args.log).expect("args.log is non-null");
22392239
unsafe {
22402240
(*jsc_vm).transpiler.log = args.log;
2241-
{
2242-
(*jsc_vm).transpiler.resolver.log = args_log_nn;
2243-
}
2244-
// TODO(b2-blocked): `Linker` is a unit stub in `bun_bundler`
2245-
// — `.log` field un-gates with `linker.rs`.
2246-
2247-
{
2248-
(*jsc_vm).transpiler.linker.log = args.log;
2249-
if let Some(pm) = (*jsc_vm).transpiler.resolver.package_manager {
2250-
// TODO(blocked_on): bun_resolver::package_json::PackageManager::log
2251-
// — the resolver-side stub only exposes `lockfile`/`on_wake`.
2252-
let _ = pm;
2253-
}
2241+
(*jsc_vm).transpiler.resolver.log = args_log_nn;
2242+
(*jsc_vm).transpiler.linker.log = args.log;
2243+
if let Some(pm) = (*jsc_vm).transpiler.resolver.package_manager {
2244+
(*pm.cast::<bun_install::PackageManager>().as_ptr()).log = args.log;
22542245
}
22552246
}
22562247
let _log_guard = scopeguard::guard(jsc_vm, move |jsc_vm| unsafe {
22572248
(*jsc_vm).transpiler.log = old_log;
2258-
2259-
{
2260-
(*jsc_vm).transpiler.resolver.log = old_log_nn;
2261-
(*jsc_vm).transpiler.linker.log = old_log;
2262-
if let Some(pm) = (*jsc_vm).transpiler.resolver.package_manager {
2263-
// TODO(blocked_on): bun_resolver::package_json::PackageManager::log
2264-
let _ = pm;
2265-
}
2249+
(*jsc_vm).transpiler.resolver.log = old_log_nn;
2250+
(*jsc_vm).transpiler.linker.log = old_log;
2251+
if let Some(pm) = (*jsc_vm).transpiler.resolver.package_manager {
2252+
(*pm.cast::<bun_install::PackageManager>().as_ptr()).log = old_log;
22662253
}
22672254
});
22682255

@@ -5029,17 +5016,23 @@ unsafe fn resolve_hook(
50295016
(*vm).log = Some(log_nn);
50305017
(*vm).transpiler.resolver.log = log_nn;
50315018
(*vm).transpiler.linker.log = log_nn.as_ptr();
5032-
// TODO(b2-cycle): `transpiler.resolver.package_manager` log swap —
5033-
// gated alongside the PM field (see transpile_source_code §log-swap).
5019+
if let Some(pm) = (*vm).transpiler.resolver.package_manager {
5020+
(*pm.cast::<bun_install::PackageManager>().as_ptr()).log = log_nn.as_ptr();
5021+
}
50345022
}
50355023
scopeguard::defer! {
50365024
// SAFETY: `vm` is the live per-thread VM; restoring the log pointers
50375025
// swapped just above so early-return paths don't leave a dangling
5038-
// stack pointer.
5026+
// stack pointer. The PM may have been lazily created inside
5027+
// `_resolve` with `pm.log = resolver.log` (our stack `log`), so
5028+
// restore it even if it was `None` at swap time.
50395029
unsafe {
50405030
(*vm).log = Some(old_log);
50415031
(*vm).transpiler.resolver.log = old_log;
50425032
(*vm).transpiler.linker.log = old_log.as_ptr();
5033+
if let Some(pm) = (*vm).transpiler.resolver.package_manager {
5034+
(*pm.cast::<bun_install::PackageManager>().as_ptr()).log = old_log.as_ptr();
5035+
}
50435036
}
50445037
}
50455038

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
import { expect, test } from "bun:test";
2+
import { bunEnv, bunExe, tempDir } from "harness";
3+
4+
// When the runtime resolver's auto-install path lazily creates the
5+
// PackageManager, it snapshots `resolver.log` into `pm.log`. If the
6+
// PackageManager is created while the resolver log is pointed at a
7+
// stack-local `Log` inside `resolveMaybeNeedsTrailingSlash`, `pm.log` must be
8+
// restored to the VM log before returning, otherwise the next resolve at a
9+
// different stack depth dereferences dead stack memory when the HTTP task
10+
// fails and calls `manager.log_mut().add_error_fmt(...)`.
11+
test("repeated failing auto-install resolves at varying stack depth don't read a dangling pm.log", async () => {
12+
// Run from an empty dir so the resolver falls through to the auto-install
13+
// path instead of stopping at the repo's own node_modules.
14+
using dir = tempDir("resolve-autoinstall-log", {});
15+
16+
const src = `
17+
const realm = new ShadowRealm();
18+
const variants = [
19+
() => realm.importValue("pkg-not-found-a", "x"),
20+
() => (() => realm.importValue("pkg-not-found-b", "x"))(),
21+
() => (() => (() => realm.importValue("pkg-not-found-c", "x"))())(),
22+
() => { try { return realm.importValue("pkg-not-found-d", "x"); } finally {} },
23+
() => [realm.importValue("pkg-not-found-e", "x")][0],
24+
() => import("pkg-not-found-f"),
25+
() => (async () => realm.importValue("pkg-not-found-g", "x"))(),
26+
];
27+
for (let i = 0; i < 100; i++) {
28+
for (const v of variants) {
29+
try {
30+
const p = v();
31+
if (p && p.catch) p.catch(() => {});
32+
} catch {}
33+
}
34+
}
35+
Bun.gc(true);
36+
console.log("ok");
37+
`;
38+
39+
await using proc = Bun.spawn({
40+
// Force the auto-install path (PackageManager lazy init inside the
41+
// resolver) without hitting the network — the registry hostname is
42+
// unroutable, so every enqueued manifest fetch fails fast and routes
43+
// through `manager.log_mut()`.
44+
cmd: [bunExe(), "--install=fallback", "-e", src],
45+
cwd: String(dir),
46+
env: {
47+
...bunEnv,
48+
BUN_CONFIG_REGISTRY: "http://127.0.0.1:1",
49+
},
50+
stdout: "pipe",
51+
stderr: "pipe",
52+
});
53+
54+
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
55+
56+
expect(stderr).toBe("");
57+
expect(stdout.trim()).toBe("ok");
58+
expect(exitCode).toBe(0);
59+
});

0 commit comments

Comments
 (0)