Improve Valgrind instrumentation of memory allocations

In smalloc, let Valgrind do detection of red zone modifications. In
synchr.c, let Valgrind perform use-after-free detection of
synchronization objects.

Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>

diff --git a/configure b/configure
index f38e9c75684b84b11d35db8a6203ee55553150f1..ddf03a6b8e1b12faddb1f4a9310d30d45214dac7 100755 (executable)
--- a/configure
+++ b/configure
@@ -2050,6 +2050,24 @@ fi
 print_config "strndup" "$strndup"
 
 ##########################################
+# <valgrind/drd.h> probe
+# Note: presence of <valgrind/drd.h> implies that <valgrind/valgrind.h> is
+# also available but not the other way around.
+if test "$valgrind_dev" != "yes" ; then
+  valgrind_dev="no"
+fi
+cat > $TMPC << EOF
+#include <valgrind/drd.h>
+int main(int argc, char **argv)
+{
+  return 0;
+}
+EOF
+if compile_prog "" "" "valgrind_dev"; then
+  valgrind_dev="yes"
+fi
+print_config "Valgrind headers" "$valgrind_dev"
+
 # check march=armv8-a+crc+crypto
 if test "$march_armv8_a_crc_crypto" != "yes" ; then
   march_armv8_a_crc_crypto="no"
@@ -2354,6 +2372,9 @@ fi
 if test "$disable_opt" = "yes" ; then
   output_sym "CONFIG_DISABLE_OPTIMIZATIONS"
 fi
+if test "$valgrind_dev" = "yes"; then
+  output_sym "CONFIG_VALGRIND_DEV"
+fi
 if test "$zlib" = "no" ; then
   echo "Consider installing zlib-dev (zlib-devel, some fio features depend on it."
   if test "$build_static" = "yes"; then

diff --git a/fio_sem.c b/fio_sem.c
index b9a1bf1e988d550f8ad439983bc7c014b22cc32a..20fcfccc02eea36c3d6d32cd442d47f34b1bbe95 100644 (file)
--- a/fio_sem.c
+++ b/fio_sem.c
@@ -1,6 +1,11 @@
 #include <string.h>
 #include <sys/mman.h>
 #include <assert.h>
+#ifdef CONFIG_VALGRIND_DEV
+#include <valgrind/valgrind.h>
+#else
+#define RUNNING_ON_VALGRIND 0
+#endif
 
 #include "log.h"
 #include "fio_sem.h"
@@ -12,13 +17,17 @@
 void __fio_sem_remove(struct fio_sem *sem)
 {
        assert(sem->magic == FIO_SEM_MAGIC);
+       pthread_mutex_destroy(&sem->lock);
        pthread_cond_destroy(&sem->cond);
 
        /*
-        * Ensure any subsequent attempt to grab this semaphore will fail
-        * with an assert, instead of just silently hanging.
-        */
-       memset(sem, 0, sizeof(*sem));
+        * When not running on Valgrind, ensure any subsequent attempt to grab
+        * this semaphore will fail with an assert, instead of just silently
+        * hanging. When running on Valgrind, let Valgrind detect
+        * use-after-free.
+         */
+       if (!RUNNING_ON_VALGRIND)
+               memset(sem, 0, sizeof(*sem));
 }
 
 void fio_sem_remove(struct fio_sem *sem)
@@ -32,6 +41,8 @@ int __fio_sem_init(struct fio_sem *sem, int value)
        int ret;
 
        sem->value = value;
+       /* Initialize .waiters explicitly for Valgrind. */
+       sem->waiters = 0;
        sem->magic = FIO_SEM_MAGIC;
 
        ret = mutex_cond_init_pshared(&sem->lock, &sem->cond);

diff --git a/rwlock.c b/rwlock.c
index 7d9ad731e2ca0de41fd9f3281e5ab2916f6d7fd1..00e3809890829592e41acc5c7027a2a381c5d532 100644 (file)
--- a/rwlock.c
+++ b/rwlock.c
@@ -28,6 +28,7 @@ void fio_rwlock_unlock(struct fio_rwlock *lock)
 void fio_rwlock_remove(struct fio_rwlock *lock)
 {
        assert(lock->magic == FIO_RWLOCK_MAGIC);
+       pthread_rwlock_destroy(&lock->lock);
        munmap((void *) lock, sizeof(*lock));
 }
 

diff --git a/smalloc.c b/smalloc.c
index d19e108a35e0919aada104e4965471e32d8e7384..13995acce07f56c3b396427bb058676889148988 100644 (file)
--- a/smalloc.c
+++ b/smalloc.c
@@ -12,6 +12,13 @@
 #include <sys/types.h>
 #include <limits.h>
 #include <fcntl.h>
+#ifdef CONFIG_VALGRIND_DEV
+#include <valgrind/valgrind.h>
+#else
+#define RUNNING_ON_VALGRIND 0
+#define VALGRIND_MALLOCLIKE_BLOCK(addr, size, rzB, is_zeroed) do { } while (0)
+#define VALGRIND_FREELIKE_BLOCK(addr, rzB) do { } while (0)
+#endif
 
 #include "fio.h"
 #include "fio_sem.h"
@@ -49,6 +56,12 @@ struct pool {
        size_t mmap_size;
 };
 
+#ifdef SMALLOC_REDZONE
+#define REDZONE_SIZE sizeof(unsigned int)
+#else
+#define REDZONE_SIZE 0
+#endif
+
 struct block_hdr {
        size_t size;
 #ifdef SMALLOC_REDZONE
@@ -258,6 +271,10 @@ static void fill_redzone(struct block_hdr *hdr)
 {
        unsigned int *postred = postred_ptr(hdr);
 
+       /* Let Valgrind fill the red zones. */
+       if (RUNNING_ON_VALGRIND)
+               return;
+
        hdr->prered = SMALLOC_PRE_RED;
        *postred = SMALLOC_POST_RED;
 }
@@ -266,6 +283,10 @@ static void sfree_check_redzone(struct block_hdr *hdr)
 {
        unsigned int *postred = postred_ptr(hdr);
 
+       /* Let Valgrind check the red zones. */
+       if (RUNNING_ON_VALGRIND)
+               return;
+
        if (hdr->prered != SMALLOC_PRE_RED) {
                log_err("smalloc pre redzone destroyed!\n"
                        " ptr=%p, prered=%x, expected %x\n",
@@ -333,6 +354,7 @@ void sfree(void *ptr)
        }
 
        if (pool) {
+               VALGRIND_FREELIKE_BLOCK(ptr, REDZONE_SIZE);
                sfree_pool(pool, ptr);
                return;
        }
@@ -423,7 +445,7 @@ static void *smalloc_pool(struct pool *pool, size_t size)
        return ptr;
 }
 
-void *smalloc(size_t size)
+static void *__smalloc(size_t size, bool is_zeroed)
 {
        unsigned int i, end_pool;
 
@@ -439,6 +461,9 @@ void *smalloc(size_t size)
 
                        if (ptr) {
                                last_pool = i;
+                               VALGRIND_MALLOCLIKE_BLOCK(ptr, size,
+                                                         REDZONE_SIZE,
+                                                         is_zeroed);
                                return ptr;
                        }
                }
@@ -456,9 +481,14 @@ void *smalloc(size_t size)
        return NULL;
 }
 
+void *smalloc(size_t size)
+{
+       return __smalloc(size, false);
+}
+
 void *scalloc(size_t nmemb, size_t size)
 {
-       return smalloc(nmemb * size);
+       return __smalloc(nmemb * size, true);
 }
 
 char *smalloc_strdup(const char *str)