[wasm][wasmfx] Trap instead of throw an exception [v8/v8 : main]

[Francis McCabe (Gerrit)]

Attention needed from Jakob Kummerow and Thibaud Michaud

Francis McCabe added 1 comment

Patchset-level comments

File-level comment, Patchset 1 (Latest):

Francis McCabe . resolved

Suspend and Switch were throwing suspenderror exceptions. CHanged to trapping.

Open in Gerrit

Related details

Attention is currently required from:

• Jakob Kummerow
• Thibaud Michaud

Submit Requirements:

• Code-Owners
• Code-Review
• Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: v8/v8

Gerrit-Branch: main

Gerrit-Change-Id: Iae7999b972569fbb188423119050a46ba0d80c9c

Gerrit-Change-Number: 7718708

Gerrit-PatchSet: 1

Gerrit-Owner: Francis McCabe <f...@chromium.org>

Gerrit-Reviewer: Francis McCabe <f...@chromium.org>

Gerrit-Reviewer: Jakob Kummerow <jkum...@chromium.org>

Gerrit-Reviewer: Thibaud Michaud <thib...@chromium.org>

Gerrit-CC: V8 LUCI CQ <v8-s...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-Attention: Thibaud Michaud <thib...@chromium.org>

Gerrit-Attention: Jakob Kummerow <jkum...@chromium.org>

Gerrit-Comment-Date: Wed, 01 Apr 2026 15:58:31 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

[Thibaud Michaud (Gerrit)]

Attention needed from Francis McCabe and Jakob Kummerow

Thibaud Michaud added 3 comments

Patchset-level comments

File-level comment, Patchset 1 (Latest):

Thibaud Michaud . resolved

Thanks! Looks good so far, but the change is incomplete so I don't think we should land it as is. See comments below.

File src/wasm/turboshaft-graph-interface.cc

Line 4135, Patchset 1 (Latest): __ TrapIf(is_on_central_stack, TrapId::kTrapSuspend);

Thibaud Michaud . unresolved

This only handles a particular case: when we are currently on the central stack.
The more general case is that we can't have any central stack frames in the captured stack chain. This is handled separately in the C call, which returns null instead of the target stack if the suspend/switch is invalid. Then the builtin handles it by throwing a SuspendError.
So we also need to change the builtins to trap instead of throw.

File test/mjsunit/wasm/stack-switching.js

Line 397, Patchset 1 (Latest): WebAssembly.SuspendError, /WasmFX: unhandled suspend/);

Thibaud Michaud . unresolved

See here and 4 lines below, we still throw a `SuspendError` instead of trapping because this is the more general case.
Same with the `SuspendError` test in `stack-switching-switch.js`.

Open in Gerrit

Related details

Attention is currently required from:

• Francis McCabe
• Jakob Kummerow

Submit Requirements:

• Code-Owners
• Code-Review

• No-Unresolved-Comments

• Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: v8/v8

Gerrit-Branch: main

Gerrit-Change-Id: Iae7999b972569fbb188423119050a46ba0d80c9c

Gerrit-Change-Number: 7718708

Gerrit-PatchSet: 1

Gerrit-Owner: Francis McCabe <f...@chromium.org>

Gerrit-Reviewer: Francis McCabe <f...@chromium.org>

Gerrit-Reviewer: Jakob Kummerow <jkum...@chromium.org>

Gerrit-Reviewer: Thibaud Michaud <thib...@chromium.org>

Gerrit-CC: V8 LUCI CQ <v8-s...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-Attention: Francis McCabe <f...@chromium.org>

Gerrit-Attention: Jakob Kummerow <jkum...@chromium.org>

Gerrit-Comment-Date: Wed, 01 Apr 2026 16:37:34 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

[Francis McCabe (Gerrit)]

Attention needed from Francis McCabe

Francis McCabe added 1 comment

Patchset-level comments

File-level comment, Patchset 1 (Latest):

Francis McCabe . resolved

Am abandoning this CL. I will be creating a new one with under my google.com email.
And I will reflect the comments.

Open in Gerrit

Related details

Attention is currently required from:

• Francis McCabe

Submit Requirements:

• Code-Owners
• Code-Review
• No-Unresolved-Comments
• Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: v8/v8

Gerrit-Branch: main

Gerrit-Change-Id: Iae7999b972569fbb188423119050a46ba0d80c9c

Gerrit-Change-Number: 7718708

Gerrit-PatchSet: 1

Gerrit-Owner: Francis McCabe <f...@chromium.org>

Gerrit-Reviewer: Francis McCabe <f...@chromium.org>

Gerrit-Reviewer: Thibaud Michaud <thib...@chromium.org>

Gerrit-CC: Francis McCabe <f...@google.com>

Gerrit-CC: Jakob Kummerow <jkum...@chromium.org>

Gerrit-CC: V8 LUCI CQ <v8-s...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-Attention: Francis McCabe <f...@chromium.org>

Gerrit-Comment-Date: Wed, 01 Apr 2026 18:13:27 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

[Francis McCabe (Gerrit)]

Attention needed from Jakob Kummerow and Thibaud Michaud

Francis McCabe added 1 comment

Patchset-level comments

File-level comment, Patchset 3 (Latest):

Francis McCabe . resolved

We should trap (throw a runtime error) instead throwing a regular exception.

Open in Gerrit

Related details

Attention is currently required from:

• Jakob Kummerow
• Thibaud Michaud

Submit Requirements:

