Skip to content

Conversation

@aheejin
Copy link
Member

@aheejin aheejin commented Aug 5, 2019

This adds basic support for exception handling instructions, according
to the spec:
https://github.com/WebAssembly/exception-handling/blob/master/proposals/Exceptions.md

This PR includes support for:

  • Binary reading/writing
  • Wast reading/writing
  • Stack IR
  • Validation
  • binaryen.js + C API
  • Few IR routines: branch-utils, type-updating, etc
  • Few passes: just enough to make wasm-opt -O pass
  • Tests

This PR does not include support for many optimization passes, fuzzer,
or interpreter. They will be follow-up PRs.

Try-catch construct is modeled in Binaryen IR in a similar manner to
that of if-else: each of try body and catch body will contain a block,
which can be omitted if there is only a single instruction. This block
will not be emitted in wast or binary, as in if-else. As in if-else,
class Try contains two expressions each for try body and catch body,
and catch is not modeled as an instruction. exnref value pushed by
catch is get by pop instruction.

br_on_exn is special: it returns different types of values when taken
and not taken. We make exnref, the type br_on_exn pushes if not
taken, as br_on_exn's type.

This adds basic support for exception handling instructions, according
to the spec:
https://github.com/WebAssembly/exception-handling/blob/master/proposals/Exceptions.md

This PR includes support for:
- Binary reading/writing
- Wast reading/writing
- Stack IR
- Validation (including a br_on_exn validator)
- binaryen.js + C API
- Few IR routines: branch-utils, type-updating, etc
- Few passes: just enough to make `wasm-opt -O` pass
- Tests

This PR does not include support for many optimization passes or the
fuzzer. They will be follow-up PRs.

Try-catch construct is modeled in Binaryen IR in a similar manner to
that of if-else: each of try body and catch body will contain a block,
which can be omitted if there is only a single instruction. This block
will not be emitted in wast or binary, as in if-else. As in if-else,
`class Try` contains two expressions each for try body and catch body,
and `catch` is not modeled as an instruction. `exnref` value pushed by
`catch` or `br_on_exn` is get by `pop` instruction.

`br_on_exn` is special: it returns different types of values when taken
and not taken. This makes validation a little complicated.
@aheejin aheejin requested review from kripken and tlively August 5, 2019 14:28
Copy link
Member

@kripken kripken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work!

@aheejin
Copy link
Member Author

aheejin commented Aug 6, 2019

We currently have a Travis CI error which is not reproducible locally, so I added a few debugging commits that comment out some tests or add debug printfs. They will be deleted after debugging.

(Added later: this has been deleted now)

@aheejin
Copy link
Member Author

aheejin commented Aug 7, 2019

Talked with @kripken offline and decided do these things:

  1. Make exnref, not extracted event type, as br_on_exn's type. This will remove the need for the special validation rule for br_on_exn and also removes the need to use exnref.pop after br_on_exn.
  2. Keep event type copy within BrOnExn class. It is necessary if we do 1, because BrOnExn does not have the event type as its type anymore.

Will update the code with this change soon.

@aheejin
Copy link
Member Author

aheejin commented Aug 7, 2019

Talked with @kripken offline and decided do these things:

  1. Make exnref, not extracted event type, as br_on_exn's type. This will remove the need for the special validation rule for br_on_exn and also removes the need to use exnref.pop after br_on_exn.
  2. Keep event type copy within BrOnExn class. It is necessary if we do 1, because BrOnExn does not have the event type as its type anymore.

Will update the code with this change soon.

Updated.

Copy link
Member

@kripken kripken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! lgtm, with just some final minor comments/suggestions.

Copy link
Member

@kripken kripken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, lgtm!

@aheejin
Copy link
Member Author

aheejin commented Aug 12, 2019

Thank you for the review!

@aheejin aheejin merged commit e2f49d8 into WebAssembly:master Aug 12, 2019
@aheejin aheejin deleted the eh branch August 12, 2019 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants