From 4957a32f9e739e3a291dde1b5b06216f54f27083 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Thu, 28 Oct 2021 08:53:33 +0200 Subject: [PATCH] BUILD: atomic: prefer __atomic_compare_exchange_n() for __ha_cas_dw() __atomic_compare_exchange() is incorrectly documented in the gcc builtins doc, it says the desired value is "type *desired" while in reality it is "const type *desired" as expected since that value must in no way be modified by the operation. However it seems that clang has implemented it as documented, and reports build warnings when fed a const. This is quite problematic because it means we have to betry the callers, pretending we won't touch their constants but not knowing what the compiler would do with them, and possibly hiding future bugs. Instead of forcing a cast, let's just switch to the better __atomic_compare_exchange_n() that takes a value instead of a pointer. At least with this one there is no doubt about how the input will be used. It was verified that the output object code is the same both in clang and gcc with this change. --- include/haproxy/atomic.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/haproxy/atomic.h b/include/haproxy/atomic.h index 3198b381a..ddc809e4f 100644 --- a/include/haproxy/atomic.h +++ b/include/haproxy/atomic.h @@ -752,8 +752,8 @@ static forceinline int __ha_cas_dw(void *target, void *compare, const void *set) /* returns 0 on failure, non-zero on success */ static __inline int __ha_cas_dw(void *target, void *compare, const void *set) { - return __atomic_compare_exchange((__int128*)target, (__int128*)compare, (const __int128*)set, - 0, __ATOMIC_RELAXED, __ATOMIC_RELAXED); + return __atomic_compare_exchange_n((__int128*)target, (__int128*)compare, *(const __int128*)set, + 0, __ATOMIC_RELAXED, __ATOMIC_RELAXED); } #else // neither ARMv8.1-A atomics nor 128-bit atomics