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

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)

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
 

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 );
 }
 
 

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 \

helgrind/tests/pth_destroy_cond.c

@@ -0,0 +1,39 @@
+#include <stdio.h>
+#include <pthread.h>
+#include <errno.h>
+// 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; 
+}

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)
+

helgrind/tests/pth_destroy_cond.stdout.exp

@@ -0,0 +1 @@
+finished

helgrind/tests/pth_destroy_cond.vgtest

@@ -0,0 +1,3 @@
+prog: pth_destroy_cond
+vgopts: -q
+stderr_filter_args: pth_destroy_cond.c