-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Update libcxxabi to llvm-12 #14288
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
Merged
Merged
Update libcxxabi to llvm-12 #14288
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Third part of a 3 part series! |
d6ca966
to
b8acb91
Compare
kripken
approved these changes
May 25, 2021
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once more nice use of upstream work! (LIBCXXABI_NON_DEMANGLING_TERMINATE
)
Followup to #14280 Changes from upstream can be seen at: llvm/llvm-project@llvmorg-12.0.0...emscripten-core:emscripten-libs-12.0.0 Fixes: #14244
aheejin
added a commit
to aheejin/emscripten
that referenced
this pull request
Oct 5, 2022
In two-phase unwinding, the first phase is the search phase (`_UA_SEARCH_PHASE`) and the second one is the cleanup phase (`_UA_CLEANUP_PHASE`). The search phase searches up the stack to see if there is a matching catch handler, and if it finds one, it caches the result. And in the second cleanup phase, it retrieves the cached result (to avoid doing same work twice) and unwinds the stack. Wasm does not do the two-phase unwinding; it only has a single phase. We used `_UA_CLEANUP_PHASE` for this single phase, so in Wasm the cleanup phase is supposed to the search. So we several many custom `#ifdef`s to use the code guarded by `_UA_SEARCH_PHASE`, for example: https://github.com/aheejin/emscripten/blob/d57db5bea1719319a680699c50b91fa3d88fa0ec/system/lib/libcxxabi/src/cxa_personality.cpp#L771-L776 https://github.com/aheejin/emscripten/blob/d57db5bea1719319a680699c50b91fa3d88fa0ec/system/lib/libcxxabi/src/cxa_personality.cpp#L850-L855 These are apparently gone in emscripten-core#14288, which replaced many `if`s with `assert`s. This in effect removed our special handling for `_UA_CLEANUP_PHASE`; there are several `assert`s that asserts the current phase is `_UA_SEARCH_PHASE`, while Wasm is in `_UA_CLEANUP_PHASE`. But this has not caused problems so far because we have built libc++abi with `-NDEBUG`, so all assertions were no-op. https://github.com/emscripten-core/emscripten/blob/40fb7d2071e439f1de614898b88518df582faa94/tools/system_libs.py#L1366 But this is now a problem because emscripten-core#17979 adds a debug build of libc++abi, which enables assertions. Come to think of it, I'm not sure why I decided to use `_UA_CLEANUP_PHASE` for our single phase in the first place. If we use `_UA_SEARCH_PHASE`, we can remove more our custom code and reduce the difference between our port and the upstream library.
aheejin
added a commit
that referenced
this pull request
Oct 5, 2022
In two-phase unwinding, the first phase is the search phase (`_UA_SEARCH_PHASE`) and the second one is the cleanup phase (`_UA_CLEANUP_PHASE`). The search phase searches up the stack to see if there is a matching catch handler, and if it finds one, it caches the result. And in the second cleanup phase, it retrieves the cached result (to avoid doing same work twice) and unwinds the stack. Wasm does not do the two-phase unwinding; it only has a single phase. We used `_UA_CLEANUP_PHASE` for this single phase, so in Wasm the cleanup phase is supposed to the search. So we several many custom `#ifdef`s to use the code guarded by `_UA_SEARCH_PHASE`, for example: https://github.com/aheejin/emscripten/blob/d57db5bea1719319a680699c50b91fa3d88fa0ec/system/lib/libcxxabi/src/cxa_personality.cpp#L771-L776 https://github.com/aheejin/emscripten/blob/d57db5bea1719319a680699c50b91fa3d88fa0ec/system/lib/libcxxabi/src/cxa_personality.cpp#L850-L855 These are apparently gone in #14288, which replaced many `if`s with `assert`s. This in effect removed our special handling for `_UA_CLEANUP_PHASE`; there are several `assert`s that asserts the current phase is `_UA_SEARCH_PHASE`, while Wasm is in `_UA_CLEANUP_PHASE`. But this has not caused problems so far because we have built libc++abi with `-NDEBUG`, so all assertions were no-op. https://github.com/emscripten-core/emscripten/blob/40fb7d2071e439f1de614898b88518df582faa94/tools/system_libs.py#L1366 But this is now a problem because #17979 adds a debug build of libc++abi, which enables assertions. Come to think of it, I'm not sure why I decided to use `_UA_CLEANUP_PHASE` for our single phase in the first place. If we use `_UA_SEARCH_PHASE`, we can remove more our custom code and reduce the difference between our port and the upstream library.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Update libcxxabi to llvm-12
Followup to #14280
Changes from upstream can be seen at:
llvm/llvm-project@llvmorg-12.0.0...emscripten-core:emscripten-libs-12.0.0
Fixes: #14244