From de99f1e3cdabf335c77db19e19c493f5034bc93c Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Fri, 31 May 2024 05:55:37 +0000 Subject: [PATCH 1/3] [EH] Add __cxa_init_primary_exception to cxa_noexception.cpp tl;dr: This adds `__cxa_init_primary_exception` to `cxa_noexception.cpp`. Other targets build `cxa_exception.cpp` in `-fignore-exceptions` mode and build `cxa_noexception.cpp` only in `-fno-exeptions` mode. But we build `cxa_noexception.cpp` in `-fignore-exceptions` mode, which means it needs the definition for `__cxa_init_primary_exception` for linking to succeed when no EH argument is given (which means `-fignore-exceptions`). --- Long version (Feel free to skip): Background: After #21638, `__cxa_init_primary_exception` was added in libcxxabi: https://github.com/emscripten-core/emscripten/blob/28c10a1c5e2862edadd68ab627478204ae96d134/system/lib/libcxxabi/src/cxa_exception.cpp#L209-L226 https://github.com/emscripten-core/emscripten/blob/28c10a1c5e2862edadd68ab627478204ae96d134/system/lib/libcxxabi/src/cxa_exception_emscripten.cpp#L155-L162 Currently the files containing `__cxa_init_primary_exception`, `cxa_exception.cpp` and `cxa_exception_emscripten.cpp`, are only compiled when any of the EH mode is specified. `cxa_exception.cpp` is compiled when Wasm EH is selected, and `cxa_exception_emscripten.cpp` is compiled when Emscripten EH is selected: https://github.com/emscripten-core/emscripten/blob/28c10a1c5e2862edadd68ab627478204ae96d134/tools/system_libs.py#L1599-L1608 and this function is called from `make_exception_ptr` in libcxx: https://github.com/emscripten-core/emscripten/blob/28c10a1c5e2862edadd68ab627478204ae96d134/system/lib/libcxx/include/__exception/exception_ptr.h#L87-L99 And `make_exception_ptr` is called from `std::promise`'s destructor: https://github.com/emscripten-core/emscripten/blob/28c10a1c5e2862edadd68ab627478204ae96d134/system/lib/libcxx/include/future#L1161-L1168 --- Bug: Currently any program that calls `std::promise`'s destructor without specifying any exception-related arguments fails, saying `undefined symbol: __cxa_init_primary_exception`. Not specifying any exception arguments, meaning not specifying any among `-fno-exceptions`, `-fwasm-exceptions`, `-fexceptions`, or `-sDISABLE_EXCEPTION_CATCHING=0`, defaults to `-fignore-exceptions`, which allows throwing but not catching. --- Analysis: The callsite of `__cxa_init_primary_exception` in `make_exception_ptr` is guarded with `#ifndef _LIBCPP_HAS_NO_EXCEPTIONS`, so it seems it is supposed to be included only when exceptions are enabled. This `_LIBCPP_HAS_NO_EXCEPTIONS` is defined when `__cpp_exceptions` is defined: https://github.com/emscripten-core/emscripten/blob/28c10a1c5e2862edadd68ab627478204ae96d134/system/lib/libcxx/include/__config#L644-L646 And that `__cpp_exceptions` is defined in clang, when `-fcxx-exceptions` is given: https://github.com/llvm/llvm-project/blob/be566d2eacdaed972b90d2eeb1e66d732c9fe7c1/clang/lib/Frontend/InitPreprocessor.cpp#L638-L639 And that `-fcxx-exceptions` can be specified in command line, but it is also programmatically added here: https://github.com/llvm/llvm-project/blob/be566d2eacdaed972b90d2eeb1e66d732c9fe7c1/clang/lib/Driver/ToolChains/Clang.cpp#L371-L388 You can see it is added by default unless the arch is XCore or PS4/PS5, which means for Wasm it has been always added so far, unless `-fno-exceptions` is explicitly specified. So I tried checking the arguments there in Clang and adding `-fcxx-exceptions` only if either of `-fwasm-exceptions` or `-enable-emscripten-cxx-exceptions` is given. But this fails when none of EH is selected (which means `-fignore-exceptions`), because if `-fcxx-exceptions` is not added, we can't use keywords like `throw`. So basically the problem is, other targets build `cxa_exception.cpp` in `-fignore-exceptions` mode and build `cxa_noexception.cpp` only in `-fno-exeptions` mode. But we build `cxa_noexception.cpp` in `-fignore-exceptions` mode, which means it needs the definition for `__cxa_init_primary_exception` for linking to succeed, because `_LIBCPP_HAS_NO_EXCEPTIONS` cannot be defined in `-fignore-exceptions` (because we couldn't disable `-fcxx-exceptions` above in Clang to use `throw`) So this adds `__cxa_init_primary_exception` to `cxa_noexception.cpp`. --- system/lib/libcxxabi/src/cxa_noexception.cpp | 8 ++++++++ test/test_other.py | 20 ++++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/system/lib/libcxxabi/src/cxa_noexception.cpp b/system/lib/libcxxabi/src/cxa_noexception.cpp index 47c0db67c0fe9..b70468347045e 100644 --- a/system/lib/libcxxabi/src/cxa_noexception.cpp +++ b/system/lib/libcxxabi/src/cxa_noexception.cpp @@ -74,6 +74,14 @@ void __cxa_free_exception(void *thrown_object) throw() { ((char *)cxa_exception_from_thrown_object(thrown_object)); free((void *)raw_buffer); } + +__cxa_exception* +__cxa_init_primary_exception(void* object, + std::type_info* tinfo, + void*(_LIBCXXABI_DTOR_FUNC* dest)(void*)) throw() { + __cxa_exception* exception_header = cxa_exception_from_thrown_object(object); + return exception_header; +} #endif } // extern "C" diff --git a/test/test_other.py b/test/test_other.py index acd408f079f0b..44d4c7fbe3915 100644 --- a/test/test_other.py +++ b/test/test_other.py @@ -14859,3 +14859,23 @@ def test_mimalloc_headers(self): def test_SUPPORT_BIG_ENDIAN(self): # Just a simple build-only test for now self.run_process([EMCC, '-sSUPPORT_BIG_ENDIAN', test_file('hello_world.c')]) + + @parameterized({ + 'noexcept': ['-fno-exceptions'], + 'default': [], + 'except': ['-sDISABLE_EXCEPTION_CATCHING=0'], + 'except_wasm': ['-fwasm-exceptions'], + 'except_wasm_exnref': ['-fwasm-exceptions', '-sWASM_EXNREF'] + }) + def test_std_promise(self, *args): + # Regression test for the bug where std::promise's destructor caused a link + # error with __cxa_init_primary_exception when no exception argument is + # given (which defaults to -fignore-exceptions) + create_file('src.cpp', r''' + #include + int main() { + std::promise p; + return 0; + } + ''') + self.run_process([EMXX, 'src.cpp', '-pthread'] + list(args)) From ca096fb4d70f54da0f562cee10c21fdcf2c0d1b2 Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Fri, 31 May 2024 08:29:04 +0000 Subject: [PATCH 2/3] test name change --- test/test_other.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_other.py b/test/test_other.py index 75d43e50193cf..ffdbb7eafd395 100644 --- a/test/test_other.py +++ b/test/test_other.py @@ -14899,7 +14899,7 @@ def test_SUPPORT_BIG_ENDIAN(self): 'except_wasm': ['-fwasm-exceptions'], 'except_wasm_exnref': ['-fwasm-exceptions', '-sWASM_EXNREF'] }) - def test_std_promise(self, *args): + def test_std_promise_link(self, *args): # Regression test for the bug where std::promise's destructor caused a link # error with __cxa_init_primary_exception when no exception argument is # given (which defaults to -fignore-exceptions) From 350d7d164a6b2340c2ffcdddab3c96d54db6c603 Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Fri, 31 May 2024 08:30:05 +0000 Subject: [PATCH 3/3] comment fix --- test/test_other.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test_other.py b/test/test_other.py index ffdbb7eafd395..937f99d561afd 100644 --- a/test/test_other.py +++ b/test/test_other.py @@ -14900,8 +14900,8 @@ def test_SUPPORT_BIG_ENDIAN(self): 'except_wasm_exnref': ['-fwasm-exceptions', '-sWASM_EXNREF'] }) def test_std_promise_link(self, *args): - # Regression test for the bug where std::promise's destructor caused a link - # error with __cxa_init_primary_exception when no exception argument is + # Regression test for a bug where std::promise's destructor caused a link + # error with __cxa_init_primary_exception when no exception argument was # given (which defaults to -fignore-exceptions) create_file('src.cpp', r''' #include