diff options
author | Abseil Team <absl-team@google.com> | 2019-03-22T19·20-0700 |
---|---|---|
committer | Derek Mauro <dmauro@google.com> | 2019-03-22T19·30-0400 |
commit | eab2078b53c9e3d9d240135c09d27e3393acb50a (patch) | |
tree | 4d89c6a5877c78cd0e816d6824ca6ba6baf0684c /absl/synchronization | |
parent | 253eb7416421661873afbaa33828a850db978541 (diff) |
Export of internal Abseil changes.
-- 8b7c3bc2fb69608e9b2389b1be0b0de840a4c59d by Derek Mauro <dmauro@google.com>: Set correct flags for clang-cl. https://github.com/abseil/abseil-cpp/pull/278 clang-cl produce binaries with MSVC ABI and wants to be as flag-compatible with pure MSVC as possible, so this leads to all sorts of weird cases. clang-cl alias /Wall as clang's -Weverything which is way too verbose, so it needs /W3 like pure MSVC. clang-cl only understand GCC style warning flags (-W[no]blah) and just silent drop MSVC style warning flags (/wd[num]). clang-cl needs MSVC define flags since it is consuming the same header files as pure MSVC. CMake set CMAKE_CXX_COMPILER_ID as Clang when clang-cl is detected, so need extra if (MSVC) to differentiate it. We are not doing clang-cl specialization in Bazel as currently there is no reliable way to detect clang-cl in Bazel.. Other changes: Add ABSL_ prefix to variable names to avoid name collision in CMake. PiperOrigin-RevId: 239841297 -- add96c3fc067d5c7b6f016d2ba74725a443a185e by CJ Johnson <johnsoncj@google.com>: Eventually Storage will need to refer to the type `absl::InlinedVector<...>*`. This can be done via a forward declaration. However, doing so would move the defaulted allocator template parameter to the forward declaration and thus inside an internal file. Instead of doing that, this change gives Storage access to the template and it's parameters so the complete type can be formed without including it. PiperOrigin-RevId: 239811298 -- b5f5279f1b13b09cae5c745597d64ea1efab146b by CJ Johnson <johnsoncj@google.com>: Simplify/cleanup the benchmark tests for InlinedVector PiperOrigin-RevId: 239805767 -- f5991e51b43b13a0ae95025474071f5039a33d27 by Matt Calabrese <calabrese@google.com>: Update the internal-only IsSwappable traits to be nested inside of namespace absl so that the script to add inline namespaces for LTS releases works with the implementation. PiperOrigin-RevId: 239622024 -- d1cb234dc5706f033ad56f4eb16d94ac5da80d52 by Abseil Team <absl-team@google.com>: Mutex: fix tsan annotations This fixes 2 bugs: 1. We call cond directly in Mutex::AwaitCommon without using EvalConditionAnnotated. As the result we call into user code ignoring synchronization, miss synchronization and report false positives later. Use EvalConditionAnnotated to call cond as we should. 2. We call Mutex invariant ignoring synchronization. Result is the same: we miss synchronization and report false positive races later. Reuse EvalConditionAnnotated to call mutex invariant too. PiperOrigin-RevId: 239583878 -- 52295e4922a9b408fa0dd03d27bc91ccc6645cd7 by Abseil Team <absl-team@google.com>: Clarify how to obtain the same behavior as std::unordered_map::erase if need be. PiperOrigin-RevId: 239549513 -- 6e76e68ed084fd1247981dbb92677ce8e563b0ec by Jon Cohen <cohenjon@google.com>: Avoid the -S -B form of `cmake` since it's only supported starting in CMake 3.13 PiperOrigin-RevId: 239473143 GitOrigin-RevId: 8b7c3bc2fb69608e9b2389b1be0b0de840a4c59d Change-Id: Ib6d356fa1a7435260273df991e65df4149bd5861
Diffstat (limited to 'absl/synchronization')
-rw-r--r-- | absl/synchronization/mutex.cc | 107 |
1 files changed, 69 insertions, 38 deletions
diff --git a/absl/synchronization/mutex.cc b/absl/synchronization/mutex.cc index 6b2eb3314796..37ffff3da872 100644 --- a/absl/synchronization/mutex.cc +++ b/absl/synchronization/mutex.cc @@ -118,6 +118,10 @@ ABSL_CONST_INIT absl::base_internal::AtomicHook< } // namespace +static inline bool EvalConditionAnnotated(const Condition *cond, Mutex *mu, + bool locking, bool trylock, + bool read_lock); + void RegisterMutexProfiler(void (*fn)(int64_t wait_timestamp)) { submit_profile_data.Store(fn); } @@ -233,15 +237,14 @@ enum { // Mutex and CondVar events passed as "ev" to PostSynchEvent SYNCH_EV_SIGNALALL, }; -enum { // Event flags - SYNCH_F_R = 0x01, // reader event - SYNCH_F_LCK = 0x02, // PostSynchEvent called with mutex held - SYNCH_F_ACQ = 0x04, // event is an acquire +enum { // Event flags + SYNCH_F_R = 0x01, // reader event + SYNCH_F_LCK = 0x02, // PostSynchEvent called with mutex held + SYNCH_F_TRY = 0x04, // TryLock or ReaderTryLock + SYNCH_F_UNLOCK = 0x08, // Unlock or ReaderUnlock SYNCH_F_LCK_W = SYNCH_F_LCK, SYNCH_F_LCK_R = SYNCH_F_LCK | SYNCH_F_R, - SYNCH_F_ACQ_W = SYNCH_F_ACQ, - SYNCH_F_ACQ_R = SYNCH_F_ACQ | SYNCH_F_R, }; } // anonymous namespace @@ -250,20 +253,20 @@ static const struct { int flags; const char *msg; } event_properties[] = { - { SYNCH_F_LCK_W|SYNCH_F_ACQ_W, "TryLock succeeded " }, - { 0, "TryLock failed " }, - { SYNCH_F_LCK_R|SYNCH_F_ACQ_R, "ReaderTryLock succeeded " }, - { 0, "ReaderTryLock failed " }, - { SYNCH_F_ACQ_W, "Lock blocking " }, - { SYNCH_F_LCK_W, "Lock returning " }, - { SYNCH_F_ACQ_R, "ReaderLock blocking " }, - { SYNCH_F_LCK_R, "ReaderLock returning " }, - { SYNCH_F_LCK_W, "Unlock " }, - { SYNCH_F_LCK_R, "ReaderUnlock " }, - { 0, "Wait on " }, - { 0, "Wait unblocked " }, - { 0, "Signal on " }, - { 0, "SignalAll on " }, + {SYNCH_F_LCK_W | SYNCH_F_TRY, "TryLock succeeded "}, + {0, "TryLock failed "}, + {SYNCH_F_LCK_R | SYNCH_F_TRY, "ReaderTryLock succeeded "}, + {0, "ReaderTryLock failed "}, + {0, "Lock blocking "}, + {SYNCH_F_LCK_W, "Lock returning "}, + {0, "ReaderLock blocking "}, + {SYNCH_F_LCK_R, "ReaderLock returning "}, + {SYNCH_F_LCK_W | SYNCH_F_UNLOCK, "Unlock "}, + {SYNCH_F_LCK_R | SYNCH_F_UNLOCK, "ReaderUnlock "}, + {0, "Wait on "}, + {0, "Wait unblocked "}, + {0, "Signal on "}, + {0, "SignalAll on "}, }; static absl::base_internal::SpinLock synch_event_mu( @@ -415,9 +418,26 @@ static void PostSynchEvent(void *obj, int ev) { ABSL_RAW_LOG(INFO, "%s%p %s %s", event_properties[ev].msg, obj, (e == nullptr ? "" : e->name), buffer); } - if ((event_properties[ev].flags & SYNCH_F_LCK) != 0 && e != nullptr && - e->invariant != nullptr) { - (*e->invariant)(e->arg); + const int flags = event_properties[ev].flags; + if ((flags & SYNCH_F_LCK) != 0 && e != nullptr && e->invariant != nullptr) { + // Calling the invariant as is causes problems under ThreadSanitizer. + // We are currently inside of Mutex Lock/Unlock and are ignoring all + // memory accesses and synchronization. If the invariant transitively + // synchronizes something else and we ignore the synchronization, we will + // get false positive race reports later. + // Reuse EvalConditionAnnotated to properly call into user code. + struct local { + static bool pred(SynchEvent *ev) { + (*ev->invariant)(ev->arg); + return false; + } + }; + Condition cond(&local::pred, e); + Mutex *mu = static_cast<Mutex *>(obj); + const bool locking = (flags & SYNCH_F_UNLOCK) == 0; + const bool trylock = (flags & SYNCH_F_TRY) != 0; + const bool read_lock = (flags & SYNCH_F_R) != 0; + EvalConditionAnnotated(&cond, mu, locking, trylock, read_lock); } UnrefSynchEvent(e); } @@ -1553,7 +1573,7 @@ bool Mutex::AwaitCommon(const Condition &cond, KernelTimeout t) { ABSL_TSAN_MUTEX_PRE_LOCK(this, TsanFlags(how)); this->LockSlowLoop(&waitp, flags); bool res = waitp.cond != nullptr || // => cond known true from LockSlowLoop - cond.Eval(); + EvalConditionAnnotated(&cond, this, true, false, how == kShared); ABSL_TSAN_MUTEX_POST_LOCK(this, TsanFlags(how), 0); return res; } @@ -1731,12 +1751,17 @@ void Mutex::LockSlow(MuHow how, const Condition *cond, int flags) { // Compute cond->Eval() and tell race detectors that we do it under mutex mu. static inline bool EvalConditionAnnotated(const Condition *cond, Mutex *mu, - bool locking, Mutex::MuHow how) { + bool locking, bool trylock, + bool read_lock) { // Delicate annotation dance. // We are currently inside of read/write lock/unlock operation. // All memory accesses are ignored inside of mutex operations + for unlock // operation tsan considers that we've already released the mutex. bool res = false; +#ifdef THREAD_SANITIZER + const int flags = read_lock ? __tsan_mutex_read_lock : 0; + const int tryflags = flags | (trylock ? __tsan_mutex_try_lock : 0); +#endif if (locking) { // For lock we pretend that we have finished the operation, // evaluate the predicate, then unlock the mutex and start locking it again @@ -1744,24 +1769,26 @@ static inline bool EvalConditionAnnotated(const Condition *cond, Mutex *mu, // Note: we can't simply do POST_LOCK, Eval, PRE_LOCK, because then tsan // will think the lock acquisition is recursive which will trigger // deadlock detector. - ABSL_TSAN_MUTEX_POST_LOCK(mu, TsanFlags(how), 0); + ABSL_TSAN_MUTEX_POST_LOCK(mu, tryflags, 0); res = cond->Eval(); - ABSL_TSAN_MUTEX_PRE_UNLOCK(mu, TsanFlags(how)); - ABSL_TSAN_MUTEX_POST_UNLOCK(mu, TsanFlags(how)); - ABSL_TSAN_MUTEX_PRE_LOCK(mu, TsanFlags(how)); + // There is no "try" version of Unlock, so use flags instead of tryflags. + ABSL_TSAN_MUTEX_PRE_UNLOCK(mu, flags); + ABSL_TSAN_MUTEX_POST_UNLOCK(mu, flags); + ABSL_TSAN_MUTEX_PRE_LOCK(mu, tryflags); } else { // Similarly, for unlock we pretend that we have unlocked the mutex, // lock the mutex, evaluate the predicate, and start unlocking it again // to match the annotation at the end of outer unlock operation. - ABSL_TSAN_MUTEX_POST_UNLOCK(mu, TsanFlags(how)); - ABSL_TSAN_MUTEX_PRE_LOCK(mu, TsanFlags(how)); - ABSL_TSAN_MUTEX_POST_LOCK(mu, TsanFlags(how), 0); + ABSL_TSAN_MUTEX_POST_UNLOCK(mu, flags); + ABSL_TSAN_MUTEX_PRE_LOCK(mu, flags); + ABSL_TSAN_MUTEX_POST_LOCK(mu, flags, 0); res = cond->Eval(); - ABSL_TSAN_MUTEX_PRE_UNLOCK(mu, TsanFlags(how)); + ABSL_TSAN_MUTEX_PRE_UNLOCK(mu, flags); } // Prevent unused param warnings in non-TSAN builds. static_cast<void>(mu); - static_cast<void>(how); + static_cast<void>(trylock); + static_cast<void>(read_lock); return res; } @@ -1807,7 +1834,8 @@ bool Mutex::LockSlowWithDeadline(MuHow how, const Condition *cond, v, (how->fast_or | (v & zap_desig_waker[flags & kMuHasBlocked])) + how->fast_add, std::memory_order_acquire, std::memory_order_relaxed)) { - if (cond == nullptr || EvalConditionAnnotated(cond, this, true, how)) { + if (cond == nullptr || + EvalConditionAnnotated(cond, this, true, false, how == kShared)) { return true; } unlock = true; @@ -1825,7 +1853,8 @@ bool Mutex::LockSlowWithDeadline(MuHow how, const Condition *cond, } this->LockSlowLoop(&waitp, flags); return waitp.cond != nullptr || // => cond known true from LockSlowLoop - cond == nullptr || EvalConditionAnnotated(cond, this, true, how); + cond == nullptr || + EvalConditionAnnotated(cond, this, true, false, how == kShared); } // RAW_CHECK_FMT() takes a condition, a printf-style format string, and @@ -1881,7 +1910,8 @@ void Mutex::LockSlowLoop(SynchWaitParams *waitp, int flags) { waitp->how->fast_add, std::memory_order_acquire, std::memory_order_relaxed)) { if (waitp->cond == nullptr || - EvalConditionAnnotated(waitp->cond, this, true, waitp->how)) { + EvalConditionAnnotated(waitp->cond, this, true, false, + waitp->how == kShared)) { break; // we timed out, or condition true, so return } this->UnlockSlow(waitp); // got lock but condition false @@ -1924,7 +1954,8 @@ void Mutex::LockSlowLoop(SynchWaitParams *waitp, int flags) { std::memory_order_release, std::memory_order_relaxed)); if (waitp->cond == nullptr || - EvalConditionAnnotated(waitp->cond, this, true, waitp->how)) { + EvalConditionAnnotated(waitp->cond, this, true, false, + waitp->how == kShared)) { break; // we timed out, or condition true, so return } this->UnlockSlow(waitp); // got lock but condition false |