From: Willy Tarreau Date: Tue, 5 May 2020 13:58:00 +0000 (+0200) Subject: BUG/MINOR: threads: fix multiple use of argument inside HA_ATOMIC_CAS() X-Git-Tag: v2.2-dev7~4 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=d66345d6b0639dfc42986a4326e6f6f6d18e053e;p=thirdparty%2Fhaproxy.git BUG/MINOR: threads: fix multiple use of argument inside HA_ATOMIC_CAS() When threads are disabled, HA_ATOMIC_CAS() becomes a simple compound expression. However this expression presents a problem, which is that its arguments are evaluated multiple times, once for the comparison and once again for the assignement. This presents a risk of performing some side-effect operations twice in the non-threaded case (e.g. in case of auto-increment or function return). The macro was rewritten using local copies for arguments like the other macros do. Fortunately a complete inspection of the code indicates that this case currently never happens. It was however responsible for the strict-aliasing warning emitted when building fd.c without threads but with 64-bit CAS. This may be backported as far as 1.8 though it will not fix any existing bug and is more of a long-term safety measure in case a future fix would depend on this behavior. --- diff --git a/include/common/hathreads.h b/include/common/hathreads.h index 95dfb051b2..1350f19046 100644 --- a/include/common/hathreads.h +++ b/include/common/hathreads.h @@ -77,7 +77,12 @@ extern THREAD_LOCAL struct thread_info *ti; /* thread_info for the current threa #define __decl_rwlock(lock) #define __decl_aligned_rwlock(lock) -#define HA_ATOMIC_CAS(val, old, new) ({((*val) == (*old)) ? (*(val) = (new) , 1) : (*(old) = *(val), 0);}) +#define HA_ATOMIC_CAS(val, old, new) \ + ({ \ + typeof(val) _v = (val); \ + typeof(old) _o = (old); \ + (*_v == *_o) ? ((*_v = (new)), 1) : ((*_o = *_v), 0); \ + }) /* warning, n is a pointer to the double value for dwcas */ #define HA_ATOMIC_DWCAS(val, o, n) \