getsentry / symbolic

Stack trace symbolication library written in Rust
https://github.com/getsentry/symbolic#readme
MIT License
439 stars 76 forks source link

Support for wasm exception handling legacy proposal #850

Closed trzeciak closed 5 hours ago

trzeciak commented 1 month ago

Symbolic supports wasm files without exceptions, but there are problems with wasm files with exceptions.

As an introduction (we may not use exceptions at all xD): we can use legacy exceptions (it has been in browsers for 3 years), we can use (new/current) exceptions handling purposal (end of last year).

The problem with the current exceptions handler proposal is its still low support on the customers side. Therefore, in this issue I would like to raise the topic of support for legacy proposals.

Having a sample main file that throws an exception:

//  example_exception.cpp
#include <stdio.h>
#include <stdexcept>
int main() {
    printf("hello..\n");
    throw std::runtime_error("Kerfufle");
    printf("world!\n");
    return 0;
}

And compile it (using emscripten with support for exceptions) and try validate it (using symbolic object_debug), we got this error:

$ emcc -fwasm-exceptions -o example_exception.wasm example_exception.cpp # latest (v3.1.61)
$ ./symbolic/object_debug example_exception.wasm # latest (master)
Inspecting example_exception.wasm
File format: wasm
Objects:
 - Error: invalid wasm file
   caused by unimplemented validation of deprecated opcode (at offset 0x252)

This happens because the validator in the wasmparser library has stopped supporting legacy-exceptions (https://github.com/bytecodealliance/wasm-tools/pull/1333).

When I restore support for deprecated opcode: https://github.com/trzeciak/wasm-tools/commit/871dbcba98ad05dc3ec55d1ed91a948a2b0f9c35, it allows to symbolic work correctly:

$ ./patched/object_debug example_exception.wasm # master+revert-deprecated-try-catch-catch_all-opcodes
Inspecting example_exception.wasm
File format: wasm
Objects:
 - wasm32: 00000000-0000-0000-0000-000000000000
   code id:      -
   object kind:  library
   load address: 0x0
   symbol table: true
   debug info:   false
   unwind info:  false
   is malformed: false

If I understand correctly:

to ensure the greatest coverage for our customers, it may be a good idea to use the legacy-exceptions for today and switch to the new exceptions after some time.


The question is, could we update the symbolic code to support legacy and new exception-handling?

I did some preliminary research and it looks like the changes should be made around symbolic-debuginfo/src/wasm/parser.rs, replacing wasmparser 'validator' with 'parser'.

Do you think this makes sense, and is it worth preparing a pull-request that ensures this (although I'm not sure I can do it myself)?

loewenheim commented 4 weeks ago

A PR for this would be welcome, yes.

trzeciak commented 4 weeks ago

Okay, I'll wait for the answer to this question: https://github.com/bytecodealliance/wasm-tools/issues/1653, because restoring it in the wasmparser behind the flag seems easier to implement.

trzeciak commented 5 hours ago

It seems to be working, thanks to others for their help, done.