Minimize use of AO_ATTR_NO_SANITIZE_THREAD in atomic_ops_malloc/stack

(fix commit c058d9d)

* src/atomic_ops_malloc.c [AO_THREAD_SANITIZER] (AO_malloc):
Use AO_store() only if AO_USE_ALMOST_LOCK_FREE; add comment.
* src/atomic_ops_malloc.c (AO_free): Remove TSan workaround (do not
check AO_THREAD_SANITIZER macro).
* src/atomic_ops_stack.c (AO_copy_before_cas): New inline function.
* 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_stack_push_explicit_aux_release): Replace *x=next with
AO_copy_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 AO_copy_before_cas(&next,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 f78ff4054f26fcb65008376b7bd1a2602029041b..06ed8330b41dc1d22491da073ca4bb248d706095 100644 (file)
--- a/src/atomic_ops_malloc.c
+++ b/src/atomic_ops_malloc.c
@@ -317,7 +317,8 @@ AO_malloc(size_t sz)
     add_chunk_as(chunk, log_sz);
     result = AO_stack_pop(AO_free_list+log_sz);
   }
-# ifdef AO_THREAD_SANITIZER
+# if defined(AO_THREAD_SANITIZER) && defined(AO_USE_ALMOST_LOCK_FREE)
+    /* A data race with AO_stack_pop() called above is a false positive. */
     AO_store(result, log_sz);
 # else
     *result = log_sz;
@@ -339,11 +340,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 b57ed854f34bd782bfad88c6ac17ab26ebdb2f48..a957642ba5bb542625f8654a6bd4cd9d461adad5 100644 (file)
--- a/src/atomic_ops_stack.c
+++ b/src/atomic_ops_stack.c
@@ -22,6 +22,14 @@
 #define AO_REQUIRE_CAS
 #include "atomic_ops_stack.h"
 
+/* The 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).     */
+AO_ATTR_NO_SANITIZE_THREAD
+AO_INLINE void AO_copy_before_cas(AO_t *pdest, AO_t *psrc)
+{
+  *pdest = *psrc;
+}
+
 #ifdef AO_USE_ALMOST_LOCK_FREE
 
   void AO_pause(int); /* defined in atomic_ops.c */
@@ -46,7 +54,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 +101,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 */
+      AO_copy_before_cas(x, &next);
     }
   while (AO_EXPECT_FALSE(!AO_compare_and_swap_release(list, next, x_bits)));
 }
@@ -207,14 +214,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 */
+      AO_copy_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       */
@@ -227,7 +233,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)
 {
 #   ifdef __clang__
@@ -244,8 +249,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;
+      AO_copy_before_cas(&next, cptr);
     } while (AO_EXPECT_FALSE(!AO_compare_double_and_swap_double_release(list,
                                         cversion, (AO_t)cptr,
                                         cversion+1, (AO_t)next)));
@@ -274,7 +280,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;
+      AO_copy_before_cas(element, &next_ptr);
     } while (!AO_compare_and_swap_double_release(
                            list, version,
                            version+1, (AO_t) element));
@@ -289,14 +295,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;
+      AO_copy_before_cas(&next, 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 */