util: Detect use-after-destroy in simple_mtx

This makes simple_mtx_destroy set the counter to an invalid canary
value and then makes lock/unlock assert that the value is legal.

That way, calling lock/unlock after destroy will assert fail,
rather than deadlocking or potentially even working.

This has caught real deadlocks in dEQP multithreaded tests (in st/mesa
shader variant zombie list handling), which have since been fixed.

Reviewed-by: Tapani Pälli <tapani.palli@intel.com>
Reviewed-by: Eric Engestrom <eric@engestrom.ch>

diff --git a/src/util/simple_mtx.h b/src/util/simple_mtx.h
index 94ab6fcef1ff469a340a5bb36c4e24c1f41aeaba..c0cc13b3663ec75fb49619482f41221b79e63cc6 100644 (file)
--- a/src/util/simple_mtx.h
+++ b/src/util/simple_mtx.h
@@ -60,6 +60,8 @@ typedef struct {
 
 #define _SIMPLE_MTX_INITIALIZER_NP { 0 }
 
+#define _SIMPLE_MTX_INVALID_VALUE 0xd0d0d0d0
+
 static inline void
 simple_mtx_init(simple_mtx_t *mtx, ASSERTED int type)
 {
@@ -69,8 +71,9 @@ simple_mtx_init(simple_mtx_t *mtx, ASSERTED int type)
 }
 
 static inline void
-simple_mtx_destroy(UNUSED simple_mtx_t *mtx)
+simple_mtx_destroy(simple_mtx_t *mtx)
 {
+   mtx->val = _SIMPLE_MTX_INVALID_VALUE;
 }
 
 static inline void
@@ -79,6 +82,9 @@ simple_mtx_lock(simple_mtx_t *mtx)
    uint32_t c;
 
    c = __sync_val_compare_and_swap(&mtx->val, 0, 1);
+
+   assert(c != _SIMPLE_MTX_INVALID_VALUE);
+
    if (__builtin_expect(c != 0, 0)) {
       if (c != 2)
          c = __sync_lock_test_and_set(&mtx->val, 2);
@@ -95,6 +101,9 @@ simple_mtx_unlock(simple_mtx_t *mtx)
    uint32_t c;
 
    c = __sync_fetch_and_sub(&mtx->val, 1);
+
+   assert(c != _SIMPLE_MTX_INVALID_VALUE);
+
    if (__builtin_expect(c != 1, 0)) {
       mtx->val = 0;
       futex_wake(&mtx->val, 1);