Skip to content

Commit cc1dfb3

Browse files
ldionneEricWF
ldionne
and
EricWF
authored
[libc++] Fix bug in atomic_ref's calculation of lock_free-ness (llvm#99570)
The builtin __atomic_always_lock_free takes into account the type of the pointer provided as the second argument. Because we were passing void*, rather than T*, the calculation failed. This meant that atomic_ref<T>::is_always_lock_free was only true for char & bool. This bug exists elsewhere in the atomic library (when using GCC, we fail to pass a pointer at all, and we fail to correctly align the atomic like _Atomic would). This change also attempts to start sorting out testing difficulties with this function that caused the bug to exist by using the __GCC_ATOMIC_(CHAR|SHORT|INT|LONG|LLONG|POINTER)_IS_LOCK_FREE predefined macros to establish an expected value for `is_always_lock_free` and `is_lock_free` for the respective types, as well as types with matching sizes and compatible alignment values. Using these compiler pre-defines we can actually validate that certain types, like char and int, are actually always lock free like they are on every platform in the wild. Note that this patch was actually authored by Eric Fiselier but I picked up the patch and GitHub won't let me set Eric as the primary author. Co-authored-by: Eric Fiselier <[email protected]>
1 parent 761372e commit cc1dfb3

File tree

5 files changed

+312
-125
lines changed

5 files changed

+312
-125
lines changed

‎libcxx/include/__atomic/atomic_ref.h

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,19 @@ _LIBCPP_BEGIN_NAMESPACE_STD
4242

4343
#if _LIBCPP_STD_VER >= 20
4444

