diff --git a/src/bundler/LinkerContext.rs b/src/bundler/LinkerContext.rs index bc47073ee17d..ffc0cbdeacd5 100644 --- a/src/bundler/LinkerContext.rs +++ b/src/bundler/LinkerContext.rs @@ -1582,6 +1582,9 @@ pub(crate) enum MatchImportKind { ProbablyTypescriptType, /// The import resolved to multiple symbols via "export * from" Ambiguous, + /// Tracing the import's "export * from" alternatives would have + /// overflowed the stack + StackOverflow, } pub struct ChunkMeta { @@ -3490,7 +3493,12 @@ impl<'a> LinkerContext<'a> { // // This uses a O(n^2) array scan instead of a O(n) map because the vast // majority of cases have one or two elements - for prev_tracker in &self.cycle_detector[cycle_detector_top..] { + // + // The entries below `cycle_detector_top` are the chains of the callers + // this is tracing an "export * from" alternative for (`Found` arm); an + // alternative leading back into one of them would otherwise recurse + // forever. + for prev_tracker in &self.cycle_detector { if import_tracker_eq(&tracker, prev_tracker) { result = MatchImport { kind: MatchImportKind::Cycle, @@ -3723,8 +3731,21 @@ impl<'a> LinkerContext<'a> { [ambiguous_tracker.data.source_index.get() as usize] .contains(&ambiguous_tracker.data.import_ref) { + // One frame per nested alternative; the chain can be as + // long as the input likes. + if !bun_core::StackCheck::init().is_safe_to_recurse() { + result = MatchImport { + kind: MatchImportKind::StackOverflow, + ..Default::default() + }; + break 'loop_; + } let ambig = self.match_import_with_export(ambiguous_tracker.data, re_exports); + if ambig.kind == MatchImportKind::StackOverflow { + result = ambig; + break 'loop_; + } ambiguous_results.push(ambig); } else { ambiguous_results.push(MatchImport { @@ -3783,6 +3804,11 @@ impl<'a> LinkerContext<'a> { // loop is done. All remaining exit paths are below this point. self.cycle_detector.truncate(cycle_detector_top); + // Not every alternative was traced, so there is nothing to compare. + if result.kind == MatchImportKind::StackOverflow { + return result; + } + // If there is a potential ambiguity, all results must be the same for ambig in &ambiguous_results { if *ambig != result { @@ -3912,7 +3938,7 @@ impl<'a> LinkerContext<'a> { ..Default::default() })); } - MatchImportKind::Cycle => { + MatchImportKind::Cycle | MatchImportKind::StackOverflow => { let source = self.get_source(source_index); let r = lex::range_of_identifier(source, named_import.alias_loc); // SAFETY: arena `*const [u8]` valid for the link pass. @@ -3920,13 +3946,19 @@ impl<'a> LinkerContext<'a> { .alias .expect("infallible: alias present") .slice(); + let what = if result.kind == MatchImportKind::Cycle { + "Detected cycle" + } else { + "Maximum call stack size exceeded" + }; // Split-borrow with `named_import` — `log_disjoint` returns // the disjoint `Transpiler.log` backref. self.log_disjoint().add_range_error_fmt( Some(source), r, format_args!( - "Detected cycle while resolving import \"{}\"", + "{} while resolving import \"{}\"", + what, bstr::BStr::new(alias), ), ); diff --git a/test/bundler/bundler_edgecase.test.ts b/test/bundler/bundler_edgecase.test.ts index 7fe388d3a623..6666089d9fff 100644 --- a/test/bundler/bundler_edgecase.test.ts +++ b/test/bundler/bundler_edgecase.test.ts @@ -2831,6 +2831,115 @@ describe("bundler", () => { }, 120_000, ); + // When a name reaches a file through two different `export *` statements, + // the linker traces each alternative to check that they end at the same + // symbol; an alternative that is itself a re-export is traced by a recursive + // call. These cycles lead straight back into an import that is still being + // traced, which used to recurse until the stack overflowed (the recursive + // call only checked for cycles within its own chain). esbuild reports the + // same error for both graphs. backend: "cli" so that the old crash killed a + // child process rather than the test runner. + itBundled("edgecase/ExportStarAmbiguityCycleIntoBarrel", { + files: { + "/entry.js": `import { x } from "./barrel.js"; console.log(x);`, + "/barrel.js": `export * from "./a.js"; export * from "./b.js";`, + "/a.js": `export const x = 1;`, + "/b.js": `export { x } from "./barrel.js";`, + }, + backend: "cli", + bundleErrors: { + "/b.js": ['Ambiguous import "x" has multiple matching exports'], + }, + }); + // Here the import being traced is only re-entered two recursive calls down + // (b2 -> barrel1 -> b1 -> barrel2 -> b2), so every chain on the stack has to + // be checked, not just the caller's. + itBundled("edgecase/ExportStarAmbiguityCycleAcrossBarrels", { + files: { + "/entry.js": `import { x } from "./barrel1.js"; console.log(x);`, + "/a.js": `export const x = 1;`, + "/barrel1.js": `export * from "./a.js"; export * from "./b1.js";`, + "/b1.js": `export { x } from "./barrel2.js";`, + "/barrel2.js": `export * from "./a.js"; export * from "./b2.js";`, + "/b2.js": `export { x } from "./barrel1.js";`, + }, + backend: "cli", + bundleErrors: { + "/b2.js": ['Ambiguous import "x" has multiple matching exports'], + }, + }); + // Alternatives traced one after the other may pass through the same + // re-export (both b and c go through shared); that is not a cycle. All + // three alternatives end at a.x, so the import is not ambiguous either. + itBundled("edgecase/ExportStarAmbiguityAlternativesShareReExport", { + files: { + "/entry.js": `import { x } from "./barrel.js"; console.log(x);`, + "/barrel.js": `export * from "./a.js"; export * from "./b.js"; export * from "./c.js";`, + "/a.js": `export const x = 1;`, + "/b.js": `export { x } from "./shared.js";`, + "/c.js": `export { x } from "./shared.js";`, + "/shared.js": `export { x } from "./a.js";`, + }, + run: { stdout: "1" }, + }); + // An acyclic version of the above: barrel N's second `export *` leads to a + // re-export from barrel N+1, so tracing the alternatives recurses once per + // barrel. Bun.build() links on a thread with a 2 MiB stack, which a + // debug+ASAN build overflowed a little past 400 barrels (it now stops with a + // build error shortly before that); a release build bundles this chain. + // Either outcome is fine, what must not happen is the process dying, which + // is why the build runs in a child. Mostly spent parsing the 1200 files + // under ASAN, hence the timeout. + test.concurrent( + "edgecase/ExportStarAmbiguityDeepReExportChain", + async () => { + const depth = 600; + using dir = tempDir("export-star-ambiguity-deep-chain", { + "a.js": `export const x = 1;\n`, + ...Object.fromEntries( + Array.from({ length: depth }, (_, i) => [ + `barrel${i}.js`, + `export * from "./a.js";\nexport * from "./b${i}.js";\n`, + ]), + ), + ...Object.fromEntries( + Array.from({ length: depth }, (_, i) => [`b${i}.js`, `export { x } from "./barrel${i + 1}.js";\n`]), + ), + [`barrel${depth}.js`]: `export * from "./a.js";\n`, + "entry.js": `import { x } from "./barrel0.js";\nconsole.log(x);\n`, + "build-fixture.ts": /* ts */ ` + const result = await Bun.build({ entrypoints: [import.meta.dir + "/entry.js"], throw: false }); + console.log( + JSON.stringify({ + success: result.success, + logs: result.logs.map(log => log.message), + output: result.success ? await result.outputs[0].text() : null, + }), + ); + `, + }); + await using proc = Bun.spawn({ + cmd: [bunExe(), "build-fixture.ts"], + cwd: String(dir), + env: bunEnv, + stdio: ["ignore", "pipe", "pipe"], + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stderr, exitCode, signalCode: proc.signalCode }).toEqual({ stderr: "", exitCode: 0, signalCode: null }); + const result = JSON.parse(stdout); + if (result.success) { + expect(result.logs).toEqual([]); + expect(result.output).toContain("var x = 1;"); + } else { + expect(result).toEqual({ + success: false, + logs: ['Maximum call stack size exceeded while resolving import "x"'], + output: null, + }); + } + }, + 120_000, + ); itBundled("edgecase/NonAsciiPathDerivedWrapperName", { files: { "/entry.ts": /* js */ `