<div dir="auto"><div dir="auto"><br></div><div dir="auto">ABI v2 isnt stable yet. Its just a staging area.</div><div dir="auto">It appears we need to communicate this more clearly.</div><div dir="auto"><br></div><div dir="auto">/Eric</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed., Jul. 10, 2019, 12:48 a.m. Kristina Brooks, <<a href="mailto:notstina@gmail.com">notstina@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Had the same breakage (LIBCPP_ABI=2), currently using a hack to<br>
provide backwards compatibility to anything that imported it (by just<br>
exporting a dummy symbol so dynamic linking works). I think I<br>
mentioned this on IRC to Eric, not sure if he noticed.<br>
<br>
< kristina_> EricWF: for _LIBCPP_HAS_TRIVIAL_MUTEX_DESTRUCTION is it<br>
possible to provide a symbol for backwards compat inside libc++ like<br>
you did with condvar?<br>
< kristina_> somehow this worked before though - #if<br>
_LIBCPP_ABI_VERSION == 1 ||<br>
!defined(_LIBCPP_HAS_TRIVIAL_MUTEX_DESTRUCTION)<br>
< kristina_> at _LIBCPP_ABI_VERSION=2<br>
<br>
It would be really nice to have it exported just for compatibility in<br>
upstream libc++ without needing local patches, although I'm not sure<br>
how libc++ maintainers feel about that.<br>
<br>
Thank you.<br>
<br>
On Tue, Jul 9, 2019 at 12:09 AM Petr Hosek via libcxx-commits<br>
<<a href="mailto:libcxx-commits@lists.llvm.org" target="_blank" rel="noreferrer">libcxx-commits@lists.llvm.org</a>> wrote:<br>
><br>
> FYI we've seen ABI breakage in our build:<br>
><br>
> ld.lld: error: undefined symbol: std::__2::mutex::~mutex()<br>
> >>> referenced by ErrorHandling.cpp:60 (/b/s/w/ir/k/llvm-project/llvm/lib/Support/ErrorHandling.cpp:60)<br>
> >>>               ErrorHandling.cpp.o:(_GLOBAL__sub_I_ErrorHandling.cpp) in archive ../../garnet/third_party/llvm/lib/libLLVMSupport.a<br>
><br>
> I'm not sure if it's related to our use of ABI v2? It's something we can handle by rolling out a new version of LLVM prebuilt, but I thought you may want to be aware.<br>
><br>
> On Sat, Jul 6, 2019 at 6:21 PM Eric Fiselier via libcxx-commits <<a href="mailto:libcxx-commits@lists.llvm.org" target="_blank" rel="noreferrer">libcxx-commits@lists.llvm.org</a>> wrote:<br>
>><br>
>> Author: ericwf<br>
>> Date: Sat Jul  6 18:20:54 2019<br>
>> New Revision: 365273<br>
>><br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=365273&view=rev" rel="noreferrer noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=365273&view=rev</a><br>
>> Log:<br>
>> Fix PR27658 - Make ~mutex trivial when possible.<br>
>><br>
>> Currently std::mutex has a constexpr constructor, but a non-trivial<br>
>> destruction.<br>
>><br>
>> The constexpr constructor is required to ensure the construction of a<br>
>> mutex with static storage duration happens at compile time, during<br>
>> constant initialization, and not during dynamic initialization.<br>
>> This means that static mutex's are always initialized and can be used<br>
>> safely during dynamic initialization without the "static initialization<br>
>> order fiasco".<br>
>><br>
>> A trivial destructor is important for similar reasons. If a mutex is<br>
>> used during dynamic initialization it might also be used during program<br>
>> termination. If a static mutex has a non-trivial destructor it will be<br>
>> invoked during termination. This can introduce the "static<br>
>> deinitialization order fiasco".<br>
>><br>
>> Additionally, function-local statics emit a guard variable around<br>
>> non-trivially destructible types. This results in horrible codegen and<br>
>> adds a runtime cost to every call to that function. non-local static's<br>
>> also result in slightly worse codegen but it's not as big of a problem.<br>
>><br>
>> Example codegen can be found here: <a href="https://goo.gl/3CSzbM" rel="noreferrer noreferrer" target="_blank">https://goo.gl/3CSzbM</a><br>
>><br>
>> Note: This optimization is not safe with every pthread implementation.<br>
>> Some implementations allocate on the first call to pthread_mutex_lock<br>
>> and free the allocation in pthread_mutex_destroy.<br>
>><br>
>> Also, changing the triviality of the destructor is not an ABI break.<br>
>> At least to the best of my knowledge :-)<br>
>><br>
>> Added:<br>
>>     libcxx/trunk/src/mutex_destructor.cpp<br>
>> Modified:<br>
>>     libcxx/trunk/include/__config<br>
>>     libcxx/trunk/include/__mutex_base<br>
>>     libcxx/trunk/src/CMakeLists.txt<br>
>>     libcxx/trunk/src/mutex.cpp<br>
>>     libcxx/trunk/test/std/thread/thread.mutex/thread.mutex.requirements/thread.mutex.requirements.mutex/thread.mutex.class/default.pass.cpp<br>
>><br>
>> Modified: libcxx/trunk/include/__config<br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/__config?rev=365273&r1=365272&r2=365273&view=diff" rel="noreferrer noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/__config?rev=365273&r1=365272&r2=365273&view=diff</a><br>
>> ==============================================================================<br>
>> --- libcxx/trunk/include/__config (original)<br>
>> +++ libcxx/trunk/include/__config Sat Jul  6 18:20:54 2019<br>
>> @@ -1097,6 +1097,14 @@ _LIBCPP_FUNC_VIS extern "C" void __sanit<br>
>>         _LIBCPP_HAS_NO_THREADS is defined.<br>
>>  #endif<br>
>><br>
>> +// The Apple, glibc, and Bionic implementation of pthreads implements<br>
>> +// pthread_mutex_destroy as nop for regular mutexes.<br>
>> +// TODO(EricWF): Enable this optimization on Apple and Bionic platforms after<br>
>> +// speaking to their respective stakeholders.<br>
>> +#if defined(_LIBCPP_HAS_THREAD_API_PTHREAD) && defined(__GLIBC__)<br>
>> +# define _LIBCPP_HAS_TRIVIAL_MUTEX_DESTRUCTION<br>
>> +#endif<br>
>> +<br>
>>  // Systems that use capability-based security (FreeBSD with Capsicum,<br>
>>  // Nuxi CloudABI) may only provide local filesystem access (using *at()).<br>
>>  // Functions like open(), rename(), unlink() and stat() should not be<br>
>><br>
>> Modified: libcxx/trunk/include/__mutex_base<br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/__mutex_base?rev=365273&r1=365272&r2=365273&view=diff" rel="noreferrer noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/__mutex_base?rev=365273&r1=365272&r2=365273&view=diff</a><br>
>> ==============================================================================<br>
>> --- libcxx/trunk/include/__mutex_base (original)<br>
>> +++ libcxx/trunk/include/__mutex_base Sat Jul  6 18:20:54 2019<br>
>> @@ -36,28 +36,24 @@ _LIBCPP_BEGIN_NAMESPACE_STD<br>
>>  #  endif<br>
>>  #endif  // _LIBCPP_THREAD_SAFETY_ANNOTATION<br>
>><br>
>> +<br>
>>  class _LIBCPP_TYPE_VIS _LIBCPP_THREAD_SAFETY_ANNOTATION(capability("mutex")) mutex<br>
>>  {<br>
>> -#ifndef _LIBCPP_CXX03_LANG<br>
>>      __libcpp_mutex_t __m_ = _LIBCPP_MUTEX_INITIALIZER;<br>
>> -#else<br>
>> -    __libcpp_mutex_t __m_;<br>
>> -#endif<br>
>><br>
>>  public:<br>
>>      _LIBCPP_INLINE_VISIBILITY<br>
>> -#ifndef _LIBCPP_CXX03_LANG<br>
>> -    constexpr mutex() = default;<br>
>> +    _LIBCPP_CONSTEXPR mutex() = default;<br>
>> +<br>
>> +    mutex(const mutex&) = delete;<br>
>> +    mutex& operator=(const mutex&) = delete;<br>
>> +<br>
>> +#if defined(_LIBCPP_HAS_TRIVIAL_MUTEX_DESTRUCTION)<br>
>> +    ~mutex() = default;<br>
>>  #else<br>
>> -    mutex() _NOEXCEPT {__m_ = (__libcpp_mutex_t)_LIBCPP_MUTEX_INITIALIZER;}<br>
>> -#endif<br>
>>      ~mutex() _NOEXCEPT;<br>
>> +#endif<br>
>><br>
>> -private:<br>
>> -    mutex(const mutex&);// = delete;<br>
>> -    mutex& operator=(const mutex&);// = delete;<br>
>> -<br>
>> -public:<br>
>>      void lock() _LIBCPP_THREAD_SAFETY_ANNOTATION(acquire_capability());<br>
>>      bool try_lock() _NOEXCEPT _LIBCPP_THREAD_SAFETY_ANNOTATION(try_acquire_capability(true));<br>
>>      void unlock() _NOEXCEPT _LIBCPP_THREAD_SAFETY_ANNOTATION(release_capability());<br>
>><br>
>> Modified: libcxx/trunk/src/CMakeLists.txt<br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/CMakeLists.txt?rev=365273&r1=365272&r2=365273&view=diff" rel="noreferrer noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/CMakeLists.txt?rev=365273&r1=365272&r2=365273&view=diff</a><br>
>> ==============================================================================<br>
>> --- libcxx/trunk/src/CMakeLists.txt (original)<br>
>> +++ libcxx/trunk/src/CMakeLists.txt Sat Jul  6 18:20:54 2019<br>
>> @@ -22,6 +22,7 @@ set(LIBCXX_SOURCES<br>
>>    locale.cpp<br>
>>    memory.cpp<br>
>>    mutex.cpp<br>
>> +  mutex_destructor.cpp<br>
>>    new.cpp<br>
>>    optional.cpp<br>
>>    random.cpp<br>
>><br>
>> Modified: libcxx/trunk/src/mutex.cpp<br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/mutex.cpp?rev=365273&r1=365272&r2=365273&view=diff" rel="noreferrer noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/mutex.cpp?rev=365273&r1=365272&r2=365273&view=diff</a><br>
>> ==============================================================================<br>
>> --- libcxx/trunk/src/mutex.cpp (original)<br>
>> +++ libcxx/trunk/src/mutex.cpp Sat Jul  6 18:20:54 2019<br>
>> @@ -25,10 +25,7 @@ const defer_lock_t  defer_lock = {};<br>
>>  const try_to_lock_t try_to_lock = {};<br>
>>  const adopt_lock_t  adopt_lock = {};<br>
>><br>
>> -mutex::~mutex() _NOEXCEPT<br>
>> -{<br>
>> -    __libcpp_mutex_destroy(&__m_);<br>
>> -}<br>
>> +// ~mutex is defined elsewhere<br>
>><br>
>>  void<br>
>>  mutex::lock()<br>
>><br>
>> Added: libcxx/trunk/src/mutex_destructor.cpp<br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/mutex_destructor.cpp?rev=365273&view=auto" rel="noreferrer noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/mutex_destructor.cpp?rev=365273&view=auto</a><br>
>> ==============================================================================<br>
>> --- libcxx/trunk/src/mutex_destructor.cpp (added)<br>
>> +++ libcxx/trunk/src/mutex_destructor.cpp Sat Jul  6 18:20:54 2019<br>
>> @@ -0,0 +1,51 @@<br>
>> +//===--------------------- mutex_destructor.cpp ---------------------------===//<br>
>> +//<br>
>> +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.<br>
>> +// See <a href="https://llvm.org/LICENSE.txt" rel="noreferrer noreferrer" target="_blank">https://llvm.org/LICENSE.txt</a> for license information.<br>
>> +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception<br>
>> +//<br>
>> +//===----------------------------------------------------------------------===//<br>
>> +<br>
>> +// Define ~mutex.<br>
>> +//<br>
>> +// On some platforms ~mutex has been made trivial and the definition is only<br>
>> +// provided for ABI compatibility.<br>
>> +//<br>
>> +// In order to avoid ODR violations within libc++ itself, we need to ensure<br>
>> +// that *nothing* sees the non-trivial mutex declaration. For this reason<br>
>> +// we re-declare the entire class in this file instead of using<br>
>> +// _LIBCPP_BUILDING_LIBRARY to change the definition in the headers.<br>
>> +<br>
>> +#include "__config"<br>
>> +#include "__threading_support"<br>
>> +<br>
>> +#if !defined(_LIBCPP_HAS_NO_THREADS)<br>
>> +#if _LIBCPP_ABI_VERSION == 1 || !defined(_LIBCPP_HAS_TRIVIAL_MUTEX_DESTRUCTION)<br>
>> +#define NEEDS_MUTEX_DESTRUCTOR<br>
>> +#endif<br>
>> +#endif<br>
>> +<br>
>> +_LIBCPP_BEGIN_NAMESPACE_STD<br>
>> +<br>
>> +#ifdef NEEDS_MUTEX_DESTRUCTOR<br>
>> +class _LIBCPP_TYPE_VIS mutex<br>
>> +{<br>
>> +    __libcpp_mutex_t __m_ = _LIBCPP_MUTEX_INITIALIZER;<br>
>> +<br>
>> +public:<br>
>> +    _LIBCPP_ALWAYS_INLINE _LIBCPP_INLINE_VISIBILITY<br>
>> +    constexpr mutex() = default;<br>
>> +    mutex(const mutex&) = delete;<br>
>> +    mutex& operator=(const mutex&) = delete;<br>
>> +    ~mutex() noexcept;<br>
>> +};<br>
>> +<br>
>> +<br>
>> +mutex::~mutex() _NOEXCEPT<br>
>> +{<br>
>> +    __libcpp_mutex_destroy(&__m_);<br>
>> +}<br>
>> +<br>
>> +#endif // !_LIBCPP_HAS_NO_THREADS<br>
>> +_LIBCPP_END_NAMESPACE_STD<br>
>> +<br>
>><br>
>> Modified: libcxx/trunk/test/std/thread/thread.mutex/thread.mutex.requirements/thread.mutex.requirements.mutex/thread.mutex.class/default.pass.cpp<br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/thread/thread.mutex/thread.mutex.requirements/thread.mutex.requirements.mutex/thread.mutex.class/default.pass.cpp?rev=365273&r1=365272&r2=365273&view=diff" rel="noreferrer noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/thread/thread.mutex/thread.mutex.requirements/thread.mutex.requirements.mutex/thread.mutex.class/default.pass.cpp?rev=365273&r1=365272&r2=365273&view=diff</a><br>
>> ==============================================================================<br>
>> --- libcxx/trunk/test/std/thread/thread.mutex/thread.mutex.requirements/thread.mutex.requirements.mutex/thread.mutex.class/default.pass.cpp (original)<br>
>> +++ libcxx/trunk/test/std/thread/thread.mutex/thread.mutex.requirements/thread.mutex.requirements.mutex/thread.mutex.class/default.pass.cpp Sat Jul  6 18:20:54 2019<br>
>> @@ -21,8 +21,8 @@<br>
>><br>
>>  int main(int, char**)<br>
>>  {<br>
>> -    static_assert(std::is_nothrow_default_constructible<std::mutex>::value, "");<br>
>> -    std::mutex m;<br>
>> -<br>
>> +  static_assert(std::is_nothrow_default_constructible<std::mutex>::value, "");<br>
>> +  std::mutex m;<br>
>> +  ((void)m);<br>
>>    return 0;<br>
>>  }<br>
>><br>
>><br>
>> _______________________________________________<br>
>> libcxx-commits mailing list<br>
>> <a href="mailto:libcxx-commits@lists.llvm.org" target="_blank" rel="noreferrer">libcxx-commits@lists.llvm.org</a><br>
>> <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/libcxx-commits" rel="noreferrer noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/libcxx-commits</a><br>
><br>
> _______________________________________________<br>
> libcxx-commits mailing list<br>
> <a href="mailto:libcxx-commits@lists.llvm.org" target="_blank" rel="noreferrer">libcxx-commits@lists.llvm.org</a><br>
> <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/libcxx-commits" rel="noreferrer noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/libcxx-commits</a><br>
</blockquote></div>