From: Julian Seward Date: Mon, 14 Oct 2013 13:51:25 +0000 (+0000) Subject: Fix #323432: When calling pthread_cond_destroy or pthread_mutex_destroy X-Git-Tag: svn/VALGRIND_3_9_0~49 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e1b6cccf9116cb1a73467d9750bf4f0ca4ae86ad;p=thirdparty%2Fvalgrind.git Fix #323432: When calling pthread_cond_destroy or pthread_mutex_destroy with initializers as argument Helgrind (incorrectly) reports errors. (Peter Boström, valgrind@pbos.me) git-svn-id: svn://svn.valgrind.org/valgrind/trunk@13642 --- diff --git a/helgrind/helgrind.h b/helgrind/helgrind.h index 8d5686694d..5e155f047e 100644 --- a/helgrind/helgrind.h +++ b/helgrind/helgrind.h @@ -77,7 +77,7 @@ typedef _VG_USERREQ__HG_PTH_API_ERROR, /* char*, int */ _VG_USERREQ__HG_PTHREAD_JOIN_POST, /* pthread_t of quitter */ _VG_USERREQ__HG_PTHREAD_MUTEX_INIT_POST, /* pth_mx_t*, long mbRec */ - _VG_USERREQ__HG_PTHREAD_MUTEX_DESTROY_PRE, /* pth_mx_t* */ + _VG_USERREQ__HG_PTHREAD_MUTEX_DESTROY_PRE, /* pth_mx_t*, long isInit */ _VG_USERREQ__HG_PTHREAD_MUTEX_UNLOCK_PRE, /* pth_mx_t* */ _VG_USERREQ__HG_PTHREAD_MUTEX_UNLOCK_POST, /* pth_mx_t* */ _VG_USERREQ__HG_PTHREAD_MUTEX_LOCK_PRE, /* pth_mx_t*, long isTryLock */ @@ -86,7 +86,7 @@ typedef _VG_USERREQ__HG_PTHREAD_COND_BROADCAST_PRE, /* pth_cond_t* */ _VG_USERREQ__HG_PTHREAD_COND_WAIT_PRE, /* pth_cond_t*, pth_mx_t* */ _VG_USERREQ__HG_PTHREAD_COND_WAIT_POST, /* pth_cond_t*, pth_mx_t* */ - _VG_USERREQ__HG_PTHREAD_COND_DESTROY_PRE, /* pth_cond_t* */ + _VG_USERREQ__HG_PTHREAD_COND_DESTROY_PRE, /* pth_cond_t*, long isInit */ _VG_USERREQ__HG_PTHREAD_RWLOCK_INIT_POST, /* pth_rwlk_t* */ _VG_USERREQ__HG_PTHREAD_RWLOCK_DESTROY_PRE, /* pth_rwlk_t* */ _VG_USERREQ__HG_PTHREAD_RWLOCK_LOCK_PRE, /* pth_rwlk_t*, long isW */ diff --git a/helgrind/hg_intercepts.c b/helgrind/hg_intercepts.c index ffe08bf1b0..f18e1616bb 100644 --- a/helgrind/hg_intercepts.c +++ b/helgrind/hg_intercepts.c @@ -154,13 +154,27 @@ #include #include +/* A standalone memcmp. */ +__attribute__((noinline)) +static int my_memcmp ( const void* ptr1, const void* ptr2, size_t size) +{ + unsigned char* uchar_ptr1 = (unsigned char*) ptr1; + unsigned char* uchar_ptr2 = (unsigned char*) ptr2; + size_t i; + for (i = 0; i < size; ++i) { + if (uchar_ptr1[i] != uchar_ptr2[i]) + return (uchar_ptr1[i] < uchar_ptr2[i]) ? -1 : 1; + } + return 0; +} /* A lame version of strerror which doesn't use the real libc strerror_r, since using the latter just generates endless more threading errors (glibc goes off and does tons of crap w.r.t. locales etc) */ static const HChar* lame_strerror ( long err ) -{ switch (err) { +{ + switch (err) { case EPERM: return "EPERM: Operation not permitted"; case ENOENT: return "ENOENT: No such file or directory"; case ESRCH: return "ESRCH: No such process"; @@ -446,14 +460,23 @@ PTH_FUNC(int, pthreadZumutexZudestroy, // pthread_mutex_destroy pthread_mutex_t *mutex) { int ret; + unsigned long mutex_is_init; OrigFn fn; + VALGRIND_GET_ORIG_FN(fn); if (TRACE_PTH_FNS) { fprintf(stderr, "<< pthread_mxdestroy %p", mutex); fflush(stderr); } - DO_CREQ_v_W(_VG_USERREQ__HG_PTHREAD_MUTEX_DESTROY_PRE, - pthread_mutex_t*,mutex); + if (mutex != NULL) { + static const pthread_mutex_t mutex_init = PTHREAD_MUTEX_INITIALIZER; + mutex_is_init = my_memcmp(mutex, &mutex_init, sizeof(*mutex)) == 0; + } else { + mutex_is_init = 0; + } + + DO_CREQ_v_WW(_VG_USERREQ__HG_PTHREAD_MUTEX_DESTROY_PRE, + pthread_mutex_t*, mutex, unsigned long, mutex_is_init); CALL_FN_W_W(ret, fn, mutex); @@ -976,6 +999,7 @@ __attribute__((noinline)) static int pthread_cond_destroy_WRK(pthread_cond_t* cond) { int ret; + unsigned long cond_is_init; OrigFn fn; VALGRIND_GET_ORIG_FN(fn); @@ -985,8 +1009,15 @@ static int pthread_cond_destroy_WRK(pthread_cond_t* cond) fflush(stderr); } - DO_CREQ_v_W(_VG_USERREQ__HG_PTHREAD_COND_DESTROY_PRE, - pthread_cond_t*,cond); + if (cond != NULL) { + const pthread_cond_t cond_init = PTHREAD_COND_INITIALIZER; + cond_is_init = my_memcmp(cond, &cond_init, sizeof(*cond)) == 0; + } else { + cond_is_init = 0; + } + + DO_CREQ_v_WW(_VG_USERREQ__HG_PTHREAD_COND_DESTROY_PRE, + pthread_cond_t*, cond, unsigned long, cond_is_init); CALL_FN_W_W(ret, fn, cond); diff --git a/helgrind/hg_main.c b/helgrind/hg_main.c index 0accfe2e07..3d241bf9fa 100644 --- a/helgrind/hg_main.c +++ b/helgrind/hg_main.c @@ -1873,13 +1873,15 @@ void evh__HG_PTHREAD_MUTEX_INIT_POST( ThreadId tid, } static -void evh__HG_PTHREAD_MUTEX_DESTROY_PRE( ThreadId tid, void* mutex ) +void evh__HG_PTHREAD_MUTEX_DESTROY_PRE( ThreadId tid, void* mutex, + Bool mutex_is_init ) { Thread* thr; Lock* lk; if (SHOW_EVENTS >= 1) - VG_(printf)("evh__hg_PTHREAD_MUTEX_DESTROY_PRE(ctid=%d, %p)\n", - (Int)tid, (void*)mutex ); + VG_(printf)("evh__hg_PTHREAD_MUTEX_DESTROY_PRE" + "(ctid=%d, %p, isInit=%d)\n", + (Int)tid, (void*)mutex, (Int)mutex_is_init ); thr = map_threads_maybe_lookup( tid ); /* cannot fail - Thread* must already exist */ @@ -1887,6 +1889,14 @@ void evh__HG_PTHREAD_MUTEX_DESTROY_PRE( ThreadId tid, void* mutex ) lk = map_locks_maybe_lookup( (Addr)mutex ); + if (lk == NULL && mutex_is_init) { + /* We're destroying a mutex which we don't have any record of, + and which appears to have the value PTHREAD_MUTEX_INITIALIZER. + Assume it never got used, and so we don't need to do anything + more. */ + goto out; + } + if (lk == NULL || (lk->kind != LK_nonRec && lk->kind != LK_mbRec)) { HG_(record_error_Misc)( thr, "pthread_mutex_destroy with invalid argument" ); @@ -1915,6 +1925,7 @@ void evh__HG_PTHREAD_MUTEX_DESTROY_PRE( ThreadId tid, void* mutex ) del_LockN( lk ); } + out: if (HG_(clo_sanity_flags) & SCE_LOCKS) all__sanity_check("evh__hg_PTHREAD_MUTEX_DESTROY_PRE"); } @@ -2077,7 +2088,7 @@ static void evh__HG_PTHREAD_SPIN_LOCK_POST( ThreadId tid, static void evh__HG_PTHREAD_SPIN_DESTROY_PRE( ThreadId tid, void* slock ) { - evh__HG_PTHREAD_MUTEX_DESTROY_PRE( tid, slock ); + evh__HG_PTHREAD_MUTEX_DESTROY_PRE( tid, slock, 0/*!isInit*/ ); } @@ -2148,7 +2159,8 @@ static CVInfo* map_cond_to_CVInfo_lookup_NO_alloc ( void* cond ) { } } -static void map_cond_to_CVInfo_delete ( ThreadId tid, void* cond ) { +static void map_cond_to_CVInfo_delete ( ThreadId tid, + void* cond, Bool cond_is_init ) { Thread* thr; UWord keyW, valW; @@ -2162,9 +2174,9 @@ static void map_cond_to_CVInfo_delete ( ThreadId tid, void* cond ) { tl_assert(cvi); tl_assert(cvi->so); if (cvi->nWaiters > 0) { - HG_(record_error_Misc)(thr, - "pthread_cond_destroy:" - " destruction of condition variable being waited upon"); + HG_(record_error_Misc)( + thr, "pthread_cond_destroy:" + " destruction of condition variable being waited upon"); /* Destroying a cond var being waited upon outcome is EBUSY and variable is not destroyed. */ return; @@ -2175,8 +2187,14 @@ static void map_cond_to_CVInfo_delete ( ThreadId tid, void* cond ) { cvi->mx_ga = 0; HG_(free)(cvi); } else { - HG_(record_error_Misc)(thr, - "pthread_cond_destroy: destruction of unknown cond var"); + /* We have no record of this CV. So complain about it + .. except, don't bother to complain if it has exactly the + value PTHREAD_COND_INITIALIZER, since it might be that the CV + was initialised like that but never used. */ + if (!cond_is_init) { + HG_(record_error_Misc)( + thr, "pthread_cond_destroy: destruction of unknown cond var"); + } } } @@ -2395,17 +2413,17 @@ static void evh__HG_PTHREAD_COND_INIT_POST ( ThreadId tid, static void evh__HG_PTHREAD_COND_DESTROY_PRE ( ThreadId tid, - void* cond ) + void* cond, Bool cond_is_init ) { /* Deal with destroy events. The only purpose is to free storage associated with the CV, so as to avoid any possible resource leaks. */ if (SHOW_EVENTS >= 1) VG_(printf)("evh__HG_PTHREAD_COND_DESTROY_PRE" - "(ctid=%d, cond=%p)\n", - (Int)tid, (void*)cond ); + "(ctid=%d, cond=%p, cond_is_init=%d)\n", + (Int)tid, (void*)cond, (Int)cond_is_init ); - map_cond_to_CVInfo_delete( tid, cond ); + map_cond_to_CVInfo_delete( tid, cond, cond_is_init ); } @@ -4821,8 +4839,9 @@ Bool hg_handle_client_request ( ThreadId tid, UWord* args, UWord* ret) evh__HG_PTHREAD_MUTEX_INIT_POST( tid, (void*)args[1], args[2] ); break; + /* mutex=arg[1], mutex_is_init=arg[2] */ case _VG_USERREQ__HG_PTHREAD_MUTEX_DESTROY_PRE: - evh__HG_PTHREAD_MUTEX_DESTROY_PRE( tid, (void*)args[1] ); + evh__HG_PTHREAD_MUTEX_DESTROY_PRE( tid, (void*)args[1], args[2] != 0 ); break; case _VG_USERREQ__HG_PTHREAD_MUTEX_UNLOCK_PRE: // pth_mx_t* @@ -4866,9 +4885,9 @@ Bool hg_handle_client_request ( ThreadId tid, UWord* args, UWord* ret) (void*)args[1], (void*)args[2] ); break; - /* cond=arg[1] */ + /* cond=arg[1], cond_is_init=arg[2] */ case _VG_USERREQ__HG_PTHREAD_COND_DESTROY_PRE: - evh__HG_PTHREAD_COND_DESTROY_PRE( tid, (void*)args[1] ); + evh__HG_PTHREAD_COND_DESTROY_PRE( tid, (void*)args[1], args[2] != 0 ); break; /* Thread successfully completed pthread_cond_wait, cond=arg[1],