tsan: intercept libatomic

[dvyukov]

Summary

When compiler can't emit native code for atomic operations, it instead emits calls to libatomic which provides fallback atomic operation implementation. For all relevant architectures the fallback happens only for weirdly-sized atomics (e.g. 7 or 64 bytes). There are 4 relevant functions in libatomic: atomic_load, atomic_store, atomic_exchange and atomic_compare_exchange (i.e. we can't do fetch_add on a 64-byte struct). See section 1.2.4 of http://stdatomic.gforge.inria.fr for details of the interface.

Libatomic fallback causes both false positives and false negatives for tsan. Current implementation uses pthread_mutex_t to guard atomic operations, which tsan intercepts and that causes massive false negatives. However, 16-byte operations are done with CMPXCHG16B if possible and that becomes completely invisible to tsan and causes false positives.

Intercept libatomic functions for variable-sized atomics and model them precisely for tsan.

[kcc]

lib/interception/interception.h

77

"we may have been already included "
something here is redundant

78

do we include std headers?

156

Sounds like an intrusive change. How portable is it?
Any other alternatives?

[dvyukov]

lib/interception/interception.h

77

What is redundant here? Looks proper to me.
http://lmgtfy.com/?q=%22may+have+been+already%22

78

this change resolves the only issue that prevented us from including std headers

you can consider that for builtins compiler has preincluded some magic standard headers that define builtins in a way that precludes most of usages

156

I can't think of any other alternatives. One can't refer to builtins other than calling them.
More portable then the rest of magic here as this involves only compiler, while the rest also involves assembler and linker.

[kcc]

lib/interception/interception.h

77

"been" feels redundant (not a native English speaker here)
"we may have already included "?

78

Sorry, please explain in more detail.

Where are we including / not including the std headers?
In user code or in the sanitizer run-time?

One other thing that we can do with builtins is to replace them with hard calls at compile time,
we already do that with memset in asan. Is that applicable here?

[kubamracek]

This patch currently doesn’t compile on Darwin:

