Bug 484480 - False positives when using sem_trywait

diff --git a/.gitignore b/.gitignore
index f8b3cc0d50b058294d686c55404d26a2939a340d..347be07dee1adff7915e4cbc40e176de3a6c3e57 100644 (file)
--- a/.gitignore
+++ b/.gitignore
@@ -668,6 +668,7 @@
 /helgrind/tests/bug322621
 /helgrind/tests/bug327548
 /helgrind/tests/bug392331
+/helgrind/tests/bug484480
 /helgrind/tests/cond_init_destroy
 /helgrind/tests/cond_timedwait_invalid
 /helgrind/tests/cond_timedwait_test
@@ -690,6 +691,7 @@
 /helgrind/tests/pth_mempcpy_false_races
 /helgrind/tests/rwlock_race
 /helgrind/tests/rwlock_test
+/helgrind/tests/sem_timedwait
 /helgrind/tests/shmem_abits
 /helgrind/tests/stackteardown
 /helgrind/tests/t2t

diff --git a/NEWS b/NEWS
index 3160bbe213ab32e171aa523c378bbcf6d6611d59..818901e7f1d73f198d82cfbfa8dcd60284103761 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -74,6 +74,7 @@ are not entered into bugzilla tend to get forgotten about or ignored.
 483786  Incorrect parameter indexing in FreeBSD clock_nanosleep syscall wrapper
 484002  Add suppression for invalid read in glibc's __wcpncpy_avx2() via wcsxfrm()
 484426  aarch64: 0.5 gets rounded to 0
+484480  False positives when using sem_trywait
 n-i-bz  Add redirect for memccpy
 
 To see details of a given bug, visit

diff --git a/configure.ac b/configure.ac
index 8279683ac3e8285cdbec74ee31d754df65c6d8fa..e7bfe41854ae33ce2d348f7cc67ff915efca8e72 100755 (executable)
--- a/configure.ac
+++ b/configure.ac
@@ -4971,7 +4971,8 @@ AC_CHECK_FUNCS([     \
         wcsncpy      \
         free_aligned_sized \
         wcpncpy      \
-        wcsxfrm
+        wcsxfrm      \
+        sem_timedwait
         ])
 
 # AC_CHECK_LIB adds any library found to the variable LIBS, and links these
@@ -5015,6 +5016,8 @@ AM_CONDITIONAL([HAVE_WCPNCPY],
                [test x$ac_cv_func_wcpncpy = xyes])
 AM_CONDITIONAL([HAVE_WCSXFRM],
                [test x$ac_cv_func_wcsxfrm = xyes])
+AM_CONDITIONAL([HAVE_SEM_TIMEDWAIT],
+               [test x$ac_cv_func_sem_timedwait = xyes])
 
 if test x$VGCONF_PLATFORM_PRI_CAPS = xMIPS32_LINUX \
      -o x$VGCONF_PLATFORM_PRI_CAPS = xMIPS64_LINUX \

diff --git a/helgrind/hg_intercepts.c b/helgrind/hg_intercepts.c
index 8c98e4ee032ba9e3d0408e9485ee96392549623b..28dca3c715092a4d72a7f7e54d890fde8fb29273 100644 (file)
--- a/helgrind/hg_intercepts.c
+++ b/helgrind/hg_intercepts.c
@@ -3192,6 +3192,132 @@ static int sem_wait_WRK(sem_t* sem)
 #  error "Unsupported OS"
 #endif
 
