Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 8 additions & 11 deletions src/js_parser/p.rs
Original file line number Diff line number Diff line change
Expand Up @@ -338,16 +338,6 @@
pub has_commonjs_export_names: bool,

pub stack_check: bun_core::StackCheck,
/// Hard recursion cap for `parse_stmt`. Zig relies on `stack_check` alone,
/// but its `parseStmt` uses an `inline` switch that pulls every `t_*`
/// handler into one multi-KB frame, so 15k nested statements exhaust the
/// 18 MB Windows stack and trip `is_safe_to_recurse()`. Rust dispatches to
/// out-of-line `t_*` fns; the `parse_stmt`→`t_for` cycle is only a few
/// hundred bytes, so the 15k-level `lots-of-for-loop.js` fixture (~4 MB)
/// never trips the 256 KB threshold on Windows' 18 MB worker stack — parse
/// completes, then the (uncapped) visitor/printer recurse 15k times and
/// hard-overflow. Same `MAX_STMT_DEPTH` rationale as `interchange/json.rs`.
pub parse_stmt_depth: u32,

pub reported_stack_overflow: core::cell::Cell<bool>,

Expand Down Expand Up @@ -3342,6 +3332,14 @@
}

fn hoist_symbols(&mut self, mut scope: js_ast::StoreRef<js_ast::Scope>) {
// This runs before the visit pass, so it walks the scope tree at the full
// nesting depth the parser allowed; deep trees must error here instead of
// overflowing the stack.
if !self.stack_check.is_safe_to_recurse() || self.reported_stack_overflow.get() {
self.report_stack_overflow(bun_ast::Loc::EMPTY);
return;
}

Check warning on line 3341 in src/js_parser/p.rs

View check run for this annotation

Claude / Claude Code Review

recursive_set_strict_mode still unguarded after MAX_STMT_DEPTH removal

Nit: the resolved review thread (3295321360) flagged both `hoist_symbols` *and* `recursive_set_strict_mode` as unguarded scope-tree walks exposed by removing `MAX_STMT_DEPTH`; commit 6a694997 guarded `hoist_symbols` here but `recursive_set_strict_mode` (src/ast/scope.rs:244-251) is still unguarded and runs at p.rs:3171/3174/3177 — *before* this guard — for any source with an `import`/`export`/top-level `await`. Its per-level frame is tiny (one enum compare + assignment + slice iterator, ~50-80B
Comment thread
robobun marked this conversation as resolved.

// `StoreRef` is the arena back-pointer with safe `Deref`/`DerefMut` —
// scope is arena-owned and valid for the parser 'a lifetime; the visit
// pass is single-threaded so no aliasing `&mut` is outstanding. Read the
Expand Down Expand Up @@ -9244,7 +9242,6 @@
named_exports: Default::default(),
log,
stack_check: bun_core::StackCheck::init(),
parse_stmt_depth: 0,
reported_stack_overflow: core::cell::Cell::new(false),
ts_infer_constraint_backtracks: Vec::new(),
arena,
Expand Down
17 changes: 3 additions & 14 deletions src/js_parser/parse/parse_stmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1895,21 +1895,16 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
}

pub fn parse_stmt(&mut self, opts: &mut ParseStatementOptions<'a>) -> Result<Stmt> {
// PORT NOTE: Zig only checks `stack_check`; the hard cap is added so
// Windows' 18 MB worker stack (where the small Rust `parse_stmt`→`t_*`
// frames never exhaust it) still throws before the uncapped visitor/
// printer pass hard-overflows. See `P::parse_stmt_depth` field doc.
if self.parse_stmt_depth >= MAX_STMT_DEPTH || !self.stack_check.is_safe_to_recurse() {
if !self.stack_check.is_safe_to_recurse() {
// TODO(port): bun_core::throw_stack_overflow() not yet exported; map to a SyntaxError
// until the StackOverflow error variant lands.
return Err(err!("StackOverflow"));
}
Comment thread
claude[bot] marked this conversation as resolved.
self.parse_stmt_depth += 1;

// Zig used `inline ... => |function| @field(@This(), @tagName(function))(...)` to dispatch
// by token name via comptime reflection. Rust has no `@field`/`@tagName`; expand the arms.
let loc = self.lexer.loc();
let result = match self.lexer.token {
match self.lexer.token {
T::TSemicolon => Self::t_semicolon(self),
T::TAt => Self::t_at(self, opts),

Expand All @@ -1935,12 +1930,6 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
T::TOpenBrace => Self::t_open_brace(self, opts, loc),

_ => Self::parse_stmt_fallthrough(self, opts, loc),
};
self.parse_stmt_depth -= 1;
result
}
}
}

/// See `P::parse_stmt_depth` — sized so the visitor/printer (larger per-level
/// frames, no stack check) fit on the smallest 4 MB POSIX worker stack.
const MAX_STMT_DEPTH: u32 = 1000;
8 changes: 8 additions & 0 deletions src/js_parser/visit/visit_stmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,14 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
stmts: &mut StmtList<'a>,
stmt: &mut Stmt,
) -> Result<(), Error> {
// Statements nest arbitrarily deep (e.g. hundreds of `{` blocks), and each
// level stacks a `visit_stmts` + `s_*` frame, so guard here like
// `visit_expr_in_out` does for expressions.
if !self.stack_check.is_safe_to_recurse() || self.reported_stack_overflow.get() {
self.report_stack_overflow(stmt.loc);
return Ok(());
}

let p = self;
// By default any statement ends the const local prefix
let was_after_after_const_local_prefix = p.cur_scope().is_after_const_local_prefix;
Expand Down
12 changes: 12 additions & 0 deletions src/js_printer/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5235,6 +5235,11 @@ pub mod __gated_printer {
}

pub fn print_stmt(&mut self, stmt: Stmt, tlmtlo: TopLevel) -> Result<(), bun_core::Error> {
if !self.stack_check.is_safe_to_recurse() {
self.stack_overflowed = true;
return Ok(());
}
Comment thread
robobun marked this conversation as resolved.

let prev_stmt_tag = self.prev_stmt_tag;
// Zig: `defer { p.prev_stmt_tag = std.meta.activeTag(stmt.data); }`
// PORT NOTE: reshaped for borrowck — scopeguard would hold `&mut self.prev_stmt_tag`
Expand Down Expand Up @@ -6691,6 +6696,13 @@ pub mod __gated_printer {
}

pub fn print_if(&mut self, s: &S::If, loc: bun_ast::Loc, tlmtlo: TopLevel) {
// `else if` chains recurse here directly without passing through
// `print_stmt`, so they need their own guard.
if !self.stack_check.is_safe_to_recurse() {
self.stack_overflowed = true;
return;
}

self.print_space_before_identifier();
self.add_source_mapping(loc);
self.print(b"if");
Expand Down
38 changes: 38 additions & 0 deletions test/bundler/transpiler/transpiler.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4130,6 +4130,44 @@ it("deeply nested TypeScript types error instead of crashing the process", () =>
expect([exitCode, signalCode ?? undefined]).toEqual([0, undefined]);
}, 60_000);

it("deeply nested statement blocks error instead of crashing the process", () => {
const script = `
const repeat = (fill, count) => Buffer.alloc(fill.length * count, fill).toString();
const shapes = [
n => repeat("{", n) + 'class Test1 { static "prop1" = 0; }' + repeat("}", n),
n => repeat("{", n) + "let x = 1;" + repeat("}", n),
n => repeat("if (x) {", n) + "y();" + repeat("}", n),
n => "if (x) { y(); }" + repeat(" else if (x) { y(); }", n),
];
const check = (transpiler, src) => {
try {
transpiler.transformSync(src);
} catch (e) {
if (!/Maximum call stack size exceeded|StackOverflow/.test(String(e?.message))) throw e;
}
};
for (const shape of shapes) {
for (const n of [600, 800, 990]) {
check(
new Bun.Transpiler({ loader: "tsx", target: "bun", minifyWhitespace: true, deadCodeElimination: true }),
shape(n),
);
check(new Bun.Transpiler({ loader: "js" }), shape(n));
}
}
console.log("depth-ok");
`;
const { stdout, exitCode, signalCode } = Bun.spawnSync({
cmd: [bunExe(), "-e", script],
stdout: "pipe",
stderr: "pipe",
env: bunEnv,
});

expect(stdout.toString()).toBe("depth-ok\n");
expect([exitCode, signalCode ?? undefined]).toEqual([0, undefined]);
});

it("running a file with deeply nested unary operators does not crash the process", () => {
const code = Buffer.alloc(2 * 4000, "- ").toString() + "1";
const { exitCode, signalCode } = Bun.spawnSync({
Expand Down
Loading