1717971 - Crash on `try-delegate` instruction targeting catchless `try`

Status: RESOLVED FIXED

(Core :: JavaScript: WebAssembly, defect, P1)

Description

[Asumu Takikawa]

Attachment: Crashing test case

The attached test case crashes with the recent change for supporting catchless try blocks in Wasm exception handling.

Test failure looks like this:

/home/asumu/mozilla-unified/js/src/obj-eh/dist/bin/js -f /home/asumu/mozilla-unified/js/src/jit-test/lib/prologue.js --wasm-exceptions -e 'const platform='"'"'linux'"'"'' -e 'const libdir='"'"'/home/asumu/mozilla-unified/js/src/jit-test/lib/'"'"'' -f /home/asumu/mozilla-unified/js/src/jit-test/lib/wasm.js -f /tmp/crash.js                      
Illegal instruction

I believe this is a case that's underspecified by the spec currently, and the crash is likely caused by delegate trying to jump to catch instruction code that doesn't exist.

I filed a spec issue for clarifying the semantics here: https://github.com/WebAssembly/exception-handling/issues/164

In the meantime, what's the best way to address this?

Comment 1

[Lars T Hansen [:lth]]

(In reply to Asumu Takikawa from comment #0)

In the meantime, what's the best way to address this?

Disable exception handling by default in Nightly, probably.

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1716043

Comment 3

[Asumu Takikawa]

Attachment: Bug 1717971 - Fix combination of try-delegate and catchless try

See https://github.com/WebAssembly/exception-handling/issues/164
for a discussion of the intended behavior of this case.

Comment 4

[Asumu Takikawa]

Attachment: Bug 1717971 - Add tests for use of both try-delegate and catchless try

Depends on D118789

Comment 5

[Asumu Takikawa]

There was a relatively quick consensus around what the semantics should be for this case in https://github.com/WebAssembly/exception-handling/issues/164 (everyone so far has agreed that a try-delegate delegating to a catchless try block should effectively rethrow from the latter), so I've attached patches that implement that behavior. I split out the test cases into a separate patch in case we wanted to land them separately.

Would also be happy to submit a patch to disable exceptions on nightly if that's preferred.

Comment 6

[Lars T Hansen [:lth]]

Fixing instead of disabling is fine with me :-) This needs to land before the freeze on July 10, and sooner is better. Mozilla is closed June 26 through July 5. It would be best if Ryan were to review this since he's been shepherding these patches.

Comment 7

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Fix combination of try-delegate and catchless try r=rhunt
https://hg.mozilla.org/integration/autoland/rev/bfae6deb139721f4da4a5771a835bbf867588cf4
https://hg.mozilla.org/mozilla-central/rev/bfae6deb1397

Comment 8

[Ryan Hunt [:rhunt] (on leave until early May)]

This only affected the current nightly, 91, based on the regressing bug. The feature is additionally nightly only too. We should land the test too.

Comment 9

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Add tests for use of both try-delegate and catchless try r=rhunt
https://hg.mozilla.org/integration/autoland/rev/9d23f98bd233a7860be34caf79bd140c2accfb70
https://hg.mozilla.org/mozilla-central/rev/9d23f98bd233