Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No tsan interceptors for pthread_mutex_clocklock and other new POSIX APIs #62623

Open
jwakely opened this issue May 9, 2023 · 0 comments
Open
Labels
compiler-rt:tsan Thread sanitizer

Comments

@jwakely
Copy link
Contributor

jwakely commented May 9, 2023

POSIX-1:202x includes several new functions that TSan needs to support:

Austin Group Defect 1216 is applied, adding pthread_cond_clockwait(), pthread_mutex_clocklock(), pthread_rwlock_clockrdlock(), pthread_rwlock_clockwrlock(), and sem_clockwait() to the list of functions that synchronize memory.

(If it matters, pthread_cond_clockwait is required to have cancellation points, and pthread_rwlock_clockwrlock, pthread_rwlock_clockrdlock, and sem_clockwait may have cancellation points.)

Glibc already supports these, and so the following program gives a false positive warning from TSan:

#include <stdio.h>
#include <pthread.h>

pthread_mutex_t m = PTHREAD_MUTEX_INITIALIZER;
struct timespec ts = { 0 };

void* tfunc(void* p)
{
    if (!pthread_mutex_trylock(&m))
    {
        puts("Locked mutex in second thread");
        pthread_mutex_unlock(&m);
    }
    else
        puts("Second thread could not lock mutex");
    return p;
}

int main()
{
    if (!pthread_mutex_clocklock(&m, CLOCK_REALTIME, &ts))
    {
        puts("Locked mutex in main() thread");
        pthread_t thr;
        pthread_create(&thr, 0, tfunc, 0);
        pthread_join(thr, 0);
        pthread_mutex_unlock(&m);
    }
    else
        puts("Failed to lock mutex");
}
==================
WARNING: ThreadSanitizer: unlock of an unlocked mutex (or by a wrong thread) (pid=1)
    #0 pthread_mutex_unlock <null> (libtsan.so.2+0x50241) (BuildId: 4e0d0a71dfb217392f9c0b4e6f757e50fa8e9242)
    #1 main /app/example.c:27 (output.s+0x401287) (BuildId: 06c6ce9eaa9b45a121f91cff2016ac89d2e036dc)

  Location is global 'm' of size 40 at 0x000000404080 (output.s+0x404080)

  Mutex M0 (0x000000404080) created at:
    #0 pthread_mutex_unlock <null> (libtsan.so.2+0x50241) (BuildId: 4e0d0a71dfb217392f9c0b4e6f757e50fa8e9242)
    #1 main /app/example.c:27 (output.s+0x401287) (BuildId: 06c6ce9eaa9b45a121f91cff2016ac89d2e036dc)

SUMMARY: ThreadSanitizer: unlock of an unlocked mutex (or by a wrong thread) (/opt/compiler-explorer/gcc-13.1.0/lib64/libtsan.so.2+0x50241) (BuildId: 4e0d0a71dfb217392f9c0b4e6f757e50fa8e9242) in pthread_mutex_unlock
==================
ThreadSanitizer: reported 1 warnings

A similar false positive is given for any use of functions like std::timed_mutex::try_lock_for with libstdc++, as that uses the new Glibc functions when available.

@EugeneZelenko EugeneZelenko added compiler-rt:tsan Thread sanitizer and removed new issue labels May 9, 2023
wangliu-iscas pushed a commit to plctlab/patchwork-gcc that referenced this issue May 10, 2023
This patch would avoid TSan false positives when using timed waiting
functions on mutexes and condvars, but as noted below, it changes the
semantics.

I'm not sure whether we want this workaround in place until tsan gets
fixed.

On one hand, there's no guarantee that those functions use the right
clock anyway (and they won't do unless a recent-ish glibc is used). But
on the other hand, if they normally would use the right clock because
you have glibc support, it's not ideal for tsan to cause a different
clock to be used.

-- >8 --

As noted in llvm/llvm-project#62623 there are
no tsan interceptors for the new POSIX-1:202x APIs added by
https://austingroupbugs.net/view.php?id=1216 so tsan gives false
positive warnings.

Disable the uses of the new APIs when tsan is active. This changes the
semantics of those 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_COND_CLOCKWAIT): Define
	_GLIBCXX_USE_PTHREAD_COND_CLOCKWAIT to depend on _GLIBCXX_TSAN.
	(GLIBCXX_CHECK_PTHREAD_MUTEX_CLOCKLOCK): Likewise for
	_GLIBCXX_USE_PTHREAD_MUTEX_CLOCKLOCK.
	(GLIBCXX_CHECK_PTHREAD_RWLOCK_CLOCKLOCK): Likewise for
	_GLIBCXX_USE_PTHREAD_RWLOCK_CLOCKLOCK.
	* configure: Regenerate.
wangliu-iscas pushed a commit to plctlab/patchwork-gcc that referenced this issue May 11, 2023
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 llvm/llvm-project#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.
nstester pushed a commit to nstester/gcc that referenced this issue May 16, 2023
As noted in llvm/llvm-project#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.

Reviewed-by: Thomas Rodgers <trodgers@redhat.com>
Reviewed-by: Mike Crowe <mac@mcrowe.com>

libstdc++-v3/ChangeLog:

	* acinclude.m4 (GLIBCXX_CHECK_PTHREAD_MUTEX_CLOCKLOCK): Define
	_GLIBCXX_USE_PTHREAD_MUTEX_CLOCKLOCK in terms of _GLIBCXX_TSAN.
	* configure: Regenerate.
wangliu-iscas pushed a commit to plctlab/patchwork-gcc that referenced this issue May 16, 2023
Tested powerpc64le-linux. Pushed to trunk.

-- >8 --

As noted in llvm/llvm-project#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.

Reviewed-by: Thomas Rodgers <trodgers@redhat.com>
Reviewed-by: Mike Crowe <mac@mcrowe.com>

libstdc++-v3/ChangeLog:

	* acinclude.m4 (GLIBCXX_CHECK_PTHREAD_MUTEX_CLOCKLOCK): Define
	_GLIBCXX_USE_PTHREAD_MUTEX_CLOCKLOCK in terms of _GLIBCXX_TSAN.
	* configure: Regenerate.
vitalybuka pushed a commit that referenced this issue Dec 18, 2023
The function `pthread_mutex_clocklock` is not supported by TSAN yet,
which is mentioned by[
/issues/62623](#62623 (comment)).
This patch is to handle this function.
qihangkong pushed a commit to rvgpu/rvgpu-llvm that referenced this issue Apr 23, 2024
The function `pthread_mutex_clocklock` is not supported by TSAN yet,
which is mentioned by[
llvm/llvm-project/issues/62623](llvm/llvm-project#62623 (comment)).
This patch is to handle this function.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler-rt:tsan Thread sanitizer
Projects
None yet
Development

No branches or pull requests

2 participants