diff --git a/src/js_parser/p.rs b/src/js_parser/p.rs index daf1e90b0626..ee96e8472e4c 100644 --- a/src/js_parser/p.rs +++ b/src/js_parser/p.rs @@ -1002,6 +1002,8 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool, const SEMA: bool> self.options.allow_unresolved, crate::parser::options::AllowUnresolved::All ) + // After a stack overflow the visit skipped a part of the arguments. + || self.reported_stack_overflow.get() { return Ok(()); } @@ -1623,7 +1625,8 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool, const SEMA: bool> return self.transpose_require_resolve_known_string(arg); } - if self.options.warn_about_unbundled_modules { + // After a stack overflow the visit skipped a part of the arguments. + if self.options.warn_about_unbundled_modules && !self.reported_stack_overflow.get() { // Use a debug log so people can see this if they want to let r = js_lexer::range_of_identifier(self.source, arg.loc); self.log() @@ -7517,6 +7520,9 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool, const SEMA: bool> } } } + // The visit skipped this method. + js_ast::ExprData::EMissing(_) + if self.reported_stack_overflow.get() => {} _ => unreachable!(), } } @@ -7880,6 +7886,12 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool, const SEMA: bool> ) { use js_ast::g::PropertyKind; + // The visit skipped this method. + if self.reported_stack_overflow.get() && prop.value.is_some_and(|value| value.is_missing()) + { + return; + } + // Local helper: bump-alloc an arg pair and call __legacyMetadataTS. // pulled out of the per-arm code to cut a ~3x repetition. macro_rules! push_metadata { @@ -8616,7 +8628,8 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool, const SEMA: bool> /// /// This function replaces all specifier strings with `e_special.resolved_specifier_string` pub(crate) fn handle_import_meta_hot_accept_call(&mut self, call: &mut E::Call) { - if call.args.len_u32() == 0 { + // After a stack overflow the visit skipped a part of the arguments. + if call.args.len_u32() == 0 || self.reported_stack_overflow.get() { return; } // match `data` by value (it is `Copy`) so the `StoreRef<_>` diff --git a/src/js_parser/visit/visit_expr.rs b/src/js_parser/visit/visit_expr.rs index 59748411dabf..dcb6d78fac4c 100644 --- a/src/js_parser/visit/visit_expr.rs +++ b/src/js_parser/visit/visit_expr.rs @@ -44,6 +44,10 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool, const SEMA: bool> pub(crate) fn visit_expr_in_out(&mut self, e: &mut Expr, in_: ExprIn) { if !self.stack_check.is_safe_to_recurse() || self.reported_stack_overflow.get() { + // The caller reads `e` as visited. A revisit can be inside a shared `define` value. + if !self.is_revisit_for_substitution { + e.data = Data::EMissing(E::Missing {}); + } self.report_stack_overflow(e.loc); return; } @@ -691,7 +695,10 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool, const SEMA: bool> if let Some(ref_) = ref_ && !p.options.features.is_macro_runtime { - if let Some(macro_ref_data) = p.macro_.refs.get(&ref_).copied() { + // After a stack overflow the visit skipped a part of the template. + if let Some(macro_ref_data) = p.macro_.refs.get(&ref_).copied() + && !p.reported_stack_overflow.get() + { p.ignore_usage(ref_); if p.is_control_flow_dead { *e = p.new_expr(E::Undefined {}, e_.tag.unwrap().loc); @@ -2199,7 +2206,8 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool, const SEMA: bool> return; } - if p.options.warn_about_unbundled_modules { + // After a stack overflow the visit skipped a part of the arguments. + if p.options.warn_about_unbundled_modules && !p.reported_stack_overflow.get() { let r = js_lexer::range_of_identifier(p.source, e_.target.loc); p.log() .add_range_debug( @@ -2275,7 +2283,11 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool, const SEMA: bool> } if Self::ALLOW_MACROS { - if is_macro_ref && !p.options.features.is_macro_runtime { + // After a stack overflow the visit skipped a part of the arguments. + if is_macro_ref + && !p.options.features.is_macro_runtime + && !p.reported_stack_overflow.get() + { let ref_ = match &e_.target.data { Data::EImportIdentifier(ident) => ident.ref_, Data::EDot(dot) => { @@ -2505,7 +2517,8 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool, const SEMA: bool> } // If control flow is dead, just return false without validation errors - if p.is_control_flow_dead { + // After a stack overflow the visit skipped a part of the arguments. + if p.is_control_flow_dead || p.reported_stack_overflow.get() { return Some(p.new_expr(E::Boolean { value: false }, loc)); } diff --git a/test/bundler/transpiler/transpiler.test.js b/test/bundler/transpiler/transpiler.test.js index 2ec82493d4ff..6d0c86068d5e 100644 --- a/test/bundler/transpiler/transpiler.test.js +++ b/test/bundler/transpiler/transpiler.test.js @@ -5412,6 +5412,242 @@ it("deeply nested expressions error instead of crashing the process", () => { expect([exitCode, signalCode ?? undefined]).toEqual([0, undefined]); }, 60_000); +// After a stack overflow the visit pass skips each expression that it did not visit yet. The code +// that runs after a skipped visit must not read that expression. The tests with a child process +// are not concurrent: a debug build is slow when several children fill their stacks at once. +describe("a stack overflow in the visit pass", () => { + const overflow = "Maximum call stack size exceeded"; + const repeat = (fill, count) => Buffer.alloc(fill.length * count, fill).toString(); + const messageOf = call => { + try { + call(); + return "no error"; + } catch (e) { + return e.message; + } + }; + // A number of member levels at which the visit pass of this build overflows the stack. + let limit = 1024; + while (messageOf(() => new Bun.Transpiler({ loader: "js" }).transformSync("a" + repeat(".b", limit))) !== overflow) { + if (limit >= 1 << 22) throw new Error("no overflow at " + limit + " levels"); + limit *= 2; + } + // The visit pass overflows the stack in this member chain, and skips what comes after it. + const chain = "a" + repeat(".b", 2 * limit); + // The visit pass gives a skipped identifier no symbol. The length of this name is greater + // than the number of symbols in each module below. + const name = repeat("L", 2000); + + const prelude = ` + const repeat = (fill, count) => Buffer.alloc(fill.length * count, fill).toString(); + const overflow = "${overflow}"; + const chain = "a" + repeat(".b", ${2 * limit}); + const name = repeat("L", ${name.length}); + // The message of the error that the call throws. + const outcome = async call => { + try { + await call(); + return "no error"; + } catch (e) { + return e.message; + } + }; + // The smallest number of levels at which the visit of build(levels) overflows. This also + // parses each of the next "after" numbers of levels. + const firstOverflow = async (transpiler, build, after = 0) => { + const overflows = async levels => { + const message = await outcome(() => transpiler.transformSync(build(levels))); + // A smaller number of levels can overflow the printer. + if (message !== overflow && message !== "no error" && !message.includes("StackOverflow")) { + throw new Error(levels + " levels: " + message); + } + return message === overflow; + }; + let below = 0; + let first = 1024; + while (!(await overflows(first))) { + if (first >= 1 << 22) throw new Error("no overflow at " + first + " levels"); + below = first; + first *= 2; + } + while (first - below > 1) { + const middle = (below + first) >> 1; + if (await overflows(middle)) first = middle; + else below = middle; + } + for (let levels = first + 1; levels <= first + after; levels++) await overflows(levels); + return first; + }; + `; + + // Runs bun with the arguments, in a directory with the files. + async function spawn(args, files = {}) { + using dir = tempDir("visit-overflow", files); + await using proc = Bun.spawn({ + cmd: [bunExe(), ...args], + env: bunEnv, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + return { stdout, stderr, exitCode, signalCode: proc.signalCode }; + } + const run = script => spawn(["-e", prelude + script]); + const ok = { stdout: "ok\n", stderr: "", exitCode: 0, signalCode: null }; + // What a bun command that fails with the one error gives. + const oneError = ({ stderr, ...rest }) => ({ errors: stderr.match(/^error: .*$/gm), ...rest }); + const failed = { errors: ["error: " + overflow], stdout: "", exitCode: 1, signalCode: null }; + + it.each(["js", "ts"])("is the one error for a skipped default value or typeof operand, %s", async loader => { + const result = await run(` + const cases = [ + ["[" + chain + ", " + name + " = 1] = y;", "transformSync", "transform", "scan"], + ["({ a: " + chain + ", " + name + " = 1 } = y);", "transformSync"], + ["x = typeof (false ? " + chain + " : " + name + ");", "transformSync"], + ]; + for (const [source, ...methods] of cases) { + for (const method of methods) { + const transpiler = new Bun.Transpiler({ loader: "${loader}" }); + const message = await outcome(() => transpiler[method](source)); + if (message !== overflow) throw new Error(method + " of " + source.slice(-32) + ": " + message); + if (transpiler.transformSync("f(1);") !== "f(1);\\n") throw new Error("the instance does not work"); + } + } + console.log("ok"); + `); + expect(result).toEqual(ok); + }); + + it("is the one error for a TypeScript class with a method after it", async () => { + const result = await run(` + const legacyDecorators = { compilerOptions: { experimentalDecorators: true, emitDecoratorMetadata: true } }; + const cases = [ + ["class A { a = " + chain + "; m() {} }"], + ["export default class { a = " + chain + "; get g() { return 1; } }"], + ["class A { a = " + chain + "; @dec m(@dec x: number): void {} }", legacyDecorators], + ["class A { @dec a = " + chain + "; constructor() {} }"], + ]; + for (const [source, tsconfig] of cases) { + const transpiler = new Bun.Transpiler({ loader: "ts", tsconfig }); + const message = await outcome(() => transpiler.transformSync(source)); + if (message !== overflow) throw new Error(source.slice(-40) + ": " + message); + } + console.log("ok"); + `); + expect(result).toEqual(ok); + }); + + it("is the one error for require() and require.resolve() at the debug log level", () => { + // At this log level, a require() or require.resolve() of something that is not a string + // literal logs a note, and the transpiler then throws "Parse error" with the list of messages. + const transpiler = new Bun.Transpiler({ loader: "js", target: "node", logLevel: "debug" }); + const calls = [ + `require(${chain})`, + `require.resolve(${chain} ? "./d" : "./e")`, + `require.resolve(y ? ${chain} : "./d")`, + ]; + const messages = calls.map(call => messageOf(() => transpiler.transformSync(`x = ${call};`))); + expect(messages).toEqual([overflow, overflow, overflow]); + }); + + // The identifier is the deepest expression of a member chain. At the first number of levels + // that overflows, the visit of the identifier is the first visit that is skipped. The right + // side of the assignment is deeper, so there the identifier is the first a few levels later. + it.each([ + ["a call", `levels => name + "()" + repeat(".b", levels)`], + ["an assignment", `levels => "(" + name + " = 1)" + repeat(".b", levels)`], + ])("is the one error at the depth where it starts, %s", async (_, build) => { + const result = await run(` + await firstOverflow(new Bun.Transpiler({ loader: "js" }), ${build}, 4); + console.log("ok"); + `); + expect(result).toEqual(ok); + }); + + it("does not change a define value that the minifier visits again", async () => { + const result = await run(` + // A define value lives as long as the instance. The minifier puts "y" in the place of "x" + // and visits the statement again. Only that second visit goes into the value of X. + const depth = 200; + const transpiler = new Bun.Transpiler({ + loader: "js", + define: { X: repeat('{"a":', depth) + "[1,2,3]" + repeat("}", depth) }, + minify: { syntax: true }, + deadCodeElimination: true, + }); + const printed = () => transpiler.transformSync("export default X;"); + const before = printed(); + const build = target => levels => "function f() { let x = y; return x + " + target + repeat(".b", levels) + "; }"; + // The overflow is in the value of X: a chain of 2 more levels without the value has none. + const levels = await firstOverflow(transpiler, build("X")); + const alone = await outcome(() => transpiler.transformSync(build("Z")(levels + 2))); + if (alone === overflow) throw new Error("no overflow in the define value"); + if (printed() !== before) throw new Error("the define value changed"); + console.log("ok"); + `); + expect(result).toEqual(ok); + }); + + it("is the one error for a React Fast Refresh signature with a hook argument after it", async () => { + const result = await spawn(["build", "--react-fast-refresh", "--external", "react", "component.jsx"], { + "component.jsx": ` + import { useReducer } from "react"; + export function Component() { + const [state] = useReducer(${chain}, [${name}]); + return state; + } + `, + }); + expect(oneError(result)).toEqual(failed); + }); + + it.concurrent.each([ + ["require()", `x = require(${chain});`], + ["require.resolve()", `x = require.resolve(${chain});`], + ["feature()", `import { feature } from "bun:bundle";\nif (feature(${chain})) {}`], + ["a macro", `import { m } from "./macro.ts" with { type: "macro" };\nexport const x = m(${chain});`], + [ + "a macro with a template", + `import { m } from "./macro.ts" with { type: "macro" };\nexport const x = m\`\${${chain}}\`;`, + ], + // Only the module format of the dev server keeps import.meta.hot. + [ + "import.meta.hot.accept()", + `import "./dep.ts";\nimport.meta.hot.accept([${chain}, "./dep.ts"], () => {});`, + "internal_bake_dev", + ], + ])("is the one error for the argument of %s", async (_, source, format = "esm") => { + using dir = tempDir("visit-overflow-argument", { + "entry.ts": source, + "macro.ts": "export function m(...args) { return args.length; }", + "dep.ts": "export default 1;", + }); + const result = await Bun.build({ + entrypoints: [join(String(dir), "entry.ts")], + // With an empty list, a specifier that is not a string literal is an error. + allowUnresolved: [], + format, + throw: false, + }); + const errors = result.logs.filter(log => log.level === "error").map(log => log.message); + expect(errors).toEqual([overflow]); + }); + + it("is the one error for a skipped default value in a file that runs", async () => { + const result = await spawn(["index.js"], { "index.js": `[${chain}, ${name} = 1] = globalThis.y;` }); + expect(oneError(result)).toEqual(failed); + }); + + // The visit pass makes no part for a statement that it skips, and `_parse` moves the part of a + // hoistable statement to the top: panic: unreachable. https://github.com/oven-sh/bun/pull/44137 + it.todo("is the one error for a hoistable statement after it in a file that runs", async () => { + const source = `export default function f() { return ${chain}; }\nclass B {}`; + const result = await spawn(["index.js"], { "index.js": source }); + expect(oneError(result)).toEqual(failed); + }); +}); + it("deeply nested TypeScript types error instead of crashing the process", () => { const script = ` const repeat = (fill, count) => Buffer.alloc(fill.length * count, fill).toString();