Skip to content

Fix segfault inspecting JSX with circular props - #30585

Closed
robobun wants to merge 1 commit into
mainfrom
farm/898354b3/fix-jsx-circular-props-segfault
Closed

Fix segfault inspecting JSX with circular props#30585
robobun wants to merge 1 commit into
mainfrom
farm/898354b3/fix-jsx-circular-props-segfault

Conversation

@robobun

@robobun robobun commented May 12, 2026

Copy link
Copy Markdown
Collaborator

What

Bun.inspect (and console.log) segfaulted on JSX/React elements where props (or any property other than children) pointed back at the element itself, creating a circular reference.

Repro

const el = Bun.markdown.react("hello");
el.props = el;
Bun.inspect(el); // SIGSEGV (stack overflow)

Root cause

The generic printAs path guards against circular references by tracking visited objects in a map and by checking the stack budget — but only when Tag.canHaveCircularReferences() returns true for the current tag. .JSX was missing from that list, so inspecting a JSX element just recursed into this.format for each prop with no bookkeeping. When a prop referenced the element itself, this ran until the native stack was exhausted.

Fix

Add .JSX to canHaveCircularReferences(). The existing visited-map + stack-check code in printAs then handles JSX the same as Object, Array, Map, etc.: the second visit prints [Circular] instead of recursing.

Added regression tests covering circular props and circular children.

@robobun

robobun commented May 12, 2026

Copy link
Copy Markdown
Collaborator Author
Updated 3:07 PM PT - May 12th, 2026

@robobun, your commit d83710e has 1 failures in Build #53855 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 30585

That installs a local version of the PR into your bun-30585 executable, so you can run:

bun-30585 --bun

@coderabbitai

coderabbitai Bot commented May 12, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack
No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: b76e2853-53a6-4b12-a668-34e076a72ca9

📥 Commits

Reviewing files that changed from the base of the PR and between 39540fd and d83710e.

📒 Files selected for processing (2)
  • src/jsc/ConsoleObject.zig
  • test/js/bun/util/inspect.test.js

Walkthrough

This PR extends Bun's Bun.inspect to safely handle circular references in JSX values. The implementation adds .JSX to the circular-reference-tracking behavior, and two test cases verify the formatter does not crash when JSX elements contain circular references through props or children.

Changes

JSX Circular Reference Detection

Layer / File(s) Summary
JSX circular reference support
src/jsc/ConsoleObject.zig, test/js/bun/util/inspect.test.js
canHaveCircularReferences now includes .JSX in its tag switch to recognize JSX values as capable of circular references. Two test cases verify Bun.inspect handles el.props = el and el.props.children = el without crashing and includes [Circular] in the output.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely captures the main fix: preventing segfaults when inspecting JSX elements with circular references in props.
Description check ✅ Passed The description comprehensively covers both required template sections with clear problem statement, reproduction case, root cause analysis, and verification through added tests.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

Copy link
Copy Markdown
Contributor

This PR may be a duplicate of:

  1. Fix stack overflow in Bun.inspect with circular JSX elements #30584 - Also fixes stack overflow in Bun.inspect with circular JSX elements
  2. Fix stack overflow in Bun.inspect for circular JSX elements #30126 - Also fixes stack overflow in Bun.inspect for circular JSX elements
  3. console: guard JSX/Proxy recursion in Bun.inspect (stack overflow SIGSEGV) #29709 - Also fixes stack overflow in Bun.inspect() on circular JSX elements

🤖 Generated with Claude Code

@robobun

robobun commented May 12, 2026

Copy link
Copy Markdown
Collaborator Author

Duplicate of #30584 which is more complete: also fixes the same issue in src/test_runner/pretty_format.zig and handles non-object props safely.

@robobun robobun closed this May 12, 2026
@robobun
robobun deleted the farm/898354b3/fix-jsx-circular-props-segfault branch May 12, 2026 22:07

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — minimal one-tag addition to the existing circular-ref guard, with regression tests.

Extended reasoning...

Overview

This PR fixes a segfault when Bun.inspect / console.log is called on a JSX element whose props (or children) reference the element itself. The fix is a single token addition in src/jsc/ConsoleObject.zig: .JSX is appended to the canHaveCircularReferences() switch arm alongside .Function, .Array, .Object, .Map, .Set, .Error, .Class, .Event. Two regression tests in test/js/bun/util/inspect.test.js cover circular props and circular children.

Security risks

None. This is a defensive change in the inspect/formatting path — it adds a stack-safety check and visited-map lookup for a tag that was previously skipping them. No auth, crypto, permissions, parsing of untrusted input, or external I/O is touched.

Level of scrutiny

Low. The mechanism (canHaveCircularReferences gating the visited-map + stack_check.isSafeToRecurse() block at ConsoleObject.zig:2092–2121) is already exercised by every Object/Array/Map inspect call; this just opts .JSX into the same well-trodden path. The .JSX printAs branch (line 3077+) clearly recurses via this.format for each prop and child, so the omission was an obvious oversight rather than intentional.

Other factors

  • The change follows the established pattern exactly — no new logic, just one more enum value in the allowlist.
  • Regression tests assert [Circular] appears rather than snapshotting full output, so they're robust to formatting tweaks.
  • Bun.markdown.react is used elsewhere in the test suite (test/js/bun/md/md-react.test.ts), so it's a known-good way to construct a JSX-tagged value without a JSX transform.
  • No prior reviewer comments, no CODEOWNERS concerns, and the bug-hunting system found nothing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant