From: Julian Seward Date: Mon, 21 Jan 2008 14:19:07 +0000 (+0000) Subject: drd changes (Bart Van Assche) X-Git-Tag: svn/VALGRIND_3_4_0~1094 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=02c9054a2b521f3121b19318b4372b3f4d5cae83;p=thirdparty%2Fvalgrind.git 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 --- diff --git a/exp-drd/TODO.txt b/exp-drd/TODO.txt index 123226e468..4e089939f7 100644 --- a/exp-drd/TODO.txt +++ b/exp-drd/TODO.txt @@ -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 diff --git a/exp-drd/drd_intercepts.c b/exp-drd/drd_intercepts.c index ad6746c4a2..0bd716334c 100644 --- a/exp-drd/drd_intercepts.c +++ b/exp-drd/drd_intercepts.c @@ -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; diff --git a/exp-drd/drd_main.c b/exp-drd/drd_main.c index 5353cfb253..b70151cd7a 100644 --- a/exp-drd/drd_main.c +++ b/exp-drd/drd_main.c @@ -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; diff --git a/exp-drd/drd_mutex.c b/exp-drd/drd_mutex.c index 39aec2374d..dc517f84da 100644 --- a/exp-drd/drd_mutex.c +++ b/exp-drd/drd_mutex.c @@ -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: + */ diff --git a/exp-drd/drd_mutex.h b/exp-drd/drd_mutex.h index b5b06e4834..3a291795b8 100644 --- a/exp-drd/drd_mutex.h +++ b/exp-drd/drd_mutex.h @@ -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); diff --git a/exp-drd/tests/Makefile.am b/exp-drd/tests/Makefile.am index 4e68ed311c..e60b41bd89 100644 --- a/exp-drd/tests/Makefile.am +++ b/exp-drd/tests/Makefile.am @@ -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 diff --git a/exp-drd/tests/matinv.c b/exp-drd/tests/matinv.c index fdd86993cd..8e92754c3d 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 #include #include diff --git a/exp-drd/tests/pth_barrier.c b/exp-drd/tests/pth_barrier.c new file mode 100644 index 0000000000..c436c916bd --- /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 +#include +#include +#include + + +/*********************/ +/* 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 0000000000..d18786f806 --- /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 0000000000..0c7d29dc9d --- /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 c6f0a9f62c..a097b0a636 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)