An error message is now printed if two different threads call

pthread_cond_*wait() on the same condition variable but with a different
mutex argument. Added regression test pth_inconsistent_cond_wait.

git-svn-id: svn://svn.valgrind.org/valgrind/trunk@8298

diff --git a/exp-drd/drd_cond.c b/exp-drd/drd_cond.c
index 46b994e6730fbff1c9d8d6d2ae35011832373542..7cdd581ad8f5f8d9af0c2480e557176cc8bd7701 100644 (file)
--- a/exp-drd/drd_cond.c
+++ b/exp-drd/drd_cond.c
@@ -203,12 +203,16 @@ int cond_pre_wait(const Addr cond, const Addr mutex)
   {
     p->mutex = mutex;
   }
-  else
+  else if (p->mutex != mutex)
   {
-    // TO DO: print a proper error message if two different threads call
-    // pthread_cond_*wait() on the same condition variable but with a different
-    // mutex argument.
-    tl_assert(p->mutex == mutex);
+    CondWaitErrInfo cwei
+      = { .cond = cond, .mutex1 = p->mutex, .mutex2 = mutex };
+    VG_(maybe_record_error)(VG_(get_running_tid)(),
+                            CondWaitErr,
+                            VG_(get_IP)(VG_(get_running_tid)()),
+                            "Inconsistent association of condition variable"
+                            " and mutex",
+                            &cwei);
   }
   return ++p->waiter_count;
 }

diff --git a/exp-drd/drd_error.c b/exp-drd/drd_error.c
index 842a6379dc4f0d44b987b18d9ab37f62d16a2770..e66fdac5762c8c3f3d5c8c3191b799ac5734934b 100644 (file)
--- a/exp-drd/drd_error.c
+++ b/exp-drd/drd_error.c
@@ -187,6 +187,17 @@ static void drd_tool_error_pp(Error* const e)
     VG_(pp_ExeContext)(VG_(get_error_where)(e));
     break;
   }
