Fix data race in freed_pool

This adds _cairo_atomic_int_get_relaxed and _cairo_atomic_int_set_relaxed which
are meant to have a behaviour of relaxed read/writes in C11's memory model. This
patch also uses these new function to fix a data race with freed_pool_t's |top|
data member.

Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=90318
Signed-off-by: Wan-Teh Chang <wtc@google.com>
Signed-off-by: Uli Schlachter <psychon@znc.in>

src/cairo-atomic-private.h

@@ -75,6 +75,18 @@ _cairo_atomic_int_get (cairo_atomic_int_t *x)
     return __atomic_load_n(x, __ATOMIC_SEQ_CST);
 }
 
+static cairo_always_inline cairo_atomic_int_t
+_cairo_atomic_int_get_relaxed (cairo_atomic_int_t *x)
+{
+    return __atomic_load_n(x, __ATOMIC_RELAXED);
+}
+
+static cairo_always_inline void
+_cairo_atomic_int_set_relaxed (cairo_atomic_int_t *x, cairo_atomic_int_t val)
+{
+    __atomic_store_n(x, val, __ATOMIC_RELAXED);
+}
+
 static cairo_always_inline void *
 _cairo_atomic_ptr_get (void **x)
 {
@@ -157,6 +169,18 @@ _cairo_atomic_int_get (cairo_atomic_int_t *x)
     return *x;
 }
 
+static cairo_always_inline cairo_atomic_int_t
+_cairo_atomic_int_get_relaxed (cairo_atomic_int_t *x)
+{
+    return *x;
+}
+
+static cairo_always_inline void
+_cairo_atomic_int_set_relaxed (cairo_atomic_int_t *x, cairo_atomic_int_t val)
+{
+    *x = val;
+}
+
 static cairo_always_inline void *
 _cairo_atomic_ptr_get (void **x)
 {
@@ -165,6 +189,8 @@ _cairo_atomic_ptr_get (void **x)
 }
 #else
 # define _cairo_atomic_int_get(x) (*x)
+# define _cairo_atomic_int_get_relaxed(x) (*x)
+# define _cairo_atomic_int_set_relaxed(x, val) (*x) = (val)
 # define _cairo_atomic_ptr_get(x) (*x)
 #endif
 
@@ -200,6 +226,8 @@ typedef long long cairo_atomic_intptr_t;
 typedef  AO_t cairo_atomic_int_t;
 
 # define _cairo_atomic_int_get(x) (AO_load_full (x))
+# define _cairo_atomic_int_get_relaxed(x) (AO_load_full (x))
+# define _cairo_atomic_int_set_relaxed(x, val) (AO_store_full ((x), (val)))
 
 # define _cairo_atomic_int_inc(x) ((void) AO_fetch_and_add1_full(x))
 # define _cairo_atomic_int_dec(x) ((void) AO_fetch_and_sub1_full(x))
@@ -230,6 +258,8 @@ typedef unsigned long long cairo_atomic_intptr_t;
 typedef int32_t cairo_atomic_int_t;
 
 # define _cairo_atomic_int_get(x) (OSMemoryBarrier(), *(x))
+# define _cairo_atomic_int_get_relaxed(x) *(x)
+# define _cairo_atomic_int_set_relaxed(x, val) *(x) = (val)
 
 # define _cairo_atomic_int_inc(x) ((void) OSAtomicIncrement32Barrier (x))
 # define _cairo_atomic_int_dec(x) ((void) OSAtomicDecrement32Barrier (x))
@@ -288,9 +318,15 @@ _cairo_atomic_ptr_cmpxchg_return_old_impl (void **x, void *oldv, void *newv);
 #ifdef ATOMIC_OP_NEEDS_MEMORY_BARRIER
 cairo_private cairo_atomic_int_t
 _cairo_atomic_int_get (cairo_atomic_int_t *x);
+cairo_private cairo_atomic_int_t
+_cairo_atomic_int_get_relaxed (cairo_atomic_int_t *x);
+void
+_cairo_atomic_int_set_relaxed (cairo_atomic_int_t *x, cairo_atomic_int_t val);
 # define _cairo_atomic_ptr_get(x) (void *) _cairo_atomic_int_get((cairo_atomic_int_t *) x)
 #else
 # define _cairo_atomic_int_get(x) (*x)
