[PATCH] D99434: [TSAN] Honor failure memory orders in AtomicCAS

[Dmitry Vyukov via Phabricator via llvm-commits]

dvyukov added inline comments.


================
Comment at: compiler-rt/lib/tsan/rtl/tsan_interface_atomic.cpp:406
+  // 31.7.2.18: "The failure argument shall not be memory_order_release
+  // nor memory_order_acq_rel". LLVM (2021-05) fallbacks to Monotonic (relaxed)
+  // when those are used.
----------------
Can it be seq_cst?
If yes, then I think write_lock below is incorrect and checking only IsAcquireOrder(fmo) is incorrect as well.


================
Comment at: compiler-rt/lib/tsan/rtl/tsan_interface_atomic.cpp:416
   T cc = *c;
   T pr = func_cas(a, cc, v);
+  if (pr == cc) {
----------------
The CAS needs to be evaluated under the sync object mutex, otherwise we can get inconsistent value/memory visibility. If we are going to lock the mutex, we need to lock it before CAS.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99434/new/

https://reviews.llvm.org/D99434