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.

Sign up for GitHub

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

Jump to bottom

[D150897] [runtimes] Use LLVM libunwind from libc++abi and compiler-rt by default #77662

Closed
ldionne opened this issue Jan 10, 2024 · 1 comment · Fixed by #77687
Closed

[D150897] [runtimes] Use LLVM libunwind from libc++abi and compiler-rt by default #77662

ldionne opened this issue Jan 10, 2024 · 1 comment · Fixed by #77687
Assignees
@ldionne
Labels
cmake Build system in general and CMake in particular libc++abi libc++abi C++ Runtime Library. Not libc++. phabricator Related to migration from Phabricator

Comments

@ldionne
Copy link
Member

ldionne commented Jan 10, 2024

This issue tracks picking up https://reviews.llvm.org/D150897 from the Phabricator archive.
@ldionne ldionne added libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. awaiting-review Has pending Phabricator review labels Jan 10, 2024
@ldionne ldionne self-assigned this Jan 10, 2024
@ldionne ldionne added phabricator Related to migration from Phabricator and removed awaiting-review Has pending Phabricator review labels Jan 10, 2024
ldionne added a commit to ldionne/llvm-project that referenced this issue Jan 10, 2024
@ldionne
[runtimes] Use LLVM libunwind from libc++abi and compiler-rt by default
6235f49 
I recently came across LIBCXXABI_USE_LLVM_UNWINDER and was surprised
to notice it was disabled by default. Since we build libunwind by
default and ship it in the LLVM toolchain, it would seem to make
sense that libc++ and libc++abi rely on libunwind for unwinding
instead of using the system-provided unwinding library (if any).

Most importantly, using the system unwinder implies that libc++abi is
ABI compatible with that system unwinder, which is not necessarily the
case. Hence, it makes a lot more sense to instead default to using the
known-to-be-compatible LLVM unwinder, and let vendors manually select
a different unwinder if desired.

Differential Revision: https://reviews.llvm.org/D150897
Fixes llvm#77662
rdar://120801778
@ldionne ldionne mentioned this issue Jan 10, 2024
Merged
ldionne added a commit that referenced this issue Jan 11, 2024
@ldionne
[runtimes] Use LLVM libunwind from libc++abi by default (#77687)
8f90e69 
I recently came across LIBCXXABI_USE_LLVM_UNWINDER and was surprised to
notice it was disabled by default. Since we build libunwind by default
and ship it in the LLVM toolchain, it would seem to make sense that
libc++ and libc++abi rely on libunwind for unwinding instead of using
the system-provided unwinding library (if any).

Most importantly, using the system unwinder implies that libc++abi is
ABI compatible with that system unwinder, which is not necessarily the
case. Hence, it makes a lot more sense to instead default to using the
known-to-be-compatible LLVM unwinder, and let vendors manually select a
different unwinder if desired.

As a follow-up change, we should probably apply the same default to
compiler-rt.

Differential Revision: https://reviews.llvm.org/D150897
Fixes #77662
rdar://120801778
@EugeneZelenko EugeneZelenko added cmake Build system in general and CMake in particular and removed libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. labels Jan 11, 2024
@ldionne
Copy link
Member Author

ldionne commented Jan 11, 2024

This is kind of a libc++/libc++abi issue really since this has to do with which libunwind we use from libc++abi. I'd like to keep the libc++abi tag on this.

@ldionne ldionne added the libc++abi libc++abi C++ Runtime Library. Not libc++. label Jan 11, 2024
searlmc1 pushed a commit to ROCm/llvm-project that referenced this issue Jan 13, 2024
@ldionne @ronlieb
[runtimes] Use LLVM libunwind from libc++abi by default (llvm#77687)
a81aa94 
I recently came across LIBCXXABI_USE_LLVM_UNWINDER and was surprised to
notice it was disabled by default. Since we build libunwind by default
and ship it in the LLVM toolchain, it would seem to make sense that
libc++ and libc++abi rely on libunwind for unwinding instead of using
the system-provided unwinding library (if any).

Most importantly, using the system unwinder implies that libc++abi is
ABI compatible with that system unwinder, which is not necessarily the
case. Hence, it makes a lot more sense to instead default to using the
known-to-be-compatible LLVM unwinder, and let vendors manually select a
different unwinder if desired.

As a follow-up change, we should probably apply the same default to
compiler-rt.

Differential Revision: https://reviews.llvm.org/D150897
Fixes llvm#77662
rdar://120801778

Change-Id: Id2a0483ca5b032faec2d5c81753aea9a08695b4f
justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this issue Jan 28, 2024
@ldionne @justinfargnoli
[runtimes] Use LLVM libunwind from libc++abi by default (llvm#77687)
dc1c778 
I recently came across LIBCXXABI_USE_LLVM_UNWINDER and was surprised to
notice it was disabled by default. Since we build libunwind by default
and ship it in the LLVM toolchain, it would seem to make sense that
libc++ and libc++abi rely on libunwind for unwinding instead of using
the system-provided unwinding library (if any).

Most importantly, using the system unwinder implies that libc++abi is
ABI compatible with that system unwinder, which is not necessarily the
case. Hence, it makes a lot more sense to instead default to using the
known-to-be-compatible LLVM unwinder, and let vendors manually select a
different unwinder if desired.

As a follow-up change, we should probably apply the same default to
compiler-rt.

Differential Revision: https://reviews.llvm.org/D150897
Fixes llvm#77662
rdar://120801778
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ldionne ldionne

Labels
cmake Build system in general and CMake in particular libc++abi libc++abi C++ Runtime Library. Not libc++. phabricator Related to migration from Phabricator
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[runtimes] Use LLVM libunwind from libc++abi by default ldionne/llvm-project
2 participants
@ldionne @EugeneZelenko