Skip to content

[libc++] Fix bug in atomic_ref's calculation of lock_free-ness #99570

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion libcxx/include/__atomic/atomic_ref.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
// The purpose of this trick is to make sure that we provide an object with the correct alignment
// to __atomic_is_always_lock_free, since that answer depends on the 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:
Expand Down Expand Up @@ -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_); }

Expand Down
165 changes: 165 additions & 0 deletions libcxx/test/std/atomics/atomics.lockfree/is_always_lock_free.cpp
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this got backported, it's worth noting that this file is apparently not named correctly (missing .pass.cpp), and thus the test doesn't run, see #105966

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the heads up, I'll fix that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Original file line number Diff line number Diff line change
@@ -0,0 +1,165 @@
//===----------------------------------------------------------------------===//
//
// 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

// <atomic>
//
// template <class T>
// class atomic;
//
// static constexpr bool is_always_lock_free;

#include <atomic>
#include <cassert>
#include <cstddef>

#include "test_macros.h"
#include "atomic_helpers.h"

template <typename T>
void check_always_lock_free(std::atomic<T> const& a) {
using InfoT = LockFreeStatusInfo<T>;

constexpr std::same_as<const bool> decltype(auto) is_always_lock_free = std::atomic<T>::is_always_lock_free;

// If we know the status of T for sure, validate the exact result of the function.
if constexpr (InfoT::status_known) {
constexpr LockFreeStatus known_status = InfoT::value;
if constexpr (known_status == LockFreeStatus::always) {
static_assert(is_always_lock_free, "is_always_lock_free is inconsistent with known lock-free status");
assert(a.is_lock_free() && "is_lock_free() is inconsistent with known lock-free status");
} else if constexpr (known_status == LockFreeStatus::never) {
static_assert(!is_always_lock_free, "is_always_lock_free is inconsistent with known lock-free status");
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.
}
}

// In all cases, also sanity-check it based on the implication always-lock-free => lock-free.
if (is_always_lock_free) {
std::same_as<bool> decltype(auto) is_lock_free = a.is_lock_free();
assert(is_lock_free);
}
ASSERT_NOEXCEPT(a.is_lock_free());
}

#define CHECK_ALWAYS_LOCK_FREE(T) \
do { \
typedef T type; \
type obj{}; \
std::atomic<type> a(obj); \
check_always_lock_free(a); \
} while (0)

