On Fri, 12 May 2023 at 11:30, Mike Crowe <m...@mcrowe.com> wrote:
> On Thursday 11 May 2023 at 21:52:22 +0100, Jonathan Wakely wrote:
> > On Thu, 11 May 2023 at 13:42, Jonathan Wakely <jwak...@redhat.com>
> wrote:
> >
> > >
> > >
> > > On Thu, 11 May 2023 at 13:19, Mike Crowe <m...@mcrowe.com> wrote:
> > >
> > >> However, ...
> > >>
> > >> > > diff --git a/libstdc++-v3/acinclude.m4 b/libstdc++-v3/acinclude.m4
> > >> > > index 89e7f5f5f45..e2700b05ec3 100644
> > >> > > --- a/libstdc++-v3/acinclude.m4
> > >> > > +++ b/libstdc++-v3/acinclude.m4
> > >> > > @@ -4284,7 +4284,7 @@
> > >> AC_DEFUN([GLIBCXX_CHECK_PTHREAD_COND_CLOCKWAIT], [
> > >> > > [glibcxx_cv_PTHREAD_COND_CLOCKWAIT=no])
> > >> > > ])
> > >> > > if test $glibcxx_cv_PTHREAD_COND_CLOCKWAIT = yes; then
> > >> > > - AC_DEFINE(_GLIBCXX_USE_PTHREAD_COND_CLOCKWAIT, 1, [Define if
> > >> > > pthread_cond_clockwait is available in <pthread.h>.])
> > >> > > + AC_DEFINE(_GLIBCXX_USE_PTHREAD_COND_CLOCKWAIT,
> > >> (_GLIBCXX_TSAN==0),
> > >> > > [Define if pthread_cond_clockwait is available in <pthread.h>.])
> > >> > > fi
> > >>
> > >> TSan does appear to have an interceptor for pthread_cond_clockwait,
> even
> > >> if
> > >> it lacks the others. Does this mean that this part is unnecessary?
> > >>
> > >
> > > Ah good point, thanks. I grepped for clocklock but not clockwait.
> > >
> >
> > In fact it seems like we don't need to change
> > _GLIBCXX_USE_PTHREAD_RWLOCK_CLOCKLOCK either, because I don't get any
> tsan
> > warnings for that. It doesn't have interceptors for
> > pthread_rwlock_{rd,wr}lock, but it doesn't complain anyway (maybe it's
> > simply not instrumenting the rwlock functions at all?!)
>
> It looks like TSan does have interceptors for pthread_rwlock_timedrdlock
> etc. I can't explain why this doesn't cause problems when libstdc++ uses
> pthread_rwlock_clockrdlock etc.
>
I think glibc has renamed the rwlock functions, and so the interceptors no
longer work.
# ifdef __USE_XOPEN2K
/* Try to acquire read lock for RWLOCK or return after specfied time. */
# ifndef __USE_TIME_BITS64
extern int pthread_rwlock_timedrdlock (pthread_rwlock_t *__restrict
__rwlock,
const struct timespec *__restrict
__abstime) __THROWNL __nonnull ((1, 2));
# else
# ifdef __REDIRECT_NTHNL
extern int __REDIRECT_NTHNL (pthread_rwlock_timedrdlock,
(pthread_rwlock_t *__restrict __rwlock,
const struct timespec *__restrict __abstime),
__pthread_rwlock_timedrdlock64)
__nonnull ((1, 2));
# else
# define pthread_rwlock_timedrdlock __pthread_rwlock_timedrdlock64
# endif
# endif
# endif
If glibc is really providing a function called
__pthread_rwlock_timedrdlock64 then will tsan be able to intercept that?
> > So I'm now retesting with this version of the patch, which only touches
> the
> > USE_PTHREAD_LOCKLOCK macro.
> >
> > Please take another look, thanks.
>
> > commit 4fc14825c125eece32980df21d09da35e3d5bac6
> > Author: Jonathan Wakely <jwak...@redhat.com>
> > Date: Tue May 9 09:30:48 2023
> >
> > libstdc++: Do not use pthread_mutex_clocklock with ThreadSanitizer
> >
> > As noted in https://github.com/llvm/llvm-project/issues/62623 there
> are
> > no tsan interceptors for some of the new POSIX-1:202x APIs added by
> > https://austingroupbugs.net/view.php?id=1216 so tsan gives false
> > positive warnings for try_lock_for on timed mutexes.
> >
> > Disable the uses of the new pthread_mutex_clocklock API when tsan is
> > active. This changes the semantics of the try_lock_for functions,
> > because it can change which clock is used for the wait. This means
> those
> > functions might be affected by system clock adjustments when tsan is
> > used, when they would not be affected otherwise.
> >
> > libstdc++-v3/ChangeLog:
> >
> > * acinclude.m4 (GLIBCXX_CHECK_PTHREAD_MUTEX_CLOCKLOCK):
> Define
> > _GLIBCXX_USE_PTHREAD_MUTEX_CLOCKLOCK in terms of
> _GLIBCXX_TSAN.
> > * configure: Regenerate.
> >
> > diff --git a/libstdc++-v3/acinclude.m4 b/libstdc++-v3/acinclude.m4
> > index 89e7f5f5f45..dce3d16aa5c 100644
> > --- a/libstdc++-v3/acinclude.m4
> > +++ b/libstdc++-v3/acinclude.m4
> > @@ -4314,7 +4314,7 @@ AC_DEFUN([GLIBCXX_CHECK_PTHREAD_MUTEX_CLOCKLOCK], [
> > [glibcxx_cv_PTHREAD_MUTEX_CLOCKLOCK=no])
> > ])
> > if test $glibcxx_cv_PTHREAD_MUTEX_CLOCKLOCK = yes; then
> > - AC_DEFINE(_GLIBCXX_USE_PTHREAD_MUTEX_CLOCKLOCK, 1, [Define if
> pthread_mutex_clocklock is available in <pthread.h>.])
> > + AC_DEFINE(_GLIBCXX_USE_PTHREAD_MUTEX_CLOCKLOCK, (_GLIBCXX_TSAN==0),
> [Define if pthread_mutex_clocklock is available in <pthread.h>.])
> > fi
> >
> > CXXFLAGS="$ac_save_CXXFLAGS"
>
> LGTM.
>
> Mike.
>
>
