[wasm] Fix Wasm-in-JS inlining with lazy validation [v8/v8 : main]

0 views
Skip to first unread message

Daniel Lehmann (Gerrit)

unread,
Oct 22, 2025, 11:29:13 AM (yesterday) Oct 22
to Clemens Backes, Matthias Liedtke, V8 LUCI CQ, dmercadi...@chromium.org, v8-re...@googlegroups.com
Attention needed from Clemens Backes

Daniel Lehmann added 1 comment

Patchset-level comments
File-level comment, Patchset 2 (Latest):
Daniel Lehmann . resolved

Clemens, could you please take a look (to spread reviewer load).
CC'ing Matthias to keep him in the loop wrt. Wasm-in-JS inlining.

Open in Gerrit

Related details

Attention is currently required from:
  • Clemens Backes
Submit Requirements:
  • requirement satisfiedCode-Owners
  • requirement is not satisfiedCode-Review
  • requirement is not satisfiedReview-Enforcement
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: v8/v8
Gerrit-Branch: main
Gerrit-Change-Id: I6fa376075462d8b1ddcc22a505ebb0e1f5e8937a
Gerrit-Change-Number: 7072914
Gerrit-PatchSet: 2
Gerrit-Owner: Daniel Lehmann <dleh...@chromium.org>
Gerrit-Reviewer: Clemens Backes <clem...@chromium.org>
Gerrit-Reviewer: Daniel Lehmann <dleh...@chromium.org>
Gerrit-CC: Matthias Liedtke <mlie...@chromium.org>
Gerrit-Attention: Clemens Backes <clem...@chromium.org>
Gerrit-Comment-Date: Wed, 22 Oct 2025 15:29:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
satisfied_requirement
unsatisfied_requirement
open
diffy

Daniel Lehmann (Gerrit)

unread,
Oct 22, 2025, 11:59:16 AM (24 hours ago) Oct 22
to Clemens Backes, Matthias Liedtke, V8 LUCI CQ, dmercadi...@chromium.org, v8-re...@googlegroups.com
Attention needed from Clemens Backes

Daniel Lehmann added 2 comments

File src/compiler/turboshaft/wasm-in-js-inlining-reducer-inl.h
Line 1372, Patchset 2 (Latest): CHECK(v8_flags.wasm_lazy_validation);
Daniel Lehmann . unresolved

I don't think we can have any other configuration where we end up in this branch. E.g., Wasm eager compilation, PGO, or compilation hints would not trigger JS compilation (in which we would inline the Wasm function).

