Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC] Add Wasm exception support #198

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

whitequark
Copy link
Contributor

@whitequark whitequark commented Oct 8, 2021

This PR is an early preview. I'm interested in feedback on the general approach!

First, a demo:
$ cat >eh.cpp <<END
#include <stdio.h>

__attribute__((noinline)) void doit() {
  throw 1;
}

int main() {
  try {
    puts("before");
    doit();
    puts("after");
  } catch(int) {
    puts("caught");
  }
}
END
$ cat >eh.mjs <<END
import { WASI } from 'wasi';
import { readFile } from 'fs/promises';

(async function() {
  const wasi = new WASI({
    returnOnExit: true
  });
  const wasm = await WebAssembly.compile(
    await readFile(new URL('./eh.wasm', import.meta.url))
  );
  const instance = await WebAssembly.instantiate(wasm, {
    wasi_snapshot_preview1: wasi.wasiImport
  });
  wasi.start(instance);
})();
END
$ ./build/install/opt/wasi-sdk/bin/clang++ eh.cpp -o eh.wasm -fwasm-exceptions -lunwind
$ node --version
v16.10.0
$ node --experimental-wasi-unstable-preview1 --experimental-wasm-eh eh.mjs
(node:1205685) ExperimentalWarning: WASI is an experimental feature. This feature could change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
before
caught

Most of the changes here are in my LLVM fork. Although the commits are attributed to me, this is mostly a port of the work done in Emscripten by @aheejin.

Items that should be resolved before I could see this PR as mergeable:

  • The Clang patch that adds __USING_WASM_EXCEPTIONS__ to predefined macros should be upstreamed.
  • The Clang compiler driver should add -lunwind to the linker command line when called with -fwasm-exceptions.
  • There are two asserts in libc++abi marked as XXX whitequark that are #ifdef'd out. The first of these is not disabled in Emscripten but necessary for unwinding to work, and the second is removed in Emscripten without indicating why. This should be investigated.
  • The personality function used in libc++abi should match the one used in LLVM. Currently, libc++abi uses __gxx_personality_wasm0 and LLVM uses __gxx_wasm_personality_v0. (This works because on WebAssembly libunwind calls the personality function directly. That's also something I suspect is planned to be changed, though I do not know for sure.)
  • For unknown reasons, exceptions cannot be caught in -flto builds.
  • The libc++abi changes should be upstreamed.
  • The libunwind changes should be upstreamed.

Something should be discussed is whether the build process in wasi-sdk should be altered so that it builds two copies of libc++ and libc++abi; one with -fwasm-exceptions and one without. Otherwise, any code that pulls in libc++, even that which is built with -fno-exceptions, will require libunwind to be linked in, and code size will be larger. However, as far as I know, this requires either more compiler driver changes or complicates the build process for downstream code, so I'm not sure how desirable it is.

@whitequark whitequark changed the title [RFC] Add exception handling support [RFC] Add Wasm exception support Oct 8, 2021
@sunfishcode
Copy link
Member

The Clang patch that adds USING_WASM_EXCEPTIONS to predefined macros should be upstreamed.

Do we need a wasm-specific macro here, or can we just use __EXCEPTIONS? Emscripten has multiple macros because it has multiple flavors of exception handling, however wasi-sdk doesn't support exceptions at all right now, so the EH proposal will be the only form.

-fwasm-exceptions

Similarly, do we need a wasm-specific option here, or can we just use -fexceptions?

Something should be discussed is whether the build process in wasi-sdk should be altered so that it builds two copies of libc++ and libc++abi

I expect we will indeed need to build two copies of libc++, as many wasm engines today don't support the EH proposal yet, so we don't want make existing libc++ users incompatible with those engines.

@whitequark
Copy link
Contributor Author

whitequark commented Oct 8, 2021

Do we need a wasm-specific macro here, or can we just use __EXCEPTIONS? Emscripten has multiple macros because it has multiple flavors of exception handling, however wasi-sdk doesn't support exceptions at all right now, so the EH proposal will be the only form.

The implementation of the personality function in upstream libc++abi has two major code paths: one for DWARF exceptions (where the unwinder IP is the actual instruction pointer) and one for SjLj exceptions (where the unwinder IP is a landing pad index). The semantics required from the personality by Wasm exceptions is mostly similar to that of SjLj exceptions, but it diverges in a few important places because SjLj exceptions are two-phase and Wasm exceptions are single-phase. The __USING_WASM_EXCEPTIONS__ predefined macro allows libc++abi to handle all of these three cases while sharing most of the code in the personality function.

Also, this macro fits well into the existing series of predefined macros in the frontend, so I don't see a reason to avoid defining it. (I was surprised to see it's not already there.)

