[libcxx] Specify unwind lib before other system libraries.

[ychen]

Summary

For the same reason as D57496. This and D57496 combined make local unwind lib
picked up by dyld in a libcxx/libcxxabi/libunwind build on OSX.

before
➜ libcxx otool -L lib/libc++.dylib
lib/libc++.dylib:

@rpath/libc++.1.dylib (compatibility version 1.0.0, current version 1.0.0)
@rpath/libc++abi.1.dylib (compatibility version 1.0.0, current version 1.0.0)
/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1252.200.5)
@rpath/libunwind.1.dylib (compatibility version 1.0.0, current version 1.0.0)

to
➜ libcxx otool -L lib/libc++.dylib
lib/libc++.dylib:

@rpath/libc++.1.dylib (compatibility version 1.0.0, current version 1.0.0)
@rpath/libc++abi.1.dylib (compatibility version 1.0.0, current version 1.0.0)
@rpath/libunwind.1.dylib (compatibility version 1.0.0, current version 1.0.0)
/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1252.200.5)

[ychen]

• update

[phosek]

LGTM

[ychen]

Could you please commit this for me? Thanks.

[ldionne]

@phosek Has this been committed?

[phosek]

Not yet, but it'll probably need to be reworked given all the changes that landed in libc++.

[ychen]

rebase

[ldionne]

I'll admit that I find it very brittle that ordering matters. We can do this, however it would be better to find another solution that doesn't require depending on the ordering of -l parameters. I don't know what that would be, though.

[ychen]

In D57646#1490180, @ldionne wrote:

I'll admit that I find it very brittle that ordering matters. We can do this, however it would be better to find another solution that doesn't require depending on the ordering of -l parameters. I don't know what that would be, though.

install_name_tool -change could adjust the order for executable as a post build action. If there are additional dependent libraries, then reorder logic need to be updated. I'm not sure it would be an improvement than just put more comments in the existing code since overriding libSystem.B.dylib is a rare case.

[ldionne]

In D57646#1490202, @tabloid.adroit wrote:

In D57646#1490180, @ldionne wrote:

I'll admit that I find it very brittle that ordering matters. We can do this, however it would be better to find another solution that doesn't require depending on the ordering of -l parameters. I don't know what that would be, though.

install_name_tool -change could adjust the order for executable as a post build action. If there are additional dependent libraries, then reorder logic need to be updated. I'm not sure it would be an improvement than just put more comments in the existing code since overriding libSystem.B.dylib is a rare case.

innstall_name_tool is only available on Darwin unless I'm mistaken, so I think it's better not to add something that only works on a specific platform. It's an interesting suggestion nonetheless.

[ychen]

• add comment

[ychen]

Hi ldionne, I definitely agree install_name_tool is not a desired solution. I've also added some comments so people realize the issue when modifying the code.

[ychen]

ping.

[ldionne]

[Github PR transition cleanup]

Abandoning since this will be solved by https://github.com/llvm/llvm-project/pull/66940.

libcxx/src/CMakeLists.txt

 split_list(LIBCXX_LINK_FLAGS)
 
 # Build the shared library.
 if (LIBCXX_ENABLE_SHARED)
   add_library(cxx_shared SHARED ${exclude_from_all} ${LIBCXX_SOURCES} ${LIBCXX_HEADERS})
   if(COMMAND llvm_setup_rpath)
     llvm_setup_rpath(cxx_shared)
   endif()
-  cxx_link_system_libraries(cxx_shared)
-  target_link_libraries(cxx_shared PRIVATE ${LIBCXX_LIBRARIES})
-  set_target_properties(cxx_shared
-    PROPERTIES
-      COMPILE_FLAGS "${LIBCXX_COMPILE_FLAGS}"
-      LINK_FLAGS    "${LIBCXX_LINK_FLAGS}"
-      OUTPUT_NAME   "c++"
-      VERSION       "${LIBCXX_ABI_VERSION}.0"
-      SOVERSION     "${LIBCXX_ABI_VERSION}"
-      DEFINE_SYMBOL ""
-  )
-  cxx_set_common_defines(cxx_shared)
 
   # Link against LLVM libunwind
   if (LIBCXXABI_USE_LLVM_UNWINDER)
     if (NOT LIBCXXABI_STATICALLY_LINK_UNWINDER_IN_SHARED_LIBRARY AND (TARGET unwind_shared OR HAVE_LIBUNWIND))
       target_link_libraries(cxx_shared PUBLIC unwind_shared)
       list(APPEND LIBCXX_INTERFACE_LIBRARIES unwind_shared) # For the linker script
     elseif (LIBCXXABI_STATICALLY_LINK_UNWINDER_IN_SHARED_LIBRARY AND (TARGET unwind_static OR HAVE_LIBUNWIND))
       # libunwind is already included in libc++abi
if (LIBCXX_STATICALLY_LINK_ABI_IN_SHARED_LIBRARY)
     else()
       target_link_libraries(cxx_shared PRIVATE "-Wl,--whole-archive,-Bstatic" "${LIBCXX_CXX_STATIC_ABI_LIBRARY}" "-Wl,-Bdynamic,--no-whole-archive")
     endif()
   else()
     target_link_libraries(cxx_shared PUBLIC "${LIBCXX_CXX_SHARED_ABI_LIBRARY}")
     list(APPEND LIBCXX_INTERFACE_LIBRARIES "${LIBCXX_CXX_SHARED_ABI_LIBRARY}") # For the linker script
   endif()
 
+  # Link system libraries after libunwind & libc++abi so that
+  # local libunwind & libc++abi have higher precedence than their counterparts
+  # in system lib.
+  cxx_link_system_libraries(cxx_shared)
+  target_link_libraries(cxx_shared PRIVATE ${LIBCXX_LIBRARIES})
+  set_target_properties(cxx_shared
+    PROPERTIES
+      COMPILE_FLAGS "${LIBCXX_COMPILE_FLAGS}"
+      LINK_FLAGS    "${LIBCXX_LINK_FLAGS}"
+      OUTPUT_NAME   "c++"
+      VERSION       "${LIBCXX_ABI_VERSION}.0"
+      SOVERSION     "${LIBCXX_ABI_VERSION}"
+      DEFINE_SYMBOL ""
+  )
+  cxx_set_common_defines(cxx_shared)
+
   # Maybe re-export symbols from libc++abi
   if (APPLE AND (LIBCXX_CXX_ABI_LIBNAME STREQUAL "libcxxabi" OR
                  LIBCXX_CXX_ABI_LIBNAME STREQUAL "default")
             AND NOT DEFINED LIBCXX_OSX_REEXPORT_LIBCXXABI_SYMBOLS)
     set(LIBCXX_OSX_REEXPORT_LIBCXXABI_SYMBOLS ON)
   endif()
 
   if (LIBCXX_OSX_REEXPORT_LIBCXXABI_SYMBOLS)