45+
// These types are required to make __atomic_is_always_lock_free work across GCC and Clang.
46+
// The purpose of this trick is to make sure that we provide an object with the correct alignment
47+
// to __atomic_is_always_lock_free, since that answer depends on the alignment.
48+
template <size_t _Alignment>
49+
struct __alignment_checker_type {
50+
alignas(_Alignment) char __data;
51+
};
52+
53+
template <size_t _Alignment>
54+
struct __get_aligner_instance {
55+
static constexpr __alignment_checker_type<_Alignment> __instance{};
56+
};
57+
4558
template <class _Tp>
4659
struct __atomic_ref_base {
4760
protected:
@@ -105,7 +118,7 @@ struct __atomic_ref_base {
105118
// that the pointer is going to be aligned properly at runtime because that is a (checked) precondition
106119
// of atomic_ref's constructor.
107120
static constexpr bool is_always_lock_free =
108-
__atomic_always_lock_free(sizeof(_Tp), reinterpret_cast<void*>(-required_alignment));
121+
__atomic_always_lock_free(sizeof(_Tp), &__get_aligner_instance<required_alignment>::__instance);
109122

110123
_LIBCPP_HIDE_FROM_ABI bool is_lock_free() const noexcept { return __atomic_is_lock_free(sizeof(_Tp), __ptr_); }
111124

‎libcxx/test/std/atomics/atomics.lockfree/is_always_lock_free.cpp

Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
//
9+
// UNSUPPORTED: c++03, c++11, c++14
10+
11+
// <atomic>
12+
//
13+
// template <class T>
14+
// class atomic;
15+
//
16+
// static constexpr bool is_always_lock_free;
17+
18+
#include <atomic>
19+
#include <cassert>
20+
#include <cstddef>
21+
22+
#include "test_macros.h"
23+
#include "atomic_helpers.h"
24+
25+
template <typename T>
26+
void check_always_lock_free(std::atomic<T> const& a) {
27+
using InfoT = LockFreeStatusInfo<T>;
28+
29+
constexpr std::same_as<const bool> decltype(auto) is_always_lock_free = std::atomic<T>::is_always_lock_free;
30+
31+
// If we know the status of T for sure, validate the exact result of the function.
32+
if constexpr (InfoT::status_known) {
33+
constexpr LockFreeStatus known_status = InfoT::value;
34+
if constexpr (known_status == LockFreeStatus::always) {
35+
static_assert(is_always_lock_free, "is_always_lock_free is inconsistent with known lock-free status");
36+
assert(a.is_lock_free() && "is_lock_free() is inconsistent with known lock-free status");
37+
} else if constexpr (known_status == LockFreeStatus::never) {
38+
static_assert(!is_always_lock_free, "is_always_lock_free is inconsistent with known lock-free status");
39+
assert(!a.is_lock_free() && "is_lock_free() is inconsistent with known lock-free status");
40+
} else {
41+
assert(a.is_lock_free() || !a.is_lock_free()); // This is kinda dumb, but we might as well call the function once.
42+
}
43+
}
44+
45+
// In all cases, also sanity-check it based on the implication always-lock-free => lock-free.
46+
if (is_always_lock_free) {
47+
std::same_as<bool> decltype(auto) is_lock_free = a.is_lock_free();
48+
assert(is_lock_free);
49+
}
50+
ASSERT_NOEXCEPT(a.is_lock_free());
51+
}
52+
53+
#define CHECK_ALWAYS_LOCK_FREE(T) \
54+
do { \
55+
typedef T type; \
56+
type obj{}; \
57+
std::atomic<type> a(obj); \
58+
check_always_lock_free(a); \
59+
} while (0)
60+
61+
void test() {
62+
char c = 'x';
63+
check_always_lock_free(std::atomic<char>(c));
64+
65+
int i = 0;
66+
check_always_lock_free(std::atomic<int>(i));
67+
68+
float f = 0.f;
69+
check_always_lock_free(std::atomic<float>(f));
70+
71+
int* p = &i;
72+
check_always_lock_free(std::atomic<int*>(p));
73+
74+
CHECK_ALWAYS_LOCK_FREE(bool);
75+
CHECK_ALWAYS_LOCK_FREE(char);
76+
CHECK_ALWAYS_LOCK_FREE(signed char);
77+
CHECK_ALWAYS_LOCK_FREE(unsigned char);
78+
#if TEST_STD_VER > 17 && defined(__cpp_char8_t)
79+
CHECK_ALWAYS_LOCK_FREE(char8_t);
80+
#endif
81+
CHECK_ALWAYS_LOCK_FREE(char16_t);
82+
CHECK_ALWAYS_LOCK_FREE(char32_t);
83+
CHECK_ALWAYS_LOCK_FREE(wchar_t);
84+
CHECK_ALWAYS_LOCK_FREE(short);
85+
CHECK_ALWAYS_LOCK_FREE(unsigned short);
86+
CHECK_ALWAYS_LOCK_FREE(int);
87+
CHECK_ALWAYS_LOCK_FREE(unsigned int);
88+
CHECK_ALWAYS_LOCK_FREE(long);
89+
CHECK_ALWAYS_LOCK_FREE(unsigned long);
90+
CHECK_ALWAYS_LOCK_FREE(long long);
91+
CHECK_ALWAYS_LOCK_FREE(unsigned long long);
92+
CHECK_ALWAYS_LOCK_FREE(std::nullptr_t);
93+
CHECK_ALWAYS_LOCK_FREE(void*);
94+
CHECK_ALWAYS_LOCK_FREE(float);
95+
CHECK_ALWAYS_LOCK_FREE(double);
96+
CHECK_ALWAYS_LOCK_FREE(long double);
97+
#if __has_attribute(vector_size) && defined(_LIBCPP_VERSION)
98+
CHECK_ALWAYS_LOCK_FREE(int __attribute__((vector_size(1 * sizeof(int)))));
99+
CHECK_ALWAYS_LOCK_FREE(int __attribute__((vector_size(2 * sizeof(int)))));
100+
CHECK_ALWAYS_LOCK_FREE(int __attribute__((vector_size(4 * sizeof(int)))));
101+
CHECK_ALWAYS_LOCK_FREE(int __attribute__((vector_size(16 * sizeof(int)))));
102+
CHECK_ALWAYS_LOCK_FREE(int __attribute__((vector_size(32 * sizeof(int)))));
103+
CHECK_ALWAYS_LOCK_FREE(float __attribute__((vector_size(1 * sizeof(float)))));
104+
CHECK_ALWAYS_LOCK_FREE(float __attribute__((vector_size(2 * sizeof(float)))));
105+
CHECK_ALWAYS_LOCK_FREE(float __attribute__((vector_size(4 * sizeof(float)))));
106+
CHECK_ALWAYS_LOCK_FREE(float __attribute__((vector_size(16 * sizeof(float)))));
107+
CHECK_ALWAYS_LOCK_FREE(float __attribute__((vector_size(32 * sizeof(float)))));
108+
CHECK_ALWAYS_LOCK_FREE(double __attribute__((vector_size(1 * sizeof(double)))));
109+
CHECK_ALWAYS_LOCK_FREE(double __attribute__((vector_size(2 * sizeof(double)))));
110+
CHECK_ALWAYS_LOCK_FREE(double __attribute__((vector_size(4 * sizeof(double)))));
111+
CHECK_ALWAYS_LOCK_FREE(double __attribute__((vector_size(16 * sizeof(double)))));
112+
CHECK_ALWAYS_LOCK_FREE(double __attribute__((vector_size(32 * sizeof(double)))));
113+
#endif // __has_attribute(vector_size) && defined(_LIBCPP_VERSION)
114+
CHECK_ALWAYS_LOCK_FREE(struct Empty{});
115+
CHECK_ALWAYS_LOCK_FREE(struct OneInt { int i; });
116+
CHECK_ALWAYS_LOCK_FREE(struct IntArr2 { int i[2]; });
117+
CHECK_ALWAYS_LOCK_FREE(struct FloatArr3 { float i[3]; });
118+
CHECK_ALWAYS_LOCK_FREE(struct LLIArr2 { long long int i[2]; });
119+
CHECK_ALWAYS_LOCK_FREE(struct LLIArr4 { long long int i[4]; });
120+
CHECK_ALWAYS_LOCK_FREE(struct LLIArr8 { long long int i[8]; });
121+
CHECK_ALWAYS_LOCK_FREE(struct LLIArr16 { long long int i[16]; });
122+
CHECK_ALWAYS_LOCK_FREE(struct Padding {
123+
char c; /* padding */
124+
long long int i;
125+
});
126+
CHECK_ALWAYS_LOCK_FREE(union IntFloat {
127+
int i;
128+
float f;
129+
});
130+
CHECK_ALWAYS_LOCK_FREE(enum class CharEnumClass : char{foo});
131+
132+
// C macro and static constexpr must be consistent.
133+
enum class CharEnumClass : char { foo };
134+
static_assert(std::atomic<bool>::is_always_lock_free == (2 == ATOMIC_BOOL_LOCK_FREE), "");
135+
static_assert(std::atomic<char>::is_always_lock_free == (2 == ATOMIC_CHAR_LOCK_FREE), "");
136+
static_assert(std::atomic<CharEnumClass>::is_always_lock_free == (2 == ATOMIC_CHAR_LOCK_FREE), "");
137+
static_assert(std::atomic<signed char>::is_always_lock_free == (2 == ATOMIC_CHAR_LOCK_FREE), "");
138+
static_assert(std::atomic<unsigned char>::is_always_lock_free == (2 == ATOMIC_CHAR_LOCK_FREE), "");
139+
#if TEST_STD_VER > 17 && defined(__cpp_char8_t)
140+
static_assert(std::atomic<char8_t>::is_always_lock_free == (2 == ATOMIC_CHAR8_T_LOCK_FREE), "");
141+
#endif
142+
static_assert(std::atomic<char16_t>::is_always_lock_free == (2 == ATOMIC_CHAR16_T_LOCK_FREE), "");
143+
static_assert(std::atomic<char32_t>::is_always_lock_free == (2 == ATOMIC_CHAR32_T_LOCK_FREE), "");
144+
static_assert(std::atomic<wchar_t>::is_always_lock_free == (2 == ATOMIC_WCHAR_T_LOCK_FREE), "");
145+
static_assert(std::atomic<short>::is_always_lock_free == (2 == ATOMIC_SHORT_LOCK_FREE), "");
146+
static_assert(std::atomic<unsigned short>::is_always_lock_free == (2 == ATOMIC_SHORT_LOCK_FREE), "");
147+
static_assert(std::atomic<int>::is_always_lock_free == (2 == ATOMIC_INT_LOCK_FREE), "");
148+
static_assert(std::atomic<unsigned int>::is_always_lock_free == (2 == ATOMIC_INT_LOCK_FREE), "");
149+
static_assert(std::atomic<long>::is_always_lock_free == (2 == ATOMIC_LONG_LOCK_FREE), "");
150+
static_assert(std::atomic<unsigned long>::is_always_lock_free == (2 == ATOMIC_LONG_LOCK_FREE), "");
151+
static_assert(std::atomic<long long>::is_always_lock_free == (2 == ATOMIC_LLONG_LOCK_FREE), "");
152+
static_assert(std::atomic<unsigned long long>::is_always_lock_free == (2 == ATOMIC_LLONG_LOCK_FREE), "");
153+
static_assert(std::atomic<void*>::is_always_lock_free == (2 == ATOMIC_POINTER_LOCK_FREE), "");
154+
static_assert(std::atomic<std::nullptr_t>::is_always_lock_free == (2 == ATOMIC_POINTER_LOCK_FREE), "");
155+
156+
#if TEST_STD_VER >= 20
157+
static_assert(std::atomic_signed_lock_free::is_always_lock_free, "");
158+
static_assert(std::atomic_unsigned_lock_free::is_always_lock_free, "");
159+
#endif
160+
}
161+
162+
int main(int, char**) {
163+
test();
164+
return 0;
165+
}

‎libcxx/test/std/atomics/atomics.lockfree/isalwayslockfree.pass.cpp

Lines changed: 0 additions & 120 deletions
This file was deleted.

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

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,10 @@
99
// UNSUPPORTED: c++03, c++11, c++14, c++17
1010

1111
// <atomic>
12-
12+
//
13+
// template <class T>
14+
// class atomic_ref;
15+
//
1316
// static constexpr bool is_always_lock_free;
1417
// bool is_lock_free() const noexcept;
1518

@@ -18,10 +21,29 @@
1821
#include <concepts>
1922

2023
#include "test_macros.h"
24+
#include "atomic_helpers.h"
2125

2226
template <typename T>
23-
void check_always_lock_free(std::atomic_ref<T> const a) {
24-
std::same_as<const bool> decltype(auto) is_always_lock_free = std::atomic_ref<T>::is_always_lock_free;
27+
void check_always_lock_free(std::atomic_ref<T> const& a) {
28+
using InfoT = LockFreeStatusInfo<T>;
29+
30+
constexpr std::same_as<const bool> decltype(auto) is_always_lock_free = std::atomic_ref<T>::is_always_lock_free;
31+
32+
// If we know the status of T for sure, validate the exact result of the function.
33+
if constexpr (InfoT::status_known) {
34+
constexpr LockFreeStatus known_status = InfoT::value;
35+
if constexpr (known_status == LockFreeStatus::always) {
36+
static_assert(is_always_lock_free, "is_always_lock_free is inconsistent with known lock-free status");
37+
assert(a.is_lock_free() && "is_lock_free() is inconsistent with known lock-free status");
38+
} else if constexpr (known_status == LockFreeStatus::never) {
39+
static_assert(!is_always_lock_free, "is_always_lock_free is inconsistent with known lock-free status");
40+
assert(!a.is_lock_free() && "is_lock_free() is inconsistent with known lock-free status");
41+
} else {
42+
assert(a.is_lock_free() || !a.is_lock_free()); // This is kinda dumb, but we might as well call the function once.
43+
}
44+
}
45+
46+
// In all cases, also sanity-check it based on the implication always-lock-free => lock-free.
2547
if (is_always_lock_free) {
2648
std::same_as<bool> decltype(auto) is_lock_free = a.is_lock_free();
2749
assert(is_lock_free);
@@ -33,10 +55,14 @@ void check_always_lock_free(std::atomic_ref<T> const a) {
3355
do { \
3456
typedef T type; \
3557
type obj{}; \
36-
check_always_lock_free(std::atomic_ref<type>(obj)); \
58+
std::atomic_ref<type> a(obj); \
59+
check_always_lock_free(a); \
3760
} while (0)
3861

3962
void test() {
63+
char c = 'x';
64+
check_always_lock_free(std::atomic_ref<char>(c));
65+
4066
int i = 0;
4167
check_always_lock_free(std::atomic_ref<int>(i));
4268

0 commit comments

Comments
 (0)