Minimize use of AO_ATTR_NO_SANITIZE_THREAD in atomic_ops_malloc/stack

(Cherry-pick commits c058d9d, 110b0dc, 3aefd4e, 441415c from 'master'.)

* src/atomic_ops_malloc.c (AO_malloc, AO_free): Remove TSan workaround
(do not check AO_THREAD_SANITIZER macro).
* src/atomic_ops_stack.c (store_before_cas): New static function (or
defined as a macro).
* src/atomic_ops_stack.c [!USE_ALMOST_LOCK_FREE] (load_before_cas):
Likewiese.
* src/atomic_ops_stack.c [AO_USE_ALMOST_LOCK_FREE]
(AO_stack_push_explicit_aux_release): Remove AO_ATTR_NO_SANITIZE_THREAD.
* src/atomic_ops_stack.c [!USE_ALMOST_LOCK_FREE
&& AO_HAVE_compare_double_and_swap_double] (AO_stack_push_release,
AO_stack_pop_acquire): Likewise.
* src/atomic_ops_stack.c [AO_USE_ALMOST_LOCK_FREE
&& AO_THREAD_SANITIZER] (AO_load_next): New static function (with
AO_ATTR_NO_SANITIZE_THREAD attribute); add comments.
* src/atomic_ops_stack.c [AO_USE_ALMOST_LOCK_FREE
&& !AO_THREAD_SANITIZER] (AO_load_next): Define to AO_load.
* src/atomic_ops_stack.c [AO_USE_ALMOST_LOCK_FREE]
(AO_stack_pop_explicit_aux_acquire): Replace AO_load(first_ptr) with
AO_load_next(first_ptr).
* src/atomic_ops_stack.c [AO_USE_ALMOST_LOCK_FREE]
(AO_stack_push_explicit_aux_release): Replace *x=next with
store_before_cas(x,next).
* src/atomic_ops_stack.c [!USE_ALMOST_LOCK_FREE]
(AO_stack_push_release): Likewise.
* src/atomic_ops_stack.c [!USE_ALMOST_LOCK_FREE] (AO_stack_pop_acquire):
Replace next=*cptr with load_before_cas((AO_t*)cptr).
* src/atomic_ops_stack.c [!USE_ALMOST_LOCK_FREE
&& AO_HAVE_compare_and_swap_double] (AO_stack_pop_acquire): Reformat
code.

diff --git a/src/atomic_ops_malloc.c b/src/atomic_ops_malloc.c
index acb00a042a26617a85751e1b27562743f2a359d2..f86585d43c70d2be888dce72ede9615b6a8364c5 100644 (file)
--- a/src/atomic_ops_malloc.c
+++ b/src/atomic_ops_malloc.c
@@ -336,11 +336,7 @@ AO_malloc(size_t sz)
     add_chunk_as(chunk, log_sz);
     result = AO_stack_pop(AO_free_list+log_sz);
   }
-# ifdef AO_THREAD_SANITIZER
-    AO_store(result, log_sz);
-# else
-    *result = log_sz;
-# endif
+  *result = log_sz;
 # ifdef AO_TRACE_MALLOC
     fprintf(stderr, "%p: AO_malloc(%lu) = %p\n",
             (void *)pthread_self(), (unsigned long)sz, (void *)(result + 1));
@@ -359,11 +355,7 @@ AO_free(void *p)
     return;
 
   base = (AO_t *)p - 1;
-# ifdef AO_THREAD_SANITIZER
-    log_sz = (int)AO_load(base);
-# else
-    log_sz = (int)(*base);
-# endif
+  log_sz = (int)(*base);
 # ifdef AO_TRACE_MALLOC
     fprintf(stderr, "%p: AO_free(%p sz:%lu)\n", (void *)pthread_self(), p,
             log_sz > LOG_MAX_SIZE ? (unsigned)log_sz : 1UL << log_sz);

diff --git a/src/atomic_ops_stack.c b/src/atomic_ops_stack.c
index c364b672be317cc5770665f8e459409082666e08..11d385ec87dfee6fcb79f1ad00b41c282af7d629 100644 (file)
--- a/src/atomic_ops_stack.c
+++ b/src/atomic_ops_stack.c
@@ -22,6 +22,18 @@
 #define AO_REQUIRE_CAS
 #include "atomic_ops_stack.h"
 
+/* This function call must be a part of a do-while loop with a CAS      */
+/* designating the condition of the loop (see the use cases below).     */
+#ifdef AO_THREAD_SANITIZER
+  AO_ATTR_NO_SANITIZE_THREAD
+  static void store_before_cas(AO_t *addr, AO_t value)
+  {
+    *addr = value;
+  }
+#else
+# define store_before_cas(addr, value) (void)(*(addr) = (value))
+#endif
+
 #ifdef AO_USE_ALMOST_LOCK_FREE
 
   void AO_pause(int); /* defined in atomic_ops.c */
@@ -46,7 +58,6 @@
 /* to be inserted.                                                      */
 /* Both list headers and link fields contain "perturbed" pointers, i.e. */
 /* pointers with extra bits "or"ed into the low order bits.             */
