]> git.ipfire.org Git - thirdparty/valgrind.git/commitdiff
Bug 446139 DRD/Helgrind with std::shared_timed_mutex::try_lock_until and try_lock_sha...
authorPaul Floyd <pjfloyd@wanadoo.fr>
Wed, 1 Dec 2021 23:03:27 +0000 (00:03 +0100)
committerPaul Floyd <pjfloyd@wanadoo.fr>
Wed, 1 Dec 2021 23:03:27 +0000 (00:03 +0100)
also
Bug 446138 DRD/Helgrind with std::timed_mutex::try_lock_until false positives

.gitignore
configure.ac
drd/drd_pthread_intercepts.c
drd/tests/Makefile.am
drd/tests/shared_timed_mutex.cpp [new file with mode: 0644]
drd/tests/shared_timed_mutex.stderr.exp [new file with mode: 0644]
drd/tests/shared_timed_mutex.vgtest [new file with mode: 0644]
drd/tests/timed_mutex.cpp [new file with mode: 0644]
drd/tests/timed_mutex.stderr.exp [new file with mode: 0644]
drd/tests/timed_mutex.vgtest [new file with mode: 0644]
helgrind/hg_intercepts.c

index fd85516b28098ebe31cb39791085d3be826b793f..bf83c0f8c0ffd30449196b518068e3a8b1aa7d80 100644 (file)
 /drd/tests/sem_as_mutex
 /drd/tests/sem_open
 /drd/tests/sem_wait
+/drd/tests/shared_timed_mutex
 /drd/tests/sigalrm
 /drd/tests/std_atomic
 /drd/tests/std_list
 /drd/tests/threaded-fork
 /drd/tests/thread_name
 /drd/tests/thread_name_freebsd
+/drd/tests/timed_mutex
 /drd/tests/trylock
 /drd/tests/tsan_unittest
 /drd/tests/unit_bitmap
index 4623d12b5b1c94771a11aee099adf3ad8485c252..96df114baeca0cee8c332ec35e69b74ce1e7fa4d 100755 (executable)
@@ -2011,6 +2011,52 @@ AC_LANG(C)
 
 AM_CONDITIONAL(CXX_CAN_INCLUDE_CONDITION_VARIABLE_HEADER, test x$ac_cxx_can_include_condition_variable_header = xyes)
 
+# check for std::shared_timed_mutex, this is a C++ 14 feature
+
+AC_MSG_CHECKING([that C++ compiler can use std::shared_timed_mutex])
+AC_LANG(C++)
+safe_CXXFLAGS=$CXXFLAGS
+CXXFLAGS="-std=c++1y -pthread"
+
+AC_COMPILE_IFELSE([AC_LANG_SOURCE([
+#include <shared_mutex>
+std::shared_timed_mutex test_mutex;
+])],
+[
+ac_cxx_can_use_shared_timed_mutex=yes
+AC_MSG_RESULT([yes])
+], [
+ac_cxx_can_use_shared_timed_mutex=no
+AC_MSG_RESULT([no])
+])
+CXXFLAGS=$safe_CXXFLAGS
+AC_LANG(C)
+
+AM_CONDITIONAL(CXX_CAN_USE_SHARED_TIMED_MUTEX, test x$ac_cxx_can_use_shared_timed_mutex = xyes)
+
+# check for std::shared_mutex, this is a C++ 11 feature
+
+AC_MSG_CHECKING([that C++ compiler can use std::timed_mutex])
+AC_LANG(C++)
+safe_CXXFLAGS=$CXXFLAGS
+CXXFLAGS="-std=c++0x -pthread"
+
+AC_COMPILE_IFELSE([AC_LANG_SOURCE([
+#include <mutex>
+std::timed_mutex test_mutex;
+])],
+[
+ac_cxx_can_use_timed_mutex=yes
+AC_MSG_RESULT([yes])
+], [
+ac_cxx_can_use_timed_mutex=no
+AC_MSG_RESULT([no])
+])
+CXXFLAGS=$safe_CXXFLAGS
+AC_LANG(C)
+
+AM_CONDITIONAL(CXX_CAN_USE_TIMED_MUTEX, test x$ac_cxx_can_use_timed_mutex = xyes)
+
 # On aarch64 before glibc 2.20 we would get the kernel user_pt_regs instead
 # of the user_regs_struct from sys/user.h. They are structurally the same
 # but we get either one or the other.
