[libcxx] [libcxxabi] Set flags for visibility when statically linking libcxxabi into libcxx for windows

[mstorsjo]

Summary

Visibility attributes on windows (dllexport, dllimport) don't only have an effect when compiling the definitions of symbols, but also affect how undefined references to symbols behave (expecting them to live in the same DLL or not).

If we are building libcxxabi statically, with the intent to bundle it into the built libcxx (which is the only form that really works on windows, due to their interdependencies), we should have it flag its symbols as if it was being built in the same way as libcxx.

I.e., if libcxx is being built shared and libcxxabi is built static (to be linked into libcxx), libcxxabi should include dllexport on its symbols and don't import symbols from libcxx with dllimport as they are going to reside in the same DLL. And if building a static libcxx and going to bundle libcxxabi into it, it needs to access cxxabi symbols without dllimport attributes. (libcxxabi doesn't add #define _LIBCXXABI_DISABLE_VISIBILITY_ANNOTATIONS to any header like libcxx does, if configured for a static-only build.)

[mati865]

Sorry for the delay.
LGTM from MinGW point of view.

[ldionne]

libcxx/CMakeLists.txt

879

Why does _LIBCXXABI_BUILDING_LIBRARY have an impact when building libc++?

libcxxabi/CMakeLists.txt

297

IMO, the right thing to do here is to get rid of the places where we #define _LIBCPP_BUILDING_LIBRARY in libcxxabi, and define _LIBCPP_BUILDING_LIBRARY unconditionally when building libc++abi. Then, as a follow-up change, we should clarify the status of these source files that are shared/duplicated between libc++ and libc++abi (that's stdlib_new_delete.cpp and stdlib_exception.cpp AFAICT).

[rnk]

I read the commit message, and what you said makes sense to me. As I understand it, there are two supported configs on Windows:

• Shared: libc++ and libc++abi as one DLL.
• Static: both libc++ and libc++abi are static libraries.

Is it possible to simplify the cmake conditionals if we add some checks for that?

[mstorsjo]

In D90021#2354009, @rnk wrote:

I read the commit message, and what you said makes sense to me. As I understand it, there are two supported configs on Windows:

• Shared: libc++ and libc++abi as one DLL.
• Static: both libc++ and libc++abi are static libraries.

... with an option in libcxx to merge the static libc++abi into the static libc++

But yeah, if using both libraries together, this pretty much are the only working combinations. Additionally, there's also the option to use libc++ standalone on top of MSVC's ABI runtime, both as static and dynamic library.

Is it possible to simplify the cmake conditionals if we add some checks for that?

I think that might be possible yeah, but not sure how big variance there is in how this all is hooked up, as most existing setups probably have done more or less of adhoc hackery to get it to work. (My builds so far amounted to building them each standalone, by passing these flags manually with CMAKE_CXX_FLAGS.)

libcxx/CMakeLists.txt

879

In practice, _LIBCXXABI_DISABLE_VISIBILITY_ANNOTATIONS works essentially the same. If there's inline definitions in headers that would need to be exported, I think _LIBCXXABI_BUILDING_LIBRARY would be a bit more consistent (as the statically linked libcxxabi would essentially be a part of the final libc++ DLL), but I'm not sure it matters.

libcxxabi/CMakeLists.txt

297

fallback_malloc.cpp also has an instance of it.

[mstorsjo]

libcxx/CMakeLists.txt

879

Phrasing it differently: If nothing is defined, any reference to symbols provided by libcxxabi, in code generated within libcxx, is generated as if it is going to be imported from a different DLL. If _LIBCXXABI_DISABLE_VISIBILITY_ANNOTATIONS is defined, the references are as if the code is going to be within the same DLL (which is true).

If _LIBCXXABI_BUILDING_LIBRARY is defined, any reference to the symbols are as if they are in the same DLL (same as with _LIBCXXABI_DISABLE_VISIBILITY_ANNOTATIONS), but any time such a symbol is defined (which only would happen for inline functions in headers), it also generates an annotation in the object file, saying that this particular symbol should be exported out from the DLL.

