Menu
▾
▴
[Valgrind-developers] valgrind: r7091 - branches/THRCHECK/thrcheck
|
From: <sv...@va...> - 2007-11-05 02:11:58
|
Author: sewardj
Date: 2007-11-05 02:11:57 +0000 (Mon, 05 Nov 2007)
New Revision: 7091
Log:
Add machinery to check for bogus mutex arguments to pthread_cond_wait,
since the documentation claims that functionality is provided :-)
Modified:
branches/THRCHECK/thrcheck/tc_intercepts.c
branches/THRCHECK/thrcheck/tc_main.c
Modified: branches/THRCHECK/thrcheck/tc_intercepts.c
===================================================================
--- branches/THRCHECK/thrcheck/tc_intercepts.c 2007-11-05 02:10:33 UTC (rev 7090)
+++ branches/THRCHECK/thrcheck/tc_intercepts.c 2007-11-05 02:11:57 UTC (rev 7091)
@@ -86,6 +86,19 @@
_arg1,_arg2,0,0,0); \
} while (0)
+#define DO_CREQ_W_WW(_resF, _creqF, _ty1F,_arg1F, _ty2F,_arg2F) \
+ do { \
+ Word _res, _arg1, _arg2; \
+ assert(sizeof(_ty1F) == sizeof(Word)); \
+ assert(sizeof(_ty2F) == sizeof(Word)); \
+ _arg1 = (Word)(_arg1F); \
+ _arg2 = (Word)(_arg2F); \
+ VALGRIND_DO_CLIENT_REQUEST(_res, 2, \
+ (_creqF), \
+ _arg1,_arg2,0,0,0); \
+ _resF = _res; \
+ } while (0)
+
#define DO_CREQ_v_WWW(_creqF, _ty1F,_arg1F, \
_ty2F,_arg2F, _ty3F, _arg3F) \
do { \
@@ -525,6 +538,8 @@
{
int ret;
OrigFn fn;
+ unsigned long mutex_is_valid;
+
VALGRIND_GET_ORIG_FN(fn);
if (TRACE_PTH_FNS) {
@@ -532,26 +547,39 @@
fflush(stderr);
}
+ /* Tell the tool a cond-wait is about to happen, so it can check
+ for bogus argument values. In return it tells us whether it
+ thinks the mutex is valid or not. */
+ DO_CREQ_W_WW(mutex_is_valid,
+ _VG_USERREQ__TC_PTHREAD_COND_WAIT_PRE,
+ pthread_cond_t*,cond, pthread_mutex_t*,mutex);
+ assert(mutex_is_valid == 1 || mutex_is_valid == 0);
+
/* Tell the tool we're about to drop the mutex. This reflects the
fact that in a cond_wait, we show up holding the mutex, and the
call atomically drops the mutex and waits for the cv to be
signalled. */
- DO_CREQ_v_W(_VG_USERREQ__TC_PTHREAD_MUTEX_UNLOCK_PRE,
- pthread_mutex_t*,mutex);
+ if (mutex_is_valid) {
+ DO_CREQ_v_W(_VG_USERREQ__TC_PTHREAD_MUTEX_UNLOCK_PRE,
+ pthread_mutex_t*,mutex);
+ }
CALL_FN_W_WW(ret, fn, cond,mutex);
- /* And now we have the mutex again, regardless of the error code
- returned. */
- // FIXME: but only if we actually had it before the call
- DO_CREQ_v_W(_VG_USERREQ__TC_PTHREAD_MUTEX_LOCK_POST,
- pthread_mutex_t*,mutex);
+ /* these conditionals look stupid, but compare w/ same logic for
+ pthread_cond_timedwait below */
+ if (ret == 0 && mutex_is_valid) {
+ /* and now we have the mutex again */
+ DO_CREQ_v_W(_VG_USERREQ__TC_PTHREAD_MUTEX_LOCK_POST,
+ pthread_mutex_t*,mutex);
+ }
- if (ret == 0) {
+ if (ret == 0 && mutex_is_valid) {
DO_CREQ_v_WW(_VG_USERREQ__TC_PTHREAD_COND_WAIT_POST,
pthread_cond_t*,cond, pthread_mutex_t*,mutex);
+ }
- } else {
+ if (ret != 0) {
DO_PthAPIerror( "pthread_cond_wait", ret );
}
@@ -570,6 +598,7 @@
{
int ret;
OrigFn fn;
+ unsigned long mutex_is_valid;
VALGRIND_GET_ORIG_FN(fn);
if (TRACE_PTH_FNS) {
@@ -578,29 +607,38 @@
fflush(stderr);
}
+ /* Tell the tool a cond-wait is about to happen, so it can check
+ for bogus argument values. In return it tells us whether it
+ thinks the mutex is valid or not. */
+ DO_CREQ_W_WW(mutex_is_valid,
+ _VG_USERREQ__TC_PTHREAD_COND_WAIT_PRE,
+ pthread_cond_t*,cond, pthread_mutex_t*,mutex);
+ assert(mutex_is_valid == 1 || mutex_is_valid == 0);
+
/* Tell the tool we're about to drop the mutex. This reflects the
fact that in a cond_wait, we show up holding the mutex, and the
call atomically drops the mutex and waits for the cv to be
signalled. */
- DO_CREQ_v_W(_VG_USERREQ__TC_PTHREAD_MUTEX_UNLOCK_PRE,
- pthread_mutex_t*,mutex);
+ if (mutex_is_valid) {
+ DO_CREQ_v_W(_VG_USERREQ__TC_PTHREAD_MUTEX_UNLOCK_PRE,
+ pthread_mutex_t*,mutex);
+ }
CALL_FN_W_WWW(ret, fn, cond,mutex,abstime);
- /* And now we have the mutex again, regardless of the error code
- returned. In particular we still have it even if
- ret==ETIMEDOUT. */
- // FIXME: but only if we actually had it before the call
- DO_CREQ_v_W(_VG_USERREQ__TC_PTHREAD_MUTEX_LOCK_POST,
- pthread_mutex_t*,mutex);
+ if ((ret == 0 || ret == ETIMEDOUT) && mutex_is_valid) {
+ /* and now we have the mutex again */
+ DO_CREQ_v_W(_VG_USERREQ__TC_PTHREAD_MUTEX_LOCK_POST,
+ pthread_mutex_t*,mutex);
+ }
- if (ret == 0) {
+ if (ret == 0 && mutex_is_valid) {
DO_CREQ_v_WW(_VG_USERREQ__TC_PTHREAD_COND_WAIT_POST,
pthread_cond_t*,cond, pthread_mutex_t*,mutex);
+ }
- } else {
- if (ret != ETIMEDOUT)
- DO_PthAPIerror( "pthread_cond_timedwait", ret );
+ if (ret != 0 && ret != ETIMEDOUT) {
+ DO_PthAPIerror( "pthread_cond_timedwait", ret );
}
if (TRACE_PTH_FNS) {
Modified: branches/THRCHECK/thrcheck/tc_main.c
===================================================================
--- branches/THRCHECK/thrcheck/tc_main.c 2007-11-05 02:10:33 UTC (rev 7090)
+++ branches/THRCHECK/thrcheck/tc_main.c 2007-11-05 02:11:57 UTC (rev 7091)
@@ -5918,7 +5918,7 @@
&& TC_(elemBag)( lk->heldBy, (Word)thr ) > 0 ) {
/* uh, it's a non-recursive lock and we already w-hold it, and
this is a real lock operation (not a speculative "tryLock"
- kind of thing. Duh. Deadlock coming up; but at least
+ kind of thing). Duh. Deadlock coming up; but at least
produce an error message. */
record_error_Misc( thr, "Attempt to re-lock a "
"non-recursive lock I already hold" );
@@ -6031,10 +6031,15 @@
}
}
-static void evh__TC_PTHREAD_COND_WAIT_PRE ( ThreadId tid,
+/* returns True if it reckons 'mutex' is valid and held by this
+ thread, else False */
+static Bool evh__TC_PTHREAD_COND_WAIT_PRE ( ThreadId tid,
void* cond, void* mutex )
{
Thread* thr;
+ Lock* lk;
+ Bool lk_valid = True;
+
if (SHOW_EVENTS >= 1)
VG_(printf)("evh__tc_PTHREAD_COND_WAIT_PRE"
"(ctid=%d, cond=%p, mutex=%p)\n",
@@ -6044,7 +6049,41 @@
thr = map_threads_maybe_lookup( tid );
tl_assert(thr); /* cannot fail - Thread* must already exist */
+ lk = map_locks_maybe_lookup( (Addr)mutex );
+
+ /* Check for stupid mutex arguments. There are various ways to be
+ a bozo. Only complain once, though, even if more than one thing
+ is wrong. */
+ if (lk == NULL) {
+ lk_valid = False;
+ record_error_Misc(
+ thr,
+ "pthread_cond_wait called with invalid mutex" );
+ } else {
+ tl_assert( is_sane_LockN(lk) );
+ if (lk->kind == LK_rdwr) {
+ lk_valid = False;
+ record_error_Misc(
+ thr, "pthread_cond_wait called with mutex "
+ "of type pthread_rwlock_t*" );
+ } else
+ if (lk->heldBy == NULL) {
+ lk_valid = False;
+ record_error_Misc(
+ thr, "pthread_cond_wait called with un-held mutex");
+ } else
+ if (lk->heldBy != NULL
+ && TC_(elemBag)( lk->heldBy, (Word)thr ) == 0) {
+ lk_valid = False;
+ record_error_Misc(
+ thr, "pthread_cond_wait called with mutex "
+ "held by a different thread" );
+ }
+ }
+
// error-if: cond is also associated with a different mutex
+
+ return lk_valid;
}
static void evh__TC_PTHREAD_COND_WAIT_POST ( ThreadId tid,
@@ -7477,17 +7516,22 @@
evh__TC_PTHREAD_COND_SIGNAL_PRE( tid, (void*)args[1] );
break;
- /* Entry into pthread_cond_wait, cond=arg[1], mutex=arg[2] */
- case _VG_USERREQ__TC_PTHREAD_COND_WAIT_PRE:
- evh__TC_PTHREAD_COND_WAIT_PRE( tid,
- (void*)args[1], (void*)args[2] );
+ /* Entry into pthread_cond_wait, cond=arg[1], mutex=arg[2].
+ Returns a flag indicating whether or not the mutex is believed to be
+ valid for this operation. */
+ case _VG_USERREQ__TC_PTHREAD_COND_WAIT_PRE: {
+ Bool mutex_is_valid
+ = evh__TC_PTHREAD_COND_WAIT_PRE( tid, (void*)args[1],
+ (void*)args[2] );
+ *ret = mutex_is_valid ? 1 : 0;
break;
+ }
/* Thread successfully completed pthread_cond_wait, cond=arg[1],
mutex=arg[2] */
case _VG_USERREQ__TC_PTHREAD_COND_WAIT_POST:
evh__TC_PTHREAD_COND_WAIT_POST( tid,
- (void*)args[1], (void*)args[2] );
+ (void*)args[1], (void*)args[2] );
break;
case _VG_USERREQ__TC_PTHREAD_RWLOCK_INIT_POST:
@@ -8371,7 +8415,7 @@
clo_happens_before = 0;
else if (VG_CLO_STREQ(arg, "--happens-before=threads"))
clo_happens_before = 1;
- else if (VG_CLO_STREQ(arg, "--happens-before=condvars"))
+ else if (VG_CLO_STREQ(arg, "--happens-before=all"))
clo_happens_before = 2;
else if (VG_CLO_STREQ(arg, "--gen-vcg=no"))
@@ -8424,8 +8468,8 @@
static void tc_print_usage ( void )
{
VG_(printf)(
-" --happens-before=none|threads|condvars [condvars] consider no events,\n"
-" thread create/join, thread create/join/cvsignal/cvwait as sync points\n"
+" --happens-before=none|threads|all [all] consider no events, thread\n"
+" create/join, create/join/cvsignal/cvwait/semwait/post as sync points\n"
" --trace-addr=0xXXYYZZ show all state changes for address 0xXXYYZZ\n"
" --trace-level=0|1|2 verbosity level of --trace-addr [1]\n"
);
|
