[EH] Make interpreter handle uncaught exceptions #4369
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When a wasm exception is thrown and uncaught in the interpreter, it caused the whole interpreter to crash, rather than gracefully reporting it. This fixes the problem, and also compares whether an uncaught exception happened when comparing the results before and after optimizations in `--fuzz-exec`. To do that, when `--fuzz-exec` is given, we now compare results even when the function does not have return values. Logs for some existing test have changed because of this.
aheejin
commented
Dec 6, 2021
| } else { | ||
| std::cout << ret << '\n'; | ||
| } | ||
| FunctionResult ret = run(func, wasm, instance); |
Member
Author
There was a problem hiding this comment.
This part is mostly indentation changes due to the removal of if
Member
Author
|
When comparing results, now we compare exceptions. Should we compare whether a trap has occurred too? I think this can depend on some trap-related options, so if we do that, it'd better be done as a followup. |
kripken
approved these changes
Dec 7, 2021
| return {}; | ||
| } catch (const WasmException& e) { | ||
| std::cout << "[exception thrown: " << e << "]" << std::endl; | ||
| return {{}, true}; |
Member
There was a problem hiding this comment.
Separate from this PR, I like this idea of tracking an exception, and I think maybe we should track traps too. That is, we could have an enum "normal, exception, trap", and then line 242 would mark "trap" like this line would mark "exception".
Member
Author
There was a problem hiding this comment.
Do you mind if I change to that in a followup?
aheejin
added a commit
to aheejin/binaryen
that referenced
this pull request
Dec 21, 2021
We used to only compare return values, and in WebAssembly#4369 we started comparing whether an uncaught exception was thrown. This also adds whether a trap occurred to `ExecutionResults`. So in `--fuzz-exec`, if a program with a trap loses the trap or vice versa, it will error out saying the result has changed, unless either of `--ignore-implicit-traps` or `--trans-never-happen` is set.
aheejin
added a commit
to aheejin/binaryen
that referenced
this pull request
Dec 23, 2021
We used to only compare return values, and in WebAssembly#4369 we started comparing whether an uncaught exception was thrown. This also adds whether a trap occurred to `ExecutionResults`. So in `--fuzz-exec`, if a program with a trap loses the trap or vice versa, it will error out saying the result has changed, unless either of `--ignore-implicit-traps` or `--trans-never-happen` is set.
aheejin
added a commit
that referenced
this pull request
Dec 30, 2021
We used to only compare return values, and in #4369 we started comparing whether an uncaught exception was thrown. This also adds whether a trap occurred to `ExecutionResults`. So in `--fuzz-exec`, if a program with a trap loses the trap or vice versa, it will error out saying the result has changed, unless either of `--ignore-implicit-traps` or `--trans-never-happen` is set.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When a wasm exception is thrown and uncaught in the interpreter, it
caused the whole interpreter to crash, rather than gracefully reporting
it. This fixes the problem, and also compares whether an uncaught
exception happened when comparing the results before and after
optimizations in
--fuzz-exec. To do that, when--fuzz-execis given,we now compare results even when the function does not have return
values. Logs for some existing test have changed because of this.