-
Notifications
You must be signed in to change notification settings - Fork 13k
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
[libc++] How to override a single setting in __availability? #87012
Comments
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.
These settings are not intended to be "customizable" in the general sense. Specifically, these settings represent features that are only disabled when the deployment target is too old to support them, such as trying to use These settings are inherently temporal: whenever we don't support any platform that requires the old behavior, we remove the setting and unconditionally use the new behavior. Hence, we don't want to start allowing for individual settings to be toggled based on preference, since that's not their goal. Many other settings are toggle-able individually and we document them at https://libcxx.llvm.org/UsingLibcxx.html#libc-configuration-macros, but something like If you want |
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 #11079 and #20154. So while resolving the conflicts, I effectively reverted #11079 and #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 #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 #21603.
Thanks for the reply. We don't have a concrete requirement for now, but will post an issue later if we get to work on it. |
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.
WebAssembly's emscripten toolchain (https://github.com/emscripten-core/emscripten) has been using
_LIBCPP_AVAILABILITY_HAS_NO_VERBOSE_ABORT
not to pay for the increased code size of__libcpp_verbose_abort
so far. But after #71002, that macro does not exist. We don't provide our own vendor annotation, and defining_LIBCPP_AVAILABILITY_HAS_VERBOSE_ABORT
as 0 is overridden by this line:llvm-project/libcxx/include/__availability
Line 138 in 38f5596
I read the discussions in that PR that it is recommended to create our own availability markup like Apple:
llvm-project/libcxx/include/__availability
Line 163 in 38f5596
But to do that it looks we have to copy the whole list of macros here
llvm-project/libcxx/include/__availability
Lines 86 to 161 in 38f5596
and copy-paste them to our section in order to just change the one line (
_LIBCPP_AVAILABILITY_HAS_VERBOSE_ABORT
). Is this the recommended way of changing one setting? Is there a way we can control this elsewhere?This doesn't necessarily have to be about
_LIBCPP_AVAILABILITY_HAS_VERBOSE_ABORT
, but it can be more general question about what is the recommended way of toggling one setting without copy-pasting dozens of settings to add anotherifdef (__SOMEPLATFORM__)
in__availability
.A similar concern was posted in #71002 (comment) by @wang-bin.
cc @philnik777 @ldionne
The text was updated successfully, but these errors were encountered: