Skip to content

Commit

Permalink
[runtimes] Fix link order of system librarires on Apple platforms
Browse files Browse the repository at this point in the history 
On Apple platforms, we always support the -nostdlib++ flag. Hence, it is
not necessary to manually link against system libraries. In fact, doing
so causes us to link against libSystem explicitly, which messes up with
the order of libraries we should use. Indeed:

   Before patch, using the system unwinder (LIBCXXABI_USE_LLVM_UNWINDER = OFF)
   ===========================================================================
   $ otool -L lib/{libc++.1.dylib,libc++abi.1.dylib,libunwind.1.dylib}
   lib/libc++.1.dylib:
         @rpath/libc++.1.dylib
         /usr/lib/libSystem.B.dylib
         @rpath/libc++abi.1.dylib
   lib/libc++abi.1.dylib:
         @rpath/libc++abi.1.dylib
         /usr/lib/libSystem.B.dylib
   lib/libunwind.1.dylib:
         @rpath/libunwind.1.dylib
         /usr/lib/libSystem.B.dylib

   After patch, using the system unwinder (LIBCXXABI_USE_LLVM_UNWINDER = OFF)
   ===========================================================================
   $ otool -L lib/{libc++.1.dylib,libc++abi.1.dylib,libunwind.1.dylib}
   lib/libc++.1.dylib:
         @rpath/libc++.1.dylib
         @rpath/libc++abi.1.dylib
         /usr/lib/libSystem.B.dylib
   lib/libc++abi.1.dylib:
         @rpath/libc++abi.1.dylib
         /usr/lib/libSystem.B.dylib
   lib/libunwind.1.dylib:
         @rpath/libunwind.1.dylib
         /usr/lib/libSystem.B.dylib

   Before patch, with the LLVM unwinder (LIBCXXABI_USE_LLVM_UNWINDER = ON)
   =======================================================================
   $ otool -L lib/{libc++.1.dylib,libc++abi.1.dylib,libunwind.1.dylib}
   lib/libc++.1.dylib:
         @rpath/libc++.1.dylib
         /usr/lib/libSystem.B.dylib
         @rpath/libc++abi.1.dylib
         @rpath/libunwind.1.dylib
   lib/libc++abi.1.dylib:
         @rpath/libc++abi.1.dylib
         /usr/lib/libSystem.B.dylib
         @rpath/libunwind.1.dylib
   lib/libunwind.1.dylib:
         @rpath/libunwind.1.dylib
         /usr/lib/libSystem.B.dylib

   After patch, with the LLVM unwinder (LIBCXXABI_USE_LLVM_UNWINDER = ON)
   ======================================================================
   $ otool -L lib/{libc++.1.dylib,libc++abi.1.dylib,libunwind.1.dylib}
   lib/libc++.1.dylib:
         @rpath/libc++.1.dylib
         @rpath/libc++abi.1.dylib
         @rpath/libunwind.1.dylib
         /usr/lib/libSystem.B.dylib
   lib/libc++abi.1.dylib:
         @rpath/libc++abi.1.dylib
         @rpath/libunwind.1.dylib
         /usr/lib/libSystem.B.dylib
   lib/libunwind.1.dylib:
         @rpath/libunwind.1.dylib
         /usr/lib/libSystem.B.dylib

As we can see, libSystem appears before the just-built libraries before
the patch, which causes the libunwind.dylib bundled in libSystem.dylib
to be used instead of the just-built libunwind.dylib.

We didn't notice the issue until recently when I tried to update the
macOS CI builders to macOS 13.5, where it is necessary to use the
right libunwind library (the exact reason still needs to be investigated).
  • Loading branch information
@ldionne
ldionne committed Sep 20, 2023
1 parent e3b1662 commit 7d297dd
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 27 deletions.
28 changes: 13 additions & 15 deletions libcxx/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -659,24 +659,22 @@ function(cxx_link_system_libraries target)
target_add_link_flags_if_supported(${target} PRIVATE "--unwindlib=none")
endif()

if (LIBCXX_HAS_SYSTEM_LIB)
target_link_libraries(${target} PRIVATE System)
endif()

if (LIBCXX_HAS_PTHREAD_LIB)
target_link_libraries(${target} PRIVATE pthread)
endif()
if (NOT APPLE) # On Apple platforms, we always use -nostdlib++ so we don't need to re-add other libraries
if (LIBCXX_HAS_PTHREAD_LIB)
target_link_libraries(${target} PRIVATE pthread)
endif()

if (LIBCXX_HAS_C_LIB)
target_link_libraries(${target} PRIVATE c)
endif()
if (LIBCXX_HAS_C_LIB)
target_link_libraries(${target} PRIVATE c)
endif()

if (LIBCXX_HAS_M_LIB)
target_link_libraries(${target} PRIVATE m)
endif()
if (LIBCXX_HAS_M_LIB)
target_link_libraries(${target} PRIVATE m)
endif()

if (LIBCXX_HAS_RT_LIB)
target_link_libraries(${target} PRIVATE rt)
if (LIBCXX_HAS_RT_LIB)
target_link_libraries(${target} PRIVATE rt)
endif()
endif()

