[committed] libstdc++: Add TSan annotations to std::atomic<shared_ptr<T>>

[committed] libstdc++: Add TSan annotations to std::atomic<shared_ptr<T>>

[Jonathan Wakely]

Tested powerpc64le-linux, pushed to trunk.

-- >8 --

This adds annotations to std::atomic<shared_ptr<T>> to enable TSan to
understand the custom locking. Without this, TSan reports data races for
accesses to the _M_ptr member, even though those are correctly
synchronized using atomic operations on the tagged pointer.

libstdc++-v3/ChangeLog:

	* include/bits/shared_ptr_atomic.h (_GLIBCXX_TSAN_MUTEX_DESTROY)
	(_GLIBCXX_TSAN_MUTEX_PRE_LOCK, _GLIBCXX_TSAN_MUTEX_POST_LOCK)
	(_GLIBCXX_TSAN_MUTEX_PRE_UNLOCK, _GLIBCXX_TSAN_MUTEX_POST_UNLOCK)
	(_GLIBCXX_TSAN_MUTEX_PRE_SIGNAL, _GLIBCXX_TSAN_MUTEX_POST_SIGNAL):
	Define macros for TSan annotation functions.
	(_Sp_atomic::_Atomic_count): Add annotations.
---
 libstdc++-v3/include/bits/shared_ptr_atomic.h | 38 +++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/libstdc++-v3/include/bits/shared_ptr_atomic.h b/libstdc++-v3/include/bits/shared_ptr_atomic.h
index d4bd712fc7d..4580807f42c 100644
--- a/libstdc++-v3/include/bits/shared_ptr_atomic.h
+++ b/libstdc++-v3/include/bits/shared_ptr_atomic.h
@@ -32,6 +32,30 @@
 
 #include <bits/atomic_base.h>
 
