[wasm] Fix unterminated block in WKI return_call for inlined functions [v8/v8 : main]
Jakob Kummerow (Gerrit)
Jakob Kummerow added 1 comment![]( "Open in Gerrit")
This is not a fix: it effectively doesn't change any behavior (the old code would take the `return` on line 2732), and doesn't avoid the failure (run the repro to see for yourself).
Related details
- Ayako Iwasaki
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ayako Iwasaki (Gerrit)
Ayako Iwasaki added 1 comment![]( "Open in Gerrit")
This is not a fix: it effectively doesn't change any behavior (the old code would take the `return` on line 2732), and doesn't avoid the failure (run the repro to see for yourself).
Sorry about PS1. That was indeed my misunderstanding and a no-op as Jakob san pointed out.
I uploaded PS2 as the actual fix, adding a decoder->current_catch() == -1 check
to skip the WKI fast path when inside a try block.
HandleWellKnownImport uses kCatchInThisFrame internally, which routes exceptions to the enclosing try's catch block. But return_call doesn't call MarkMightThrow(), so at kExprEnd the decoder skips the implicit CatchAll/Rethrow for catch-less try blocks, leaving that catch block unterminated in the graph.
Outside try blocks (current_catch() == -1), kCatchInThisFrame does not create exception-routing blocks, so the WKI fast path is safe there.
Tested with a debug build (is_debug=true, v8_enable_slow_dchecks=true). repro-529842414.js triggers a DCHECK failure before the fix and exits cleanly after. return-call-wki.js (no try blocks) still passes.
Related details
- Jakob Kummerow
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ayako Iwasaki (Gerrit)
Ayako Iwasaki added 1 comment![]( "Open in Gerrit")
Daniel san created a fix (https://chromium-review.googlesource.com/c/v8/v8/+/8025515) and we can safely close this PL. Sorry to bother you.
Related details
- Jakob Kummerow
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ayako Iwasaki (Gerrit)
Ayako Iwasaki abandoned this change.
Ayako Iwasaki abandoned this change![]( "Open in Gerrit")
Related details
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jakob Kummerow (Gerrit)
Jakob Kummerow added 1 comment![]( "Open in Gerrit")
Thanks for following up here, and don't worry about having introduced a regression: it happens to all of us, that's why we have tests and fuzzers and so on.
I agree that crrev.com/c/8025515 has a few bits we'd still have to add here (in particular: great test coverage), so let's go with that. Sorry for the collision with your work; we appreciate your prompt effort to fix this, and are looking forward to future contributions 😊
Related details
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |