Created
April 6, 2018 14:39
-
-
Save capflam/807c754ca3a70844e3e7b47e96e05332 to your computer and use it in GitHub Desktop.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
From 12456b597dbfe231ba5ea33457676ea59b505ed5 Mon Sep 17 00:00:00 2001 | |
From: Christopher Faulet <cfaulet@haproxy.com> | |
Date: Fri, 6 Apr 2018 15:33:30 +0200 | |
Subject: [PATCH] BUG/MEDIUM: threads: Fix the max/min calculation because of | |
name clashes | |
MIME-Version: 1.0 | |
Content-Type: text/plain; charset=UTF-8 | |
Content-Transfer-Encoding: 8bit | |
With gcc < 4.7, when HAProxy is built with threads, the macros | |
HA_ATOMOC_CAS/XCHG/STORE relies on the legacy ‘__sync’ builtins. These macros | |
are slightly complicated than the versions relying on the '_atomic' | |
builtins. Internally, some local variables are defined, prefixed with '__' to | |
avoid name clashes with the caller. | |
On the other hand, the macros HA_ATOMIC_UPDATE_MIN/MAX call HA_ATOMIC_CAS. Some | |
local variables are also definied in these macros, following the same naming | |
rule as below. The problem is that '__new' variable is used in | |
HA_ATOMIC_MIN/_MAX and in HA_ATOMIC_CAS. Obviously, the behaviour is undefined | |
because '__new' in HA_ATOMIC_CAS is left uninitialized. Unfortunatly gcc fails | |
to detect this error. | |
To fix the problem, all internal variables to macros are now suffixed with name | |
of the macros to avoid clashes (for instance, '__new_cas' in HA_ATOMIC_CAS). | |
This patch must be backported in 1.8. | |
--- | |
include/common/hathreads.h | 92 +++++++++++++++++++++++----------------------- | |
1 file changed, 47 insertions(+), 45 deletions(-) | |
diff --git a/include/common/hathreads.h b/include/common/hathreads.h | |
index e8b754cf..b0e4b23e 100644 | |
--- a/include/common/hathreads.h | |
+++ b/include/common/hathreads.h | |
@@ -41,26 +41,26 @@ extern THREAD_LOCAL unsigned long tid_bit; /* The bit corresponding to the threa | |
#define HA_ATOMIC_OR(val, flags) ({*(val) |= (flags);}) | |
#define HA_ATOMIC_XCHG(val, new) \ | |
({ \ | |
- typeof(*(val)) __old = *(val); \ | |
+ typeof(*(val)) __old_xchg = *(val); \ | |
*(val) = new; \ | |
- __old; \ | |
+ __old_xchg; \ | |
}) | |
#define HA_ATOMIC_STORE(val, new) ({*(val) = new;}) | |
#define HA_ATOMIC_UPDATE_MAX(val, new) \ | |
({ \ | |
- typeof(*(val)) __new = (new); \ | |
+ typeof(*(val)) __new_max = (new); \ | |
\ | |
- if (*(val) < __new) \ | |
- *(val) = __new; \ | |
+ if (*(val) < __new_max) \ | |
+ *(val) = __new_max; \ | |
*(val); \ | |
}) | |
#define HA_ATOMIC_UPDATE_MIN(val, new) \ | |
({ \ | |
- typeof(*(val)) __new = (new); \ | |
+ typeof(*(val)) __new_min = (new); \ | |
\ | |
- if (*(val) > __new) \ | |
- *(val) = __new; \ | |
+ if (*(val) > __new_min) \ | |
+ *(val) = __new_min; \ | |
*(val); \ | |
}) | |
@@ -124,38 +124,38 @@ extern THREAD_LOCAL unsigned long tid_bit; /* The bit corresponding to the threa | |
* but only if it differs from the expected one. If it's the same it's a race | |
* thus we try again to avoid confusing a possibly sensitive caller. | |
*/ | |
-#define HA_ATOMIC_CAS(val, old, new) \ | |
- ({ \ | |
- typeof((val)) __val = (val); \ | |
- typeof((old)) __oldp = (old); \ | |
- typeof(*(old)) __oldv; \ | |
- typeof((new)) __new = (new); \ | |
- int __ret; \ | |
- do { \ | |
- __oldv = *__val; \ | |
- __ret = __sync_bool_compare_and_swap(__val, *__oldp, __new); \ | |
- } while (!__ret && *__oldp == __oldv); \ | |
- if (!__ret) \ | |
- *__oldp = __oldv; \ | |
- __ret; \ | |
+#define HA_ATOMIC_CAS(val, old, new) \ | |
+ ({ \ | |
+ typeof((val)) __val_cas = (val); \ | |
+ typeof((old)) __oldp_cas = (old); \ | |
+ typeof(*(old)) __oldv_cas; \ | |
+ typeof((new)) __new_cas = (new); \ | |
+ int __ret_cas; \ | |
+ do { \ | |
+ __oldv_cas = *__val_cas; \ | |
+ __ret_cas = __sync_bool_compare_and_swap(__val_cas, *__oldp_cas, __new_cas); \ | |
+ } while (!__ret_cas && *__oldp_cas == __oldv_cas); \ | |
+ if (!__ret_cas) \ | |
+ *__oldp_cas = __oldv_cas; \ | |
+ __ret_cas; \ | |
}) | |
-#define HA_ATOMIC_XCHG(val, new) \ | |
- ({ \ | |
- typeof((val)) __val = (val); \ | |
- typeof(*(val)) __old; \ | |
- typeof((new)) __new = (new); \ | |
- do { __old = *__val; \ | |
- } while (!__sync_bool_compare_and_swap(__val, __old, __new)); \ | |
- __old; \ | |
+#define HA_ATOMIC_XCHG(val, new) \ | |
+ ({ \ | |
+ typeof((val)) __val_xchg = (val); \ | |
+ typeof(*(val)) __old_xchg; \ | |
+ typeof((new)) __new_xchg = (new); \ | |
+ do { __old_xchg = *__val_xchg; \ | |
+ } while (!__sync_bool_compare_and_swap(__val_xchg, __old_xchg, __new_xchg)); \ | |
+ __old_xchg; \ | |
}) | |
-#define HA_ATOMIC_STORE(val, new) \ | |
- ({ \ | |
- typeof((val)) __val = (val); \ | |
- typeof(*(val)) __old; \ | |
- typeof((new)) __new = (new); \ | |
- do { __old = *__val; \ | |
- } while (!__sync_bool_compare_and_swap(__val, __old, __new)); \ | |
+#define HA_ATOMIC_STORE(val, new) \ | |
+ ({ \ | |
+ typeof((val)) __val_store = (val); \ | |
+ typeof(*(val)) __old_store; \ | |
+ typeof((new)) __new_store = (new); \ | |
+ do { __old_store = *__val_store; \ | |
+ } while (!__sync_bool_compare_and_swap(__val_store, __old_store, __new_store)); \ | |
}) | |
#else | |
/* gcc >= 4.7 */ | |
@@ -170,19 +170,21 @@ extern THREAD_LOCAL unsigned long tid_bit; /* The bit corresponding to the threa | |
#define HA_ATOMIC_UPDATE_MAX(val, new) \ | |
({ \ | |
- typeof(*(val)) __old = *(val); \ | |
- typeof(*(val)) __new = (new); \ | |
+ typeof(*(val)) __old_max = *(val); \ | |
+ typeof(*(val)) __new_max = (new); \ | |
\ | |
- while (__old < __new && !HA_ATOMIC_CAS(val, &__old, __new)); \ | |
- (*val); \ | |
+ while (__old_max < __new_max && \ | |
+ !HA_ATOMIC_CAS(val, &__old_max, __new_max)); \ | |
+ *(val); \ | |
}) | |
#define HA_ATOMIC_UPDATE_MIN(val, new) \ | |
({ \ | |
- typeof((*val)) __old = *(val); \ | |
- typeof((*val)) __new = (new); \ | |
+ typeof(*(val)) __old_min = *(val); \ | |
+ typeof(*(val)) __new_min = (new); \ | |
\ | |
- while (__old > __new && !HA_ATOMIC_CAS(val, &__old, __new)); \ | |
- (*val); \ | |
+ while (__old_min > __new_min && \ | |
+ !HA_ATOMIC_CAS(val, &__old_min, __new_min)); \ | |
+ *(val); \ | |
}) | |
#define HA_BARRIER() pl_barrier() | |
-- | |
2.14.3 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment