Skip to content

[v24.x] deps: V8: cherry-pick 657d8de27427#62784

Open
guybedford wants to merge 2 commits intonodejs:v24.x-stagingfrom
guybedford:maglev-throwing-eager-inline
Open

[v24.x] deps: V8: cherry-pick 657d8de27427#62784
guybedford wants to merge 2 commits intonodejs:v24.x-stagingfrom
guybedford:maglev-throwing-eager-inline

Conversation

@guybedford
Copy link
Copy Markdown
Contributor

@guybedford guybedford commented Apr 17, 2026

This backports the V8 commit to fix a correctness bug in WebAssembly Exception Handling under Maglev where throwing nodes inside eager-inlined functions could escape their enclosing try/catch handler.

Original commit message:

[maglev] Fix throwing node inside eager inlining

This commit refactors the exception handling logic to correctly identify
and associate nodes with their respective `catch` blocks, even
when multiple levels of inlining are involved.

Previously, the check `!IsInsideTryBlock() && !is_eager_inline()` was
insufficient to determine if catch block inside `CatchDetails` was
already created.

Specifically, consider the case where:
1. Function `bar` is non-eagerly inlined into `foo`.
2. `foo` contains a `catch` block.
3. `bar` calls `in_bar`, which is eagerly inlined.
4. A node within `in_bar` can `throw`.

In this scenario, `is_eager_inline` would be true when compiling
`in_bar`, leading to an incorrect assumption that the catch block didn't exist yet.

This change addresses the issue by propagating a boolean value via
`CatchDetails`. This boolean accurately indicates whether a `catch`
block is present in the call chain, allowing for correct exception
handling regardless of inlining depth or eagerness.

Refs: v8/v8@657d8de

victorgomes and others added 2 commits April 16, 2026 18:04
Original commit message:

    [maglev] Fix throwing node inside eager inlining

    This commit refactors the exception handling logic to correctly identify
    and associate nodes with their respective `catch` blocks, even
    when multiple levels of inlining are involved.

    Previously, the check `!IsInsideTryBlock() && !is_eager_inline()` was
    insufficient to determine if catch block inside `CatchDetails` was
    already created.

    Specifically, consider the case where:
    1. Function `bar` is non-eagerly inlined into `foo`.
    2. `foo` contains a `catch` block.
    3. `bar` calls `in_bar`, which is eagerly inlined.
    4. A node within `in_bar` can `throw`.

    In this scenario, `is_eager_inline` would be true when compiling
    `in_bar`, leading to an incorrect assumption that the catch block didn't exist yet.

    This change addresses the issue by propagating a boolean value via
    `CatchDetails`. This boolean accurately indicates whether a `catch`
    block is present in the call chain, allowing for correct exception
    handling regardless of inlining depth or eagerness.

    Fixed: 417768368
    Change-Id: Ic52f72f302b4dc644bdcad939addf98111bc525b
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6563500
    Commit-Queue: Victor Gomes <victorgomes@chromium.org>
    Reviewed-by: Darius Mercadier <dmercadier@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#100380}

Refs: v8/v8@657d8de
@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 build files or the CI. 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 Apr 17, 2026
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 build files or the CI. 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.

4 participants