google / sanitizers

AddressSanitizer, ThreadSanitizer, MemorySanitizer
Other
11.5k stars 1.04k forks source link

tsan: false negative on atomic operations #1014

Open dvyukov opened 6 years ago

dvyukov commented 6 years ago

The program is:

#include <thread>
#include <atomic>

struct node { std::atomic<int> value; };
std::atomic<node*> _node{nullptr};

void f1() {
  auto n = new node();
  n->value.store(1, std::memory_order_release);
  _node.store(n, std::memory_order_relaxed);
}

void f2() {
  auto n = _node.load(std::memory_order_relaxed);
  while (n == nullptr)
    n = _node.load(std::memory_order_relaxed);
#if 1
  n->value.fetch_add(1, std::memory_order_acquire);
#else
  int v = n->value.load(std::memory_order_acquire);
  n->value.store(1, std::memory_order_relaxed);
#endif
}

int main() {
  std::thread t1(f1);
  std::thread t2(f2);
  t1.join();
  t2.join();
  return 0;
}

With the fetch_add it correctly produces:

WARNING: ThreadSanitizer: data race (pid=143269)
  Atomic write of size 4 at 0x7b0400000800 by thread T2:
    #0 __tsan_atomic32_fetch_add tsan_interface_atomic.cc:616 (a.out+0x49b974)
    #1 std::__atomic_base<int>::fetch_add(int, std::memory_order) atomic_base.h:514:16 (a.out+0x4c668b)
    #2 f2() /tmp/tsan.cc:18 (a.out+0x4c668b)

  Previous write of size 8 at 0x7b0400000800 by thread T1:
    #0 operator new(unsigned long) tsan_new_delete.cc:65 (a.out+0x4c553a)
    #1 f1() /tmp/tsan.cc:8:12 (a.out+0x4c64ac)

  Location is heap block of size 4 at 0x7b0400000800 allocated by thread T1:
    #0 operator new(unsigned long) tsan_new_delete.cc:65 (a.out+0x4c553a)
    #1 f1() /tmp/tsan.cc:8:12 (a.out+0x4c64ac)

But with separate load/store it does not yield the race report, though it looks exactly the same wrt raciness.

See #1009 for context. Credit goes to @mpoeter. FTR, llvm is on 336240.

dvyukov commented 6 years ago

@mpoeter notes in #1009 that we could report the first access as read, which would be more precise. May be not feasible/easy to do as we model RMW as a single write IIRC.