From a208c42977adda96ac76f08352ece90d72dc897e Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Fri, 29 Sep 2023 16:45:01 -0700 Subject: [PATCH 1/4] [EH] Make Wasm EH rethrow print stack traces In Wasm EH, even if we set `-sASSERTION` or `-sEXCEPTION_STACK_TRACES`, if we rethrow an exception in the code, we lose the effect of that option because previously we called `__throw_exception_with_stack_trace` only in `__cxa_throw`: https://github.com/emscripten-core/emscripten/blob/9ce7020632aa6f7578c6e40e197b800a4dd7073f/system/lib/libcxxabi/src/cxa_exception.cpp#L294-L296 This adds the same mechanism to `__cxa_rethrow` (which is used for C++ `throw;`) and `__cxa_rethrow_primary_exception` (which is used for `std::rethrow_exception`). This does not solve the problem of losing stack traces _before_ the rethrowing. libc++abi's `__cxa_rethrow` and `__cxa_rethrow_primary_exception` are implemented as throwing a pointer in the same way we first throw it and they are not aware of any metadata. This happens even in the native platform with GDB; GDB's backtrace only shows stack traces after rethrowing. We may try to fix this later by passing `exnref` (https://github.com/WebAssembly/exception-handling/issues/281) to the library, but this is likely to take some time. Partially fixes #20301. --- system/lib/libcxxabi/src/cxa_exception.cpp | 16 ++++++ test/test_other.py | 58 ++++++++++++++++++++++ 2 files changed, 74 insertions(+) diff --git a/system/lib/libcxxabi/src/cxa_exception.cpp b/system/lib/libcxxabi/src/cxa_exception.cpp index 975c5bbbe9de..65f3369e6424 100644 --- a/system/lib/libcxxabi/src/cxa_exception.cpp +++ b/system/lib/libcxxabi/src/cxa_exception.cpp @@ -644,6 +644,14 @@ void __cxa_rethrow() { } #ifdef __USING_SJLJ_EXCEPTIONS__ _Unwind_SjLj_RaiseException(&exception_header->unwindHeader); +#elif __USING_WASM_EXCEPTIONS__ +#ifdef NDEBUG + _Unwind_RaiseException(&exception_header->unwindHeader); +#else + // In debug mode, call a JS library function to use WebAssembly.Exception JS + // API, which enables us to include stack traces + __throw_exception_with_stack_trace(&exception_header->unwindHeader); +#endif #else _Unwind_RaiseException(&exception_header->unwindHeader); #endif @@ -769,6 +777,14 @@ __cxa_rethrow_primary_exception(void* thrown_object) dep_exception_header->unwindHeader.exception_cleanup = dependent_exception_cleanup; #ifdef __USING_SJLJ_EXCEPTIONS__ _Unwind_SjLj_RaiseException(&dep_exception_header->unwindHeader); +#elif __USING_WASM_EXCEPTIONS__ +#ifdef NDEBUG + _Unwind_RaiseException(&exception_header->unwindHeader); +#else + // In debug mode, call a JS library function to use WebAssembly.Exception JS + // API, which enables us to include stack traces + __throw_exception_with_stack_trace(&exception_header->unwindHeader); +#endif #else _Unwind_RaiseException(&dep_exception_header->unwindHeader); #endif diff --git a/test/test_other.py b/test/test_other.py index ef6abe3c1e12..4eef40ac8f3f 100644 --- a/test/test_other.py +++ b/test/test_other.py @@ -8593,6 +8593,64 @@ def test_exceptions_stack_trace_and_message(self, wasm_eh): for check in stack_trace_checks: self.assertFalse(re.search(check, err), 'Expected regex "%s" to not match on:\n%s' % (check, err)) + # Rethrowing exception currently loses the stack trace before the + # rethrowing due to how rethrowing is implemented. So in the examples below + # we don't print 'bar' at the moment. + # TODO Make rethrow preserve stack traces before rethrowing? + rethrow_src1 = r''' + #include + + void bar() { + throw std::runtime_error("my message"); + } + void foo() { + try { + bar(); + } catch (...) { + throw; // rethrowing by throw; + } + } + int main() { + foo(); + return 0; + } + ''' + rethrow_src2 = r''' + #include + + void bar() { + throw std::runtime_error("my message"); + } + void foo() { + try { + bar(); + } catch (...) { + auto e = std::current_exception(); + std::rethrow_exception(e); // rethrowing by std::rethrow_exception + } + } + int main() { + foo(); + return 0; + } + ''' + rethrow_stack_trace_checks = [ + 'std::runtime_error[:,][ ]?my message', # 'std::runtime_error: my message' for Emscripten EH + 'at ((src.wasm.)?_?__cxa_rethrow|___resumeException)', # '___resumeException' (JS symbol) for Emscripten EH + 'at (src.wasm.)?foo', + 'at (src.wasm.)?main'] + + self.set_setting('ASSERTIONS', 1) + self.clear_setting('EXCEPTION_STACK_TRACES') + err = self.do_run(rethrow_src1, emcc_args=emcc_args, assert_all=True, + assert_returncode=NON_ZERO, + expected_output=rethrow_stack_trace_checks, regex=True) + self.assertNotContained('bar', err) + err = self.do_run(rethrow_src2, emcc_args=emcc_args, assert_all=True, + assert_returncode=NON_ZERO, + expected_output=rethrow_stack_trace_checks, regex=True) + self.assertNotContained('bar', err) + @requires_node def test_jsrun(self): print(config.NODE_JS) From a7c5fe55bdc2432eddd28f0f64033ff80fd2ae83 Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Mon, 2 Oct 2023 14:54:07 -0700 Subject: [PATCH 2/4] Address comments --- system/lib/libcxxabi/src/cxa_exception.cpp | 9 ++++---- test/test_other.py | 24 ++++++++++++++++++---- 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/system/lib/libcxxabi/src/cxa_exception.cpp b/system/lib/libcxxabi/src/cxa_exception.cpp index 65f3369e6424..0033a2e7d43a 100644 --- a/system/lib/libcxxabi/src/cxa_exception.cpp +++ b/system/lib/libcxxabi/src/cxa_exception.cpp @@ -779,11 +779,12 @@ __cxa_rethrow_primary_exception(void* thrown_object) _Unwind_SjLj_RaiseException(&dep_exception_header->unwindHeader); #elif __USING_WASM_EXCEPTIONS__ #ifdef NDEBUG - _Unwind_RaiseException(&exception_header->unwindHeader); + _Unwind_RaiseException(&exception_header->unwindHeader); #else - // In debug mode, call a JS library function to use WebAssembly.Exception JS - // API, which enables us to include stack traces - __throw_exception_with_stack_trace(&exception_header->unwindHeader); + // In debug mode, call a JS library function to use + // WebAssembly.Exception JS API, which enables us to include stack + // traces + __throw_exception_with_stack_trace(&exception_header->unwindHeader); #endif #else _Unwind_RaiseException(&dep_exception_header->unwindHeader); diff --git a/test/test_other.py b/test/test_other.py index 4eef40ac8f3f..bbd0becdc0ed 100644 --- a/test/test_other.py +++ b/test/test_other.py @@ -8593,9 +8593,26 @@ def test_exceptions_stack_trace_and_message(self, wasm_eh): for check in stack_trace_checks: self.assertFalse(re.search(check, err), 'Expected regex "%s" to not match on:\n%s' % (check, err)) - # Rethrowing exception currently loses the stack trace before the - # rethrowing due to how rethrowing is implemented. So in the examples below - # we don't print 'bar' at the moment. + @parameterized({ + '': (False,), + 'wasm': (True,), + }) + def test_exceptions_rethrow_stack_trace_and_message(self, wasm_eh): + emcc_args = ['-g'] + if wasm_eh: + # FIXME Node v18.13 (LTS as of Jan 2023) has not yet implemented the new + # optional 'traceStack' option in WebAssembly.Exception constructor + # (https://developer.mozilla.org/en-US/docs/WebAssembly/JavaScript_interface/Exception/Exception) + # and embeds stack traces unconditionally. Change this back to + # self.require_wasm_eh() if this issue is fixed later. + self.require_v8() + emcc_args += ['-fwasm-exceptions'] + else: + emcc_args += ['-fexceptions'] + + # Rethrowing exception currently loses the stack trace before the rethrowing + # due to how rethrowing is implemented. So in the examples below we don't + # print 'bar' at the moment. # TODO Make rethrow preserve stack traces before rethrowing? rethrow_src1 = r''' #include @@ -8641,7 +8658,6 @@ def test_exceptions_stack_trace_and_message(self, wasm_eh): 'at (src.wasm.)?main'] self.set_setting('ASSERTIONS', 1) - self.clear_setting('EXCEPTION_STACK_TRACES') err = self.do_run(rethrow_src1, emcc_args=emcc_args, assert_all=True, assert_returncode=NON_ZERO, expected_output=rethrow_stack_trace_checks, regex=True) From d3f7e5b2300e5fdba19e1c492bf4c37528acd9c0 Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Mon, 2 Oct 2023 15:44:37 -0700 Subject: [PATCH 3/4] Simplify if --- system/lib/libcxxabi/src/cxa_exception.cpp | 20 ++++---------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/system/lib/libcxxabi/src/cxa_exception.cpp b/system/lib/libcxxabi/src/cxa_exception.cpp index 0033a2e7d43a..b609d44a900b 100644 --- a/system/lib/libcxxabi/src/cxa_exception.cpp +++ b/system/lib/libcxxabi/src/cxa_exception.cpp @@ -287,14 +287,10 @@ __cxa_throw(void *thrown_object, std::type_info *tinfo, void (_LIBCXXABI_DTOR_FU #ifdef __USING_SJLJ_EXCEPTIONS__ _Unwind_SjLj_RaiseException(&exception_header->unwindHeader); -#elif __USING_WASM_EXCEPTIONS__ -#ifdef NDEBUG - _Unwind_RaiseException(&exception_header->unwindHeader); -#else +#elif defined(__USING_WASM_EXCEPTIONS__) && !defined(NDEBUG) // In debug mode, call a JS library function to use WebAssembly.Exception JS // API, which enables us to include stack traces __throw_exception_with_stack_trace(&exception_header->unwindHeader); -#endif #else _Unwind_RaiseException(&exception_header->unwindHeader); #endif @@ -644,14 +640,10 @@ void __cxa_rethrow() { } #ifdef __USING_SJLJ_EXCEPTIONS__ _Unwind_SjLj_RaiseException(&exception_header->unwindHeader); -#elif __USING_WASM_EXCEPTIONS__ -#ifdef NDEBUG - _Unwind_RaiseException(&exception_header->unwindHeader); -#else +#elif defined(__USING_WASM_EXCEPTIONS__) && !defined(NDEBUG) // In debug mode, call a JS library function to use WebAssembly.Exception JS // API, which enables us to include stack traces __throw_exception_with_stack_trace(&exception_header->unwindHeader); -#endif #else _Unwind_RaiseException(&exception_header->unwindHeader); #endif @@ -777,17 +769,13 @@ __cxa_rethrow_primary_exception(void* thrown_object) dep_exception_header->unwindHeader.exception_cleanup = dependent_exception_cleanup; #ifdef __USING_SJLJ_EXCEPTIONS__ _Unwind_SjLj_RaiseException(&dep_exception_header->unwindHeader); -#elif __USING_WASM_EXCEPTIONS__ -#ifdef NDEBUG - _Unwind_RaiseException(&exception_header->unwindHeader); -#else +#elif defined(__USING_WASM_EXCEPTIONS__) && !defined(NDEBUG) // In debug mode, call a JS library function to use // WebAssembly.Exception JS API, which enables us to include stack // traces __throw_exception_with_stack_trace(&exception_header->unwindHeader); -#endif #else - _Unwind_RaiseException(&dep_exception_header->unwindHeader); + _Unwind_RaiseException(&exception_header->unwindHeader); #endif // Some sort of unwinding error. Note that terminate is a handler. __cxa_begin_catch(&dep_exception_header->unwindHeader); From ca2da5c41f443372316891de8b09f50efa2f386b Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Mon, 2 Oct 2023 17:11:59 -0700 Subject: [PATCH 4/4] Use self.emcc_args --- test/test_other.py | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/test/test_other.py b/test/test_other.py index bbd0becdc0ed..2213231cc99b 100644 --- a/test/test_other.py +++ b/test/test_other.py @@ -8529,7 +8529,7 @@ def test_exceptions_stack_trace_and_message(self, wasm_eh): return 0; } ''' - emcc_args = ['-g'] + self.emcc_args = ['-g'] # Stack trace and message example for this example code: # exiting due to exception: [object WebAssembly.Exception],Error: std::runtime_error,my message @@ -8557,9 +8557,9 @@ def test_exceptions_stack_trace_and_message(self, wasm_eh): # self.require_wasm_eh() if this issue is fixed later. self.require_v8() - emcc_args += ['-fwasm-exceptions'] + self.emcc_args += ['-fwasm-exceptions'] else: - emcc_args += ['-fexceptions'] + self.emcc_args += ['-fexceptions'] # Stack traces are enabled when either of ASSERTIONS or # EXCEPTION_STACK_TRACES is enabled. You can't disable @@ -8568,16 +8568,14 @@ def test_exceptions_stack_trace_and_message(self, wasm_eh): # Prints stack traces self.set_setting('ASSERTIONS', 1) self.set_setting('EXCEPTION_STACK_TRACES', 1) - self.do_run(src, emcc_args=emcc_args, assert_all=True, - assert_returncode=NON_ZERO, expected_output=stack_trace_checks, - regex=True) + self.do_run(src, assert_all=True, assert_returncode=NON_ZERO, + expected_output=stack_trace_checks, regex=True) # Prints stack traces self.set_setting('ASSERTIONS', 0) self.set_setting('EXCEPTION_STACK_TRACES', 1) - self.do_run(src, emcc_args=emcc_args, assert_all=True, - assert_returncode=NON_ZERO, expected_output=stack_trace_checks, - regex=True) + self.do_run(src, assert_all=True, assert_returncode=NON_ZERO, + expected_output=stack_trace_checks, regex=True) # Not allowed self.set_setting('ASSERTIONS', 1) @@ -8589,7 +8587,7 @@ def test_exceptions_stack_trace_and_message(self, wasm_eh): # Doesn't print stack traces self.set_setting('ASSERTIONS', 0) self.set_setting('EXCEPTION_STACK_TRACES', 0) - err = self.do_run(src, emcc_args=emcc_args, assert_returncode=NON_ZERO) + err = self.do_run(src, assert_returncode=NON_ZERO) for check in stack_trace_checks: self.assertFalse(re.search(check, err), 'Expected regex "%s" to not match on:\n%s' % (check, err)) @@ -8598,7 +8596,7 @@ def test_exceptions_stack_trace_and_message(self, wasm_eh): 'wasm': (True,), }) def test_exceptions_rethrow_stack_trace_and_message(self, wasm_eh): - emcc_args = ['-g'] + self.emcc_args = ['-g'] if wasm_eh: # FIXME Node v18.13 (LTS as of Jan 2023) has not yet implemented the new # optional 'traceStack' option in WebAssembly.Exception constructor @@ -8606,9 +8604,9 @@ def test_exceptions_rethrow_stack_trace_and_message(self, wasm_eh): # and embeds stack traces unconditionally. Change this back to # self.require_wasm_eh() if this issue is fixed later. self.require_v8() - emcc_args += ['-fwasm-exceptions'] + self.emcc_args += ['-fwasm-exceptions'] else: - emcc_args += ['-fexceptions'] + self.emcc_args += ['-fexceptions'] # Rethrowing exception currently loses the stack trace before the rethrowing # due to how rethrowing is implemented. So in the examples below we don't @@ -8658,12 +8656,10 @@ def test_exceptions_rethrow_stack_trace_and_message(self, wasm_eh): 'at (src.wasm.)?main'] self.set_setting('ASSERTIONS', 1) - err = self.do_run(rethrow_src1, emcc_args=emcc_args, assert_all=True, - assert_returncode=NON_ZERO, + err = self.do_run(rethrow_src1, assert_all=True, assert_returncode=NON_ZERO, expected_output=rethrow_stack_trace_checks, regex=True) self.assertNotContained('bar', err) - err = self.do_run(rethrow_src2, emcc_args=emcc_args, assert_all=True, - assert_returncode=NON_ZERO, + err = self.do_run(rethrow_src2, assert_all=True, assert_returncode=NON_ZERO, expected_output=rethrow_stack_trace_checks, regex=True) self.assertNotContained('bar', err)