]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
BUG/MINOR: threads: fix multiple use of argument inside HA_ATOMIC_UPDATE_{MIN,MAX}()
authorWilly Tarreau <w@1wt.eu>
Tue, 5 May 2020 14:13:36 +0000 (16:13 +0200)
committerWilly Tarreau <w@1wt.eu>
Tue, 5 May 2020 14:18:52 +0000 (16:18 +0200)
Just like in previous patch, it happens that HA_ATOMIC_UPDATE_MIN() and
HA_ATOMIC_UPDATE_MAX() would evaluate the (val) argument up to 3 times.
However this time it affects both thread and non-thread versions. It's
strange because the copy was properly performed for the (new) argument
in order to avoid this. Anyway it was done for the "val" one as well.

A quick code inspection showed that this currently has no effect as
these macros are fairly limited in usage.

It would be best to backport this for long-term stability (till 1.8)
but it will not fix an existing bug.

include/common/hathreads.h

index 1350f190464ff79ca5549c42b91109b3191ac948..cc54574fad77f9f68619d27787b903af3275fbb6 100644 (file)
@@ -135,20 +135,22 @@ extern THREAD_LOCAL struct thread_info *ti; /* thread_info for the current threa
 #define HA_ATOMIC_STORE(val, new)    ({*(val) = new;})
 #define HA_ATOMIC_UPDATE_MAX(val, new)                                 \
        ({                                                              \
+               typeof(val) __val = (val);                              \
                typeof(*(val)) __new_max = (new);                       \
                                                                        \
-               if (*(val) < __new_max)                                 \
-                       *(val) = __new_max;                             \
-               *(val);                                                 \
+               if (*__val < __new_max)                                 \
+                       *__val = __new_max;                             \
+               *__val;                                                 \
        })
 
 #define HA_ATOMIC_UPDATE_MIN(val, new)                                 \
        ({                                                              \
+               typeof(val) __val = (val);                              \
                typeof(*(val)) __new_min = (new);                       \
                                                                        \
-               if (*(val) > __new_min)                                 \
-                       *(val) = __new_min;                             \
-               *(val);                                                 \
+               if (*__val > __new_min)                                 \
+                       *__val = __new_min;                             \
+               *__val;                                                 \
        })
 
 #define HA_BARRIER() do { } while (0)
@@ -410,21 +412,23 @@ static inline unsigned long thread_isolated()
 
 #define HA_ATOMIC_UPDATE_MAX(val, new)                                 \
        ({                                                              \
-               typeof(*(val)) __old_max = *(val);                      \
+               typeof(val) __val = (val);                              \
+               typeof(*(val)) __old_max = *__val;                      \
                typeof(*(val)) __new_max = (new);                       \
                                                                        \
                while (__old_max < __new_max &&                         \
-                      !HA_ATOMIC_CAS(val, &__old_max, __new_max));     \
-               *(val);                                                 \
+                      !HA_ATOMIC_CAS(__val, &__old_max, __new_max));   \
+               *__val;                                                 \
        })
 #define HA_ATOMIC_UPDATE_MIN(val, new)                                 \
        ({                                                              \
-               typeof(*(val)) __old_min = *(val);                      \
+               typeof(val) __val = (val);                              \
+               typeof(*(val)) __old_min = *__val;                      \
                typeof(*(val)) __new_min = (new);                       \
                                                                        \
                while (__old_min > __new_min &&                         \
-                      !HA_ATOMIC_CAS(val, &__old_min, __new_min));     \
-               *(val);                                                 \
+                      !HA_ATOMIC_CAS(__val, &__old_min, __new_min));   \
+               *__val;                                                 \
        })
 
 #define HA_BARRIER() pl_barrier()