From 2e4e7d04d262ba206b1bfa3893edf8fc8441d633 Mon Sep 17 00:00:00 2001 From: Kuba Mracek Date: Fri, 21 Apr 2017 17:18:14 +0000 Subject: [PATCH] [tsan] Ignore memory accesses for libignored modules for "external" races On Darwin, the setting ignore_noninstrumented_modules is used to suppress false positives in code that users don't have control of. The recently added "external" API (which can be used to detect races on objects provided by system libraries, but the race is actually user's fault) ignores this flag and it can report issues in non-instrumented modules. This patch fixes that. Differential Revision: https://reviews.llvm.org/D31553 llvm-svn: 301000 --- compiler-rt/lib/tsan/rtl/tsan_external.cc | 19 ++++-- compiler-rt/lib/tsan/rtl/tsan_interceptors.cc | 2 +- compiler-rt/lib/tsan/rtl/tsan_interceptors.h | 2 + .../Darwin/external-ignore-noninstrumented.cc | 19 ++++++ compiler-rt/test/tsan/Darwin/external-lib.cc | 68 +++++++++++++++++++ .../Darwin/external-noninstrumented-module.cc | 27 ++++++++ compiler-rt/test/tsan/Darwin/external.cc | 66 +----------------- 7 files changed, 133 insertions(+), 70 deletions(-) create mode 100644 compiler-rt/test/tsan/Darwin/external-ignore-noninstrumented.cc create mode 100644 compiler-rt/test/tsan/Darwin/external-lib.cc create mode 100644 compiler-rt/test/tsan/Darwin/external-noninstrumented-module.cc diff --git a/compiler-rt/lib/tsan/rtl/tsan_external.cc b/compiler-rt/lib/tsan/rtl/tsan_external.cc index dc8ec62322ce..971d1a01b688 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_external.cc +++ b/compiler-rt/lib/tsan/rtl/tsan_external.cc @@ -11,6 +11,7 @@ // //===----------------------------------------------------------------------===// #include "tsan_rtl.h" +#include "tsan_interceptors.h" namespace __tsan { @@ -57,9 +58,12 @@ void __tsan_external_read(void *addr, void *caller_pc, void *tag) { CHECK_LT(tag, atomic_load(&used_tags, memory_order_relaxed)); ThreadState *thr = cur_thread(); thr->external_tag = (uptr)tag; - FuncEntry(thr, (uptr)caller_pc); - MemoryRead(thr, CALLERPC, (uptr)addr, kSizeLog8); - FuncExit(thr); + if (caller_pc) FuncEntry(thr, (uptr)caller_pc); + bool in_ignored_lib; + if (!caller_pc || !libignore()->IsIgnored((uptr)caller_pc, &in_ignored_lib)) { + MemoryRead(thr, CALLERPC, (uptr)addr, kSizeLog8); + } + if (caller_pc) FuncExit(thr); thr->external_tag = 0; } @@ -68,9 +72,12 @@ void __tsan_external_write(void *addr, void *caller_pc, void *tag) { CHECK_LT(tag, atomic_load(&used_tags, memory_order_relaxed)); ThreadState *thr = cur_thread(); thr->external_tag = (uptr)tag; - FuncEntry(thr, (uptr)caller_pc); - MemoryWrite(thr, CALLERPC, (uptr)addr, kSizeLog8); - FuncExit(thr); + if (caller_pc) FuncEntry(thr, (uptr)caller_pc); + bool in_ignored_lib; + if (!caller_pc || !libignore()->IsIgnored((uptr)caller_pc, &in_ignored_lib)) { + MemoryWrite(thr, CALLERPC, (uptr)addr, kSizeLog8); + } + if (caller_pc) FuncExit(thr); thr->external_tag = 0; } } // extern "C" diff --git a/compiler-rt/lib/tsan/rtl/tsan_interceptors.cc b/compiler-rt/lib/tsan/rtl/tsan_interceptors.cc index 5ad7a5909389..334cc326daf6 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_interceptors.cc +++ b/compiler-rt/lib/tsan/rtl/tsan_interceptors.cc @@ -210,7 +210,7 @@ struct ThreadSignalContext { // The object is 64-byte aligned, because we want hot data to be located in // a single cache line if possible (it's accessed in every interceptor). static ALIGNED(64) char libignore_placeholder[sizeof(LibIgnore)]; -static LibIgnore *libignore() { +LibIgnore *libignore() { return reinterpret_cast(&libignore_placeholder[0]); } diff --git a/compiler-rt/lib/tsan/rtl/tsan_interceptors.h b/compiler-rt/lib/tsan/rtl/tsan_interceptors.h index 72534f4a24a6..de47466501da 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_interceptors.h +++ b/compiler-rt/lib/tsan/rtl/tsan_interceptors.h @@ -19,6 +19,8 @@ class ScopedInterceptor { bool ignoring_; }; +LibIgnore *libignore(); + } // namespace __tsan #define SCOPED_INTERCEPTOR_RAW(func, ...) \ diff --git a/compiler-rt/test/tsan/Darwin/external-ignore-noninstrumented.cc b/compiler-rt/test/tsan/Darwin/external-ignore-noninstrumented.cc new file mode 100644 index 000000000000..d2acaf54b263 --- /dev/null +++ b/compiler-rt/test/tsan/Darwin/external-ignore-noninstrumented.cc @@ -0,0 +1,19 @@ +// RUN: %clangxx_tsan -shared %p/external-lib.cc -fno-sanitize=thread -DUSE_TSAN_CALLBACKS \ +// RUN: -o %t-lib.dylib -install_name @rpath/`basename %t-lib.dylib` + +// RUN: %clangxx_tsan -shared %p/external-noninstrumented-module.cc %t-lib.dylib -fno-sanitize=thread \ +// RUN: -o %t-module.dylib -install_name @rpath/`basename %t-module.dylib` + +// RUN: %clangxx_tsan %s %t-module.dylib -o %t +// RUN: %run %t 2>&1 | FileCheck %s + +#include + +extern "C" void NonInstrumentedModule(); +int main(int argc, char *argv[]) { + NonInstrumentedModule(); + fprintf(stderr, "Done.\n"); +} + +// CHECK-NOT: WARNING: ThreadSanitizer +// CHECK: Done. diff --git a/compiler-rt/test/tsan/Darwin/external-lib.cc b/compiler-rt/test/tsan/Darwin/external-lib.cc new file mode 100644 index 000000000000..f0afdf1dc060 --- /dev/null +++ b/compiler-rt/test/tsan/Darwin/external-lib.cc @@ -0,0 +1,68 @@ +// This file is used from other tests. +// RUN: true + +#include +#include +#include + +struct MyObject; +typedef MyObject *MyObjectRef; +extern "C" { + void InitializeLibrary(); + MyObject *ObjectCreate(); + long ObjectRead(MyObject *); + void ObjectWrite(MyObject *, long); + void ObjectWriteAnother(MyObject *, long); +} + +struct MyObject { + long _val; + long _another; +}; + +#if defined(USE_TSAN_CALLBACKS) +static void *tag; +void *(*callback_register_tag)(const char *object_type); +void *(*callback_assign_tag)(void *addr, void *tag); +void (*callback_read)(void *addr, void *caller_pc, void *tag); +void (*callback_write)(void *addr, void *caller_pc, void *tag); +#endif + +void InitializeLibrary() { +#if defined(USE_TSAN_CALLBACKS) + callback_register_tag = (decltype(callback_register_tag))dlsym(RTLD_DEFAULT, "__tsan_external_register_tag"); + callback_assign_tag = (decltype(callback_assign_tag))dlsym(RTLD_DEFAULT, "__tsan_external_assign_tag"); + callback_read = (decltype(callback_read))dlsym(RTLD_DEFAULT, "__tsan_external_read"); + callback_write = (decltype(callback_write))dlsym(RTLD_DEFAULT, "__tsan_external_write"); + tag = callback_register_tag("MyLibrary::MyObject"); +#endif +} + +MyObject *ObjectCreate() { + MyObject *ref = (MyObject *)malloc(sizeof(MyObject)); +#if defined(USE_TSAN_CALLBACKS) + callback_assign_tag(ref, tag); +#endif + return ref; +} + +long ObjectRead(MyObject *ref) { +#if defined(USE_TSAN_CALLBACKS) + callback_read(ref, __builtin_return_address(0), tag); +#endif + return ref->_val; +} + +void ObjectWrite(MyObject *ref, long val) { +#if defined(USE_TSAN_CALLBACKS) + callback_write(ref, __builtin_return_address(0), tag); +#endif + ref->_val = val; +} + +void ObjectWriteAnother(MyObject *ref, long val) { +#if defined(USE_TSAN_CALLBACKS) + callback_write(ref, __builtin_return_address(0), tag); +#endif + ref->_another = val; +} diff --git a/compiler-rt/test/tsan/Darwin/external-noninstrumented-module.cc b/compiler-rt/test/tsan/Darwin/external-noninstrumented-module.cc new file mode 100644 index 000000000000..ce65970834e5 --- /dev/null +++ b/compiler-rt/test/tsan/Darwin/external-noninstrumented-module.cc @@ -0,0 +1,27 @@ +// This file is used from other tests. +// RUN: true + +#include + +#include +#include + +struct MyObject; +typedef MyObject *MyObjectRef; +extern "C" { + void InitializeLibrary(); + MyObject *ObjectCreate(); + long ObjectRead(MyObject *); + void ObjectWrite(MyObject *, long); + void ObjectWriteAnother(MyObject *, long); +} + +extern "C" void NonInstrumentedModule() { + InitializeLibrary(); + + MyObjectRef ref = ObjectCreate(); + std::thread t1([ref]{ ObjectWrite(ref, 42); }); + std::thread t2([ref]{ ObjectWrite(ref, 43); }); + t1.join(); + t2.join(); +} diff --git a/compiler-rt/test/tsan/Darwin/external.cc b/compiler-rt/test/tsan/Darwin/external.cc index 2605480d7b82..66881d3e5beb 100644 --- a/compiler-rt/test/tsan/Darwin/external.cc +++ b/compiler-rt/test/tsan/Darwin/external.cc @@ -1,12 +1,12 @@ -// RUN: %clangxx_tsan %s -shared -DSHARED_LIB \ +// RUN: %clangxx_tsan %p/external-lib.cc -shared \ // RUN: -o %t-lib-instrumented.dylib \ // RUN: -install_name @rpath/`basename %t-lib-instrumented.dylib` -// RUN: %clangxx_tsan %s -shared -DSHARED_LIB -fno-sanitize=thread \ +// RUN: %clangxx_tsan %p/external-lib.cc -shared -fno-sanitize=thread \ // RUN: -o %t-lib-noninstrumented.dylib \ // RUN: -install_name @rpath/`basename %t-lib-noninstrumented.dylib` -// RUN: %clangxx_tsan %s -shared -DSHARED_LIB -fno-sanitize=thread -DUSE_TSAN_CALLBACKS \ +// RUN: %clangxx_tsan %p/external-lib.cc -shared -fno-sanitize=thread -DUSE_TSAN_CALLBACKS \ // RUN: -o %t-lib-noninstrumented-callbacks.dylib \ // RUN: -install_name @rpath/`basename %t-lib-noninstrumented-callbacks.dylib` @@ -23,8 +23,6 @@ #include -#include -#include #include #include @@ -38,62 +36,6 @@ extern "C" { void ObjectWriteAnother(MyObject *, long); } -#if defined(SHARED_LIB) - -struct MyObject { - long _val; - long _another; -}; - -#if defined(USE_TSAN_CALLBACKS) -static void *tag; -void *(*callback_register_tag)(const char *object_type); -void *(*callback_assign_tag)(void *addr, void *tag); -void (*callback_read)(void *addr, void *caller_pc, void *tag); -void (*callback_write)(void *addr, void *caller_pc, void *tag); -#endif - -void InitializeLibrary() { -#if defined(USE_TSAN_CALLBACKS) - callback_register_tag = (decltype(callback_register_tag))dlsym(RTLD_DEFAULT, "__tsan_external_register_tag"); - callback_assign_tag = (decltype(callback_assign_tag))dlsym(RTLD_DEFAULT, "__tsan_external_assign_tag"); - callback_read = (decltype(callback_read))dlsym(RTLD_DEFAULT, "__tsan_external_read"); - callback_write = (decltype(callback_write))dlsym(RTLD_DEFAULT, "__tsan_external_write"); - tag = callback_register_tag("MyLibrary::MyObject"); -#endif -} - -MyObject *ObjectCreate() { - MyObject *ref = (MyObject *)malloc(sizeof(MyObject)); -#if defined(USE_TSAN_CALLBACKS) - callback_assign_tag(ref, tag); -#endif - return ref; -} - -long ObjectRead(MyObject *ref) { -#if defined(USE_TSAN_CALLBACKS) - callback_read(ref, __builtin_return_address(0), tag); -#endif - return ref->_val; -} - -void ObjectWrite(MyObject *ref, long val) { -#if defined(USE_TSAN_CALLBACKS) - callback_write(ref, __builtin_return_address(0), tag); -#endif - ref->_val = val; -} - -void ObjectWriteAnother(MyObject *ref, long val) { -#if defined(USE_TSAN_CALLBACKS) - callback_write(ref, __builtin_return_address(0), tag); -#endif - ref->_another = val; -} - -#else // defined(SHARED_LIB) - int main(int argc, char *argv[]) { InitializeLibrary(); @@ -159,5 +101,3 @@ int main(int argc, char *argv[]) { fprintf(stderr, "WW test done\n"); // CHECK: WW test done } - -#endif // defined(SHARED_LIB)