[libcxx] Make __shared_weak_count VTable consistent across all build configurations

[ldionne]

Summary

This patch ensures that __shared_weak_count provides a consistent vtable regardless of if RTTI is enabled or if we are targeting a static or shared libc++ build.

This patch is technically ABI breaking, but only for a very specific configuration that neither Apple nor FreeBSD should be shipping.

@dexonsmith @emaste can you confirm that you don't ship builds of libc++ that have both (A) RTTI disabled, and (B) Manually define _LIBCPP_BUILD_STATIC?

Note that _LIBCPP_BUILD_STATIC is not normally defined when building libc++.a, but instead it must be manually provided by the user or the __config_site.

[dexonsmith]

Breaking the ABI of that configuration SGTM.

src/memory.cpp

126–130 ↗

(On Diff #97746)

If this is the right change, seems like the 0/nullptr change can be a separate NFC pre-commit.

However, I wonder if this should call std::terminate or abort or something.

• Previously, if someone with RTTI accidentally linked against such a library, they'd get a link error in this case.
• Now, what happens?

[EricWF]

src/memory.cpp

126–130 ↗

(On Diff #97746)

Now, what happens?

That's a good question without an easy answer.

Previously, if someone with RTTI accidentally linked against such a library, they'd get a link error in this case.

Yep, that is true.

However if someone compiles a program with -fno-rtti -D_LIBCPP_BUILD_STATIC and links it to a library built w/ RTTI they will run into runtime errors and incorrect codegen since the vtable layout has silently changed. This is the problem I'm trying to fix.

Now, what happens?

After this change it still fails to link when the library is built w/o RTTI but the program is. This is caused by the missing typeinfo definition for __shared_weak_count.

I wonder if this should call std::terminate or abort or something.

This should never actually get called when programs are built with -fno-rtti because all of the call sites should be #ifndef _LIBCPP_NO_RTTI away, so there is no reason to abort or terminate.

[ldionne]

Rebase onto master

[ldionne]

Remove uses of _LIBCPP_BUILD_STATIC

[ldionne]

For libc++: We never manually define _LIBCPP_BUILD_STATIC, so libc++ always contains __shared_weak_count::__get_deleter.
For libc++abi: It could happen that _LIBCPP_BUILD_STATIC is defined through the CMake logic, however libc++abi always builds with RTTI enabled. So !defined(_LIBCPP_NO_RTTI) is always true anyway, and we always get the declaration of __shared_weak_count::__get_deleter.

I think this is a no-op change.

libcxx/include/memory

void __release_shared() _NOEXCEPT {
         __release_weak();
     }
 #endif
     void __release_weak() _NOEXCEPT;
     _LIBCPP_INLINE_VISIBILITY
     long use_count() const _NOEXCEPT {return __shared_count::use_count();}
     __shared_weak_count* lock() _NOEXCEPT;
 
-    // Define the function out only if we build static libc++ without RTTI.
-    // Otherwise we may break clients who need to compile their projects with
-    // -fno-rtti and yet link against a libc++.dylib compiled
-    // without -fno-rtti.
-#if !defined(_LIBCPP_NO_RTTI) || !defined(_LIBCPP_BUILD_STATIC)
     virtual const void* __get_deleter(const type_info&) const _NOEXCEPT;
-#endif
 private:
     virtual void __on_zero_shared_weak() _NOEXCEPT = 0;
 };
 
 template <class _Tp, class _Dp, class _Alloc>
 class __shared_ptr_pointer
     : public __shared_weak_count
 {

libcxx/src/memory.cpp

while (object_owners != -1)
         if (__libcpp_atomic_compare_exchange(&__shared_owners_,
                                              &object_owners,
                                              object_owners+1))
             return this;
     }
     return nullptr;
 }
 
-#if !defined(_LIBCPP_NO_RTTI) || !defined(_LIBCPP_BUILD_STATIC)
-
 const void*
 __shared_weak_count::__get_deleter(const type_info&) const _NOEXCEPT
 {
     return nullptr;
 }
 
-#endif  // _LIBCPP_NO_RTTI
-
 #if !defined(_LIBCPP_HAS_NO_ATOMIC_HEADER)
 
 _LIBCPP_SAFE_STATIC static const std::size_t __sp_mut_count = 16;
 _LIBCPP_SAFE_STATIC static __libcpp_mutex_t mut_back[__sp_mut_count] =
 {
     _LIBCPP_MUTEX_INITIALIZER, _LIBCPP_MUTEX_INITIALIZER, _LIBCPP_MUTEX_INITIALIZER, _LIBCPP_MUTEX_INITIALIZER,
     _LIBCPP_MUTEX_INITIALIZER, _LIBCPP_MUTEX_INITIALIZER, _LIBCPP_MUTEX_INITIALIZER, _LIBCPP_MUTEX_INITIALIZER,
     _LIBCPP_MUTEX_INITIALIZER, _LIBCPP_MUTEX_INITIALIZER, _LIBCPP_MUTEX_INITIALIZER, _LIBCPP_MUTEX_INITIALIZER,

libcxxabi/CMakeLists.txt

if (LIBCXXABI_ENABLE_ASSERTIONS)
   if (uppercase_CMAKE_BUILD_TYPE STREQUAL "RELEASE")
     list(APPEND LIBCXXABI_COMPILE_FLAGS -UNDEBUG)
   endif()
 else()
   if (NOT uppercase_CMAKE_BUILD_TYPE STREQUAL "RELEASE")
     list(APPEND LIBCXXABI_COMPILE_FLAGS -DNDEBUG)
   endif()
 endif()
-# Static library
-if (NOT LIBCXXABI_ENABLE_SHARED)
-  list(APPEND LIBCXXABI_COMPILE_FLAGS -D_LIBCPP_BUILD_STATIC)
-endif()
 
 # Threading
 if (NOT LIBCXXABI_ENABLE_THREADS)
   if (LIBCXXABI_HAS_PTHREAD_API)
     message(FATAL_ERROR "LIBCXXABI_HAS_PTHREAD_API can only"
                         " be set to ON when LIBCXXABI_ENABLE_THREADS"
                         " is also set to ON.")
   endif()