From: Philippe Waroquiers Date: Fri, 6 Jul 2012 23:38:24 +0000 (+0000) Subject: 295590 Helgrind: Assertion 'cvi->nWaiters > 0' failed when cond var being waited... X-Git-Tag: svn/VALGRIND_3_8_0~147 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=74bc30c150e40f2e0322e8d06f618d6df3bdf92e;p=thirdparty%2Fvalgrind.git 295590 Helgrind: Assertion 'cvi->nWaiters > 0' failed when cond var being waited upon destroyed * when cond var is destroyed, in the PRE, report an error if nwaiters > 0. * when cond_wait succeeds, get the cond var but do not create one in helgrind (it must exist if cond_wait was done). Report an error if cond not found (assuming this is caused by a destroy done while the thread was cond_wait-ing). * added a test git-svn-id: svn://svn.valgrind.org/valgrind/trunk@12721 --- diff --git a/NEWS b/NEWS index df7ec8b847..dd9c6e0e15 100644 --- a/NEWS +++ b/NEWS @@ -171,6 +171,7 @@ where XXXXXX is the bug number as listed below. 295089 can not annotate source for both helgrind and drd 295221 POWER Processor decimal floating point instruction support missing 295428 coregrind/m_main.c has incorrect x86 assembly for darwin +295590 Helgrind: Assertion 'cvi->nWaiters > 0' failed when cond var being waited upon destroyed 295799 Missing \n with get_vbits in gdbserver when line is % 80 and there are some unaddressable bytes 296422 Add translation chaining support 296457 vex amd64->IR: 0x66 0xF 0x3A 0xDF 0xD1 0x1 0xE8 0x6A (dup of AES) diff --git a/docs/internals/3_7_BUGSTATUS.txt b/docs/internals/3_7_BUGSTATUS.txt index a501d434bc..80a120acb8 100644 --- a/docs/internals/3_7_BUGSTATUS.txt +++ b/docs/internals/3_7_BUGSTATUS.txt @@ -175,10 +175,6 @@ get fixed. not high prio **possible 3.8.0 (easy to fix?) -295590 Helgrind: hg_main.c:2298 (evh__HG_PTHREAD_COND_WAIT_POST): - Assertion 'cvi->nWaiters > 0' failed. - **possible 3.8.0 - 295617 ARM - Add some missing syscalls **possible 3.8.0, needs landing diff --git a/helgrind/hg_main.c b/helgrind/hg_main.c index 9bc36414b0..b46fc0e492 100644 --- a/helgrind/hg_main.c +++ b/helgrind/hg_main.c @@ -2139,17 +2139,41 @@ static CVInfo* map_cond_to_CVInfo_lookup_or_alloc ( void* cond ) { } } -static void map_cond_to_CVInfo_delete ( void* cond ) { +static CVInfo* map_cond_to_CVInfo_lookup_NO_alloc ( void* cond ) { + UWord key, val; + map_cond_to_CVInfo_INIT(); + if (VG_(lookupFM)( map_cond_to_CVInfo, &key, &val, (UWord)cond )) { + tl_assert(key == (UWord)cond); + return (CVInfo*)val; + } else { + return NULL; + } +} + +static void map_cond_to_CVInfo_delete ( ThreadId tid, void* cond ) { + Thread* thr; UWord keyW, valW; + + thr = map_threads_maybe_lookup( tid ); + tl_assert(thr); /* cannot fail - Thread* must already exist */ + map_cond_to_CVInfo_INIT(); if (VG_(delFromFM)( map_cond_to_CVInfo, &keyW, &valW, (UWord)cond )) { CVInfo* cvi = (CVInfo*)valW; tl_assert(keyW == (UWord)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"); + } libhb_so_dealloc(cvi->so); cvi->mx_ga = 0; HG_(free)(cvi); + } else { + HG_(record_error_Misc)(thr, + "pthread_cond_destroy: destruction of unknown cond var"); } } @@ -2320,7 +2344,17 @@ static void evh__HG_PTHREAD_COND_WAIT_POST ( ThreadId tid, // error-if: cond is also associated with a different mutex - cvi = map_cond_to_CVInfo_lookup_or_alloc( cond ); + cvi = map_cond_to_CVInfo_lookup_NO_alloc( cond ); + if (!cvi) { + /* This could be either a bug in helgrind or the guest application + that did an error (e.g. cond var was destroyed by another thread. + Let's assume helgrind is perfect ... + Note that this is similar to drd behaviour. */ + HG_(record_error_Misc)(thr, "condition variable has been destroyed while" + " being waited upon"); + return; + } + tl_assert(cvi); tl_assert(cvi->so); tl_assert(cvi->nWaiters > 0); @@ -2351,7 +2385,7 @@ static void evh__HG_PTHREAD_COND_DESTROY_PRE ( ThreadId tid, "(ctid=%d, cond=%p)\n", (Int)tid, (void*)cond ); - map_cond_to_CVInfo_delete( cond ); + map_cond_to_CVInfo_delete( tid, cond ); } diff --git a/helgrind/tests/Makefile.am b/helgrind/tests/Makefile.am index 3ca723b72b..77c17f55ce 100644 --- a/helgrind/tests/Makefile.am +++ b/helgrind/tests/Makefile.am @@ -40,6 +40,8 @@ EXTRA_DIST = \ pth_barrier1.vgtest pth_barrier1.stdout.exp pth_barrier1.stderr.exp \ pth_barrier2.vgtest pth_barrier2.stdout.exp pth_barrier2.stderr.exp \ pth_barrier3.vgtest pth_barrier3.stdout.exp pth_barrier3.stderr.exp \ + pth_destroy_cond.vgtest \ + pth_destroy_cond.stdout.exp pth_destroy_cond.stderr.exp \ pth_spinlock.vgtest pth_spinlock.stdout.exp pth_spinlock.stderr.exp \ rwlock_race.vgtest rwlock_race.stdout.exp rwlock_race.stderr.exp \ rwlock_test.vgtest rwlock_test.stdout.exp rwlock_test.stderr.exp \ @@ -106,6 +108,7 @@ check_PROGRAMS = \ locked_vs_unlocked1 \ locked_vs_unlocked2 \ locked_vs_unlocked3 \ + pth_destroy_cond \ t2t \ tc01_simple_race \ tc02_simple_tls \ diff --git a/helgrind/tests/pth_destroy_cond.c b/helgrind/tests/pth_destroy_cond.c new file mode 100644 index 0000000000..54f4f433c7 --- /dev/null +++ b/helgrind/tests/pth_destroy_cond.c @@ -0,0 +1,39 @@ +#include +#include +#include +// test program from johan.walles (bug 295590) +// This test verifies that helgrind detects (and does not crash) when +// the guest application wrongly destroys a cond var being waited +// upon. +pthread_mutex_t mutex; +pthread_cond_t cond; +pthread_t thread; +int ready = 0; + +void *ThreadFunction(void *ptr) +{ + pthread_mutex_lock(&mutex); + ready = 1; + pthread_cond_signal(&cond); + pthread_cond_destroy(&cond); // ERROR!!! + pthread_mutex_unlock(&mutex); + return NULL; +} + +int main() +{ + pthread_mutex_init(&mutex, NULL); + pthread_cond_init(&cond, NULL); + + pthread_mutex_lock(&mutex); + pthread_create(&thread, NULL, ThreadFunction, (void*) NULL); + while (!ready) { // to insure ourselves against spurious wakeups + pthread_cond_wait(&cond, &mutex); + } + pthread_mutex_unlock(&mutex); + + pthread_join(thread, NULL); + pthread_mutex_destroy(&mutex); + printf("finished\n"); + return 0; +} diff --git a/helgrind/tests/pth_destroy_cond.stderr.exp b/helgrind/tests/pth_destroy_cond.stderr.exp new file mode 100644 index 0000000000..4f9eeb77c5 --- /dev/null +++ b/helgrind/tests/pth_destroy_cond.stderr.exp @@ -0,0 +1,28 @@ +---Thread-Announcement------------------------------------------ + +Thread #x was created + ... + by 0x........: pthread_create_WRK (hg_intercepts.c:...) + by 0x........: pthread_create@* (hg_intercepts.c:...) + by 0x........: main (pth_destroy_cond.c:29) + +---------------------------------------------------------------- + +Thread #x: pthread_cond_destroy: destruction of condition variable being waited upon + at 0x........: pthread_cond_destroy_WRK (hg_intercepts.c:...) + by 0x........: pthread_cond_destroy@* (hg_intercepts.c:...) + by 0x........: ThreadFunction (pth_destroy_cond.c:18) + by 0x........: mythread_wrapper (hg_intercepts.c:...) + ... + +---Thread-Announcement------------------------------------------ + +Thread #x is the program's root thread + +---------------------------------------------------------------- + +Thread #x: condition variable has been destroyed while being waited upon + at 0x........: pthread_cond_wait_WRK (hg_intercepts.c:...) + by 0x........: pthread_cond_wait@* (hg_intercepts.c:...) + by 0x........: main (pth_destroy_cond.c:31) + diff --git a/helgrind/tests/pth_destroy_cond.stdout.exp b/helgrind/tests/pth_destroy_cond.stdout.exp new file mode 100644 index 0000000000..f6563985ac --- /dev/null +++ b/helgrind/tests/pth_destroy_cond.stdout.exp @@ -0,0 +1 @@ +finished diff --git a/helgrind/tests/pth_destroy_cond.vgtest b/helgrind/tests/pth_destroy_cond.vgtest new file mode 100644 index 0000000000..3f1fdf46f6 --- /dev/null +++ b/helgrind/tests/pth_destroy_cond.vgtest @@ -0,0 +1,3 @@ +prog: pth_destroy_cond +vgopts: -q +stderr_filter_args: pth_destroy_cond.c