| Message ID | CACb0b4n6MiTz2iV5Ef9HoupLdub65_A8MbY_1dmg+7sLKOOKCQ@mail.gmail.com |
|---|---|
| State | Committed |
| Commit | 3b7cb33033fbe6af6c1c4ef014f7353479d1dd6b |
| Delegated to: | Thomas Rodgers |
| Headers |
Return-Path: <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 27B983858C54 for <patchwork@sourceware.org>; Thu, 11 May 2023 20:53:06 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 27B983858C54 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1683838386; bh=RKYySIpioQyP+GJvofg3IvS/w/CcC6ndB68/CrthHN4=; h=References:In-Reply-To:Date:Subject:To:Cc:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=P4cAIhMpmAAUgCESV6qPItVsIRJDeAXvBWxe47H8RcutJ18iCG7s0ld++HDlQiI3c r0zUs0qG2ftpTsIUNIZ7xgxQbl/BTwDmVzilHZZOYtaR+Iv7UFADJBdHwKzhzFshFe MNk9wMKZmWfW/U0wl3YeRKAnPh+HEOEbVNNRXDBc= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 29D843858D1E for <gcc-patches@gcc.gnu.org>; Thu, 11 May 2023 20:52:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 29D843858D1E Received: from mail-lf1-f71.google.com (mail-lf1-f71.google.com [209.85.167.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-546-kjXj2SbrOSqDH3_HB07Rmw-1; Thu, 11 May 2023 16:52:35 -0400 X-MC-Unique: kjXj2SbrOSqDH3_HB07Rmw-1 Received: by mail-lf1-f71.google.com with SMTP id 2adb3069b0e04-4edd54a0eaeso5340141e87.0 for <gcc-patches@gcc.gnu.org>; Thu, 11 May 2023 13:52:34 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1683838353; x=1686430353; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=vhYfl4CcC89Y6dNZsBVhHovXEmniD3wfhrgcxRhLtL4=; b=UoBFwWKbg829rq//jESFwwm+YdIwla+x1l6wM/GUbsEnh4Kvr/vra/DAFtsIHdTyOC iBmp6cGk/ld2bcMuWA1xdJLDCMgWmOLjDcfxyDQ5D7OVWOuCAQOHXcb8fYDo/Q1nXhsV rbo/tQVlU/LZctWulu4GyYYjsTRCs20YfzWF0//MMp3Yp+s48EgKZtsBr7TxE737tPIy dWAg7u6owHwRsm/ojwLV+Kgk4T137rM6HJ6N3Q9Uj0P2S1/cPHO3WZfPJPPkZtcw7xiK v68zIOD74Mw6w/VH3WHHyc7RjneYlLOYrWdn46nwgSAWWSQyHXHu8Wkds8+vMgcQ1baY ZJyQ== X-Gm-Message-State: AC+VfDyZHbEgxXKChJo0JaJPFjjdBuX3dkwgwKdk7J9zkQzPUAPogIRB oKARPqZG13QVGHGVGQ8MWe5yGoOIp+gCjgXwHyUqkBNrQyecg+D/VGWNzbh1lIKbRlwRAxD8149 WhwXTjubAUVLFtkI2++gWzIVzLzfdocehPQ== X-Received: by 2002:a2e:9818:0:b0:2a8:c842:d30c with SMTP id a24-20020a2e9818000000b002a8c842d30cmr3952906ljj.44.1683838353620; Thu, 11 May 2023 13:52:33 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ4eipcVOFy2ODWdwJ501tlJS2PMJ8PXV7lZBzxVjhsAg6zlbmg/NzUf1JMjFOAaGsbODbosyHbFec5wkPwNKBE= X-Received: by 2002:a2e:9818:0:b0:2a8:c842:d30c with SMTP id a24-20020a2e9818000000b002a8c842d30cmr3952901ljj.44.1683838353312; Thu, 11 May 2023 13:52:33 -0700 (PDT) MIME-Version: 1.0 References: <20230510112009.633444-1-jwakely@redhat.com> <CACb0b4nocPyOFrWgWz07AWjF_z+Ncm0+qCNPzdU6JoFdaGtCJg@mail.gmail.com> <ZFzdW4gP0x/d+ghs@mcrowe.com> <CACb0b4kiw81heWQQNdj0nhY64OxJz+imDCp=0ac1gxVZ_VWqfA@mail.gmail.com> In-Reply-To: <CACb0b4kiw81heWQQNdj0nhY64OxJz+imDCp=0ac1gxVZ_VWqfA@mail.gmail.com> Date: Thu, 11 May 2023 21:52:22 +0100 Message-ID: <CACb0b4n6MiTz2iV5Ef9HoupLdub65_A8MbY_1dmg+7sLKOOKCQ@mail.gmail.com> Subject: [PATCH v2] libstdc++: Do not use pthread_mutex_clocklock with ThreadSanitizer To: Mike Crowe <mac@mcrowe.com> Cc: "libstdc++" <libstdc++@gcc.gnu.org>, gcc Patches <gcc-patches@gcc.gnu.org>, Thomas Rodgers <trodgers@redhat.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: multipart/mixed; boundary="000000000000e8beea05fb712a7b" X-Spam-Status: No, score=-12.2 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, HTML_MESSAGE, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-Content-Filtered-By: Mailman/MimeDel 2.1.29 X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list <gcc-patches.gcc.gnu.org> List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=unsubscribe> List-Archive: <https://gcc.gnu.org/pipermail/gcc-patches/> List-Post: <mailto:gcc-patches@gcc.gnu.org> List-Help: <mailto:gcc-patches-request@gcc.gnu.org?subject=help> List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=subscribe> From: Jonathan Wakely via Gcc-patches <gcc-patches@gcc.gnu.org> Reply-To: Jonathan Wakely <jwakely@redhat.com> Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> |
| Series |
[v2] libstdc++: Do not use pthread_mutex_clocklock with ThreadSanitizer
|
|
Commit Message
Jonathan Wakely
May 11, 2023, 8:52 p.m. UTC
On Thu, 11 May 2023 at 13:42, Jonathan Wakely <jwakely@redhat.com> wrote: > > > On Thu, 11 May 2023 at 13:19, Mike Crowe <mac@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?!) 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 <jwakely@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.
Comments
On Thursday 11 May 2023 at 21:52:22 +0100, Jonathan Wakely wrote: > On Thu, 11 May 2023 at 13:42, Jonathan Wakely <jwakely@redhat.com> wrote: > > > > > > > On Thu, 11 May 2023 at 13:19, Mike Crowe <mac@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. > 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 <jwakely@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.
On Fri, 12 May 2023 at 11:30, Mike Crowe <mac@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 <jwakely@redhat.com> > wrote: > > > > > > > > > > > On Thu, 11 May 2023 at 13:19, Mike Crowe <mac@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 <jwakely@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. > >
On Friday 12 May 2023 at 11:32:56 +0100, Jonathan Wakely wrote: > On Fri, 12 May 2023 at 11:30, Mike Crowe <mac@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 <jwakely@redhat.com> > > wrote: > > > > On Thu, 11 May 2023 at 13:19, Mike Crowe <mac@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? I'm by no means an expert, but I would guess not. I suspect that the renaming was introduced as part of the Y2038 fixes and TSan hasn't caught up with them either. Mike.
On Thu, May 11, 2023 at 1:52 PM Jonathan Wakely <jwakely@redhat.com> wrote: > On Thu, 11 May 2023 at 13:42, Jonathan Wakely <jwakely@redhat.com> wrote: > >> >> >> On Thu, 11 May 2023 at 13:19, Mike Crowe <mac@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?!) > > So I'm now retesting with this version of the patch, which only touches > the USE_PTHREAD_LOCKLOCK macro. > > Please take another look, thanks. > > LGTM.
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"