+//-----------------------------------------------------------
+// glibc:   sem_trywait
+// glibc:   sem_trywait@GLIBC_2.0
+// glibc:   sem_trywait@@GLIBC_2.1
+// darwin:  sem_trywait
+// Solaris: sema_trywait (sem_teywait is built on top of sema_trywait)
+// FreeBSD: sem_trywait (libc)
+//
+/* trywait: decrement semaphore if non-zero otherwise return error */
+__attribute__((noinline))
+static int sem_trywait_WRK(sem_t* sem)
+{
+   OrigFn fn;
+   int    ret;
+   VALGRIND_GET_ORIG_FN(fn);
+
+   if (TRACE_SEM_FNS) {
+      fprintf(stderr, "<< sem_trywait(%p) ", sem);
+      fflush(stderr);
+   }
+
+   DO_CREQ_v_W(_VG_USERREQ__HG_POSIX_SEM_WAIT_PRE, sem_t*,sem);
+
+   CALL_FN_W_W(ret, fn, sem);
+
+   DO_CREQ_v_WW(_VG_USERREQ__HG_POSIX_SEM_WAIT_POST, sem_t*,sem,
+                long, (ret == 0) ? True : False);
+
+   if (ret != 0) {
+      DO_PthAPIerror( "sem_trywait", SEM_ERROR );
+   }
+
+   if (TRACE_SEM_FNS) {
+      fprintf(stderr, " sem_trywait -> %d >>\n", ret);
+      fflush(stderr);
+   }
+
+   return ret;
+}
+
+#if defined(VGO_linux)
+PTH_FUNC(int, semZutrywait, sem_t* sem) { /* sem_trywait */
+   return sem_trywait_WRK(sem);
+}
+PTH_FUNC(int, semZutrywaitZAZa, sem_t* sem) { /* sem_trywait@* */
+   return sem_trywait_WRK(sem);
+}
+#elif defined(VGO_darwin)
+PTH_FUNC(int, semZutrywait, sem_t* sem) { /* sem_trywait */
+   return sem_wait_WRK(sem);
+}
+#elif defined(VGO_freebsd)
+LIBC_FUNC(int, semZutrywait, sem_t* sem) { /* sem_trywait */
+   return sem_trywait_WRK(sem);
+}
+#elif defined(VGO_solaris)
+PTH_FUNC(int, semaZutrywait, sem_t *sem) { /* sema_trywait */
+   return sem_trywait_WRK(sem);
+}
+#else
+#  error "Unsupported OS"
+#endif
+
+//-----------------------------------------------------------
+// glibc:   sem_timedwait
+// glibc:   sem_timedwait@GLIBC_2.0
+// glibc:   sem_timedwait@@GLIBC_2.1
+// darwin:  does not exist
+// Solaris: sema_timedwait (sem_timedwait is built on top of sema_timedwait)
+// FreeBSD: sem_timedwait (libc)
+//
+/* trywait: decrement semaphore if non-zero otherwise return error */
+__attribute__((noinline))
+static int sem_timedwait_WRK(sem_t* sem, const struct timespec* abs_timeout)
+{
+   OrigFn fn;
+   int    ret;
+   VALGRIND_GET_ORIG_FN(fn);
+
+   if (TRACE_SEM_FNS) {
+      fprintf(stderr, "<< sem_timedwait(%p, %p) ", sem, abs_timeout);
+      fflush(stderr);
+   }
+
+   DO_CREQ_v_W(_VG_USERREQ__HG_POSIX_SEM_WAIT_PRE, sem_t*,sem);
+
+   CALL_FN_W_WW(ret, fn, sem, abs_timeout);
+
+   DO_CREQ_v_WW(_VG_USERREQ__HG_POSIX_SEM_WAIT_POST, sem_t*,sem,
+                long, (ret == 0) ? True : False);
+
+   if (ret != 0) {
+      DO_PthAPIerror( "sem_timedwait", SEM_ERROR );
+   }
+
+   if (TRACE_SEM_FNS) {
+      fprintf(stderr, " sem_timedwait -> %d >>\n", ret);
+      fflush(stderr);
+   }
+
+   return ret;
+}
+
+#if defined(VGO_linux)
+PTH_FUNC(int, semZutimedwait, sem_t* sem, const struct timespec* abs_timeout) { /* sem_timedwait */
+   return sem_timedwait_WRK(sem, abs_timeout);
+}
+PTH_FUNC(int, semZutimedwaitZAZa, sem_t* sem, const struct timespec* abs_timeout) { /* sem_timedwait@* */
+   return sem_timedwait_WRK(sem, abs_timeout);
+}
+#elif defined(VGO_darwin)
+
+// does not exist
+
+#elif defined(VGO_freebsd)
+LIBC_FUNC(int, semZutimedwait, sem_t* sem, const struct timespec* abs_timeout) { /* sem_timedwait */
+   return sem_timedwait_WRK(sem, abs_timeout);
+}
+#elif defined(VGO_solaris)
+PTH_FUNC(int, semaZutimedwait, sem_t *sem, const struct timespec* abs_timeout) { /* sema_timedwait */
+   return sem_timedwait_WRK(sem, abs_timeout);
+}
+#else
+#  error "Unsupported OS"
+#endif
+
 
 //-----------------------------------------------------------
 // glibc:   sem_post

diff --git a/helgrind/tests/Makefile.am b/helgrind/tests/Makefile.am
index 28c0793296168608f97bc0c016cb6d4e0035c77a..927e11116c1ee9ae2fa156a62f2f3051b7af251e 100755 (executable)
--- a/helgrind/tests/Makefile.am
+++ b/helgrind/tests/Makefile.am
@@ -24,6 +24,7 @@ EXTRA_DIST = \
        bug392331.vgtest bug392331.stdout.exp bug392331.stderr.exp \
        bug392331_supp.vgtest bug392331_supp.stdout.exp bug392331_supp.stderr.exp \
        bug392331.supp \
+       bug484480.vgtest bug484480.stderr.exp bug484480.stdout.exp \
        cond_init_destroy.vgtest cond_init_destroy.stderr.exp \
        cond_timedwait_invalid.vgtest cond_timedwait_invalid.stdout.exp \
                cond_timedwait_invalid.stderr.exp \
@@ -71,6 +72,7 @@ EXTRA_DIST = \
        pth_spinlock.vgtest pth_spinlock.stdout.exp pth_spinlock.stderr.exp \
        rwlock_race.vgtest rwlock_race.stdout.exp rwlock_race.stderr.exp \
        rwlock_test.vgtest rwlock_test.stdout.exp rwlock_test.stderr.exp \