+# define _cairo_atomic_int_get_relaxed(x) (*x)
+# define _cairo_atomic_int_set_relaxed(x, val) (*x) = (val)
 # define _cairo_atomic_ptr_get(x) (*x)
 #endif
 

src/cairo-atomic.c

@@ -101,6 +101,20 @@ _cairo_atomic_int_get (cairo_atomic_intptr_t *x)
 
     return ret;
 }
+
+cairo_atomic_intptr_t
+_cairo_atomic_int_get_relaxed (cairo_atomic_intptr_t *x)
+{
+    return _cairo_atomic_int_get (x);
+}
+
+void
+_cairo_atomic_int_set_relaxed (cairo_atomic_intptr_t *x, cairo_atomic_intptr_t val)
+{
+    CAIRO_MUTEX_LOCK (_cairo_atomic_mutex);
+    *x = val;
+    CAIRO_MUTEX_UNLOCK (_cairo_atomic_mutex);
+}
 #endif
 
 #endif

src/cairo-freed-pool-private.h

@@ -51,7 +51,7 @@ CAIRO_BEGIN_DECLS
 #define MAX_FREED_POOL_SIZE 16
 typedef struct {
     void *pool[MAX_FREED_POOL_SIZE];
-    int top;
+    cairo_atomic_int_t top;
 } freed_pool_t;
 
 static cairo_always_inline void *
@@ -81,13 +81,13 @@ _freed_pool_get (freed_pool_t *pool)
     void *ptr;
     int i;
 
-    i = pool->top - 1;
+    i = _cairo_atomic_int_get_relaxed (&pool->top) - 1;
     if (i < 0)
 	i = 0;
 
     ptr = _atomic_fetch (&pool->pool[i]);
     if (likely (ptr != NULL)) {
-	pool->top = i;
+	_cairo_atomic_int_set_relaxed (&pool->top, i);
 	return ptr;
     }
 
@@ -103,11 +103,11 @@ _freed_pool_put (freed_pool_t *pool, void *ptr)
 {
     int i;
 
-    i = pool->top;
+    i = _cairo_atomic_int_get_relaxed (&pool->top);
     if (likely (i < ARRAY_LENGTH (pool->pool) &&
 		_atomic_store (&pool->pool[i], ptr)))
     {
-	pool->top = i + 1;
+	_cairo_atomic_int_set_relaxed (&pool->top, i + 1);
 	return;
     }
 

src/cairo-freed-pool.c

@@ -50,13 +50,13 @@ _freed_pool_get_search (freed_pool_t *pool)
     for (i = ARRAY_LENGTH (pool->pool); i--;) {
 	ptr = _atomic_fetch (&pool->pool[i]);
 	if (ptr != NULL) {
-	    pool->top = i;
+	    _cairo_atomic_int_set_relaxed (&pool->top, i);
 	    return ptr;
 	}
     }
 
     /* empty */
-    pool->top = 0;
+    _cairo_atomic_int_set_relaxed (&pool->top, 0);
     return NULL;
 }
 
@@ -67,13 +67,13 @@ _freed_pool_put_search (freed_pool_t *pool, void *ptr)
 
     for (i = 0; i < ARRAY_LENGTH (pool->pool); i++) {
 	if (_atomic_store (&pool->pool[i], ptr)) {
-	    pool->top = i + 1;
+	    _cairo_atomic_int_set_relaxed (&pool->top, i + 1);
 	    return;
 	}
     }
 
     /* full */
-    pool->top = i;
+    _cairo_atomic_int_set_relaxed (&pool->top, i);
     free (ptr);
 }
 
@@ -87,7 +87,7 @@ _freed_pool_reset (freed_pool_t *pool)
 	pool->pool[i] = NULL;
     }
 
-    pool->top = 0;
+    _cairo_atomic_int_set_relaxed (&pool->top, 0);
 }
 
 #endif