Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update libcxx and libcxxabi to LLVM 18.1.2 #21638

Merged
merged 21 commits into from
Apr 3, 2024

Conversation

aheejin
Copy link
Member

@aheejin aheejin commented Mar 28, 2024

This updates libcxx and libcxxabi to LLVM 18.1.2. Due to llvm/llvm-project#65534 we need to update both at the same time.

Things to note:

Each fix is described in its own commit, except for the fixes required to resolve conflicts when merging our changes, which I wasn't able to commit separately.

Fixes #21603.

llvm/llvm-project#65534 introduces
`__cxa_init_primary_exception` and uses this in libcxx. Like several
other methods like the below in `cxa_exception.cpp`,
https://github.com/emscripten-core/emscripten/blob/fbdd9249e939660cb0d20f00d6bc1897c2f3905e/system/lib/libcxxabi/src/cxa_exception.cpp#L264-L269
this new function takes a pointer to a destructor, and in Wasm
destructor returns a `this` pointer, which requires
`ifdef __USING_WASM_EXCEPTION__` on its signature.

And that it is also used in libcxx means we need to guard some code in
libcxx with `ifdef __USING_WASM_EXCEPTION__` for the signature
difference, and we need to build libcxx with
`-D__USING_WASM_EXCEPTIONS__` when Wasm EH is used.
LLVM 18.1.2 adds C++20 time zone support
(https://libcxx.llvm.org/DesignDocs/TimeZone.html) to libcxx, which
requires access IANA Time Zone Database. Currently it seems it only
supports Linux:
https://github.com/llvm/llvm-project/blob/26a1d6601d727a96f4301d0d8647b5a42760ae0c/libcxx/src/tz.cpp#L45-L49

So this excludes the two source files from build (which is done via
`CMakeLists.txt` in the upstream LLVM) and sets
`_LIBCPP_HAS_NO_TIME_ZONE_DATABASE` macro in `__config_site`. In future
maybe we can consider implementing this in JS.
Previously we had `_LIBCPP_AVAILABILITY_HAS_NO_VERBOSE_ABORT` but it has
changed to `_LIBCPP_AVAILABILITY_HAS_VERBOSE_ABORT` that takes a value.

We set `_LIBCPP_AVAILABILITY_HAS_NO_VERBOSE_ABORT` when we updated
libcxx to LLVM 17 due to code size increases it incurred.
LLVM 18.1.2 adds support for "Hardening Modes" to libcxx
(https://libcxx.llvm.org/Hardening.html). There are four modes: none,
fast, extensive and debug. Different hardening modes make different
trade-offs between the amount of checking and runtime performance. We
for now disable it (i.e., set it to 'none') so that we don't have any
effects on the performance. We can consider enabling it when we ever get
to enable the debug version of libcxx.
llvm/llvm-project#77883 adds a way for vendors
to override how we handle assertions. If a vendor doesn't want to
override it, `CMakeLists.txt` will copy the provided default template
assertion handler file
(https://github.com/llvm/llvm-project/blob/26a1d6601d727a96f4301d0d8647b5a42760ae0c/libcxx/vendor/llvm/default_assertion_handler.in)
to libcxx's generated include dir, which defaults to
`SYSROOT/include/c++/v1`:
https://github.com/llvm/llvm-project/blob/26a1d6601d727a96f4301d0d8647b5a42760ae0c/libcxx/CMakeLists.txt#L72-L73
https://github.com/llvm/llvm-project/blob/26a1d6601d727a96f4301d0d8647b5a42760ae0c/libcxx/CMakeLists.txt#L439
https://github.com/llvm/llvm-project/blob/26a1d6601d727a96f4301d0d8647b5a42760ae0c/libcxx/include/CMakeLists.txt#L1024

We don't use CMake, so this renames the provided
`vendor/llvm/default_assertion_handler.in` to `__assert_handler` in our
`include` directory, which will be copied to `SYSROOT/include/c++/v1`.
Copy link
Collaborator

@sbc100 sbc100 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm! Thanks for all the detailed investigation here!

In emscripten-core#20707 we decided to disable `__libcpp_verbose_abort` not to incur
the code size increase that brings (it brings in `vfprintf` and its
family). We disabled it in adding
`#define _LIBCPP_AVAILABILITY_HAS_NO_VERBOSE_ABORT` in `__config_site`.

That `_NO_` macros are gone in LLVM 18.1.2, so I changed it to
`#define _LIBCPP_AVAILABILITY_HAS_VERBOSE_ABORT 0`
emscripten-core@2bd7c67
But apparently, when including `__availability`, that setting is
overridden by this line:
https://github.com/llvm/llvm-project/blob/38f5596feda3276a8aa64fc14e074334017088ca/libcxx/include/__availability#L138
because we don't define our own vendor availability annotations
https://github.com/emscripten-core/emscripten/blob/193b2bff980529dbf5d067f01761ed8a6e41189f/system/lib/libcxx/include/__config_site#L4

I asked about this in
llvm/llvm-project#87012 and
llvm/llvm-project#71002 (comment)
(and more comments below)
but didn't get an actionable answer yet.

This disables `__libcpp_verbose_abort` in the code directly for now,
hopefully temporarily.
llvm/llvm-project#65534 added
`__cxa_init_primary_exception` function and use it in both in
libcxxabi's `__cxa_throw` and also directly in libcxx. Because we create
exceptions in JS, we need a counterpart in JS exception library too.
The previous attempt
(emscripten-core@39c27d1)
made `__cxa_init_primary_exception` return the result of
[`storeException`](https://github.com/emscripten-core/emscripten/blob/f8375b9192b8416442cf698a5a1cac9d65fd8974/src/parseTools.mjs#L587-L590)
and `__cxa_throw` will call it and throw its result.

But in wasm64 mode, `return` will be wrapped with `makeReturn64`, so
`__cxa_init_primary_exception`'s return value will be wrapped with
`BigInt`.

When `EXCEPTION_STACK_TRACES` is, this errors out because we end up
trying to wrap an instance of `CppException` with `BigInt`.

When `EXCEPTION_STACK_TRACES` is off, because `__cxa_throw` calls
`__cxa_init_primary_exception`, `__cxa_throw` ends up throwing a
`BigInt`, which errors out when trying to add `0` to it in the `catch`
clause in `invoke`:
```
function invoke_v(index) {
 var sp = stackSave();
 try {
  getWasmTableEntry(Number(index))();
 } catch (e) {
  stackRestore(sp);
  // This will error out with
  // "TypeError: Cannot mix BigInt and other types, use explicit
  //  conversions"
  if (e !== e + 0)
    throw e;
  _setThrew(1, 0);
 }
}
```

Currently `__cxa_throw` throws a number or a `CppException` (depending
on whether `EXCEPTION_STACK_TRACES` is true) and not a `BigInt`.
(Unlike `return`, `throw` does not get wrapped)

To preserve this behavior, this reverts the changes in `__cxa_throw` so
it doesn't call `__cxa_init_primary_exception`. We still need to provide
`__cxa_init_primary_exception` in JS because libcxx's
`make_exception_ptr` uses it, so make it just return the exception
pointer.
@aheejin
Copy link
Member Author

aheejin commented Mar 30, 2024

The CI seems to pass all tests except for other. test_minimal_runtime_code_size_hello_embind_val, where the binary size was increased by 3 bytes for an unrelated reason. (This PR itself also increases the size by 150 bytes, but this looks within a reasonable boundary)
I will rebase this after #21659 lands.

@aheejin aheejin marked this pull request as ready for review March 30, 2024 17:54
// Here, we throw an exception after recording a couple of values that we need to remember
// We also remember that it was the last exception thrown as we need to know that later.
__cxa_throw__deps: ['$ExceptionInfo', '$exceptionLast', '$uncaughtExceptionCount'],
__cxa_throw__deps: ['$exceptionLast', '$uncaughtExceptionCount', '__cxa_init_primary_exception'],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why remove ExceptionInfo dependency here? This function clearly directly uses ExceptionInfo.

On the other hand I don't see it using __cxa_init_primary_exception.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what's left from my previous (abandoned) approach. Will restore it.

@@ -102,9 +102,17 @@ var LibraryExceptions = {
}
},

__cxa_init_primary_exception__deps: ['$ExceptionInfo'],
__cxa_init_primary_exception: (ptr, type, destructor) => {
#if EXCEPTION_DEBUG
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function looks like it could be a native symbol?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now that this is not being used in __cxa_throw I think we can use the native version of this. But if I delete this test_dylink_exceptions_try_catch_6 errors out with

error: undefined symbol: __cxa_init_primary_exception (referenced by root reference (e.g. compiled C/C++ code))
warning: To disable errors for undefined symbols use `-sERROR_ON_UNDEFINED_SYMBOLS=0`
warning: ___cxa_init_primary_exception may need to be added to EXPORTED_FUNCTIONS if it arrives from a system library

I guess I need to add this symbol to some list of exported functions...? Can you help me where?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if you add __cxa_init_primary_exception to cxa_exception_emscripten.cpp that should do the trick. No need to export anything I would hope.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this was added in llvm/llvm-project#65534

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done: a4ec944

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sbc100 from the compile_commands.json:

{
  "directory": "/sources/ITK-5.3.0/build",
  "command": "/emsdk/upstream/emscripten/em++ -I/sources/ITK-5.3.0/Modules/ThirdParty/DoubleConversion/src -I/sources/ITK-5.3.0/build/Modules/ThirdParty/DoubleConversion/src/double-conversion -I/sources/ITK-5.3.0/build/Modules/ThirdParty/Eigen3/src -I/sources/ITK-5.3.0/build/Modules/ThirdParty/KWSys/src -I/sources/ITK-5.3.0/Modules/ThirdParty/VNL/src/vxl/v3p/netlib -I/sources/ITK-5.3.0/Modules/ThirdParty/VNL/src/vxl/vcl -I/sources/ITK-5.3.0/Modules/ThirdParty/VNL/src/vxl/core -I/sources/ITK-5.3.0/build/Modules/ThirdParty/VNL/src/vxl/v3p/netlib -I/sources/ITK-5.3.0/build/Modules/ThirdParty/VNL/src/vxl/vcl -I/sources/ITK-5.3.0/build/Modules/ThirdParty/VNL/src/vxl/core -I/sources/ITK-5.3.0/build/Modules/Core/Common -I/sources/ITK-5.3.0/Modules/Core/Common/include -I/sources/ITK-5.3.0/Modules/ThirdParty/VNL/src/vxl/core/vnl/algo -I/sources/ITK-5.3.0/Modules/ThirdParty/VNL/src/vxl/core/vnl -I/sources/ITK-5.3.0/build/Modules/ThirdParty/VNL/src/vxl/core/vnl -isystem /sources/install/Eigen3/include/eigen3 -pthread -sSHARED_MEMORY=1  -Wall -Wcast-align -Wdisabled-optimization -Wextra -Wformat=2 -Winvalid-pch -Wno-format-nonliteral -Wpointer-arith -Wshadow -Wunused -Wwrite-strings -Wno-strict-overflow -Wno-deprecated -Wno-invalid-offsetof -Wno-undefined-var-template -Woverloaded-virtual  -O3 -DNDEBUG -fPIC -std=c++14 -o Modules/Core/Common/src/CMakeFiles/ITKCommon.dir/itkPoolMultiThreader.cxx.o -c /sources/ITK-5.3.0/Modules/Core/Common/src/itkPoolMultiThreader.cxx",
  "file": "/sources/ITK-5.3.0/Modules/Core/Common/src/itkPoolMultiThreader.cxx"
}

compile_commands.json

Copy link
Member Author

@aheejin aheejin May 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sbc100

I suspect that what is happening here is that itkPoolMultiThreader.cxx.o is being built with exception handling support but that the link command not including exception handling (e.g. via -sDISABLE_EXCEPTION_CATCHING=0 or -fexceptions)? Does this sounds correct @aheejin ?

I thought so too, but the compile command @mzukovec gave didn't have any EH options, strange.

@aheejin do you why some functions like __cxa_rethrow_primary_exception have stubs in cxa_noexception.cpp but not __cxa_init_primary_exception

I guess they are the only functions called from libc++ where the code is not guarded with #ifndef _LIBCPP_HAS_NO_EXCEPTIONS. Apparently all uses of __cxa_init_primary_exceptions are guarded with #ifndef _LIBCPP_HAS_NO_EXCEPTIONS, so not sure what's causing the issue.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mzukovec Can you possibly share the itkPoolMultiThreader.cxx.o and itkPoolMultiThreader.cxx?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mzukovec Can you possibly share the itkPoolMultiThreader.cxx.o and itkPoolMultiThreader.cxx?

Sure thing.
itkPoolMultiThreader.zip

Link to the used ITK revision in case it's needed later on.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think #22027 will fix this. Thanks for the report!

Given that our JS `__cxa_throw` does not depend on
`__cxa_init_primary_exception` anymore, there's no reason this function
needs to be in JS. This moves the function to the native library. Also
this changes a couple `ifdef __USING_WASM_EXCEPTIONS__`s to
`ifdef __wasm__` because Wasm's destructor type pertains to Emscripten
EH too.
Copy link
Collaborator

@sbc100 sbc100 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@aheejin aheejin merged commit 37055c7 into emscripten-core:main Apr 3, 2024
29 checks passed
@aheejin aheejin deleted the update_libcxx branch April 3, 2024 05:43
impact-maker pushed a commit to impact-maker/emscripten that referenced this pull request Apr 11, 2024
This updates libcxx and libcxxabi to LLVM 18.1.2. Due to
llvm/llvm-project#65534 we need to update both
at the same time.

Things to note:

- Disable hardening mode:
  LLVM 18.1.2 adds support for "Hardening Modes" to libcxx
  (https://libcxx.llvm.org/Hardening.html). There are four modes: none,
  fast, extensive and debug. Different hardening modes make different
  trade-offs between the amount of checking and runtime performance. We
  for now disable it (i.e., set it to 'none') so that we don't have any
  effects on the performance. We can consider enabling it when we ever
  get to enable the debug version of libcxx.

- Disable C++20 time zone support:
  LLVM 18.1.2 adds C++20 time zone support
  (https://libcxx.llvm.org/DesignDocs/TimeZone.html) to libcxx, which
  requires access IANA Time Zone Database. Currently it seems it only
  supports Linux:
  https://github.com/llvm/llvm-project/blob/26a1d6601d727a96f4301d0d8647b5a42760ae0c/libcxx/src/tz.cpp#L45-L49
  So this excludes the two source files from build (which is done via
  `CMakeLists.txt` in the upstream LLVM) and sets
  `_LIBCPP_HAS_NO_TIME_ZONE_DATABASE` macro in `__config_site`. In
  future maybe we can consider implementing this in JS.

- `__cxa_init_primary_exception` support:
  llvm/llvm-project#65534 introduces
  `__cxa_init_primary_exception` and uses this in libcxx. Like several
  other methods like the below in `cxa_exception.cpp`,
  https://github.com/emscripten-core/emscripten/blob/fbdd9249e939660cb0d20f00d6bc1897c2f3905e/system/lib/libcxxabi/src/cxa_exception.cpp#L264-L269
  this new function takes a pointer to a destructor, and in Wasm
  destructor returns a `this` pointer, which requires `ifdef
  __USING_WASM_EXCEPTION__` on its signature. And that it is also used
  in libcxx means we need to guard some code in libcxx with `ifdef
  __USING_WASM_EXCEPTION__` for the signature difference, and we need to
  build libcxx with `-D__USING_WASM_EXCEPTIONS__` when Wasm EH is used.
  Also we add Emscripte EH's counterpart in
  `cxa_exception_emscripten.cpp` too.

- This version fixes long-running misbehaviors of `operator new`
  llvm/llvm-project#69498 seems to fix some
  long-running misbhaviors of `operator new`, which in emscripten we
  tried to fix in emscripten-core#11079 and emscripten-core#20154. So while resolving the conflicts, I
  effectively reverted emscripten-core#11079 and emscripten-core#20154 in
  `libcxx/src/stdlib_new_delete.cpp` and `libcxx/src/new.cpp`.

- Add default `__assertion_handler` to `libcxx/include`:
  llvm/llvm-project#77883 adds a way for vendors
  to override how we handle assertions. If a vendor doesn't want to
  override it, CMake will copy the provided [default template assertion
  handler
  file](https://github.com/llvm/llvm-project/blob/26a1d6601d727a96f4301d0d8647b5a42760ae0c/libcxx/vendor/llvm/default_assertion_handler.in)
  to libcxx's generated include dir, which defaults to
  `SYSROOT/include/c++/v1`:
  https://github.com/llvm/llvm-project/blob/26a1d6601d727a96f4301d0d8647b5a42760ae0c/libcxx/CMakeLists.txt#L72-L73
  https://github.com/llvm/llvm-project/blob/26a1d6601d727a96f4301d0d8647b5a42760ae0c/libcxx/CMakeLists.txt#L439
  https://github.com/llvm/llvm-project/blob/26a1d6601d727a96f4301d0d8647b5a42760ae0c/libcxx/include/CMakeLists.txt#L1024
  We don't use CMake, so this renames the provided
  `vendor/llvm/default_assertion_handler.in` to `__assert_handler` in
  our `libcxx/include` directory, which will be copied to
  `SYSROOT/include/c++/v1`.

- Disable `__libcpp_verbose_abort directly` in code:
  In emscripten-core#20707 we decided to disable `__libcpp_verbose_abort` not to incur
  the code size increase that brings (it brings in `vfprintf` and its
  family). We disabled it in adding
  `#define _LIBCPP_AVAILABILITY_HAS_NO_VERBOSE_ABORT` in
  `__config_site`. But that `_NO_` macros are gone in LLVM 18.1.2, and
  changing it to `#define _LIBCPP_AVAILABILITY_HAS_VERBOSE_ABORT 0` does
  not work because it is overridden by this line, unless we provide our
  own vendor annotations:
  https://github.com/llvm/llvm-project/blob/38f5596feda3276a8aa64fc14e074334017088ca/libcxx/include/__availability#L138
  I asked about this in
  llvm/llvm-project#87012 and
  llvm/llvm-project#71002 (comment)
  (and more comments below)
  but didn't get an actionable answer yet. So this disables
  `__libcpp_verbose_abort` in the code directly for now, hopefully
  temporarily.

Each fix is described in its own commit, except for the fixes required
to resolve conflicts when merging our changes, which I wasn't able to
commit separately.

Fixes emscripten-core#21603.
aheejin added a commit to aheejin/llvm-project that referenced this pull request May 22, 2024
This upstreams more recent, mostly EH changes from libcxx and libcxxabi:
- `__cxa_init_primary_exception`-related changes made when updating to
  LLVM 18.1.2 (emscripten-core/emscripten#21638)
- Removes ctype macros
  (emscripten-core/emscripten#20960)
- Guard destructor changes with `__wasm__`
  (emscripten-core/emscripten#21974)
aheejin added a commit to llvm/llvm-project that referenced this pull request May 22, 2024
This upstreams more recent, mostly EH changes from libcxx and libcxxabi:
- `__cxa_init_primary_exception`-related changes made when updating to
LLVM 18.1.2 (emscripten-core/emscripten#21638)
- Removes ctype macros
(emscripten-core/emscripten#20960)
- Guard destructor changes with `__wasm__`
(emscripten-core/emscripten#21974)
aheejin added a commit to aheejin/emscripten that referenced this pull request May 31, 2024
Background:

After emscripten-core#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 the 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 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`)

---

Solution:

So this adds `__cxa_init_primary_exception` to `cxa_noexception.cpp`.
aheejin added a commit to aheejin/emscripten that referenced this pull request May 31, 2024
Background:

After emscripten-core#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 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`)

---

Solution:

So this adds `__cxa_init_primary_exception` to `cxa_noexception.cpp`.
aheejin added a commit to aheejin/emscripten that referenced this pull request May 31, 2024
Background:

After emscripten-core#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 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`)

---

Solution:

So this adds `__cxa_init_primary_exception` to `cxa_noexception.cpp`.
aheejin added a commit to aheejin/emscripten that referenced this pull request May 31, 2024
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 emscripten-core#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 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`.
aheejin added a commit to aheejin/emscripten that referenced this pull request May 31, 2024
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 emscripten-core#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`.
aheejin added a commit to aheejin/emscripten that referenced this pull request May 31, 2024
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 emscripten-core#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`.
aheejin added a commit that referenced this pull request May 31, 2024
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`.
msorvig pushed a commit to msorvig/emscripten that referenced this pull request Jun 4, 2024
…ten-core#22027)

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 emscripten-core#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`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Template Instantiation Error in system/lib/libcxx/include/__memory/pointer_traits.h
3 participants