Skip to content

Commit

Permalink
[EH] Make lsan/asan work with Wasm EH (emscripten-core#23552)
Browse files Browse the repository at this point in the history
`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 emscripten-core#21124.
  • Loading branch information
aheejin authored Jan 31, 2025
1 parent 44fd294 commit d50043f
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 14 deletions.
4 changes: 2 additions & 2 deletions system/lib/libcxxabi/src/cxa_exception.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
15 changes: 15 additions & 0 deletions system/lib/libcxxabi/src/cxa_exception_js_utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -78,6 +86,13 @@ void __get_exception_message(void* thrown_object, char** type, char** message) {
static_cast<const __shim_type_info*>(&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<const std::exception*>(thrown_object)->what();
*message = (char*)malloc(strlen(what) + 1);
Expand Down
6 changes: 0 additions & 6 deletions test/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -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':
Expand Down Expand Up @@ -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()
Expand Down
6 changes: 0 additions & 6 deletions test/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit d50043f

Please sign in to comment.