[libcxxabi] Fix cmake order dependency wrt dllexporting

[ldionne]

Summary

If LIBCXX_ENABLE_SHARED isn't explicitly set on the cmake command
line, isn't set in the cache, and the libcxxabi project is configured
before libcxx, then LIBCXX_ENABLE_SHARED isn't defined yet. Once
the libcxx cmake project has been parsed, LIBCXX_ENABLE_SHARED would
have been set to its default value of ON.

This makes sure that the symbols are properly dllexported in such
a configuration scenario.

[ldionne]

Like I said in D120719, I am not fond of this approach because it preserves the circular dependency between libc++ and libc++abi. My primary goal is that libc++abi doesn't have to have too much knowledge about how libc++ is configured (ideally it would require no such knowledge).

To this end, we could either:

1. Ask Windows vendors to bite the bullet and start defining LIBCXXABI_HERMETIC_STATIC_LIBRARY manually, or
2. Set LIBCXXABI_HERMETIC_STATIC_LIBRARY to ON by default on Windows.

[mstorsjo]

In D120982#3359946, @ldionne wrote:

Like I said in D120719, I am not fond of this approach because it preserves the circular dependency between libc++ and libc++abi. My primary goal is that libc++abi doesn't have to have too much knowledge about how libc++ is configured (ideally it would require no such knowledge).

To this end, we could either:

1. Ask Windows vendors to bite the bullet and start defining LIBCXXABI_HERMETIC_STATIC_LIBRARY manually, or
2. Set LIBCXXABI_HERMETIC_STATIC_LIBRARY to ON by default on Windows.

I understand that you want to untangle the circular dependencies between the two libraries - I totally see the value in that. But I ask that we don't do it in a way that break users upfront without a deprecation period over a release to let users migrate to the new way of configuring the interdependencies of these libraries.

We can't blindly set the LIBCXXABI_HERMETIC_STATIC_LIBRARY to one value or another as that breaks either build config (but "breaking" the static build isn't visible in the CI setup). When building with LIBCXX_ENABLE_STATIC_ABI_LIBRARY=ON (which is essentially the only way that works to combine the libraries on Windows), LIBCXXABI_HERMETIC_STATIC_LIBRARY would have to be set to the negation of LIBCXX_ENABLE_SHARED. (Builds with shared+static get an incorrect version for the static library though - that's a longer standing issue I've planned to look into in the future.)

[ldionne]

After speaking offline with @mstorsjo , I suggest we go ahead with this patch, however we also add a configure-time warning and a release note saying that this behavior is deprecated and will be removed in LLVM 16.

libcxxabi/CMakeLists.txt

310

Here we can add a hard error if LIBCXXABI_HERMETIC_STATIC_LIBRARY=ON, since the two options would contradict one another.

313–318

Here we can add a warning if LIBCXXABI_HERMETIC_STATIC_LIBRARY=OFF explaining that this behavior is deprecated and will be removed in LLVM 16, asking users to set LIBCXXABI_HERMETIC_STATIC_LIBRARY=ON explicitly if they want this configuration.

[mstorsjo]

In D120982#3360146, @ldionne wrote:

After speaking offline with @mstorsjo , I suggest we go ahead with this patch, however we also add a configure-time warning and a release note saying that this behavior is deprecated and will be removed in LLVM 16.

That sounds like a reasonable timeplan to me, and the warning locations sound sensible to me, too. I'll try to amend the patch as suggested and test run it so that it behaves as expected.

[ldionne]

libcxxabi/CMakeLists.txt

310

Actually, we can't, because it would start erroring out if someone used LIBCXXABI_HERMETIC_STATIC_LIBRARY and the if() was satisfied above.

[ldionne]

Add warning and release note.

[ldionne]

I will land this if CI passes, since this is on the critical path for a couple other changes, and we've agreed on the direction.

[mstorsjo]

In D120982#3364775, @ldionne wrote:

I will land this if CI passes, since this is on the critical path for a couple other changes, and we've agreed on the direction.

