[compiler-rt] c3c324d - tsan: lock ScopedErrorReportLock around fork
Dmitry Vyukov via llvm-commits
llvm-commits at lists.llvm.org
Thu Jul 15 12:00:17 PDT 2021
Author: Dmitry Vyukov
Date: 2021-07-15T21:00:11+02:00
New Revision: c3c324dddf73bfc85034267901fce22002a4bb78
URL: https://github.com/llvm/llvm-project/commit/c3c324dddf73bfc85034267901fce22002a4bb78
DIFF: https://github.com/llvm/llvm-project/commit/c3c324dddf73bfc85034267901fce22002a4bb78.diff
LOG: tsan: lock ScopedErrorReportLock around fork
Currently we don't lock ScopedErrorReportLock around fork
and it mostly works becuase tsan has own report_mtx that
is locked around fork and tsan reports.
However, sanitizer_common code prints some own reports
which are not protected by tsan's report_mtx. So it's better
to lock ScopedErrorReportLock explicitly.
Reviewed By: melver
Differential Revision: https://reviews.llvm.org/D106048
Added:
Modified:
compiler-rt/lib/sanitizer_common/sanitizer_common.h
compiler-rt/lib/sanitizer_common/sanitizer_mutex.h
compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_report.cpp
compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.h
compiler-rt/lib/sanitizer_common/sanitizer_thread_safety.h
compiler-rt/lib/tsan/rtl/tsan_rtl.cpp
Removed:
################################################################################
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_common.h b/compiler-rt/lib/sanitizer_common/sanitizer_common.h
index c3ebe27492ce..cbdbb0c4c4bd 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_common.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_common.h
@@ -237,10 +237,16 @@ void SetPrintfAndReportCallback(void (*callback)(const char *));
// Lock sanitizer error reporting and protects against nested errors.
class ScopedErrorReportLock {
public:
- ScopedErrorReportLock();
- ~ScopedErrorReportLock();
+ ScopedErrorReportLock() ACQUIRE(mutex_) { Lock(); }
+ ~ScopedErrorReportLock() RELEASE(mutex_) { Unlock(); }
- static void CheckLocked();
+ static void Lock() ACQUIRE(mutex_);
+ static void Unlock() RELEASE(mutex_);
+ static void CheckLocked() CHECK_LOCKED(mutex_);
+
+ private:
+ static atomic_uintptr_t reporting_thread_;
+ static StaticSpinMutex mutex_;
};
extern uptr stoptheworld_tracer_pid;
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_mutex.h b/compiler-rt/lib/sanitizer_common/sanitizer_mutex.h
index 122a7a8d5a3d..5bae0ed0ab65 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_mutex.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_mutex.h
@@ -38,7 +38,7 @@ class MUTEX StaticSpinMutex {
void Unlock() RELEASE() { atomic_store(&state_, 0, memory_order_release); }
- void CheckLocked() const CHECK_LOCKED {
+ void CheckLocked() const CHECK_LOCKED() {
CHECK_EQ(atomic_load(&state_, memory_order_relaxed), 1);
}
@@ -84,7 +84,7 @@ class MUTEX BlockingMutex {
// maintaining complex state to work around those situations, the check only
// checks that the mutex is owned, and assumes callers to be generally
// well-behaved.
- void CheckLocked() const CHECK_LOCKED;
+ void CheckLocked() const CHECK_LOCKED();
private:
// Solaris mutex_t has a member that requires 64-bit alignment.
@@ -131,7 +131,7 @@ class MUTEX RWMutex {
(void)prev;
}
- void CheckLocked() const CHECK_LOCKED {
+ void CheckLocked() const CHECK_LOCKED() {
CHECK_NE(atomic_load(&state_, memory_order_relaxed), kUnlocked);
}
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_report.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_report.cpp
index 1f65a6aef710..f330ed36640a 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_report.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_report.cpp
@@ -250,17 +250,17 @@ void HandleDeadlySignal(void *siginfo, void *context, u32 tid,
#endif // !SANITIZER_FUCHSIA && !SANITIZER_GO
-static atomic_uintptr_t reporting_thread = {0};
-static StaticSpinMutex CommonSanitizerReportMutex;
+atomic_uintptr_t ScopedErrorReportLock::reporting_thread_ = {0};
+StaticSpinMutex ScopedErrorReportLock::mutex_;
-ScopedErrorReportLock::ScopedErrorReportLock() {
+void ScopedErrorReportLock::Lock() {
uptr current = GetThreadSelf();
for (;;) {
uptr expected = 0;
- if (atomic_compare_exchange_strong(&reporting_thread, &expected, current,
+ if (atomic_compare_exchange_strong(&reporting_thread_, &expected, current,
memory_order_relaxed)) {
// We've claimed reporting_thread so proceed.
- CommonSanitizerReportMutex.Lock();
+ mutex_.Lock();
return;
}
@@ -282,13 +282,11 @@ ScopedErrorReportLock::ScopedErrorReportLock() {
}
}
-ScopedErrorReportLock::~ScopedErrorReportLock() {
- CommonSanitizerReportMutex.Unlock();
- atomic_store_relaxed(&reporting_thread, 0);
+void ScopedErrorReportLock::Unlock() {
+ mutex_.Unlock();
+ atomic_store_relaxed(&reporting_thread_, 0);
}
-void ScopedErrorReportLock::CheckLocked() {
- CommonSanitizerReportMutex.CheckLocked();
-}
+void ScopedErrorReportLock::CheckLocked() { mutex_.CheckLocked(); }
} // namespace __sanitizer
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.h b/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.h
index 6c80d545469b..0b28bbe6ddf6 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.h
@@ -95,7 +95,7 @@ class MUTEX ThreadRegistry {
uptr GetMaxAliveThreads();
void Lock() ACQUIRE() { mtx_.Lock(); }
- void CheckLocked() const CHECK_LOCKED { mtx_.CheckLocked(); }
+ void CheckLocked() const CHECK_LOCKED() { mtx_.CheckLocked(); }
void Unlock() RELEASE() { mtx_.Unlock(); }
// Should be guarded by ThreadRegistryLock.
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_thread_safety.h b/compiler-rt/lib/sanitizer_common/sanitizer_thread_safety.h
index 949b98f4c1ad..52b25edaa7a3 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_thread_safety.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_thread_safety.h
@@ -36,7 +36,7 @@
#define RELEASE_SHARED(...) \
THREAD_ANNOTATION(release_shared_capability(__VA_ARGS__))
#define EXCLUDES(...) THREAD_ANNOTATION(locks_excluded(__VA_ARGS__))
-#define CHECK_LOCKED THREAD_ANNOTATION(assert_capability(this))
+#define CHECK_LOCKED(...) THREAD_ANNOTATION(assert_capability(__VA_ARGS__))
#define NO_THREAD_SAFETY_ANALYSIS THREAD_ANNOTATION(no_thread_safety_analysis)
#endif
diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl.cpp b/compiler-rt/lib/tsan/rtl/tsan_rtl.cpp
index 7f769d017935..2d49b9a39a54 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_rtl.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_rtl.cpp
@@ -534,6 +534,7 @@ int Finalize(ThreadState *thr) {
void ForkBefore(ThreadState *thr, uptr pc) NO_THREAD_SAFETY_ANALYSIS {
ctx->thread_registry->Lock();
ctx->report_mtx.Lock();
+ ScopedErrorReportLock::Lock();
// Suppress all reports in the pthread_atfork callbacks.
// Reports will deadlock on the report_mtx.
// We could ignore sync operations as well,
@@ -548,6 +549,7 @@ void ForkBefore(ThreadState *thr, uptr pc) NO_THREAD_SAFETY_ANALYSIS {
void ForkParentAfter(ThreadState *thr, uptr pc) NO_THREAD_SAFETY_ANALYSIS {
thr->suppress_reports--; // Enabled in ForkBefore.
thr->ignore_interceptors--;
+ ScopedErrorReportLock::Unlock();
ctx->report_mtx.Unlock();
ctx->thread_registry->Unlock();
}
@@ -555,6 +557,7 @@ void ForkParentAfter(ThreadState *thr, uptr pc) NO_THREAD_SAFETY_ANALYSIS {
void ForkChildAfter(ThreadState *thr, uptr pc) NO_THREAD_SAFETY_ANALYSIS {
thr->suppress_reports--; // Enabled in ForkBefore.
thr->ignore_interceptors--;
+ ScopedErrorReportLock::Unlock();
ctx->report_mtx.Unlock();
ctx->thread_registry->Unlock();
More information about the llvm-commits
mailing list