Skip to content

[v24.x backport] deps,v8: backport stack frame optimisation changes - #66241

Draft
aduh95 wants to merge 5 commits into
nodejs:v24.x-stagingfrom
aduh95:backport-stack-frame-opti
Draft

aduh95 wants to merge 5 commits into
nodejs:v24.x-stagingfrom
aduh95:backport-stack-frame-opti

Conversation

@aduh95

@aduh95 aduh95 commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Backport of v8/v8@853553f + #65764 + #66249 to address #64879

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/gyp
  • @nodejs/security-wg
  • @nodejs/v8-update

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to Node.js builds or CI infrastructure. needs-ci PRs that need a full CI run. v24.x Issues that can be reproduced on v24.x or PRs targeting the v24.x-staging branch. v8 engine Issues and PRs related to the V8 dependency. labels Sep 23, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@richardlau

Copy link
Copy Markdown
Member

Will this introduce the regression in #66074 to 24.x?

@aduh95

aduh95 commented Sep 23, 2026

Copy link
Copy Markdown
Contributor Author

Will this introduce the regression in #66074 to 24.x?

Yes

@aduh95
aduh95 marked this pull request as draft September 23, 2026 14:06
victorgomes and others added 5 commits September 26, 2026 18:56
Original commit message:

    [trace] Don't add 'new' to Wasm call site

    When constructors builtins are inlined in Maglev, but the
    constructor itself is not, we need to mark the call site
    flag kIsConstructor. This allows the stack trace builder to add
    the 'new' string.

    However, this should not be done if the target is Wasm.
    This creates the same stack trace string as Ignition.

    Fixed: 450308355
    Change-Id: I5c21eba3b46f97ddca9c51c21a66aa8169c0293c
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7136920
    Reviewed-by: Marja Hölttä <marja@chromium.org>
    Auto-Submit: Victor Gomes <victorgomes@chromium.org>
    Commit-Queue: Victor Gomes <victorgomes@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#103617}

Refs: v8/v8@853553f
Original commit message:

    [objects]: Defer CallSiteInfo creation

    Store the raw data in a FixedArray and create the CallSiteInfo objects
    only on demand.

    This can be further optimized to omit CallSiteInfo creation altogether
    in some code paths, but currently those code paths are not critically important.

    Change-Id: I6480862caf6b64020737527c571e3e3eac704ed3
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7673818
    Commit-Queue: Marja Hölttä <marja@chromium.org>
    Reviewed-by: Leszek Swirski <leszeks@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#106127}

Refs: v8/v8@ebd1578
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: nodejs#65764
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Original commit message:

    [stack traces] Reduce stack frame summarization costs

    During stack trace capture, Summarize() is the most expensive step — it
    creates a full TranslatedState for every optimized frame even though
    most frames are never inspected. This CL reduces that cost in two ways:

    1. Lightweight Summarize() for optimized frames: instead of building a
       full TranslatedState, walk only the deopt translation frame headers
       and resolve function/receiver via ResolveTaggedValue(), falling back
       to the full TranslatedState path for wasm-inlined or unresolvable
       closures.

    2. Deferred baseline frames: during CaptureSimpleStackTrace, baseline
       frames store the raw Code + PC offset and defer bytecode offset
       resolution to ExpandDeferredFrames(), which runs lazily before the
       stack trace is formatted or inspected.

    A new Torque bitfield flag (is_deferred_baseline_frame) marks entries in
    the raw capture array that still need resolution. All consumers
    (GetSimpleStackTrace, GetDetailedStackTraceFromCallSiteInfos,
    GetFormattedStack, PrintCurrentStackTrace) call ExpandDeferredFrames()
    before processing the array.

    Change-Id: I1fe8cce918ba129d655d66f608ac6aa0ed160920
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7722138
    Commit-Queue: Leszek Swirski <leszeks@chromium.org>
    Auto-Submit: Jakob Linke <jgruber@chromium.org>
    Reviewed-by: Leszek Swirski <leszeks@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#106237}

Refs: v8/v8@74e153d
Refs: v8/v8@c9c0abf
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: nodejs#65764
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Original commit message:

    [stack traces] Fall back to full walk when receiver is unboxed

    The lightweight OptimizedJSFrame::Summarize path added in
    crrev.com/c/7722138 assumed that closure and receiver are always
    encoded as LITERAL or TAGGED_STACK_SLOT in the deopt translation.
    This holds for the closure (a JSFunction reference is always tagged)
    but not for the receiver: the "receiver" slot in an InterpretedFrame
    translation is just parameter 0 of the (possibly inlined) frame and
    is emitted via BuildDeoptFrameSingleValue, which encodes it according
    to the value's representation. An inlined frame whose `this` was
    typed as Float64 ends up as DOUBLE_STACK_SLOT, hitting UNREACHABLE in
    ResolveTaggedValue.

    Rename ResolveTaggedValue to TryResolveTaggedValue, return
    std::optional, and fall back to SummarizeFull when the receiver isn't
    a directly-resolvable tagged value.

    Fixed: 499260582
    Change-Id: I1f9cdd28e4b6b76a253a46e43b248d9239a4ecd8
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7748309
    Auto-Submit: Jakob Linke <jgruber@chromium.org>
    Reviewed-by: Leszek Swirski <leszeks@chromium.org>
    Commit-Queue: Leszek Swirski <leszeks@chromium.org>
    Commit-Queue: Jakob Linke <jgruber@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#106423}

