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

Consider adding support for MacOS exception ports #2456

Closed
Jake-Shadle opened this issue Nov 28, 2020 · 11 comments
Closed

Consider adding support for MacOS exception ports #2456

Jake-Shadle opened this issue Nov 28, 2020 · 11 comments

Comments

@Jake-Shadle
Copy link

Jake-Shadle commented Nov 28, 2020

Currently, wasmtime hooks various signals in traphandlers on Unix platforms before executing wasm code so that it can catch and handle various runtime issues without actually crashing the host program. However, on MacOS there is an additional error handling mechanism, Mach Exceptions. By default a Mach exception will be turned into a unix signal, however, if the application binds their own exception port via task_set_exception_ports or similar, it's up to that handler to actual forward to the default signal mechanism, but in most cases they will probably just abort the process instead, even though wasmtime would have successfully recovered program execution in its signal handler.

Feature

The feature I'm proposing would be for wasmtime's traphandler to use exception ports when targeting MacOS (and I guess iOS, but I would assume that target is not relevant for wasmtime due to its code execution rules?), forwarding exceptions to whatever previously registered exception port if the exception is not one of the one's wasmtime wants to handle.

Benefit

The application could have previously registered signal handlers or exception ports on Mac and still function as expected whenever wasmtime executes exceptional wasm code.

Implementation

* I think basically the implementation is mostly conceptually the same as the current signal handling approach, but basically it would be something like...

  1. Store the previous exception port(s)
  2. Set exception port to wasmtime's own handler
  3. When entering trap handling, signal to the handler thread (shared memory, ipc, whatever) that the handler should treat the exceptions it can handle (the equivalents of the SIGILL etc) as runtime errors, but all other exceptions, or when not executing wasm code, should be forwarded to the previous exception port(s)
  4. When exiting trap handling, tell the handler thread to always forward exceptions

Also, one thing that would possibly make this easier would just be to directly signal the thread, similar to Mac's own ux_handler code as seen here.

* All of this information is gleaned from Mach Exceptions, Signas, and Breakpad's Exception Handler, as the official Apple documentation is, frankly, garbage.

Alternatives

The alternative would just be to not do this at all, but in that case I think having a warning in the documentation somewhere so that people running wasmtime on a Mac are aware that having task*_set_exception_ports in their program might mean that wasmtime's normal behavior of catching and handling exception wasm code won't even execute depending on how the user exception handler is implemented.

@Jake-Shadle
Copy link
Author

Oh and yes, I found this because I added breakpad to our project so we can report crashes effectively, but then someone on mac had a panic in some of their WASM code, so I had to go down this rabbit hole.

@alexcrichton
Copy link
Member

Thanks for the report! This seems like something that might be worthwhile to investigate, but somewhat orthogonally the signal handling in wasmtime should forward signals to the previous signal handler if any is registered. Did you find this to not work, however? If so could you describe a bit more about the bug that's happening with the signal handlers today?

@bjorn3
Copy link
Contributor

bjorn3 commented Nov 30, 2020

I think the problem is that the exception gets delivered to an exception port registered by the application before the wasmtime signal handler has the chance to run, thus causing all exceptions that should be handled by wasmtime to instead go to the exception port registered by the application.

@Jake-Shadle
Copy link
Author

Yes, exactly what @bjorn3 said, I've worked around this by instead changing our breakpad fork to just not register the exception ports and instead hook all of the signals that the exception handler used to cover, which works nicely since it can seamlessly work with wasmtime, but it seems like having this in wasmtime would make sense so that it's not at the mercy of whatever other exception handler can be registered that doesn't play nicely.

I was actually surprised that breakpad didn't have an option to change this, but I guess having both breakpad alongside a piece of code like wasmtime that depends on handling signals is fairly niche or something.

@fitzgen
Copy link
Member

fitzgen commented Nov 30, 2020

Supporting this makes sense to me. Probably faster too, not that it really matters in this particular case 😆

@alexcrichton
Copy link
Member

Ah yes thanks for clarifying! Also, yes, agreed we should add this to Wasmtime! Just need to figure out how...

@sunfishcode
Copy link
Member

The old SignalHandlers.cpp file removed in #1365 theoretically had code to do this, though the code is in C++; it may help show what needs to be done here.

@alexcrichton
Copy link
Member

I was poking around this a bit last night (here's the file in its full glory), and one snag is going to be that the exception handler runs on a separate thread because a different thread receives the mach message. Our determination of whether a wasm trap ocurred, however, requires access to thread-local state (e.g. to read the store's map of registered JIT addresses and see if we collide with any of them). If we're running the handler on a separate thread, though, I'm not sure how the mach exception handling thread will know how to associated a store's JIT data with the exception that it just received.

