From f650912c46a89c1e05d3eebdb65fb87ecc59ed32 Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Fri, 6 May 2022 16:09:48 -0700 Subject: [PATCH 1/6] Make `do_run` tests functions consistent - `do_run` also sets `expected_output=None`, like `do_runf`. - `do_run`, `do_run_from_file`, and `do_run_in_out_file_test` now return the stdout+stderr output, like `do_runf`. --- tests/common.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/common.py b/tests/common.py index f4dfcb71eb4a..e8910f41a04e 100644 --- a/tests/common.py +++ b/tests/common.py @@ -1100,7 +1100,7 @@ def filtered_js_engines(self, js_engines=None): banned = [b[0] for b in self.banned_js_engines if b] return [engine for engine in js_engines if engine and engine[0] not in banned] - def do_run(self, src, expected_output, force_c=False, **kwargs): + def do_run(self, src, expected_output=None, force_c=False, **kwargs): if 'no_build' in kwargs: filename = src else: @@ -1109,21 +1109,21 @@ def do_run(self, src, expected_output, force_c=False, **kwargs): else: filename = 'src.cpp' write_file(filename, src) - self._build_and_run(filename, expected_output, **kwargs) + return self._build_and_run(filename, expected_output, **kwargs) def do_runf(self, filename, expected_output=None, **kwargs): return self._build_and_run(filename, expected_output, **kwargs) ## Just like `do_run` but with filename of expected output def do_run_from_file(self, filename, expected_output_filename, **kwargs): - self._build_and_run(filename, read_file(expected_output_filename), **kwargs) + return self._build_and_run(filename, read_file(expected_output_filename), **kwargs) def do_run_in_out_file_test(self, *path, **kwargs): srcfile = test_file(*path) out_suffix = kwargs.pop('out_suffix', '') outfile = shared.unsuffixed(srcfile) + out_suffix + '.out' expected = read_file(outfile) - self._build_and_run(srcfile, expected, **kwargs) + return self._build_and_run(srcfile, expected, **kwargs) ## Does a complete test - builds, runs, checks output, etc. def _build_and_run(self, filename, expected_output, args=[], output_nicerizer=None, From a6145eb195a233fcda170d2403460d4878fb9ae8 Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Fri, 6 May 2022 00:34:18 -0700 Subject: [PATCH 2/6] [EH] Make abort() work with Wasm EH We used to implement `abort()` with throwing a JS exception. #9730 changed it to `RuntimeError`, because it is the class a trap becomes once it hits a JS frame. But this turned out to be not working, because Wasm EH currently does not assume all `RuntimeError`s are from traps; it decides whether a `RuntimeError` is from a trap or not based on a hidden field within the object. Relevant past discussion: https://github.com/WebAssembly/exception-handling/issues/89#issuecomment-559533953 So at the moment we don't have a way of throwing a trap from JS, which is inconvenient. I think we eventually want to make JS API for this, but for the moment, this PR resorts to a wasm function that traps. This at least makes calling `std::terminate` work. So far calling it exhausted the call stack and aborted. Fixes #16407. --- emcc.py | 3 +++ src/preamble.js | 11 ++++++++++- system/lib/compiler-rt/exception_builtins.S | 6 ++++++ tests/test_core.py | 14 ++++++++++++++ tools/system_libs.py | 3 ++- 5 files changed, 35 insertions(+), 2 deletions(-) create mode 100644 system/lib/compiler-rt/exception_builtins.S diff --git a/emcc.py b/emcc.py index 37a8a25ce588..87a8c3879e91 100755 --- a/emcc.py +++ b/emcc.py @@ -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 diff --git a/src/preamble.js b/src/preamble.js index 2090af133dbb..2b223b014739 100644 --- a/src/preamble.js +++ b/src/preamble.js @@ -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 is not working now, because Wasm EH 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 trap from JS. TODO Make a JS API that + // allows this. // 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 + // In the meantime, we resort to wasm code for trapping. + ___trap(); +#else /** @suppress {checkTypes} */ var e = new WebAssembly.RuntimeError(what); @@ -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; +#endif } // {{MEM_INITIALIZER}} diff --git a/system/lib/compiler-rt/exception_builtins.S b/system/lib/compiler-rt/exception_builtins.S new file mode 100644 index 000000000000..58fb769a4da0 --- /dev/null +++ b/system/lib/compiler-rt/exception_builtins.S @@ -0,0 +1,6 @@ +.globl __trap + +__trap: + .functype __trap() -> () + unreachable + end_function diff --git a/tests/test_core.py b/tests/test_core.py index 2a03fac6f32d..5f06ab47787c 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -1639,6 +1639,20 @@ class Polymorphic {virtual void member(){}}; } ''', 'exception caught: std::bad_typeid') + @with_both_eh_sjlj + def test_terminate_abort(self): + # std::terminate eventually calls abort(). We used ti implement abort() with + # throwing a JS exception, but this can be again caught by std::terminate's + # cleanup and cause an infinite loop. When Wasm EH is enabled, abort() is + # implemented by a trap now. + err = self.do_run(r''' +#include +int main() { + std::terminate(); +} +''', assert_returncode=NON_ZERO) + self.assertNotContained('Maximum call stack size exceeded', err) + def test_iostream_ctors(self): # iostream stuff must be globally constructed before user global # constructors, so iostream works in global constructors diff --git a/tools/system_libs.py b/tools/system_libs.py index 0eadc8ad4143..ecb1350521db 100644 --- a/tools/system_libs.py +++ b/tools/system_libs.py @@ -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', + 'exception_builtins.S' ]) From dd74bc5b908daf16916f5ab53aaa80f818d365d9 Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Fri, 6 May 2022 19:11:38 -0700 Subject: [PATCH 3/6] Implement `__trap` in C --- system/lib/compiler-rt/exception_builtins.S | 6 ------ system/lib/compiler-rt/exception_builtins.c | 3 +++ tools/system_libs.py | 2 +- 3 files changed, 4 insertions(+), 7 deletions(-) delete mode 100644 system/lib/compiler-rt/exception_builtins.S create mode 100644 system/lib/compiler-rt/exception_builtins.c diff --git a/system/lib/compiler-rt/exception_builtins.S b/system/lib/compiler-rt/exception_builtins.S deleted file mode 100644 index 58fb769a4da0..000000000000 --- a/system/lib/compiler-rt/exception_builtins.S +++ /dev/null @@ -1,6 +0,0 @@ -.globl __trap - -__trap: - .functype __trap() -> () - unreachable - end_function diff --git a/system/lib/compiler-rt/exception_builtins.c b/system/lib/compiler-rt/exception_builtins.c new file mode 100644 index 000000000000..8c714078ae91 --- /dev/null +++ b/system/lib/compiler-rt/exception_builtins.c @@ -0,0 +1,3 @@ +void __trap() { + __builtin_trap(); +} diff --git a/tools/system_libs.py b/tools/system_libs.py index c0d10791d481..ccf0ff98e778 100644 --- a/tools/system_libs.py +++ b/tools/system_libs.py @@ -709,7 +709,7 @@ class libcompiler_rt(MTLibrary, SjLjLibrary): 'stack_limits.S', 'emscripten_setjmp.c', 'emscripten_exception_builtins.c', - 'exception_builtins.S' + 'exception_builtins.c' ]) From f4e389a4ef2a4f59db632402e7688e978ab4625b Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Mon, 9 May 2022 17:32:37 -0700 Subject: [PATCH 4/6] Apply suggestions from code review Co-authored-by: Alon Zakai --- src/preamble.js | 8 ++++---- tests/test_core.py | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/preamble.js b/src/preamble.js index 2b223b014739..c4472becda99 100644 --- a/src/preamble.js +++ b/src/preamble.js @@ -605,18 +605,18 @@ 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 is not working now, because Wasm EH currently does not assume + // 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 trap from JS. TODO Make a JS API that - // allows this. + // 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 - // In the meantime, we resort to wasm code for trapping. + // See above, in the meantime, we resort to wasm code for trapping. ___trap(); #else /** @suppress {checkTypes} */ diff --git a/tests/test_core.py b/tests/test_core.py index 5f06ab47787c..75da33efff05 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -1641,7 +1641,7 @@ class Polymorphic {virtual void member(){}}; @with_both_eh_sjlj def test_terminate_abort(self): - # std::terminate eventually calls abort(). We used ti implement abort() with + # std::terminate eventually calls abort(). We used to implement abort() with # throwing a JS exception, but this can be again caught by std::terminate's # cleanup and cause an infinite loop. When Wasm EH is enabled, abort() is # implemented by a trap now. From b6fc45f4232ad1477684827af9370c36685b58d4 Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Mon, 9 May 2022 18:59:58 -0700 Subject: [PATCH 5/6] exception_builtins.c -> __trap.c --- system/lib/compiler-rt/{exception_builtins.c => __trap.c} | 0 tools/system_libs.py | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename system/lib/compiler-rt/{exception_builtins.c => __trap.c} (100%) diff --git a/system/lib/compiler-rt/exception_builtins.c b/system/lib/compiler-rt/__trap.c similarity index 100% rename from system/lib/compiler-rt/exception_builtins.c rename to system/lib/compiler-rt/__trap.c diff --git a/tools/system_libs.py b/tools/system_libs.py index ccf0ff98e778..4b51d9fdf0cd 100644 --- a/tools/system_libs.py +++ b/tools/system_libs.py @@ -709,7 +709,7 @@ class libcompiler_rt(MTLibrary, SjLjLibrary): 'stack_limits.S', 'emscripten_setjmp.c', 'emscripten_exception_builtins.c', - 'exception_builtins.c' + '__trap.c' ]) From 9ee27ca043534ed147f1bfd8f0b46a8dec945dce Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Tue, 10 May 2022 15:21:50 -0700 Subject: [PATCH 6/6] Address comments --- tests/test_core.py | 22 +++++++++++++--------- tools/system_libs.py | 2 +- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/tests/test_core.py b/tests/test_core.py index 75da33efff05..a8d0bf337a6f 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -1640,18 +1640,22 @@ class Polymorphic {virtual void member(){}}; ''', 'exception caught: std::bad_typeid') @with_both_eh_sjlj - def test_terminate_abort(self): - # std::terminate eventually calls abort(). We used to implement abort() with - # throwing a JS exception, but this can be again caught by std::terminate's - # cleanup and cause an infinite loop. When Wasm EH is enabled, abort() is - # implemented by a trap now. - err = self.do_run(r''' -#include + def test_abort_no_dtors(self): + # abort() should not run destructors + out = self.do_run(r''' +#include +#include + +struct Foo { + ~Foo() { std::cout << "Destructing Foo" << std::endl; } +}; + int main() { - std::terminate(); + Foo f; + abort(); } ''', assert_returncode=NON_ZERO) - self.assertNotContained('Maximum call stack size exceeded', err) + self.assertNotContained('Destructing Foo', out) def test_iostream_ctors(self): # iostream stuff must be globally constructed before user global diff --git a/tools/system_libs.py b/tools/system_libs.py index 4b51d9fdf0cd..dd0d64b45e1d 100644 --- a/tools/system_libs.py +++ b/tools/system_libs.py @@ -709,7 +709,7 @@ class libcompiler_rt(MTLibrary, SjLjLibrary): 'stack_limits.S', 'emscripten_setjmp.c', 'emscripten_exception_builtins.c', - '__trap.c' + '__trap.c', ])