From d50043f00f9bbc12171325bb56b8b0df37cbb106 Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Thu, 30 Jan 2025 23:08:01 -0800 Subject: [PATCH] [EH] Make lsan/asan work with Wasm EH (#23552) `std::rethrow_exception` leaked memory because dependent exceptions were not recognized as such and thus not freed properly here: https://github.com/emscripten-core/emscripten/blob/44fd294e8244afa710cecb2c10c9a644a6549b8e/system/lib/libcxxabi/src/cxa_exception.cpp#L575-L583 The reason for that was, in `__cxa_rethrow_primary_exception`, we were incorrectly throwing the primary exception, not the dependent one. I think the error was introduced during copy-pasting. `isDependentException` method was copied from `cxa_exception.cpp`, because it is a static function, as in the case for other utility functions in that file. This also adds handling for dependent exceptions in `__get_exception_message`. Fixes #21124. --- system/lib/libcxxabi/src/cxa_exception.cpp | 4 ++-- .../lib/libcxxabi/src/cxa_exception_js_utils.cpp | 15 +++++++++++++++ test/common.py | 6 ------ test/test_core.py | 6 ------ 4 files changed, 17 insertions(+), 14 deletions(-) diff --git a/system/lib/libcxxabi/src/cxa_exception.cpp b/system/lib/libcxxabi/src/cxa_exception.cpp index 06dd9a8f2b375..4b82d0bf066ee 100644 --- a/system/lib/libcxxabi/src/cxa_exception.cpp +++ b/system/lib/libcxxabi/src/cxa_exception.cpp @@ -789,9 +789,9 @@ __cxa_rethrow_primary_exception(void* thrown_object) // 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); + __throw_exception_with_stack_trace(&dep_exception_header->unwindHeader); #else - _Unwind_RaiseException(&exception_header->unwindHeader); + _Unwind_RaiseException(&dep_exception_header->unwindHeader); #endif // Some sort of unwinding error. Note that terminate is a handler. __cxa_begin_catch(&dep_exception_header->unwindHeader); diff --git a/system/lib/libcxxabi/src/cxa_exception_js_utils.cpp b/system/lib/libcxxabi/src/cxa_exception_js_utils.cpp index a898a0232dc73..ce9de25eb42dd 100644 --- a/system/lib/libcxxabi/src/cxa_exception_js_utils.cpp +++ b/system/lib/libcxxabi/src/cxa_exception_js_utils.cpp @@ -38,6 +38,14 @@ static inline __cxa_exception* cxa_exception_from_unwind_exception( return cxa_exception_from_thrown_object(unwind_exception + 1); } +#ifdef __WASM_EXCEPTIONS__ +struct __cxa_dependent_exception; +uint64_t __getExceptionClass(const _Unwind_Exception* unwind_exception); +static bool isDependentException(_Unwind_Exception* unwind_exception) { + return (__getExceptionClass(unwind_exception) & 0xFF) == 0x01; +} +#endif + extern "C" { void* __thrown_object_from_unwind_exception( @@ -78,6 +86,13 @@ void __get_exception_message(void* thrown_object, char** type, char** message) { static_cast(&typeid(std::exception)); int can_catch = catch_type->can_catch(thrown_type, thrown_object); if (can_catch) { +#if __WASM_EXCEPTIONS__ + if (isDependentException(&exception_header->unwindHeader)) + thrown_object = + reinterpret_cast<__cxa_dependent_exception*>(exception_header) + ->primaryException; +#endif + const char* what = static_cast(thrown_object)->what(); *message = (char*)malloc(strlen(what) + 1); diff --git a/test/common.py b/test/common.py index da0f9e0863c1d..619d95e26a3c7 100644 --- a/test/common.py +++ b/test/common.py @@ -641,9 +641,6 @@ def metafunc(self, mode, *args, **kwargs): # Wasm EH is currently supported only in wasm backend and V8 if self.is_wasm2js(): self.skipTest('wasm2js does not support wasm EH/SjLj') - # FIXME Temporarily disabled. Enable this later when the bug is fixed. - if '-fsanitize=address' in self.emcc_args: - self.skipTest('Wasm EH does not work with asan yet') self.emcc_args.append('-fwasm-exceptions') self.set_setting('SUPPORT_LONGJMP', 'wasm') if mode == 'wasm': @@ -677,9 +674,6 @@ def metafunc(self, mode, *args, **kwargs): if mode in {'wasm', 'wasm_legacy'}: if self.is_wasm2js(): self.skipTest('wasm2js does not support wasm SjLj') - # FIXME Temporarily disabled. Enable this later when the bug is fixed. - if '-fsanitize=address' in self.emcc_args: - self.skipTest('Wasm EH does not work with asan yet') self.set_setting('SUPPORT_LONGJMP', 'wasm') if mode == 'wasm': self.require_wasm_eh() diff --git a/test/test_core.py b/test/test_core.py index 7bb70aa7e0d77..7dcd5cb82540b 100644 --- a/test/test_core.py +++ b/test/test_core.py @@ -1328,8 +1328,6 @@ def test_exceptions_refcount(self): @with_all_eh_sjlj def test_exceptions_primary(self): - if '-fsanitize=leak' in self.emcc_args and '-fwasm-exceptions' in self.emcc_args: - self.skipTest('https://github.com/emscripten-core/emscripten/issues/21124') self.do_core_test('test_exceptions_primary.cpp') @with_all_eh_sjlj @@ -1346,8 +1344,6 @@ def test_exceptions_multiple_inherit(self): @with_all_eh_sjlj def test_exceptions_multiple_inherit_rethrow(self): - if '-fsanitize=leak' in self.emcc_args and '-fwasm-exceptions' in self.emcc_args: - self.skipTest('https://github.com/emscripten-core/emscripten/issues/21124') self.do_core_test('test_exceptions_multiple_inherit_rethrow.cpp') @with_all_eh_sjlj @@ -1499,8 +1495,6 @@ def test_exceptions_longjmp2(self): @with_all_eh_sjlj def test_exceptions_longjmp3(self): - if '-fwasm-exceptions' in self.emcc_args: - self.skipTest('https://github.com/emscripten-core/emscripten/issues/17004') self.do_core_test('test_exceptions_longjmp3.cpp') @with_all_eh_sjlj