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