• Code-Owners
• Code-Review

• Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: v8/v8

Gerrit-Branch: main

Gerrit-Change-Id: I3376c0a974c6b5f33ba82a750c3d022afc928a25

Gerrit-Change-Number: 7722047

Gerrit-PatchSet: 3

Gerrit-Owner: Francis McCabe <f...@google.com>

Gerrit-Reviewer: Francis McCabe <f...@google.com>

Gerrit-Reviewer: Jakob Kummerow <jkum...@chromium.org>

Gerrit-Reviewer: Thibaud Michaud <thib...@chromium.org>

Gerrit-CC: AyeAye <android-bu...@system.gserviceaccount.com>

Gerrit-CC: V8 LUCI CQ <v8-s...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-Attention: Thibaud Michaud <thib...@chromium.org>

Gerrit-Attention: Jakob Kummerow <jkum...@chromium.org>

Gerrit-Comment-Date: Thu, 02 Apr 2026 00:17:07 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

[Thibaud Michaud (Gerrit)]

Thibaud Michaud abandoned this change.

View Change

Abandoned Abandoned in favor of https://crrev.com/c/7722047

Thibaud Michaud abandoned this change

Related details

Attention set is empty

Submit Requirements:

• Code-Review

• No-Unresolved-Comments

• Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: abandon

Gerrit-Project: v8/v8

Gerrit-Branch: main

Gerrit-Change-Id: Iae7999b972569fbb188423119050a46ba0d80c9c

Gerrit-Change-Number: 7718708

Gerrit-PatchSet: 1

Gerrit-Owner: Francis McCabe <f...@chromium.org>

Gerrit-Reviewer: Francis McCabe <f...@chromium.org>

Gerrit-Reviewer: Thibaud Michaud <thib...@chromium.org>

Gerrit-CC: Francis McCabe <f...@google.com>

Gerrit-CC: Jakob Kummerow <jkum...@chromium.org>

[Thibaud Michaud (Gerrit)]

Attention needed from Francis McCabe and Jakob Kummerow

Thibaud Michaud voted and added 2 comments

Votes added by Thibaud Michaud

Code-Review

+1

2 comments

Patchset-level comments

File-level comment, Patchset 3 (Latest):

Thibaud Michaud . resolved

LGTM with a nit

File src/builtins/x64/builtins-x64.cc

Line 5, Patchset 3 (Latest):#include "src/codegen/interface-descriptors.h"

Thibaud Michaud . unresolved

I don't think this include is needed, at least not for this change's delta.
If it is, please move it behind the #if guard with the other includes.

Open in Gerrit

Related details

Attention is currently required from:

• Francis McCabe
• Jakob Kummerow

Submit Requirements:

• Code-Owners

• Code-Review
• No-Unresolved-Comments

• Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: v8/v8

Gerrit-Branch: main

Gerrit-Change-Id: I3376c0a974c6b5f33ba82a750c3d022afc928a25

Gerrit-Change-Number: 7722047

Gerrit-PatchSet: 3

Gerrit-Owner: Francis McCabe <f...@google.com>

Gerrit-Reviewer: Francis McCabe <f...@google.com>

Gerrit-Reviewer: Jakob Kummerow <jkum...@chromium.org>

Gerrit-Reviewer: Thibaud Michaud <thib...@chromium.org>

Gerrit-CC: AyeAye <android-bu...@system.gserviceaccount.com>

Gerrit-CC: V8 LUCI CQ <v8-s...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-Attention: Francis McCabe <f...@google.com>

Gerrit-Attention: Jakob Kummerow <jkum...@chromium.org>

Gerrit-Comment-Date: Thu, 02 Apr 2026 10:36:50 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: Yes

[Francis McCabe (Gerrit)]

Attention needed from Jakob Kummerow and Thibaud Michaud

Francis McCabe added 2 comments

Patchset-level comments

File-level comment, Patchset 4 (Latest):

Francis McCabe . resolved

Removed funny include

File src/builtins/x64/builtins-x64.cc

Line 5, Patchset 3:#include "src/codegen/interface-descriptors.h"

Thibaud Michaud . resolved

I don't think this include is needed, at least not for this change's delta.
If it is, please move it behind the #if guard with the other includes.

Francis McCabe

Acknowledged

Open in Gerrit

Related details

Attention is currently required from:

• Jakob Kummerow
• Thibaud Michaud

Submit Requirements:

• Code-Owners
• Code-Review

• Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: v8/v8

Gerrit-Branch: main

Gerrit-Change-Id: I3376c0a974c6b5f33ba82a750c3d022afc928a25

Gerrit-Change-Number: 7722047

Gerrit-PatchSet: 4

Gerrit-Owner: Francis McCabe <f...@google.com>

Gerrit-Reviewer: Francis McCabe <f...@google.com>

Gerrit-Reviewer: Jakob Kummerow <jkum...@chromium.org>

Gerrit-Reviewer: Thibaud Michaud <thib...@chromium.org>

Gerrit-CC: AyeAye <android-bu...@system.gserviceaccount.com>

Gerrit-CC: V8 LUCI CQ <v8-s...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-Attention: Thibaud Michaud <thib...@chromium.org>

Gerrit-Attention: Jakob Kummerow <jkum...@chromium.org>

Gerrit-Comment-Date: Thu, 02 Apr 2026 17:04:58 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

Comment-In-Reply-To: Thibaud Michaud <thib...@chromium.org>