public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jonathan Wakely <jwakely@redhat.com>
To: Mike Crowe <mac@mcrowe.com>
Cc: "libstdc++" <libstdc++@gcc.gnu.org>,
	gcc Patches <gcc-patches@gcc.gnu.org>,
	 Thomas Rodgers <trodgers@redhat.com>
Subject: [PATCH v2] libstdc++: Do not use pthread_mutex_clocklock with ThreadSanitizer
Date: Thu, 11 May 2023 21:52:22 +0100	[thread overview]
Message-ID: <CACb0b4n6MiTz2iV5Ef9HoupLdub65_A8MbY_1dmg+7sLKOOKCQ@mail.gmail.com> (raw)
In-Reply-To: <CACb0b4kiw81heWQQNdj0nhY64OxJz+imDCp=0ac1gxVZ_VWqfA@mail.gmail.com>


[-- Attachment #1.1: Type: text/plain, Size: 1551 bytes --]

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.

[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 1711 bytes --]

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"

  reply	other threads:[~2023-05-11 20:52 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-10 11:20 [RFC] " Jonathan Wakely
2023-05-10 11:31 ` Jonathan Wakely
2023-05-11 12:19   ` Mike Crowe
2023-05-11 12:42     ` Jonathan Wakely
2023-05-11 20:52       ` Jonathan Wakely [this message]
2023-05-12 10:30         ` [PATCH v2] " Mike Crowe
2023-05-12 10:32           ` Jonathan Wakely
2023-05-15 12:03             ` Mike Crowe
2023-05-15 19:58         ` Thomas Rodgers
2023-05-11 15:54     ` [RFC] " Thomas Rodgers
2023-05-11 16:04       ` Jonathan Wakely

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CACb0b4n6MiTz2iV5Ef9HoupLdub65_A8MbY_1dmg+7sLKOOKCQ@mail.gmail.com \
    --to=jwakely@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=libstdc++@gcc.gnu.org \
    --cc=mac@mcrowe.com \
    --cc=trodgers@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).