bytecodealliance / wasmtime

A fast and secure runtime for WebAssembly
https://wasmtime.dev/
Apache License 2.0
15.08k stars 1.26k forks source link

Can't unwind wasm frames on aarch64-darwin with system libunwind #2808

Open bnjbvr opened 3 years ago

bnjbvr commented 3 years ago

On Mac aarch64 (e.g. M1 MBP), the system's libunwind doesn't behave as on other platforms, and doesn't properly unwind wasm frames using the information we provide it (CFI etc.). The PR that introduces the switch from using signal handlers to mach ports to handle software traps has something that worked, using a custom build of libunwind and custom directives for linking against it.

Some ways to fix this:

fitzgen commented 3 years ago

Related: https://github.com/bytecodealliance/wasmtime/issues/2459

akirilov-arm commented 2 years ago

@bnjbvr @cfallin @fitzgen Is this issue still relevant? It affects the tests in tests/all/traps.rs in particular.

fitzgen commented 2 years ago

This is probably still relevant for when we emit unwind info, which is a configuration we still want to support to integrate with external tools like GDB/profilers/etc, but it is no longer an issue that is critical for correctness (e.g. reclaiming GC garbage).

bnjbvr commented 2 years ago

This is also used to compute backtraces, right? I'll note to try it again on my side.

fitzgen commented 2 years ago

Backtraces are now captured by walking frame pointers since https://github.com/bytecodealliance/wasmtime/pull/4431

bnjbvr commented 2 years ago

Very sweet! I've just retried on my mac m1, and indeed these tests now pass after uncommenting the cfg_attr (I'll submit a PR), and in our embedding I can see a full stack trace \o/ Thanks @fitzgen for working on this!

akirilov-arm commented 2 years ago

FYI recently I did a little investigation for my back-edge CFI work, and the only remaining user I could find of the backtrace crate, and by extension the system unwinder, is the wasmtime-fiber crate. In fact, that is just a test dependency, and, more precisely, only one test needs it - backtrace_traces_to_host.

Of course, there is still the issue about integrating with external tools such as debuggers.

fitzgen commented 2 years ago

the only remaining user I could find of the backtrace crate, and by extension the system unwinder, is the wasmtime-fiber crate. In fact, that is just a test dependency, and, more precisely, only one test needs it - backtrace_traces_to_host.

FWIW, I think this is 100% okay. We shouldn't be concerned about dev deps in the same way we are about regular deps.

Of course, there is still the issue about integrating with external tools such as debuggers.

We do still emit native unwind info by default, but also have a wasmtime::Config knob to turn it off should an embedder choose to.

fitzgen commented 2 years ago

Can we close this issue now? @bnjbvr I can't remember: did you submit the PR you mentioned you'd make in https://github.com/bytecodealliance/wasmtime/issues/2808#issuecomment-1212060039? I don't see any cfg_attr in tests/all/traps.rs anymore so I assume so.

bnjbvr commented 2 years ago

Yep, I did submit this PR, so we could consider this closed. I didn't link this issue in the PR because the native unwinders would still require this to work properly, but maybe that can be deferred to a future issue.