]> git.ipfire.org Git - thirdparty/valgrind.git/commitdiff
drd changes (Bart Van Assche)
authorJulian Seward <jseward@acm.org>
Mon, 21 Jan 2008 14:19:07 +0000 (14:19 +0000)
committerJulian Seward <jseward@acm.org>
Mon, 21 Jan 2008 14:19:07 +0000 (14:19 +0000)
- The exp-drd regression tests now run without producing assertion
failures and without hanging on Red Hat 7.3. It doesn't make sense
however to run exp-drd on Red Hat 7.3 -- while exp-drd works fine with
the NPTL, more work would be required to make exp-drd work with
linuxthreads.
- Converted several tl_assert() calls into error messages.
- Added a regression test called pth_barrier, which tests whether data
races are detected in a program that uses barriers. The output exp-drd
produces for this test program is not yet correct however.
- Updated exp-drd/TODO.txt.

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

exp-drd/TODO.txt
exp-drd/drd_intercepts.c
exp-drd/drd_main.c
exp-drd/drd_mutex.c
exp-drd/drd_mutex.h
exp-drd/tests/Makefile.am
exp-drd/tests/matinv.c
exp-drd/tests/pth_barrier.c [new file with mode: 0644]
exp-drd/tests/pth_barrier.stderr.exp [new file with mode: 0644]
exp-drd/tests/pth_barrier.vgtest [new file with mode: 0644]
exp-drd/tests/pth_cond_race.stderr.exp

index 123226e4686b155501865853f0162b40e651b9f1..4e089939f77c46f0f140d38c71375662824d3e03 100644 (file)
@@ -5,7 +5,6 @@ Last updated February 22, 2006
 Data-race detection algorithm
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 - pthread rwlock state tracking and support.
-- Fix Fedora 7 / Fedora 8 pth_cond_race regression test failure.
 - Implement segment merging, such that the number of segments per thread
   remains limited even when there is no synchronization between threads.
 - Find out why a race is reported on std::string::string(std::string const&)
@@ -13,6 +12,8 @@ Data-race detection algorithm
 - Eliminate the upper bounds on the number of mutexes, condition variables,
   semaphores and barriers by converting arrays into OSet's.
 - Add a regression test for pthread_mutex_timedlock().
+- Find a way for suppressing races on _IO_2_1_stdout (this race is triggered
+  by calling printf() from more than one thread).
 - Performance testing and tuning.
 - testing on PPC and AIX (current implementation is only tested on X86 and
   AMD64).
@@ -40,6 +41,9 @@ Known bugs
 ~~~~~~~~~~
 - Gets killed by the OOM handler for realistically sized applications,
   e.g. knode and OpenOffice.
+- When pthread_barrier_wait() is called, some real races are suppressed.
+- Does not work with a glibc library compiled with linuxthreads -- NPTL is
+  required for proper operation.
 - [x86_64] Reports "Allocation context: unknown" for BSS symbols on AMD64
   (works fine on i386). This is a bug in Valgrind's debug info reader
   -- VG_(find_seginfo)() returns NULL for BSS symbols on x86_64. Not yet in
index ad6746c4a24f1e8253d5cff86a54368f5982515c..0bd716334ccd493c9baee8be2c72366dae50f7c8 100644 (file)
@@ -165,7 +165,7 @@ static void vg_set_main_thread_state(void)
 }
 
 // pthread_create
