Repository navigation
Conversation
The JSX printer read one child for every index below `length` and wrote one line for each, holes included. A sparse array as `props.children` did not finish, and an arguments object with `length` above 2^32 - 1 ended in `panic: int cast: TryFromIntError(PosOverflow)`. The children arm now reads own slots. At the first hole it counts the indexes that the list holds. A list with at most 65,536 holes prints as before, one line for each index. A list with more holes is walked by the indexes it holds, and each run of holes is one `N x empty items` line. PresentIndexes.cpp is the one walk of the indexed storage of an array or an arguments object. Bun__JSArray__nextPresentIndex moves there from bindings.cpp and shares its loops.
|
Status: ready for review (#47526). CI (build 136012, finished): 183 of 188 jobs passed. The 5 failed jobs all fail on one test, Reproduce const element = children => ({ $$typeof: Symbol.for("react.element"), type: "div", key: null, ref: null, props: { children } });
const c = ["a"];
c[4294967294] = "x";
console.log(element(c)); // main: does not finish, 966 MB in 5 s. This branch: 48 bytes.Run the tests with |
|
Updated 2:22 AM PT - Oct 31st, 2026
❌ @robobun, your commit 5caf3e5 has 1 failures in
🧪 To try this PR locally: bunx bun-pr 47526That installs a local version of the PR into your bun-47526 --bun |
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Present-index traversal bindings src/jsc/bindings/PresentIndexes.cpp, src/jsc/bindings/bindings.cpp, src/jsc/JSValue.rs |
Native traversal counts and collects present indexes for arrays and arguments objects. Rust helpers expose the range lookup. The prior Bun__JSArray__nextPresentIndex implementation is removed from bindings.cpp. |
JSX child-range formatting src/jsc/ConsoleObject.rs, test/js/bun/util/inspect.test.js |
The formatter reads initial children directly, then processes the remaining range with indexed reads or present-index traversal. Consecutive holes can render as summarized lines. Tests cover array and arguments-object storage, inherited values, mutations, nested elements, and large lengths. |
Possibly related PRs
- oven-sh/bun#42247: Adds present-index traversal for inspection of arguments objects with very large
lengthvalues.
Suggested reviewers: alii, jarred-sumner
Merge Risk: 
5caf3
-
The test now checks inherited-getter calls after printing the element. No actionable merge-blocking risk was identified; merge after normal checks pass.
Pre-merge checks: 
-
Check name Status Explanation Title check The title names the main change: print_jsx (JSX children printing in console) now walks children by the indexes they hold when a list has many holes. The title matches the raw summary, which describes… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Description check The description is complete and relevant. It explains the problem, the fix, trade-offs, known limitations, and verification results. It does not use the exact template headings, but the required conte… Full details: Title check
-
Explanation
The title names the main change: print_jsx (JSX children printing in console) now walks children by the indexes they hold when a list has many holes. The title matches the raw summary, which describes the indexed-read and present-index walk for JSX children with holes, and matches the objectives, which describe reading own slots and walking held indexes past 65,536 holes. The title is specific and concise, is one sentence, and does not use vague terms.
Full details: Description check
-
Explanation
The description is complete and relevant. It explains the problem, the fix, trade-offs, known limitations, and verification results. It does not use the exact template headings, but the required content appears under equivalent sections: “Fix” covers what the PR does, and “Checks” covers verification.
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
Beyond the inline findings, I also checked two things in the new hole path and found them sound: the index list from present_indexes_in going stale when a child's formatting runs user code (each index is re-read with get_direct_index and an emptied slot is folded into the next hole run, so no stale value is printed), and the std::sort being skipped on overflow passes in Bun__JSObject__presentIndexesInRange (the Rust retry loop always ends with a pass where count <= capacity, so the returned list is sorted).
Extended reasoning...
The change rewrites the JSX children arm of the console formatter in src/jsc/ConsoleObject.rs, adds a C ABI index-walker in the new src/jsc/bindings/PresentIndexes.cpp (moving the old nextPresentIndex helper out of bindings.cpp), and adds Rust wrappers plus 37 tests. It touches no auth, crypto or injection surface; the risk is memory/exception safety in unsafe FFI and the sparse-array walk. Inline findings are posted, so a human look is already signaled; this note only records what else was examined.
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
Additional findings (outside the current diff — GitHub can't attach inline comments there):
-
🟡
src/jsc/ConsoleObject.rs— nit, pre-existing: a user who inspects an element whose arguments-object children has nolength(orlength = Infinity) now gets a line reading2251799813685244 x empty itemsinstead of a hang.children.get_lengthat src/jsc/ConsoleObject.rs:5237 turns the C++ sentinel infinity for "no length property" into 2^51-1, and print_jsx_children_from_hole prints that as a hole run. Fix: treat a missing or non-finitelengthas zero (or as the arguments the object holds) before the hole walk, and pin the output in the test at test/js/bun/util/inspect.test.js:1741-1743 rather than asserting onlytypeof.Why this was flagged
Trigger:
const a = (function(){return arguments})(1,2,3); delete a.length; console.log(element(a)), ora.length = Infinity, reached through print_jsx's Tag::Array children arm. JSC__JSValue__getLengthIfPropertyExistsInternal at src/jsc/bindings/bindings.cpp:3098-3112 returnsstd::numeric_limits<double>::infinity()when the object has nolength; JSValue::get_length at src/jsc/JSValue.rs:1366-1371 only maps f64::MAX to 0 and clamps infinity to 2^51-1. print_jsx then calls print_jsx_children_from_hole at src/jsc/ConsoleObject.rs:5271 with length 2251799813685247, which prints1,2,3and2251799813685244 x empty items. The base hangs on the same input, so the PR turns a hang into a nonsensical count. The new test at test/js/bun/util/inspect.test.js:1741-1743 runs this exact case and asserts onlytypeof ... === "string", so the output is unpinned.Verification: Trigger: a JSX element whose props.children is an arguments object with
lengthdeleted or set to Infinity. JSValue::get_length (src/jsc/JSValue.rs:1366-1371) clamps the infinity sentinel to 2^51-1, so print_jsx_children_from_hole prints "2251799813685244 x empty items". On the base branch the same input hangs rather than prints. test/js/bun/util/inspect.test.js:1741-1743 leaves the output unverified.
|
About the arguments object with no |
Problem
console.log(el)does not finish whenel.props.childrenis a sparse array (c = ["a"]; c[4294967294] = "x"): 966 MB in 5 s.Bun.inspect(el)aborts.lengthabove 2^32 - 1 ends inpanic: int cast: TryFromIntError(PosOverflow).print_jsx(src/jsc/ConsoleObject.rs:5246) prints one line for each index belowlength. Found by fuzzing, no user report.Fix
N x empty itemsline.Bun__JSArray__nextPresentIndexmoves frombindings.cppto the newPresentIndexes.cpp, the one walk of JSC index storage.test/js/bun/util/inspect.test.js(37 new tests, main fails 30), console suites. Self-reviewed: 30 concerns raised, 26 addressed, 4 rejected (Notes).Background
lengththat the list does not hold:Array(5)has five.empty itemfor every hole. It changestest.eachtitles and stored matcher messages, so it is a separate decision.Downsides
["a", , "bb"].bun:testsnapshots and diffs of the element, and a bare arguments object (inspect: don't probe every index of an arguments object with a huge length #42247).Notes
Reproduce (main 34cbc70 and 1.4.3-canary, linux x64)
Bun.inspect(element(c))on main fills memory. Under a 6 GB limit it ends inmemory allocation of 2147483648 bytes failed, exit 134.What prints
undefined, or what the prototype chain holdsundefinedline for each holeempty itemorN x empty itemsfor each run of holes, as an array prints themNumbers. Release builds of main 34cbc70 and of this PR. Instructions and cycles are user mode, read through
perf_event_open,BUN_JSC_useJIT=0.perf,valgrindandbloatyare not installed in the build container.['a', , 'bb']11,375 -> 11,561 instructions, cycles 1.021x (control 0.999x). Frozen with one hole in 100 / 1,000 / 10,000 / 160,000 children: instructions -1.4% / -2.2% / -1.5% / -1.5%, cycles 0.984x / 0.982x / 1.021x / 0.905x (control 0.987x / 1.006x / 1.024x / 1.070x).Bun__JSArray__nextPresentIndex405 -> 453. Externs +1, host functions +0, bindings.cpp -66 lines.print_array(Rust unchanged, walker moved), instructions per print:[1, 2, 3]6,194.7 -> 6,194.5.[1, , 3]6,168.7 -> 6,169.4. 100 runs of holes 142,466 -> 142,561. Frozen with 100 runs 393,764 -> 394,368. That is +1 to +9 for each run of holes: the storage switch compiles to bit tests wherebindings.cpphad a jump table.jsx_child_atexists for the frame. With the read called in place,print_jsx::<false>has a frame of 0x1c8 and the dense nesting depth is 13,194. No test pins this.[1, 2, 3]runs the same 6,194 instructions and 1,145 branches on both builds and none of this code. It takes 1,382 cycles on the main build and 1,631 on this one, with 35 against 67 L1 instruction cache misses per print. On the previous base the two builds were at 1,534 and 1,562.Checks
length, getter and non-enumerable steps. As built, 20,000 of 20,000 lists print the same bytes on both builds: as JSX children without and with colors, and as a bare value. 11,544 of them have holes.lengthand are not comparable.PresentIndexes.cpp, each built and run: no sort (4 tests fail), no skip of an argument that is in its slot and in the butterfly (2), no end bound on the contiguous vector (1), on the ArrayStorage vector (1), on the sparse map (4), no start bound on the sparse map (4),nextPresentIndexwithout the sparse map (the test of inspect: don't probe every hole when printing a sparse array #33158 fails).BUN_JSC_validateExceptionChecks=1reports nothing on the new paths.inspect.test.js,snapshot.test.ts(withFORCE_COLOR=1),test/js/bun/console/,test/js/web/console/,md-react,14477,inspect-error,reportError,expect.test.js: 868 pass, 0 fail.test/internal/source-lints/: 194 pass.Self-review. A first version printed
empty itemfor every hole and kept a second copy of the storage walk. That text reaches stored snapshots by two routes:test.eachtitles with%p, and matcher messages intoThrowErrorMatchingSnapshot. With snapshots written by main andCI=true, the first version fails both and this version passes both. This version keeps the text of main for every list within the budget and has one walk. Rejected:print_arrayhere too. inspect: don't probe every index of an arguments object with a huge length #42247 owns it. The lister here already knows the argument slots, so that PR can use it.console.logformatter #46729. Either order gives the same arm.empty itemfor every hole now. It is a text change and a separate decision. For that decision: main prints the sameArray(3)as[ 3 x empty items ]when the element has a second child, and as threeundefinedlines only when it is the single child.Other PRs
console.logformatter #46729 prints snapshots through this arm. If it lands first, this PR adds thepretty_formatbranch that keeps the loop of main for snapshots. If this PR lands first, that rebase takes the arm as it is here plus that branch.Bun__JSArray__nextPresentIndexinbindings.cpp. Whichever lands second moves its change toPresentIndexes.cpp.Bun__JSArray__presentIndexes.JSValue::present_indexes_in(0..len)gives the same list.length == 0exit that prints<div(inspect, bun:test: self-close JSX elements whose children are "" or [] #38935), thelengthread itself (jsc: get_length returns 0 for objects without a length property; bun:test reads callback arity before wrapping it #38910), the React field reads (console: read React element fields without running getters or Proxy traps #40319).length, or withlength = Infinity, gets 2^51 - 1 fromJSValue::get_length. As children it printed without end on main. Here it prints its arguments and one2251799813685244 x empty itemsline. jsc: get_length returns 0 for objects without a length property; bun:test reads callback arity before wrapping it #38910 changes that read, so the test asserts only that the print returns.print_array), One value formatter for console, Bun.inspect, the error printer and bun:test #44268 and the branchrobobun/0ac6db53/console-list-length-reader(a list of the held indexes, with a cutoff of 8 holes for each run).Still hit, not in this PR
bun:testsnapshots and matcher diffs print throughsrc/runtime/test_runner/pretty_format.rs, which has the same loop. Committed snapshots hold one line for each hole.lengthprinted bare or inside an array (inspect: don't probe every index of an arguments object with a huge length #42247),console.tablerows (console.table: walk an array by its own properties instead of trusting length #41119),AggregateError.errors(error printer: print an AggregateError itself, then its members indented under it #47114, console: do not hang on an iterator that never ends #42264).lengthoutside the printers:structuredClone, deep equality (deepEquals: compare only the held indices of arrays that claim far more than they hold #42238),partialDeepStrictEqual(node:assert: match arrays with holes by their own keys inpartialDeepStrictEqual#45244), macros (macros: fail the build on process.exit, Error returns, sparse arrays, and promises that never settle #40769),YAML.stringify.Bun.inspectstill aborts for any value whose text does not fit in memory.