-AO_ATTR_NO_SANITIZE_THREAD
 void AO_stack_push_explicit_aux_release(volatile AO_t *list, AO_t *x,
                                         AO_stack_aux *a)
 {
@@ -94,7 +105,7 @@ void AO_stack_push_explicit_aux_release(volatile AO_t *list, AO_t *x,
   do
     {
       next = AO_load(list);
-      *x = next; /* data race is OK here */
+      store_before_cas(x, next);
     }
   while (AO_EXPECT_FALSE(!AO_compare_and_swap_release(list, next, x_bits)));
 }
@@ -115,6 +126,21 @@ void AO_stack_push_explicit_aux_release(volatile AO_t *list, AO_t *x,
 # define PRECHECK(a)
 #endif
 
+/* This function is used before CAS in the below AO_stack_pop() and the */
+/* data race (reported by TSan) is OK because it results in a retry.    */
+#ifdef AO_THREAD_SANITIZER
+  AO_ATTR_NO_SANITIZE_THREAD
+  static AO_t AO_load_next(volatile AO_t *first_ptr)
+  {
+    /* Assuming an architecture on which loads of word type are atomic. */
+    /* AO_load cannot be used here because it cannot be instructed to   */
+    /* suppress the warning about the race.                             */
+    return *first_ptr;
+  }
+#else
+# define AO_load_next AO_load
+#endif
+
 AO_t *
 AO_stack_pop_explicit_aux_acquire(volatile AO_t *list, AO_stack_aux * a)
 {
@@ -169,7 +195,7 @@ AO_stack_pop_explicit_aux_acquire(volatile AO_t *list, AO_stack_aux * a)
     goto retry;
   }
   first_ptr = AO_REAL_NEXT_PTR(first);
-  next = AO_load(first_ptr);
+  next = AO_load_next(first_ptr);
 # if defined(__alpha__) && (__GNUC__ == 4)
     if (!AO_compare_and_swap_release(list, first, next))
 # else
@@ -197,6 +223,25 @@ AO_stack_pop_explicit_aux_acquire(volatile AO_t *list, AO_stack_aux * a)
 
 #else /* ! USE_ALMOST_LOCK_FREE */
 
+/* The functionality is the same as of AO_load_next but the atomicity   */
+/* is not needed.  The usage is similar to that of store_before_cas.    */
+#ifdef AO_THREAD_SANITIZER
+  /* TODO: If compiled by Clang (as of clang-4.0) with -O3 flag,        */
+  /* no_sanitize attribute is ignored unless the argument is volatile.  */
+# if defined(__clang__)
+#   define LOAD_BEFORE_CAS_VOLATILE volatile
+# else
+#   define LOAD_BEFORE_CAS_VOLATILE /* empty */
+# endif
+  AO_ATTR_NO_SANITIZE_THREAD
+  static AO_t load_before_cas(const LOAD_BEFORE_CAS_VOLATILE AO_t *addr)
+  {
+    return *addr;
+  }
+#else
+# define load_before_cas(addr) (*(addr))
+#endif
+
 /* Better names for fields in AO_stack_t */
 #define ptr AO_val2
 #define version AO_val1
@@ -209,14 +254,13 @@ AO_stack_pop_explicit_aux_acquire(volatile AO_t *list, AO_stack_aux * a)
   volatile /* non-static */ AO_t AO_noop_sink;
 #endif
 
-AO_ATTR_NO_SANITIZE_THREAD
 void AO_stack_push_release(AO_stack_t *list, AO_t *element)
 {
     AO_t next;
 
     do {
       next = AO_load(&(list -> ptr));
-      *element = next; /* data race is OK here */
+      store_before_cas(element, next);
     } while (AO_EXPECT_FALSE(!AO_compare_and_swap_release(&(list -> ptr),
                                                       next, (AO_t)element)));
     /* This uses a narrow CAS here, an old optimization suggested       */
@@ -229,7 +273,6 @@ void AO_stack_push_release(AO_stack_t *list, AO_t *element)
 #   endif
 }
 
-AO_ATTR_NO_SANITIZE_THREAD
 AO_t *AO_stack_pop_acquire(AO_stack_t *list)
 {
 #   if defined(__clang__) && !AO_CLANG_PREREQ(3, 5)
@@ -246,8 +289,9 @@ AO_t *AO_stack_pop_acquire(AO_stack_t *list)
       /* Version must be loaded first.  */
       cversion = AO_load_acquire(&(list -> version));
       cptr = (AO_t *)AO_load(&(list -> ptr));
-      if (cptr == 0) return 0;
-      next = *cptr; /* data race is OK here */
+      if (NULL == cptr)
+        return NULL;
+      next = load_before_cas((AO_t *)cptr);
     } while (AO_EXPECT_FALSE(!AO_compare_double_and_swap_double_release(list,
                                         cversion, (AO_t)cptr,
                                         cversion+1, (AO_t)next)));
@@ -273,7 +317,7 @@ void AO_stack_push_release(AO_stack_t *list, AO_t *element)
       /* Again version must be loaded first, for different reason.      */
       version = AO_load_acquire(&(list -> version));
       next_ptr = AO_load(&(list -> ptr));
-      *element = next_ptr;
+      store_before_cas(element, next_ptr);
     } while (!AO_compare_and_swap_double_release(
                            list, version,
                            version+1, (AO_t) element));
@@ -288,14 +332,15 @@ AO_t *AO_stack_pop_acquire(AO_stack_t *list)
     do {
       cversion = AO_load_acquire(&(list -> version));
       cptr = (AO_t *)AO_load(&(list -> ptr));
-      if (cptr == 0) return 0;
-      next = *cptr;
-    } while (!AO_compare_double_and_swap_double_release
-                    (list, cversion, (AO_t) cptr, cversion+1, next));
+      if (NULL == cptr)
+        return NULL;
+      next = load_before_cas(cptr);
+    } while (!AO_compare_double_and_swap_double_release(list,
+                                                cversion, (AO_t)cptr,
+                                                cversion+1, next));
     return cptr;
 }
 
-
 #endif /* AO_HAVE_compare_and_swap_double */
 
 #endif /* ! USE_ALMOST_LOCK_FREE */