[libcxx] Have separate abi library flags for static and shared builds

[martell]

Summary

While we can enable both LIBCXX_ENABLE_SHARED and LIBCXX_ENABLE_STATIC because we only build one set of objects this inadvertently ends up messing up the flags of one of the two builds.
This is because _LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS should be set for a static build but not for a shared build.

Also on that note static and shared builds need to be able to decide which type of abi library to link to independently so
I have introduced LIBCXX_ENABLE_STATIC_ABI_LIBRARY_FOR_STATIC and LIBCXX_ENABLE_STATIC_ABI_LIBRARY_FOR_SHARED
(better naming suggestions are also welcome)

The default is currently off for both but I would like to suggest we set LIBCXX_ENABLE_STATIC_ABI_LIBRARY_FOR_STATIC to ON by default.
I believe static builds by default should try to use a static libc++abi and combine them into one library.
Shared builds should remain using a shared libc++abi as the default.

This gives the power at the cmake level to override this and link shared with static and static with shared if desired.

I also cleaned up some small nits also.
llvm-ar exports COFF objects as .obj so the python script to combine the libs should allow not just .o
compiler-rt is added to the default libs in config-ix.cmake so we should not manually add -rtlib=compiler-rt to the link flags

[martell]

refactor commit to only intended changes

[martell]

@mstorsjo: I created this patch so we could build static and shared libcxx for windows in the one build.
This also addresses issues so that we can combine libcxxabi and libcxx into the one libcxx library using llvm-ar.

There is also one other patch for unwind support, while I understand exceptions in general and a little at the assembly level implementing that piece is currently outside my skillset.
I will add you as a reviewer there also, hopefully I can find time to address the comments from others over the weekend.

[mstorsjo]

@martell - I forgot to look at this patch - I've been experimenting with the same, with building libc++ and libc++abi for mingw now.

Full static builds run fine (requiring quite a bit of cmake fiddling though, and a few minor patches that I've sent separately now, that I see that you've fixed similarly more or less here, bundled up into the big change), but the shared builds fail with some symbols that aren't properly dllexported/imported across the border between libc++ and libc++abi. Did you run into the same issues, and are they fixed by doing what you suggest here (linking in libc++abi statically into libc++.dll)? To do this, did you need to build libc++abi with some extra cmake flags, like _LIBCPP_BUILDING_LIBRARY, to pretend it's all part of the same library build?

[martell]

update to HEAD

[martell]

@mstorsjo sorry I missed that comment before. I currently succeed in building shared and static versions by just changing
# define _LIBCPP_DLL_VIS __declspec(dllimport) to # define _LIBCPP_DLL_VIS in __config and applying D33620

The correct fix is probably by editing libcxxabi to correctly export symbols.
The real issue here though is when we are using a static libc++ clang needs to be able to set _LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS when using static at compile time which is messy.
I am avoiding that by doing that one line change above and the patch linked.

[mstorsjo]

In D33638#867523, @martell wrote:

@mstorsjo sorry I missed that comment before. I currently succeed in building shared and static versions by just changing
# define _LIBCPP_DLL_VIS __declspec(dllimport) to # define _LIBCPP_DLL_VIS in __config and applying D33620

The correct fix is probably by editing libcxxabi to correctly export symbols.
The real issue here though is when we are using a static libc++ clang needs to be able to set _LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS when using static at compile time which is messy.
I am avoiding that by doing that one line change above and the patch linked.

Hmm, maybe that's the same issue that I'm working around by passing -flto-visibility-public-std, which makes it skip the implicit dllimport for some implicitly defined C++ symbols.

[mstorsjo]

In D33638#867661, @mstorsjo wrote:

Hmm, maybe that's the same issue that I'm working around by passing -flto-visibility-public-std, which makes it skip the implicit dllimport for some implicitly defined C++ symbols.

Sorry, that's actually -Xclang -flto-visibility-public-std. In current testing (without building libcxx as a DLL), I'm building calling code with these flags:

