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 <noreply@anthropic.com>
This commit is contained in:
Claude Bot
2026-01-27 07:31:22 +00:00
parent ba426210c2
commit ed440fb408
2 changed files with 181 additions and 16 deletions

View File

@@ -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;
},

View File

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