[libcxx-commits] [PATCH] D116689: [libunwind][libcxxabi] Use object libraries in the build

[Martin Storsjö via Phabricator via libcxx-commits]

mstorsjo added a comment.

In D116689#3498081 <https://reviews.llvm.org/D116689#3498081>, @phosek wrote:

> In D116689#3497481 <https://reviews.llvm.org/D116689#3497481>, @mstorsjo wrote:
>
>> @phosek Do you have time to pick this up and complete it? It'd be essential for a build cleanup for Windows - but the patch itself is a bit too far outside of my cmake-fu.
>
> I have uploaded an updated version which simplifies the implementation and seems to be working for me locally although I haven't done extensive testing yet.

This version breaks things for me; I'm building with `LIBCXX_ENABLE_STATIC_ABI_LIBRARY` and `LIBCXXABI_USE_LLVM_UNWINDER` but not `LIBCXXABI_STATICALLY_LINK_UNWINDER_IN_SHARED_LIBRARY`. In this case, there's no shared libcxxabi (I build with `LIBCXXABI_ENABLE_SHARED=OFF`), so libcxx would need to retain the code to link against the libunwind cmake target, it can't be carried transitively via libcxxabi (cxxabi_shared_objects). I.e. this case gets lost:

  if(LIBCXXABI_USE_LLVM_UNWINDER AND NOT LIBCXXABI_STATICALLY_LINK_UNWINDER_IN_SHARED_LIBRARY)
     target_link_libraries(cxxabi_shared PUBLIC unwind_shared)
   endif() 



>> For the mingw builds, the essential component is that `cxx_shared` uses `cxxabi_objects_shared`. However, the tricky thing is that `cxxabi_shared` isn't built (it's disabled, as it can't be linked as it has inverse dependencies on symbols provided by libcxx), but the `cxxabi_objects_shared` component still would need to be built).
>
> Is the current implementation sufficient for your needs?

No - to make it work, I currently apply this diff:

  diff --git a/libcxxabi/src/CMakeLists.txt b/libcxxabi/src/CMakeLists.txt
  index 52d9c8046f9a..c2cce23c25b2 100644
  --- a/libcxxabi/src/CMakeLists.txt
  +++ b/libcxxabi/src/CMakeLists.txt
  @@ -176,7 +176,7 @@ if (NOT TARGET pstl::ParallelSTL)
   endif()
   
   # Build the shared library.
  -if (LIBCXXABI_ENABLE_SHARED)
  +if (LIBCXXABI_ENABLE_SHARED OR ((LIBCXX_ENABLE_SHARED OR NOT DEFINED LIBCXX_ENABLE_SHARED) AND LIBCXX_ENABLE_STATIC_ABI_LIBRARY))
     add_library(cxxabi_shared_objects OBJECT ${LIBCXXABI_SOURCES} ${LIBCXXABI_HEADERS})
     target_link_libraries(cxxabi_shared_objects PRIVATE cxx-headers ${LIBCXXABI_SHARED_LIBRARIES} ${LIBCXXABI_LIBRARIES})
     target_link_libraries(cxxabi_shared_objects PUBLIC cxxabi-headers)
  @@ -188,7 +188,9 @@ if (LIBCXXABI_ENABLE_SHARED)
         COMPILE_FLAGS "${LIBCXXABI_COMPILE_FLAGS}"
         DEFINE_SYMBOL ""
     )
  +endif()
   
  +if (LIBCXXABI_ENABLE_SHARED)
     add_library(cxxabi_shared SHARED $<TARGET_OBJECTS:cxxabi_shared_objects>)
     set_target_properties(cxxabi_shared 
       PROPERTIES

I.e. I need to build `cxxabi_shared_objects` (as those are needed to be merged into `cxx_shared`) but I can't build `cxxabi_shared` (because that's impossible to link when Windows shared libraries can't have undefined references).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116689/new/

https://reviews.llvm.org/D116689