File test/mjsunit/regress/wasm/regress-441844767.js
Line 19, Patchset 2 (Latest):try {
Daniel Lehmann . unresolved

I manually reduced this from the Clusterfuzz test case, no idea where the try/catch came from there, but I didn't want to spend much more time on it.

Open in Gerrit

Related details

Attention is currently required from:
  • Clemens Backes
Submit Requirements:
    • requirement satisfiedCode-Owners
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    • requirement is not satisfiedReview-Enforcement
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: v8/v8
    Gerrit-Branch: main
    Gerrit-Change-Id: I6fa376075462d8b1ddcc22a505ebb0e1f5e8937a
    Gerrit-Change-Number: 7072914
    Gerrit-PatchSet: 2
    Gerrit-Owner: Daniel Lehmann <dleh...@chromium.org>
    Gerrit-Reviewer: Clemens Backes <clem...@chromium.org>
    Gerrit-Reviewer: Daniel Lehmann <dleh...@chromium.org>
    Gerrit-CC: Matthias Liedtke <mlie...@chromium.org>
    Gerrit-Attention: Clemens Backes <clem...@chromium.org>
    Gerrit-Comment-Date: Wed, 22 Oct 2025 15:59:11 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Matthias Liedtke (Gerrit)

    unread,
    Oct 22, 2025, 12:03:30 PM (23 hours ago) Oct 22
    to Daniel Lehmann, Clemens Backes, V8 LUCI CQ, dmercadi...@chromium.org, v8-re...@googlegroups.com
    Attention needed from Clemens Backes and Daniel Lehmann

    Matthias Liedtke voted and added 1 comment

    Votes added by Matthias Liedtke

    Code-Review+1

    1 comment

    Patchset-level comments
    Matthias Liedtke . resolved

    LGTM, thanks for fixing, not sure why ClusterFuzz resolved the issue already but your reproducer looks nice, so we should have something reliable now. 😊

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Clemens Backes
    • Daniel Lehmann
    Submit Requirements:
    • requirement satisfiedCode-Owners
    • requirement satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    • requirement satisfiedReview-Enforcement
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: v8/v8
    Gerrit-Branch: main
    Gerrit-Change-Id: I6fa376075462d8b1ddcc22a505ebb0e1f5e8937a
    Gerrit-Change-Number: 7072914
    Gerrit-PatchSet: 2
    Gerrit-Owner: Daniel Lehmann <dleh...@chromium.org>
    Gerrit-Reviewer: Clemens Backes <clem...@chromium.org>
    Gerrit-Reviewer: Daniel Lehmann <dleh...@chromium.org>
    Gerrit-Reviewer: Matthias Liedtke <mlie...@chromium.org>
    Gerrit-Attention: Daniel Lehmann <dleh...@chromium.org>
    Gerrit-Attention: Clemens Backes <clem...@chromium.org>
    Gerrit-Comment-Date: Wed, 22 Oct 2025 16:03:25 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: Yes
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Matthias Liedtke (Gerrit)

    unread,
    Oct 22, 2025, 12:06:10 PM (23 hours ago) Oct 22
    to Daniel Lehmann, Clemens Backes, V8 LUCI CQ, dmercadi...@chromium.org, v8-re...@googlegroups.com
    Attention needed from Clemens Backes and Daniel Lehmann

    Matthias Liedtke added 2 comments

    File src/compiler/turboshaft/wasm-in-js-inlining-reducer-inl.h
    Line 1372, Patchset 2 (Latest): CHECK(v8_flags.wasm_lazy_validation);
    Daniel Lehmann . unresolved

    I don't think we can have any other configuration where we end up in this branch. E.g., Wasm eager compilation, PGO, or compilation hints would not trigger JS compilation (in which we would inline the Wasm function).

    Matthias Liedtke

    Yeah, I think it's good to encode these assumptions into the code. If they get invalidated, we can still address it. IMO better have a chromecrash for this than having a vulnerability because we unexpectedly hit this branch. 😊

    File test/mjsunit/regress/wasm/regress-441844767.js
    Daniel Lehmann . unresolved

    I manually reduced this from the Clusterfuzz test case, no idea where the try/catch came from there, but I didn't want to spend much more time on it.

    Matthias Liedtke

    Both ochang and Fuzzilli like to insert try-catch blocks to continue execution after something that throws. E.g. in this case we need it as the first call fails and we then still need to continue to hit the interesting case.

    Gerrit-Comment-Date: Wed, 22 Oct 2025 16:06:05 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Daniel Lehmann <dleh...@chromium.org>
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Daniel Lehmann (Gerrit)

    unread,
    Oct 22, 2025, 12:09:03 PM (23 hours ago) Oct 22
    to Clemens Backes, Matthias Liedtke, V8 LUCI CQ, dmercadi...@chromium.org, v8-re...@googlegroups.com
    Attention needed from Clemens Backes

    Daniel Lehmann voted and added 3 comments

    Votes added by Daniel Lehmann

    Commit-Queue+2

    3 comments

    Patchset-level comments
    Daniel Lehmann . resolved

    Thanks for the quick review, Matthias (despite the high load!). I'll land and move Clemens to CC, happy to address further issues in a follow-up.

    File src/compiler/turboshaft/wasm-in-js-inlining-reducer-inl.h
    Line 1372, Patchset 2 (Latest): CHECK(v8_flags.wasm_lazy_validation);
    Daniel Lehmann . resolved

    I don't think we can have any other configuration where we end up in this branch. E.g., Wasm eager compilation, PGO, or compilation hints would not trigger JS compilation (in which we would inline the Wasm function).

    Matthias Liedtke

    Yeah, I think it's good to encode these assumptions into the code. If they get invalidated, we can still address it. IMO better have a chromecrash for this than having a vulnerability because we unexpectedly hit this branch. 😊

    Daniel Lehmann

    Acknowledged

    File test/mjsunit/regress/wasm/regress-441844767.js
    Daniel Lehmann . resolved

    I manually reduced this from the Clusterfuzz test case, no idea where the try/catch came from there, but I didn't want to spend much more time on it.

    Matthias Liedtke

    Both ochang and Fuzzilli like to insert try-catch blocks to continue execution after something that throws. E.g. in this case we need it as the first call fails and we then still need to continue to hit the interesting case.

    Daniel Lehmann

    D'uh, good point, since this doesn't validate, it obviously will throw during execution. Resolved, thanks 😊

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Clemens Backes
    Submit Requirements:
      • requirement satisfiedCode-Owners
      • requirement satisfiedCode-Review
      • requirement satisfiedReview-Enforcement
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: comment
      Gerrit-Project: v8/v8
      Gerrit-Branch: main
      Gerrit-Change-Id: I6fa376075462d8b1ddcc22a505ebb0e1f5e8937a
      Gerrit-Change-Number: 7072914
      Gerrit-PatchSet: 2
      Gerrit-Owner: Daniel Lehmann <dleh...@chromium.org>
      Gerrit-Reviewer: Daniel Lehmann <dleh...@chromium.org>
      Gerrit-Reviewer: Matthias Liedtke <mlie...@chromium.org>
      Gerrit-CC: Clemens Backes <clem...@chromium.org>
      Gerrit-Attention: Clemens Backes <clem...@chromium.org>
      Gerrit-Comment-Date: Wed, 22 Oct 2025 16:08:57 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: Yes
      Comment-In-Reply-To: Daniel Lehmann <dleh...@chromium.org>
      Comment-In-Reply-To: Matthias Liedtke <mlie...@chromium.org>
      satisfied_requirement
      open
      diffy

      V8 LUCI CQ (Gerrit)

      unread,
      Oct 22, 2025, 12:10:32 PM (23 hours ago) Oct 22
      to Daniel Lehmann, Clemens Backes, Matthias Liedtke, dmercadi...@chromium.org, v8-re...@googlegroups.com

      V8 LUCI CQ submitted the change

      Change information

      Commit message:
      [wasm] Fix Wasm-in-JS inlining with lazy validation
      Fixed: 441844767
      Change-Id: I6fa376075462d8b1ddcc22a505ebb0e1f5e8937a
      Commit-Queue: Daniel Lehmann <dleh...@chromium.org>
      Reviewed-by: Matthias Liedtke <mlie...@chromium.org>
      Cr-Commit-Position: refs/heads/main@{#103298}
      Files:
      • M src/compiler/turboshaft/wasm-in-js-inlining-reducer-inl.h
      • A test/mjsunit/regress/wasm/regress-441844767.js
      Change size: S
      Delta: 2 files changed, 38 insertions(+), 1 deletion(-)
      Branch: refs/heads/main
      Submit Requirements:
      • requirement satisfiedCode-Review: +1 by Matthias Liedtke
      Open in Gerrit
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: merged
      Gerrit-Project: v8/v8
      Gerrit-Branch: main
      Gerrit-Change-Id: I6fa376075462d8b1ddcc22a505ebb0e1f5e8937a
      Gerrit-Change-Number: 7072914
      Gerrit-PatchSet: 3
      Gerrit-Owner: Daniel Lehmann <dleh...@chromium.org>
      Gerrit-Reviewer: Daniel Lehmann <dleh...@chromium.org>
      Gerrit-Reviewer: Matthias Liedtke <mlie...@chromium.org>
      Gerrit-CC: Clemens Backes <clem...@chromium.org>
      open
      diffy
      satisfied_requirement

      Darius Mercadier (Gerrit)

      unread,
      3:02 AM (8 hours ago) 3:02 AM
      to Daniel Lehmann, V8 LUCI CQ, Clemens Backes, Matthias Liedtke, dmercadi...@chromium.org, v8-re...@googlegroups.com
      Attention needed from Daniel Lehmann

      Darius Mercadier added 1 comment

      File src/compiler/turboshaft/wasm-in-js-inlining-reducer-inl.h
      Line 1378, Patchset 3 (Latest): return OpIndex::Invalid();
      Darius Mercadier . unresolved

      nit: `V<Any>::Invalid()`
      (and maybe consider replacing all other occurrences of OpIndex::Invalid() throughout this function)

      Open in Gerrit

      Related details

      Attention is currently required from:
      • Daniel Lehmann
      Submit Requirements:
      • requirement satisfiedCode-Owners
      • requirement satisfiedCode-Review
      • requirement satisfiedReview-Enforcement
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: comment
      Gerrit-Project: v8/v8
      Gerrit-Branch: main
      Gerrit-Change-Id: I6fa376075462d8b1ddcc22a505ebb0e1f5e8937a
      Gerrit-Change-Number: 7072914
      Gerrit-PatchSet: 3
      Gerrit-Owner: Daniel Lehmann <dleh...@chromium.org>
      Gerrit-Reviewer: Daniel Lehmann <dleh...@chromium.org>
      Gerrit-Reviewer: Matthias Liedtke <mlie...@chromium.org>
      Gerrit-CC: Clemens Backes <clem...@chromium.org>
      Gerrit-CC: Darius Mercadier <dmerc...@chromium.org>
      Gerrit-Attention: Daniel Lehmann <dleh...@chromium.org>
      Gerrit-Comment-Date: Thu, 23 Oct 2025 07:02:30 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: No
      satisfied_requirement
      open
      diffy

      Clemens Backes (Gerrit)

      unread,
      3:37 AM (8 hours ago) 3:37 AM
      to Daniel Lehmann, V8 LUCI CQ, Darius Mercadier, Matthias Liedtke, dmercadi...@chromium.org, v8-re...@googlegroups.com
      Attention needed from Daniel Lehmann

      Clemens Backes voted and added 1 comment

      Votes added by Clemens Backes

      Code-Review+1

      1 comment

      Patchset-level comments
      File-level comment, Patchset 3 (Latest):
      Clemens Backes . resolved

      LGTM from my side (same fix as always with lazy validation).

      Open in Gerrit

      Related details

      Attention is currently required from:
      • Daniel Lehmann
      Submit Requirements:
      • requirement satisfiedCode-Owners
      • requirement satisfiedCode-Review
      • requirement satisfiedReview-Enforcement
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: comment
      Gerrit-Project: v8/v8
      Gerrit-Branch: main
      Gerrit-Change-Id: I6fa376075462d8b1ddcc22a505ebb0e1f5e8937a
      Gerrit-Change-Number: 7072914
      Gerrit-PatchSet: 3
      Gerrit-Owner: Daniel Lehmann <dleh...@chromium.org>
      Gerrit-Reviewer: Clemens Backes <clem...@chromium.org>
      Gerrit-Reviewer: Daniel Lehmann <dleh...@chromium.org>
      Gerrit-Reviewer: Matthias Liedtke <mlie...@chromium.org>
      Gerrit-CC: Darius Mercadier <dmerc...@chromium.org>
      Gerrit-Attention: Daniel Lehmann <dleh...@chromium.org>
      Gerrit-Comment-Date: Thu, 23 Oct 2025 07:37:35 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: Yes
      satisfied_requirement
      open
      diffy
      Reply all
      Reply to author
      Forward
      0 new messages