Similarly, do we need a wasm-specific option here, or can we just use -fexceptions?

The -fwasm-exceptions option is currently upstream in Clang, so that's what I used. However, this comment in Emscripten suggests that eventually -fwasm-exceptions will become -fexceptions.

I expect we will indeed need to build two copies of libc++, as many wasm engines today don't support the EH proposal yet, so we don't want make existing libc++ users incompatible with those engines.

That makes sense. Do you have any preferences on how to switch between the builds of libc++/libc++abi? I'd probably patch the compiler driver to pick different versions of the libraries depending on whether -fwasm-exceptions is given. I'd probably call e.g. the libc++ built with exceptions libc++.a and the one without libc++-noexcept.a. Let me know if I should do it any differently.

@whitequark
Copy link
Contributor Author

I've compiled all of the YoWASP tools with the patched SDK and I've tested yosys, nextpnr-ecp5, ecppack (a somewhat diverse set of codebases, the last two including boost). I confirmed that exception handling works just fine.

@sunfishcode
Copy link
Member

Do you have any preferences on how to switch between the builds of libc++/libc++abi? I'd probably patch the compiler driver to pick different versions of the libraries depending on whether -fwasm-exceptions is given. I'd probably call e.g. the libc++ built with exceptions libc++.a and the one without libc++-noexcept.a. Let me know if I should do it any differently.

That sounds reasonable to me.

Our existing builds use the name "libc++.a" for a library which does not have exception support, but I think if someone tries to mix a new clang with exception support with an old libc++.a without it, it would fail at link time anyway, so I don't think it'll be a problem in practice.

@HaydnTrigg
Copy link

I'm working on a project called Blam Creation Suite and I require the exception support with wasi.

I've created a fork of llvm and a fork of the wasi-sdk based off @whitequark 's code and I've merged them to the latest version and updated the makefile. I haven't checked over in detail what changes were made to llvm-project, I simply did a blind merge and prayed as there was little conflicts.

https://github.com/ChimpsAtSea/wasi-sdk
https://github.com/ChimpsAtSea/llvm-project

There is also an issue I've raised with llvm-project regarding a lack of the NDEBUG macro being defined causing an issue with libunwind wanting to use the logAPIs symbol. So there is an extra change on top of the original Makefile changes to manually add -DNDEBUG onto the command line which is a workaround for the issue as of the time of writing.

llvm/llvm-project#59776

Support for -fwasi-exceptions seems to be built in without any further concerns and there doesn't seem to be any activity on this for over a year. Is there a roadmap for getting exception support added to mainline wasi?

@whitequark
Copy link
Contributor Author

Rebased on top of latest wasi-sdk.

@abrown
Copy link
Collaborator

abrown commented Jul 25, 2023

Talking to @sunfishcode and @sbc100 about this:

  • exceptions are not yet supported in standalone engines (see roadmap), which are the main implementations of WASI (targeted by wasi-sdk) — but they are at phase 3
  • wasi-sdk needs to be conservative here for the time being, waiting until there is more support for exceptions to add it in by default
  • the LLVM changes will need to be upstreamed; that work can happen now
  • we started considering ways of building and installing multiple versions of standard libraries (i.e., libc++.a, libc++abi.a); maybe the clang driver can pick between these versions, but that will require upstream clang changes

Overall we all wanted to include exception support in wasi-sdk especially as the proposal reaches phase 4; it just make take a bit of time to get there.

@aheejin
Copy link
Member

aheejin commented Sep 22, 2023

Wasm EH's libcxxabi/libunwind changes have now been upstreamed:
llvm/llvm-project@e6cbba7
llvm/llvm-project@058222b

@whitequark
Copy link
Contributor Author

@aheejin o/ Fantastic work!! I'm really excited about it, too!

@torokati44
Copy link

@abrown:

exceptions are not yet supported in standalone engines

They can be enabled now with an experimental flag in both Node.js and Deno, and they work out of the box in Firefox. 👀

@abrown
Copy link
Collaborator

abrown commented Oct 25, 2024

Yes, the proposal has certainly advanced over the last year; maybe it is now time to revisit this for wasi-sdk.

@whitequark
Copy link
Contributor Author

🎉 Looking forward to being able to use exceptions!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants