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

[ldionne]

Summary

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).

[ldionne]

Adding libunwind and vendors. I am creating this patch but I expect that someone might be able to explain why we've always done it differently. Also adding @sylvestre.ledru since he handles some LLVM packages and I suspect he might know some context around this.

[dim]

As long as the setting "OFF" is also tested somehow, so it does not bitrot, this seems pretty much OK to me?

[sylvestre.ledru]

Dunno the why but i have been shipping with LIBCXXABI_USE_LLVM_UNWINDER=ON for a while now. It works great :)

[MaskRay]

LGTM from #libunwind side.

On Linux, both libgcc_s.so.1/libgcc_eh.a and libunwind.so/libunwind.a are popular. Considering also BSD, I think the majority of libc++ users have switched to using llvm-project/libunwind primarily, so this default switch makes a lot of sense.

The "Level 1: Base ABI of Itanium C++ ABI: Exception Handling" part of libunwind is very stable, so I don't worry too much that doing this change will degrade libgcc_s.so.1/libgcc_eh.a compatibility in the long run.
Having a LIBCXXABI_USE_LLVM_UNWINDER=ON build bot will definitely be great, but I am fine we don't have it due to resource limitation/etc.

[manojgupta]

GCC's libgcc_s/libgcc and llvm's libunwind are not ABI compatible, the code will compile/link but will crash at runtime for calls to functions like backtrace() because functions from libunwind and libgcc will get intermingled. So this is a risky change for the users. I'd strongly advise to make a note of this is release notes.

[dim]

In D150897#4354202, @MaskRay wrote:

Considering also BSD, I think the majority of libc++ users have switched to using llvm-project/libunwind primarily, so this default switch makes a lot of sense.

On FreeBSD at least, we have been using LLVM libunwind since quite a while (@emaste might remember when we introduced it), but due to historical reasons we don't use libc++abi but libcxxrt as the ABI library. So this is bit of a strange situation, and why I am interested in keeping the "use the system libunwind case" still tested somehow.

So I would find it nice if the "With LLVM's libunwind" job that is going to be removed, would be replaced by another one called ""Without LLVM's libunwind" :)

[ldionne]

Thanks everyone for chiming in, this is exactly what I was looking for!

In D150897#4354210, @manojgupta wrote:

GCC's libgcc_s/libgcc and llvm's libunwind are not ABI compatible, the code will compile/link but will crash at runtime for calls to functions like backtrace() because functions from libunwind and libgcc will get intermingled. So this is a risky change for the users. I'd strongly advise to make a note of this is release notes.

Do you mean that on platforms where system libraries are using libgcc_s as the unwinder by default, trying to then use libc++ (built against LLVM's libunwind) e.g. from a LLVM release would create these ABI issues? So essentially, a vendor shipping the LLVM toolchain on such a platform would want to *not* ship libunwind (and make sure that libc++ uses the system provided unwinder) for the sake of interoperability with the base system?

I think that makes sense, and indeed that would then merit a release note for sure.

What I would then suggest is:

1. We make this change with a release note
2. We introduce a new CI job called "with the system unwinder"
3. In a separate change, I could refactor the way we select the unwinder to match what we do for libc++abi, i.e. we would have LIBCXXABI_UNWINDER_LIBRARY="libunwind|gcc|whatever"

That way, we'd ship a LLVM toolchain that uses its own stuff by default, but a vendor would still be free to select a different unwinder library. That would make the situation with the unwinder library choice consistent with how we handle picking the ABI library.

[manojgupta]

The plan sgtm.

[MaskRay]

In D150897#4354210, @manojgupta wrote:

GCC's libgcc_s/libgcc and llvm's libunwind are not ABI compatible, the code will compile/link but will crash at runtime for calls to functions like backtrace() because functions from libunwind and libgcc will get intermingled. So this is a risky change for the users. I'd strongly advise to make a note of this is release notes.

The _Unwind_* functions are ABI incompatible, but _Unwind_Context structs are not.

You probably mean glibc's arm port (https://bugs.chromium.org/p/chromium/issues/detail?id=1162190#c16). I have a write-up about when glibc invokes dlopen on libgcc_s.so.1 (pthread_exit/pthread_cancel, and backtrace-family functions):
https://gist.github.com/MaskRay/3d05f5613a2f8e774aad26ee3da4e696

For pthread_exit/pthread_cancel, dlopening libgcc_s.so.1 is undesired but benign. For backtrace, backtrace_symbols, and backtrace_symbols_fd declared in <execinfo.h>, I agree that they are problematic on arm and need attention on other ports (but I don't find another issue yet).

backtrace-family functions are not recommended, so the cross-walking problem should be limited. That said, I want to check whether this can be improved on glibc'c side.

[phosek]

We've been using this configuration in our toolchain for years, both for Fuchsia and for Linux, without any issues. I share @manojgupta's concerns though regarding compatibility with system libraries.

Relatedly, I think we should also consider changing the defaults for COMPILER_RT_USE_LLVM_UNWINDER and potentially also LIBCXXABI_USE_COMPILER_RT, LIBCXX_USE_COMPILER_RT, COMPILER_RT_USE_BUILTINS_LIBRARY.

[manojgupta]

You probably mean glibc's arm port (https://bugs.chromium.org/p/chromium/issues/detail?id=1162190#c16

Yes, we had discovered real issues with libgcc/libunwind intertwining so this is not just hypothetical and this is why llvm-libgcc project now exists.

[zibi]

On z/OS, we do build it with LIBCXXABI_USE_LLVM_UNWINDER=OFF so we will have to turn this off explicitly in cache.

[ldionne]

Add a release note and also use LLVM libunwind by default when building compiler-rt.

Most vendors use a custom CMake cache that sets these settings appropriately. However, I don't think we use any CMake cache when we build the "upstream" LLVM releases. I wonder what this means for the upstream LLVM release, and in fact I wonder what we expect folks to do at all with the runtimes shipped alongside the LLVM release.

To expand on that: if our intent is that folks can use upstream LLVM releases to build a toolchain and have e.g. their .sos interoperate with the rest of the system out of the box, I don't think that works if we even ship libc++/libc++abi. In a sense, the upstream LLVM toolchain we ship should always use the system-provided runtimes (Linux: libstdc++ libgcc_s etc, Apple: libc++ libc++abi libunwind etc, FreeBSD: libc++ libcxxrt etc). On the other hand, if the goal of the "upstream LLVM release" is to ship a full toolchain that is consistent with itself, then I think this change gets us closer to that.

[ldionne]

Rebase and try to fix CI.

compiler-rt/CMakeLists.txt

 # This needs to be a full C++ library for linking gtest and unit tests.
 # TODO: Mark this as internal flag, most users should use COMPILER_RT_CXX_LIBRARY.
 set(SANITIZER_TEST_CXX "default" CACHE STRING
     "Specify C++ library to use for tests.")
 set(CXXLIBS none default libstdc++ libc++)
 set_property(CACHE SANITIZER_TEST_CXX PROPERTY STRINGS ;${CXXLIBS})
 handle_default_cxx_lib(SANITIZER_TEST_CXX)
 
-option(COMPILER_RT_USE_LLVM_UNWINDER "Use the LLVM unwinder." OFF)
+option(COMPILER_RT_USE_LLVM_UNWINDER "Use the LLVM unwinder." ON)
 cmake_dependent_option(COMPILER_RT_ENABLE_STATIC_UNWINDER
   "Statically link the LLVM unwinder." OFF
   "COMPILER_RT_USE_LLVM_UNWINDER" OFF)
 
 set(DEFAULT_SANITIZER_USE_STATIC_LLVM_UNWINDER OFF)
 if (FUCHSIA)
   set(DEFAULT_SANITIZER_USE_STATIC_LLVM_UNWINDER ON)
 elseif (DEFINED LIBUNWIND_ENABLE_SHARED AND NOT LIBUNWIND_ENABLE_SHARED)

libcxx/CMakeLists.txt

 
 option(LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS
   "Build libc++ with definitions for operator new/delete. These are normally
    defined in libc++abi, but this option can be used to define them in libc++
    instead. If you define them in libc++, make sure they are NOT defined in
    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})
 if (LIBCXX_BUILD_32_BITS)
   message(FATAL_ERROR "LIBCXX_BUILD_32_BITS is not supported anymore when building the runtimes, please specify a full triple instead.")
 endif()
 
 # Feature options -------------------------------------------------------------

libcxx/cmake/caches/AArch64.cmake

-set(LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "")
 set(CMAKE_CXX_COMPILER_TARGET "aarch64-linux-gnu" CACHE STRING "")

libcxx/cmake/caches/AIX.cmake

 set(CMAKE_BUILD_TYPE Release CACHE STRING "")
 set(CMAKE_BUILD_WITH_INSTALL_RPATH ON CACHE BOOL "")
 set(CMAKE_C_FLAGS "-D__LIBC_NO_CPP_MATH_OVERLOADS__" CACHE STRING "")
 set(CMAKE_CXX_FLAGS "-D__LIBC_NO_CPP_MATH_OVERLOADS__" CACHE STRING "")
 set(CMAKE_SHARED_LINKER_FLAGS "-Wl,-G -Wl,-bcdtors:all:-2147483548:s" CACHE STRING "")
 set(CMAKE_AR "/usr/bin/ar" CACHE FILEPATH "")
 
 set(LIBCXX_USE_COMPILER_RT ON CACHE BOOL "")
 set(LIBCXX_ABI_VERSION "1" CACHE STRING "")
 set(LIBCXX_ENABLE_ABI_LINKER_SCRIPT OFF CACHE BOOL "")
 set(LIBCXX_ENABLE_SHARED ON CACHE BOOL "")
 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 "")

