[libcxx-commits] [libcxx] 6e22b53 - [libc++] Fix `std::atomic::wait` ulock wait UL_COMPARE_AND_WAIT64 (#92783)
via libcxx-commits
libcxx-commits at lists.llvm.org
Sat Jun 1 13:12:08 PDT 2024
Author: Hui
Date: 2024-06-01T21:12:04+01:00
New Revision: 6e22b538da4b09efb10a59582a3f43d8128ae7d1
URL: https://github.com/llvm/llvm-project/commit/6e22b538da4b09efb10a59582a3f43d8128ae7d1
DIFF: https://github.com/llvm/llvm-project/commit/6e22b538da4b09efb10a59582a3f43d8128ae7d1.diff
LOG: [libc++] Fix `std::atomic::wait` ulock wait UL_COMPARE_AND_WAIT64 (#92783)
in `atomic::wait`, when we call the platform wait ulock_wait , we are
using UL_COMPARE_AND_WAIT. But we should use UL_COMPARE_AND_WAIT64
instead as the address we are waiting for is a 64 bit integer.
fixes https://github.com/llvm/llvm-project/issues/85107
It is rather hard to test directly because in `atomic::wait`, before
calling into the platform wait, our c++ code has some poll logic which
checks the value not changing. Thus in this patch, the test is using the
internal function.
Added:
libcxx/test/libcxx/atomics/atomics.syn/wait.pass.cpp
Modified:
libcxx/src/atomic.cpp
Removed:
################################################################################
diff --git a/libcxx/src/atomic.cpp b/libcxx/src/atomic.cpp
index 44100d4414e0d..7142f38c96090 100644
--- a/libcxx/src/atomic.cpp
+++ b/libcxx/src/atomic.cpp
@@ -69,17 +69,20 @@ extern "C" int __ulock_wait(
uint32_t operation, void* addr, uint64_t value, uint32_t timeout); /* timeout is specified in microseconds */
extern "C" int __ulock_wake(uint32_t operation, void* addr, uint64_t wake_value);
-# define UL_COMPARE_AND_WAIT 1
+// https://github.com/apple/darwin-xnu/blob/2ff845c2e033bd0ff64b5b6aa6063a1f8f65aa32/bsd/sys/ulock.h#L82
+# define UL_COMPARE_AND_WAIT64 5
# define ULF_WAKE_ALL 0x00000100
static void
__libcpp_platform_wait_on_address(__cxx_atomic_contention_t const volatile* __ptr, __cxx_contention_t __val) {
- __ulock_wait(UL_COMPARE_AND_WAIT, const_cast<__cxx_atomic_contention_t*>(__ptr), __val, 0);
+ static_assert(sizeof(__cxx_atomic_contention_t) == 8, "Waiting on 8 bytes value");
+ __ulock_wait(UL_COMPARE_AND_WAIT64, const_cast<__cxx_atomic_contention_t*>(__ptr), __val, 0);
}
static void __libcpp_platform_wake_by_address(__cxx_atomic_contention_t const volatile* __ptr, bool __notify_one) {
+ static_assert(sizeof(__cxx_atomic_contention_t) == 8, "Waking up on 8 bytes value");
__ulock_wake(
- UL_COMPARE_AND_WAIT | (__notify_one ? 0 : ULF_WAKE_ALL), const_cast<__cxx_atomic_contention_t*>(__ptr), 0);
+ UL_COMPARE_AND_WAIT64 | (__notify_one ? 0 : ULF_WAKE_ALL), const_cast<__cxx_atomic_contention_t*>(__ptr), 0);
}
#elif defined(__FreeBSD__) && __SIZEOF_LONG__ == 8
diff --git a/libcxx/test/libcxx/atomics/atomics.syn/wait.pass.cpp b/libcxx/test/libcxx/atomics/atomics.syn/wait.pass.cpp
new file mode 100644
index 0000000000000..f29dc6eb463fe
--- /dev/null
+++ b/libcxx/test/libcxx/atomics/atomics.syn/wait.pass.cpp
@@ -0,0 +1,32 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+// UNSUPPORTED: no-threads
+// This bug was first fixed in LLVM 19
+// UNSUPPORTED: using-built-library-before-llvm-19
+// XFAIL: availability-synchronization_library-missing
+
+#include <atomic>
+#include <cassert>
+
+void test_85107() {
+ if constexpr (sizeof(std::__cxx_contention_t) == 8 && sizeof(long) > 4) {
+ // https://github.com/llvm/llvm-project/issues/85107
+ // [libc++] atomic_wait uses UL_COMPARE_AND_WAIT when it should use UL_COMPARE_AND_WAIT64 on Darwin
+ constexpr std::__cxx_contention_t old_val = 0;
+ constexpr std::__cxx_contention_t new_val = old_val + (1ll << 32);
+ std::__cxx_atomic_contention_t ct(new_val);
+ std::__libcpp_atomic_wait(&ct, old_val); // this will hang forever if the bug is present
+ }
+}
+
+int main(int, char**) {
+ test_85107();
+
+ return 0;
+}
More information about the libcxx-commits
mailing list