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

[EH] Make abort() work with Wasm EH #16910

Merged
merged 7 commits into from
May 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions emcc.py
Original file line number Diff line number Diff line change
Expand Up @@ -2566,6 +2566,9 @@ def get_full_import_name(name):
if settings.SUPPORT_LONGJMP == 'wasm':
settings.DEFAULT_LIBRARY_FUNCS_TO_INCLUDE.append('__c_longjmp')

if settings.EXCEPTION_HANDLING:
settings.REQUIRED_EXPORTS += ['__trap']

return target, wasm_target


Expand Down
11 changes: 10 additions & 1 deletion src/preamble.js
Original file line number Diff line number Diff line change
Expand Up @@ -605,12 +605,20 @@ function abort(what) {
// Use a wasm runtime error, because a JS error might be seen as a foreign
// exception, which means we'd run destructors on it. We need the error to
// simply make the program stop.
// FIXME This approach does not work in Wasm EH because it currently does not assume
// all RuntimeErrors are from traps; it decides whether a RuntimeError is from
// a trap or not based on a hidden field within the object. So at the moment
// we don't have a way of throwing a wasm trap from JS. TODO Make a JS API that
// allows this in the wasm spec.

// Suppress closure compiler warning here. Closure compiler's builtin extern
// defintion for WebAssembly.RuntimeError claims it takes no arguments even
// though it can.
// TODO(https://github.com/google/closure-compiler/pull/3913): Remove if/when upstream closure gets fixed.

#if EXCEPTION_HANDLING == 1
// See above, in the meantime, we resort to wasm code for trapping.
___trap();
#else
/** @suppress {checkTypes} */
var e = new WebAssembly.RuntimeError(what);

Expand All @@ -621,6 +629,7 @@ function abort(what) {
// in code paths apart from instantiation where an exception is expected
// to be thrown when abort is called.
throw e;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it might make sense to delete this path and just use __trap in all cases.. but I guess that can be a followup maybe?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I didn't do that in this PR because

  1. This changes the error message slightly for existing users, which I don't think is a big deal, but just to be conservative
  2. I eventually want to revive this, by making JS API that can create traps; so I was somewhat hesitant to delete this part.

I don't think either of these is an important enough though. WDYT?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm OK with landing this now and then looking at possible simplification later.

I think I'm currently a fan of (in release builds) avoiding the JS import of abort completely.. and keeping this fulling native where possible.

#endif
}

// {{MEM_INITIALIZER}}
Expand Down
3 changes: 3 additions & 0 deletions system/lib/compiler-rt/__trap.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
void __trap() {
__builtin_trap();
}
18 changes: 18 additions & 0 deletions tests/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -1639,6 +1639,24 @@ class Polymorphic {virtual void member(){}};
}
''', 'exception caught: std::bad_typeid')

@with_both_eh_sjlj
def test_abort_no_dtors(self):
# abort() should not run destructors
out = self.do_run(r'''
#include <stdlib.h>
#include <iostream>

struct Foo {
~Foo() { std::cout << "Destructing Foo" << std::endl; }
};

int main() {
Foo f;
abort();
}
''', assert_returncode=NON_ZERO)
self.assertNotContained('Destructing Foo', out)

def test_iostream_ctors(self):
# iostream stuff must be globally constructed before user global
# constructors, so iostream works in global constructors
Expand Down
3 changes: 2 additions & 1 deletion tools/system_libs.py
Original file line number Diff line number Diff line change
Expand Up @@ -708,7 +708,8 @@ class libcompiler_rt(MTLibrary, SjLjLibrary):
'stack_ops.S',
'stack_limits.S',
'emscripten_setjmp.c',
'emscripten_exception_builtins.c'
'emscripten_exception_builtins.c',
'__trap.c',
])


Expand Down