From ed440fb40872a617e31965dc1425fc9c83c46dc3 Mon Sep 17 00:00:00 2001 From: Claude Bot Date: Tue, 27 Jan 2026 07:31:22 +0000 Subject: [PATCH] fix(macros): detect circular references and report clear error When a macro returns an object with a circular reference (either direct like `obj.self = obj` or indirect like `a.ref = b; b.ref = a`), Bun would crash with a segmentation fault due to infinite recursion in the AST visitor. This fix adds proper cycle detection during JavaScript-to-AST conversion: - Use a sentinel value (Expr.empty) to mark objects being processed - When encountering a value already being processed, emit a clear error instead of allowing circular AST nodes to be created - Move expression creation to after all children are processed Fixes #22552 Co-Authored-By: Claude Opus 4.5 --- src/ast/Macro.zig | 59 ++++++++---- test/regression/issue/22552.test.ts | 138 ++++++++++++++++++++++++++++ 2 files changed, 181 insertions(+), 16 deletions(-) create mode 100644 test/regression/issue/22552.test.ts diff --git a/src/ast/Macro.zig b/src/ast/Macro.zig index a3075050cf..a7cff2868d 100644 --- a/src/ast/Macro.zig +++ b/src/ast/Macro.zig @@ -363,20 +363,28 @@ pub const Runner = struct { const _entry = this.visited.getOrPut(this.allocator, value) catch unreachable; if (_entry.found_existing) { + // Check if we're in the middle of processing this value (circular reference) + if (_entry.value_ptr.isMissing()) { + this.log.addErrorFmt( + this.source, + this.caller.loc, + this.allocator, + "macro returned an object with a circular reference, which cannot be converted to JavaScript", + .{}, + ) catch unreachable; + return error.MacroFailed; + } return _entry.value_ptr.*; } + // Use sentinel to detect circular references during processing + _entry.value_ptr.* = Expr.empty; + var iter = try jsc.JSArrayIterator.init(value, this.global); // Process all array items var array = this.allocator.alloc(Expr, iter.len) catch unreachable; errdefer this.allocator.free(array); - const expr = Expr.init( - E.Array, - E.Array{ .items = ExprNodeList.empty, .was_originally_macro = true }, - this.caller.loc, - ); - _entry.value_ptr.* = expr; var i: usize = 0; while (try iter.next()) |item| { array[i] = try this.run(item); @@ -385,8 +393,15 @@ pub const Runner = struct { i += 1; } - expr.data.e_array.items = ExprNodeList.fromOwnedSlice(array); - expr.data.e_array.items.len = @truncate(i); + // Create and store the expression only after all items are processed + var items = ExprNodeList.fromOwnedSlice(array); + items.len = @truncate(i); + const expr = Expr.init( + E.Array, + E.Array{ .items = items, .was_originally_macro = true }, + this.caller.loc, + ); + _entry.value_ptr.* = expr; return expr; }, // TODO: optimize this @@ -394,16 +409,22 @@ pub const Runner = struct { this.is_top_level = false; const _entry = this.visited.getOrPut(this.allocator, value) catch unreachable; if (_entry.found_existing) { + // Check if we're in the middle of processing this value (circular reference) + if (_entry.value_ptr.isMissing()) { + this.log.addErrorFmt( + this.source, + this.caller.loc, + this.allocator, + "macro returned an object with a circular reference, which cannot be converted to JavaScript", + .{}, + ) catch unreachable; + return error.MacroFailed; + } return _entry.value_ptr.*; } - // Reserve a placeholder to break cycles. - const expr = Expr.init( - E.Object, - E.Object{ .properties = G.Property.List{}, .was_originally_macro = true }, - this.caller.loc, - ); - _entry.value_ptr.* = expr; + // Use sentinel to detect circular references during processing + _entry.value_ptr.* = Expr.empty; // SAFETY: tag ensures `value` is an object. const obj = value.getObject() orelse unreachable; @@ -432,7 +453,13 @@ pub const Runner = struct { }) catch |err| bun.handleOom(err); } - expr.data.e_object.properties = properties; + // Create and store the expression only after all properties are processed + const expr = Expr.init( + E.Object, + E.Object{ .properties = properties, .was_originally_macro = true }, + this.caller.loc, + ); + _entry.value_ptr.* = expr; return expr; }, diff --git a/test/regression/issue/22552.test.ts b/test/regression/issue/22552.test.ts new file mode 100644 index 0000000000..82bc63c362 --- /dev/null +++ b/test/regression/issue/22552.test.ts @@ -0,0 +1,138 @@ +// https://github.com/oven-sh/bun/issues/22552 +// Macros that return objects with circular references should produce a clear error message +// instead of crashing with a segmentation fault. + +import { expect, test } from "bun:test"; +import { bunEnv, bunExe, tempDir } from "harness"; + +test("macro with direct circular reference should error gracefully - issue #22552", async () => { + using dir = tempDir("22552-circular-direct", { + "circular.ts": ` +export function getCircularData(): any { + const obj: any = { name: "test" }; + obj.self = obj; // Direct circular reference + return obj; +} +`, + "main.ts": ` +import { getCircularData } from "./circular.ts" with { type: "macro" }; +const data = getCircularData(); +console.log(data); +`, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "build", "./main.ts"], + env: bunEnv, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + + const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + + // Should fail with a clear error message, not crash + expect(stderr).toContain("circular reference"); + expect(exitCode).toBe(1); +}); + +test("macro with indirect circular reference should error gracefully - issue #22552", async () => { + using dir = tempDir("22552-circular-indirect", { + "circular.ts": ` +export function getIndirectCircular(): any { + const a: any = { name: "a" }; + const b: any = { name: "b" }; + a.ref = b; + b.ref = a; // Indirect circular reference + return a; +} +`, + "main.ts": ` +import { getIndirectCircular } from "./circular.ts" with { type: "macro" }; +const data = getIndirectCircular(); +console.log(data); +`, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "build", "./main.ts"], + env: bunEnv, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + + const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + + // Should fail with a clear error message, not crash + expect(stderr).toContain("circular reference"); + expect(exitCode).toBe(1); +}); + +test("macro with circular array reference should error gracefully - issue #22552", async () => { + using dir = tempDir("22552-circular-array", { + "circular.ts": ` +export function getCircularArray(): any { + const arr: any[] = [1, 2, 3]; + arr.push(arr); // Array contains itself + return arr; +} +`, + "main.ts": ` +import { getCircularArray } from "./circular.ts" with { type: "macro" }; +const data = getCircularArray(); +console.log(data); +`, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "build", "./main.ts"], + env: bunEnv, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + + const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + + // Should fail with a clear error message, not crash + expect(stderr).toContain("circular reference"); + expect(exitCode).toBe(1); +}); + +test("macro with non-circular nested objects should work fine - issue #22552", async () => { + using dir = tempDir("22552-non-circular", { + "data.ts": ` +export function getNestedData(): any { + return { + level1: { + level2: { + level3: { + value: "deep" + } + } + } + }; +} +`, + "main.ts": ` +import { getNestedData } from "./data.ts" with { type: "macro" }; +const data = getNestedData(); +console.log(data.level1.level2.level3.value); +`, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "build", "./main.ts", "--outdir", "./out"], + env: bunEnv, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + + const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + + // Non-circular structures should work fine + expect(stderr).not.toContain("error"); + expect(exitCode).toBe(0); +});