[libc++abi] Do not share an object library to create the static/shared libraries

[phosek]

Summary

This change is similar to r356150, with the same motivation. The
only difference is that the method used to merge libunwind.a and
libc++abi.a had to be changed to use the same approach as libc++
since we no longer produce object libraries that could be linked
together as we did before. We reuse the libc++ script for merging
archives to avoid duplication between the two projects.

[ldionne]

I will close https://reviews.llvm.org/D60166 as being superseded by this.

libcxxabi/CMakeLists.txt

49 ↗

(On Diff #193422)

Why are we dropping this?

[phosek]

libcxxabi/CMakeLists.txt

49 ↗

(On Diff #193422)

AFAICT CMake automatically includes -fPIC for both shared and static library (I checked the generated build.ninja files) so this shouldn't be needed, or do you want to have an explicit option to disable -fPIC? If so, should there be a similar option for libunwind and libc++ as well?

[ldionne]

libcxxabi/CMakeLists.txt

49 ↗

(On Diff #193422)

I don't think it does for static libraries. From https://cmake.org/cmake/help/v3.14/prop_tgt/POSITION_INDEPENDENT_CODE.html:

The POSITION_INDEPENDENT_CODE property determines whether position independent executables or shared libraries will be created. This property is True by default for SHARED and MODULE library targets and False otherwise. This property is initialized by the value of the CMAKE_POSITION_INDEPENDENT_CODE variable if it is set when a target is created.

I'm personally fine with relying on CMAKE_POSITION_INDEPENDENT_CODE to control that, but other folks wanted more control.

[phosek]

libcxxabi/CMakeLists.txt

49 ↗

(On Diff #193422)

It seems like the -fPIC I saw in build.ninja is added by LLVM, if I do a standalone build of libc++abi, it's not there.

[phosek]

libcxxabi/src/CMakeLists.txt

193 ↗

(On Diff #193546)

Related to discussion on D60049, should this be set for shared as well or only static? It's unclear to me whether that option should control both or just static? @sbc100 what was your intention?

Personally, I'd prefer to introduce:

cmake_dependent_option(LIBCXXABI_ENABLE_PIC_SHARED_LIBRARY "LIBCXXABI_ENABLE_SHARED;LIBCXXABI_ENABLE_PIC" ON)
cmake_dependent_option(LIBCXXABI_ENABLE_PIC_STATIC_LIBRARY "LIBCXXABI_ENABLE_STATIC;LIBCXXABI_ENABLE_PIC" ON)

Then use those dependent options to control PIC independently for shared and static libraries. WDYT?

[sbc100]

libcxxabi/src/CMakeLists.txt

193 ↗

(On Diff #193546)

From a WebAssembly perspective we only needed a way to prevent the static library being built as PIC. Which we now have in the form of the LIBCXXABI_ENABLE_PIC=OFF option.

During review somebody pointed out that there are times when one might also want to disable PIC in shared libraries too so we had this option effect both which is probably fine with us too.

I'm hoping to fix the issue that is currently present in WebAssembly where PIC code can't linked into static binaries. If I'm not able to do that I'll most likely end up building everything twice creating a complete fork of the lib directory. e.g:

• lib/wasm32-unknown-unknown/pic (includes .so files and .a files built with PIC)
• lib/wasm32-unknown-unknown/nopic (includes only .a files *all* built without PIC)

Then the linker would then choose the lib directory to use based weather its building relocatable output or not.

If we end up doing that we don't need that much flexibility within a given build because we will be running cmake twice, once for pic and once for non-pic.

So basically, no I need that fine grained control for what I'm doing (yet).

[phosek]

libcxxabi/src/CMakeLists.txt

193 ↗

(On Diff #193546)

I've changed it so that ${LIBCXXABI_ENABLE_PIC} controls both the static and shared case.

[ldionne]

libcxxabi/src/CMakeLists.txt

195 ↗

(On Diff #193554)

You need to do

if (LIBCXXABI_ENABLE_PIC)
  target_set_properties(... ON)
endif()

Otherwise, you're overwriting the default that's being set through CMAKE_POSITION_INDEPENDENT_CODE.

[phosek]

libcxxabi/src/CMakeLists.txt

195 ↗

(On Diff #193554)

@sbc100 mentioned in the other comment that some users may want to have a flexibility to also disable -fPIC even for the shared case where this won't be sufficient, but I'm fine to punt on that until it's needed.

[ldionne]

Thanks for the refactoring!

libcxxabi/src/CMakeLists.txt

195 ↗

(On Diff #193554)

I'm that user. I use CMAKE_POSITION_INDEPENDENT_CODE=OFF and I don't touch those stinking fine-grained controls.

libcxxabi/trunk/src/CMakeLists.txt

 split_list(LIBCXXABI_SHARED_LINK_FLAGS)
 
 # FIXME: libc++abi.so will not link when modules are enabled because it depends
 # on symbols defined in libc++.so which has not yet been built.
 if (LLVM_ENABLE_MODULES)
   string(REPLACE "-Wl,-z,defs" "" CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS}")
 endif()
 
-macro(cxxabi_object_library name)
-  cmake_parse_arguments(ARGS "" "" "DEFINES;FLAGS" ${ARGN})
-
-  # Add a object library that contains the compiled source files.
-  add_library(${name} OBJECT ${LIBCXXABI_SOURCES} ${LIBCXXABI_HEADERS})
-  set_target_properties(${name}
-                        PROPERTIES
-                          CXX_EXTENSIONS
-                            OFF
-                          CXX_STANDARD
-                            11
-                          CXX_STANDARD_REQUIRED
-                            ON
-                          COMPILE_FLAGS
-                            "${LIBCXXABI_COMPILE_FLAGS}")
-
-  if(LIBCXXABI_ENABLE_PIC)
-    set_target_properties(${name} PROPERTIES POSITION_INDEPENDENT_CODE ON)
-  endif()
-
-  if(ARGS_DEFINES)
-    target_compile_definitions(${name} PRIVATE ${ARGS_DEFINES})
-  endif()
-
-  if(ARGS_FLAGS)
-    target_compile_options(${name} PRIVATE ${ARGS_FLAGS})
-  endif()
-endmacro()
-
-if(LIBCXXABI_HERMETIC_STATIC_LIBRARY)
-  append_flags_if_supported(CXXABI_STATIC_OBJECTS_FLAGS -fvisibility=hidden)
-  append_flags_if_supported(CXXABI_STATIC_OBJECTS_FLAGS -fvisibility-global-new-delete-hidden)
-  cxxabi_object_library(cxxabi_static_objects
-    DEFINES
-      _LIBCXXABI_DISABLE_VISIBILITY_ANNOTATIONS
-      _LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS
-    FLAGS ${CXXABI_STATIC_OBJECTS_FLAGS})
-else()
-  cxxabi_object_library(cxxabi_static_objects)
-endif()
-
-cxxabi_object_library(cxxabi_shared_objects)
-
-set(cxxabi_static_sources $<TARGET_OBJECTS:cxxabi_static_objects>)
-set(cxxabi_shared_sources $<TARGET_OBJECTS:cxxabi_shared_objects>)
-
 # Build the shared library.
 if (LIBCXXABI_ENABLE_SHARED)
-  add_library(cxxabi_shared SHARED ${cxxabi_shared_sources})
+  add_library(cxxabi_shared SHARED ${LIBCXXABI_SOURCES} ${LIBCXXABI_HEADERS})
   if(COMMAND llvm_setup_rpath)
     llvm_setup_rpath(cxxabi_shared)
   endif()
   target_link_libraries(cxxabi_shared PRIVATE ${LIBCXXABI_SHARED_LIBRARIES} ${LIBCXXABI_LIBRARIES})
   set_target_properties(cxxabi_shared
                         PROPERTIES
                           CXX_EXTENSIONS
                             OFF
                           CXX_STANDARD
                             11
                           CXX_STANDARD_REQUIRED
                             ON
+                          COMPILE_FLAGS
+                            "${LIBCXXABI_COMPILE_FLAGS}"
                           LINK_FLAGS
                             "${LIBCXXABI_LINK_FLAGS} ${LIBCXXABI_SHARED_LINK_FLAGS}"
                           OUTPUT_NAME
                             "c++abi"
-                          POSITION_INDEPENDENT_CODE
-                            ON
                           SOVERSION
                             "1"
                           VERSION
                             "1.0")
+
+  if(LIBCXXABI_ENABLE_PIC)
+    set_target_properties(cxxabi_shared PROPERTIES POSITION_INDEPENDENT_CODE ON)
+  endif()
+
   list(APPEND LIBCXXABI_BUILD_TARGETS "cxxabi_shared")
   if (LIBCXXABI_INSTALL_SHARED_LIBRARY)
     list(APPEND LIBCXXABI_INSTALL_TARGETS "cxxabi_shared")
   endif()
 endif()
 
 # Build the static library.
 if (LIBCXXABI_ENABLE_STATIC)
-  if (LIBCXXABI_USE_LLVM_UNWINDER AND LIBCXXABI_STATICALLY_LINK_UNWINDER_IN_STATIC_LIBRARY)
-    if (TARGET unwind_static OR HAVE_LIBUNWIND)
-      if(LIBUNWIND_HERMETIC_STATIC_LIBRARY)
-        list(APPEND cxxabi_static_sources $<TARGET_OBJECTS:unwind_static_objects>)
-      else()
-        list(APPEND cxxabi_static_sources $<TARGET_OBJECTS:unwind_objects>)
-      endif()
-    endif()
-  endif()
-  add_library(cxxabi_static STATIC ${cxxabi_static_sources})
+  add_library(cxxabi_static STATIC ${LIBCXXABI_SOURCES} ${LIBCXXABI_HEADERS})
   target_link_libraries(cxxabi_static PRIVATE ${LIBCXXABI_STATIC_LIBRARIES} ${LIBCXXABI_LIBRARIES})
   set_target_properties(cxxabi_static
                         PROPERTIES
                           CXX_EXTENSIONS
                             OFF
                           CXX_STANDARD
                             11
                           CXX_STANDARD_REQUIRED
                             ON
+                          COMPILE_FLAGS
+                            "${LIBCXXABI_COMPILE_FLAGS}"
                           LINK_FLAGS
                             "${LIBCXXABI_LINK_FLAGS}"
                           OUTPUT_NAME
                             "c++abi")
+
+  if(LIBCXXABI_ENABLE_PIC)
+    set_target_properties(cxxabi_static PROPERTIES POSITION_INDEPENDENT_CODE ON)
+  endif()
+
+  if(LIBCXXABI_HERMETIC_STATIC_LIBRARY)
+    append_flags_if_supported(CXXABI_STATIC_LIBRARY_FLAGS -fvisibility=hidden)
+    append_flags_if_supported(CXXABI_STATIC_LIBRARY_FLAGS -fvisibility-global-new-delete-hidden)
+    target_compile_options(cxxabi_static PRIVATE ${CXXABI_STATIC_LIBRARY_FLAGS})
+    target_compile_definitions(cxxabi_static
+      PRIVATE
+        _LIBCXXABI_DISABLE_VISIBILITY_ANNOTATIONS
+        _LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS)
+  endif()
+
   list(APPEND LIBCXXABI_BUILD_TARGETS "cxxabi_static")
   if (LIBCXXABI_INSTALL_STATIC_LIBRARY)
     list(APPEND LIBCXXABI_INSTALL_TARGETS "cxxabi_static")
   endif()
+
+  # Merge the the libc++abi.a and libunwind.a into one.
+  if(LIBCXXABI_USE_LLVM_UNWINDER AND LIBCXXABI_STATICALLY_LINK_UNWINDER_IN_STATIC_LIBRARY)
+    add_custom_command(TARGET cxxabi_static POST_BUILD
+      COMMAND ${PYTHON_EXECUTABLE} ${LIBCXXABI_LIBCXX_PATH}/utils/merge_archives.py
+      ARGS
+        -o "$<TARGET_LINKER_FILE:cxxabi_static>"
+        --ar "${CMAKE_AR}"
+        "$<TARGET_LINKER_FILE:cxxabi_static>"
+        "$<TARGET_LINKER_FILE:unwind_static>"
+      WORKING_DIRECTORY ${LIBCXXABI_BUILD_DIR}
+    )
+  endif()
 endif()
 
 # Add a meta-target for both libraries.
 add_custom_target(cxxabi DEPENDS ${LIBCXXABI_BUILD_TARGETS})
 
 if (LIBCXXABI_INSTALL_LIBRARY)
   install(TARGETS ${LIBCXXABI_INSTALL_TARGETS}
     LIBRARY DESTINATION ${LIBCXXABI_INSTALL_PREFIX}lib${LIBCXXABI_LIBDIR_SUFFIX} COMPONENT cxxabi