void test() {
char c = 'x';
check_always_lock_free(std::atomic<char>(c));

int i = 0;
check_always_lock_free(std::atomic<int>(i));

float f = 0.f;
check_always_lock_free(std::atomic<float>(f));

int* p = &i;
check_always_lock_free(std::atomic<int*>(p));

CHECK_ALWAYS_LOCK_FREE(bool);
CHECK_ALWAYS_LOCK_FREE(char);
CHECK_ALWAYS_LOCK_FREE(signed char);
CHECK_ALWAYS_LOCK_FREE(unsigned char);
#if TEST_STD_VER > 17 && defined(__cpp_char8_t)
CHECK_ALWAYS_LOCK_FREE(char8_t);
#endif
CHECK_ALWAYS_LOCK_FREE(char16_t);
CHECK_ALWAYS_LOCK_FREE(char32_t);
CHECK_ALWAYS_LOCK_FREE(wchar_t);
CHECK_ALWAYS_LOCK_FREE(short);
CHECK_ALWAYS_LOCK_FREE(unsigned short);
CHECK_ALWAYS_LOCK_FREE(int);
CHECK_ALWAYS_LOCK_FREE(unsigned int);
CHECK_ALWAYS_LOCK_FREE(long);
CHECK_ALWAYS_LOCK_FREE(unsigned long);
CHECK_ALWAYS_LOCK_FREE(long long);
CHECK_ALWAYS_LOCK_FREE(unsigned long long);
CHECK_ALWAYS_LOCK_FREE(std::nullptr_t);
CHECK_ALWAYS_LOCK_FREE(void*);
CHECK_ALWAYS_LOCK_FREE(float);
CHECK_ALWAYS_LOCK_FREE(double);
CHECK_ALWAYS_LOCK_FREE(long double);
#if __has_attribute(vector_size) && defined(_LIBCPP_VERSION)
CHECK_ALWAYS_LOCK_FREE(int __attribute__((vector_size(1 * sizeof(int)))));
CHECK_ALWAYS_LOCK_FREE(int __attribute__((vector_size(2 * sizeof(int)))));
CHECK_ALWAYS_LOCK_FREE(int __attribute__((vector_size(4 * sizeof(int)))));
CHECK_ALWAYS_LOCK_FREE(int __attribute__((vector_size(16 * sizeof(int)))));
CHECK_ALWAYS_LOCK_FREE(int __attribute__((vector_size(32 * sizeof(int)))));
CHECK_ALWAYS_LOCK_FREE(float __attribute__((vector_size(1 * sizeof(float)))));
CHECK_ALWAYS_LOCK_FREE(float __attribute__((vector_size(2 * sizeof(float)))));
CHECK_ALWAYS_LOCK_FREE(float __attribute__((vector_size(4 * sizeof(float)))));
CHECK_ALWAYS_LOCK_FREE(float __attribute__((vector_size(16 * sizeof(float)))));
CHECK_ALWAYS_LOCK_FREE(float __attribute__((vector_size(32 * sizeof(float)))));
CHECK_ALWAYS_LOCK_FREE(double __attribute__((vector_size(1 * sizeof(double)))));
CHECK_ALWAYS_LOCK_FREE(double __attribute__((vector_size(2 * sizeof(double)))));
CHECK_ALWAYS_LOCK_FREE(double __attribute__((vector_size(4 * sizeof(double)))));
CHECK_ALWAYS_LOCK_FREE(double __attribute__((vector_size(16 * sizeof(double)))));
CHECK_ALWAYS_LOCK_FREE(double __attribute__((vector_size(32 * sizeof(double)))));
#endif // __has_attribute(vector_size) && defined(_LIBCPP_VERSION)
CHECK_ALWAYS_LOCK_FREE(struct Empty{});
CHECK_ALWAYS_LOCK_FREE(struct OneInt { int i; });
CHECK_ALWAYS_LOCK_FREE(struct IntArr2 { int i[2]; });
CHECK_ALWAYS_LOCK_FREE(struct FloatArr3 { float i[3]; });
CHECK_ALWAYS_LOCK_FREE(struct LLIArr2 { long long int i[2]; });
CHECK_ALWAYS_LOCK_FREE(struct LLIArr4 { long long int i[4]; });
CHECK_ALWAYS_LOCK_FREE(struct LLIArr8 { long long int i[8]; });
CHECK_ALWAYS_LOCK_FREE(struct LLIArr16 { long long int i[16]; });
CHECK_ALWAYS_LOCK_FREE(struct Padding {
char c; /* padding */
long long int i;
});
CHECK_ALWAYS_LOCK_FREE(union IntFloat {
int i;
float f;
});
CHECK_ALWAYS_LOCK_FREE(enum class CharEnumClass : char{foo});

// C macro and static constexpr must be consistent.
enum class CharEnumClass : char { foo };
static_assert(std::atomic<bool>::is_always_lock_free == (2 == ATOMIC_BOOL_LOCK_FREE), "");
static_assert(std::atomic<char>::is_always_lock_free == (2 == ATOMIC_CHAR_LOCK_FREE), "");
static_assert(std::atomic<CharEnumClass>::is_always_lock_free == (2 == ATOMIC_CHAR_LOCK_FREE), "");
static_assert(std::atomic<signed char>::is_always_lock_free == (2 == ATOMIC_CHAR_LOCK_FREE), "");
static_assert(std::atomic<unsigned char>::is_always_lock_free == (2 == ATOMIC_CHAR_LOCK_FREE), "");
#if TEST_STD_VER > 17 && defined(__cpp_char8_t)
static_assert(std::atomic<char8_t>::is_always_lock_free == (2 == ATOMIC_CHAR8_T_LOCK_FREE), "");
#endif
static_assert(std::atomic<char16_t>::is_always_lock_free == (2 == ATOMIC_CHAR16_T_LOCK_FREE), "");
static_assert(std::atomic<char32_t>::is_always_lock_free == (2 == ATOMIC_CHAR32_T_LOCK_FREE), "");
static_assert(std::atomic<wchar_t>::is_always_lock_free == (2 == ATOMIC_WCHAR_T_LOCK_FREE), "");
static_assert(std::atomic<short>::is_always_lock_free == (2 == ATOMIC_SHORT_LOCK_FREE), "");
static_assert(std::atomic<unsigned short>::is_always_lock_free == (2 == ATOMIC_SHORT_LOCK_FREE), "");
static_assert(std::atomic<int>::is_always_lock_free == (2 == ATOMIC_INT_LOCK_FREE), "");
static_assert(std::atomic<unsigned int>::is_always_lock_free == (2 == ATOMIC_INT_LOCK_FREE), "");
static_assert(std::atomic<long>::is_always_lock_free == (2 == ATOMIC_LONG_LOCK_FREE), "");
static_assert(std::atomic<unsigned long>::is_always_lock_free == (2 == ATOMIC_LONG_LOCK_FREE), "");
static_assert(std::atomic<long long>::is_always_lock_free == (2 == ATOMIC_LLONG_LOCK_FREE), "");
static_assert(std::atomic<unsigned long long>::is_always_lock_free == (2 == ATOMIC_LLONG_LOCK_FREE), "");
static_assert(std::atomic<void*>::is_always_lock_free == (2 == ATOMIC_POINTER_LOCK_FREE), "");
static_assert(std::atomic<std::nullptr_t>::is_always_lock_free == (2 == ATOMIC_POINTER_LOCK_FREE), "");

#if TEST_STD_VER >= 20
static_assert(std::atomic_signed_lock_free::is_always_lock_free, "");
static_assert(std::atomic_unsigned_lock_free::is_always_lock_free, "");
#endif
}