One possible solution is to have a global map that's updated on entry/exit into wasm, but that would require mutex-like synchronization which could be somewhat expensive too.

@alexcrichton
Copy link
Member

Looking at SpiderMonkey, it appears that registers code segments in a process-global map which has synchronized access. I think the answer for wasmtime is likely the same. Somehow we'll need to have cheap-ish "is this a jit region" test and then some way to go from the register state to the thread-local information about the Store and such.

alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue Feb 3, 2021
This commit moves macOS to using mach ports instead of signals for
handling traps. The motivation for this is listed in bytecodealliance#2456, namely that
once mach ports are used in a process that means traditional UNIX signal
handlers won't get used. This means that if Wasmtime is integrated with
Breakpad, for example, then Wasmtime's trap handler never fires and
traps don't work.

The `traphandlers` module is refactored as part of this commit to split
the platform-specific bits into their own files (it was growing quite a
lot for one inline `cfg_if!`). The `unix.rs` and `windows.rs` files
remain the same as they were before with a few minor tweaks for some
refactored interfaces. The `macos.rs` file is brand new and lifts almost
its entire implementation from SpiderMonkey, adapted for Wasmtime
though.

The main gotcha with mach ports is that a separate thread is what
services the exception. Some unsafe magic allows this separate thread to
read non-`Send` and temporary state from other threads, but is hoped to
be safe in this context. The unfortunate downside is that calling wasm
on macOS now involves taking a global lock and modifying a global hash
map twice-per-call. I'm not entirely sure how to get out of this cost
for now, but hopefully for any embeddings on macOS it's not the end of
the world.

Closes bytecodealliance#2456
alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue Feb 3, 2021
This commit moves macOS to using mach ports instead of signals for
handling traps. The motivation for this is listed in bytecodealliance#2456, namely that
once mach ports are used in a process that means traditional UNIX signal
handlers won't get used. This means that if Wasmtime is integrated with
Breakpad, for example, then Wasmtime's trap handler never fires and
traps don't work.

The `traphandlers` module is refactored as part of this commit to split
the platform-specific bits into their own files (it was growing quite a
lot for one inline `cfg_if!`). The `unix.rs` and `windows.rs` files
remain the same as they were before with a few minor tweaks for some
refactored interfaces. The `macos.rs` file is brand new and lifts almost
its entire implementation from SpiderMonkey, adapted for Wasmtime
though.

The main gotcha with mach ports is that a separate thread is what
services the exception. Some unsafe magic allows this separate thread to
read non-`Send` and temporary state from other threads, but is hoped to
be safe in this context. The unfortunate downside is that calling wasm
on macOS now involves taking a global lock and modifying a global hash
map twice-per-call. I'm not entirely sure how to get out of this cost
for now, but hopefully for any embeddings on macOS it's not the end of
the world.

Closes bytecodealliance#2456
alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue Feb 3, 2021
This commit moves macOS to using mach ports instead of signals for
handling traps. The motivation for this is listed in bytecodealliance#2456, namely that
once mach ports are used in a process that means traditional UNIX signal
handlers won't get used. This means that if Wasmtime is integrated with
Breakpad, for example, then Wasmtime's trap handler never fires and
traps don't work.

The `traphandlers` module is refactored as part of this commit to split
the platform-specific bits into their own files (it was growing quite a
lot for one inline `cfg_if!`). The `unix.rs` and `windows.rs` files
remain the same as they were before with a few minor tweaks for some
refactored interfaces. The `macos.rs` file is brand new and lifts almost
its entire implementation from SpiderMonkey, adapted for Wasmtime
though.

The main gotcha with mach ports is that a separate thread is what
services the exception. Some unsafe magic allows this separate thread to
read non-`Send` and temporary state from other threads, but is hoped to
be safe in this context. The unfortunate downside is that calling wasm
on macOS now involves taking a global lock and modifying a global hash
map twice-per-call. I'm not entirely sure how to get out of this cost
for now, but hopefully for any embeddings on macOS it's not the end of
the world.

Closes bytecodealliance#2456
@alexcrichton
Copy link
Member

I've got a prototype of this at #2632 now. @Jake-Shadle if you're still interested mind giving that a spin and seeing if it works for your use case?

@Jake-Shadle
Copy link
Author

We'd be happy to try this out, though I probably can't get to it this week, but next week!

bnjbvr pushed a commit to bnjbvr/wasmtime that referenced this issue Mar 10, 2021
This commit moves macOS to using mach ports instead of signals for
handling traps. The motivation for this is listed in bytecodealliance#2456, namely that
once mach ports are used in a process that means traditional UNIX signal
handlers won't get used. This means that if Wasmtime is integrated with
Breakpad, for example, then Wasmtime's trap handler never fires and
traps don't work.

