From: Bart Van Assche Date: Sat, 28 Jun 2008 16:01:43 +0000 (+0000) Subject: An error message is now printed if two different threads call X-Git-Tag: svn/VALGRIND_3_4_0~416 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8db83fb46a6797ff27ad69f72260acd37f1f4dbe;p=thirdparty%2Fvalgrind.git 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 46b994e673..7cdd581ad8 100644 --- 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 842a6379dc..e66fdac576 100644 --- 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 fbff475d59..6b56051c37 100644 --- 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 5c7af6df92..a97d7aa029 100644 --- 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 index 0000000000..0f8895abfb --- /dev/null +++ b/exp-drd/tests/pth_inconsistent_cond_wait.c @@ -0,0 +1,45 @@ +#include +#include +#include + +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 index 0000000000..5300ec01d2 --- /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 index 0000000000..556b2f27c2 --- /dev/null +++ b/exp-drd/tests/pth_inconsistent_cond_wait.vgtest @@ -0,0 +1,2 @@ +prereq: ./supported_libpthread +prog: pth_inconsistent_cond_wait