Error.appendStackTrace: fix abort with unset stackTraceLimit, assertion on materialized errors, and self-append use-after-free - #37370
Conversation
… a number ErrorInstance::captureStackTrace() calls .value() on the global's stackTraceLimit optional, which is empty once Error.stackTraceLimit has been set to a non-number or deleted. With -fno-exceptions that is a plain abort(). Give the destination an empty frame list in that state instead of capturing. Also return early when the destination's error info has already been materialized: its frames were discarded and are never read again, and installing new ones trips ASSERT(!m_errorInfoMaterialized) in computeErrorInfo during GC.
|
Warning Review limit reached
Next review available in: 1 minute Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
Comment |
|
Updated 8:42 AM PT - Aug 13th, 2026
❌ @robobun, your commit c7a06b3 has 1 failures in
🧪 To try this PR locally: bunx bun-pr 37370That installs a local version of the PR into your bun-37370 --bun |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — two narrow guards in errorConstructorFuncAppendStackTrace that mirror the same handling already in errorConstructorFuncCaptureStackTrace.
What was reviewed:
setStackFrames(vm, {})produces a non-null empty vector (makeUnique<Vector<StackFrame>>), so the followingdestination->stackTrace()->appendVector(...)is safe and the source's frames still land — the first test'sappended: trueassertion pins this.- The materialized-destination early return is not a release behavior change: once
.stackis a data property the installed frames were never read anyway; this only removes the debug assertion and wasted work. - Tests follow harness conventions (subprocess with
bunEnv, concurrent stdout/stderr/exited drain, combined-object assertion,test.concurrent).
Extended reasoning...
Overview
Two guards added to errorConstructorFuncAppendStackTrace in src/jsc/bindings/FormatStackTraceForJS.cpp:
- Early-return when
destination->hasMaterializedErrorInfo()— once.stackis materialized the frames are discarded andm_errorInfoMaterializedis set; re-installing frames only tripsASSERT(!m_errorInfoMaterialized)incomputeErrorInfoduringfinalizeUnconditionally.errorConstructorFuncCaptureStackTracealready special-cases this exact state a few lines below. - Guard
captureStackTrace()onglobalObject->stackTraceLimit()being non-empty —ErrorInstance::captureStackTraceunwraps the optional with.value(), which isabort()under-fno-exceptionswhenError.stackTraceLimitis non-numeric or deleted. The fallbacksetStackFrames(vm, {})unconditionally doesmakeUnique<Vector<StackFrame>>, sostackTrace()is non-null afterward and the subsequentappendVectoron the source's frames still runs.
Two subprocess regression tests are added next to the existing non-numeric stackTraceLimit test in test/js/node/v8/capture-stack-trace.test.js.
Security risks
None. No untrusted input parsing, no allocation-size arithmetic, no new ownership. Both guards are pure short-circuits on existing state.
Level of scrutiny
Medium — this is JSC bindings C++, but the change is two conditionals that each mirror an existing pattern in the sibling function in the same file (hasMaterializedErrorInfo() branch, stackTraceLimit().value_or(...)). No new GC roots, no pointers held across JS re-entry, no exception-scope changes.
Other factors
- The PR description is precise about the mechanism (
bad_optional_access→abort()under-fno-exceptions;finalizeUnconditionallytripping the materialized assertion) and honest that the second test only fails on debug builds. - The no-op-when-materialized choice was checked for release-build behavior regression: after materialization
.stackis already a data property, so the frames the old code installed were dead — no observable change. - Tests use
test.concurrent, drain pipes concurrently, assert the combined{stdout, stderr, exitCode}object, and the first test asserts the positive contract (appended: true) rather than just "didn't crash". - No CODEOWNERS entry covers these paths.
…stack-trace-abort
…and unset-limit cases Appending an error's trace to itself made Vector::appendVector copy out of the buffer it had just reallocated and then clear() wiped the trace. Adds tests for the self-append, for destinations materialized through .sourceURL, and for Error.stackTraceLimit = undefined.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| // Appending a trace to itself would make appendVector copy out of the buffer | ||
| // it just reallocated (the span overload does not rebase the source | ||
| // pointer), and the clear() below would then wipe the trace. |
There was a problem hiding this comment.
If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code
| // Once .stack is materialized the frames are discarded and never read again; | ||
| // installing new ones only trips ASSERT(!m_errorInfoMaterialized) in | ||
| // computeErrorInfo when GC finalizes the error. |
There was a problem hiding this comment.
If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code
| // ErrorInstance::captureStackTrace() unwraps stackTraceLimit(), which is | ||
| // empty once Error.stackTraceLimit has been set to a non-number or deleted. |
There was a problem hiding this comment.
If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code
robobun
changed the title
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. The three guards look correct and mirror the existing hasMaterializedErrorInfo() handling in errorConstructorFuncCaptureStackTrace a few lines below. Because it touches GC-sensitive JSC bindings and comment-cop has flagged the three new multi-line comments (they read as invariant explanations rather than workaround justifications, but that's a maintainer call), a human look would still be worthwhile.
What was reviewed:
stackTraceLimit()guard — confirmedErrorInstance::captureStackTraceunconditionally calls.value()on the optional;setStackFrames(vm, {})gives a non-null vector so the append below still runs.hasMaterializedErrorInfo()early-return — same pattern ascaptureStackTrace; frames installed post-materialization are only ever seen byfinalizeUnconditionally.source == destination— verifiedappendVector's span overload doesn't rebase after grow, andclear()would then wipe the trace; no-op is the right semantics.- Tests — each abort case runs in a spawned child, drains stdout/stderr/exited concurrently, and asserts a positive observable (frame count /
appended: true) rather than just "didn't crash".
Extended reasoning...
Overview
Adds three early-return guards to errorConstructorFuncAppendStackTrace in src/jsc/bindings/FormatStackTraceForJS.cpp (~20 lines) and five tests in test/js/node/v8/capture-stack-trace.test.js. The guards prevent: (1) abort() from unwrapping an empty stackTraceLimit() optional, (2) a debug assertion in computeErrorInfo when frames are installed on an already-materialized error, and (3) a use-after-realloc + self-clear when source and destination are the same ErrorInstance.
Security risks
None. Error.appendStackTrace is a Bun-specific API operating on already-validated ErrorInstance objects (both arguments are dynamicDowncast-checked before the new code runs). No untrusted lengths, no allocation sizing, no string parsing.
Level of scrutiny
High — this is native JSC binding code with GC interactions (setStackFrames, materialization state, finalizeUnconditionally). The change is small and each guard mirrors an existing pattern in the same file, but the failure modes it addresses (silent SIGABRT, GC-time assertion) are exactly the kind where a subtle mistake surfaces far from the change site.
Other factors
- Correctness: All three guards check state that's already validated by the type check above them; none can enter user JS or throw, so no new exception-scope concerns.
setStackFrames(vm, {})is used identically inerrorInstanceLazyStackCustomGetteranderrorConstructorFuncCaptureStackTrace. - Tests: Thorough — a happy-path test plus four subprocess tests covering each guard, including the
.sourceURLmaterialization path and an ASAN-specificMalloc=1config for the self-append case. All usetest.concurrent, drain pipes withPromise.all, and assert combined{stdout, stderr, exitCode}objects. - Outstanding feedback: comment-cop (github-actions bot) flagged all three new multi-line comments with the "paragraph-long comment justifying a workaround" rule. The comments explain why each state is unsafe to proceed with (appendVector span rebasing,
m_errorInfoMaterializedassertion, empty optional unwrap) — these are the fix, not workarounds, and the neighboringcaptureStackTracehas an equivalent 4-line comment for the same materialization case. Whether to trim them is a maintainer style call I'm not making unilaterally.
Deferring rather than approving because JSC/GC-sensitive native code plus unaddressed automated style feedback warrants a human sign-off.
There was a problem hiding this comment.
LGTM — three straightforward guards that turn demonstrated crashes into safe no-ops, each mirrored by a subprocess test that fails on main.
What was reviewed:
- The
source == destinationandhasMaterializedErrorInfo()early returns — both convert a crash into a no-op; the materialized check mirrors the existing one inerrorConstructorFuncCaptureStackTracea few lines down. - The
stackTraceLimit()fallback:setStackFrames(vm, {})leavesstackTrace()non-null so the subsequentappendVectorstill lands the source frames — the non-numeric-limit test'sappended: trueassertion pins that. - The comment-cop bot flags look stale: the current guard comments are single-line (addressed in
96470ad2).
Extended reasoning...
Overview
Three additive guards in errorConstructorFuncAppendStackTrace (src/jsc/bindings/FormatStackTraceForJS.cpp), each fixing a distinct user-reachable crash in Error.appendStackTrace: self-append UAF via appendVector on a reallocated buffer, a debug assertion when frames are installed on an already-materialized ErrorInstance, and an abort() from unwrapping an empty stackTraceLimit() optional. Five new tests in test/js/node/v8/capture-stack-trace.test.js cover a positive baseline plus each failure mode in a spawned child.
Security risks
None. Error.appendStackTrace is a Bun-private helper on the Error constructor; the guards add early returns and a fallback branch with no new allocations, pointer lifetimes, or GC-visited state. Each change strictly narrows the reachable state space relative to the prior code.
Level of scrutiny
C++ JSC bindings normally warrant elevated scrutiny, but this diff is ~15 additive lines of guard logic inside one function, following the exact pattern already used by errorConstructorFuncCaptureStackTrace for the materialized case. No existing behaviour is removed; every path that previously crashed now returns early or takes a safe fallback. The setStackFrames(vm, {}) fallback is exercised end-to-end by the non-numeric-limit test asserting the source frames still land in the destination, so stackTrace() is demonstrably non-null afterward.
Other factors
The mechgate evidence shows the new tests fail on unfixed debug+ASAN and release builds and pass with the fix, satisfying the fails-for-the-right-reason requirement. Tests follow harness conventions: test.concurrent subprocess spawns, concurrent stdout/stderr/exited drain, combined {stdout, stderr, exitCode} assertions, bunEnv spread, and an isASAN-gated Malloc=1 env for the UAF repro. The three comment-cop bot comments flagging long comments are at outdated positions (line 0) and the head commit 96470ad2 ("shorten the guard comments") already reduced each to a single line, matching the repo's one-line comment rule. No human review comments are outstanding.
…stack-trace-abort
…alized no-op The native error printer prefers an error's frames over its .stack string, so appending onto a materialized destination used to make it print the appendStackTrace call site and empty the source on every build, not only trip the assertion on debug builds. The materialized tests now check the printed frames and that the source keeps its trace, the basic test pins the frame order and that the source is consumed, and the guard's comment says what it is actually protecting.
| // Once .stack is materialized the frames are gone. Installing new ones would make the native error | ||
| // printer (which prefers frames over .stack) show this call site plus the source's frames, and trip | ||
| // computeErrorInfo's !m_errorInfoMaterialized assertion when GC finalizes the error. Leave both | ||
| // errors alone instead, as Bun__attachAsyncStackFromPromise does. |
There was a problem hiding this comment.
If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code
|
Pushed bf028d9 after a second look at the materialized case. The "frames are never read again" rationale was not right: |
There was a problem hiding this comment.
LGTM — three targeted early-return guards fixing three documented crashes in Error.appendStackTrace, each following an existing pattern in the same file.
What was reviewed:
source == destinationguard: pointer equality on the twodynamicDowncastresults correctly detects self-append; avoids theappendVectorUAF and the subsequentclear().hasMaterializedErrorInfo()guard: matchesBun__attachAsyncStackFromPromiseand thecaptureStackTracehandling ~120 lines below; no-op is the only coherent semantics here since materialized frames can't be appended to.stackTraceLimit()fallback:setStackFrames(vm, {})is already used elsewhere in this file and leavesstackTrace()non-null so the followingappendVectorstill runs — verified by the test assertingappended: true.- Tests cover happy path plus all three crash modes, subprocess-isolated with ASAN
Malloc=1for the UAF case; fails-without-fix / passes-with-fix confirmed on both debug+ASAN and release.
The comment-cop flag on the 4-line comment at L676-679 looks like a heuristic false positive — it explains a non-obvious GC-finalizer assertion and is the same length/style as the existing comment for the identical check in errorConstructorFuncCaptureStackTrace.
Extended reasoning...
Overview
Adds three early-return guards to errorConstructorFuncAppendStackTrace in src/jsc/bindings/FormatStackTraceForJS.cpp (~17 lines) and five tests in test/js/node/v8/capture-stack-trace.test.js (~190 lines). Each guard prevents a distinct user-reachable crash: self-append use-after-free in WTF::Vector::appendVector, a debug assertion in ErrorInstance::computeErrorInfo when frames are installed on an already-materialized error, and an abort() from unwrapping an empty std::optional when Error.stackTraceLimit is non-numeric.
Security risks
None. Error.appendStackTrace is a Bun-private helper on the Error constructor that manipulates in-process stack-frame vectors. The change strictly narrows the reachable code paths (three new early returns) and adds no new allocation, no new GC roots, and no new user-observable state. The self-append guard closes a heap use-after-free that was previously reachable from user JS.
Level of scrutiny
This is JSC bindings C++, which is memory-safety territory, but the change is three defensive early returns that each mirror an existing pattern in the same file: hasMaterializedErrorInfo() is already checked in errorConstructorFuncCaptureStackTrace and Bun__attachAsyncStackFromPromise; setStackFrames(vm, {}) is already used in errorInstanceLazyStackCustomGetter; and the stackTraceLimit() optional check mirrors the .value_or() fix from #29388. No new state, no changed ownership, no reordered lifecycle. I traced that setStackFrames(vm, {}) leaves stackTrace() non-null (the non-numeric-limit test's appended: true assertion would fail or the child would crash on the appendVector deref otherwise, and it passes on debug+ASAN).
Other factors
The tests are thorough: a happy-path baseline, the three crash reproductions each subprocess-isolated (with Malloc=1 + ASAN options for the UAF), and a GC-stress variant for the materialized case via .sourceURL. The PR body and the robobun evidence block show fails-without-fix (4 on ASAN, 2 on release) and passes-with-fix on both build profiles. CI on the prior head had 177/177 jobs passing with only darwin-aarch64 lanes unscheduled. The outstanding comment-cop flag on line 679 is a style-linter heuristic, not a correctness concern — the flagged comment documents a GC-finalizer hazard and matches the existing comment on the same check in captureStackTrace. The behavioral choice to no-op on a materialized destination (rather than eagerly recompute like captureStackTrace does) is the only coherent option for an append operation, since the destination's frames no longer exist to append onto.
There was a problem hiding this comment.
LGTM — three additive early-return guards in errorConstructorFuncAppendStackTrace, each following an existing pattern in the same file or AsyncStackTrace.cpp, with thorough child-process tests verified to fail without the fix on both release and ASAN.
What was reviewed:
- Self-append guard is a pointer-equality check on the
ErrorInstance*from bothdynamicDowncastcalls; prevents theappendVectorself-span UAF and the trailingclear(). - Materialized-destination no-op matches
Bun__attachAsyncStackFromPromise(AsyncStackTrace.cpp:163); leaving the source untouched is intentional and asserted by the tests. stackTraceLimit()fallback installs an empty frame vector viasetStackFrames(vm, {})so the source's frames still append — theappended: trueassertion covers this.- Comment-cop feedback addressed in c7a06b3 (comments are now single-line).
Extended reasoning...
Overview
Adds three guards to errorConstructorFuncAppendStackTrace in src/jsc/bindings/FormatStackTraceForJS.cpp (17 lines) and five tests in test/js/node/v8/capture-stack-trace.test.js. Each guard prevents a distinct crash: source == destination avoids a self-appendVector heap-use-after-free; hasMaterializedErrorInfo() avoids the !m_errorInfoMaterialized debug assertion and the release-build printer/.stack desync; and checking globalObject->stackTraceLimit() before captureStackTrace() avoids .value() on an empty optional (abort). All three are early returns or a fallback before the existing dangerous operation, so nothing on the pre-existing happy path changes.
Security risks
None. Error.appendStackTrace is a Private-visibility Bun-internal helper on the Error constructor. The change removes three user-reachable crashes (one a heap-use-after-free) and introduces no new allocation, no new JS re-entry, and no new exception paths. The guards are strictly additive and cannot introduce new UAF or leaks.
Level of scrutiny
Medium-high because this is JSC C++ bindings, but the change is small and mechanical: each guard mirrors an existing pattern already in the file (hasMaterializedErrorInfo() at line 797 in captureStackTrace, stackTraceLimit().value_or(...) at line 787) or in AsyncStackTrace.cpp:163. setStackFrames(vm, {}) is used elsewhere in the same file. No new throw scopes are needed since the added paths either return early or call non-throwing helpers that the surrounding code already calls unchecked.
Other factors
The tests follow harness conventions well: child-process isolation for the abort/ASAN cases, test.concurrent, {...bunEnv, ...} spread, concurrent pipe draining via Promise.all, combined {stdout, stderr, exitCode} assertions, and Malloc=1 + detect_leaks=0 for the UAF repro under ASAN. The PR description's evidence block shows all five tests fail on unfixed release/ASAN builds and pass with the fix. The comment-cop bot's four "paragraph-long comment" flags were addressed in c7a06b3 — the current diff has single-line comments. The one semantic choice (materialized destination is a full no-op that leaves the source intact) is well-reasoned in the description, consistent with the sibling in AsyncStackTrace.cpp, and pinned by two tests.
1 participant
Consolidates the four open fixes for
Error.appendStackTrace(errorConstructorFuncAppendStackTraceinsrc/jsc/bindings/FormatStackTraceForJS.cpp) into one PR. This branch started as thestackTraceLimitfix and now also carries the cases from #35100, #34713 and #32098, which are closed in favour of it.Problem
Three ways to crash
Error.appendStackTrace(source, destination), all in the same ten lines:Error.stackTraceLimitset to a non-number (or deleted) and a destination with no frames:ErrorInstance::captureStackTrace()doesglobalObject->stackTraceLimit().value()on an empty optional. Bun is built without exceptions, so that is a silentabort()(panic(main thread): abort() calledon release builds, nothing at all under ASAN). Same class as captureStackTrace: don't abort when Error.stackTraceLimit is non-numeric #29388, which fixed the identical.value()inError.captureStackTrace. Reported as Error.appendStackTrace: don't abort when stackTraceLimit is unset #35100 and here..stack(or.line/.column/.sourceURL) has already been read: materializing discards the frames and setsm_errorInfoMaterialized, so the!destination->stackTrace()branch captures a fresh trace at theappendStackTracecall site, appends the source's frames to it and empties the source. The destination's.stackstring is unaffected, but Bun's native error printer (ZigException.cpp,fromErrorInstance) prefers an error's frames to its.stack, soconsole.error(destination)/Bun.inspect(destination)then show the call site and the source's frames instead of the destination's own, andsource.stackbecomesundefined. On debug builds the next GC that finalizes the destination additionally tripsASSERTION FAILED: !m_errorInfoMaterializedinErrorInstance::computeErrorInfo(ErrorInstance.cpp:368). Reported as fix(error): don't capture stack trace on materialized ErrorInstance in appendStackTrace #34713 and here.source === destination:destination->stackTrace()->appendVector(*source->stackTrace())appends a vector to itself.appendVectorgoes through thestd::span<const T>overload ofappend, whoseexpandCapacity(size_t, U*)does not rebase a pointer into the buffer it just freed, so once the trace has enough frames to reallocate the copy reads freed memory (heap-use-after-freeinerrorConstructorFuncAppendStackTrace, visible under ASAN withMalloc=1). Theclear()that follows then empties the trace and.stackbecomesundefinedon every build. Reported as Fix use-after-free in Error.appendStackTrace when source is destination #32098.Fix
source === destinationreturns early, leaving the trace as it was. Appending a trace to itself and then clearing it has no useful meaning, and this is the only way to keep the existing frames..stacksays (and asserts on debug). Leaving the source untouched as well is deliberate;Bun__attachAsyncStackFromPromise(AsyncStackTrace.cpp) bails out of materialized errors for the same reason. (Error.captureStackTracehandles the materialized case differently, by recomputing.stackeagerly; that is the right thing for a fresh capture but not for an append, which has no destination frames left to put in front of the source's.)stackTraceLimit()is empty. An error constructed in that state has no frames of its own anyway (getStackTracereturns null when the limit is unset), but the source's frames are still appended, sodestination.stackshows them. This is why the empty list is used rather than Error.appendStackTrace: don't abort when stackTraceLimit is unset #35100's approach of skipping the append as well.bun bd test test/js/node/v8/capture-stack-trace.test.js(48 pass). Without theFormatStackTraceForJS.cppchange, four of the fiveappendStackTracetests fail on both profiles: on the release build (USE_SYSTEM_BUN=1) the unset-limit test aborts, the self-append test reports zero frames, and the two materialized tests report the printer showing theappendAllcall site and the sources losing their traces; on a debug + ASAN build the unset-limit and materialized tests abort and the self-append test gets the ASAN use-after-free. The basic two-error test pins existing behaviour and passes on both.Tests
All in
test/js/node/v8/capture-stack-trace.test.js, next to the existing non-numericstackTraceLimittest. Everything that can abort runs in a child process.stackTraceLimitset to"foo", toundefined(from Error.appendStackTrace: don't abort when stackTraceLimit is unset #35100) and deleted, and the source's frames still land in the destination.stack, appended to 100 times fromappendAll():.stackunchanged,Bun.inspect(destination)still shows the destination's own frame and notappendAll, the last source keeps its trace, then GC'd for the assertion.sourceURL(and only the destination), 200 rounds inside eval'd functions so GC finalizes the frames (from fix(error): don't capture stack trace on materialized ErrorInstance in appendStackTrace #34713), asserting the unmaterialized sources keep their tracesMalloc=1under ASAN so the freed buffer is visible, asserting the ten frames survive (from Fix use-after-free in Error.appendStackTrace when source is destination #32098)Background
ErrorInstancekeeps a nativeVector<StackFrame>until something reads.stack(or line/column/sourceURL); at that point the frames are formatted into properties and dropped, andm_errorInfoMaterializedrecords that this happened. GC'sfinalizeUnconditionallyalso formats and drops frames early when one of them points at code that is about to be collected, which is the path that asserts if frames exist after materialization.JSGlobalObject::stackTraceLimit()is astd::optional<unsigned>; assigning a non-number toError.stackTraceLimitor deleting it storesnullopt, and errors created while it is unset get no frames at all.console.error,Bun.inspect, uncaught errors) throughZigException.cpp, which uses the error's native frames when it still has any and only falls back to parsing the.stackstring otherwise; that is why frames installed after materialization are visible even though.stacknever changes.Error.appendStackTraceis a Bun-specific,Private-visibility helper installed on theErrorconstructor (ZigGlobalObject.cpp); it is reachable from user code, which is how the fuzzer hit all three cases.CI
Latest head is c7a06b3 (comments condensed, no code change since bf028d9). Build #94462: 176 of 179 jobs passed, including every build lane and the x64 ASAN test lanes. The three that did not pass never started: the two
darwin 14 aarch64 - test-bunjobs are still waiting for an agent (same as on the two previous builds) and onewindows 2019 x64 - test-bunshard failed to get an agent created; the other shards of that lane passed. The yellow entries (child_process ipc handle, fs read stream, napi, bun-patch, cpu-prof, watch mode, cluster) passed on retry or alone and none of them touchError.appendStackTrace.Nothing in this change is platform specific, so I am not retriggering for those lanes. Locally, starting from a build directory that predates the main merges, the tree builds cleanly both with and without the
FormatStackTraceForJS.cppchange; without it 4 of the 5 appendStackTrace tests fail, with it the file passes 48/48.[review] gate passed · iteration 2 · 2 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 2 passed · 0 rejected · iteration 2
evidence per changed file
root cause · written by the author bot
Error.appendStackTraceassumed a happy path that user code can violate: appending an error's frames to itself passedappendVectora span into the same buffer being grown, so reallocation left a dangling pointer and a use after free; appending onto an error whose stack had already been materialized tripped a debug assertion when new frames were installed; and whenError.stackTraceLimitwas not a number the emptystackTraceLimit()optional was unwrapped unconditionally and aborted. The fix adds three guards in the binding: it returns early when source and destination are the same obje…