drd changes (Bart Van Assche)
- 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 a5019735-40e9-0310-863c-91ae7b9d1cf9
diff --git a/exp-drd/TODO.txt b/exp-drd/TODO.txt
index 123226e..4e08993 100644
--- a/exp-drd/TODO.txt
+++ b/exp-drd/TODO.txt
@@ -5,7 +5,6 @@
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 @@
- 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 @@
~~~~~~~~~~
- 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
diff --git a/exp-drd/drd_intercepts.c b/exp-drd/drd_intercepts.c
index ad6746c..0bd7163 100644
--- a/exp-drd/drd_intercepts.c
+++ b/exp-drd/drd_intercepts.c
@@ -165,7 +165,7 @@
}
// 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 @@
}
// 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 @@
}
// 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 @@
}
// 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 @@
}
// 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 @@
}
// 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 @@
}
// pthread_cond_broadcast
-PTH_FUNC(int, pthreadZucondZubroadcastZAZa, // pthread_cond_broadcast@*
+PTH_FUNC(int, pthreadZucondZubroadcastZa, // pthread_cond_broadcast*
pthread_cond_t* cond)
{
int ret;
diff --git a/exp-drd/drd_main.c b/exp-drd/drd_main.c
index 5353cfb..b70151c 100644
--- a/exp-drd/drd_main.c
+++ b/exp-drd/drd_main.c
@@ -401,24 +401,15 @@
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 @@
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;
diff --git a/exp-drd/drd_mutex.c b/exp-drd/drd_mutex.c
index 39aec23..dc517f8 100644
--- a/exp-drd/drd_mutex.c
+++ b/exp-drd/drd_mutex.c
@@ -47,6 +47,11 @@
};
+// Local functions.
+
+static void mutex_destroy(struct mutex_info* const p);
+
+
// Local variables.
static Bool s_trace_mutex;
@@ -120,11 +125,6 @@
{
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 @@
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 @@
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 @@
" 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 @@
&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 @@
{
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 @@
{
return s_mutex_lock_count;
}
+
+
+/*
+ * Local variables:
+ * c-basic-offset: 3
+ * End:
+ */
diff --git a/exp-drd/drd_mutex.h b/exp-drd/drd_mutex.h
index b5b06e4..3a29179 100644
--- a/exp-drd/drd_mutex.h
+++ b/exp-drd/drd_mutex.h
@@ -42,11 +42,13 @@
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);
diff --git a/exp-drd/tests/Makefile.am b/exp-drd/tests/Makefile.am
index 4e68ed3..e60b41b 100644
--- a/exp-drd/tests/Makefile.am
+++ b/exp-drd/tests/Makefile.am
@@ -17,6 +17,8 @@
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 @@
check_PROGRAMS = \
fp_race \
matinv \
+ pth_barrier \
pth_broadcast \
pth_cond_race \
pth_create_chain \
@@ -63,6 +66,9 @@
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
diff --git a/exp-drd/tests/matinv.c b/exp-drd/tests/matinv.c
index fdd8699..8e92754 100644
--- a/exp-drd/tests/matinv.c
+++ b/exp-drd/tests/matinv.c
@@ -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
index 0000000..c436c91
--- /dev/null
+++ b/exp-drd/tests/pth_barrier.c
@@ -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
index 0000000..d18786f
--- /dev/null
+++ b/exp-drd/tests/pth_barrier.stderr.exp
@@ -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
index 0000000..0c7d29d
--- /dev/null
+++ b/exp-drd/tests/pth_barrier.vgtest
@@ -0,0 +1,2 @@
+prog: pth_barrier
+args: 2 1 1
diff --git a/exp-drd/tests/pth_cond_race.stderr.exp b/exp-drd/tests/pth_cond_race.stderr.exp
index c6f0a9f..a097b0a 100644
--- a/exp-drd/tests/pth_cond_race.stderr.exp
+++ b/exp-drd/tests/pth_cond_race.stderr.exp
@@ -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)