Fix bug in atomic_ref's calculation of lock_free-ness.

libcxx/include/__atomic/atomic_ref.h

@@ -42,6 +42,19 @@ _LIBCPP_BEGIN_NAMESPACE_STD
 
 #if _LIBCPP_STD_VER >= 20
 
+// These types are required to make __atomic_is_always_lock_free work across GCC and Clang.
+// GCC won't allow the reinterpret_cast<_Tp*>(-required_alignment) trick initially used,
+// so we need to actually fake up an instance of a type with the correct alignment.
+template <size_t _Alignment>
+struct __alignment_checker_type {
+  alignas(_Alignment) char __data;
+};
+
+template <size_t _Alignment>
+struct __get_aligner_instance {
+  static constexpr __alignment_checker_type<_Alignment> __instance{};
+};
+
 template <class _Tp>
 struct __atomic_ref_base {
 protected:
@@ -105,7 +118,7 @@ struct __atomic_ref_base {
   // that the pointer is going to be aligned properly at runtime because that is a (checked) precondition
   // of atomic_ref's constructor.
   static constexpr bool is_always_lock_free =
-      __atomic_always_lock_free(sizeof(_Tp), reinterpret_cast<void*>(-required_alignment));
+      __atomic_always_lock_free(sizeof(_Tp), &__get_aligner_instance<required_alignment>::__instance);
 
   _LIBCPP_HIDE_FROM_ABI bool is_lock_free() const noexcept { return __atomic_is_lock_free(sizeof(_Tp), __ptr_); }
 

libcxx/test/std/atomics/atomics.ref/is_always_lock_free.pass.cpp

@@ -18,9 +18,25 @@
 #include <concepts>
 
 #include "test_macros.h"
+#include "atomic_helpers.h"
 
 template <typename T>
-void check_always_lock_free(std::atomic_ref<T> const a) {
+void check_always_lock_free(std::atomic_ref<T> const& a) {
+  using InfoT = LockFreeStatusInfo<T>;
+
+  if (InfoT::status_known) {
+    constexpr LockFreeStatus known_status = InfoT::value;
+
+    static_assert(std::atomic_ref<T>::is_always_lock_free == (known_status == LockFreeStatus::always),
+                  "is_always_lock_free is inconsistent with known lock-free status");
+    if (known_status == LockFreeStatus::always) {
+      assert(a.is_lock_free() && "is_lock_free() is inconsistent with known lock-free status");
+    } else if (known_status == LockFreeStatus::never) {
+      assert(!a.is_lock_free() && "is_lock_free() is inconsistent with known lock-free status");
+    } else {
+      assert(a.is_lock_free() || !a.is_lock_free()); // This is kinda dumb, but we might as well call the function once.
+    }
+  }
   std::same_as<const bool> decltype(auto) is_always_lock_free = std::atomic_ref<T>::is_always_lock_free;
   if (is_always_lock_free) {
     std::same_as<bool> decltype(auto) is_lock_free = a.is_lock_free();
@@ -36,7 +52,17 @@ void check_always_lock_free(std::atomic_ref<T> const a) {
     check_always_lock_free(std::atomic_ref<type>(obj));                                                                \
   } while (0)
 
+void check_always_lock_free_types() {
+  static_assert(std::atomic_ref<int>::is_always_lock_free);
+  static_assert(std::atomic_ref<char>::is_always_lock_free);
+}
+
 void test() {
+  // While it's hard to portably test the value of is_always_lock_free, since different platforms have different support
+  // for atomic operations, it's still very important to do so. Specifically, it's important to have at least
+  // a few tests that have expected values.
+  check_always_lock_free_types();
+
   int i = 0;
   check_always_lock_free(std::atomic_ref<int>(i));
 

libcxx/test/support/atomic_helpers.h

@@ -11,9 +11,104 @@
 
 #include <cassert>
 #include <cstdint>
+#include <cstddef>
+#include <type_traits>
 
 #include "test_macros.h"
 
+#if defined(TEST_COMPILER_CLANG)
+#  define TEST_ATOMIC_CHAR_LOCK_FREE __CLANG_ATOMIC_CHAR_LOCK_FREE
+#  define TEST_ATOMIC_SHORT_LOCK_FREE __CLANG_ATOMIC_SHORT_LOCK_FREE
+#  define TEST_ATOMIC_INT_LOCK_FREE __CLANG_ATOMIC_INT_LOCK_FREE
+#  define TEST_ATOMIC_LONG_LOCK_FREE __CLANG_ATOMIC_LONG_LOCK_FREE
+#  define TEST_ATOMIC_LLONG_LOCK_FREE __CLANG_ATOMIC_LLONG_LOCK_FREE
+#  define TEST_ATOMIC_POINTER_LOCK_FREE __CLANG_ATOMIC_POINTER_LOCK_FREE
+#elif defined(TEST_COMPILER_GCC)
+#  define TEST_ATOMIC_CHAR_LOCK_FREE __GCC_ATOMIC_CHAR_LOCK_FREE
+#  define TEST_ATOMIC_SHORT_LOCK_FREE __GCC_ATOMIC_SHORT_LOCK_FREE
+#  define TEST_ATOMIC_INT_LOCK_FREE __GCC_ATOMIC_INT_LOCK_FREE
+#  define TEST_ATOMIC_LONG_LOCK_FREE __GCC_ATOMIC_LONG_LOCK_FREE
+#  define TEST_ATOMIC_LLONG_LOCK_FREE __GCC_ATOMIC_LLONG_LOCK_FREE
+#  define TEST_ATOMIC_POINTER_LOCK_FREE __GCC_ATOMIC_POINTER_LOCK_FREE
+#elif TEST_COMPILER_MSVC
+// This is lifted from STL/stl/inc/atomic on github for the purposes of
+// keeping the tests compiling for MSVC's STL. It's not a perfect solution
+// but at least the tests will keep running.
+//
+// Note MSVC's STL never produces a type that is sometimes lock free, but not always lock free.
+template <class T, size_t Size = sizeof(T)>
+constexpr bool msvc_is_lock_free_macro_value() {
+  return (Size <= 8 && (Size & Size - 1) == 0) ? 2 : 0;
+}
+#  define TEST_ATOMIC_CHAR_LOCK_FREE ::msvc_is_lock_free_macro_value<char>()
+#  define TEST_ATOMIC_SHORT_LOCK_FREE ::msvc_is_lock_free_macro_value<short>()
+#  define TEST_ATOMIC_INT_LOCK_FREE ::msvc_is_lock_free_macro_value<int>()
+#  define TEST_ATOMIC_LONG_LOCK_FREE ::msvc_is_lock_free_macro_value<long>()
+#  define TEST_ATOMIC_LLONG_LOCK_FREE ::msvc_is_lock_free_macro_value<long long>()
+#  define TEST_ATOMIC_POINTER_LOCK_FREE ::msvc_is_lock_free_macro_value<void*>()
+#else
+#  error "Unknown compiler"
+#endif
+
+#ifdef TEST_COMPILER_CLANG
+#  pragma clang diagnostic push
+#  pragma clang diagnostic ignored "-Wc++11-extensions"
+#endif
+// The entire LockFreeStatus/LockFreeStatusEnum/LockFreeStatusType exists entirely to work around the support
+// for C++03, which many of our atomic tests run under. This is a bit of a hack, but it's the best we can do.
+//
+// We could limit the testing involving these things to C++11 or greater? But test coverage in C++03 seems important too.
+
+enum class LockFreeStatus : int { unknown = -1, never = 0, sometimes = 1, always = 2 };
+#define COMPARE_TYPES(T1, T2) (sizeof(T1) == sizeof(T2) && TEST_ALIGNOF(T1) >= TEST_ALIGNOF(T2))
+
+template <class T>
+struct LockFreeStatusInfo {
+  static const LockFreeStatus value = LockFreeStatus(
+      COMPARE_TYPES(T, char)
+          ? TEST_ATOMIC_CHAR_LOCK_FREE
+          : (COMPARE_TYPES(T, short)
+                 ? TEST_ATOMIC_SHORT_LOCK_FREE
+                 : (COMPARE_TYPES(T, int)
+                        ? TEST_ATOMIC_INT_LOCK_FREE
+                        : (COMPARE_TYPES(T, long)
+                               ? TEST_ATOMIC_LONG_LOCK_FREE
+                               : (COMPARE_TYPES(T, long long)
+                                      ? TEST_ATOMIC_LLONG_LOCK_FREE
+                                      : (COMPARE_TYPES(T, void*) ? TEST_ATOMIC_POINTER_LOCK_FREE : -1))))));
+
+  static const bool status_known = LockFreeStatusInfo::value != LockFreeStatus::unknown;
+};
+
+// IDK why this blows up in C++03, but it does. So we'll just disable it.
+#if TEST_STD_VER >= 11
+static_assert(LockFreeStatusInfo<char>::status_known, "");
+static_assert(LockFreeStatusInfo<short>::status_known, "");
+static_assert(LockFreeStatusInfo<int>::status_known, "");
+static_assert(LockFreeStatusInfo<long>::status_known, "");
+static_assert(LockFreeStatusInfo<long long>::status_known, "");
+static_assert(LockFreeStatusInfo<void*>::status_known, "");
+
+// I think these are always supposed to be lock free, and it's worth trying to hardcode expected values.
+static_assert(LockFreeStatusInfo<char>::value == LockFreeStatus::always, "");
+static_assert(LockFreeStatusInfo<short>::value == LockFreeStatus::always, "");
+static_assert(LockFreeStatusInfo<int>::value == LockFreeStatus::always,
+              ""); // This one may not always be lock free, but we'll let the CI decide.
+#endif
+
+// These macros are somewhat suprising to use, since they take the values 0, 1, or 2.
+// To make the tests clearer, get rid of them in preference of AtomicInfo.
+#undef TEST_ATOMIC_CHAR_LOCK_FREE
+#undef TEST_ATOMIC_SHORT_LOCK_FREE
+#undef TEST_ATOMIC_INT_LOCK_FREE
+#undef TEST_ATOMIC_LONG_LOCK_FREE
+#undef TEST_ATOMIC_LLONG_LOCK_FREE
+#undef TEST_ATOMIC_POINTER_LOCK_FREE
+
+#ifdef TEST_COMPILER_CLANG
+#  pragma clang diagnostic pop
+#endif
+
 struct UserAtomicType {
   int i;
 
@@ -64,6 +159,17 @@ struct LargeUserAtomicType {
   }
 };
 
+template <template <class TestArg> class TestFunctor>
+struct TestEachLockFreeKnownIntegralType {
+  void operator()() const {
+    TestFunctor<char>()();
+    TestFunctor<short>()();
+    TestFunctor<int>()();
+    TestFunctor<long long>()();
+    TestFunctor<void*>()();
+  }
+};
+
 template <template <class TestArg> class TestFunctor>
 struct TestEachIntegralType {
   void operator()() const {