From 0ffccc21fcd67d1e1d2a360e90f3fe8efc0d6b52 Mon Sep 17 00:00:00 2001
From: Bart Van Assche <bart.vanassche@wdc.com>
Date: Thu, 8 Mar 2018 13:41:36 -0800
Subject: [PATCH] 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>
---
 configure | 21 +++++++++++++++++++++
 fio_sem.c | 19 +++++++++++++++----
 rwlock.c  |  1 +
 smalloc.c | 34 ++++++++++++++++++++++++++++++++--
 4 files changed, 69 insertions(+), 6 deletions(-)

diff --git a/configure b/configure
index f38e9c75..ddf03a6b 100755
--- 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 b9a1bf1e..20fcfccc 100644
--- 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 7d9ad731..00e38098 100644
--- 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 d19e108a..13995acc 100644
--- 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)
-- 
2.25.1