|
From: <sv...@va...> - 2013-10-14 13:51:36
|
Author: sewardj
Date: Mon Oct 14 13:51:25 2013
New Revision: 13642
Log:
Fix #323432: When calling pthread_cond_destroy or pthread_mutex_destroy
with initializers as argument Helgrind (incorrectly) reports errors.
(Peter Boström, val...@pb...)
Modified:
trunk/helgrind/helgrind.h
trunk/helgrind/hg_intercepts.c
trunk/helgrind/hg_main.c
Modified: trunk/helgrind/helgrind.h
==============================================================================
--- trunk/helgrind/helgrind.h (original)
+++ trunk/helgrind/helgrind.h Mon Oct 14 13:51:25 2013
@@ -77,7 +77,7 @@
_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 @@
_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 */
Modified: trunk/helgrind/hg_intercepts.c
==============================================================================
--- trunk/helgrind/hg_intercepts.c (original)
+++ trunk/helgrind/hg_intercepts.c Mon Oct 14 13:51:25 2013
@@ -154,13 +154,27 @@
#include <errno.h>
#include <pthread.h>
+/* 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 @@
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 @@
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 @@
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);
Modified: trunk/helgrind/hg_main.c
==============================================================================
--- trunk/helgrind/hg_main.c (original)
+++ trunk/helgrind/hg_main.c Mon Oct 14 13:51:25 2013
@@ -1873,13 +1873,15 @@
}
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 @@
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 @@
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_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 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 @@
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 @@
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_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 @@
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 @@
(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],
|