From 155beb97492e14e29ab7af3a805bbfb97bee1e6b Mon Sep 17 00:00:00 2001 From: Hui Date: Thu, 12 Feb 2026 19:14:18 +0000 Subject: [PATCH] [libc++] Fix native wait alignment (#180928) This PR fixes two issues regarding the alignment of native wait: - In the internal platform call, the local variable is copied from a potentially non-aligned buffer - Under the unstable ABI, the predicate to test eligibility of a type being able to do native wait is purely on size. We should test also the alignment of such type is qualified for platform call --------- Co-authored-by: Louis Dionne --- .../include/__atomic/atomic_waitable_traits.h | 9 ++-- libcxx/src/atomic.cpp | 15 ++++--- .../atomics.syn/wait.native.compile.pass.cpp | 45 +++++++++++++++++++ 3 files changed, 60 insertions(+), 9 deletions(-) create mode 100644 libcxx/test/libcxx/atomics/atomics.syn/wait.native.compile.pass.cpp diff --git a/libcxx/include/__atomic/atomic_waitable_traits.h b/libcxx/include/__atomic/atomic_waitable_traits.h index 72ae4b76294d..369cd2c52b83 100644 --- a/libcxx/include/__atomic/atomic_waitable_traits.h +++ b/libcxx/include/__atomic/atomic_waitable_traits.h @@ -67,8 +67,11 @@ concept __atomic_waitable = requires(const _Tp __t, memory_order __order) { # if defined(_LIBCPP_ABI_ATOMIC_WAIT_NATIVE_BY_SIZE) -_LIBCPP_HIDE_FROM_ABI constexpr bool __has_native_atomic_wait_impl(size_t __size) { - switch (__size) { +template +_LIBCPP_HIDE_FROM_ABI constexpr bool __has_native_atomic_wait_impl() { + if (alignof(_Tp) % sizeof(_Tp) != 0) + return false; + switch (sizeof(_Tp)) { # define _LIBCPP_MAKE_CASE(n) \ case n: \ return true; @@ -82,7 +85,7 @@ _LIBCPP_HIDE_FROM_ABI constexpr bool __has_native_atomic_wait_impl(size_t __size template concept __has_native_atomic_wait = has_unique_object_representations_v<_Tp> && is_trivially_copyable_v<_Tp> && - __has_native_atomic_wait_impl(sizeof(_Tp)); + std::__has_native_atomic_wait_impl<_Tp>(); # else // _LIBCPP_ABI_ATOMIC_WAIT_NATIVE_BY_SIZE diff --git a/libcxx/src/atomic.cpp b/libcxx/src/atomic.cpp index 637c971ed889..1277b20caf64 100644 --- a/libcxx/src/atomic.cpp +++ b/libcxx/src/atomic.cpp @@ -66,7 +66,7 @@ _LIBCPP_BEGIN_NAMESPACE_STD template static void __platform_wait_on_address(void const* __ptr, void const* __val, uint64_t __timeout_ns) { static_assert(_Size == 4, "Can only wait on 4 bytes value"); - char buffer[_Size]; + alignas(__cxx_contention_t) char buffer[_Size]; std::memcpy(&buffer, const_cast(__val), _Size); static constexpr timespec __default_timeout = {2, 0}; timespec __timeout; @@ -99,15 +99,18 @@ extern "C" int __ulock_wake(uint32_t operation, void* addr, uint64_t wake_value) template static void __platform_wait_on_address(void const* __ptr, void const* __val, uint64_t __timeout_ns) { static_assert(_Size == 8 || _Size == 4, "Can only wait on 8 bytes or 4 bytes value"); - char buffer[_Size]; - std::memcpy(&buffer, const_cast(__val), _Size); auto __timeout_us = __timeout_ns == 0 ? 0 : static_cast(__timeout_ns / 1000); - if constexpr (_Size == 4) + if constexpr (_Size == 4) { + alignas(uint32_t) char buffer[_Size]; + std::memcpy(&buffer, const_cast(__val), _Size); __ulock_wait( UL_COMPARE_AND_WAIT, const_cast(__ptr), *reinterpret_cast(&buffer), __timeout_us); - else + } else { + alignas(uint64_t) char buffer[_Size]; + std::memcpy(&buffer, const_cast(__val), _Size); __ulock_wait( UL_COMPARE_AND_WAIT64, const_cast(__ptr), *reinterpret_cast(&buffer), __timeout_us); + } } template @@ -130,7 +133,7 @@ static void __platform_wake_by_address(void const* __ptr, bool __notify_one) { template static void __platform_wait_on_address(void const* __ptr, void const* __val, uint64_t __timeout_ns) { static_assert(_Size == 8, "Can only wait on 8 bytes value"); - char buffer[_Size]; + alignas(__cxx_contention_t) char buffer[_Size]; std::memcpy(&buffer, const_cast(__val), _Size); if (__timeout_ns == 0) { _umtx_op(const_cast(__ptr), UMTX_OP_WAIT, *reinterpret_cast<__cxx_contention_t*>(&buffer), nullptr, nullptr); diff --git a/libcxx/test/libcxx/atomics/atomics.syn/wait.native.compile.pass.cpp b/libcxx/test/libcxx/atomics/atomics.syn/wait.native.compile.pass.cpp new file mode 100644 index 000000000000..88dd2b58b870 --- /dev/null +++ b/libcxx/test/libcxx/atomics/atomics.syn/wait.native.compile.pass.cpp @@ -0,0 +1,45 @@ +//===----------------------------------------------------------------------===// +// +// 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 + +// When __has_native_atomic_wait is true, the atomic object's address will be directly passed to the platform's wait. +// This test ensures that types that do not satisfy the platform's wait requirement should have __has_native_atomic_wait be false. + +#include +#include +#include + +template +struct alignas(Align) Data { + char buffer[Size]; +}; + +static_assert(std::__has_native_atomic_wait); + +#if defined(_LIBCPP_ABI_ATOMIC_WAIT_NATIVE_BY_SIZE) && defined(__APPLE__) + +static_assert(std::__has_native_atomic_wait>); +static_assert(std::__has_native_atomic_wait>); + +static_assert(!std::has_unique_object_representations_v>); +static_assert(!std::__has_native_atomic_wait>, + "Object with !has_unique_object_representations_v should not have native wait"); + +static_assert(!std::__has_native_atomic_wait>, "Should only support native wait for 4 and 8 byte types"); + +// `__ulock_wait` requires the address is aligned to the requested size (4 or 8) + +static_assert(!std::__has_native_atomic_wait>, + "Should only support native wait for types with properly aligned types"); + +static_assert(!std::__has_native_atomic_wait>, + "Should only support native wait for types with properly aligned types"); + +#endif