Refs: v8/v8@1a00890
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: nodejs#65764
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Original commit message:

    [stack-traces] Fix overflow in Error.stackTraceLimit trimming

    When stack traces are captured for uncaught exceptions (enabled via
    Isolate::SetCaptureStackTraceForUncaughtExceptions, e.g. by the
    inspector or by Node.js's --trace-uncaught), CaptureAndSetErrorStack
    reuses the simple stack trace and trims it to Error.stackTraceLimit.

    Error.stackTraceLimit counts frames, but the raw call site data stores
    CallSiteInfo::Fields::kCount slots per frame, so the trim multiplied the
    limit by kCount: once in the uint32_t comparison against the array
    length and once, as int, to compute the new length. GetStackTraceLimit
    clamps the limit to [0, INT_MAX], so for very large limits the uint32_t
    product can wrap to a value below the array length. The trim branch is
    then taken although the limit exceeds the number of captured frames,
    and the int multiplication of the new length overflows.

    On main (kCount == 5) the product first wraps at 858993460. That limit
    trimmed the raw data to 4 slots (no complete frame) and 858993461 to 9
    slots (one frame), so error.stack silently lost frames. Infinity, the
    value from the Node.js report, is clamped to INT_MAX; its wrapped
    product (2147483643) is not below the array length, so on main it does
    not take the trim branch and does not reach the signed overflow.

    Fix this by comparing the limit with the number of frames in the raw
    data (length / kCount), and only multiplying once the limit is known to
    be smaller than the frame count. The resulting length is then bounded
    by the existing array length and cannot overflow. Behavior for limits
    that did not overflow is unchanged, since the raw data length is always
    a multiple of kCount.

    This regressed with https://crrev.com/c/7673818 (ebd15783b7b,
    "[objects]: Defer CallSiteInfo creation"), which switched from one
    CallSiteInfo per frame to kCount raw slots per frame.

    This is the underlying cause of Node.js issue 66074. The symptom there
    differs from main: Node's V8 14.6 backport of that change has
    kCount == 6 and uses int for the comparison and for RightTrim, so the
    product overflows for limits above 357913941. For many of those,
    including Infinity (INT_MAX * 6 wraps to -6), the result is negative
    and fails "Check failed: new_capacity > 0." in RightTrim. Comparing in
    frames avoids the overflow in both cases.

    The new cctest CaptureStackTraceForUncaughtExceptionHugeStackTraceLimit
    enables capture for uncaught exceptions and checks that limits of
    858993460, 858993461 and Infinity yield the same error.stack as a limit
    of 10, and that a limit of 1 still trims to a single frame. 858993460
    and 858993461 are the first limits whose product with kCount wraps
    around uint32_t; both fail without this change. The new test and the
    existing stack trace tests also pass in a UBSan build, with no
    diagnostics.

    Bug: 565047704
    Refs: nodejs#66074
    Change-Id: I3422ca1de6a7dd9448c7fd53fb9bc5e40e2a17c1
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/8426465
    Reviewed-by: Patrick Thier <pthier@chromium.org>
    Reviewed-by: Leszek Swirski <leszeks@chromium.org>
    Auto-Submit: eliau elkouby (‫אליהו אלקובי‬‎) <eliau.elkouby@gmail.com>
    Commit-Queue: Patrick Thier <pthier@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#110043}

Refs: v8/v8@786c1c2
Fixes: nodejs#66074
Assisted-by: a closed-source coding agent
Signed-off-by: Eliau Elkouby <145869377+eliau2005@users.noreply.github.com>
PR-URL: nodejs#66249
Reviewed-By: René <contact.9a5d6388@renegade334.me.uk>
Reviewed-By: Richard Lau <richard.lau@ibm.com>
@aduh95
aduh95 force-pushed the backport-stack-frame-opti branch from d1996b4 to ea5c29c Compare September 26, 2026 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build Issues and PRs related to Node.js builds or CI infrastructure. needs-ci PRs that need a full CI run. v8 engine Issues and PRs related to the V8 dependency. v24.x Issues that can be reproduced on v24.x or PRs targeting the v24.x-staging branch.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants