[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 21:54:56 PDT 2019
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/64615c79/attachment-0001.html>
More information about the libcxx-commits
mailing list