wasm2c: implement the exception-handling proposal #1930

keithw · 2022-06-01T08:41:10Z

~~(This PR is behind #1814, #1877, and #1887, but if there is interest in landing it sooner, it could be rebased on top of the current ToT.)~~

This PR adds support for the exception-handling proposal to wasm2c. The strategy involves setjmp at every try-catch and try-delegate, and then throwing/rethrowing uses goto (if the nearest catch is in the same function) or longjmp. There is no use of malloc/free; the wasm runtime maintains static storage for one "active exception," and one unwind target in a jmp_buf. (I couldn't think of a way to implement "zero-cost" exception handling in portable C, even with libunwind.)

This also includes code in generate-names.cc (to generate names for try blocks) and apply-names.cc (to apply the correct names to delegate targets). I think this is essentially the apply-names version of the same fix that was done to resolve-names in 6448318. (@takikawa, would you be able to take a look?)

keithw · 2022-07-01T01:10:43Z

Rebased on current main branch per conversation at #1939.

sbc100

First pass.. looks good. Didn't get a chance to read through all the new CWriter methods but if the tests pass thats pretty awesome! Thanks for all the work on this.

sbc100 · 2022-07-01T15:10:47Z

src/apply-names.cc

@@ -375,6 +375,7 @@ Result NameApplier::OnCatchExpr(TryExpr*, Catch* expr) {
 }

 Result NameApplier::OnDelegateExpr(TryExpr* expr) {
+  PopLabel();


Why was this not breaking other things?

Good question... I think nothing actually checks that the "right" names get applied to things (other than the actual try-delegate test which obviously we weren't running), and this wouldn't even have been evident in the output of wasm2wat --generate-names before there was a NameGenerator::BeginTryExpr.

src/generate-names.cc

sbc100 · 2022-07-01T15:12:50Z

src/stream.h

@@ -170,6 +170,7 @@ struct OutputBuffer {
 class MemoryStream : public Stream {
 public:
  WABT_DISALLOW_COPY_AND_ASSIGN(MemoryStream);
+  MemoryStream(MemoryStream&&) = default;


Sigh... it's a little ugly.

Pre-this change, CWriter has a single MemoryStream func_stream_ member, which is an internal temporary stream that the whole function body gets temporarily written into (in Write(const Func& func)). This is used so that it can first scan the function body to learn the stack vars used (while writing the function body to the internal temporary stream), then write the stack var declarations to the real output, and then finally copy func_stream_ to the real output.

Post-this change, CWriter stores a vector of MemoryStreams (std::vector<std::pair<std::string, MemoryStream>> func_sections_), each tagged with the label of a caught exception (or with an empty tag if the section is unconditionally included). We do this because we don't know whether the exception is going to be rethrown until later (if we see a subsequent rethrow instruction that rethrows that caught exception), and I didn't want to emit the code to copy the exception to the stack unless it's really needed. To put a MemoryStream in a vector, it needs to have a move constructor.

We could make this simpler at the cost of copying every caught exception to the stack (whether needed or not).

wasm2c/wasm-rt-impl.c

sbc100 · 2022-07-01T15:24:42Z

wasm2c/wasm-rt.h

- * Stop execution immediately and jump back to the call to `wasm_rt_try`.
- * The result of `wasm_rt_try` will be the provided trap reason.
+ * Stop execution immediately and jump back to the call to `wasm_rt_impl_try`.
+ * The result of `wasm_rt_impl_try` will be the provided trap reason.


Isn't wasm_rt_try still the public interface (defined below)?

Sadly, no -- the old function was always called wasm_rt_impl_try once the separation into header and impl happened (but the comment was not updated). This is still the right function for "entering" a WebAssembly function from the host and catching any traps (including the new "uncaught exception" trap). Maybe we should add something about this in the wasm2c README.

wasm_rt_try is a new internal thing that is used by the CWriter codegen at the beginning of a try block (and maps to WASM_RT_SETJMP in the default runtime and probably any practical runtime for this interface). It's not meant to be used by external code. Maybe we should give it a more internal-y name?

sbc100 · 2022-07-01T15:29:17Z

src/c-writer.cc

@@ -404,6 +437,16 @@ std::string CWriter::MangleMultivalueTypes(const TypeVector& types) {
  return result;
 }

+// static
+std::string CWriter::MangleTagTypes(const TypeVector& types) {
+  assert(types.size() >= 2);


Why can't a tag have a single type (e.g. i32)?

It can, but then we don't define a new struct for it -- we just use the single type directly (following the approach used for multi-value).

src/c-writer.cc

sbc100 · 2022-07-01T15:33:15Z

src/c-writer.cc

+    if (condition.empty() || func_includes_.count(condition)) {
+      stream_->WriteData(buf->data.data(), buf->data.size());
+    }
+  }


Can you explain this part?

Yes, it's part of #1930 (comment) (this is where we print out the sections of the function, and especially the sections that save caught exceptions to be rethrown later -- if there was a rethrow instruction that identifies that caught exception).

src/c-writer.cc

And use the `wasm_rt_` prefix for the one non-static global. While reviewing #1930 I noticed these globals could/should be static.

(The imports.txt and exports.txt tests require multi-table and are disabled until reference types is supported.)

…ction

keithw requested review from sbc100 and aheejin June 1, 2022 08:41

keithw force-pushed the exception-handling branch 4 times, most recently from a0bb82e to 2c8913e Compare June 4, 2022 05:35

keithw force-pushed the exception-handling branch 2 times, most recently from 134db6b to 8080754 Compare June 30, 2022 10:19

keithw mentioned this pull request Jun 30, 2022

wasm2c: run tests with -O2 on non-Windows #1939

Merged

keithw force-pushed the exception-handling branch from 8080754 to ef1d88d Compare July 1, 2022 01:08

sbc100 reviewed Jul 1, 2022

View reviewed changes

keithw force-pushed the exception-handling branch 2 times, most recently from be6bfc9 to 2a6e033 Compare July 13, 2022 00:12

sbc100 added a commit that referenced this pull request Jul 13, 2022

wasm2c: Use static for wasm-rt globals. NFC

c5dfa37

And use the `wasm_rt_` prefix for the one non-static global. While reviewing #1930 I noticed these globals could/should be static.

sbc100 mentioned this pull request Jul 13, 2022

wasm2c: Use static for wasm-rt globals. NFC #1945

Merged

sbc100 added a commit that referenced this pull request Jul 13, 2022

wasm2c: Use static for wasm-rt globals. NFC

80a96d5

And use the `wasm_rt_` prefix for the one non-static global. While reviewing #1930 I noticed these globals could/should be static.

sbc100 added a commit that referenced this pull request Jul 14, 2022

wasm2c: Use static for wasm-rt globals. NFC (#1945)

54c5535

And use the `wasm_rt_` prefix for the one non-static global. While reviewing #1930 I noticed these globals could/should be static.

keithw added 9 commits July 13, 2022 17:49

Add --enable-exceptions flag to wasm2c and run-spec-wasm2c.py

0380ede

wasm2c: implement exception-handling proposal

61f07de

wasm2c: add exception-handling tests

cef0dec

(The imports.txt and exports.txt tests require multi-table and are disabled until reference types is supported.)

Update README.md to reflect wasm2c exception handling

991286c

generate-names.cc: name try labels with "T" (instead of "R")

99fa10e

wasm-rt-impl.c: make exception globals static + move tag_count to fun…

8a15706

…ction

c-writer.cc: call init_tags() unconditionally

c9bf931

c-writer.cc: simplify handling of Block exprs

80dfa1e

wasm-rt-impl.c: statically allocate g_active_exception

1b68135

keithw force-pushed the exception-handling branch from 6f94106 to 1b68135 Compare July 14, 2022 00:51

sbc100 approved these changes Jul 14, 2022

View reviewed changes

sbc100 enabled auto-merge (squash) July 14, 2022 01:01

sbc100 merged commit ec53e18 into WebAssembly:main Jul 14, 2022

keithw deleted the exception-handling branch October 4, 2022 00:46

keithw mentioned this pull request Oct 4, 2023

Reintroducing exnref WebAssembly/exception-handling#280

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

wasm2c: implement the exception-handling proposal #1930

wasm2c: implement the exception-handling proposal #1930

keithw commented Jun 1, 2022 •

edited

Loading

keithw commented Jul 1, 2022

sbc100 left a comment

sbc100 Jul 1, 2022

keithw Jul 13, 2022

sbc100 Jul 1, 2022

keithw Jul 13, 2022 •

edited

Loading

sbc100 Jul 1, 2022

keithw Jul 13, 2022

sbc100 Jul 1, 2022

keithw Jul 13, 2022

sbc100 Jul 1, 2022

keithw Jul 13, 2022

wasm2c: implement the exception-handling proposal #1930

wasm2c: implement the exception-handling proposal #1930

Conversation

keithw commented Jun 1, 2022 • edited Loading

keithw commented Jul 1, 2022

sbc100 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

keithw Jul 13, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

keithw commented Jun 1, 2022 •

edited

Loading

keithw Jul 13, 2022 •

edited

Loading