+  case CondDestrErr: {
+    CondDestrErrInfo* cdi = (CondDestrErrInfo*)(VG_(get_error_extra)(e));
+    VG_(message)(Vg_UserMsg,
+                 "%s: cond 0x%lx, mutex 0x%lx locked by thread %d/%d",
+                 VG_(get_error_string)(e),
+                 cdi->cond, cdi->mutex,
+                 DrdThreadIdToVgThreadId(cdi->tid), cdi->tid);
+    VG_(pp_ExeContext)(VG_(get_error_where)(e));
+    mutex_first_observed(cdi->mutex);
+    break;
+  }
   case CondRaceErr: {
     CondRaceErrInfo* cei = (CondRaceErrInfo*)(VG_(get_error_extra)(e));
     VG_(message)(Vg_UserMsg,
@@ -198,15 +209,17 @@ static void drd_tool_error_pp(Error* const e)
     mutex_first_observed(cei->mutex);
     break;
   }
-  case CondDestrErr: {
-    CondDestrErrInfo* cdi = (CondDestrErrInfo*)(VG_(get_error_extra)(e));
+  case CondWaitErr: {
+    CondWaitErrInfo* cwei = (CondWaitErrInfo*)(VG_(get_error_extra)(e));
     VG_(message)(Vg_UserMsg,
-                 "%s: cond 0x%lx, mutex 0x%lx locked by thread %d/%d",
+                 "%s: condition variable 0x%lx, mutexes 0x%lx and 0x%lx",
                  VG_(get_error_string)(e),
-                 cdi->cond, cdi->mutex,
-                 DrdThreadIdToVgThreadId(cdi->tid), cdi->tid);
+                 cwei->cond,
+                 cwei->mutex1,
+                 cwei->mutex2);
     VG_(pp_ExeContext)(VG_(get_error_where)(e));
-    mutex_first_observed(cdi->mutex);
+    mutex_first_observed(cwei->mutex1);
+    mutex_first_observed(cwei->mutex2);
     break;
   }
   case SemaphoreErr: {
@@ -279,10 +292,12 @@ static UInt drd_tool_error_update_extra(Error* e)
     return sizeof(MutexErrInfo);
   case CondErr:
     return sizeof(CondErrInfo);
-  case CondRaceErr:
-    return sizeof(CondRaceErrInfo);
   case CondDestrErr:
     return sizeof(CondDestrErrInfo);
+  case CondRaceErr:
+    return sizeof(CondRaceErrInfo);
+  case CondWaitErr:
+    return sizeof(CondWaitErrInfo);
   case SemaphoreErr:
     return sizeof(SemaphoreErrInfo);
   case BarrierErr:
@@ -309,9 +324,11 @@ static Bool drd_tool_error_recog(Char* const name, Supp* const supp)
     ;
   else if (VG_(strcmp)(name, STR_CondErr) == 0)
     ;
+  else if (VG_(strcmp)(name, STR_CondDestrErr) == 0)
+    ;
   else if (VG_(strcmp)(name, STR_CondRaceErr) == 0)
     ;
-  else if (VG_(strcmp)(name, STR_CondDestrErr) == 0)
+  else if (VG_(strcmp)(name, STR_CondWaitErr) == 0)
     ;
   else if (VG_(strcmp)(name, STR_SemaphoreErr) == 0)
     ;
@@ -350,8 +367,9 @@ static Char* drd_tool_error_name(Error* e)
   case DataRaceErr:  return VGAPPEND(STR_, DataRaceErr);
   case MutexErr:     return VGAPPEND(STR_, MutexErr);
   case CondErr:      return VGAPPEND(STR_, CondErr);
-  case CondRaceErr:  return VGAPPEND(STR_, CondRaceErr);
   case CondDestrErr: return VGAPPEND(STR_, CondDestrErr);
+  case CondRaceErr:  return VGAPPEND(STR_, CondRaceErr);
+  case CondWaitErr:  return VGAPPEND(STR_, CondWaitErr);
   case SemaphoreErr: return VGAPPEND(STR_, SemaphoreErr);
   case BarrierErr:   return VGAPPEND(STR_, BarrierErr);
   case RwlockErr:    return VGAPPEND(STR_, RwlockErr);

diff --git a/exp-drd/drd_error.h b/exp-drd/drd_error.h
index fbff475d5959dcb061f011d194488c40ba9b44f5..6b56051c372e93d04648c7a934f6e535ef7c61f8 100644 (file)
--- a/exp-drd/drd_error.h
+++ b/exp-drd/drd_error.h
@@ -43,20 +43,22 @@ typedef enum {
    MutexErr       = 2,
 #define STR_CondErr      "CondErr"
    CondErr        = 3,
-#define STR_CondRaceErr  "CondRaceErr"
-   CondRaceErr    = 4,
 #define STR_CondDestrErr "CondDestrErr"
-   CondDestrErr   = 5,
+   CondDestrErr   = 4,
+#define STR_CondRaceErr  "CondRaceErr"
+   CondRaceErr    = 5,
+#define STR_CondWaitErr  "CondWaitErr"
+   CondWaitErr    = 6,
 #define STR_SemaphoreErr "SemaphoreErr"
-   SemaphoreErr   = 6,
+   SemaphoreErr   = 7,
 #define STR_BarrierErr   "BarrierErr"
-   BarrierErr     = 7,
+   BarrierErr     = 8,
 #define STR_RwlockErr    "RwlockErr"
-   RwlockErr      = 8,
+   RwlockErr      = 9,
 #define STR_HoldtimeErr  "HoldtimeErr"
-   HoldtimeErr    = 9,
+   HoldtimeErr    = 10,
 #define STR_GenericErr   "GenericErr"
-   GenericErr     = 10,
+   GenericErr     = 11,
 } DrdErrorKind;
 
 /* The classification of a faulting address. */
@@ -105,16 +107,22 @@ typedef struct {
    Addr cond;
 } CondErrInfo;
 
+typedef struct {
+   Addr        cond;
+   Addr        mutex;
+   DrdThreadId tid;
+} CondDestrErrInfo;
+
 typedef struct {
    Addr cond;
    Addr mutex;
 } CondRaceErrInfo;
 
 typedef struct {
-   Addr        cond;
-   Addr        mutex;
-   DrdThreadId tid;
-} CondDestrErrInfo;
+   Addr cond;
+   Addr mutex1;
+   Addr mutex2;
+} CondWaitErrInfo;
 
 typedef struct {
    Addr semaphore;

diff --git a/exp-drd/tests/Makefile.am b/exp-drd/tests/Makefile.am
index 5c7af6df927594d996e00545bcfb0b4715824ac6..a97d7aa029b35b06f7b72bf128543167a4878ad2 100644 (file)
--- a/exp-drd/tests/Makefile.am
+++ b/exp-drd/tests/Makefile.am
@@ -84,6 +84,8 @@ EXTRA_DIST =                                        \
        pth_detached_sem.stderr.exp                 \
        pth_detached_sem.stdout.exp                 \
        pth_detached_sem.vgtest                     \
+       pth_inconsistent_cond_wait.stderr.exp                 \
+       pth_inconsistent_cond_wait.vgtest                     \
        recursive_mutex.stderr.exp                  \
        recursive_mutex.stdout.exp                  \
        recursive_mutex.vgtest                      \
@@ -184,6 +186,7 @@ check_PROGRAMS_COMMON = \
   pth_create_chain    \
   pth_detached        \
   pth_detached_sem    \
+  pth_inconsistent_cond_wait \
   recursive_mutex     \
   rwlock_race         \
   rwlock_test         \
@@ -285,6 +288,9 @@ pth_detached_LDADD          = -lpthread
 pth_detached_sem_SOURCES    = pth_detached_sem.c
 pth_detached_sem_LDADD      = -lpthread
 
+pth_inconsistent_cond_wait_SOURCES = pth_inconsistent_cond_wait.c
+pth_inconsistent_cond_wait_LDADD   = -lpthread
+
 recursive_mutex_SOURCES     = recursive_mutex.c
 recursive_mutex_LDADD       = -lpthread
 

diff --git a/exp-drd/tests/pth_inconsistent_cond_wait.c b/exp-drd/tests/pth_inconsistent_cond_wait.c
new file mode 100644 (file)
index 0000000..0f8895a
--- /dev/null
+++ b/exp-drd/tests/pth_inconsistent_cond_wait.c
@@ -0,0 +1,45 @@
+#include <pthread.h>
+#include <semaphore.h>
+#include <unistd.h>
+
+pthread_cond_t  s_cond;
+pthread_mutex_t s_mutex1;
+pthread_mutex_t s_mutex2;
+sem_t           s_sem;
+
+void* thread1(void* arg)
+{
+  pthread_mutex_lock(&s_mutex1);
+  sem_post(&s_sem);
+  pthread_cond_wait(&s_cond, &s_mutex1);
+  pthread_mutex_unlock(&s_mutex1);
+  return 0;
+}
+
+void* thread2(void* arg)
+{
+  pthread_mutex_lock(&s_mutex2);
+  sem_post(&s_sem);
+  pthread_cond_wait(&s_cond, &s_mutex2);
+  pthread_mutex_unlock(&s_mutex2);
+  return 0;
+}
+
+int main(int argc, char** argv)
+{
+  pthread_t tid1;
+  pthread_t tid2;
+
+  sem_init(&s_sem, 0, 2);
+  pthread_cond_init(&s_cond, 0);
+  pthread_mutex_init(&s_mutex1, 0);
+  pthread_mutex_init(&s_mutex2, 0);
+  pthread_create(&tid1, 0, &thread1, 0);
+  pthread_create(&tid2, 0, &thread2, 0);
+  sem_wait(&s_sem);
+  pthread_cond_signal(&s_cond);
+  pthread_cond_signal(&s_cond);
+  pthread_join(tid1, 0);
+  pthread_join(tid2, 0);
+  return 0;
+}

diff --git a/exp-drd/tests/pth_inconsistent_cond_wait.stderr.exp b/exp-drd/tests/pth_inconsistent_cond_wait.stderr.exp
new file mode 100644 (file)
index 0000000..5300ec0
--- /dev/null
+++ b/exp-drd/tests/pth_inconsistent_cond_wait.stderr.exp
@@ -0,0 +1,31 @@
+
+Thread 3:
+Inconsistent association of condition variable and mutex: condition variable 0x........, mutexes 0x........ and 0x........
+   at 0x........: pthread_cond_wait* (drd_pthread_intercepts.c:?)
+   by 0x........: thread2 (pth_inconsistent_cond_wait.c:?)
+   by 0x........: vg_thread_wrapper (drd_pthread_intercepts.c:?)
+   by 0x........: (within libpthread-?.?.so)
+   by 0x........: clone (in /...libc...)
+Mutex 0x........ was first observed at:
+   at 0x........: pthread_mutex_init (drd_pthread_intercepts.c:?)
+   by 0x........: main (pth_inconsistent_cond_wait.c:?)
+Mutex 0x........ was first observed at:
+   at 0x........: pthread_mutex_init (drd_pthread_intercepts.c:?)
+   by 0x........: main (pth_inconsistent_cond_wait.c:?)
+
+Thread 1:
+Probably a race condition: condition variable 0x........ has been signaled but the associated mutex 0x........ is not locked by the signalling thread.
+   at 0x........: pthread_cond_signal* (drd_pthread_intercepts.c:?)
+   by 0x........: main (pth_inconsistent_cond_wait.c:?)
+Mutex 0x........ was first observed at:
+   at 0x........: pthread_mutex_init (drd_pthread_intercepts.c:?)
+   by 0x........: main (pth_inconsistent_cond_wait.c:?)
+
+Probably a race condition: condition variable 0x........ has been signaled but the associated mutex 0x........ is not locked by the signalling thread.
+   at 0x........: pthread_cond_signal* (drd_pthread_intercepts.c:?)
+   by 0x........: main (pth_inconsistent_cond_wait.c:?)
+Mutex 0x........ was first observed at:
+   at 0x........: pthread_mutex_init (drd_pthread_intercepts.c:?)
+   by 0x........: main (pth_inconsistent_cond_wait.c:?)
+
+ERROR SUMMARY: 3 errors from 3 contexts (suppressed: 0 from 0)

diff --git a/exp-drd/tests/pth_inconsistent_cond_wait.vgtest b/exp-drd/tests/pth_inconsistent_cond_wait.vgtest
new file mode 100644 (file)
index 0000000..556b2f2
--- /dev/null
+++ b/exp-drd/tests/pth_inconsistent_cond_wait.vgtest
@@ -0,0 +1,2 @@
+prereq: ./supported_libpthread
+prog: pth_inconsistent_cond_wait