[compiler-rt] 1893b63 - Avoid triggering assert when program calls OSAtomicCompareAndSwapLong

[Julian Lettner via llvm-commits]

Author: Julian Lettner
Date: 2021-07-13T09:33:50-07:00
New Revision: 1893b630fec06947b4f59e43c00db4d787f39262

URL: https://github.com/llvm/llvm-project/commit/1893b630fec06947b4f59e43c00db4d787f39262
DIFF: https://github.com/llvm/llvm-project/commit/1893b630fec06947b4f59e43c00db4d787f39262.diff

LOG: Avoid triggering assert when program calls OSAtomicCompareAndSwapLong

A previous change brought the new, relaxed implementation of "on failure
memory ordering" for synchronization primitives in LLVM over to TSan
land [1].  It included the following assert:
```
// 31.7.2.18: "The failure argument shall not be memory_order_release
// nor memory_order_acq_rel". LLVM (2021-05) fallbacks to Monotonic
// (mo_relaxed) when those are used.
CHECK(IsLoadOrder(fmo));

static bool IsLoadOrder(morder mo) {
  return mo == mo_relaxed || mo == mo_consume
      || mo == mo_acquire || mo == mo_seq_cst;
}
```

A previous workaround for a false positive when using an old Darwin
synchronization API assumed this failure mode to be unused and passed a
dummy value [2].  We update this value to `mo_relaxed` which is also the
value used by the actual implementation to avoid triggering the assert.

[1] https://reviews.llvm.org/D99434
[2] https://reviews.llvm.org/D21733

rdar://78122243

Differential Revision: https://reviews.llvm.org/D105844

Added: 
    

Modified: 
    compiler-rt/lib/tsan/rtl/tsan_interceptors_mac.cpp

Removed: 
    


################################################################################
diff  --git a/compiler-rt/lib/tsan/rtl/tsan_interceptors_mac.cpp b/compiler-rt/lib/tsan/rtl/tsan_interceptors_mac.cpp
index ed10fccc980a4..2d400c7e7098d 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_interceptors_mac.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_interceptors_mac.cpp
@@ -44,8 +44,9 @@ namespace __tsan {
 // actually aliases of each other, and we cannot have 
diff erent interceptors for
 // them, because they're actually the same function.  Thus, we have to stay
 // conservative and treat the non-barrier versions as mo_acq_rel.
-static const morder kMacOrderBarrier = mo_acq_rel;
-static const morder kMacOrderNonBarrier = mo_acq_rel;
+static constexpr morder kMacOrderBarrier = mo_acq_rel;
+static constexpr morder kMacOrderNonBarrier = mo_acq_rel;
+static constexpr morder kMacFailureOrder = mo_relaxed;
 
 #define OSATOMIC_INTERCEPTOR(return_t, t, tsan_t, f, tsan_atomic_f, mo) \
   TSAN_INTERCEPTOR(return_t, f, t x, volatile t *ptr) {                 \
@@ -110,7 +111,7 @@ OSATOMIC_INTERCEPTORS_BITWISE(OSAtomicXor, fetch_xor,
     SCOPED_TSAN_INTERCEPTOR(f, old_value, new_value, ptr);                  \
     return tsan_atomic_f##_compare_exchange_strong(                         \
         (volatile tsan_t *)ptr, (tsan_t *)&old_value, (tsan_t)new_value,    \
-        kMacOrderNonBarrier, kMacOrderNonBarrier);                          \
+        kMacOrderNonBarrier, kMacFailureOrder);                             \
   }                                                                         \
                                                                             \
   TSAN_INTERCEPTOR(bool, f##Barrier, t old_value, t new_value,              \
@@ -118,7 +119,7 @@ OSATOMIC_INTERCEPTORS_BITWISE(OSAtomicXor, fetch_xor,
     SCOPED_TSAN_INTERCEPTOR(f##Barrier, old_value, new_value, ptr);         \
     return tsan_atomic_f##_compare_exchange_strong(                         \
         (volatile tsan_t *)ptr, (tsan_t *)&old_value, (tsan_t)new_value,    \
-        kMacOrderBarrier, kMacOrderNonBarrier);                             \
+        kMacOrderBarrier, kMacFailureOrder);                                \
   }
 
 OSATOMIC_INTERCEPTORS_CAS(OSAtomicCompareAndSwapInt, __tsan_atomic32, a32, int)