+       sem_timedwait.vgtest sem_timedwait.stdout.exp sem_timedwait.stderr.exp \
        shared_timed_mutex.vgtest shared_timed_mutex.stderr.exp \
        shmem_abits.vgtest shmem_abits.stdout.exp shmem_abits.stderr.exp \
        stackteardown.vgtest stackteardown.stdout.exp stackteardown.stderr.exp \
@@ -152,6 +154,7 @@ noinst_HEADERS = safe-pthread.h safe-semaphore.h
 check_PROGRAMS = \
        annotate_hbefore \
        bug327548 \
+       bug484480 \
        cond_init_destroy \
        cond_timedwait_invalid \
        cond_timedwait_test \
@@ -244,6 +247,10 @@ if HAVE_GETADDRINFO
 check_PROGRAMS += getaddrinfo
 endif
 
+if HAVE_SEM_TIMEDWAIT
+check_PROGRAMS += sem_timedwait
+endif
+
 AM_CFLAGS   += $(AM_FLAG_M3264_PRI)
 AM_CXXFLAGS += $(AM_FLAG_M3264_PRI)
 

diff --git a/helgrind/tests/bug484480.c b/helgrind/tests/bug484480.c
new file mode 100644 (file)
index 0000000..8faae4b
--- /dev/null
+++ b/helgrind/tests/bug484480.c
@@ -0,0 +1,33 @@
+#include <pthread.h>
+#include <semaphore.h>
+#include <stdio.h>
+
+static char* result;
+static sem_t sem;
+
+static void* func(void* data)
+{
+   result = "Finished!";
+   sem_post(&sem);
+   return NULL;
+}
+
+int main(void)
+{
+   sem_init(&sem, 0, 0);
+
+   pthread_t tid;
+   pthread_create(&tid, NULL, func, NULL);
+
+   while (sem_trywait(&sem))
+      ; // do nothing but keep retrying
+
+   /* The above loop could be replaced this instead:
+    * if (sem_trywait(&sem))
+    *    sem_wait(&sem);
+    */
+
+   printf("%s\n", result);
+
+   pthread_join(tid, NULL);
+}

diff --git a/helgrind/tests/bug484480.stderr.exp b/helgrind/tests/bug484480.stderr.exp
new file mode 100644 (file)
index 0000000..e69de29

diff --git a/helgrind/tests/bug484480.stdout.exp b/helgrind/tests/bug484480.stdout.exp
new file mode 100644 (file)
index 0000000..1264c94
--- /dev/null
+++ b/helgrind/tests/bug484480.stdout.exp
@@ -0,0 +1 @@
+Finished!

diff --git a/helgrind/tests/bug484480.vgtest b/helgrind/tests/bug484480.vgtest
new file mode 100644 (file)
index 0000000..b130a27
--- /dev/null
+++ b/helgrind/tests/bug484480.vgtest
@@ -0,0 +1,2 @@
+vgopts: -q
+prog: bug484480

diff --git a/helgrind/tests/sem_timedwait.c b/helgrind/tests/sem_timedwait.c
new file mode 100644 (file)
index 0000000..4d75aca
--- /dev/null
+++ b/helgrind/tests/sem_timedwait.c
@@ -0,0 +1,42 @@
+// this is a variation of bug484480.c using sem_timedwait
+
+#include <pthread.h>
+#include <semaphore.h>
+#include <stdio.h>
+#include <time.h>
+#include <stdlib.h>
+#include <string.h>
+
+static char* result;
+static sem_t sem;
+
+static void* func(void* data)
+{
+   result = "Finished!";
+   sem_post(&sem);
+   return NULL;
+}
+
+int main(void)
+{
+   sem_init(&sem, 0, 0);
+
+   pthread_t tid;
+   pthread_create(&tid, NULL, func, NULL);
+
+   struct timespec ts;
+
+   if (clock_gettime(CLOCK_REALTIME, &ts) == -1) {
+      perror("clock_gettime");
+      exit(EXIT_FAILURE);
+   }
+   
+   ts.tv_nsec += 100000;
+
+   while (sem_timedwait(&sem, &ts))
+      ; // do nothing but keep retrying
+
+   printf("%s\n", result);
+
+   pthread_join(tid, NULL);
+}

diff --git a/helgrind/tests/sem_timedwait.stderr.exp b/helgrind/tests/sem_timedwait.stderr.exp
new file mode 100644 (file)
index 0000000..e69de29

diff --git a/helgrind/tests/sem_timedwait.stdout.exp b/helgrind/tests/sem_timedwait.stdout.exp
new file mode 100644 (file)
index 0000000..1264c94
--- /dev/null
+++ b/helgrind/tests/sem_timedwait.stdout.exp
@@ -0,0 +1 @@
+Finished!

diff --git a/helgrind/tests/sem_timedwait.vgtest b/helgrind/tests/sem_timedwait.vgtest
new file mode 100644 (file)
index 0000000..a1c1199
--- /dev/null
+++ b/helgrind/tests/sem_timedwait.vgtest
@@ -0,0 +1,3 @@
+prereq: test -e ./sem_timedwait
+vgopts: -q
+prog: sem_timedwait