+#if defined _GLIBCXX_TSAN && __has_include(<sanitizer/tsan_interface.h>)
+#include <sanitizer/tsan_interface.h>
+#define _GLIBCXX_TSAN_MUTEX_DESTROY(X) \
+  __tsan_mutex_destroy(X, __tsan_mutex_not_static)
+#define _GLIBCXX_TSAN_MUTEX_PRE_LOCK(X) \
+  __tsan_mutex_pre_lock(X, __tsan_mutex_not_static)
+#define _GLIBCXX_TSAN_MUTEX_POST_LOCK(X) \
+  __tsan_mutex_post_lock(X, __tsan_mutex_not_static, 0)
+#define _GLIBCXX_TSAN_MUTEX_PRE_UNLOCK(X) \
+  __tsan_mutex_pre_unlock(X, __tsan_mutex_not_static)
+#define _GLIBCXX_TSAN_MUTEX_POST_UNLOCK(X) \
+  __tsan_mutex_post_unlock(X, __tsan_mutex_not_static)
+#define _GLIBCXX_TSAN_MUTEX_PRE_SIGNAL(X) __tsan_mutex_pre_signal(X, 0)
+#define _GLIBCXX_TSAN_MUTEX_POST_SIGNAL(X) __tsan_mutex_post_signal(X, 0)
+#else
+#define _GLIBCXX_TSAN_MUTEX_DESTROY(X)
+#define _GLIBCXX_TSAN_MUTEX_PRE_LOCK(X)
+#define _GLIBCXX_TSAN_MUTEX_POST_LOCK(X)
+#define _GLIBCXX_TSAN_MUTEX_PRE_UNLOCK(X)
+#define _GLIBCXX_TSAN_MUTEX_POST_UNLOCK(X)
+#define _GLIBCXX_TSAN_MUTEX_PRE_SIGNAL(X)
+#define _GLIBCXX_TSAN_MUTEX_POST_SIGNAL(X)
+#endif
+
 namespace std _GLIBCXX_VISIBILITY(default)
 {
 _GLIBCXX_BEGIN_NAMESPACE_VERSION
@@ -377,6 +401,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	~_Atomic_count()
 	{
 	  auto __val = _M_val.load(memory_order_relaxed);
+	  _GLIBCXX_TSAN_MUTEX_DESTROY(&_M_val);
 	  __glibcxx_assert(!(__val & _S_lock_bit));
 	  if (auto __pi = reinterpret_cast<pointer>(__val))
 	    {
@@ -406,6 +431,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	      __current = _M_val.load(memory_order_relaxed);
 	    }
 
+	  _GLIBCXX_TSAN_MUTEX_PRE_LOCK(&_M_val);
+
 	  while (!_M_val.compare_exchange_strong(__current,
 						 __current | _S_lock_bit,
 						 __o,
@@ -416,6 +443,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 #endif
 	      __current = __current & ~_S_lock_bit;
 	    }
+	  _GLIBCXX_TSAN_MUTEX_POST_LOCK(&_M_val);
 	  return reinterpret_cast<pointer>(__current);
 	}
 
@@ -423,7 +451,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	void
 	unlock(memory_order __o) const noexcept
 	{
+	  _GLIBCXX_TSAN_MUTEX_PRE_UNLOCK(&_M_val);
 	  _M_val.fetch_sub(1, __o);
+	  _GLIBCXX_TSAN_MUTEX_POST_UNLOCK(&_M_val);
 	}
 
 	// Swaps the values of *this and __c, and unlocks *this.
@@ -434,7 +464,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	  if (__o != memory_order_seq_cst)
 	    __o = memory_order_release;
 	  auto __x = reinterpret_cast<uintptr_t>(__c._M_pi);
+	  _GLIBCXX_TSAN_MUTEX_PRE_UNLOCK(&_M_val);
 	  __x = _M_val.exchange(__x, __o);
+	  _GLIBCXX_TSAN_MUTEX_POST_UNLOCK(&_M_val);
 	  __c._M_pi = reinterpret_cast<pointer>(__x & ~_S_lock_bit);
 	}
 
@@ -443,20 +475,26 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	void
 	_M_wait_unlock(memory_order __o) const noexcept
 	{
+	  _GLIBCXX_TSAN_MUTEX_PRE_UNLOCK(&_M_val);
 	  auto __v = _M_val.fetch_sub(1, memory_order_relaxed);
+	  _GLIBCXX_TSAN_MUTEX_POST_UNLOCK(&_M_val);
 	  _M_val.wait(__v & ~_S_lock_bit, __o);
 	}
 
 	void
 	notify_one() noexcept
 	{
+	  _GLIBCXX_TSAN_MUTEX_PRE_SIGNAL(&_M_val);
 	  _M_val.notify_one();
+	  _GLIBCXX_TSAN_MUTEX_POST_SIGNAL(&_M_val);
 	}
 
 	void
 	notify_all() noexcept
 	{
+	  _GLIBCXX_TSAN_MUTEX_PRE_SIGNAL(&_M_val);
 	  _M_val.notify_all();
+	  _GLIBCXX_TSAN_MUTEX_POST_SIGNAL(&_M_val);
 	}
 #endif
 
-- 
2.37.3

Re: [committed] libstdc++: Add TSan annotations to std::atomic<shared_ptr<T>>

[Jonathan Wakely]

On Wed, 14 Sept 2022 at 23:05, Jonathan Wakely via Libstdc++
<libstdc++@gcc.gnu.org> wrote:
>
> Tested powerpc64le-linux, pushed to trunk.
>
> -- >8 --
>
> This adds annotations to std::atomic<shared_ptr<T>> to enable TSan to
> understand the custom locking. Without this, TSan reports data races for
> accesses to the _M_ptr member, even though those are correctly
> synchronized using atomic operations on the tagged pointer.
>
> libstdc++-v3/ChangeLog:
>
>         * include/bits/shared_ptr_atomic.h (_GLIBCXX_TSAN_MUTEX_DESTROY)
>         (_GLIBCXX_TSAN_MUTEX_PRE_LOCK, _GLIBCXX_TSAN_MUTEX_POST_LOCK)
>         (_GLIBCXX_TSAN_MUTEX_PRE_UNLOCK, _GLIBCXX_TSAN_MUTEX_POST_UNLOCK)
>         (_GLIBCXX_TSAN_MUTEX_PRE_SIGNAL, _GLIBCXX_TSAN_MUTEX_POST_SIGNAL):
>         Define macros for TSan annotation functions.
>         (_Sp_atomic::_Atomic_count): Add annotations.
> ---
>  libstdc++-v3/include/bits/shared_ptr_atomic.h | 38 +++++++++++++++++++
>  1 file changed, 38 insertions(+)
>
> diff --git a/libstdc++-v3/include/bits/shared_ptr_atomic.h b/libstdc++-v3/include/bits/shared_ptr_atomic.h
> index d4bd712fc7d..4580807f42c 100644
> --- a/libstdc++-v3/include/bits/shared_ptr_atomic.h
> +++ b/libstdc++-v3/include/bits/shared_ptr_atomic.h
> @@ -32,6 +32,30 @@
>
>  #include <bits/atomic_base.h>
>
> +#if defined _GLIBCXX_TSAN && __has_include(<sanitizer/tsan_interface.h>)
> +#include <sanitizer/tsan_interface.h>
> +#define _GLIBCXX_TSAN_MUTEX_DESTROY(X) \
> +  __tsan_mutex_destroy(X, __tsan_mutex_not_static)
> +#define _GLIBCXX_TSAN_MUTEX_PRE_LOCK(X) \
> +  __tsan_mutex_pre_lock(X, __tsan_mutex_not_static)
> +#define _GLIBCXX_TSAN_MUTEX_POST_LOCK(X) \
> +  __tsan_mutex_post_lock(X, __tsan_mutex_not_static, 0)
> +#define _GLIBCXX_TSAN_MUTEX_PRE_UNLOCK(X) \
> +  __tsan_mutex_pre_unlock(X, __tsan_mutex_not_static)
> +#define _GLIBCXX_TSAN_MUTEX_POST_UNLOCK(X) \
> +  __tsan_mutex_post_unlock(X, __tsan_mutex_not_static)
> +#define _GLIBCXX_TSAN_MUTEX_PRE_SIGNAL(X) __tsan_mutex_pre_signal(X, 0)
> +#define _GLIBCXX_TSAN_MUTEX_POST_SIGNAL(X) __tsan_mutex_post_signal(X, 0)
> +#else
> +#define _GLIBCXX_TSAN_MUTEX_DESTROY(X)
> +#define _GLIBCXX_TSAN_MUTEX_PRE_LOCK(X)
> +#define _GLIBCXX_TSAN_MUTEX_POST_LOCK(X)
> +#define _GLIBCXX_TSAN_MUTEX_PRE_UNLOCK(X)
> +#define _GLIBCXX_TSAN_MUTEX_POST_UNLOCK(X)
> +#define _GLIBCXX_TSAN_MUTEX_PRE_SIGNAL(X)
> +#define _GLIBCXX_TSAN_MUTEX_POST_SIGNAL(X)
> +#endif
> +
>  namespace std _GLIBCXX_VISIBILITY(default)
>  {
>  _GLIBCXX_BEGIN_NAMESPACE_VERSION
> @@ -377,6 +401,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>         ~_Atomic_count()
>         {
>           auto __val = _M_val.load(memory_order_relaxed);
> +         _GLIBCXX_TSAN_MUTEX_DESTROY(&_M_val);

After further thought, I'm not sure this is right. This tells tsan
that the "mutex" at &_M_val cannot be locked or unlocked again after
this. But what happens if the address is reused by a different
atomic<shared_ptr<T>> which happens to be at the same memory address?
Will tsan think that's an invalid use of the original "mutex" after
its destruction?

I will investigate.

We might need to stop using the __tsan_mutex_destroy call, and if so,
we can stop using the __tsan_mutex_not_static flag too. The pre/post
lock/unlock/signal pairs are still valuable without the lifetime
checking.

Re: [committed] libstdc++: Add TSan annotations to std::atomic<shared_ptr<T>>

[Jonathan Wakely]

On Wed, 14 Sept 2022 at 23:25, Jonathan Wakely wrote:
>
> On Wed, 14 Sept 2022 at 23:05, Jonathan Wakely via Libstdc++
> <libstdc++@gcc.gnu.org> wrote:
> >
> > Tested powerpc64le-linux, pushed to trunk.
> >
> > -- >8 --
> >
> > This adds annotations to std::atomic<shared_ptr<T>> to enable TSan to
> > understand the custom locking. Without this, TSan reports data races for
> > accesses to the _M_ptr member, even though those are correctly
> > synchronized using atomic operations on the tagged pointer.
> >
> > libstdc++-v3/ChangeLog:
> >
> >         * include/bits/shared_ptr_atomic.h (_GLIBCXX_TSAN_MUTEX_DESTROY)
> >         (_GLIBCXX_TSAN_MUTEX_PRE_LOCK, _GLIBCXX_TSAN_MUTEX_POST_LOCK)
> >         (_GLIBCXX_TSAN_MUTEX_PRE_UNLOCK, _GLIBCXX_TSAN_MUTEX_POST_UNLOCK)
> >         (_GLIBCXX_TSAN_MUTEX_PRE_SIGNAL, _GLIBCXX_TSAN_MUTEX_POST_SIGNAL):
> >         Define macros for TSan annotation functions.
> >         (_Sp_atomic::_Atomic_count): Add annotations.
> > ---
> >  libstdc++-v3/include/bits/shared_ptr_atomic.h | 38 +++++++++++++++++++
> >  1 file changed, 38 insertions(+)
> >
> > diff --git a/libstdc++-v3/include/bits/shared_ptr_atomic.h b/libstdc++-v3/include/bits/shared_ptr_atomic.h
> > index d4bd712fc7d..4580807f42c 100644
> > --- a/libstdc++-v3/include/bits/shared_ptr_atomic.h
> > +++ b/libstdc++-v3/include/bits/shared_ptr_atomic.h
> > @@ -32,6 +32,30 @@
> >
> >  #include <bits/atomic_base.h>
> >
> > +#if defined _GLIBCXX_TSAN && __has_include(<sanitizer/tsan_interface.h>)
> > +#include <sanitizer/tsan_interface.h>
> > +#define _GLIBCXX_TSAN_MUTEX_DESTROY(X) \
> > +  __tsan_mutex_destroy(X, __tsan_mutex_not_static)
> > +#define _GLIBCXX_TSAN_MUTEX_PRE_LOCK(X) \
> > +  __tsan_mutex_pre_lock(X, __tsan_mutex_not_static)
> > +#define _GLIBCXX_TSAN_MUTEX_POST_LOCK(X) \
> > +  __tsan_mutex_post_lock(X, __tsan_mutex_not_static, 0)
> > +#define _GLIBCXX_TSAN_MUTEX_PRE_UNLOCK(X) \
> > +  __tsan_mutex_pre_unlock(X, __tsan_mutex_not_static)
> > +#define _GLIBCXX_TSAN_MUTEX_POST_UNLOCK(X) \
> > +  __tsan_mutex_post_unlock(X, __tsan_mutex_not_static)
> > +#define _GLIBCXX_TSAN_MUTEX_PRE_SIGNAL(X) __tsan_mutex_pre_signal(X, 0)
> > +#define _GLIBCXX_TSAN_MUTEX_POST_SIGNAL(X) __tsan_mutex_post_signal(X, 0)
> > +#else
> > +#define _GLIBCXX_TSAN_MUTEX_DESTROY(X)
> > +#define _GLIBCXX_TSAN_MUTEX_PRE_LOCK(X)
> > +#define _GLIBCXX_TSAN_MUTEX_POST_LOCK(X)
> > +#define _GLIBCXX_TSAN_MUTEX_PRE_UNLOCK(X)
> > +#define _GLIBCXX_TSAN_MUTEX_POST_UNLOCK(X)
> > +#define _GLIBCXX_TSAN_MUTEX_PRE_SIGNAL(X)
> > +#define _GLIBCXX_TSAN_MUTEX_POST_SIGNAL(X)
> > +#endif
> > +
> >  namespace std _GLIBCXX_VISIBILITY(default)
> >  {
> >  _GLIBCXX_BEGIN_NAMESPACE_VERSION
> > @@ -377,6 +401,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> >         ~_Atomic_count()
> >         {
> >           auto __val = _M_val.load(memory_order_relaxed);
> > +         _GLIBCXX_TSAN_MUTEX_DESTROY(&_M_val);
>
> After further thought, I'm not sure this is right. This tells tsan
> that the "mutex" at &_M_val cannot be locked or unlocked again after
> this. But what happens if the address is reused by a different
> atomic<shared_ptr<T>> which happens to be at the same memory address?
> Will tsan think that's an invalid use of the original "mutex" after
> its destruction?

We can't easily add a call to __tsan_mutex_create, which would begin
the lifetime of a new object at that address, because the default
constructor is constexpr, and the create function isn't.

>
> I will investigate.
>
> We might need to stop using the __tsan_mutex_destroy call, and if so,
> we can stop using the __tsan_mutex_not_static flag too. The pre/post
> lock/unlock/signal pairs are still valuable without the lifetime
> checking.

[committed] libstdc++: Tweak TSan annotations for std::atomic<shared_ptr<T>>

[Jonathan Wakely]

On Wed, 14 Sept 2022 at 23:28, Jonathan Wakely wrote:
>
> On Wed, 14 Sept 2022 at 23:25, Jonathan Wakely wrote:
> >
> > On Wed, 14 Sept 2022 at 23:05, Jonathan Wakely via Libstdc++
> > <libstdc++@gcc.gnu.org> wrote:
> > > @@ -377,6 +401,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> > >         ~_Atomic_count()
> > >         {
> > >           auto __val = _M_val.load(memory_order_relaxed);
> > > +         _GLIBCXX_TSAN_MUTEX_DESTROY(&_M_val);
> >
> > After further thought, I'm not sure this is right. This tells tsan
> > that the "mutex" at &_M_val cannot be locked or unlocked again after
> > this. But what happens if the address is reused by a different
> > atomic<shared_ptr<T>> which happens to be at the same memory address?
> > Will tsan think that's an invalid use of the original "mutex" after
> > its destruction?
>
> We can't easily add a call to __tsan_mutex_create, which would begin
> the lifetime of a new object at that address, because the default
> constructor is constexpr, and the create function isn't.
>
> >
> > I will investigate.

I investigated.

There is a bug in my commit, but only that I pass
__tsan_mutex_not_static to the unlock annotations, and it's only valid
for the create and lock annotations. But it appears to simply be ignored
by the unlock functions, so it's harmless.

It seems that if __tsan_mutex_create has not been called, passing
__tsan_mutex_not_static to the lock functions implicitly begins the
lifetime of a lock. That means it's fine to "destroy" with an address
that then gets reused later for a second object, because we'll
implicitly create a new mutex (in tsan's mind) when we first lock it.
But it also means that tsan doesn't complain about this:

  using A = std::atomic<std::shared_ptr<int>>;
 alignas(A) unsigned char buf[sizeof(A)];
 A* a = new(buf) A();
 a->load();
 a->~A();
 a->load();

The second load() uses a destroyed mutex, but tsan just beings the
lifetime of a new one when we call __tsan_mutex_pre_lock(&_M_val,
__tsan_mutex_not_static). I don't think we can do anything about that,
but it's not tsan's job to detect use-after-free anyway.

Here's a patch to fix the incorrect flags being passed to the pre/post
unlock functions, and to make the lock annotations more fine-grained.

Tested powerpc64le-linux, pushed to trunk.

-- >8 --

Do not use the __tsan_mutex_not_static flag for annotation functions
where it's not a valid flag.  Also use the try_lock and try_lock_failed
flags to more precisely annotate the CAS loop used to acquire a lock.

libstdc++-v3/ChangeLog:

	* include/bits/shared_ptr_atomic.h (_GLIBCXX_TSAN_MUTEX_PRE_LOCK):
	Replace with ...
	(_GLIBCXX_TSAN_MUTEX_TRY_LOCK): ... this, add try_lock flag.
	(_GLIBCXX_TSAN_MUTEX_TRY_LOCK_FAILED): New macro using
	try_lock_failed flag
	(_GLIBCXX_TSAN_MUTEX_POST_LOCK): Rename to ...
	(_GLIBCXX_TSAN_MUTEX_LOCKED): ... this.
	(_GLIBCXX_TSAN_MUTEX_PRE_UNLOCK): Remove invalid flag.
	(_GLIBCXX_TSAN_MUTEX_POST_UNLOCK): Remove invalid flag.
	(_Sp_atomic::_Atomic_count::lock): Use new macros.
---
 libstdc++-v3/include/bits/shared_ptr_atomic.h | 26 +++++++++++--------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/libstdc++-v3/include/bits/shared_ptr_atomic.h b/libstdc++-v3/include/bits/shared_ptr_atomic.h
index 4580807f42c..55d193d4bda 100644
--- a/libstdc++-v3/include/bits/shared_ptr_atomic.h
+++ b/libstdc++-v3/include/bits/shared_ptr_atomic.h
@@ -32,24 +32,26 @@
 
 #include <bits/atomic_base.h>
 
+// Annotations for the custom locking in atomic<shared_ptr<T>>.
 #if defined _GLIBCXX_TSAN && __has_include(<sanitizer/tsan_interface.h>)
 #include <sanitizer/tsan_interface.h>
 #define _GLIBCXX_TSAN_MUTEX_DESTROY(X) \
   __tsan_mutex_destroy(X, __tsan_mutex_not_static)
-#define _GLIBCXX_TSAN_MUTEX_PRE_LOCK(X) \
-  __tsan_mutex_pre_lock(X, __tsan_mutex_not_static)
-#define _GLIBCXX_TSAN_MUTEX_POST_LOCK(X) \
+#define _GLIBCXX_TSAN_MUTEX_TRY_LOCK(X) \
+  __tsan_mutex_pre_lock(X, __tsan_mutex_not_static|__tsan_mutex_try_lock)
+#define _GLIBCXX_TSAN_MUTEX_TRY_LOCK_FAILED(X) __tsan_mutex_post_lock(X, \
+    __tsan_mutex_not_static|__tsan_mutex_try_lock_failed, 0)
+#define _GLIBCXX_TSAN_MUTEX_LOCKED(X) \
   __tsan_mutex_post_lock(X, __tsan_mutex_not_static, 0)
-#define _GLIBCXX_TSAN_MUTEX_PRE_UNLOCK(X) \
-  __tsan_mutex_pre_unlock(X, __tsan_mutex_not_static)
-#define _GLIBCXX_TSAN_MUTEX_POST_UNLOCK(X) \
-  __tsan_mutex_post_unlock(X, __tsan_mutex_not_static)
+#define _GLIBCXX_TSAN_MUTEX_PRE_UNLOCK(X) __tsan_mutex_pre_unlock(X, 0)
+#define _GLIBCXX_TSAN_MUTEX_POST_UNLOCK(X) __tsan_mutex_post_unlock(X, 0)
 #define _GLIBCXX_TSAN_MUTEX_PRE_SIGNAL(X) __tsan_mutex_pre_signal(X, 0)
 #define _GLIBCXX_TSAN_MUTEX_POST_SIGNAL(X) __tsan_mutex_post_signal(X, 0)
 #else
 #define _GLIBCXX_TSAN_MUTEX_DESTROY(X)
-#define _GLIBCXX_TSAN_MUTEX_PRE_LOCK(X)
-#define _GLIBCXX_TSAN_MUTEX_POST_LOCK(X)
+#define _GLIBCXX_TSAN_MUTEX_TRY_LOCK(X)
+#define _GLIBCXX_TSAN_MUTEX_TRY_LOCK_FAILED(X)
+#define _GLIBCXX_TSAN_MUTEX_LOCKED(X)
 #define _GLIBCXX_TSAN_MUTEX_PRE_UNLOCK(X)
 #define _GLIBCXX_TSAN_MUTEX_POST_UNLOCK(X)
 #define _GLIBCXX_TSAN_MUTEX_PRE_SIGNAL(X)
@@ -431,19 +433,21 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	      __current = _M_val.load(memory_order_relaxed);
 	    }
 
-	  _GLIBCXX_TSAN_MUTEX_PRE_LOCK(&_M_val);
+	  _GLIBCXX_TSAN_MUTEX_TRY_LOCK(&_M_val);
 
 	  while (!_M_val.compare_exchange_strong(__current,
 						 __current | _S_lock_bit,
 						 __o,
 						 memory_order_relaxed))
 	    {
+	      _GLIBCXX_TSAN_MUTEX_TRY_LOCK_FAILED(&_M_val);
 #if __cpp_lib_atomic_wait
 	      __detail::__thread_relax();
 #endif
 	      __current = __current & ~_S_lock_bit;
+	      _GLIBCXX_TSAN_MUTEX_TRY_LOCK(&_M_val);
 	    }
-	  _GLIBCXX_TSAN_MUTEX_POST_LOCK(&_M_val);
+	  _GLIBCXX_TSAN_MUTEX_LOCKED(&_M_val);
 	  return reinterpret_cast<pointer>(__current);
 	}
 
-- 
2.37.3