-PTH_FUNC(int, pthreadZucreateZAZa, // pthread_create@*
+PTH_FUNC(int, pthreadZucreateZa, // pthread_create*
               pthread_t *thread, const pthread_attr_t *attr,
               void *(*start) (void *), void *arg)
 {
@@ -362,7 +362,7 @@ PTH_FUNC(int, pthreadZumutexZuunlock, // pthread_mutex_unlock
 }
 
 // pthread_cond_init
-PTH_FUNC(int, pthreadZucondZuinitZAZa, // pthread_cond_init@*
+PTH_FUNC(int, pthreadZucondZuinitZa, // pthread_cond_init*
               pthread_cond_t* cond,
               const pthread_condattr_t* attr)
 {
@@ -377,7 +377,7 @@ PTH_FUNC(int, pthreadZucondZuinitZAZa, // pthread_cond_init@*
 }
 
 // pthread_cond_destroy
-PTH_FUNC(int, pthreadZucondZudestroyZAZa, // pthread_cond_destroy@*
+PTH_FUNC(int, pthreadZucondZudestroyZa, // pthread_cond_destroy*
               pthread_cond_t* cond)
 {
    int ret;
@@ -391,7 +391,7 @@ PTH_FUNC(int, pthreadZucondZudestroyZAZa, // pthread_cond_destroy@*
 }
 
 // pthread_cond_wait
-PTH_FUNC(int, pthreadZucondZuwaitZAZa, // pthread_cond_wait@*
+PTH_FUNC(int, pthreadZucondZuwaitZa, // pthread_cond_wait*
               pthread_cond_t *cond,
               pthread_mutex_t *mutex)
 {
@@ -408,7 +408,7 @@ PTH_FUNC(int, pthreadZucondZuwaitZAZa, // pthread_cond_wait@*
 }
 
 // pthread_cond_timedwait
-PTH_FUNC(int, pthreadZucondZutimedwaitZAZa, // pthread_cond_timedwait@*
+PTH_FUNC(int, pthreadZucondZutimedwaitZa, // pthread_cond_timedwait*
               pthread_cond_t *cond,
               pthread_mutex_t *mutex,
               const struct timespec* abstime)
@@ -426,7 +426,7 @@ PTH_FUNC(int, pthreadZucondZutimedwaitZAZa, // pthread_cond_timedwait@*
 }
 
 // pthread_cond_signal
-PTH_FUNC(int, pthreadZucondZusignalZAZa, // pthread_cond_signal@*
+PTH_FUNC(int, pthreadZucondZusignalZa, // pthread_cond_signal*
               pthread_cond_t* cond)
 {
    int   ret;
@@ -440,7 +440,7 @@ PTH_FUNC(int, pthreadZucondZusignalZAZa, // pthread_cond_signal@*
 }
 
 // pthread_cond_broadcast
-PTH_FUNC(int, pthreadZucondZubroadcastZAZa, // pthread_cond_broadcast@*
+PTH_FUNC(int, pthreadZucondZubroadcastZa, // pthread_cond_broadcast*
               pthread_cond_t* cond)
 {
    int   ret;
index 5353cfb2537d93d49382f78a50e0cf431371fe06..b70151cd7acecc6ea9be13585e81858c46a83f49 100644 (file)
@@ -401,24 +401,15 @@ static void drd_thread_finished(ThreadId tid)
    thread_finished(drd_tid);
 }
 
-void drd_pre_mutex_init(Addr mutex, SizeT size, MutexT mutex_type)
+void drd_pre_mutex_init(const Addr mutex, const SizeT size,
+                       const MutexT mutex_type)
 {
    mutex_init(mutex, size, mutex_type);
 }
 
-void drd_post_mutex_destroy(Addr mutex, MutexT mutex_type)
+void drd_post_mutex_destroy(const Addr mutex, const MutexT mutex_type)
 {
-   struct mutex_info* p;
-
-   p = mutex_get(mutex);
-   tl_assert(p);
-   if (p)
-   {
-      // TO DO: report an error in case the recursion count is not zero
-      // before asserting.
-      tl_assert(mutex_get_recursion_count(mutex) == 0);
-      mutex_destroy(p);
-   }
+   mutex_post_destroy(mutex);
 }
 
 void drd_pre_mutex_lock(const DrdThreadId drd_tid,
@@ -562,11 +553,11 @@ void drd_post_clo_init(void)
 
 static
 IRSB* drd_instrument(VgCallbackClosure* const closure,
-                    IRSB* const bb_in,
-                    VexGuestLayout* const layout,
-                    VexGuestExtents* const vge, 
-                    IRType const gWordTy,
-                    IRType const hWordTy)
+                     IRSB* const bb_in,
+                     VexGuestLayout* const layout,
+                     VexGuestExtents* const vge, 
+                     IRType const gWordTy,
+                     IRType const hWordTy)
 {
    IRDirty* di;
    Int      i;
index 39aec2374ddd964ab56d2851884cf5d42d89c711..dc517f84da67738734c6dfa85609c9c2dcb3b1c0 100644 (file)
@@ -47,6 +47,11 @@ struct mutex_info
 };
 
 
+// Local functions.
+
+static void mutex_destroy(struct mutex_info* const p);
+
+
 // Local variables.
 
 static Bool s_trace_mutex;
@@ -120,11 +125,6 @@ mutex_init(const Addr mutex, const SizeT size, const MutexT mutex_type)
 {
   struct mutex_info* mutex_p;
 
-  tl_assert(mutex_get(mutex) == 0);
-  tl_assert(mutex_type == mutex_type_mutex
-            || mutex_type == mutex_type_spinlock);
-  mutex_p = mutex_get_or_allocate(mutex, size, mutex_type);
-
   if (s_trace_mutex)
   {
     const ThreadId vg_tid = VG_(get_running_tid)();
@@ -132,14 +132,31 @@ mutex_init(const Addr mutex, const SizeT size, const MutexT mutex_type)
     VG_(message)(Vg_DebugMsg,
                  "drd_post_mutex_init  tid = %d/%d, %s 0x%lx",
                  vg_tid, drd_tid,
-                 mutex_get_typename(mutex_p),
+                 mutex_type_name(mutex_type),
                  mutex);
   }
 
+  tl_assert(mutex_type == mutex_type_mutex
+            || mutex_type == mutex_type_spinlock);
+  mutex_p = mutex_get(mutex);
+  if (mutex_p)
+  {
+    const ThreadId vg_tid = VG_(get_running_tid)();
+    MutexErrInfo MEI
+      = { mutex_p->mutex, mutex_p->recursion_count, mutex_p->owner };
+    VG_(maybe_record_error)(vg_tid,
+                            MutexErr,
+                            VG_(get_IP)(vg_tid),
+                            "Mutex reinitialization",
+                            &MEI);
+    mutex_destroy(mutex_p);
+  }
+  mutex_p = mutex_get_or_allocate(mutex, size, mutex_type);
+
   return mutex_p;
 }
 
-void mutex_destroy(struct mutex_info* const p)
+static void mutex_destroy(struct mutex_info* const p)
 {
   if (s_trace_mutex)
   {
@@ -158,6 +175,33 @@ void mutex_destroy(struct mutex_info* const p)
   p->mutex = 0;
 }
 
+void mutex_pre_destroy(struct mutex_info* const p)
+{
+   return mutex_destroy(p);
+}
+
+void mutex_post_destroy(const Addr mutex)
+{
+   struct mutex_info* p;
+
+   p = mutex_get(mutex);
+   tl_assert(p);
+   if (p)
+   {
+      if (mutex_get_recursion_count(mutex) > 0)
+      {
+        const ThreadId vg_tid = VG_(get_running_tid)();
+         MutexErrInfo MEI = { p->mutex, p->recursion_count, p->owner };
+         VG_(maybe_record_error)(vg_tid,
+                                 MutexErr,
+                                 VG_(get_IP)(vg_tid),
+                                 "Destroying locked mutex",
+                                 &MEI);
+      }
+      mutex_pre_destroy(p);
+   }
+}
+
 struct mutex_info* mutex_get(const Addr mutex)
 {
   int i;
@@ -215,7 +259,7 @@ int mutex_lock(const Addr mutex, const SizeT size, MutexT mutex_type)
                  " simultaneously by two threads (recursion count %d,"
                  " owners %d and %d) !",
                  p->mutex, p->recursion_count, p->owner, drd_tid);
-    tl_assert(0);
+    p->owner = drd_tid;
   }
   p->recursion_count++;
 
@@ -271,7 +315,18 @@ int mutex_unlock(const Addr mutex, const MutexT mutex_type)
                             &MEI);
   }
   p->recursion_count--;
-  tl_assert(p->recursion_count >= 0);
+  if (p->recursion_count < 0)
+  {
+    MutexErrInfo MEI
+      = { p->mutex, p->recursion_count, p->owner };
+    VG_(maybe_record_error)(vg_tid,
+                            MutexErr,
+                            VG_(get_IP)(vg_tid),
+                            "Attempt to unlock a mutex that is not locked",
+                            &MEI);
+    p->recursion_count = 0;
+  }
+
   if (p->recursion_count == 0)
   {
     /* This pthread_mutex_unlock() call really unlocks the mutex. Save the */
@@ -288,7 +343,12 @@ const char* mutex_get_typename(struct mutex_info* const p)
 {
   tl_assert(p);
 
-  switch (p->mutex_type)
+  return mutex_type_name(p->mutex_type);
+}
+
+const char* mutex_type_name(const MutexT mt)
+{
+  switch (mt)
   {
   case mutex_type_mutex:
     return "mutex";
@@ -360,3 +420,10 @@ ULong get_mutex_lock_count(void)
 {
   return s_mutex_lock_count;
 }
+
+
+/*
+ * Local variables:
+ * c-basic-offset: 3
+ * End:
+ */
index b5b06e483466e3189983319cdf56ce03adc3cc3e..3a291795b8561ba8d58c3c6b5233bdcdbedff765 100644 (file)
@@ -42,11 +42,13 @@ struct mutex_info;
 void mutex_set_trace(const Bool trace_mutex);
 struct mutex_info* mutex_init(const Addr mutex, const SizeT size,
                               const MutexT mutex_type);
-void mutex_destroy(struct mutex_info* const p);
+void mutex_pre_destroy(struct mutex_info* const p);
+void mutex_post_destroy(const Addr mutex);
 struct mutex_info* mutex_get(const Addr mutex);
 int mutex_lock(const Addr mutex, const SizeT size, const MutexT mutex_type);
 int mutex_unlock(const Addr mutex, const MutexT mutex_type);
 const char* mutex_get_typename(struct mutex_info* const p);
+const char* mutex_type_name(const MutexT mt);
 Bool mutex_is_locked_by(const Addr mutex, const DrdThreadId tid);
 const VectorClock* mutex_get_last_vc(const Addr mutex);
 int mutex_get_recursion_count(const Addr mutex);
index 4e68ed311cb43c5b64dddcf166e27adbadc17b6b..e60b41bd89111129797ca6b81e1af4b7e061bbb9 100644 (file)
@@ -17,6 +17,8 @@ EXTRA_DIST = $(noinst_SCRIPTS)                                  \
        fp_race2.stdout.exp fp_race2.stderr.exp                 \
        matinv.vgtest                                           \
        matinv.stdout.exp matinv.stderr.exp                     \
+       pth_barrier.vgtest                                      \
+       pth_barrier.stdout.exp pth_barrier.stderr.exp           \
        pth_broadcast.vgtest                                    \
        pth_broadcast.stdout.exp pth_broadcast.stderr.exp       \
        pth_cond_race.vgtest                                    \
@@ -48,6 +50,7 @@ AM_CXXFLAGS = $(AM_CFLAGS)
 check_PROGRAMS =   \
   fp_race          \
   matinv           \
+  pth_barrier      \
   pth_broadcast    \
   pth_cond_race    \
   pth_create_chain \
@@ -63,6 +66,9 @@ fp_race_LDADD            = -lpthread
 matinv_SOURCES           = matinv.c
 matinv_LDADD             = -lpthread -lm
 
+pth_barrier_SOURCES      = pth_barrier.c
+pth_barrier_LDADD        = -lpthread
+
 pth_broadcast_SOURCES    = pth_broadcast.c
 pth_broadcast_LDADD      = -lpthread
 
index fdd86993cd6f33fb494d75799811ef706900a310..8e92754c3d10b9ca305a534d6c342f88cc988877 100644 (file)
@@ -6,12 +6,12 @@
  */
 
 
+#define _GNU_SOURCE
+
 /***********************/
 /* Include directives. */
 /***********************/
 
-#define _GNU_SOURCE
-
 #include <assert.h>
 #include <math.h>
 #include <pthread.h>
diff --git a/exp-drd/tests/pth_barrier.c b/exp-drd/tests/pth_barrier.c
new file mode 100644 (file)
index 0000000..c436c91
--- /dev/null
@@ -0,0 +1,105 @@
+/* Test whether all data races are detected in a multithreaded program with
+ * barriers.
+ */
+
+
+#define _GNU_SOURCE
+
+/***********************/
+/* Include directives. */
+/***********************/
+
+#include <assert.h>
+#include <pthread.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+
+/*********************/
+/* Type definitions. */
+/*********************/
+
+struct threadinfo
+{
+  pthread_barrier_t* b;
+  pthread_t          tid;
+  int*               array;
+  int                iterations;
+};
+
+
+/********************/
+/* Local variables. */
+/********************/
+
+static int s_silent;
+
+
+/*************************/
+/* Function definitions. */
+/*************************/
+
+static void* threadfunc(struct threadinfo* p)
+{
+  int i;
+  int* const array = p->array;
+  pthread_barrier_t* const b = p->b;
+  if (! s_silent)
+    printf("thread %lx iteration 0\n", pthread_self());
+  pthread_barrier_wait(b);
+  for (i = 0; i < p->iterations; i++)
+  {
+    if (! s_silent)
+      printf("thread %lx iteration %d; writing to %p\n",
+             pthread_self(), i + 1, &array[i]);
+    array[i] = i;
+    pthread_barrier_wait(b);
+  }
+  return 0;
+}
+
+/** Multithreaded Gauss-Jordan algorithm. */
+static void barriers_and_races(const int nthread, const int iterations)
+{
+  int i;
+  struct threadinfo* t;
+  pthread_barrier_t b;
+  int* array;
+
+  t = malloc(nthread * sizeof(struct threadinfo));
+  array = malloc(iterations * sizeof(array[0]));
+
+  pthread_barrier_init(&b, 0, nthread);
+
+  for (i = 0; i < nthread; i++)
+  {
+    t[i].b = &b;
+    t[i].array = array;
+    t[i].iterations = iterations;
+    pthread_create(&t[i].tid, 0, (void*(*)(void*))threadfunc, &t[i]);
+  }
+
+  for (i = 0; i < nthread; i++)
+  {
+    pthread_join(t[i].tid, 0);
+  }
+
+  pthread_barrier_destroy(&b);
+
+  free(array);
+  free(t);
+}
+
+int main(int argc, char** argv)
+{
+  int nthread;
+  int iterations;
+
+  nthread = (argc > 1) ? atoi(argv[1]) : 2;
+  iterations = (argc > 2) ? atoi(argv[2]) : 3;
+  s_silent = (argc > 3) ? atoi(argv[3]) : 0;
+
+  barriers_and_races(nthread, iterations);
+
+  return 0;
+}
diff --git a/exp-drd/tests/pth_barrier.stderr.exp b/exp-drd/tests/pth_barrier.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/exp-drd/tests/pth_barrier.vgtest b/exp-drd/tests/pth_barrier.vgtest
new file mode 100644 (file)
index 0000000..0c7d29d
--- /dev/null
@@ -0,0 +1,2 @@
+prog: pth_barrier
+args: 2 1 1
index c6f0a9f62c9a8faf698c8dbe78faeb8a8db63397..a097b0a6369f67f6d1cc1bc7af0d6f9fa7c19305 100644 (file)
@@ -1,7 +1,7 @@
 
 Thread 2:
 Race condition: condition variable 0x........ has been signalled but the associated mutex 0x........ is not locked by the signalling thread
-   at 0x........: pthread_cond_signal@* (drd_intercepts.c:?)
+   at 0x........: pthread_cond_signal* (drd_intercepts.c:?)
    by 0x........: thread_func (pth_cond_race.c:?)
    by 0x........: vg_thread_wrapper (drd_intercepts.c:?)
    by 0x........: start_thread (in libpthread-?.?.so)