libcxx/cmake/caches/Apple.cmake

 set(CMAKE_BUILD_TYPE MinSizeRel CACHE STRING "")
 set(CMAKE_POSITION_INDEPENDENT_CODE OFF CACHE BOOL "")
 
 set(LIBCXX_USE_COMPILER_RT ON CACHE BOOL "")
 set(LIBCXX_ABI_VERSION "1" CACHE STRING "")
 set(LIBCXX_ENABLE_STATIC ON CACHE BOOL "")
 set(LIBCXX_ENABLE_SHARED ON CACHE BOOL "")
 set(LIBCXX_CXX_ABI libcxxabi CACHE STRING "")
 set(LIBCXX_ENABLE_VENDOR_AVAILABILITY_ANNOTATIONS ON CACHE BOOL "")
 set(LIBCXX_PSTL_CPU_BACKEND libdispatch CACHE STRING "")
 
 set(LIBCXX_HERMETIC_STATIC_LIBRARY ON CACHE BOOL "")
 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 "")

libcxx/cmake/caches/Armv7Arm.cmake

-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 "")

libcxx/cmake/caches/Armv7Thumb-no-exceptions.cmake

-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 "")
 set(LIBCXX_ENABLE_EXCEPTIONS OFF CACHE BOOL "")
 set(LIBCXXABI_ENABLE_EXCEPTIONS OFF CACHE BOOL "")

libcxx/cmake/caches/Armv8Arm.cmake

-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 "")

libcxx/cmake/caches/Armv8Thumb-no-exceptions.cmake

-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 "")
 set(LIBCXX_ENABLE_EXCEPTIONS OFF CACHE BOOL "")
 set(LIBCXXABI_ENABLE_EXCEPTIONS OFF CACHE BOOL "")

libcxx/cmake/caches/Generic-merged.cmake

 # Build a libc++ shared library, but merge libc++abi and libunwind into it.
 set(LIBCXX_ENABLE_SHARED ON CACHE BOOL "")
 set(LIBCXX_ENABLE_ABI_LINKER_SCRIPT OFF CACHE BOOL "")
 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 "")
 
 set(LIBUNWIND_ENABLE_SHARED OFF CACHE BOOL "")

libcxx/cmake/caches/Generic-msan.cmake

 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

libcxx/cmake/caches/Generic-tsan.cmake

 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

libcxx/cmake/caches/MinGW.cmake

 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 "")
 
 set(LIBCXX_USE_COMPILER_RT ON CACHE BOOL "")
 set(LIBCXXABI_USE_COMPILER_RT ON CACHE BOOL "")
 set(LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "")
 
 # Without this flag, 'long double' (which is 80 bit on x86 mingw, but
 # 64 bit in MSVC) isn't handled correctly in printf.
 set(LIBCXX_EXTRA_SITE_DEFINES "__USE_MINGW_ANSI_STDIO=1" CACHE STRING "")

