[libcxx-commits] [libcxx] [libc++] Fix bug in atomic_ref's calculation of lock_free-ness (PR #99570)

Louis Dionne via libcxx-commits libcxx-commits at lists.llvm.org
Fri Jul 19 08:00:24 PDT 2024


ldionne wrote:

> Re the android failure -- this isn't an android-specific issue, it for any x86-32 platform. The issue is that `__GCC_ATOMIC_LLONG_LOCK_FREE == 2`, while `sizeof(long long) == 8` but `alignof(long long) == 4`.
> 
> So, then, the test thinks that any 8-byte but 4-byte-aligned value is lock-free. But it's not. The macro is, (misleadingly, considering the name), _not_ promising that operations on `long long*` are lock-free, rather, that operations on _aligned_ `sizeof(long long)` sized objects are.

Thanks, this is super useful. So IIUC the problem is basically in our test suite in https://github.com/llvm/llvm-project/pull/99570/files#diff-0c1e385ae099192c3c58151d22e7bf27e60b32e257d5cd24bd179f424c7bc7b7R61 when we do:

```c++
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;
 };
```

For `struct { int x[2]; }`, we get to `COMPARE_TYPES(T, long long)`, and since:
- `sizeof(struct) == sizeof(long long) == 8`
- `alignof(struct) == 4` and `alignof(long long) == 4`

we then conclude that the `struct` should always be lock free via `TEST_ATOMIC_LLONG_LOCK_FREE` (which is really `__CLANG_ATOMIC_LLONG_LOCK_FREE`).

However, that is incorrect, since we would need `alignof(struct) == 8` for it to be lock free. Did I get that right?

If so, how the hell are we supposed to use this :-). `COMPARE_TYPES` would instead need to check `TEST_ALIGNOF(T1) >= sizeof(T2)` instead?

https://github.com/llvm/llvm-project/pull/99570


More information about the libcxx-commits mailing list