Actually, I would want to request that we defer the warning and release note for a little while. (We need to have it in place by the time LLVM 15 is finalized; the sooner before that it is settled, the better of course, but I would request to hold off of starting the deprecation cycle for it a little while.)

I was thinking of how to implement the next step for the libcxxabi<->libcxx merging for Windows (it works, but it has a number of quirks right now), and one possible path forward that I see could require further changes in how to set things up. So before telling people to migrate their setup to a new form, I think it'd be nice to wait a little if there's a different final form for LLVM 15.

[mstorsjo]

In D120982#3364991, @mstorsjo wrote:

I was thinking of how to implement the next step for the libcxxabi<->libcxx merging for Windows (it works, but it has a number of quirks right now), and one possible path forward that I see could require further changes in how to set things up. So before telling people to migrate their setup to a new form, I think it'd be nice to wait a little if there's a different final form for LLVM 15.

... but I see the value of actually committing _something_ so it doesn't get forgotten. Do we have a list of things to add before the next branch so that it doesn't get lost in the unlikely event I don't manage to get back to it? Or commit those bits, commented out? Or just commit them as-is and then remove them later if the final solution ends up different?

[ldionne]

Comment out warning

[ldionne]

In D120982#3365025, @mstorsjo wrote:

In D120982#3364991, @mstorsjo wrote:

I was thinking of how to implement the next step for the libcxxabi<->libcxx merging for Windows (it works, but it has a number of quirks right now), and one possible path forward that I see could require further changes in how to set things up. So before telling people to migrate their setup to a new form, I think it'd be nice to wait a little if there's a different final form for LLVM 15.

... but I see the value of actually committing _something_ so it doesn't get forgotten. Do we have a list of things to add before the next branch so that it doesn't get lost in the unlikely event I don't manage to get back to it?

No, we don't. We generally try to avoid being in the situation where we need to remember to do something like that.

Or commit those bits, commented out? Or just commit them as-is and then remove them later if the final solution ends up different?

Erhm, I guess I can comment the warning out, however it would be nice to soon have clarity on what's the better way of doing this instead.

[ldionne]

I will land this now since it isn't different from what had passed CI originally.

libcxx/docs/ReleaseNotes.rst

 - Support for standalone builds have been entirely removed from libc++, libc++abi and
   libunwind. Please use :ref:`these instructions <build instructions>` for building
   libc++, libc++abi and/or libunwind.
 
 - The ``{LIBCXX,LIBCXXABI,LIBUNWIND}_TARGET_TRIPLE``, ``{LIBCXX,LIBCXXABI,LIBUNWIND}_SYSROOT`` and
   ``{LIBCXX,LIBCXXABI,LIBUNWIND}_GCC_TOOLCHAIN`` CMake variables have been removed. Instead, please
   use the ``CMAKE_CXX_COMPILER_TARGET``, ``CMAKE_SYSROOT`` and ``CMAKE_CXX_COMPILER_EXTERNAL_TOOLCHAIN``
   variables provided by CMake.
+
+- When building for Windows, vendors who want to avoid dll-exporting symbols from the static libc++abi
+  library should set ``LIBCXXABI_HERMETIC_STATIC_LIBRARY=ON`` when configuring CMake. The current
+  behavior, which tries to guess the correct dll-export semantics based on whether we're building
+  the libc++ shared library, will be removed in LLVM 16.

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)
+  # If LIBCXX_ENABLE_SHARED isn't set (by the user on the cmake command
+  # line or via a cache file), use its expected default value (enabled).
+  if ((LIBCXX_ENABLE_SHARED OR NOT DEFINED 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)
+    if (NOT LIBCXXABI_HERMETIC_STATIC_LIBRARY)
+      # TODO: Enable this warning message as soon as we're sure this is the solution.
+      # message(WARNING "Implicitly disabling dllexport on Win32 is not supported anymore. Please build with "
+      #                 "LIBCXXABI_HERMETIC_STATIC_LIBRARY=ON instead. This will become an error in LLVM 16.")
+    endif()
   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)