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 8d5686694dbc8244c59c997782407329fcf06ff7..5e155f047eb130dff8951d96ed6ef970f4b379f2 100644 (file)
--- 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 ffe08bf1b05c3fc36010a1ba2bd11bb269ce6486..f18e1616bb05ed72fd52afcdb1ec83cb1959d04f 100644 (file)
--- a/helgrind/hg_intercepts.c
+++ b/helgrind/hg_intercepts.c
@@ -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 @@ 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 0accfe2e07da9dc737094eab8daa3992a71dc76d..3d241bf9fa3966446d926f97754e71b3f9ae2841 100644 (file)
--- 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],