The `traphandlers` module is refactored as part of this commit to split
the platform-specific bits into their own files (it was growing quite a
lot for one inline `cfg_if!`). The `unix.rs` and `windows.rs` files
remain the same as they were before with a few minor tweaks for some
refactored interfaces. The `macos.rs` file is brand new and lifts almost
its entire implementation from SpiderMonkey, adapted for Wasmtime
though.

The main gotcha with mach ports is that a separate thread is what
services the exception. Some unsafe magic allows this separate thread to
read non-`Send` and temporary state from other threads, but is hoped to
be safe in this context. The unfortunate downside is that calling wasm
on macOS now involves taking a global lock and modifying a global hash
map twice-per-call. I'm not entirely sure how to get out of this cost
for now, but hopefully for any embeddings on macOS it's not the end of
the world.

Closes bytecodealliance#2456
bnjbvr pushed a commit to bnjbvr/wasmtime that referenced this issue Mar 12, 2021
This commit moves macOS to using mach ports instead of signals for
handling traps. The motivation for this is listed in bytecodealliance#2456, namely that
once mach ports are used in a process that means traditional UNIX signal
handlers won't get used. This means that if Wasmtime is integrated with
Breakpad, for example, then Wasmtime's trap handler never fires and
traps don't work.

The `traphandlers` module is refactored as part of this commit to split
the platform-specific bits into their own files (it was growing quite a
lot for one inline `cfg_if!`). The `unix.rs` and `windows.rs` files
remain the same as they were before with a few minor tweaks for some
refactored interfaces. The `macos.rs` file is brand new and lifts almost
its entire implementation from SpiderMonkey, adapted for Wasmtime
though.

The main gotcha with mach ports is that a separate thread is what
services the exception. Some unsafe magic allows this separate thread to
read non-`Send` and temporary state from other threads, but is hoped to
be safe in this context. The unfortunate downside is that calling wasm
on macOS now involves taking a global lock and modifying a global hash
map twice-per-call. I'm not entirely sure how to get out of this cost
for now, but hopefully for any embeddings on macOS it's not the end of
the world.

Closes bytecodealliance#2456
bnjbvr pushed a commit to bnjbvr/wasmtime that referenced this issue Mar 16, 2021
This commit moves macOS to using mach ports instead of signals for
handling traps. The motivation for this is listed in bytecodealliance#2456, namely that
once mach ports are used in a process that means traditional UNIX signal
handlers won't get used. This means that if Wasmtime is integrated with
Breakpad, for example, then Wasmtime's trap handler never fires and
traps don't work.

The `traphandlers` module is refactored as part of this commit to split
the platform-specific bits into their own files (it was growing quite a
lot for one inline `cfg_if!`). The `unix.rs` and `windows.rs` files
remain the same as they were before with a few minor tweaks for some
refactored interfaces. The `macos.rs` file is brand new and lifts almost
its entire implementation from SpiderMonkey, adapted for Wasmtime
though.

The main gotcha with mach ports is that a separate thread is what
services the exception. Some unsafe magic allows this separate thread to
read non-`Send` and temporary state from other threads, but is hoped to
be safe in this context. The unfortunate downside is that calling wasm
on macOS now involves taking a global lock and modifying a global hash
map twice-per-call. I'm not entirely sure how to get out of this cost
for now, but hopefully for any embeddings on macOS it's not the end of
the world.

Closes bytecodealliance#2456
bnjbvr pushed a commit to bnjbvr/wasmtime that referenced this issue Mar 17, 2021
This commit moves macOS to using mach ports instead of signals for
handling traps. The motivation for this is listed in bytecodealliance#2456, namely that
once mach ports are used in a process that means traditional UNIX signal
handlers won't get used. This means that if Wasmtime is integrated with
Breakpad, for example, then Wasmtime's trap handler never fires and
traps don't work.

The `traphandlers` module is refactored as part of this commit to split
the platform-specific bits into their own files (it was growing quite a
lot for one inline `cfg_if!`). The `unix.rs` and `windows.rs` files
remain the same as they were before with a few minor tweaks for some
refactored interfaces. The `macos.rs` file is brand new and lifts almost
its entire implementation from SpiderMonkey, adapted for Wasmtime
though.

The main gotcha with mach ports is that a separate thread is what
services the exception. Some unsafe magic allows this separate thread to
read non-`Send` and temporary state from other threads, but is hoped to
be safe in this context. The unfortunate downside is that calling wasm
on macOS now involves taking a global lock and modifying a global hash
map twice-per-call. I'm not entirely sure how to get out of this cost
for now, but hopefully for any embeddings on macOS it's not the end of
the world.

Closes bytecodealliance#2456
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 a pull request may close this issue.

5 participants