if (LIBCXX_USE_COMPILER_RT)
Expand Down
4 changes: 0 additions & 4 deletions libcxx/cmake/config-ix.cmake
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,10 +110,8 @@ if(WIN32 AND NOT MINGW)
set(LIBCXX_HAS_PTHREAD_LIB NO)
set(LIBCXX_HAS_M_LIB NO)
set(LIBCXX_HAS_RT_LIB NO)
set(LIBCXX_HAS_SYSTEM_LIB NO)
set(LIBCXX_HAS_ATOMIC_LIB NO)
elseif(APPLE)
check_library_exists(System write "" LIBCXX_HAS_SYSTEM_LIB)
set(LIBCXX_HAS_PTHREAD_LIB NO)
set(LIBCXX_HAS_M_LIB NO)
set(LIBCXX_HAS_RT_LIB NO)
Expand All @@ -122,12 +120,10 @@ elseif(FUCHSIA)
set(LIBCXX_HAS_M_LIB NO)
set(LIBCXX_HAS_PTHREAD_LIB NO)
set(LIBCXX_HAS_RT_LIB NO)
set(LIBCXX_HAS_SYSTEM_LIB NO)
check_library_exists(atomic __atomic_fetch_add_8 "" LIBCXX_HAS_ATOMIC_LIB)
else()
check_library_exists(pthread pthread_create "" LIBCXX_HAS_PTHREAD_LIB)
check_library_exists(m ccos "" LIBCXX_HAS_M_LIB)
check_library_exists(rt clock_gettime "" LIBCXX_HAS_RT_LIB)
set(LIBCXX_HAS_SYSTEM_LIB NO)
check_library_exists(atomic __atomic_fetch_add_8 "" LIBCXX_HAS_ATOMIC_LIB)
endif()
2 changes: 0 additions & 2 deletions libcxxabi/cmake/config-ix.cmake
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,11 +95,9 @@ if(FUCHSIA)
set(LIBCXXABI_HAS_PTHREAD_LIB NO)
check_library_exists(c __cxa_thread_atexit_impl ""
LIBCXXABI_HAS_CXA_THREAD_ATEXIT_IMPL)
set(LIBCXXABI_HAS_SYSTEM_LIB NO)
else()
check_library_exists(dl dladdr "" LIBCXXABI_HAS_DL_LIB)
check_library_exists(pthread pthread_once "" LIBCXXABI_HAS_PTHREAD_LIB)
check_library_exists(c __cxa_thread_atexit_impl ""
LIBCXXABI_HAS_CXA_THREAD_ATEXIT_IMPL)
check_library_exists(System write "" LIBCXXABI_HAS_SYSTEM_LIB)
endif()
4 changes: 1 addition & 3 deletions libcxxabi/src/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,7 @@ if (LIBCXXABI_ENABLE_FORGIVING_DYNAMIC_CAST)
add_definitions(-D_LIBCXXABI_FORGIVING_DYNAMIC_CAST)
endif()

if (APPLE)
add_library_flags_if(LIBCXXABI_HAS_SYSTEM_LIB System)
else()
if (NOT APPLE) # On Apple platforms, we always use -nostdlib++ so we don't need to re-add other libraries
if (LIBCXXABI_ENABLE_THREADS)
add_library_flags_if(LIBCXXABI_HAS_PTHREAD_LIB pthread)
endif()
Expand Down
13 changes: 10 additions & 3 deletions libunwind/src/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,16 +65,23 @@ set(LIBUNWIND_SOURCES
${LIBUNWIND_ASM_SOURCES})

# Generate library list.
add_library_flags_if(LIBUNWIND_HAS_C_LIB c)
if (LIBUNWIND_USE_COMPILER_RT)
add_library_flags("${LIBUNWIND_BUILTINS_LIBRARY}")
else()
add_library_flags_if(LIBUNWIND_HAS_GCC_S_LIB gcc_s)
add_library_flags_if(LIBUNWIND_HAS_GCC_LIB gcc)
endif()
add_library_flags_if(LIBUNWIND_HAS_DL_LIB dl)
if (NOT APPLE) # On Apple platforms, we don't need to link explicitly against system libraries
add_library_flags_if(LIBUNWIND_HAS_C_LIB c)
add_library_flags_if(LIBUNWIND_HAS_DL_LIB dl)

if (LIBUNWIND_ENABLE_THREADS)
add_library_flags_if(LIBUNWIND_HAS_PTHREAD_LIB pthread)
add_compile_flags_if(LIBUNWIND_WEAK_PTHREAD_LIB -DLIBUNWIND_USE_WEAK_PTHREAD=1)
endif()
endif()

if (LIBUNWIND_ENABLE_THREADS)
add_library_flags_if(LIBUNWIND_HAS_PTHREAD_LIB pthread)
add_compile_flags_if(LIBUNWIND_WEAK_PTHREAD_LIB -DLIBUNWIND_USE_WEAK_PTHREAD=1)
endif()

Expand Down

0 comments on commit 7d297dd

Please sign in to comment.