From b7d6438450a3e2f4a6be4a0585ea364d72313412 Mon Sep 17 00:00:00 2001 From: Schrodinger ZHU Yifan Date: Tue, 28 Apr 2026 16:55:04 -0400 Subject: [PATCH] [libc] implement recursive mutex and fix wrong initializer (#193992) Fix #193892. This is found during libc++ bringing up process. This patch: - implement recursive mutex support for real. - fix the issue that pthread mutex initializer and public mutex interface misalign with internal representation. this was the root cause that hangs the libc++ test as padding bytes effectively pollute the futex state if default initializer was used. - additionally, we changed the field to bitfield to allow encoding more data. (e.g. the error checking flag that is not yet added). timed bit is removed as we always support it anyway. PI bit is added instead. --- .../include/llvm-libc-macros/pthread-macros.h | 12 ++- libc/include/llvm-libc-types/CMakeLists.txt | 2 +- libc/include/llvm-libc-types/__mutex_type.h | 17 ++-- libc/src/__support/threads/CMakeLists.txt | 2 + libc/src/__support/threads/fork_callbacks.cpp | 4 +- libc/src/__support/threads/linux/barrier.cpp | 7 +- libc/src/__support/threads/mutex_common.h | 1 + libc/src/__support/threads/thread.cpp | 8 +- libc/src/__support/threads/unix_mutex.h | 99 ++++++++++++------- libc/src/pthread/pthread_mutex_init.cpp | 31 ++++-- libc/src/stdlib/atexit.cpp | 3 +- libc/src/threads/mtx_init.cpp | 14 +-- .../integration/src/pthread/CMakeLists.txt | 3 + .../src/pthread/pthread_mutex_test.cpp | 99 ++++++++++++++++++- .../test/integration/src/threads/mtx_test.cpp | 37 +++++++ 15 files changed, 261 insertions(+), 78 deletions(-) diff --git a/libc/include/llvm-libc-macros/pthread-macros.h b/libc/include/llvm-libc-macros/pthread-macros.h index 58019403bb33..1619ad123d2c 100644 --- a/libc/include/llvm-libc-macros/pthread-macros.h +++ b/libc/include/llvm-libc-macros/pthread-macros.h @@ -32,15 +32,17 @@ #ifdef __linux__ #define PTHREAD_MUTEX_INITIALIZER \ { \ - /* .__timed = */ 0, /* .__recursive = */ 0, \ - /* .__robust = */ 0, /* .__owner = */ NULL, \ - /* .__lock_count = */ 0, /* .__futex_word = */ {0}, \ + /* .__ftxw = */ {0}, /* .__priority_inherit = */ 0, \ + /* .__recursive = */ 0, /* .__robust = */ 0, \ + /* .__pshared = */ 0, /* .__owner = */ 0, \ + /* .__lock_count = */ 0, \ } #else #define PTHREAD_MUTEX_INITIALIZER \ { \ - /* .__timed = */ 0, /* .__recursive = */ 0, \ - /* .__robust = */ 0, /* .__owner = */ NULL, \ + /* .__ftxw = */ {0}, /* .__priority_inherit = */ 0, \ + /* .__recursive = */ 0, /* .__robust = */ 0, \ + /* .__pshared = */ 0, /* .__owner = */ 0, \ /* .__lock_count = */ 0, \ } #endif diff --git a/libc/include/llvm-libc-types/CMakeLists.txt b/libc/include/llvm-libc-types/CMakeLists.txt index 66d507e1db3b..ad294279e0f0 100644 --- a/libc/include/llvm-libc-types/CMakeLists.txt +++ b/libc/include/llvm-libc-types/CMakeLists.txt @@ -17,7 +17,7 @@ add_header(__exec_argv_t HDR __exec_argv_t.h) add_header(__exec_envp_t HDR __exec_envp_t.h) add_header(__futex_word HDR __futex_word.h) add_header(pid_t HDR pid_t.h) -add_header(__mutex_type HDR __mutex_type.h DEPENDS .__futex_word .pid_t) +add_header(__mutex_type HDR __mutex_type.h DEPENDS .__futex_word .pid_t .size_t) add_header(__barrier_type HDR __barrier_type.h) add_header(__pthread_once_func_t HDR __pthread_once_func_t.h) add_header(__pthread_start_t HDR __pthread_start_t.h) diff --git a/libc/include/llvm-libc-types/__mutex_type.h b/libc/include/llvm-libc-types/__mutex_type.h index 835561626149..455eceb3c395 100644 --- a/libc/include/llvm-libc-types/__mutex_type.h +++ b/libc/include/llvm-libc-types/__mutex_type.h @@ -10,20 +10,23 @@ #define LLVM_LIBC_TYPES___MUTEX_TYPE_H #include "__futex_word.h" +#include "pid_t.h" +#include "size_t.h" typedef struct { - unsigned char __timed; - unsigned char __recursive; - unsigned char __robust; - - void *__owner; - unsigned long long __lock_count; - #ifdef __linux__ __futex_word __ftxw; #else #error "Mutex type not defined for the target platform." #endif + + unsigned int __priority_inherit : 1; + unsigned int __recursive : 1; + unsigned int __robust : 1; + unsigned int __pshared : 1; + + pid_t __owner; + size_t __lock_count; } __mutex_type; #endif // LLVM_LIBC_TYPES___MUTEX_TYPE_H diff --git a/libc/src/__support/threads/CMakeLists.txt b/libc/src/__support/threads/CMakeLists.txt index b603a2cfbb0e..0846a78bbf90 100644 --- a/libc/src/__support/threads/CMakeLists.txt +++ b/libc/src/__support/threads/CMakeLists.txt @@ -75,6 +75,7 @@ if(TARGET libc.src.__support.threads.${LIBC_TARGET_OS}.futex_utils) unix_mutex.h DEPENDS .raw_mutex + libc.src.__support.CPP.atomic ) add_header_library( @@ -162,6 +163,7 @@ if(TARGET libc.src.__support.threads.futex_utils) libc.src.__support.threads.sleep libc.src.__support.time.monotonicity libc.src.__support.time.abs_timeout + libc.src.__support.threads.identifier ) endif() diff --git a/libc/src/__support/threads/fork_callbacks.cpp b/libc/src/__support/threads/fork_callbacks.cpp index 49fb41a97582..b8e6b2b1ba10 100644 --- a/libc/src/__support/threads/fork_callbacks.cpp +++ b/libc/src/__support/threads/fork_callbacks.cpp @@ -34,8 +34,8 @@ class AtForkCallbackManager { public: constexpr AtForkCallbackManager() - : mtx(/*timed=*/false, /*recursive=*/false, /*robust=*/false, - /*pshared=*/false), + : mtx(/*is_priority_inherit=*/false, /*is_recursive=*/false, + /*is_robust=*/false, /*is_pshared=*/false), next_index(0) {} bool register_triple(const ForkCallbackTriple &triple) { diff --git a/libc/src/__support/threads/linux/barrier.cpp b/libc/src/__support/threads/linux/barrier.cpp index 80f25f8ac7a2..882f6fa61887 100644 --- a/libc/src/__support/threads/linux/barrier.cpp +++ b/libc/src/__support/threads/linux/barrier.cpp @@ -28,10 +28,9 @@ int Barrier::init(Barrier *b, new (&b->entering) CndVar(attr ? attr->pshared : false); new (&b->exiting) CndVar(attr ? attr->pshared : false); - auto mutex_err = Mutex::init(&b->m, false, false, false, - /*pshared=*/attr ? attr->pshared : false); - if (mutex_err != MutexError::NONE) - return EAGAIN; + new (&b->m) Mutex(/*is_priority_inherit=*/false, /*is_recursive=*/false, + /*is_robust=*/false, + /*is_pshared=*/attr ? attr->pshared : false); return 0; } diff --git a/libc/src/__support/threads/mutex_common.h b/libc/src/__support/threads/mutex_common.h index 9913f69a6a61..1c71c5c78bc2 100644 --- a/libc/src/__support/threads/mutex_common.h +++ b/libc/src/__support/threads/mutex_common.h @@ -19,6 +19,7 @@ enum class MutexError : int { TIMEOUT, UNLOCK_WITHOUT_LOCK, BAD_LOCK_STATE, + OVERFLOW, }; } // namespace LIBC_NAMESPACE_DECL diff --git a/libc/src/__support/threads/thread.cpp b/libc/src/__support/threads/thread.cpp index 9618d7829161..ae179f068b88 100644 --- a/libc/src/__support/threads/thread.cpp +++ b/libc/src/__support/threads/thread.cpp @@ -53,8 +53,8 @@ class TSSKeyMgr { public: constexpr TSSKeyMgr() - : mtx(/*timed=*/false, /*recursive=*/false, /*robust=*/false, - /*pshared=*/false) {} + : mtx(/*is_priority_inherit=*/false, /*is_recursive=*/false, + /*is_robust=*/false, /*is_pshared=*/false) {} cpp::optional new_key(TSSDtor *dtor) { cpp::lock_guard lock(mtx); @@ -112,8 +112,8 @@ class ThreadAtExitCallbackMgr { public: constexpr ThreadAtExitCallbackMgr() - : mtx(/*timed=*/false, /*recursive=*/false, /*robust=*/false, - /*pshared=*/false) {} + : mtx(/*is_priority_inherit=*/false, /*is_recursive=*/false, + /*is_robust=*/false, /*is_pshared=*/false) {} int add_callback(AtExitCallback *callback, void *obj) { cpp::lock_guard lock(mtx); diff --git a/libc/src/__support/threads/unix_mutex.h b/libc/src/__support/threads/unix_mutex.h index ac05e2528ddc..918c7e4fc1d2 100644 --- a/libc/src/__support/threads/unix_mutex.h +++ b/libc/src/__support/threads/unix_mutex.h @@ -10,9 +10,13 @@ #define LLVM_LIBC_SRC___SUPPORT_THREADS_UNIX_MUTEX_H #include "hdr/types/pid_t.h" +#include "hdr/types/size_t.h" +#include "src/__support/CPP/atomic.h" #include "src/__support/CPP/optional.h" #include "src/__support/libc_assert.h" #include "src/__support/macros/config.h" +#include "src/__support/macros/optimization.h" +#include "src/__support/threads/identifier.h" #include "src/__support/threads/mutex_common.h" #include "src/__support/threads/raw_mutex.h" @@ -20,37 +24,46 @@ namespace LIBC_NAMESPACE_DECL { // TODO: support shared/recursive/robust mutexes. class Mutex final : private RawMutex { - // reserved timed, may be useful when combined with other flags. - unsigned char timed; - unsigned char recursive; - unsigned char robust; - unsigned char pshared; + // Use bitfields to allow encoding more attributes. + // TODO: we may still need error checking or other flags and the robustness + // and priority inheritance will need to be implemented. + // See also https://github.com/llvm/llvm-project/issues/194396 + LIBC_PREFERED_TYPE(bool) unsigned int priority_inherit : 1; + LIBC_PREFERED_TYPE(bool) unsigned int recursive : 1; + LIBC_PREFERED_TYPE(bool) unsigned int robust : 1; + LIBC_PREFERED_TYPE(bool) unsigned int pshared : 1; // TLS address may not work across forked processes. Use thread id instead. - pid_t owner; - unsigned long long lock_count; + cpp::Atomic owner; + size_t lock_count; // CndVar needs to access Mutex as RawMutex friend class CndVar; -public: - LIBC_INLINE constexpr Mutex(bool is_timed, bool is_recursive, bool is_robust, - bool is_pshared) - : RawMutex(), timed(is_timed), recursive(is_recursive), robust(is_robust), - pshared(is_pshared), owner(0), lock_count(0) {} + template + LIBC_INLINE MutexError lock_impl(LockRoutine do_lock) { + if (is_recursive() && owner == internal::gettid()) { + if (LIBC_UNLIKELY(lock_count == cpp::numeric_limits::max())) + return MutexError::OVERFLOW; + lock_count++; + return MutexError::NONE; + } - LIBC_INLINE static MutexError init(Mutex *mutex, bool is_timed, bool isrecur, - bool isrobust, bool is_pshared) { - RawMutex::init(mutex); - mutex->timed = is_timed; - mutex->recursive = isrecur; - mutex->robust = isrobust; - mutex->pshared = is_pshared; - mutex->owner = 0; - mutex->lock_count = 0; - return MutexError::NONE; + MutexError res = do_lock(); + if (is_recursive() && res == MutexError::NONE) { + owner = internal::gettid(); + lock_count = 1; + } + return res; } +public: + LIBC_INLINE constexpr Mutex(bool is_priority_inherit, bool is_recursive, + bool is_robust, bool is_pshared) + : RawMutex(), priority_inherit(is_priority_inherit), + recursive(is_recursive), robust(is_robust), pshared(is_pshared), + owner(0), lock_count(0) {} + LIBC_INLINE static MutexError destroy(Mutex *lock) { LIBC_ASSERT(lock->owner == 0 && lock->lock_count == 0 && "Mutex destroyed while being locked."); @@ -58,39 +71,53 @@ public: return MutexError::NONE; } - // TODO: record owner and lock count. LIBC_INLINE MutexError lock() { - // Since timeout is not specified, we do not need to check the return value. - this->RawMutex::lock( - /* timeout=*/cpp::nullopt, this->pshared); - return MutexError::NONE; + return lock_impl([this] { + // Since timeout is not specified, we do not need to check the return + // value. + // TODO: check deadlock? POSIX made it optional. + this->RawMutex::lock(/* timeout=*/cpp::nullopt, this->pshared); + return MutexError::NONE; + }); } - // TODO: record owner and lock count. LIBC_INLINE MutexError timed_lock(internal::AbsTimeout abs_time) { - if (this->RawMutex::lock(abs_time, this->pshared)) - return MutexError::NONE; - return MutexError::TIMEOUT; + return lock_impl([this, abs_time] { + // TODO: check deadlock? POSIX made it optional. + if (this->RawMutex::lock(abs_time, this->pshared)) + return MutexError::NONE; + return MutexError::TIMEOUT; + }); } LIBC_INLINE MutexError unlock() { + if (is_recursive() && owner == internal::gettid()) { + lock_count--; + if (lock_count == 0) + owner = 0; + else + return MutexError::NONE; + } if (this->RawMutex::unlock(this->pshared)) return MutexError::NONE; return MutexError::UNLOCK_WITHOUT_LOCK; } - // TODO: record owner and lock count. LIBC_INLINE MutexError try_lock() { - if (this->RawMutex::try_lock()) - return MutexError::NONE; - return MutexError::BUSY; + return lock_impl([this] { + if (this->RawMutex::try_lock()) + return MutexError::NONE; + return MutexError::BUSY; + }); } LIBC_INLINE bool can_be_requeued() const { - return !this->pshared && !this->robust; + return !this->pshared && !this->robust && !this->recursive && + !this->priority_inherit; } LIBC_INLINE bool is_robust() const { return this->robust; } + LIBC_INLINE bool is_recursive() const { return this->recursive; } }; } // namespace LIBC_NAMESPACE_DECL diff --git a/libc/src/pthread/pthread_mutex_init.cpp b/libc/src/pthread/pthread_mutex_init.cpp index 94052e53d8c0..5bc1a6103ce5 100644 --- a/libc/src/pthread/pthread_mutex_init.cpp +++ b/libc/src/pthread/pthread_mutex_init.cpp @@ -9,6 +9,7 @@ #include "pthread_mutex_init.h" #include "pthread_mutexattr.h" +#include "src/__support/CPP/new.h" #include "src/__support/common.h" #include "src/__support/macros/config.h" #include "src/__support/threads/mutex.h" @@ -17,20 +18,34 @@ namespace LIBC_NAMESPACE_DECL { -static_assert(sizeof(Mutex) <= sizeof(pthread_mutex_t), - "The public pthread_mutex_t type cannot accommodate the internal " +static_assert(sizeof(Mutex) == sizeof(pthread_mutex_t) && + alignof(Mutex) == alignof(pthread_mutex_t), + "The public pthread_mutex_t type must exactly match the internal " "mutex type."); LLVM_LIBC_FUNCTION(int, pthread_mutex_init, (pthread_mutex_t * m, const pthread_mutexattr_t *__restrict attr)) { auto mutexattr = attr == nullptr ? DEFAULT_MUTEXATTR : *attr; - auto err = - Mutex::init(reinterpret_cast(m), /*is_timed=*/true, - get_mutexattr_type(mutexattr) & PTHREAD_MUTEX_RECURSIVE, - get_mutexattr_robust(mutexattr) & PTHREAD_MUTEX_ROBUST, - get_mutexattr_pshared(mutexattr) & PTHREAD_PROCESS_SHARED); - return err == MutexError::NONE ? 0 : EAGAIN; + bool is_recursive = false; + switch (get_mutexattr_type(mutexattr)) { + case PTHREAD_MUTEX_NORMAL: + case PTHREAD_MUTEX_ERRORCHECK: + break; + case PTHREAD_MUTEX_RECURSIVE: + is_recursive = true; + break; + } + + bool is_robust = false; + if (get_mutexattr_robust(mutexattr) == PTHREAD_MUTEX_ROBUST) + is_robust = true; + + bool is_pshared = get_mutexattr_pshared(mutexattr) == PTHREAD_PROCESS_SHARED; + + new (m) + Mutex(/*is_priority_inherit=*/false, is_recursive, is_robust, is_pshared); + return 0; } } // namespace LIBC_NAMESPACE_DECL diff --git a/libc/src/stdlib/atexit.cpp b/libc/src/stdlib/atexit.cpp index c8a15dd3cfef..5b924be3a63e 100644 --- a/libc/src/stdlib/atexit.cpp +++ b/libc/src/stdlib/atexit.cpp @@ -15,7 +15,8 @@ namespace LIBC_NAMESPACE_DECL { constinit ExitCallbackList atexit_callbacks; -Mutex handler_list_mtx(false, false, false, false); +Mutex handler_list_mtx(/*is_priority_inherit=*/false, /*is_recursive=*/false, + /*is_robust=*/false, /*is_pshared=*/false); extern "C" { diff --git a/libc/src/threads/mtx_init.cpp b/libc/src/threads/mtx_init.cpp index eb0ba5010584..c6b0278c52f7 100644 --- a/libc/src/threads/mtx_init.cpp +++ b/libc/src/threads/mtx_init.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "src/threads/mtx_init.h" +#include "src/__support/CPP/new.h" #include "src/__support/common.h" #include "src/__support/macros/config.h" #include "src/__support/threads/mutex.h" @@ -15,15 +16,16 @@ namespace LIBC_NAMESPACE_DECL { -static_assert(sizeof(Mutex) <= sizeof(mtx_t), - "The public mtx_t type cannot accommodate the internal mutex " +static_assert(sizeof(Mutex) == sizeof(mtx_t) && + alignof(Mutex) == alignof(mtx_t), + "The public mtx_t type must exactly match the internal mutex " "type."); LLVM_LIBC_FUNCTION(int, mtx_init, (mtx_t * m, int type)) { - auto err = Mutex::init(reinterpret_cast(m), type & mtx_timed, - type & mtx_recursive, /* is_robust */ false, - /* is_pshared */ false); - return err == MutexError::NONE ? thrd_success : thrd_error; + new (m) Mutex(/*is_priority_inherit=*/false, + /*is_recursive=*/static_cast(type & mtx_recursive), + /*is_robust=*/false, /*is_pshared=*/false); + return thrd_success; } } // namespace LIBC_NAMESPACE_DECL diff --git a/libc/test/integration/src/pthread/CMakeLists.txt b/libc/test/integration/src/pthread/CMakeLists.txt index 71cb039f7761..eceebb42f18c 100644 --- a/libc/test/integration/src/pthread/CMakeLists.txt +++ b/libc/test/integration/src/pthread/CMakeLists.txt @@ -15,6 +15,9 @@ add_integration_test( libc.src.pthread.pthread_mutex_lock libc.src.pthread.pthread_mutex_trylock libc.src.pthread.pthread_mutex_unlock + libc.src.pthread.pthread_mutexattr_destroy + libc.src.pthread.pthread_mutexattr_init + libc.src.pthread.pthread_mutexattr_settype libc.src.pthread.pthread_create libc.src.pthread.pthread_join ) diff --git a/libc/test/integration/src/pthread/pthread_mutex_test.cpp b/libc/test/integration/src/pthread/pthread_mutex_test.cpp index 488954c0ef25..730aadafd064 100644 --- a/libc/test/integration/src/pthread/pthread_mutex_test.cpp +++ b/libc/test/integration/src/pthread/pthread_mutex_test.cpp @@ -15,6 +15,10 @@ #include "src/pthread/pthread_mutex_lock.h" #include "src/pthread/pthread_mutex_trylock.h" #include "src/pthread/pthread_mutex_unlock.h" +#include "src/pthread/pthread_mutexattr_destroy.h" +#include "src/pthread/pthread_mutexattr_init.h" +#include "src/pthread/pthread_mutexattr_settype.h" +#include "src/string/memory_utils/inline_memcpy.h" #include "test/IntegrationTest/test.h" #include @@ -22,6 +26,15 @@ constexpr int START = 0; constexpr int MAX = 10000; +static pthread_mutex_t snapshot_mutex(const void *mutex_storage) { + pthread_mutex_t snapshot; + // The original storage may currently hold libc's internal mutex + // representation. Copy the bytes into pthread_mutex_t storage before + // inspection to avoid strict aliasing violations. + LIBC_NAMESPACE::inline_memcpy(&snapshot, mutex_storage, sizeof(snapshot)); + return snapshot; +} + pthread_mutex_t mutex; static int shared_int = START; @@ -143,6 +156,86 @@ void trylock_test() { LIBC_NAMESPACE::pthread_mutex_destroy(&trylock_mutex); } +void *trylock_other_thread(void *arg) { + auto *mutex = reinterpret_cast(arg); + int result = LIBC_NAMESPACE::pthread_mutex_trylock(mutex); + if (result == 0) + ASSERT_EQ(LIBC_NAMESPACE::pthread_mutex_unlock(mutex), 0); + return reinterpret_cast(uintptr_t(result)); +} + +void recursive_mutex_test() { + pthread_mutexattr_t attr; + pthread_mutex_t recursive_mutex; + ASSERT_EQ(LIBC_NAMESPACE::pthread_mutexattr_init(&attr), 0); + ASSERT_EQ( + LIBC_NAMESPACE::pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE), + 0); + ASSERT_EQ(LIBC_NAMESPACE::pthread_mutex_init(&recursive_mutex, &attr), 0); + ASSERT_EQ(LIBC_NAMESPACE::pthread_mutexattr_destroy(&attr), 0); + + pthread_mutex_t snapshot = snapshot_mutex(&recursive_mutex); + ASSERT_TRUE(snapshot.__recursive); + ASSERT_EQ(snapshot.__owner, 0); + ASSERT_EQ(snapshot.__lock_count, size_t(0)); + + ASSERT_EQ(LIBC_NAMESPACE::pthread_mutex_lock(&recursive_mutex), 0); + ASSERT_EQ(LIBC_NAMESPACE::pthread_mutex_lock(&recursive_mutex), 0); + + pthread_t thread; + ASSERT_EQ(LIBC_NAMESPACE::pthread_create( + &thread, nullptr, trylock_other_thread, &recursive_mutex), + 0); + void *retval = nullptr; + ASSERT_EQ(LIBC_NAMESPACE::pthread_join(thread, &retval), 0); + ASSERT_EQ(uintptr_t(retval), uintptr_t(EBUSY)); + + ASSERT_EQ(LIBC_NAMESPACE::pthread_mutex_unlock(&recursive_mutex), 0); + ASSERT_EQ(LIBC_NAMESPACE::pthread_mutex_unlock(&recursive_mutex), 0); + snapshot = snapshot_mutex(&recursive_mutex); + ASSERT_EQ(snapshot.__owner, 0); + ASSERT_EQ(snapshot.__lock_count, size_t(0)); + + ASSERT_EQ(LIBC_NAMESPACE::pthread_mutex_trylock(&recursive_mutex), 0); + ASSERT_EQ(LIBC_NAMESPACE::pthread_mutex_unlock(&recursive_mutex), 0); + ASSERT_EQ(LIBC_NAMESPACE::pthread_mutex_destroy(&recursive_mutex), 0); +} + +[[maybe_unused]] +static pthread_mutex_t test_initializer = PTHREAD_MUTEX_INITIALIZER; + +// POSIX.1 requires PTHREAD_MUTEX_INITIALIZER is consistent with +// pthread_mutex_init(m, nullptr). +void initializer_acts_the_same_as_null_attr() { + pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER; + pthread_mutex_t mutex_from_init; + ASSERT_EQ(LIBC_NAMESPACE::pthread_mutex_init(&mutex_from_init, nullptr), 0); + + pthread_mutex_t mutex_snapshot = snapshot_mutex(&mutex); + pthread_mutex_t mutex_from_init_snapshot = snapshot_mutex(&mutex_from_init); + // Do per-field comparison. We cannot do direct bytewise comparison because + // the layout has padding bits and __builtin_clear_padding is not available. + ASSERT_EQ(mutex_snapshot.__ftxw.__word, + mutex_from_init_snapshot.__ftxw.__word); + ASSERT_EQ(mutex_snapshot.__priority_inherit, + mutex_from_init_snapshot.__priority_inherit); + ASSERT_EQ(mutex_snapshot.__recursive, mutex_from_init_snapshot.__recursive); + ASSERT_EQ(mutex_snapshot.__robust, mutex_from_init_snapshot.__robust); + ASSERT_EQ(mutex_snapshot.__pshared, mutex_from_init_snapshot.__pshared); + ASSERT_EQ(mutex_snapshot.__owner, mutex_from_init_snapshot.__owner); + ASSERT_EQ(mutex_snapshot.__lock_count, mutex_from_init_snapshot.__lock_count); + + ASSERT_EQ(LIBC_NAMESPACE::pthread_mutex_lock(&mutex), 0); + ASSERT_EQ(LIBC_NAMESPACE::pthread_mutex_trylock(&mutex), EBUSY); + ASSERT_EQ(LIBC_NAMESPACE::pthread_mutex_unlock(&mutex), 0); + ASSERT_EQ(LIBC_NAMESPACE::pthread_mutex_destroy(&mutex), 0); + + ASSERT_EQ(LIBC_NAMESPACE::pthread_mutex_lock(&mutex_from_init), 0); + ASSERT_EQ(LIBC_NAMESPACE::pthread_mutex_trylock(&mutex_from_init), EBUSY); + ASSERT_EQ(LIBC_NAMESPACE::pthread_mutex_unlock(&mutex_from_init), 0); + ASSERT_EQ(LIBC_NAMESPACE::pthread_mutex_destroy(&mutex_from_init), 0); +} + static constexpr int THREAD_COUNT = 10; static pthread_mutex_t multiple_waiter_lock; static pthread_mutex_t counter_lock; @@ -199,14 +292,12 @@ void multiple_waiters() { LIBC_NAMESPACE::pthread_mutex_destroy(&counter_lock); } -// Test the initializer -[[maybe_unused]] -static pthread_mutex_t test_initializer = PTHREAD_MUTEX_INITIALIZER; - TEST_MAIN() { relay_counter(); wait_and_step(); trylock_test(); + recursive_mutex_test(); + initializer_acts_the_same_as_null_attr(); multiple_waiters(); return 0; } diff --git a/libc/test/integration/src/threads/mtx_test.cpp b/libc/test/integration/src/threads/mtx_test.cpp index 0b0a9faae24a..909c4f2c5c76 100644 --- a/libc/test/integration/src/threads/mtx_test.cpp +++ b/libc/test/integration/src/threads/mtx_test.cpp @@ -6,6 +6,7 @@ // //===----------------------------------------------------------------------===// +#include "src/string/memory_utils/inline_memcpy.h" #include "src/threads/mtx_destroy.h" #include "src/threads/mtx_init.h" #include "src/threads/mtx_lock.h" @@ -20,6 +21,15 @@ constexpr int START = 0; constexpr int MAX = 10000; +static mtx_t snapshot_mutex(const void *mutex_storage) { + mtx_t snapshot; + // The original storage may currently hold libc's internal mutex + // representation. Copy the bytes into mtx_t storage before inspection to + // avoid strict aliasing violations. + LIBC_NAMESPACE::inline_memcpy(&snapshot, mutex_storage, sizeof(snapshot)); + return snapshot; +} + mtx_t mutex; static int shared_int = START; @@ -138,6 +148,32 @@ void wait_and_step() { LIBC_NAMESPACE::mtx_destroy(&step_lock); } +void recursive_mutex_test() { + mtx_t recursive_mutex; + ASSERT_EQ(LIBC_NAMESPACE::mtx_init(&recursive_mutex, mtx_recursive), + static_cast(thrd_success)); + + mtx_t snapshot = snapshot_mutex(&recursive_mutex); + ASSERT_TRUE(snapshot.__recursive); + ASSERT_EQ(snapshot.__owner, 0); + ASSERT_EQ(snapshot.__lock_count, size_t(0)); + + ASSERT_EQ(LIBC_NAMESPACE::mtx_lock(&recursive_mutex), + static_cast(thrd_success)); + ASSERT_EQ(LIBC_NAMESPACE::mtx_lock(&recursive_mutex), + static_cast(thrd_success)); + + ASSERT_EQ(LIBC_NAMESPACE::mtx_unlock(&recursive_mutex), + static_cast(thrd_success)); + ASSERT_EQ(LIBC_NAMESPACE::mtx_unlock(&recursive_mutex), + static_cast(thrd_success)); + snapshot = snapshot_mutex(&recursive_mutex); + ASSERT_EQ(snapshot.__owner, 0); + ASSERT_EQ(snapshot.__lock_count, size_t(0)); + + LIBC_NAMESPACE::mtx_destroy(&recursive_mutex); +} + static constexpr int THREAD_COUNT = 10; static mtx_t multiple_waiter_lock; static mtx_t counter_lock; @@ -197,6 +233,7 @@ void multiple_waiters() { TEST_MAIN() { relay_counter(); wait_and_step(); + recursive_mutex_test(); multiple_waiters(); return 0; }