[libcxx-commits] [libcxx] [libc++] Fix `std::atomic::wait` ulock wait UL_COMPARE_AND_WAIT64 (PR #92783)
via libcxx-commits
libcxx-commits at lists.llvm.org
Sat Jun 1 09:40:27 PDT 2024
https://github.com/huixie90 updated https://github.com/llvm/llvm-project/pull/92783
>From c92abdccff4d81380e16e069b6d1b70e4b9ef0af Mon Sep 17 00:00:00 2001
From: Hui <hui.xie0621 at gmail.com>
Date: Mon, 20 May 2024 17:44:11 +0100
Subject: [PATCH 1/8] [libc++] Fix `std::atomic::wait` ulock wait
UL_COMPARE_AND_WAIT64
---
libcxx/src/atomic.cpp | 6 ++--
.../libcxx/atomics/atomics.syn/wait.pass.cpp | 30 +++++++++++++++++++
2 files changed, 33 insertions(+), 3 deletions(-)
create mode 100644 libcxx/test/libcxx/atomics/atomics.syn/wait.pass.cpp
diff --git a/libcxx/src/atomic.cpp b/libcxx/src/atomic.cpp
index 44100d4414e0d..031f247f88c2f 100644
--- a/libcxx/src/atomic.cpp
+++ b/libcxx/src/atomic.cpp
@@ -69,17 +69,17 @@ 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
+# 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);
+ __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) {
__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..f59db4109bfd5
--- /dev/null
+++ b/libcxx/test/libcxx/atomics/atomics.syn/wait.pass.cpp
@@ -0,0 +1,30 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+// XFAIL: availability-synchronization_library-missing
+// XFAIL: !has-64-bit-atomics
+
+#include <atomic>
+#include <cassert>
+
+#include "test_macros.h"
+
+void test_85107() {
+ // 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 + (1l << 32);
+ std::__cxx_atomic_contention_t ct(new_val);
+ std::__libcpp_atomic_wait(&ct, old_val);
+}
+
+int main(int, char**) {
+ test_85107();
+
+ return 0;
+}
>From 21d3d4ccfe70cabacc6f657bb56e08ea7f6cc5a6 Mon Sep 17 00:00:00 2001
From: Hui <hui.xie0621 at gmail.com>
Date: Mon, 20 May 2024 18:10:48 +0100
Subject: [PATCH 2/8] linux fix
---
.../test/libcxx/atomics/atomics.syn/wait.pass.cpp | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/libcxx/test/libcxx/atomics/atomics.syn/wait.pass.cpp b/libcxx/test/libcxx/atomics/atomics.syn/wait.pass.cpp
index f59db4109bfd5..9a1aa906fd83b 100644
--- a/libcxx/test/libcxx/atomics/atomics.syn/wait.pass.cpp
+++ b/libcxx/test/libcxx/atomics/atomics.syn/wait.pass.cpp
@@ -15,12 +15,14 @@
#include "test_macros.h"
void test_85107() {
- // 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 + (1l << 32);
- std::__cxx_atomic_contention_t ct(new_val);
- std::__libcpp_atomic_wait(&ct, old_val);
+ if constexpr (sizeof(std::__cxx_contention_t) == 8) {
+ // 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 + (1l << 32);
+ std::__cxx_atomic_contention_t ct(new_val);
+ std::__libcpp_atomic_wait(&ct, old_val);
+ }
}
int main(int, char**) {
>From 7a4e0a4a10d98b6f95aaae734207f4222e23b411 Mon Sep 17 00:00:00 2001
From: Hui <hui.xie0621 at gmail.com>
Date: Wed, 22 May 2024 15:15:45 +0100
Subject: [PATCH 3/8] filter no threads
---
libcxx/test/libcxx/atomics/atomics.syn/wait.pass.cpp | 1 +
1 file changed, 1 insertion(+)
diff --git a/libcxx/test/libcxx/atomics/atomics.syn/wait.pass.cpp b/libcxx/test/libcxx/atomics/atomics.syn/wait.pass.cpp
index 9a1aa906fd83b..4f20d9ec625e5 100644
--- a/libcxx/test/libcxx/atomics/atomics.syn/wait.pass.cpp
+++ b/libcxx/test/libcxx/atomics/atomics.syn/wait.pass.cpp
@@ -6,6 +6,7 @@
//
//===----------------------------------------------------------------------===//
// UNSUPPORTED: c++03, c++11, c++14, c++17
+// UNSUPPORTED: no-threads
// XFAIL: availability-synchronization_library-missing
// XFAIL: !has-64-bit-atomics
>From e06e1d58731709d7f0585a5b949c30e6b6c736e4 Mon Sep 17 00:00:00 2001
From: Hui <hui.xie0621 at gmail.com>
Date: Sun, 26 May 2024 12:07:18 +0100
Subject: [PATCH 4/8] address comment
---
libcxx/test/libcxx/atomics/atomics.syn/wait.pass.cpp | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/libcxx/test/libcxx/atomics/atomics.syn/wait.pass.cpp b/libcxx/test/libcxx/atomics/atomics.syn/wait.pass.cpp
index 4f20d9ec625e5..5b5f057a0bb61 100644
--- a/libcxx/test/libcxx/atomics/atomics.syn/wait.pass.cpp
+++ b/libcxx/test/libcxx/atomics/atomics.syn/wait.pass.cpp
@@ -8,13 +8,12 @@
// UNSUPPORTED: c++03, c++11, c++14, c++17
// UNSUPPORTED: no-threads
// XFAIL: availability-synchronization_library-missing
-// XFAIL: !has-64-bit-atomics
+// This bug was first fixed in LLVM 19
+// XFAIL: using-built-library-before-llvm-19
#include <atomic>
#include <cassert>
-#include "test_macros.h"
-
void test_85107() {
if constexpr (sizeof(std::__cxx_contention_t) == 8) {
// https://github.com/llvm/llvm-project/issues/85107
@@ -22,7 +21,7 @@ void test_85107() {
constexpr std::__cxx_contention_t old_val = 0;
constexpr std::__cxx_contention_t new_val = old_val + (1l << 32);
std::__cxx_atomic_contention_t ct(new_val);
- std::__libcpp_atomic_wait(&ct, old_val);
+ std::__libcpp_atomic_wait(&ct, old_val); // this will hang forever if the bug is present
}
}
>From f3913f06f93c515176061fffa32aaa2ee4bd5798 Mon Sep 17 00:00:00 2001
From: Hui <hui.xie0621 at gmail.com>
Date: Wed, 29 May 2024 01:43:09 +0100
Subject: [PATCH 5/8] CI
---
libcxx/test/libcxx/atomics/atomics.syn/wait.pass.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libcxx/test/libcxx/atomics/atomics.syn/wait.pass.cpp b/libcxx/test/libcxx/atomics/atomics.syn/wait.pass.cpp
index 5b5f057a0bb61..f5b8885ee9640 100644
--- a/libcxx/test/libcxx/atomics/atomics.syn/wait.pass.cpp
+++ b/libcxx/test/libcxx/atomics/atomics.syn/wait.pass.cpp
@@ -15,7 +15,7 @@
#include <cassert>
void test_85107() {
- if constexpr (sizeof(std::__cxx_contention_t) == 8) {
+ 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;
>From 08f53f4e464e4f7de7efc096f5ba9d6ca61afef6 Mon Sep 17 00:00:00 2001
From: Hui <hui.xie0621 at gmail.com>
Date: Wed, 29 May 2024 09:39:53 +0100
Subject: [PATCH 6/8] static_assert
---
libcxx/src/atomic.cpp | 2 ++
1 file changed, 2 insertions(+)
diff --git a/libcxx/src/atomic.cpp b/libcxx/src/atomic.cpp
index 031f247f88c2f..8abe820eed5ac 100644
--- a/libcxx/src/atomic.cpp
+++ b/libcxx/src/atomic.cpp
@@ -74,10 +74,12 @@ extern "C" int __ulock_wake(uint32_t operation, void* addr, uint64_t wake_value)
static void
__libcpp_platform_wait_on_address(__cxx_atomic_contention_t const volatile* __ptr, __cxx_contention_t __val) {
+ 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_WAIT64 | (__notify_one ? 0 : ULF_WAKE_ALL), const_cast<__cxx_atomic_contention_t*>(__ptr), 0);
}
>From f903ab99ed6aec66ecf04831e6ea7f65aa501bdd Mon Sep 17 00:00:00 2001
From: Hui <hui.xie0621 at gmail.com>
Date: Wed, 29 May 2024 20:19:41 +0100
Subject: [PATCH 7/8] more CI
---
libcxx/src/atomic.cpp | 1 +
libcxx/test/libcxx/atomics/atomics.syn/wait.pass.cpp | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/libcxx/src/atomic.cpp b/libcxx/src/atomic.cpp
index 8abe820eed5ac..7142f38c96090 100644
--- a/libcxx/src/atomic.cpp
+++ b/libcxx/src/atomic.cpp
@@ -69,6 +69,7 @@ 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);
+// https://github.com/apple/darwin-xnu/blob/2ff845c2e033bd0ff64b5b6aa6063a1f8f65aa32/bsd/sys/ulock.h#L82
# define UL_COMPARE_AND_WAIT64 5
# define ULF_WAKE_ALL 0x00000100
diff --git a/libcxx/test/libcxx/atomics/atomics.syn/wait.pass.cpp b/libcxx/test/libcxx/atomics/atomics.syn/wait.pass.cpp
index f5b8885ee9640..c6495ab4ebcd1 100644
--- a/libcxx/test/libcxx/atomics/atomics.syn/wait.pass.cpp
+++ b/libcxx/test/libcxx/atomics/atomics.syn/wait.pass.cpp
@@ -19,7 +19,7 @@ void test_85107() {
// 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 + (1l << 32);
+ 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
}
>From 184abafe495d748859884e67cfc2a66b854b85d4 Mon Sep 17 00:00:00 2001
From: Hui <hui.xie0621 at gmail.com>
Date: Sat, 1 Jun 2024 17:40:10 +0100
Subject: [PATCH 8/8] more CI
---
libcxx/test/libcxx/atomics/atomics.syn/wait.pass.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/libcxx/test/libcxx/atomics/atomics.syn/wait.pass.cpp b/libcxx/test/libcxx/atomics/atomics.syn/wait.pass.cpp
index c6495ab4ebcd1..f29dc6eb463fe 100644
--- a/libcxx/test/libcxx/atomics/atomics.syn/wait.pass.cpp
+++ b/libcxx/test/libcxx/atomics/atomics.syn/wait.pass.cpp
@@ -7,9 +7,9 @@
//===----------------------------------------------------------------------===//
// UNSUPPORTED: c++03, c++11, c++14, c++17
// UNSUPPORTED: no-threads
-// XFAIL: availability-synchronization_library-missing
// This bug was first fixed in LLVM 19
-// XFAIL: using-built-library-before-llvm-19
+// UNSUPPORTED: using-built-library-before-llvm-19
+// XFAIL: availability-synchronization_library-missing
#include <atomic>
#include <cassert>
More information about the libcxx-commits
mailing list