-fno-exceptions -D_LIBCXXABI_DISABLE_VISIBILITY_ANNOTATIONS -D_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS -Xclang -flto-visibility-public-std

[martell]

In D33638#867665, @mstorsjo wrote:

In D33638#867661, @mstorsjo wrote:

Hmm, maybe that's the same issue that I'm working around by passing -flto-visibility-public-std, which makes it skip the implicit dllimport for some implicitly defined C++ symbols.

Sorry, that's actually -Xclang -flto-visibility-public-std. In current testing (without building libcxx as a DLL), I'm building calling code with these flags:

-fno-exceptions -D_LIBCXXABI_DISABLE_VISIBILITY_ANNOTATIONS -D_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS -Xclang -flto-visibility-public-std

Thanks for the update, I will try this out soon.

This patch resolves the issue surrounding the objects having to be built separately for static and dll builds when trying to do both in the one cmake run.
Let's try and get this reviewed and merged first and then dig back into why we have issues with dll import and -flto-visibility-public-std.

We can create a new thread on the mailing list and ask @compnerd to weigh in on what he thinks is best to do about that.

Hey, @EricWF, @rnk would wither of you be in a position to do a review of this :)

[martell]

utils/merge_archives.py

121

It seems this landed separately in rL313072 so I can remove this line.
Please ignore this line for the review

I don't think llvm-ar should output COFF objects as .obj just because they are COFF anyway.
It is a gnu style tool and should output all objects as .o

[mstorsjo]

utils/merge_archives.py

121

I don't think llvm-ar should output COFF objects as .obj just because they are COFF anyway.
It is a gnu style tool and should output all objects as .o

It's not something that llvm-ar decides, but when creating the static library, the input files are named *.obj (which is something that cmake decides, whenever the target is windows, regardless of whether using gnu style or msvc style tools).

[mstorsjo]

lib/CMakeLists.txt

108

This line isn't necessary any longer, something similar was committed in SVN r312498; see line 94 above.

CMakeLists.txt

if (LIBCXX_CXX_ABI STREQUAL "default")
   endif()
 else()
   set(LIBCXX_CXX_ABI_LIBNAME "${LIBCXX_CXX_ABI}")
 endif()
 
 # Use a static copy of the ABI library when linking libc++. This option
 # cannot be used with LIBCXX_ENABLE_ABI_LINKER_SCRIPT.
 option(LIBCXX_ENABLE_STATIC_ABI_LIBRARY "Statically link the ABI library" OFF)
+option(LIBCXX_ENABLE_STATIC_ABI_LIBRARY_FOR_STATIC "Statically link the ABI library for Static Build" ${LIBCXX_ENABLE_STATIC_ABI_LIBRARY})
+option(LIBCXX_ENABLE_STATIC_ABI_LIBRARY_FOR_SHARED "Statically link the ABI library for Shared Build" ${LIBCXX_ENABLE_STATIC_ABI_LIBRARY})
 
 # Generate and install a linker script inplace of libc++.so. The linker script
 # will link libc++ to the correct ABI library. This option is on by default
 # on UNIX platforms other than Apple unless 'LIBCXX_ENABLE_STATIC_ABI_LIBRARY'
 # is on. This option is also disabled when the ABI library is not specified
 # or is specified to be "none".
 set(ENABLE_LINKER_SCRIPT_DEFAULT_VALUE OFF)