[ldionne]

libcxx/CMakeLists.txt

879

My point was that _LIBCXXABI_BUILDING_LIBRARY is not used anywhere in libc++, except when checking whether to enable availability attributes on Apple (which is unrelated to what you're trying to achieve). So adding this definition is effectively a no-op. Am I mistaken?

libcxxabi/CMakeLists.txt

297

Right. I would still prefer that we go down the route I laid out above, since it simplifies things in addition to making them work for your use case.

[mstorsjo]

libcxx/CMakeLists.txt

879

The libc++ code itself doesn't check _LIBCXXABI_BUILDING_LIBRARY, but it does affect what _LIBCXXABI_FUNC_VIS expands to, in the libcxxabi headers that are included by the libc++ sources.

If nothing is defined, _LIBCXXABI_FUNC_VIS expands to __declspec(dllimport), which makes the generated libc++ code reference e.g. __imp___cxa_uncaught_exceptions instead of __cxa_uncaught_exceptions directly. With _LIBCXXABI_BUILDING_LIBRARY defined, it expands to __declspec(dllexport) instead, and the generated code references __cxa_uncaught_exceptions, likewise with _LIBCXXABI_DISABLE_VISIBILITY_ANNOTATIONS.

libcxxabi/CMakeLists.txt

297

Ok, I'll give that a shot.

[mstorsjo]

Updated after landing D90476, removed nuances that don't have any practical effect (distinguishing between defining _LIBCXXABI_BUILDING_LIBRARY and _LIBCXXABI_DISABLE_VISIBILITY_ANNOTATIONS within libcxx), ending up with a fairly simple form.

libcxx/CMakeLists.txt

 # 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 (WIN32 AND LIBCXX_ENABLE_STATIC_ABI_LIBRARY)
+  # If linking libcxxabi statically into libcxx, skip the dllimport attributes
+  # on symbols we refer to from libcxxabi.
+  add_definitions(-D_LIBCXXABI_DISABLE_VISIBILITY_ANNOTATIONS)
+endif()
+
 # Setup all common build flags =================================================
 function(cxx_add_common_build_flags target)
   cxx_add_basic_build_flags(${target})
   cxx_add_warning_flags(${target})
   cxx_add_windows_flags(${target})
   cxx_add_exception_flags(${target})
   cxx_add_rtti_flags(${target})
   cxx_add_module_flags(${target})

libcxxabi/CMakeLists.txt

 add_definitions(-D_LIBCXXABI_BUILDING_LIBRARY)
 
 # libcxxabi needs to, for various reasons, include the libcpp headers as if
 # it is being built as part of libcxx.
 add_definitions(-D_LIBCPP_BUILDING_LIBRARY)
 
 # Disable DLL annotations on Windows for static builds.
 if (WIN32 AND LIBCXXABI_ENABLE_STATIC AND NOT LIBCXXABI_ENABLE_SHARED)
+  if (LIBCXX_ENABLE_SHARED AND LIBCXX_ENABLE_STATIC_ABI_LIBRARY)
+    # Building libcxxabi statically, but intending for it to be statically
+    # linked into a shared libcxx; keep dllexport enabled within libcxxabi,
+    # as the symbols will need to be exported from libcxx.
+  else()
+    # Regular static build; disable dllexports.
     add_definitions(-D_LIBCXXABI_DISABLE_VISIBILITY_ANNOTATIONS)
   endif()
+endif()
 
 add_compile_flags_if_supported(-Werror=return-type)
 
 # Get warning flags
 add_compile_flags_if_supported(-W)
 add_compile_flags_if_supported(-Wall)
 add_compile_flags_if_supported(-Wchar-subscripts)
 add_compile_flags_if_supported(-Wconversion)