libcxx/docs/BuildingLibcxx.rst

.. option:: LIBCXX_ENABLE_ABI_LINKER_SCRIPT:BOOL
   **Default**: ``ON`` by default on UNIX platforms other than Apple unless
   'LIBCXX_ENABLE_STATIC_ABI_LIBRARY' is ON. Otherwise the default value is ``OFF``.
 
   This option generate and installs a linker script as ``libc++.so`` which
   links the correct ABI library.
 
 .. 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.
 
 .. option:: LIBCXXABI_ADDITIONAL_COMPILE_FLAGS:STRING
 
   **Default**: ``""``
 

libcxx/docs/ReleaseNotes/18.rst

- This release of libc++ added missing visibility annotations on some types in the library. Users compiling with
   symbols from being part of your ABI.
 
 - The name mangling for intantiations of ``std::projected`` has changed in order to implement P2538R1. This technically
   results in an ABI break, however in practice we expect uses of ``std::projected`` in ABI-sensitive places to be
   extremely rare. Any error resulting from this change should result in a link-time error.
 
 Build System Changes
 --------------------
+
+- ``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.

libcxx/utils/ci/buildkite-pipeline.yml

agents:
       queue: "libcxx-builders"
       os: "linux"
     retry:
       automatic:
         - exit_status: -1  # Agent was lost
           limit: 2
     timeout_in_minutes: 120
 
-  - label: "With LLVM's libunwind"
-    command: "libcxx/utils/ci/run-buildbot generic-with_llvm_unwinder"
-    artifact_paths:
-      - "**/test-results.xml"
-      - "**/*.abilist"
-    env:
-        CC: "clang-${LLVM_HEAD_VERSION}"
-        CXX: "clang++-${LLVM_HEAD_VERSION}"
-        ENABLE_CLANG_TIDY: "On"
-    agents:
-      queue: "libcxx-builders"
-      os: "linux"
-    retry:
-      automatic:
-        - exit_status: -1  # Agent was lost
-          limit: 2
-    timeout_in_minutes: 120
-
   - label: "Modular build with Local Submodule Visibility"
     command: "libcxx/utils/ci/run-buildbot generic-modules-lsv"
     artifact_paths:
       - "**/test-results.xml"
       - "**/*.abilist"
     env:
         CC: "clang-${LLVM_HEAD_VERSION}"
         CXX: "clang++-${LLVM_HEAD_VERSION}"

libcxx/utils/ci/run-buildbot

generic-safe-mode)
     check-abi-list
 ;;
 generic-debug-mode)
     clean
     generate-cmake -C "${MONOREPO_ROOT}/libcxx/cmake/caches/Generic-debug-mode.cmake"
     check-runtimes
     check-abi-list
 ;;
-generic-with_llvm_unwinder)
-    clean
-    generate-cmake -DLIBCXXABI_USE_LLVM_UNWINDER=ON
-    check-runtimes
-;;
 #
 # Module builds
 #
 generic-modules)
     clean
     generate-cmake -C "${MONOREPO_ROOT}/libcxx/cmake/caches/Generic-modules.cmake"
     check-runtimes
     check-abi-list

libcxxabi/CMakeLists.txt

 
 # Define options.
 option(LIBCXXABI_ENABLE_EXCEPTIONS
   "Provide support for exceptions in the runtime.
   When disabled, libc++abi does not support stack unwinding and other exceptions-related features." ON)
 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)
 option(LIBCXXABI_HAS_PTHREAD_API "Ignore auto-detection and force use of pthread API" OFF)
 option(LIBCXXABI_HAS_WIN32_THREAD_API "Ignore auto-detection and force use of win32 thread API" OFF)
 option(LIBCXXABI_HAS_EXTERNAL_THREAD_API
   "Build libc++abi with an externalized threading API.
   This option may only be set to ON when LIBCXXABI_ENABLE_THREADS=ON." OFF)