-if (LLVM_HAVE_LINK_VERSION_SCRIPT AND NOT LIBCXX_ENABLE_STATIC_ABI_LIBRARY
+if (LLVM_HAVE_LINK_VERSION_SCRIPT AND NOT LIBCXX_ENABLE_STATIC_ABI_LIBRARY_FOR_SHARED
       AND NOT LIBCXX_CXX_ABI_LIBNAME STREQUAL "none"
       AND NOT LIBCXX_CXX_ABI_LIBNAME STREQUAL "default"
       AND PYTHONINTERP_FOUND
       AND LIBCXX_ENABLE_SHARED)
     set(ENABLE_LINKER_SCRIPT_DEFAULT_VALUE ON)
 endif()
 
 option(LIBCXX_ENABLE_ABI_LINKER_SCRIPT
-      "Use and install a linker script for the given ABI library"
+      "Use and install a linker script for the given ABI library for Shared Build"
       ${ENABLE_LINKER_SCRIPT_DEFAULT_VALUE})
 
 set(ENABLE_NEW_DELETE_DEFAULT ON)
 if (LIBCXXABI_ENABLE_NEW_DELETE_DEFINITIONS)
 # FIXME: This option should default to off. Unfortunatly GCC 4.9 fails to link
 # programs due to undefined references to new/delete in libc++abi so to work
 # around this libc++abi currently defaults LIBCXXABI_ENABLE_NEW_DELETE_DEFINITIONS
 # to ON. Once the GCC bug has been worked around this option should be changed
if (CMAKE_SIZEOF_VOID_P EQUAL 8 AND NOT WIN32)
   if (LIBCXX_BUILD_32_BITS AND NOT LLVM_BUILD_32_BITS) # Don't duplicate the output from LLVM
     message(STATUS "Building 32 bits executables and libraries.")
   endif()
 elseif(LIBCXX_BUILD_32_BITS)
   message(FATAL_ERROR "LIBCXX_BUILD_32_BITS=ON is not supported on this platform.")
 endif()
 
 # Check that this option is not enabled on Apple and emit a usage warning.
-if (LIBCXX_ENABLE_STATIC_ABI_LIBRARY)
+if (LIBCXX_ENABLE_STATIC_ABI_LIBRARY_FOR_STATIC OR LIBCXX_ENABLE_STATIC_ABI_LIBRARY_FOR_SHARED)
   if (APPLE)
-    message(FATAL_ERROR "LIBCXX_ENABLE_STATIC_ABI_LIBRARY is not supported on OS X")
+    message(FATAL_ERROR "LIBCXX_ENABLE_STATIC_ABI_LIBRARY options are not supported on OS X")
   else()
-    message(WARNING "LIBCXX_ENABLE_STATIC_ABI_LIBRARY is an experimental option")
+    message(WARNING "LIBCXX_ENABLE_STATIC_ABI_LIBRARY options are experimental")
   endif()
   if (LIBCXX_ENABLE_STATIC AND NOT PYTHONINTERP_FOUND)
     message(FATAL_ERROR "LIBCXX_ENABLE_STATIC_ABI_LIBRARY requires python but it was not found.")
   endif()
 endif()
 
 if (LIBCXX_ENABLE_ABI_LINKER_SCRIPT)
     if (APPLE)
       message(FATAL_ERROR "LIBCXX_ENABLE_ABI_LINKER_SCRIPT cannot be used on APPLE targets")
     endif()
     if (NOT PYTHONINTERP_FOUND)
       message(FATAL_ERROR "LIBCXX_ENABLE_ABI_LINKER_SCRIPT requires python but it was not found.")
     endif()
     if (NOT LIBCXX_ENABLE_SHARED)
       message(FATAL_ERROR "LIBCXX_ENABLE_ABI_LINKER_SCRIPT is only available for shared library builds.")
     endif()
 endif()
 
-if (LIBCXX_ENABLE_STATIC_ABI_LIBRARY AND LIBCXX_ENABLE_ABI_LINKER_SCRIPT)
+if (LIBCXX_ENABLE_STATIC_ABI_LIBRARY_FOR_SHARED AND LIBCXX_ENABLE_ABI_LINKER_SCRIPT)
     message(FATAL_ERROR "Conflicting options given.
-        LIBCXX_ENABLE_STATIC_ABI_LIBRARY cannot be specified with
+        LIBCXX_ENABLE_STATIC_ABI_LIBRARY_FOR_SHARED cannot be specified with
         LIBCXX_ENABLE_ABI_LINKER_SCRIPT")
 endif()
 
 if (LIBCXX_HAS_MUSL_LIBC AND NOT LIBCXX_INSTALL_SUPPORT_HEADERS)
   message(FATAL_ERROR "LIBCXX_INSTALL_SUPPORT_HEADERS can not be turned off"
                       "when building for Musl with LIBCXX_HAS_MUSL_LIBC.")
 endif()
 

 config_define_if_not(LIBCXX_ENABLE_MONOTONIC_CLOCK _LIBCPP_HAS_NO_MONOTONIC_CLOCK)
 config_define_if_not(LIBCXX_ENABLE_THREAD_UNSAFE_C_FUNCTIONS _LIBCPP_HAS_NO_THREAD_UNSAFE_C_FUNCTIONS)
 
 config_define_if(LIBCXX_HAS_PTHREAD_API _LIBCPP_HAS_THREAD_API_PTHREAD)
 config_define_if(LIBCXX_HAS_EXTERNAL_THREAD_API _LIBCPP_HAS_THREAD_API_EXTERNAL)
 config_define_if(LIBCXX_BUILD_EXTERNAL_THREAD_LIBRARY _LIBCPP_HAS_THREAD_LIBRARY_EXTERNAL)
 config_define_if(LIBCXX_HAS_MUSL_LIBC _LIBCPP_HAS_MUSL_LIBC)
 
-# By default libc++ on Windows expects to use a shared library, which requires
-# the headers to use DLL import/export semantics. However when building a
-# static library only we modify the headers to disable DLL import/export.
-if (DEFINED WIN32 AND LIBCXX_ENABLE_STATIC AND NOT LIBCXX_ENABLE_SHARED)
-  message(STATUS "Generating custom __config for non-DLL Windows build")
-  config_define(ON _LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS)
-endif()
-
 if (LIBCXX_NEEDS_SITE_CONFIG)
   configure_file("include/__config_site.in"
                  "${LIBCXX_BINARY_DIR}/__config_site"
                  @ONLY)
 
   # Provide the config definitions by included the generated __config_site
   # file at compile time.
   if("${CMAKE_CXX_COMPILER_ID}" STREQUAL "MSVC" OR "${CMAKE_CXX_SIMULATE_ID}" STREQUAL "MSVC")

lib/CMakeLists.txt

if (LIBCXXABI_USE_LLVM_UNWINDER)
   if (NOT LIBCXXABI_ENABLE_STATIC_UNWINDER AND (TARGET unwind_shared OR HAVE_LIBUNWIND))
     add_interface_library(unwind_shared)
   elseif (LIBCXXABI_ENABLE_STATIC_UNWINDER AND (TARGET unwind_static OR HAVE_LIBUNWIND))
     add_interface_library(unwind_static)
   else()
     add_interface_library(unwind)
   endif()
 endif()
+if (MINGW)
+  # MINGW_LIBRARIES is defined in config-ix.cmake
+  list(APPEND LIBCXX_LIBRARIES ${MINGW_LIBRARIES})
+endif()
 
 # Setup flags.
-if (NOT WIN32)
-  add_flags_if_supported(-fPIC)
-endif()
-
 add_link_flags_if_supported(-nodefaultlibs)
 
 if (LIBCXX_TARGETING_MSVC)
   if (LIBCXX_DEBUG_BUILD)
     set(LIB_SUFFIX "d")
   else()
     set(LIB_SUFFIX "")
   endif()
add_link_flags(
       "${OSX_RE_EXPORT_LINE}"
       "-Wl,-force_symbols_not_weak_list,${CMAKE_CURRENT_SOURCE_DIR}/notweak.exp"
       "-Wl,-force_symbols_weak_list,${CMAKE_CURRENT_SOURCE_DIR}/weak.exp")
   endif()
 endif()
 
 split_list(LIBCXX_COMPILE_FLAGS)
 split_list(LIBCXX_LINK_FLAGS)
+set(LIBCXX_TARGETS)
 
+set(LIBCXX_SHARED_COMPILE_FLAGS "")
+# Build the shared library.
+if (LIBCXX_ENABLE_SHARED)
   # Add an object library that contains the compiled source files.
-add_library(cxx_objects OBJECT ${exclude_from_all} ${LIBCXX_SOURCES} ${LIBCXX_HEADERS})
-if(WIN32 AND NOT MINGW)
-  target_compile_definitions(cxx_objects
+  add_library(cxx_shared_objects OBJECT ${exclude_from_all} ${LIBCXX_SOURCES} ${LIBCXX_HEADERS})
+  if(LIBCXX_ENABLE_STATIC_ABI_LIBRARY_FOR_SHARED)
+    set(LIBCXX_SHARED_COMPILE_FLAGS "-D_LIBCXXABI_DISABLE_VISIBILITY_ANNOTATIONS")
+  endif()
+  if(MSVC)
+    target_compile_definitions(cxx_shared_objects
                                PRIVATE
                                # Ignore the -MSC_VER mismatch, as we may build
                                # with a different compatibility version.
                                _ALLOW_MSC_VER_MISMATCH
                                # Don't check the msvcprt iterator debug levels
                                # as we will define the iterator types; libc++
                                # uses a different macro to identify the debug
                                # level.
                                _ALLOW_ITERATOR_DEBUG_LEVEL_MISMATCH
                                # We are building the c++ runtime, don't pull in
                                # msvcprt.
                                _CRTBLD
                                # Don't warn on the use of "deprecated"
                                # "insecure" functions which are standards
                                # specified.
                                _CRT_SECURE_NO_WARNINGS
                                # Use the ISO conforming behaviour for conversion
                                # in printf, scanf.
                                _CRT_STDIO_ISO_WIDE_SPECIFIERS)
   endif()
-
-set_target_properties(cxx_objects
+  set_target_properties(cxx_shared_objects
     PROPERTIES
-    COMPILE_FLAGS "${LIBCXX_COMPILE_FLAGS}"
+      COMPILE_FLAGS "${LIBCXX_COMPILE_FLAGS} ${LIBCXX_SHARED_COMPILE_FLAGS}"
   )
-
-set(LIBCXX_TARGETS)
-
-# Build the shared library.
-if (LIBCXX_ENABLE_SHARED)
-  add_library(cxx_shared SHARED $<TARGET_OBJECTS:cxx_objects>)
+  add_library(cxx_shared SHARED $<TARGET_OBJECTS:cxx_shared_objects>)
   target_link_libraries(cxx_shared ${LIBCXX_LIBRARIES})
   set_target_properties(cxx_shared
     PROPERTIES
       LINK_FLAGS    "${LIBCXX_LINK_FLAGS}"
       OUTPUT_NAME   "c++"
       VERSION       "${LIBCXX_ABI_VERSION}.0"
       SOVERSION     "${LIBCXX_ABI_VERSION}"
   )
   list(APPEND LIBCXX_TARGETS "cxx_shared")
   if(WIN32 AND NOT MINGW AND NOT "${CMAKE_HOST_SYSTEM_NAME}" STREQUAL "Windows")
     # Since we most likely do not have a mt.exe replacement, disable the
     # manifest bundling.  This allows a normal cmake invocation to pass which
     # will attempt to use the manifest tool to generate the bundled manifest
     set_target_properties(cxx_shared PROPERTIES
                           APPEND_STRING PROPERTY LINK_FLAGS " /MANIFEST:NO")
   endif()
 endif()
 
+set(LIBCXX_STATIC_COMPILE_FLAGS "")
 # Build the static library.
 if (LIBCXX_ENABLE_STATIC)
-  add_library(cxx_static STATIC $<TARGET_OBJECTS:cxx_objects>)
+  # Add an object library that contains the compiled source files.
+  add_library(cxx_static_objects OBJECT ${exclude_from_all} ${LIBCXX_SOURCES} ${LIBCXX_HEADERS})
+  list(APPEND LIBCXX_STATIC_COMPILE_FLAGS "-D_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS")
+  if(LIBCXX_ENABLE_STATIC_ABI_LIBRARY_FOR_STATIC)
+    list(APPEND LIBCXX_STATIC_COMPILE_FLAGS "-D_LIBCXXABI_DISABLE_VISIBILITY_ANNOTATIONS")
+  endif()
+  if(MSVC)
+    target_compile_definitions(cxx_static_objects
+                               PRIVATE
+                                 # Ignore the -MSC_VER mismatch, as we may build
+                                 # with a different compatibility version.
+                                 _ALLOW_MSC_VER_MISMATCH
+                                 # Don't check the msvcprt iterator debug levels
+                                 # as we will define the iterator types; libc++
+                                 # uses a different macro to identify the debug
+                                 # level.
+                                 _ALLOW_ITERATOR_DEBUG_LEVEL_MISMATCH
+                                 # We are building the c++ runtime, don't pull in
+                                 # msvcprt.
+                                 _CRTBLD
+                                 # Don't warn on the use of "deprecated"
+                                 # "insecure" functions which are standards
+                                 # specified.
+                                 _CRT_SECURE_NO_WARNINGS
+                                 # Use the ISO conforming behaviour for conversion
+                                 # in printf, scanf.
+                                 _CRT_STDIO_ISO_WIDE_SPECIFIERS)
+  endif()
+  split_list(LIBCXX_STATIC_COMPILE_FLAGS)
+  set_target_properties(cxx_static_objects
+    PROPERTIES
+      COMPILE_FLAGS "${LIBCXX_COMPILE_FLAGS} ${LIBCXX_STATIC_COMPILE_FLAGS}"
+  )
+  add_library(cxx_static STATIC $<TARGET_OBJECTS:cxx_static_objects>)
   target_link_libraries(cxx_static ${LIBCXX_LIBRARIES})
   set(CMAKE_STATIC_LIBRARY_PREFIX "lib")
   set_target_properties(cxx_static
     PROPERTIES
       LINK_FLAGS    "${LIBCXX_LINK_FLAGS}"
       OUTPUT_NAME   "c++"
   )
 
   list(APPEND LIBCXX_TARGETS "cxx_static")
   # Attempt to merge the libc++.a archive and the ABI library archive into one.
-  if (LIBCXX_ENABLE_STATIC_ABI_LIBRARY)
+  if (LIBCXX_ENABLE_STATIC_ABI_LIBRARY_FOR_STATIC)
     set(MERGE_ARCHIVES_SEARCH_PATHS "")
     if (LIBCXX_CXX_ABI_LIBRARY_PATH)
       set(MERGE_ARCHIVES_SEARCH_PATHS "-L${LIBCXX_CXX_ABI_LIBRARY_PATH}")
     endif()
     if ((TARGET ${LIBCXX_CXX_ABI_LIBRARY}) OR
         (${LIBCXX_CXX_ABI_LIBRARY} MATCHES "cxxabi(_static|_shared)?" AND HAVE_LIBCXXABI))
       set(MERGE_ARCHIVES_ABI_TARGET "$<TARGET_LINKER_FILE:${LIBCXX_CXX_ABI_LIBRARY}>")
     else()

utils/merge_archives.py

def main():
 
     global temp_directory_root
     temp_directory_root = tempfile.mkdtemp('.libcxx.merge.archives')
 
     for arc in archives:
         execute_command_verbose([ar_exe, '-x', arc], cwd=temp_directory_root,
                                 verbose=args.verbose)
 
-    files = glob.glob(os.path.join(temp_directory_root, '*.o'))
+    files = glob.glob(os.path.join(temp_directory_root, '*.o*'))
     if not files:
         print_and_exit('Failed to glob for %s' % glob_path)
     cmd = [ar_exe, '-qc', args.output] + files
     execute_command_verbose(cmd, cwd=temp_directory_root, verbose=args.verbose)
 
 
 if __name__ == '__main__':
     main()
     exit_with_cleanups(0)