Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
6 changes: 4 additions & 2 deletions src/js_parser/p.rs
Original file line number Diff line number Diff line change
Expand Up @@ -345,8 +345,10 @@ pub struct P<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> {
/// 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`.
/// would complete and hand a 15k-deep AST to the visitor/printer, whose
/// per-level frames are far larger (they carry their own stack checks in
/// `visit_and_append_stmt`/`print_stmt`, but the cap keeps the failure
/// deterministic). Same `MAX_STMT_DEPTH` rationale as `interchange/json.rs`.
pub parse_stmt_depth: u32,

pub reported_stack_overflow: core::cell::Cell<bool>,
Expand Down
10 changes: 6 additions & 4 deletions src/js_parser/parse/parse_stmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1897,8 +1897,9 @@ 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.
// frames never exhaust it) still rejects absurd nesting deterministically
// instead of deferring to the visitor/printer stack checks.
// See `P::parse_stmt_depth` field doc.
if self.parse_stmt_depth >= MAX_STMT_DEPTH || !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.
Expand Down Expand Up @@ -1941,6 +1942,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
}
}

/// 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.
/// See `P::parse_stmt_depth` — a deterministic upper bound; depths below it are
/// still guarded by the per-level stack checks in the visitor/printer, whose
/// frames are much larger than `parse_stmt`'s.
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
5 changes: 5 additions & 0 deletions src/js_printer/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5196,6 +5196,11 @@
}

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(());
}

Check warning on line 5202 in src/js_printer/lib.rs

View check run for this annotation

Claude / Claude Code Review

print_if self-recursion bypasses the new print_stmt stack guard

Nit: `print_if` self-recurses on else-if chains (lib.rs:6727-6728) without going through `print_stmt`, so the new stack guard here doesn't cover `if(x){} else if(x){} else if(x){} ...` — and `t_if` parses those in a loop, so `MAX_STMT_DEPTH` doesn't bound them either. In the same-thread pipeline the visitor's new guard trips first so this is defense-in-depth, but it's the same "printer on a thread with less stack headroom" rationale you gave for guarding `print_stmt` — consider adding the same `
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
37 changes: 37 additions & 0 deletions test/bundler/transpiler/transpiler.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4044,6 +4044,43 @@ it("deeply nested expressions 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),
];
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]);
}, 60_000);
Comment thread
robobun marked this conversation as resolved.
Outdated

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