int main(int, char**) {
test();
return 0;
}
120 changes: 0 additions & 120 deletions libcxx/test/std/atomics/atomics.lockfree/isalwayslockfree.pass.cpp

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@
// UNSUPPORTED: c++03, c++11, c++14, c++17

// <atomic>

//
// template <class T>
// class atomic_ref;
//
// static constexpr bool is_always_lock_free;
// bool is_lock_free() const noexcept;

Expand All @@ -18,10 +21,29 @@
#include <concepts>

#include "test_macros.h"
#include "atomic_helpers.h"

template <typename T>
void check_always_lock_free(std::atomic_ref<T> const a) {
std::same_as<const bool> decltype(auto) is_always_lock_free = std::atomic_ref<T>::is_always_lock_free;
void check_always_lock_free(std::atomic_ref<T> const& a) {
using InfoT = LockFreeStatusInfo<T>;

constexpr std::same_as<const bool> decltype(auto) is_always_lock_free = std::atomic_ref<T>::is_always_lock_free;

// If we know the status of T for sure, validate the exact result of the function.
if constexpr (InfoT::status_known) {
constexpr LockFreeStatus known_status = InfoT::value;
if constexpr (known_status == LockFreeStatus::always) {
static_assert(is_always_lock_free, "is_always_lock_free is inconsistent with known lock-free status");
assert(a.is_lock_free() && "is_lock_free() is inconsistent with known lock-free status");
} else if constexpr (known_status == LockFreeStatus::never) {
static_assert(!is_always_lock_free, "is_always_lock_free is inconsistent with known lock-free status");
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.
}
}

// In all cases, also sanity-check it based on the implication always-lock-free => lock-free.
if (is_always_lock_free) {
std::same_as<bool> decltype(auto) is_lock_free = a.is_lock_free();
assert(is_lock_free);
Expand All @@ -33,10 +55,14 @@ void check_always_lock_free(std::atomic_ref<T> const a) {
do { \
typedef T type; \
type obj{}; \
check_always_lock_free(std::atomic_ref<type>(obj)); \
std::atomic_ref<type> a(obj); \
check_always_lock_free(a); \
} while (0)

void test() {
char c = 'x';
check_always_lock_free(std::atomic_ref<char>(c));

int i = 0;
check_always_lock_free(std::atomic_ref<int>(i));

Expand Down
Loading
Loading