From: Bart Van Assche Date: Fri, 29 Feb 2008 11:00:17 +0000 (+0000) Subject: Bug fix: "mutex reinitialization" error message is no longer printed for the tc09_bad... X-Git-Tag: svn/VALGRIND_3_4_0~1005 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=035f1574d5d5aef6f939ba608327e22425e2293d;p=thirdparty%2Fvalgrind.git Bug fix: "mutex reinitialization" error message is no longer printed for the tc09_bad_unlock test. git-svn-id: svn://svn.valgrind.org/valgrind/trunk@7506 --- diff --git a/exp-drd/TODO.txt b/exp-drd/TODO.txt index 6048173c28..6fc515463e 100644 --- a/exp-drd/TODO.txt +++ b/exp-drd/TODO.txt @@ -65,10 +65,6 @@ Known bugs -- VG_(find_seginfo)() returns NULL for BSS symbols on x86_64. Not yet in the KDE bug tracking system. - tc04_free_lock fails on AMD64 + openSUSE 10.3 (free() locked mutex). -- tc09_bad_unlock: the 'Mutex reinitialization' message should not be printed. -- --trace-mem=yes can cause crashes. This might be caused by the code that - prints backtraces. An example (AMD64): - ./vg-in-place --tool=exp-drd --trace-mem=yes exp-drd/tests/pth_barrier 2 2 1 Known performance issues: - According to cachegrind, VG_(OSet_Next)() is taking up most CPU cycles. diff --git a/exp-drd/drd_clientobj.c b/exp-drd/drd_clientobj.c index f5ef669629..b9f931025a 100644 --- a/exp-drd/drd_clientobj.c +++ b/exp-drd/drd_clientobj.c @@ -104,6 +104,11 @@ drd_clientobj_add(const Addr a1, const Addr a2, const ObjType t) tl_assert(a1 < a2 && a1 + 4096 > a2); tl_assert(! drd_clientobj_present(a1, a2)); tl_assert(VG_(OSetGen_Lookup)(s_clientobj, &a1) == 0); +#if 0 + VG_(message)(Vg_DebugMsg, + "registering client obj [0x%lx,0x%lx[ of type %d", + a1, a2, t); +#endif p = VG_(OSetGen_AllocNode)(s_clientobj, sizeof(*p)); VG_(memset)(p, 0, sizeof(*p)); p->any.a1 = a1; @@ -123,8 +128,8 @@ Bool drd_clientobj_remove(const Addr addr) if (p) { #if 0 - VG_(message)(Vg_DebugMsg, "removing client obj [%p,%p[\n", - p->any.a1, p->any.a2); + VG_(message)(Vg_DebugMsg, "removing client obj [0x%lx,0x%lx[ of type %d", + p->any.a1, p->any.a2, p->any.type); #endif tl_assert(VG_(OSetGen_Lookup)(s_clientobj, &addr) == 0); drd_finish_suppression(p->any.a1, p->any.a2); @@ -138,15 +143,37 @@ Bool drd_clientobj_remove(const Addr addr) void drd_clientobj_stop_using_mem(const Addr a1, const Addr a2) { + Addr removed_at; DrdClientobj* p; + +#if 0 + VG_(message)(Vg_DebugMsg, "drd_clientobj_stop_using_mem [0x%lx,0x%lx[", + a1, a2); +#endif tl_assert(s_clientobj); VG_(OSetGen_ResetIter)(s_clientobj); - for ( ; (p = VG_(OSetGen_Next)(s_clientobj)) != 0; ) + p = VG_(OSetGen_Next)(s_clientobj); + for ( ; p != 0; ) { if ((a1 <= p->any.a1 && p->any.a1 < a2) || (a1 < p->any.a2 && p->any.a2 <= a2)) { +#if 0 + VG_(message)(Vg_DebugMsg, "drd_clientobj_stop_using_mem [0x%lx,0x%lx[", + a1, a2); +#endif + removed_at = p->any.a1; drd_clientobj_remove(p->any.a1); + /* The above call removes an element from the oset and hence invalidates */ + /* the iterator. Set the iterator back. */ + VG_(OSetGen_ResetIter)(s_clientobj); + while ((p = VG_(OSetGen_Next)(s_clientobj)) != 0 + && p->any.a1 <= removed_at) + { } + } + else + { + p = VG_(OSetGen_Next)(s_clientobj); } } } diff --git a/exp-drd/drd_clientobj.h b/exp-drd/drd_clientobj.h index 972495a36c..f2b0ff9bdb 100644 --- a/exp-drd/drd_clientobj.h +++ b/exp-drd/drd_clientobj.h @@ -39,7 +39,7 @@ union drd_clientobj; // Type definitions. -typedef enum { ClientMutex, } ObjType; +typedef enum { ClientMutex = 1, } ObjType; struct any { diff --git a/exp-drd/drd_mutex.c b/exp-drd/drd_mutex.c index 16cd92ef36..2718dd5f6a 100644 --- a/exp-drd/drd_mutex.c +++ b/exp-drd/drd_mutex.c @@ -76,6 +76,17 @@ void mutex_initialize(struct mutex_info* const p, /** Deallocate the memory that was allocated by mutex_initialize(). */ static void mutex_cleanup(struct mutex_info* p) { + if (s_trace_mutex) + { + const ThreadId vg_tid = VG_(get_running_tid)(); + const DrdThreadId drd_tid = VgThreadIdToDrdThreadId(vg_tid); + VG_(message)(Vg_DebugMsg, + "drd_pre_mutex_destroy tid = %d/%d, %s 0x%lx", + vg_tid, drd_tid, + mutex_get_typename(p), + p->a1); + } + if (mutex_is_locked(p)) { MutexErrInfo MEI = { p->a1, p->recursion_count, p->owner }; @@ -149,7 +160,7 @@ mutex_init(const Addr mutex, const SizeT size, const MutexT mutex_type) VG_(get_IP)(vg_tid), "Mutex reinitialization", &MEI); - mutex_destroy(mutex_p); + return mutex_p; } mutex_p = mutex_get_or_allocate(mutex, size, mutex_type); @@ -158,17 +169,6 @@ mutex_init(const Addr mutex, const SizeT size, const MutexT mutex_type) static void mutex_destroy(struct mutex_info* const p) { - if (s_trace_mutex) - { - const ThreadId vg_tid = VG_(get_running_tid)(); - const DrdThreadId drd_tid = VgThreadIdToDrdThreadId(vg_tid); - VG_(message)(Vg_DebugMsg, - "drd_pre_mutex_destroy tid = %d/%d, %s 0x%lx", - vg_tid, drd_tid, - mutex_get_typename(p), - p->a1); - } - drd_clientobj_remove(p->a1); } @@ -306,18 +306,19 @@ int mutex_post_lock(const Addr mutex, const SizeT size, MutexT mutex_type) * Update mutex_info state when unlocking the pthread_mutex_t mutex. * Note: this function must be called before pthread_mutex_unlock() is called, * or a race condition is triggered ! + * @return New value of the mutex recursion count. * @param mutex Pointer to pthread_mutex_t data structure in the client space. * @param tid ThreadId of the thread calling pthread_mutex_unlock(). * @param vc Pointer to the current vector clock of thread tid. */ int mutex_unlock(const Addr mutex, const MutexT mutex_type) { - const DrdThreadId drd_tid = VgThreadIdToDrdThreadId(VG_(get_running_tid)()); - const ThreadId vg_tid = DrdThreadIdToVgThreadId(drd_tid); + const DrdThreadId drd_tid = thread_get_running_tid(); + const ThreadId vg_tid = VG_(get_running_tid)(); const VectorClock* const vc = thread_get_vc(drd_tid); struct mutex_info* const p = mutex_get(mutex); - if (s_trace_mutex) + if (s_trace_mutex && p != 0) { VG_(message)(Vg_DebugMsg, "drd_pre_mutex_unlock tid = %d/%d, %s 0x%lx rc %d", @@ -368,10 +369,6 @@ int mutex_unlock(const Addr mutex, const MutexT mutex_type) } tl_assert(p->mutex_type == mutex_type); tl_assert(p->owner != DRD_INVALID_THREADID); -#if 0 - tl_assert(mutex_type == mutex_type_mutex - || mutex_type == mutex_type_spinlock); -#endif if (p->owner != drd_tid) { diff --git a/exp-drd/drd_suppression.c b/exp-drd/drd_suppression.c index 513851ed0e..71a4c608e7 100644 --- a/exp-drd/drd_suppression.c +++ b/exp-drd/drd_suppression.c @@ -75,14 +75,12 @@ void drd_finish_suppression(const Addr a1, const Addr a2) } tl_assert(a1 < a2); -#if 0 if (! drd_is_suppressed(a1, a2)) { - VG_(message)(Vg_DebugMsg, "?? not suppressed ??"); + VG_(message)(Vg_DebugMsg, "?? [0x%lx,0x%lx[ not suppressed ??", a1, a2); VG_(get_and_pp_StackTrace)(VG_(get_running_tid)(), 12); tl_assert(False); } -#endif bm_clear(s_suppressed, a1, a2); } diff --git a/exp-drd/tests/tc09_bad_unlock.stderr.exp b/exp-drd/tests/tc09_bad_unlock.stderr.exp index b20cc188af..40b301ccc5 100644 --- a/exp-drd/tests/tc09_bad_unlock.stderr.exp +++ b/exp-drd/tests/tc09_bad_unlock.stderr.exp @@ -23,11 +23,6 @@ Not a mutex by 0x........: nearly_main (tc09_bad_unlock.c:41) by 0x........: main (tc09_bad_unlock.c:49) -Mutex reinitialization: address 0x........, recursion count 0, owner 1. - at 0x........: pthread_mutex_init (drd_intercepts.c:?) - by 0x........: nearly_main (tc09_bad_unlock.c:23) - by 0x........: main (tc09_bad_unlock.c:50) - Attempt to unlock a mutex that is not locked: address 0x........, recursion count -1, owner 1. at 0x........: pthread_mutex_unlock (drd_intercepts.c:?) by 0x........: nearly_main (tc09_bad_unlock.c:27) @@ -52,4 +47,4 @@ Not a mutex by 0x........: nearly_main (tc09_bad_unlock.c:41) by 0x........: main (tc09_bad_unlock.c:50) -ERROR SUMMARY: 9 errors from 9 contexts (suppressed: 0 from 0) +ERROR SUMMARY: 8 errors from 8 contexts (suppressed: 0 from 0)