Skip to content

[runtimes] Use LLVM libunwind from libc++abi by default #77687

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 4 commits into from
Jan 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .github/workflows/libcxx-build-and-test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,6 @@ jobs:
'generic-no-rtti',
'generic-optimized-speed',
'generic-static',
'generic-with_llvm_unwinder',
# TODO Find a better place for the benchmark and bootstrapping builds to live. They're either very expensive
# or don't provide much value since the benchmark run results are too noise on the bots.
'benchmarks',
Expand Down
2 changes: 1 addition & 1 deletion libcxx/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ option(LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS
libc++abi. Doing otherwise is an ODR violation." OFF)
# Build libc++abi with libunwind. We need this option to determine whether to
# link with libunwind or libgcc_s while running the test cases.
option(LIBCXXABI_USE_LLVM_UNWINDER "Build and use the LLVM unwinder." OFF)
option(LIBCXXABI_USE_LLVM_UNWINDER "Build and use the LLVM unwinder." ON)

# Target options --------------------------------------------------------------
option(LIBCXX_BUILD_32_BITS "Build 32 bit multilib libc++. This option is not supported anymore when building the runtimes. Please specify a full triple instead." ${LLVM_BUILD_32_BITS})
Expand Down
1 change: 0 additions & 1 deletion libcxx/cmake/caches/AArch64.cmake
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
set(LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "")
set(CMAKE_CXX_COMPILER_TARGET "aarch64-linux-gnu" CACHE STRING "")
1 change: 0 additions & 1 deletion libcxx/cmake/caches/AIX.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,5 @@ set(LIBCXX_ENABLE_STATIC OFF CACHE BOOL "")
set(LIBCXXABI_ENABLE_SHARED ON CACHE BOOL "")
set(LIBCXXABI_ENABLE_STATIC OFF CACHE BOOL "")
set(LIBCXX_CXX_ABI libcxxabi CACHE STRING "")
set(LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "")
set(LIBUNWIND_ENABLE_SHARED ON CACHE BOOL "")
set(LIBUNWIND_ENABLE_STATIC OFF CACHE BOOL "")
3 changes: 3 additions & 0 deletions libcxx/cmake/caches/AndroidNDK.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ set(LIBCXX_ENABLE_ABI_LINKER_SCRIPT OFF CACHE BOOL "")
set(LIBCXX_STATICALLY_LINK_ABI_IN_SHARED_LIBRARY ON CACHE BOOL "")
set(LIBCXXABI_ENABLE_SHARED OFF CACHE BOOL "")

# Android uses its own unwinder library
set(LIBCXXABI_USE_LLVM_UNWINDER OFF CACHE BOOL "")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm. The Android runtime unwinder is also LLVM libunwind; is there any harm in building and using our own instead of the NDK's? CC @rprichard

Copy link
Contributor

Choose a reason for hiding this comment

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

The Android NDK currently sources all of libunwind, libc++abi, and libc++ from the same LLVM commit, so maybe it's better to build the unwinder library at the same time as libc++.

OTOH, Android's libc.so exports libunwind's EH API as of API 30 (Android 11), so eventually the NDK might be using an unwinder that's from a much older LLVM commit. It'll be a long time before the NDK targets API 30 as a minimum.

IIRC, when I last looked at the Linux libc++ CI tests (i.e. non-Android), the LLVM libunwind was built in the same runtimes CMake invocation as libc++, but the result wasn't actually used or tested. When I set up Android CI testing, I initially had it build libunwind as part of the CI testing, but I wanted the result to resemble what the NDK actually ships, which required a separate step for libunwind (and patching the result into a new resource dir) (see https://reviews.llvm.org/D139147?id=549856, search for "Build libunwind.a"). This patching was dropped in favor of using a libunwind.a from an Android Clang build.


# Clang links libc++ by default, but it doesn't exist yet. The libc++ CMake
# files specify -nostdlib++ to avoid this problem, but CMake's default "compiler
# works" testing doesn't pass that flag, so force those tests to pass.
Expand Down
1 change: 1 addition & 0 deletions libcxx/cmake/caches/Apple.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ set(LIBCXXABI_HERMETIC_STATIC_LIBRARY ON CACHE BOOL "")

set(LIBCXXABI_ENABLE_ASSERTIONS OFF CACHE BOOL "")
set(LIBCXXABI_ENABLE_FORGIVING_DYNAMIC_CAST ON CACHE BOOL "")
set(LIBCXXABI_USE_LLVM_UNWINDER OFF CACHE BOOL "") # libunwind is built separately

set(LIBCXX_TEST_CONFIG "apple-libc++-shared.cfg.in" CACHE STRING "")
set(LIBCXXABI_TEST_CONFIG "apple-libc++abi-shared.cfg.in" CACHE STRING "")
1 change: 0 additions & 1 deletion libcxx/cmake/caches/Armv7Arm.cmake
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
set(LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "")
set(CMAKE_CXX_COMPILER_TARGET "armv7l-linux-gnueabihf" CACHE STRING "")
set(CMAKE_CXX_FLAGS "-marm" CACHE STRING "")
set(CMAKE_C_FLAGS "-marm" CACHE STRING "")
1 change: 0 additions & 1 deletion libcxx/cmake/caches/Armv7M-picolibc.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ set(LIBCXXABI_ENABLE_STATIC ON CACHE BOOL "")
set(LIBCXXABI_ENABLE_STATIC_UNWINDER ON CACHE BOOL "")
set(LIBCXXABI_ENABLE_THREADS OFF CACHE BOOL "")
set(LIBCXXABI_USE_COMPILER_RT ON CACHE BOOL "")
set(LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "")
set(LIBCXX_ENABLE_EXCEPTIONS ON CACHE BOOL "")
set(LIBCXX_ENABLE_FILESYSTEM OFF CACHE STRING "")
set(LIBCXX_ENABLE_MONOTONIC_CLOCK OFF CACHE BOOL "")
Expand Down
1 change: 0 additions & 1 deletion libcxx/cmake/caches/Armv7Thumb-no-exceptions.cmake
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
set(LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "")
set(CMAKE_CXX_COMPILER_TARGET "armv7l-linux-gnueabihf" CACHE STRING "")
set(CMAKE_CXX_FLAGS "-mthumb" CACHE STRING "")
set(CMAKE_C_FLAGS "-mthumb" CACHE STRING "")
Expand Down
1 change: 0 additions & 1 deletion libcxx/cmake/caches/Armv8Arm.cmake
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
set(LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "")
set(CMAKE_CXX_COMPILER_TARGET "armv8l-linux-gnueabihf" CACHE STRING "")
set(CMAKE_CXX_FLAGS "-marm" CACHE STRING "")
set(CMAKE_C_FLAGS "-marm" CACHE STRING "")
1 change: 0 additions & 1 deletion libcxx/cmake/caches/Armv8Thumb-no-exceptions.cmake
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
set(LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "")
set(CMAKE_CXX_COMPILER_TARGET "armv8l-linux-gnueabihf" CACHE STRING "")
set(CMAKE_CXX_FLAGS "-mthumb" CACHE STRING "")
set(CMAKE_C_FLAGS "-mthumb" CACHE STRING "")
Expand Down
1 change: 0 additions & 1 deletion libcxx/cmake/caches/Generic-merged.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ set(LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE BOOL "")
set(LIBCXX_STATICALLY_LINK_ABI_IN_SHARED_LIBRARY ON CACHE BOOL "")

set(LIBCXXABI_ENABLE_SHARED OFF CACHE BOOL "")
set(LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "")
set(LIBCXXABI_ENABLE_STATIC_UNWINDER ON CACHE BOOL "")
set(LIBCXXABI_STATICALLY_LINK_UNWINDER_IN_STATIC_LIBRARY ON CACHE BOOL "")

Expand Down
1 change: 1 addition & 0 deletions libcxx/cmake/caches/Generic-msan.cmake
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
set(LLVM_USE_SANITIZER "MemoryWithOrigins" CACHE STRING "")
set(LIBCXXABI_USE_LLVM_UNWINDER OFF CACHE BOOL "") # MSAN is compiled against the system unwinder, which leads to false positives
1 change: 1 addition & 0 deletions libcxx/cmake/caches/Generic-tsan.cmake
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
set(LLVM_USE_SANITIZER "Thread" CACHE STRING "")
set(LIBCXXABI_USE_LLVM_UNWINDER OFF CACHE BOOL "") # TSAN is compiled against the system unwinder, which leads to false positives
1 change: 0 additions & 1 deletion libcxx/cmake/caches/MinGW.cmake
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
set(LIBCXX_CXX_ABI libcxxabi CACHE STRING "")
set(LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "")

set(LIBCXXABI_ENABLE_SHARED OFF CACHE BOOL "")
set(LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE BOOL "")
Expand Down
2 changes: 1 addition & 1 deletion libcxx/docs/BuildingLibcxx.rst
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ ABI Library Specific Options

.. option:: LIBCXXABI_USE_LLVM_UNWINDER:BOOL

**Default**: ``OFF``
**Default**: ``ON``

Build and use the LLVM unwinder. Note: This option can only be used when
libc++abi is the C++ ABI library used.
Expand Down
7 changes: 7 additions & 0 deletions libcxx/docs/ReleaseNotes/18.rst
Original file line number Diff line number Diff line change
Expand Up @@ -191,3 +191,10 @@ Build System Changes
passing ``-Dexecutor=...`` to ``llvm-lit``. Alternatively, this flag can be made persistent in the generated test
configuration file by passing ``-DLIBCXX_TEST_PARAMS=executor=...``. This also applies to the ``LIBUWIND_EXECTOR``
and ``LIBCXXABI_EXECUTOR`` CMake variables. LLVM 19 will completely remove support for the ``*_EXECUTOR`` variables.

- ``LIBCXXABI_USE_LLVM_UNWINDER`` and ``COMPILER_RT_USE_LLVM_UNWINDER`` switched defaults from ``OFF`` to ``ON``.
This means that by default, libc++abi and compiler-rt will link against the LLVM provided ``libunwind`` library
instead of the system-provided unwinding library. If you are building the LLVM runtimes with the goal of shipping
them so that they can interoperate with other system-provided libraries that might be using a different unwinding
library (such as ``libgcc_s``), you should pass ``LIBCXXABI_USE_LLVM_UNWINDER=OFF`` and ``COMPILER_RT_USE_LLVM_UNWINDER=OFF``
to make sure the system-provided unwinding library is used by the LLVM runtimes.
5 changes: 0 additions & 5 deletions libcxx/utils/ci/run-buildbot
Original file line number Diff line number Diff line change
Expand Up @@ -439,11 +439,6 @@ generic-hardening-mode-debug)
check-runtimes
check-abi-list
;;
generic-with_llvm_unwinder)
clean
generate-cmake -DLIBCXXABI_USE_LLVM_UNWINDER=ON
check-runtimes
;;
#
# Module builds
#
Expand Down
2 changes: 1 addition & 1 deletion libcxxabi/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ option(LIBCXXABI_ENABLE_EXCEPTIONS
option(LIBCXXABI_ENABLE_ASSERTIONS "Enable assertions independent of build mode." ON)
option(LIBCXXABI_ENABLE_PEDANTIC "Compile with pedantic enabled." OFF)
option(LIBCXXABI_ENABLE_WERROR "Fail and stop if a warning is triggered." OFF)
option(LIBCXXABI_USE_LLVM_UNWINDER "Build and use the LLVM unwinder." OFF)
option(LIBCXXABI_USE_LLVM_UNWINDER "Build and use the LLVM unwinder." ON)
option(LIBCXXABI_ENABLE_STATIC_UNWINDER "Statically link the LLVM unwinder." OFF)
option(LIBCXXABI_USE_COMPILER_RT "Use compiler-rt instead of libgcc" OFF)
option(LIBCXXABI_ENABLE_THREADS "Build with threads enabled" ON)
Expand Down