Index: lib/scudo/standalone/fuchsia.cc =================================================================== --- lib/scudo/standalone/fuchsia.cc +++ lib/scudo/standalone/fuchsia.cc @@ -14,8 +14,10 @@ #include "mutex.h" #include "string_utils.h" -#include // for PAGE_SIZE -#include // for getenv() +#include // for sync_mutex_t +#include // for PAGE_SIZE +#include // for getenv() +#include #include #include @@ -90,7 +92,8 @@ } uintptr_t P; - zx_vm_option_t MapFlags = ZX_VM_PERM_READ | ZX_VM_PERM_WRITE; + zx_vm_option_t MapFlags = + ZX_VM_PERM_READ | ZX_VM_PERM_WRITE | ZX_VM_ALLOW_FAULTS; const uint64_t Offset = Addr ? reinterpret_cast(Addr) - Data->VmarBase : 0; if (Offset) @@ -149,18 +152,21 @@ const char *getEnv(const char *Name) { return getenv(Name); } -void BlockingMutex::wait() { - const zx_status_t Status = - _zx_futex_wait(reinterpret_cast(OpaqueStorage), MtxSleeping, - ZX_HANDLE_INVALID, ZX_TIME_INFINITE); - if (Status != ZX_ERR_BAD_STATE) - CHECK_EQ(Status, ZX_OK); // Normal race +// Note: we need to flag these methods with __TA_NO_THREAD_SAFETY_ANALYSIS +// because the Fuchsia implementation of sync_mutex_t has clang thread safety +// annotations. Were we to apply proper capability annotations to the top level +// BlockingMutex class itself, they would not be needed. As it stands, the +// thread analysis thinks that we are locking the mutex and accidentally leaving +// it locked on the way out. +void BlockingMutex::lock() __TA_NO_THREAD_SAFETY_ANALYSIS { + // Size and alignment must be compatible between both types. + COMPILER_CHECK(sizeof(sync_mutex_t) <= sizeof(OpaqueStorage)); + COMPILER_CHECK(!(alignof(decltype(OpaqueStorage)) % alignof(sync_mutex_t))); + sync_mutex_lock(reinterpret_cast(OpaqueStorage)); } -void BlockingMutex::wake() { - const zx_status_t Status = - _zx_futex_wake(reinterpret_cast(OpaqueStorage), 1); - CHECK_EQ(Status, ZX_OK); +void BlockingMutex::unlock() __TA_NO_THREAD_SAFETY_ANALYSIS { + sync_mutex_unlock(reinterpret_cast(OpaqueStorage)); } u64 getMonotonicTime() { return _zx_clock_get_monotonic(); } Index: lib/scudo/standalone/linux.cc =================================================================== --- lib/scudo/standalone/linux.cc +++ lib/scudo/standalone/linux.cc @@ -84,14 +84,22 @@ // Calling getenv should be fine (c)(tm) at any time. const char *getEnv(const char *Name) { return getenv(Name); } -void BlockingMutex::wait() { - syscall(SYS_futex, reinterpret_cast(OpaqueStorage), FUTEX_WAIT_PRIVATE, - MtxSleeping, nullptr, nullptr, 0); -} - -void BlockingMutex::wake() { - syscall(SYS_futex, reinterpret_cast(OpaqueStorage), FUTEX_WAKE_PRIVATE, - 1, nullptr, nullptr, 0); +void BlockingMutex::lock() { + atomic_u32 *M = reinterpret_cast(&OpaqueStorage); + if (atomic_exchange(M, MtxLocked, memory_order_acquire) == MtxUnlocked) + return; + while (atomic_exchange(M, MtxSleeping, memory_order_acquire) != MtxUnlocked) + syscall(SYS_futex, reinterpret_cast(OpaqueStorage), + FUTEX_WAIT_PRIVATE, MtxSleeping, nullptr, nullptr, 0); +} + +void BlockingMutex::unlock() { + atomic_u32 *M = reinterpret_cast(&OpaqueStorage); + const u32 V = atomic_exchange(M, MtxUnlocked, memory_order_release); + DCHECK_NE(V, MtxUnlocked); + if (V == MtxSleeping) + syscall(SYS_futex, reinterpret_cast(OpaqueStorage), + FUTEX_WAKE_PRIVATE, 1, nullptr, nullptr, 0); } u64 getMonotonicTime() { Index: lib/scudo/standalone/mutex.h =================================================================== --- lib/scudo/standalone/mutex.h +++ lib/scudo/standalone/mutex.h @@ -57,34 +57,19 @@ void operator=(const SpinMutex &) = delete; }; -enum MutexState { MtxUnlocked = 0, MtxLocked = 1, MtxSleeping = 2 }; - class BlockingMutex { public: - explicit constexpr BlockingMutex(LinkerInitialized) : OpaqueStorage{0} {} + explicit constexpr BlockingMutex(LinkerInitialized) : OpaqueStorage{} {} BlockingMutex() { memset(this, 0, sizeof(*this)); } - void wait(); - void wake(); - void lock() { - atomic_u32 *M = reinterpret_cast(&OpaqueStorage); - if (atomic_exchange(M, MtxLocked, memory_order_acquire) == MtxUnlocked) - return; - while (atomic_exchange(M, MtxSleeping, memory_order_acquire) != MtxUnlocked) - wait(); - } - void unlock() { - atomic_u32 *M = reinterpret_cast(&OpaqueStorage); - const u32 V = atomic_exchange(M, MtxUnlocked, memory_order_release); - DCHECK_NE(V, MtxUnlocked); - if (V == MtxSleeping) - wake(); - } + void lock(); + void unlock(); void checkLocked() { atomic_u32 *M = reinterpret_cast(&OpaqueStorage); CHECK_NE(MtxUnlocked, atomic_load_relaxed(M)); } private: + enum MutexState { MtxUnlocked = 0, MtxLocked = 1, MtxSleeping = 2 }; uptr OpaqueStorage[1]; };