[compiler-rt] r291631 - [tsan] Implement a 'ignore_noninstrumented_modules' flag to better suppress false positive races
Kuba Mracek via llvm-commits
llvm-commits at lists.llvm.org
Tue Jan 10 16:54:27 PST 2017
Author: kuba.brecka
Date: Tue Jan 10 18:54:26 2017
New Revision: 291631
URL: http://llvm.org/viewvc/llvm-project?rev=291631&view=rev
Log:
[tsan] Implement a 'ignore_noninstrumented_modules' flag to better suppress false positive races
On Darwin, we currently use 'ignore_interceptors_accesses', which is a heavy-weight solution that simply turns of race detection in all interceptors. This was done to suppress false positives coming from system libraries (non-instrumented code), but it also silences a lot of real races. This patch implements an alternative approach that should allow us to enable interceptors and report races coming from them, but only if they are called directly from instrumented code.
The patch matches the caller PC in each interceptors. For non-instrumented code, we call ThreadIgnoreBegin.
The assumption here is that the number of instrumented modules is low. Most likely there's only one (the instrumented main executable) and all the other modules are system libraries (non-instrumented).
Differential Revision: https://reviews.llvm.org/D28264
Added:
compiler-rt/trunk/test/tsan/Darwin/ignore-noninstrumented.mm
Modified:
compiler-rt/trunk/lib/sanitizer_common/sanitizer_libignore.cc
compiler-rt/trunk/lib/sanitizer_common/sanitizer_libignore.h
compiler-rt/trunk/lib/tsan/rtl/tsan_flags.inc
compiler-rt/trunk/lib/tsan/rtl/tsan_interceptors.cc
compiler-rt/trunk/lib/tsan/rtl/tsan_interceptors.h
Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_libignore.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_libignore.cc?rev=291631&r1=291630&r2=291631&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_libignore.cc (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_libignore.cc Tue Jan 10 18:54:26 2017
@@ -78,10 +78,12 @@ void LibIgnore::OnLibraryLoaded(const ch
lib->templ, mod.full_name());
lib->loaded = true;
lib->name = internal_strdup(mod.full_name());
- const uptr idx = atomic_load(&loaded_count_, memory_order_relaxed);
- code_ranges_[idx].begin = range.beg;
- code_ranges_[idx].end = range.end;
- atomic_store(&loaded_count_, idx + 1, memory_order_release);
+ const uptr idx =
+ atomic_load(&ignored_ranges_count_, memory_order_relaxed);
+ CHECK_LT(idx, kMaxLibs);
+ ignored_code_ranges_[idx].begin = range.beg;
+ ignored_code_ranges_[idx].end = range.end;
+ atomic_store(&ignored_ranges_count_, idx + 1, memory_order_release);
break;
}
}
@@ -92,6 +94,29 @@ void LibIgnore::OnLibraryLoaded(const ch
Die();
}
}
+
+ // Track instrumented ranges.
+ if (track_instrumented_libs_) {
+ for (const auto &mod : modules) {
+ if (!mod.instrumented())
+ continue;
+ for (const auto &range : mod.ranges()) {
+ if (!range.executable)
+ continue;
+ if (IsPcInstrumented(range.beg) && IsPcInstrumented(range.end - 1))
+ continue;
+ VReport(1, "Adding instrumented range %p-%p from library '%s'\n",
+ range.beg, range.end, mod.full_name());
+ const uptr idx =
+ atomic_load(&instrumented_ranges_count_, memory_order_relaxed);
+ CHECK_LT(idx, kMaxLibs);
+ instrumented_code_ranges_[idx].begin = range.beg;
+ instrumented_code_ranges_[idx].end = range.end;
+ atomic_store(&instrumented_ranges_count_, idx + 1,
+ memory_order_release);
+ }
+ }
+ }
}
void LibIgnore::OnLibraryUnloaded() {
Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_libignore.h
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_libignore.h?rev=291631&r1=291630&r2=291631&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_libignore.h (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_libignore.h Tue Jan 10 18:54:26 2017
@@ -30,6 +30,9 @@ class LibIgnore {
// Must be called during initialization.
void AddIgnoredLibrary(const char *name_templ);
+ void IgnoreNoninstrumentedModules(bool enable) {
+ track_instrumented_libs_ = enable;
+ }
// Must be called after a new dynamic library is loaded.
void OnLibraryLoaded(const char *name);
@@ -37,8 +40,14 @@ class LibIgnore {
// Must be called after a dynamic library is unloaded.
void OnLibraryUnloaded();
- // Checks whether the provided PC belongs to one of the ignored libraries.
- bool IsIgnored(uptr pc) const;
+ // Checks whether the provided PC belongs to one of the ignored libraries or
+ // the PC should be ignored because it belongs to an non-instrumented module
+ // (when ignore_noninstrumented_modules=1). Also returns true via
+ // "pc_in_ignored_lib" if the PC is in an ignored library, false otherwise.
+ bool IsIgnored(uptr pc, bool *pc_in_ignored_lib) const;
+
+ // Checks whether the provided PC belongs to an instrumented module.
+ bool IsPcInstrumented(uptr pc) const;
private:
struct Lib {
@@ -53,26 +62,48 @@ class LibIgnore {
uptr end;
};
+ inline bool IsInRange(uptr pc, const LibCodeRange &range) const {
+ return (pc >= range.begin && pc < range.end);
+ }
+
static const uptr kMaxLibs = 128;
// Hot part:
- atomic_uintptr_t loaded_count_;
- LibCodeRange code_ranges_[kMaxLibs];
+ atomic_uintptr_t ignored_ranges_count_;
+ LibCodeRange ignored_code_ranges_[kMaxLibs];
+
+ atomic_uintptr_t instrumented_ranges_count_;
+ LibCodeRange instrumented_code_ranges_[kMaxLibs];
// Cold part:
BlockingMutex mutex_;
uptr count_;
Lib libs_[kMaxLibs];
+ bool track_instrumented_libs_;
// Disallow copying of LibIgnore objects.
LibIgnore(const LibIgnore&); // not implemented
void operator = (const LibIgnore&); // not implemented
};
-inline bool LibIgnore::IsIgnored(uptr pc) const {
- const uptr n = atomic_load(&loaded_count_, memory_order_acquire);
+inline bool LibIgnore::IsIgnored(uptr pc, bool *pc_in_ignored_lib) const {
+ const uptr n = atomic_load(&ignored_ranges_count_, memory_order_acquire);
+ for (uptr i = 0; i < n; i++) {
+ if (IsInRange(pc, ignored_code_ranges_[i])) {
+ *pc_in_ignored_lib = true;
+ return true;
+ }
+ }
+ *pc_in_ignored_lib = false;
+ if (track_instrumented_libs_ && !IsPcInstrumented(pc))
+ return true;
+ return false;
+}
+
+inline bool LibIgnore::IsPcInstrumented(uptr pc) const {
+ const uptr n = atomic_load(&instrumented_ranges_count_, memory_order_acquire);
for (uptr i = 0; i < n; i++) {
- if (pc >= code_ranges_[i].begin && pc < code_ranges_[i].end)
+ if (IsInRange(pc, instrumented_code_ranges_[i]))
return true;
}
return false;
Modified: compiler-rt/trunk/lib/tsan/rtl/tsan_flags.inc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/rtl/tsan_flags.inc?rev=291631&r1=291630&r2=291631&view=diff
==============================================================================
--- compiler-rt/trunk/lib/tsan/rtl/tsan_flags.inc (original)
+++ compiler-rt/trunk/lib/tsan/rtl/tsan_flags.inc Tue Jan 10 18:54:26 2017
@@ -79,5 +79,8 @@ TSAN_FLAG(bool, die_after_fork, true,
TSAN_FLAG(const char *, suppressions, "", "Suppressions file name.")
TSAN_FLAG(bool, ignore_interceptors_accesses, false,
"Ignore reads and writes from all interceptors.")
+TSAN_FLAG(bool, ignore_noninstrumented_modules, false,
+ "Interceptors should only detect races when called from instrumented "
+ "modules.")
TSAN_FLAG(bool, shared_ptr_interceptor, true,
"Track atomic reference counting in libc++ shared_ptr and weak_ptr.")
Modified: compiler-rt/trunk/lib/tsan/rtl/tsan_interceptors.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/rtl/tsan_interceptors.cc?rev=291631&r1=291630&r2=291631&view=diff
==============================================================================
--- compiler-rt/trunk/lib/tsan/rtl/tsan_interceptors.cc (original)
+++ compiler-rt/trunk/lib/tsan/rtl/tsan_interceptors.cc Tue Jan 10 18:54:26 2017
@@ -231,6 +231,8 @@ void InitializeLibIgnore() {
if (0 == internal_strcmp(s->type, kSuppressionLib))
libignore()->AddIgnoredLibrary(s->templ);
}
+ if (flags()->ignore_noninstrumented_modules)
+ libignore()->IgnoreNoninstrumentedModules(true);
libignore()->OnLibraryLoaded(0);
}
@@ -252,31 +254,20 @@ static unsigned g_thread_finalize_key;
ScopedInterceptor::ScopedInterceptor(ThreadState *thr, const char *fname,
uptr pc)
- : thr_(thr)
- , pc_(pc)
- , in_ignored_lib_(false) {
+ : thr_(thr), pc_(pc), in_ignored_lib_(false), ignoring_(false) {
Initialize(thr);
- if (!thr_->is_inited)
- return;
- if (!thr_->ignore_interceptors)
- FuncEntry(thr, pc);
+ if (!thr_->is_inited) return;
+ if (!thr_->ignore_interceptors) FuncEntry(thr, pc);
DPrintf("#%d: intercept %s()\n", thr_->tid, fname);
- if (!thr_->in_ignored_lib && libignore()->IsIgnored(pc)) {
- in_ignored_lib_ = true;
- thr_->in_ignored_lib = true;
- ThreadIgnoreBegin(thr_, pc_);
- }
- if (flags()->ignore_interceptors_accesses) ThreadIgnoreBegin(thr_, pc_);
+ ignoring_ =
+ !thr_->in_ignored_lib && (flags()->ignore_interceptors_accesses ||
+ libignore()->IsIgnored(pc, &in_ignored_lib_));
+ EnableIgnores();
}
ScopedInterceptor::~ScopedInterceptor() {
- if (!thr_->is_inited)
- return;
- if (flags()->ignore_interceptors_accesses) ThreadIgnoreEnd(thr_, pc_);
- if (in_ignored_lib_) {
- thr_->in_ignored_lib = false;
- ThreadIgnoreEnd(thr_, pc_);
- }
+ if (!thr_->is_inited) return;
+ DisableIgnores();
if (!thr_->ignore_interceptors) {
ProcessPendingSignals(thr_);
FuncExit(thr_);
@@ -284,20 +275,24 @@ ScopedInterceptor::~ScopedInterceptor()
}
}
-void ScopedInterceptor::UserCallbackStart() {
- if (flags()->ignore_interceptors_accesses) ThreadIgnoreEnd(thr_, pc_);
- if (in_ignored_lib_) {
- thr_->in_ignored_lib = false;
- ThreadIgnoreEnd(thr_, pc_);
+void ScopedInterceptor::EnableIgnores() {
+ if (ignoring_) {
+ ThreadIgnoreBegin(thr_, pc_);
+ if (in_ignored_lib_) {
+ DCHECK(!thr_->in_ignored_lib);
+ thr_->in_ignored_lib = true;
+ }
}
}
-void ScopedInterceptor::UserCallbackEnd() {
- if (in_ignored_lib_) {
- thr_->in_ignored_lib = true;
- ThreadIgnoreBegin(thr_, pc_);
+void ScopedInterceptor::DisableIgnores() {
+ if (ignoring_) {
+ ThreadIgnoreEnd(thr_, pc_);
+ if (in_ignored_lib_) {
+ DCHECK(thr_->in_ignored_lib);
+ thr_->in_ignored_lib = false;
+ }
}
- if (flags()->ignore_interceptors_accesses) ThreadIgnoreBegin(thr_, pc_);
}
#define TSAN_INTERCEPT(func) INTERCEPT_FUNCTION(func)
Modified: compiler-rt/trunk/lib/tsan/rtl/tsan_interceptors.h
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/rtl/tsan_interceptors.h?rev=291631&r1=291630&r2=291631&view=diff
==============================================================================
--- compiler-rt/trunk/lib/tsan/rtl/tsan_interceptors.h (original)
+++ compiler-rt/trunk/lib/tsan/rtl/tsan_interceptors.h Tue Jan 10 18:54:26 2017
@@ -10,12 +10,13 @@ class ScopedInterceptor {
public:
ScopedInterceptor(ThreadState *thr, const char *fname, uptr pc);
~ScopedInterceptor();
- void UserCallbackStart();
- void UserCallbackEnd();
+ void DisableIgnores();
+ void EnableIgnores();
private:
ThreadState *const thr_;
const uptr pc_;
bool in_ignored_lib_;
+ bool ignoring_;
};
} // namespace __tsan
@@ -39,10 +40,10 @@ class ScopedInterceptor {
/**/
#define SCOPED_TSAN_INTERCEPTOR_USER_CALLBACK_START() \
- si.UserCallbackStart();
+ si.DisableIgnores();
#define SCOPED_TSAN_INTERCEPTOR_USER_CALLBACK_END() \
- si.UserCallbackEnd();
+ si.EnableIgnores();
#define TSAN_INTERCEPTOR(ret, func, ...) INTERCEPTOR(ret, func, __VA_ARGS__)
Added: compiler-rt/trunk/test/tsan/Darwin/ignore-noninstrumented.mm
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/tsan/Darwin/ignore-noninstrumented.mm?rev=291631&view=auto
==============================================================================
--- compiler-rt/trunk/test/tsan/Darwin/ignore-noninstrumented.mm (added)
+++ compiler-rt/trunk/test/tsan/Darwin/ignore-noninstrumented.mm Tue Jan 10 18:54:26 2017
@@ -0,0 +1,53 @@
+// Check that ignore_noninstrumented_modules=1 supresses races from system libraries on OS X.
+
+// RUN: %clang_tsan %s -o %t -framework Foundation
+
+// Check that without the flag, there are false positives.
+// RUN: %deflake %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-RACE
+
+// With ignore_noninstrumented_modules=1, no races are reported.
+// RUN: %env_tsan_opts=ignore_noninstrumented_modules=1 %run %t 2>&1 | FileCheck %s
+
+// With ignore_noninstrumented_modules=1, races in user's code are still reported.
+// RUN: %env_tsan_opts=ignore_noninstrumented_modules=1 %deflake %run %t race 2>&1 | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-RACE
+
+#import <Foundation/Foundation.h>
+
+#import "../test.h"
+
+char global_buf[64];
+
+void *Thread1(void *x) {
+ barrier_wait(&barrier);
+ strcpy(global_buf, "hello world");
+ return NULL;
+}
+
+void *Thread2(void *x) {
+ strcpy(global_buf, "world hello");
+ barrier_wait(&barrier);
+ return NULL;
+}
+
+int main(int argc, char *argv[]) {
+ fprintf(stderr, "Hello world.\n");
+
+ // NSUserDefaults uses XPC which triggers the false positive.
+ NSDictionary *d = [[NSUserDefaults standardUserDefaults] dictionaryRepresentation];
+ fprintf(stderr, "d = %p\n", d);
+
+ if (argc > 1 && strcmp(argv[1], "race") == 0) {
+ barrier_init(&barrier, 2);
+ pthread_t t[2];
+ pthread_create(&t[0], NULL, Thread1, NULL);
+ pthread_create(&t[1], NULL, Thread2, NULL);
+ pthread_join(t[0], NULL);
+ pthread_join(t[1], NULL);
+ }
+
+ fprintf(stderr, "Done.\n");
+}
+
+// CHECK: Hello world.
+// CHECK-RACE: SUMMARY: ThreadSanitizer: data race
+// CHECK: Done.
More information about the llvm-commits
mailing list