index 95127b42c6c7d25e19f824781b9a0f222153699b..c18e888678f67644a39327a13f2453a714611f82 100644 (file)
@@ -1016,6 +1016,26 @@ PTH_FUNCS(int,
           (mutex, timeout));
 #endif /* VGO_solaris */
 
+static __always_inline
+int pthread_mutex_clocklock_intercept(pthread_mutex_t *mutex,
+                                      clockid_t clockid,
+                                      const struct timespec *abs_timeout)
+{
+   int   ret;
+   OrigFn fn;
+   VALGRIND_GET_ORIG_FN(fn);
+   VALGRIND_DO_CLIENT_REQUEST_STMT(VG_USERREQ__PRE_MUTEX_LOCK,
+                                   mutex, DRD_(mutex_type)(mutex), 0, 0, 0);
+   CALL_FN_W_WWW(ret, fn, mutex, clockid, abs_timeout);
+   VALGRIND_DO_CLIENT_REQUEST_STMT(VG_USERREQ__POST_MUTEX_LOCK,
+                                   mutex, ret == 0, 0, 0, 0);
+   return ret;
+}
+
+PTH_FUNCS(int, pthreadZumutexZuclocklock, pthread_mutex_clocklock_intercept,
+          (pthread_mutex_t *mutex, clockid_t clockid, const struct timespec *abs_timeout),
+          (mutex, clockid, abs_timeout));
+
 static __always_inline
 int pthread_mutex_unlock_intercept(pthread_mutex_t *mutex)
 {
@@ -1794,6 +1814,27 @@ PTH_FUNCS(int, pthreadZurwlockZureltimedrdlockZunp,
           (rwlock, timeout));
 #endif /* VGO_solaris */
 
+static __always_inline
+int pthread_rwlock_clockrdlock_intercept(pthread_rwlock_t* rwlock,
+                                         clockid_t clockid,
+                                         const struct timespec *timeout)
+{
+   int   ret;
+   OrigFn fn;
+   VALGRIND_GET_ORIG_FN(fn);
+   VALGRIND_DO_CLIENT_REQUEST_STMT(VG_USERREQ__PRE_RWLOCK_RDLOCK,
+                                   rwlock, 0, 0, 0, 0);
+   CALL_FN_W_WWW(ret, fn, rwlock, clockid, timeout);
+   VALGRIND_DO_CLIENT_REQUEST_STMT(VG_USERREQ__POST_RWLOCK_RDLOCK,
+                                   rwlock, ret == 0, 0, 0, 0);
+   return ret;
+}
+
+PTH_FUNCS(int,
+          pthreadZurwlockZuclockrdlock, pthread_rwlock_clockrdlock_intercept,
+          (pthread_rwlock_t* rwlock, clockid_t clockid, const struct timespec *timeout),
+          (rwlock, clockid, timeout));
+
 static __always_inline
 int pthread_rwlock_timedwrlock_intercept(pthread_rwlock_t* rwlock,
                                          const struct timespec *timeout)
@@ -1820,6 +1861,28 @@ PTH_FUNCS(int, pthreadZurwlockZureltimedwrlockZunp,
           (rwlock, timeout));
 #endif /* VGO_solaris */
 
+static __always_inline
+int pthread_rwlock_clockwrlock_intercept(pthread_rwlock_t* rwlock,
+                                         clockid_t clockid,
+                                         const struct timespec *timeout)
+{
+   int   ret;
+   OrigFn fn;
+   VALGRIND_GET_ORIG_FN(fn);
+   VALGRIND_DO_CLIENT_REQUEST_STMT(VG_USERREQ__PRE_RWLOCK_WRLOCK,
+                                   rwlock, 0, 0, 0, 0);
+   CALL_FN_W_WWW(ret, fn, rwlock, clockid, timeout);
+   VALGRIND_DO_CLIENT_REQUEST_STMT(VG_USERREQ__POST_RWLOCK_WRLOCK,
+                                   rwlock, ret == 0, 0, 0, 0);
+   return ret;
+}
+
+PTH_FUNCS(int,
+          pthreadZurwlockZuclockwrlock, pthread_rwlock_clockwrlock_intercept,
+          (pthread_rwlock_t* rwlock, clockid_t clockid, const struct timespec *timeout),
+          (rwlock, clockid, timeout));
+
+
 static __always_inline
 int pthread_rwlock_tryrdlock_intercept(pthread_rwlock_t* rwlock)
 {
index ac487def3c328a4c9c8d327b703e596edc809d8d..d7d73447bdbe156a90257cfba389053228ba5d33 100755 (executable)
@@ -275,6 +275,8 @@ EXTRA_DIST =                                        \
        sem_open_traced.vgtest                      \
        sem_wait.stderr.exp                         \
        sem_wait.vgtest                             \
+       shared_timed_mutex.stderr.exp               \
+       shared_timed_mutex.vgtest                   \
        swapcontext.stderr.exp                      \
        swapcontext.vgtest                          \
        sigalrm.stderr.exp                          \
@@ -369,6 +371,8 @@ EXTRA_DIST =                                        \
        threaded-fork.vgtest                        \
        threaded-fork-vcs.stderr.exp                \
        threaded-fork-vcs.vgtest                    \
+       timed_mutex.stderr.exp                      \
+       timed_mutex.vgtest                          \
        tls_threads.stderr.exp                      \
        tls_threads.vgtest                          \
        trylock.stderr.exp                          \
@@ -468,6 +472,16 @@ check_PROGRAMS += \
     condvar
 endif
 
+if CXX_CAN_USE_SHARED_TIMED_MUTEX
+check_PROGRAMS += \
+    shared_timed_mutex
+endif
+
+if CXX_CAN_USE_TIMED_MUTEX
+check_PROGRAMS += \
+    timed_mutex
+endif
+
 if HAVE_OPENMP
 check_PROGRAMS += omp_matinv omp_prime omp_printf
 endif
@@ -593,6 +607,9 @@ if VGCONF_OS_IS_SOLARIS
 rwlock_test_CFLAGS          += -D__EXTENSIONS__
 endif
 
+shared_timed_mutex_SOURCES  = shared_timed_mutex.cpp
+shared_timed_mutex_CXXFLAGS = $(AM_CXXFLAGS) -std=c++1y
+
 std_atomic_SOURCES          = std_atomic.cpp
 std_atomic_CXXFLAGS         = $(AM_CXXFLAGS) -std=c++0x -Wno-sign-compare
 
@@ -628,3 +645,5 @@ if VGCONF_OS_IS_SOLARIS
 swapcontext_CFLAGS          += -D__EXTENSIONS__
 endif
 
+timed_mutex_SOURCES         = timed_mutex.cpp
+timed_mutex_CXXFLAGS        = $(AM_CXXFLAGS) -std=c++0x
diff --git a/drd/tests/shared_timed_mutex.cpp b/drd/tests/shared_timed_mutex.cpp
new file mode 100644 (file)
index 0000000..306d6bf
--- /dev/null
@@ -0,0 +1,63 @@
+#include <thread>
+#include <iostream>
+#include <chrono>
+#include <shared_mutex>
+#include <mutex>
+#include <cassert>
+#include <condition_variable>
+
+std::shared_timed_mutex test_mutex;
+std::mutex cv_mutex;
+std::condition_variable cv;
+int global;
+bool reads_done = false;
+void f()
+{
+    auto now=std::chrono::steady_clock::now();
+    if (test_mutex.try_lock_until(now + std::chrono::seconds(3)))
+    {
+       --global;
+       test_mutex.unlock();
+    }
+    else
+    {
+        std::cerr << "Lock failed\n";
+    }
+}
+
+void g()
+{
+    auto now=std::chrono::steady_clock::now();
+    if (test_mutex.try_lock_shared_until(now + std::chrono::seconds(2)))
+    {
+       test_mutex.unlock_shared();
+    }
+    else
+    {
+        std::cerr << "Lock shared failed\n";
+    }
+    std::unique_lock<std::mutex> lock(cv_mutex);
+    reads_done = true;
+    cv.notify_all();
+}
+int main()
+{
+    global = 1;
+    test_mutex.lock_shared();
+    std::thread t1(f);
+    std::thread t2(g);
+    {
+       std::unique_lock<std::mutex> lock(cv_mutex);
+       while (!reads_done)
+       {
+          cv.wait(lock);
+       }
+    }
+    test_mutex.unlock_shared();
+    t1.join();
+    t2.join();
+    assert(global == 0);
+}
+
diff --git a/drd/tests/shared_timed_mutex.stderr.exp b/drd/tests/shared_timed_mutex.stderr.exp
new file mode 100644 (file)
index 0000000..d18786f
--- /dev/null
@@ -0,0 +1,3 @@
+
+
+ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
diff --git a/drd/tests/shared_timed_mutex.vgtest b/drd/tests/shared_timed_mutex.vgtest
new file mode 100644 (file)
index 0000000..e3be864
--- /dev/null
@@ -0,0 +1,3 @@
+prereq: ./supported_libpthread && [ -e shared_timed_mutex ]
+vgopts: --check-stack-var=yes --read-var-info=yes
+prog: shared_timed_mutex
diff --git a/drd/tests/timed_mutex.cpp b/drd/tests/timed_mutex.cpp
new file mode 100644 (file)
index 0000000..b1bb61e
--- /dev/null
@@ -0,0 +1,29 @@
+#include <thread>
+#include <iostream>
+#include <chrono>
+#include <mutex>
+#include <cassert>
+std::timed_mutex test_mutex;
+int global;
+void f()
+{
+    auto now=std::chrono::steady_clock::now();
+    test_mutex.try_lock_until(now + std::chrono::seconds(11));
+    --global;
+    test_mutex.unlock();
+}
+int main()
+{
+    global = 0;
+    auto now=std::chrono::steady_clock::now();
+    test_mutex.try_lock_until(now + std::chrono::seconds(11));
+    ++global;
+    std::thread t(f);
+    test_mutex.unlock();
+    t.join();
+    assert(global == 0);
+}
+
diff --git a/drd/tests/timed_mutex.stderr.exp b/drd/tests/timed_mutex.stderr.exp
new file mode 100644 (file)
index 0000000..d18786f
--- /dev/null
@@ -0,0 +1,3 @@
+
+
+ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
diff --git a/drd/tests/timed_mutex.vgtest b/drd/tests/timed_mutex.vgtest
new file mode 100644 (file)
index 0000000..60769b2
--- /dev/null
@@ -0,0 +1,3 @@
+prereq: ./supported_libpthread && [ -e timed_mutex ]
+vgopts: --check-stack-var=yes --read-var-info=yes
+prog: timed_mutex
index 49c3ddcd9ea0c43486d5f2c461938c1c40b0b8b9..c9b3f85ef3f4ffd131baa3f448ce580cda1b1238 100644 (file)
@@ -1110,6 +1110,53 @@ PTH_FUNC(int, pthreadZumutexZureltimedlock, // pthread_mutex_reltimedlock
 }
 #endif
 
+//-----------------------------------------------------------
+// glibc:   pthread_mutex_clocklock
+//
+// pthread_mutex_clocklock.  Identical logic to pthread_mutex_timedlock.
+__attribute__((noinline))
+static int mutex_clocklock_WRK(pthread_mutex_t *mutex,
+                               clockid_t clockid,
+                               void *timeout)
+{
+   int    ret;
+   OrigFn fn;
+   VALGRIND_GET_ORIG_FN(fn);
+   if (TRACE_PTH_FNS) {
+      fprintf(stderr, "<< pthread_mxclocklock %p %p", mutex, timeout);
+      fflush(stderr);
+   }
+
+   DO_CREQ_v_WW(_VG_USERREQ__HG_PTHREAD_MUTEX_LOCK_PRE,
+                pthread_mutex_t*,mutex, long,1/*isTryLock-ish*/);
+
+   CALL_FN_W_WWW(ret, fn, mutex, clockid, timeout);
+
+   /* There's a hole here: libpthread now knows the lock is locked,
+      but the tool doesn't, so some other thread could run and detect
+      that the lock has been acquired by someone (this thread).  Does
+      this matter?  Not sure, but I don't think so. */
+
+   DO_CREQ_v_WW(_VG_USERREQ__HG_PTHREAD_MUTEX_LOCK_POST,
+                pthread_mutex_t *, mutex, long, (ret == 0) ? True : False);
+
+   if (ret != 0) {
+      if (ret != ETIMEDOUT)
+         DO_PthAPIerror( "pthread_mutex_clocklock", ret );
+   }
+
+   if (TRACE_PTH_FNS) {
+      fprintf(stderr, " :: mxclocklock -> %d >>\n", ret);
+   }
+   return ret;
+}
+
+PTH_FUNC(int, pthreadZumutexZuclocklock, // pthread_mutex_clocklock
+         pthread_mutex_t *mutex,
+         clockid_t clockid,
+         void *timeout) {
+   return mutex_clocklock_WRK(mutex, clockid, timeout);
+}
 
 //-----------------------------------------------------------
 // glibc:   pthread_mutex_unlock
@@ -2685,6 +2732,48 @@ PTH_FUNC(int, pthreadZurwlockZutimedrdlock, // pthread_rwlock_timedrdlock
 #  error "Unsupported OS"
 #endif
 
+//-----------------------------------------------------------
+// glibc:   pthread_rwlock_clockrdlock
+//
+__attribute__((noinline)) __attribute__((unused))
+static int pthread_rwlock_clockrdlock_WRK(pthread_rwlock_t *rwlock,
+                                          clockid_t clockid,
+                                          const struct timespec *timeout)
+{
+   int    ret;
+   OrigFn fn;
+   VALGRIND_GET_ORIG_FN(fn);
+   if (TRACE_PTH_FNS) {
+      fprintf(stderr, "<< pthread_rwl_clockrdl %p", rwlock); fflush(stderr);
+   }
+
+   DO_CREQ_v_WWW(_VG_USERREQ__HG_PTHREAD_RWLOCK_LOCK_PRE,
+                 pthread_rwlock_t *, rwlock,
+                 long, 0/*isW*/, long, 0/*isTryLock*/);
+
+   CALL_FN_W_WWW(ret, fn, rwlock, clockid, timeout);
+
+   DO_CREQ_v_WWW(_VG_USERREQ__HG_PTHREAD_RWLOCK_LOCK_POST,
+                 pthread_rwlock_t *, rwlock, long, 0/*isW*/,
+                 long, (ret == 0) ? True : False);
+   if (ret != 0) {
+      DO_PthAPIerror("pthread_rwlock_clockrdlock", ret);
+   }
+
+   if (TRACE_PTH_FNS) {
+      fprintf(stderr, " :: rwl_clockrdl -> %d >>\n", ret);
+   }
+   return ret;
+}
+#if defined(VGO_linux)
+PTH_FUNC(int, pthreadZurwlockZuclockrdlock, // pthread_rwlock_clockrdlock
+              pthread_rwlock_t *rwlock,
+              clockid_t clockid,
+              const struct timespec *timeout) {
+   return pthread_rwlock_clockrdlock_WRK(rwlock, clockid, timeout);
+}
+#endif
+
 
 //-----------------------------------------------------------
 // glibc:   Unhandled
@@ -2746,6 +2835,49 @@ PTH_FUNC(int, pthreadZurwlockZutimedwrlock, // pthread_rwlock_timedwrlock
 #endif
 
 
+//-----------------------------------------------------------
+// glibc:   pthread_rwlock_clockwrlock
+//
+__attribute__((noinline)) __attribute__((unused))
+static int pthread_rwlock_clockwrlock_WRK(pthread_rwlock_t *rwlock,
+                                          clockid_t clockid,
+                                          const struct timespec *timeout)
+{
+   int    ret;
+   OrigFn fn;
+   VALGRIND_GET_ORIG_FN(fn);
+   if (TRACE_PTH_FNS) {
+      fprintf(stderr, "<< pthread_rwl_clockwrl %p", rwlock); fflush(stderr);
+   }
+
+   DO_CREQ_v_WWW(_VG_USERREQ__HG_PTHREAD_RWLOCK_LOCK_PRE,
+                 pthread_rwlock_t *, rwlock,
+                 long, 1/*isW*/, long, 0/*isTryLock*/);
+
+   CALL_FN_W_WWW(ret, fn, rwlock, clockid, timeout);
+
+   DO_CREQ_v_WWW(_VG_USERREQ__HG_PTHREAD_RWLOCK_LOCK_POST,
+                 pthread_rwlock_t *, rwlock, long, 1/*isW*/,
+                 long, (ret == 0) ? True : False);
+   if (ret != 0) {
+      DO_PthAPIerror("pthread_rwlock_clockwrlock", ret);
+   }
+
+   if (TRACE_PTH_FNS) {
+      fprintf(stderr, " :: rwl_clockwrl -> %d >>\n", ret);
+   }
+   return ret;
+}
+#if defined(VGO_linux)
+PTH_FUNC(int, pthreadZurwlockZuclockwrlock, // pthread_rwlock_clockwrlock
+              pthread_rwlock_t *rwlock,
+              clockid_t clockid,
+              const struct timespec *timeout) {
+   return pthread_rwlock_clockwrlock_WRK(rwlock, clockid, timeout);
+}
+#endif
+
+
 //-----------------------------------------------------------
 // glibc:   pthread_rwlock_unlock
 // darwin:  pthread_rwlock_unlock