tsan_interceptors.cc:1347:24: error: conflicting types for '__atomic_load'
  TSAN_INTERCEPTOR(void, __atomic_load, SIZE_T n, void *a, void *v, int ord) {
tsan_interceptors.cc:1347:24: note: '__atomic_load' is a builtin with type 'void ()'
tsan_interceptors.cc:1347:24: error: builtin functions must be directly called
  TSAN_INTERCEPTOR(void, __atomic_load, SIZE_T n, void *a, void *v, int ord) {
tsan_interceptors.cc:1348:27: error: builtin functions must be directly called
  SCOPED_TSAN_INTERCEPTOR(__atomic_load, n, a, v, ord);
tsan_interceptors.cc:1348:51: error: too many arguments to function call, expected 3, have 4
  SCOPED_TSAN_INTERCEPTOR(__atomic_load, n, a, v, ord);

I noticed that lib/builtins/atomic.c uses #pragma redefine_extname to work around this. And this also seems to work for Darwin interceptors, e.g. something like:

#pragma redefine_extname __atomic_load_c ___atomic_load
TSAN_INTERCEPTOR(void, __atomic_load_c, SIZE_T n, void *a, void *v, int ord) {
   SCOPED_TSAN_INTERCEPTOR(__atomic_load_c, n, a, v, ord);
   AtomicLoad(thr, pc, n, a, v, ord);
}

Can this also be used on Linux?

test/tsan/atomic_test.cc

1

Add -std=c++11 to all compile commands.

5–11

-latomic doesn’t compile on Darwin. On Darwin, this parameter is not needed.

[dvyukov]

In D25509#572230, @kubabrecka wrote:

This patch currently doesn’t compile on Darwin:

tsan_interceptors.cc:1347:24: error: conflicting types for '__atomic_load'
  TSAN_INTERCEPTOR(void, __atomic_load, SIZE_T n, void *a, void *v, int ord) {
tsan_interceptors.cc:1347:24: note: '__atomic_load' is a builtin with type 'void ()'
tsan_interceptors.cc:1347:24: error: builtin functions must be directly called
  TSAN_INTERCEPTOR(void, __atomic_load, SIZE_T n, void *a, void *v, int ord) {
tsan_interceptors.cc:1348:27: error: builtin functions must be directly called
  SCOPED_TSAN_INTERCEPTOR(__atomic_load, n, a, v, ord);
tsan_interceptors.cc:1348:51: error: too many arguments to function call, expected 3, have 4
  SCOPED_TSAN_INTERCEPTOR(__atomic_load, n, a, v, ord);

I noticed that lib/builtins/atomic.c uses #pragma redefine_extname to work around this. And this also seems to work for Darwin interceptors, e.g. something like:

#pragma redefine_extname __atomic_load_c ___atomic_load
TSAN_INTERCEPTOR(void, __atomic_load_c, SIZE_T n, void *a, void *v, int ord) {
   SCOPED_TSAN_INTERCEPTOR(__atomic_load_c, n, a, v, ord);
   AtomicLoad(thr, pc, n, a, v, ord);
}

Can this also be used on Linux?

redefine_extname seems to be introduced only for compatibility with solaris compiler and has a number of downsides as compared to asm labels:
https://gcc.gnu.org/onlinedocs/gcc/Symbol-Renaming-Pragmas.html

Why not use asm labels on darwin instead?

I think we need something along the lines of:

• 1. • ../../interception/interception.h (revision 283521) +++ ../../interception/interception.h (working copy) @@ -160,9 +160,9 @@ }
  2. define ASSIGN_REAL(dst, src) REAL(dst) = REAL(src) #else // APPLE
  3. define REAL(x) x +# define REAL(x) __interceptor_fake_ ## x
  4. define DECLARE_REAL(ret_type, func, ...) \
• extern "C" ret_type func(VA_ARGS); + extern "C" ret_type interceptor_fake_ ## func(VA_ARGS__) asm(#func);
  1. define ASSIGN_REAL(x, y) #endif // APPLE

But interception.h is total mess so probably I missed something. Can you please test this change on darwin?

lib/interception/interception.h

77

will do

78

Where are we including / not including the std headers?

we don't

Is that applicable here?

Probably.
Is it better? Why?

[dvyukov]

test/tsan/atomic_test.cc

1

Isn't -std=c++11 a part of %clangxx_tsan?
How does "#include <atomic>" work then?

[dvyukov]

In D25509#572530, @dvyukov wrote:

I think we need something along the lines of:

Please use "View Raw" to view the patch. I've tried posting the patch as is and prefixed with spaces. This thing does not seem to be meant for showing programming stuff...

[jakubjelinek]

Defining the builtins directly is certainly not going to work very well.
E.g. GCC libatomic uses:

void libat_load (size_t, void *, void *, int) __asm ("__atomic_load");

or e.g.

void lbat_load (size_t, void *, void *, int);
...
typeof (libat_load) export_load __asm ("__atomic_load") __attribute__((alias ("libat_load")));

or ifuncs.

[kubamracek]

test/tsan/atomic_test.cc

1

Apparently, we only add -std=c++11 when we have an instrumented libcxx, see test/tsan/lit.cfg. I submitted https://reviews.llvm.org/D25740 to always use -std=c++11.

[kubamracek]

In D25509#572539, @dvyukov wrote:

In D25509#572530, @dvyukov wrote:

I think we need something along the lines of:

Please use "View Raw" to view the patch. I've tried posting the patch as is and prefixed with spaces. This thing does not seem to be meant for showing programming stuff...

Prefix code blocks with 4 spaces, then they will get displayed nicely.

[kubamracek]

Why not use asm labels on darwin instead?
I think we need something along the lines of ...

This breaks a lot of other interceptors with various errors ranging from “use of undeclared identifier” to "cannot apply asm label to function after its first use” and requires a lot of other changes to interception.h to make it work (I gave up after 3 or 4 tries to fix this). Do we really need these intrusive changes to interception.h? What about declaring a new type of interception and using it just for the libatomic wrappers (and leave the existing interceptors as they are)?

[kubamracek]

Any news here? I didn't want to block this patch, I only noticed that it doesn't build on macOS.

[dvyukov]

Any news here?

No news yet. I wanted to get a darwin machine somewhere to figure out a working solution, but did not figure out where to get a good darwin machine yet... and then was paged out by other stuff.

I didn't want to block this patch, I only noticed that it doesn't build on macOS.

Well, that's serious enough reason to not submit it as is :)

What about declaring a new type of interception and using it just for the libatomic wrappers (and leave the existing interceptors as they are)?

We can, of course, resort to this.
But generally I prefer to have a single solution that works in all cases, rather than one solution that works for A but not for B + another that works for B but not for A.

How bad are darwin breakages? I mean while testing linux build I had to do changes like:

• int res = vswprintf(str, size, format, ap); + int res = WRAP(vswprintf)(str, size, format, ap);

Which was just incorrect code in the first place. All intercepted functions should be called via WRAP or REAL.
Can it's be the case that we just need more of that for darwin? And maybe declare some interceptrs before calling the "real" counterparts (looking at errors that you mentioned)?

[dvyukov]

In D25509#642551, @dvyukov wrote:

Any news here?

No news yet. I wanted to get a darwin machine somewhere to figure out a working solution, but did not figure out where to get a good darwin machine yet... and then was paged out by other stuff.

I didn't want to block this patch, I only noticed that it doesn't build on macOS.

Well, that's serious enough reason to not submit it as is :)

What about declaring a new type of interception and using it just for the libatomic wrappers (and leave the existing interceptors as they are)?

We can, of course, resort to this.
But generally I prefer to have a single solution that works in all cases, rather than one solution that works for A but not for B + another that works for B but not for A.

How bad are darwin breakages? I mean while testing linux build I had to do changes like:

• int res = vswprintf(str, size, format, ap); + int res = WRAP(vswprintf)(str, size, format, ap);

/\/\/\

You can see that I prefixed it with 4 spaces, but it's still broken when displayed....

[kubamracek]

One way to make this work on Darwin is by using the __asm trick:

extern "C" void my__atomic_load (SIZE_T, void *, void *, int) __asm ("___atomic_load");
extern "C" void my__atomic_store (SIZE_T, void *, void *, int) __asm ("___atomic_store");
extern "C" void my__atomic_exchange (SIZE_T, void *, void *, void *, int) __asm ("___atomic_exchange");
extern "C" bool my__atomic_compare_exchange (SIZE_T, void *, void *, void *, int, int) __asm ("___atomic_compare_exchange");

TSAN_INTERCEPTOR(void, my__atomic_load, SIZE_T n, void *a, void *v, int ord) {
  SCOPED_TSAN_INTERCEPTOR(my__atomic_load, n, a, v, ord);
  AtomicLoad(thr, pc, n, a, v, ord);
}

TSAN_INTERCEPTOR(void, my__atomic_store, SIZE_T n, void *a, void *v, int ord) {
  SCOPED_TSAN_INTERCEPTOR(my__atomic_store, n, a, v, ord);
  AtomicStore(thr, pc, n, a, v, ord);
}

TSAN_INTERCEPTOR(void, my__atomic_exchange, SIZE_T n, void *a, void *v, void *ret,
    int ord) {
  SCOPED_TSAN_INTERCEPTOR(my__atomic_exchange, n, a, v, ret, ord);
  AtomicExchange(thr, pc, n, a, v, ret, ord);
}

TSAN_INTERCEPTOR(bool, my__atomic_compare_exchange, SIZE_T n, void *a, void *cmp,
    void *v, int sord, int ford) {
  SCOPED_TSAN_INTERCEPTOR(my__atomic_compare_exchange, n, a, cmp, v, sord, ford);
  return AtomicCompareExchange(thr, pc, n, a, cmp, v, sord, ford);
}

I think this is a pretty clean solution (it just allows us to use the __atomic_* symbols without parse errors). Note that Darwin needs triple underscores there, I believe this is different from Linux. If I also remove -latomic from test_atomic.cc, this builds fine and passes the tests.

Let me know if/why you don't like this solution and I can test other approaches on Darwin.

[dvyukov]

One way to make this work on Darwin is by using the __asm trick:

Thank you for testing.

I updated this patch to use __asm instead of asm, and added _ to asm names on darwin. Does this work on darwin?
One thing that I did not do yet is removing -latomic from tests. Linux linker requires it, I think it is wrong, but it is the way it is.
Do you have a good suggestion for how to conditionally remove it on darwin? Maybe we should make lit always add -latomic for tsan tests on linux?

The test also currently fails with libc++:
https://llvm.org/bugs/show_bug.cgi?id=31620

[kubamracek]

Does this work on darwin?

Still fails to build -- the lines you changed in interception.h aren't even used on Darwin.

tsan_interceptors.cc:1388:24: error: conflicting types for '__atomic_load'
TSAN_INTERCEPTOR(void, __atomic_load, SIZE_T n, void *a, void *v, int ord) {
                       ^
tsan_interceptors.cc:1388:24: note: '__atomic_load' is a builtin with type 'void ()'
tsan_interceptors.cc:1388:24: error: builtin functions must be directly called
TSAN_INTERCEPTOR(void, __atomic_load, SIZE_T n, void *a, void *v, int ord) {
                       ^
tsan_interceptors.cc:1389:27: error: builtin functions must be directly called
  SCOPED_TSAN_INTERCEPTOR(__atomic_load, n, a, v, ord);

My suggestion was to declare __interceptor_fake_* directly in tsan_interceptors.cc just for the few __atomic_* functions, but I guess you're looking for a general solution.

Do you have a good suggestion for how to conditionally remove it on darwin?

Adding a %link_atomic lit substitution (that would expand to nothing on Darwin) sounds good enough to me.

lib/interception/interception.h

155–159

This part is already only used on Linux. There's a #ifdef __APPLE_ above (line 99).

[dvyukov]

I don't have time to figure out how to support Mac. Filed https://github.com/google/sanitizers/issues/816 so that this is not lost.

lib/interception/interception.h

 // How it works:
 // To replace system functions on Linux we just need to declare functions
 // with same names in our library and then obtain the real function pointers
 // using dlsym().
 // There is one complication. A user may also intercept some of the functions
 // we intercept. To resolve this we declare our interceptors with __interceptor_
 // prefix, and then make actual interceptors weak aliases to __interceptor_
 // functions.
+// Another complication is that we may have been already included declaration
+// for an intercepted function from standard headers, and then get declaration
+// mismatch between standard and our signatures (e.g. standard declarations can
+// declare some arguments as __restrict). To circumvent this we declare actual
+// interceptors with __interceptor_fake_ prefix and them strip the prefix with
+// asm directive.
+// Another complication is that an intercepted function can also be a compiler
+// builtin (e.g. __atomic_load). Fortunately, __interceptor_fake_ hack also
+// resolves this problem.
 //
 // This is not so on Mac OS, where the two-level namespace makes
 // our replacement functions invisible to other libraries. This may be overcomed
 // using the DYLD_FORCE_FLAT_NAMESPACE, but some errors loading the shared
 // libraries in Chromium were noticed when doing so.
 // Instead we create a dylib containing a __DATA,__interpose section that
 // associates library functions with their wrappers. When this dylib is
 // preloaded before an executable using DYLD_INSERT_LIBRARIES, it routes all

 // in position-independent (-fPIC / -fPIE) mode.
 # define DECLARE_WRAPPER(ret_type, func, ...) \
      extern "C" ret_type func(__VA_ARGS__) \
      __attribute__((alias("__interceptor_" #func), visibility("default")));
 #else
 # define WRAP(x) __interceptor_ ## x
 # define WRAPPER_NAME(x) "__interceptor_" #x
 # define INTERCEPTOR_ATTRIBUTE __attribute__((visibility("default")))
 # define DECLARE_WRAPPER(ret_type, func, ...) \
-    extern "C" ret_type func(__VA_ARGS__) \
+    extern "C" ret_type __interceptor_fake_ ## func(__VA_ARGS__) asm(#func) \
     __attribute__((weak, alias("__interceptor_" #func), visibility("default")));
 #endif
 
 #if !defined(__APPLE__)
 # define PTR_TO_REAL(x) real_##x
 # define REAL(x) __interception::PTR_TO_REAL(x)
 # define FUNC_TYPE(x) x##_f
 
 # define DECLARE_REAL(ret_type, func, ...) \
     typedef ret_type (*FUNC_TYPE(func))(__VA_ARGS__); \
     namespace __interception { \

lib/interception/interception_linux.h

 bool GetRealFunctionAddress(const char *func_name, uptr *func_addr,
     uptr real, uptr wrapper);
 void *GetFuncAddrVer(const char *func_name, const char *ver);
 }  // namespace __interception
 
 #define INTERCEPT_FUNCTION_LINUX_OR_FREEBSD(func)                          \
   ::__interception::GetRealFunctionAddress(                                \
       #func, (::__interception::uptr *)&__interception::PTR_TO_REAL(func), \
-      (::__interception::uptr) & (func),                                   \
+      (::__interception::uptr) & (__interceptor_fake_ ## func),            \
       (::__interception::uptr) & WRAP(func))
 
 #if !defined(__ANDROID__)  // android does not have dlvsym
 #define INTERCEPT_FUNCTION_VER_LINUX_OR_FREEBSD(func, symver) \
   (::__interception::real_##func = (func##_f)(                \
        unsigned long)::__interception::GetFuncAddrVer(#func, symver))
 #else
 #define INTERCEPT_FUNCTION_VER_LINUX_OR_FREEBSD(func, symver) \
   INTERCEPT_FUNCTION_LINUX_OR_FREEBSD(func)
 #endif  // !defined(__ANDROID__)
 
 #endif  // INTERCEPTION_LINUX_H
 #endif  // __linux__ || __FreeBSD__

lib/msan/msan_interceptors.cc

INTERCEPTOR(int, vswprintf, void *str, uptr size, void *format, va_list ap) {
   }
   return res;
 }
 
 INTERCEPTOR(int, swprintf, void *str, uptr size, void *format, ...) {
   ENSURE_MSAN_INITED();
   va_list ap;
   va_start(ap, format);
-  int res = vswprintf(str, size, format, ap);
+  int res = WRAP(vswprintf)(str, size, format, ap);
   va_end(ap);
   return res;
 }
 
 INTERCEPTOR(SIZE_T, strxfrm, char *dest, const char *src, SIZE_T n) {
   ENSURE_MSAN_INITED();
   CHECK_UNPOISONED(src, REAL(strlen)(src) + 1);
   SIZE_T res = REAL(strxfrm)(dest, src, n);

lib/tsan/rtl/tsan_interceptors.cc

while (v != 2) {
       v = atomic_load(a, memory_order_acquire);
     }
     if (!thr->in_ignored_lib)
       Acquire(thr, pc, (uptr)o);
   }
   return 0;
 }
 
+TSAN_INTERCEPTOR(void, __atomic_load, SIZE_T n, void *a, void *v, int ord) {
+  SCOPED_TSAN_INTERCEPTOR(__atomic_load, n, a, v, ord);
+  AtomicLoad(thr, pc, n, a, v, ord);
+}
+
+TSAN_INTERCEPTOR(void, __atomic_store, SIZE_T n, void *a, void *v, int ord) {
+  SCOPED_TSAN_INTERCEPTOR(__atomic_store, n, a, v, ord);
+  AtomicStore(thr, pc, n, a, v, ord);
+}
+
+TSAN_INTERCEPTOR(void, __atomic_exchange, SIZE_T n, void *a, void *v, void *ret,
+    int ord) {
+  SCOPED_TSAN_INTERCEPTOR(__atomic_exchange, n, a, v, ret, ord);
+  AtomicExchange(thr, pc, n, a, v, ret, ord);
+}
+
+TSAN_INTERCEPTOR(bool, __atomic_compare_exchange, SIZE_T n, void *a, void *cmp,
+    void *v, int sord, int ford) {
+  SCOPED_TSAN_INTERCEPTOR(__atomic_compare_exchange, n, a, cmp, v, sord, ford);
+  return AtomicCompareExchange(thr, pc, n, a, cmp, v, sord, ford);
+}
+
 #if SANITIZER_LINUX && !SANITIZER_ANDROID
 TSAN_INTERCEPTOR(int, __fxstat, int version, int fd, void *buf) {
   SCOPED_TSAN_INTERCEPTOR(__fxstat, version, fd, buf);
   if (fd > 0)
     FdAccess(thr, pc, fd);
   return REAL(__fxstat)(version, fd, buf);
 }
 #define TSAN_MAYBE_INTERCEPT___FXSTAT TSAN_INTERCEPT(__fxstat)

 }
 
 TSAN_INTERCEPTOR(sighandler_t, signal, int sig, sighandler_t h) {
   sigaction_t act;
   act.sa_handler = h;
   internal_memset(&act.sa_mask, -1, sizeof(act.sa_mask));
   act.sa_flags = 0;
   sigaction_t old;
-  int res = sigaction(sig, &act, &old);
+  int res = WRAP(sigaction)(sig, &act, &old);
   if (res)
     return SIG_ERR;
   return old.sa_handler;
 }
 
 TSAN_INTERCEPTOR(int, raise, int sig) {
   SCOPED_TSAN_INTERCEPTOR(raise, sig);
   ThreadSignalContext *sctx = SigCtx(thr);
#endif
   TSAN_INTERCEPT(pthread_rwlock_unlock);
 
   TSAN_INTERCEPT(pthread_barrier_init);
   TSAN_INTERCEPT(pthread_barrier_destroy);
   TSAN_INTERCEPT(pthread_barrier_wait);
 
   TSAN_INTERCEPT(pthread_once);
 
+  TSAN_INTERCEPT(__atomic_load);
+  TSAN_INTERCEPT(__atomic_store);
+  TSAN_INTERCEPT(__atomic_exchange);
+  TSAN_INTERCEPT(__atomic_compare_exchange);
+
   TSAN_INTERCEPT(fstat);
   TSAN_MAYBE_INTERCEPT___FXSTAT;
   TSAN_MAYBE_INTERCEPT_FSTAT64;
   TSAN_MAYBE_INTERCEPT___FXSTAT64;
   TSAN_INTERCEPT(open);
   TSAN_MAYBE_INTERCEPT_OPEN64;
   TSAN_INTERCEPT(creat);
   TSAN_MAYBE_INTERCEPT_CREAT64;

lib/tsan/rtl/tsan_interface_atomic.cc

 }
 
 static void AtomicFence(ThreadState *thr, uptr pc, morder mo) {
   // FIXME(dvyukov): not implemented.
   __sync_synchronize();
 }
 #endif
 
+namespace __tsan {
+void AtomicLoad(ThreadState *thr, uptr pc, uptr n, void *a, void *v, int ord) {
+  const morder mo = static_cast<morder>(ord);
+  CHECK(IsLoadOrder(mo));
+  SyncVar *s = ctx->metamap.GetOrCreateAndLock(thr, pc, (uptr)a, false);
+  if (IsAcquireOrder(mo))
+    AcquireImpl(thr, pc, &s->clock);
+  internal_memcpy(v, a, n);
+  s->mtx.ReadUnlock();
+  MemoryReadAtomic(thr, pc, (uptr)a, kSizeLog1);
+}
+
+void AtomicStore(ThreadState *thr, uptr pc, uptr n, void *a, void *v, int ord) {
+  const morder mo = static_cast<morder>(ord);
+  CHECK(IsStoreOrder(mo));
+  MemoryWriteAtomic(thr, pc, (uptr)a, kSizeLog1);
+  SyncVar *s = ctx->metamap.GetOrCreateAndLock(thr, pc, (uptr)a, true);
+  if (IsReleaseOrder(mo)) {
+    thr->fast_state.IncrementEpoch();
+    // Can't increment epoch w/o writing to the trace as well.
+    TraceAddEvent(thr, thr->fast_state, EventTypeMop, 0);
+    ReleaseImpl(thr, pc, &s->clock);
+  }
+  internal_memcpy(v, a, n);
+  s->mtx.Unlock();
+}
+
+void AtomicExchange(ThreadState *thr, uptr pc, uptr n, void *a, void *v,
+                    void *ret, int ord) {
+  const morder mo = static_cast<morder>(ord);
+  MemoryWriteAtomic(thr, pc, (uptr)a, kSizeLog1);
+  SyncVar *s = ctx->metamap.GetOrCreateAndLock(thr, pc, (uptr)a, true);
+  thr->fast_state.IncrementEpoch();
+  // Can't increment epoch w/o writing to the trace as well.
+  TraceAddEvent(thr, thr->fast_state, EventTypeMop, 0);
+  if (IsAcqRelOrder(mo))
+    AcquireReleaseImpl(thr, pc, &s->clock);
+  else if (IsReleaseOrder(mo))
+    ReleaseImpl(thr, pc, &s->clock);
+  else if (IsAcquireOrder(mo))
+    AcquireImpl(thr, pc, &s->clock);
+  internal_memcpy(ret, a, n);
+  internal_memcpy(a, v, n);
+  s->mtx.Unlock();
+}
+
+bool AtomicCompareExchange(ThreadState *thr, uptr pc, uptr n, void *a, void *c,
+                           void *v, int sord, int ford) {
+  (void)ford;
+  const morder mo = static_cast<morder>(sord);
+  MemoryWriteAtomic(thr, pc, (uptr)a, kSizeLog1);
+  SyncVar *s = ctx->metamap.GetOrCreateAndLock(thr, pc, (uptr)a, true);
+  thr->fast_state.IncrementEpoch();
+  // Can't increment epoch w/o writing to the trace as well.
+  TraceAddEvent(thr, thr->fast_state, EventTypeMop, 0);
+  if (IsAcqRelOrder(mo))
+    AcquireReleaseImpl(thr, pc, &s->clock);
+  else if (IsReleaseOrder(mo))
+    ReleaseImpl(thr, pc, &s->clock);
+  else if (IsAcquireOrder(mo))
+    AcquireImpl(thr, pc, &s->clock);
+  bool res = internal_memcmp(a, c, n) == 0;
+  internal_memcpy((res ? a : c), (res ? v : a), n);
+  s->mtx.Unlock();
+  return res;
+}
+}  // namespace __tsan
+
 // Interface functions follow.
 #ifndef SANITIZER_GO
 
 // C/C++
 
 #define SCOPED_ATOMIC(func, ...) \
     const uptr callpc = (uptr)__builtin_return_address(0); \
     uptr pc = StackTrace::GetCurrentPc(); \

lib/tsan/rtl/tsan_rtl.h

 void Release(ThreadState *thr, uptr pc, uptr addr);
 void ReleaseStore(ThreadState *thr, uptr pc, uptr addr);
 void AfterSleep(ThreadState *thr, uptr pc);
 void AcquireImpl(ThreadState *thr, uptr pc, SyncClock *c);
 void ReleaseImpl(ThreadState *thr, uptr pc, SyncClock *c);
 void ReleaseStoreImpl(ThreadState *thr, uptr pc, SyncClock *c);
 void AcquireReleaseImpl(ThreadState *thr, uptr pc, SyncClock *c);
 
+// Variable-size atomic operations for libatomic interceptors.
+void AtomicLoad(ThreadState *thr, uptr pc, uptr n, void *a, void *v, int ord);
+void AtomicStore(ThreadState *thr, uptr pc, uptr n, void *a, void *v, int ord);
+void AtomicExchange(ThreadState *thr, uptr pc, uptr n, void *a, void *v,
+                    void *ret, int ord);
+bool AtomicCompareExchange(ThreadState *thr, uptr pc, uptr n, void *a, void *c,
+                           void *v, int sord, int ford);
+
 // The hacky call uses custom calling convention and an assembly thunk.
 // It is considerably faster that a normal call for the caller
 // if it is not executed (it is intended for slow paths from hot functions).
 // The trick is that the call preserves all registers and the compiler
 // does not treat it as a call.
 // If it does not work for you, use normal call.
 #if !SANITIZER_DEBUG && defined(__x86_64__) && !SANITIZER_MAC
 // The caller may not create the stack frame for itself at all,

test/tsan/atomic_test.cc

+// RUN: %clangxx_tsan -O1 %s -o %t -DTEST_TYPE=char && %deflake %run %t | FileCheck %s
+// RUN: %clangxx_tsan -O1 %s -o %t -DTEST_TYPE=short && %deflake %run %t | FileCheck %s
+// RUN: %clangxx_tsan -O1 %s -o %t -DTEST_TYPE=int && %deflake %run %t | FileCheck %s
+// RUN: %clangxx_tsan -O1 %s -o %t -DTEST_TYPE=long && %deflake %run %t | FileCheck %s
+// RUN: %clangxx_tsan -O1 %s -o %t -DTEST_TYPE="MyStruct<6>" -latomic && %deflake %run %t | FileCheck %s
+// RUN: %clangxx_tsan -O1 %s -o %t -DTEST_TYPE="MyStruct<8>" -latomic && %deflake %run %t | FileCheck %s
+// RUN: %clangxx_tsan -O1 %s -o %t -DTEST_TYPE="MyStruct<12>" -latomic && %deflake %run %t | FileCheck %s
+// RUN: %clangxx_tsan -O1 %s -o %t -DTEST_TYPE="MyStruct<16>" -latomic && %deflake %run %t | FileCheck %s
+// RUN: %clangxx_tsan -O1 %s -o %t -DTEST_TYPE="MyStruct<24>" -latomic && %deflake %run %t | FileCheck %s
+// RUN: %clangxx_tsan -O1 %s -o %t -DTEST_TYPE="MyStruct<32>" -latomic && %deflake %run %t | FileCheck %s
+// RUN: %clangxx_tsan -O1 %s -o %t -DTEST_TYPE="MyStruct<128>" -latomic && %deflake %run %t | FileCheck %s
+#include "test.h"
+#include <memory.h>
+#include <atomic>
+
+// Test operation and synchronization provided by atomic variables of different
+// sizes, including sizes > 16 which are handled by libatomic.
+// Note that the only allowed atomic operations for these larger types are:
+// load, store, exchange, compare_exchange.
+
+template<int kSize>
+struct MyStruct {
+  char data[kSize];
+
+  explicit MyStruct(char v = 0) noexcept {
+    memset(&data[0], v, sizeof(data));
+  }
+
+  bool operator == (const MyStruct &other) const {
+    return memcmp(&data[0], &other.data[0], sizeof(data)) == 0;
+  }
+
+  bool operator != (const MyStruct &other) const {
+    return !(*this == other);
+  }
+
+  operator int() const {
+    return data[0];
+  }
+};
+
+#ifdef TEST_TYPE
+typedef TEST_TYPE T;
+#else
+typedef long T;
+#endif
+
+struct Data {
+  long pad0;
+  std::atomic<T> a;
+  long pad1;
+  long v;
+};
+
+const int kTestCount = 7;
+Data data[2 * kTestCount];
+
+void Test(int test, Data *p, bool main_thread) {
+  if (test == 0) {
+    // Test that we detect races between atomic and non-atomic accesses.
+    if (main_thread)
+      p->a.store(T(1));
+    else
+      memset(&p->a, 0, 1);
+// CHECK: Test 0 forward
+// CHECK: ThreadSanitizer: data race
+// CHECK: Test 0 reverse
+// CHECK: ThreadSanitizer: data race
+
+  } else if (test == 1) {
+    // Normal acquire-release synchronization.
+    if (p->a.load(std::memory_order_acquire) == T(0)) {
+      p->v = 42;
+      p->a.store(T(17), std::memory_order_release);
+    } else {
+      if (p->v != 42) {
+        fprintf(stderr, "%d: bad value %ld\n", __LINE__, p->v);
+        exit(0);
+      }
+      if (p->a.load(std::memory_order_relaxed) != T(17)) {
+        fprintf(stderr, "%d: bad atomic value %d\n", __LINE__,
+                (int)p->a.load());
+        exit(0);
+      }
+    }
+// CHECK: Test 1 forward
+// CHECK-NOT: ThreadSanitizer: data race
+// CHECK: Test 1 reverse
+// CHECK-NOT: ThreadSanitizer: data race
+
+  } else if (test == 2) {
+    // The same as the previous case, but with memory_order_relaxed for store.
+    // Ensure that atomics don't over-synchronize (that would happen if e.g. we
+    // would intercept underlying pthread_mutex_t operations in libatomic
+    // emulation).
+    if (p->a.load(std::memory_order_acquire) == T(0)) {
+      p->v = 42;
+      p->a.store(T(17), std::memory_order_relaxed);
+    } else {
+      if (p->v != 42) {
+        fprintf(stderr, "%d: bad value %ld\n", __LINE__, p->v);
+        exit(0);
+      }
+      if (p->a.load(std::memory_order_relaxed) != T(17)) {
+        fprintf(stderr, "%d: bad atomic value %d\n", __LINE__,
+                (int)p->a.load());
+        exit(0);
+      }
+    }
+// CHECK: Test 2 forward
+// CHECK: ThreadSanitizer: data race
+// CHECK: Test 2 reverse
+// CHECK: ThreadSanitizer: data race
+
+  } else if (test == 3) {
+    // The same as the previous case, but with memory_order_relaxed for load.
+    if (p->a.load(std::memory_order_relaxed) == T(0)) {
+      p->v = 42;
+      p->a.store(T(17), std::memory_order_release);
+    } else {
+      if (p->v != 42) {
+        fprintf(stderr, "%d: bad value %ld\n", __LINE__, p->v);
+        exit(0);
+      }
+      if (p->a.load(std::memory_order_relaxed) != T(17)) {
+        fprintf(stderr, "%d: bad atomic value %d\n", __LINE__,
+                (int)p->a.load());
+        exit(0);
+      }
+    }
+// CHECK: Test 3 forward
+// CHECK: ThreadSanitizer: data race
+// CHECK: Test 3 reverse
+// CHECK: ThreadSanitizer: data race
+
+  } else if (test == 4) {
+    // Acquire-relase synchronization, but using exchange/compare_exchange.
+    T cmp(17);
+    if (!p->a.compare_exchange_strong(cmp, T(18), std::memory_order_acquire)) {
+      // libc++ has a bug that causes this check to fail:
+      // https://llvm.org/bugs/show_bug.cgi?id=30675
+      if (false && cmp != T(0)) {
+        fprintf(stderr, "%d: bad atomic value %d\n", __LINE__, (int)cmp);
+        exit(0);
+      }
+      p->v = 42;
+      T old = p->a.exchange(T(17), std::memory_order_release);
+      if (old != T(0)) {
+        fprintf(stderr, "%d: bad atomic value %d\n", __LINE__, (int)old);
+        exit(0);
+      }
+    } else {
+      if (p->v != 42) {
+        fprintf(stderr, "%d: bad value %ld\n", __LINE__, p->v);
+        exit(0);
+      }
+      if (cmp != T(17)) {
+        fprintf(stderr, "%d: bad atomic value %d\n", __LINE__, (int)cmp);
+        exit(0);
+      }
+      if (p->a.load(std::memory_order_relaxed) != T(18)) {
+        fprintf(stderr, "%d: bad atomic value %d\n", __LINE__,
+                (int)p->a.load());
+        exit(0);
+      }
+    }
+// CHECK: Test 4 forward
+// CHECK-NOT: ThreadSanitizer: data race
+// CHECK: Test 4 reverse
+// CHECK-NOT: ThreadSanitizer: data race
+
+  } else if (test == 5) {
+    // The same as the previous case, but with memory_order_relaxed for
+    // exchange.
+    T cmp(17);
+    if (!p->a.compare_exchange_strong(cmp, T(18), std::memory_order_acquire)) {
+      p->v = 42;
+      p->a.exchange(T(17), std::memory_order_relaxed);
+    } else {
+      if (p->v != 42) {
+        fprintf(stderr, "%d: bad value %ld\n", __LINE__, p->v);
+        exit(0);
+      }
+    }
+// CHECK: Test 5 forward
+// CHECK: ThreadSanitizer: data race
+// CHECK: Test 5 reverse
+// CHECK: ThreadSanitizer: data race
+
+  } else if (test == 6) {
+    // The same as the previous case, but with memory_order_relaxed for
+    // compare_exchange.
+    T cmp(17);
+    if (!p->a.compare_exchange_strong(cmp, T(18), std::memory_order_relaxed)) {
+      p->v = 42;
+      p->a.exchange(T(17), std::memory_order_release);
+    } else {
+      if (p->v != 42) {
+        fprintf(stderr, "%d: bad value %ld\n", __LINE__, p->v);
+        exit(0);
+      }
+    }
+// CHECK: Test 6 forward
+// CHECK: ThreadSanitizer: data race
+// CHECK: Test 6 reverse
+// CHECK: ThreadSanitizer: data race
+  }
+}
+
+void *Thread(void *p) {
+  for (int i = 0; i < kTestCount; i++) {
+    Test(i, &data[i * 2], false);
+    barrier_wait(&barrier);
+    barrier_wait(&barrier);
+    fprintf(stderr, "Test %d reverse\n", i);
+    Test(i, &data[i * 2 + 1], false);
+  }
+  return 0;
+}
+
+int main() {
+  fprintf(stderr, "data size: %zu\n", sizeof(T));
+  for (int i = 0; i < 2 * kTestCount; i++) {
+    data[i].pad0 = 100;
+    data[i].pad1 = 200;
+  }
+  barrier_init(&barrier, 2);
+  pthread_t t;
+  pthread_create(&t, 0, Thread, 0);
+  for (int i = 0; i < kTestCount; i++) {
+    barrier_wait(&barrier);
+    fprintf(stderr, "Test %d forward\n", i);
+    Test(i, &data[2 * i], true);
+    Test(i, &data[2 * i + 1], true);
+    barrier_wait(&barrier);
+  }
+  pthread_join(t, 0);
+  for (int i = 0; i < 2 * kTestCount; i++) {
+    if (data[i].pad0 != 100 || data[i].pad1 != 200) {
+      fprintf(stderr, "padding overwritten %d: %lu/%lu\n",
+              i, data[i].pad0, data[i].pad1);
+// CHECK-NOT: padding overwritten
+    }
+  }
+}