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

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
llvm · Jan 11, 2024 · 8f90e69 · 8f90e69 · maryammo · Jan 12, 2024
1 parent 3b3ee1f
commit 8f90e69
Show file tree

Hide file tree

Showing 19 changed files with 16 additions and 18 deletions.
diff --git a/.github/workflows/libcxx-build-and-test.yaml b/.github/workflows/libcxx-build-and-test.yaml
@@ -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',

diff --git a/libcxx/CMakeLists.txt b/libcxx/CMakeLists.txt
@@ -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})

diff --git a/libcxx/cmake/caches/AArch64.cmake b/libcxx/cmake/caches/AArch64.cmake
@@ -1,2 +1 @@
-set(LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "")
 set(CMAKE_CXX_COMPILER_TARGET "aarch64-linux-gnu" CACHE STRING "")
diff --git a/libcxx/cmake/caches/AIX.cmake b/libcxx/cmake/caches/AIX.cmake
@@ -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 "")
diff --git a/libcxx/cmake/caches/AndroidNDK.cmake b/libcxx/cmake/caches/AndroidNDK.cmake
@@ -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 "")
+
 # 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.

diff --git a/libcxx/cmake/caches/Apple.cmake b/libcxx/cmake/caches/Apple.cmake
@@ -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 "")
diff --git a/libcxx/cmake/caches/Armv7Arm.cmake b/libcxx/cmake/caches/Armv7Arm.cmake
@@ -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 "")
diff --git a/libcxx/cmake/caches/Armv7M-picolibc.cmake b/libcxx/cmake/caches/Armv7M-picolibc.cmake
@@ -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 "")

diff --git a/libcxx/cmake/caches/Armv7Thumb-no-exceptions.cmake b/libcxx/cmake/caches/Armv7Thumb-no-exceptions.cmake
@@ -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 "")

diff --git a/libcxx/cmake/caches/Armv8Arm.cmake b/libcxx/cmake/caches/Armv8Arm.cmake
@@ -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 "")
diff --git a/libcxx/cmake/caches/Armv8Thumb-no-exceptions.cmake b/libcxx/cmake/caches/Armv8Thumb-no-exceptions.cmake
@@ -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 "")

diff --git a/libcxx/cmake/caches/Generic-merged.cmake b/libcxx/cmake/caches/Generic-merged.cmake
@@ -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 "")
 

diff --git a/libcxx/cmake/caches/Generic-msan.cmake b/libcxx/cmake/caches/Generic-msan.cmake
@@ -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
diff --git a/libcxx/cmake/caches/Generic-tsan.cmake b/libcxx/cmake/caches/Generic-tsan.cmake
@@ -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
diff --git a/libcxx/cmake/caches/MinGW.cmake b/libcxx/cmake/caches/MinGW.cmake
@@ -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 "")

diff --git a/libcxx/docs/BuildingLibcxx.rst b/libcxx/docs/BuildingLibcxx.rst
@@ -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.

diff --git a/libcxx/docs/ReleaseNotes/18.rst b/libcxx/docs/ReleaseNotes/18.rst
@@ -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.
diff --git a/libcxx/utils/ci/run-buildbot b/libcxx/utils/ci/run-buildbot
@@ -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
 #

diff --git a/libcxxabi/CMakeLists.txt b/libcxxabi/CMakeLists.txt
@@ -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)