* [PATCH] Fix catomic_* macros
@ 2006-10-17 11:36 Jakub Jelinek
2006-10-18 19:04 ` Ulrich Drepper
0 siblings, 1 reply; 2+ messages in thread
From: Jakub Jelinek @ 2006-10-17 11:36 UTC (permalink / raw)
To: Ulrich Drepper; +Cc: Glibc hackers
Hi!
Lazy binding seems to be completely broken on ppc{,64}, ld.so gets stuck
in _dl_fixup's __rtld_mrlock_unlock (l->l_scoperec_lock);
I have tracked down the problem to non-unique local variable names
in nested atomic.h macros, where we end up with uninitialized variables:
__typeof (__memp) __memp = (__memp);
As this is not the first time we have been chasing a problem like this,
I think it would be best to use per-macro unique prefixes for macro local
variables rather than just hope they don't clash.
The patch below changes just include/atomic.h (which is enough to
get ppc{,64} ld.so working right).
__atg5_ stands for __ ATomic.h Generic 5th macro _, I know it isn't
very nice acronym but all that is needed is something that's unlikely
to appear elsewhere in libc and be reasonably short.
2006-10-17 Jakub Jelinek <jakub@redhat.com>
* include/atomic.h: Add a unique prefix to all local variables
in macros.
* csu/tst-atomic.c (do_test): Test also catomic_* macros.
--- libc/include/atomic.h.jj 2006-10-17 00:12:32.000000000 +0200
+++ libc/include/atomic.h 2006-10-17 12:18:08.000000000 +0200
@@ -39,7 +39,12 @@
Architectures must provide a few lowlevel macros (the compare
and exchange definitions). All others are optional. They
should only be provided if the architecture has specific
- support for the operation. */
+ support for the operation.
+
+ As <atomic.h> macros are usually heavily nested and often use local
+ variables to make sure side-effects are evaluated properly, use for
+ macro local variables a per-macro unique prefix. This file uses
+ __atgN_ prefix where N is different in each macro. */
#include <stdlib.h>
@@ -50,33 +55,33 @@
and following args. */
#define __atomic_val_bysize(pre, post, mem, ...) \
({ \
- __typeof (*mem) __result; \
+ __typeof (*mem) __atg1_result; \
if (sizeof (*mem) == 1) \
- __result = pre##_8_##post (mem, __VA_ARGS__); \
+ __atg1_result = pre##_8_##post (mem, __VA_ARGS__); \
else if (sizeof (*mem) == 2) \
- __result = pre##_16_##post (mem, __VA_ARGS__); \
+ __atg1_result = pre##_16_##post (mem, __VA_ARGS__); \
else if (sizeof (*mem) == 4) \
- __result = pre##_32_##post (mem, __VA_ARGS__); \
+ __atg1_result = pre##_32_##post (mem, __VA_ARGS__); \
else if (sizeof (*mem) == 8) \
- __result = pre##_64_##post (mem, __VA_ARGS__); \
+ __atg1_result = pre##_64_##post (mem, __VA_ARGS__); \
else \
abort (); \
- __result; \
+ __atg1_result; \
})
#define __atomic_bool_bysize(pre, post, mem, ...) \
({ \
- int __result; \
+ int __atg2_result; \
if (sizeof (*mem) == 1) \
- __result = pre##_8_##post (mem, __VA_ARGS__); \
+ __atg2_result = pre##_8_##post (mem, __VA_ARGS__); \
else if (sizeof (*mem) == 2) \
- __result = pre##_16_##post (mem, __VA_ARGS__); \
+ __atg2_result = pre##_16_##post (mem, __VA_ARGS__); \
else if (sizeof (*mem) == 4) \
- __result = pre##_32_##post (mem, __VA_ARGS__); \
+ __atg2_result = pre##_32_##post (mem, __VA_ARGS__); \
else if (sizeof (*mem) == 8) \
- __result = pre##_64_##post (mem, __VA_ARGS__); \
+ __atg2_result = pre##_64_##post (mem, __VA_ARGS__); \
else \
abort (); \
- __result; \
+ __atg2_result; \
})
@@ -124,8 +129,9 @@
# define atomic_compare_and_exchange_bool_acq(mem, newval, oldval) \
({ /* Cannot use __oldval here, because macros later in this file might \
call this macro with __oldval argument. */ \
- __typeof (oldval) __old = (oldval); \
- atomic_compare_and_exchange_val_acq (mem, newval, __old) != __old; \
+ __typeof (oldval) __atg3_old = (oldval); \
+ atomic_compare_and_exchange_val_acq (mem, newval, __atg3_old) \
+ != __atg3_old; \
})
# endif
#endif
@@ -140,8 +146,9 @@
# define catomic_compare_and_exchange_bool_acq(mem, newval, oldval) \
({ /* Cannot use __oldval here, because macros later in this file might \
call this macro with __oldval argument. */ \
- __typeof (oldval) __old = (oldval); \
- catomic_compare_and_exchange_val_acq (mem, newval, __old) != __old; \
+ __typeof (oldval) __atg4_old = (oldval); \
+ catomic_compare_and_exchange_val_acq (mem, newval, __atg4_old) \
+ != __atg4_old; \
})
# endif
#endif
@@ -162,18 +169,17 @@
/* Store NEWVALUE in *MEM and return the old value. */
#ifndef atomic_exchange_acq
# define atomic_exchange_acq(mem, newvalue) \
- ({ __typeof (*(mem)) __oldval; \
- __typeof (mem) __memp = (mem); \
- __typeof (*(mem)) __value = (newvalue); \
+ ({ __typeof (*(mem)) __atg5_oldval; \
+ __typeof (mem) __atg5_memp = (mem); \
+ __typeof (*(mem)) __atg5_value = (newvalue); \
\
do \
- __oldval = *__memp; \
- while (__builtin_expect (atomic_compare_and_exchange_bool_acq (__memp, \
- __value, \
- __oldval),\
- 0)); \
+ __atg5_oldval = *__atg5_memp; \
+ while (__builtin_expect \
+ (atomic_compare_and_exchange_bool_acq (__atg5_memp, __atg5_value, \
+ __atg5_oldval), 0)); \
\
- __oldval; })
+ __atg5_oldval; })
#endif
#ifndef atomic_exchange_rel
@@ -184,54 +190,53 @@
/* Add VALUE to *MEM and return the old value of *MEM. */
#ifndef atomic_exchange_and_add
# define atomic_exchange_and_add(mem, value) \
- ({ __typeof (*(mem)) __oldval; \
- __typeof (mem) __memp = (mem); \
- __typeof (*(mem)) __value = (value); \
+ ({ __typeof (*(mem)) __atg6_oldval; \
+ __typeof (mem) __atg6_memp = (mem); \
+ __typeof (*(mem)) __atg6_value = (value); \
\
do \
- __oldval = *__memp; \
- while (__builtin_expect (atomic_compare_and_exchange_bool_acq (__memp, \
- __oldval \
- + __value,\
- __oldval),\
- 0)); \
+ __atg6_oldval = *__atg6_memp; \
+ while (__builtin_expect \
+ (atomic_compare_and_exchange_bool_acq (__atg6_memp, \
+ __atg6_oldval \
+ + __atg6_value, \
+ __atg6_oldval), 0)); \
\
- __oldval; })
+ __atg6_oldval; })
#endif
#ifndef catomic_exchange_and_add
# define catomic_exchange_and_add(mem, value) \
- ({ __typeof (*(mem)) __oldv; \
- __typeof (mem) __memp = (mem); \
- __typeof (*(mem)) __value = (value); \
+ ({ __typeof (*(mem)) __atg7_oldv; \
+ __typeof (mem) __atg7_memp = (mem); \
+ __typeof (*(mem)) __atg7_value = (value); \
\
do \
- __oldv = *__memp; \
- while (__builtin_expect (catomic_compare_and_exchange_bool_acq (__memp, \
- __oldv \
- + __value,\
- __oldv), \
- 0)); \
+ __atg7_oldv = *__atg7_memp; \
+ while (__builtin_expect \
+ (catomic_compare_and_exchange_bool_acq (__atg7_memp, \
+ __atg7_oldv \
+ + __atg7_value, \
+ __atg7_oldv), 0)); \
\
- __oldv; })
+ __atg7_oldv; })
#endif
#ifndef atomic_max
# define atomic_max(mem, value) \
do { \
- __typeof (*(mem)) __oldval; \
- __typeof (mem) __memp = (mem); \
- __typeof (*(mem)) __value = (value); \
+ __typeof (*(mem)) __atg8_oldval; \
+ __typeof (mem) __atg8_memp = (mem); \
+ __typeof (*(mem)) __atg8_value = (value); \
do { \
- __oldval = *__memp; \
- if (__oldval >= __value) \
+ __atg8_oldval = *__atg8_memp; \
+ if (__atg8_oldval >= __atg8_value) \
break; \
- } while (__builtin_expect (atomic_compare_and_exchange_bool_acq (__memp, \
- __value, \
- __oldval),\
- 0)); \
+ } while (__builtin_expect \
+ (atomic_compare_and_exchange_bool_acq (__atg8_memp, __atg8_value,\
+ __atg8_oldval), 0)); \
} while (0)
#endif
@@ -239,17 +244,17 @@
#ifndef catomic_max
# define catomic_max(mem, value) \
do { \
- __typeof (*(mem)) __oldv; \
- __typeof (mem) __memp = (mem); \
- __typeof (*(mem)) __value = (value); \
+ __typeof (*(mem)) __atg9_oldv; \
+ __typeof (mem) __atg9_memp = (mem); \
+ __typeof (*(mem)) __atg9_value = (value); \
do { \
- __oldv = *__memp; \
- if (__oldv >= __value) \
+ __atg9_oldv = *__atg9_memp; \
+ if (__atg9_oldv >= __atg9_value) \
break; \
- } while (__builtin_expect (catomic_compare_and_exchange_bool_acq (__memp, \
- __value,\
- __oldv),\
- 0)); \
+ } while (__builtin_expect \
+ (catomic_compare_and_exchange_bool_acq (__atg9_memp, \
+ __atg9_value, \
+ __atg9_oldv), 0)); \
} while (0)
#endif
@@ -257,17 +262,17 @@
#ifndef atomic_min
# define atomic_min(mem, value) \
do { \
- __typeof (*(mem)) __oldval; \
- __typeof (mem) __memp = (mem); \
- __typeof (*(mem)) __value = (value); \
+ __typeof (*(mem)) __atg10_oldval; \
+ __typeof (mem) __atg10_memp = (mem); \
+ __typeof (*(mem)) __atg10_value = (value); \
do { \
- __oldval = *__memp; \
- if (__oldval <= __value) \
+ __atg10_oldval = *__atg10_memp; \
+ if (__atg10_oldval <= __atg10_value) \
break; \
- } while (__builtin_expect (atomic_compare_and_exchange_bool_acq (__memp, \
- __value, \
- __oldval),\
- 0)); \
+ } while (__builtin_expect \
+ (atomic_compare_and_exchange_bool_acq (__atg10_memp, \
+ __atg10_value, \
+ __atg10_oldval), 0)); \
} while (0)
#endif
@@ -340,35 +345,34 @@
/* Decrement *MEM if it is > 0, and return the old value. */
#ifndef atomic_decrement_if_positive
# define atomic_decrement_if_positive(mem) \
- ({ __typeof (*(mem)) __oldval; \
- __typeof (mem) __memp = (mem); \
+ ({ __typeof (*(mem)) __atg11_oldval; \
+ __typeof (mem) __atg11_memp = (mem); \
\
do \
{ \
- __oldval = *__memp; \
- if (__builtin_expect (__oldval <= 0, 0)) \
+ __atg11_oldval = *__atg11_memp; \
+ if (__builtin_expect (__atg11_oldval <= 0, 0)) \
break; \
} \
- while (__builtin_expect (atomic_compare_and_exchange_bool_acq (__memp, \
- __oldval \
- - 1, \
- __oldval),\
- 0));\
- __oldval; })
+ while (__builtin_expect \
+ (atomic_compare_and_exchange_bool_acq (__atg11_memp, \
+ __atg11_oldval - 1, \
+ __atg11_oldval), 0)); \
+ __atg11_oldval; })
#endif
#ifndef atomic_add_negative
# define atomic_add_negative(mem, value) \
- ({ __typeof (value) __aan_value = (value); \
- atomic_exchange_and_add (mem, __aan_value) < -__aan_value; })
+ ({ __typeof (value) __atg12_value = (value); \
+ atomic_exchange_and_add (mem, __atg12_value) < -__atg12_value; })
#endif
#ifndef atomic_add_zero
# define atomic_add_zero(mem, value) \
- ({ __typeof (value) __aaz_value = (value); \
- atomic_exchange_and_add (mem, __aaz_value) == -__aaz_value; })
+ ({ __typeof (value) __atg13_value = (value); \
+ atomic_exchange_and_add (mem, __atg13_value) == -__atg13_value; })
#endif
@@ -380,108 +384,102 @@
#ifndef atomic_bit_test_set
# define atomic_bit_test_set(mem, bit) \
- ({ __typeof (*(mem)) __oldval; \
- __typeof (mem) __memp = (mem); \
- __typeof (*(mem)) __mask = ((__typeof (*(mem))) 1 << (bit)); \
+ ({ __typeof (*(mem)) __atg14_old; \
+ __typeof (mem) __atg14_memp = (mem); \
+ __typeof (*(mem)) __atg14_mask = ((__typeof (*(mem))) 1 << (bit)); \
\
do \
- __oldval = (*__memp); \
- while (__builtin_expect (atomic_compare_and_exchange_bool_acq (__memp, \
- __oldval \
- | __mask, \
- __oldval),\
- 0)); \
+ __atg14_old = (*__atg14_memp); \
+ while (__builtin_expect \
+ (atomic_compare_and_exchange_bool_acq (__atg14_memp, \
+ __atg14_old | __atg14_mask,\
+ __atg14_old), 0)); \
\
- __oldval & __mask; })
+ __atg14_old & __atg14_mask; })
#endif
/* Atomically *mem &= mask. */
#ifndef atomic_and
# define atomic_and(mem, mask) \
do { \
- __typeof (*(mem)) __oldval; \
- __typeof (mem) __memp = (mem); \
- __typeof (*(mem)) __mask = (mask); \
+ __typeof (*(mem)) __atg15_old; \
+ __typeof (mem) __atg15_memp = (mem); \
+ __typeof (*(mem)) __atg15_mask = (mask); \
\
do \
- __oldval = (*__memp); \
- while (__builtin_expect (atomic_compare_and_exchange_bool_acq (__memp, \
- __oldval \
- & __mask, \
- __oldval), \
- 0)); \
+ __atg15_old = (*__atg15_memp); \
+ while (__builtin_expect \
+ (atomic_compare_and_exchange_bool_acq (__atg15_memp, \
+ __atg15_old & __atg15_mask, \
+ __atg15_old), 0)); \
} while (0)
#endif
/* Atomically *mem &= mask and return the old value of *mem. */
#ifndef atomic_and_val
# define atomic_and_val(mem, mask) \
- ({ __typeof (*(mem)) __oldval; \
- __typeof (mem) __memp = (mem); \
- __typeof (*(mem)) __mask = (mask); \
+ ({ __typeof (*(mem)) __atg16_old; \
+ __typeof (mem) __atg16_memp = (mem); \
+ __typeof (*(mem)) __atg16_mask = (mask); \
\
do \
- __oldval = (*__memp); \
- while (__builtin_expect (atomic_compare_and_exchange_bool_acq (__memp, \
- __oldval \
- & __mask, \
- __oldval),\
- 0)); \
+ __atg16_old = (*__atg16_memp); \
+ while (__builtin_expect \
+ (atomic_compare_and_exchange_bool_acq (__atg16_memp, \
+ __atg16_old & __atg16_mask,\
+ __atg16_old), 0)); \
\
- __oldval; })
+ __atg16_old; })
#endif
/* Atomically *mem |= mask and return the old value of *mem. */
#ifndef atomic_or
# define atomic_or(mem, mask) \
do { \
- __typeof (*(mem)) __oldval; \
- __typeof (mem) __memp = (mem); \
- __typeof (*(mem)) __mask = (mask); \
+ __typeof (*(mem)) __atg17_old; \
+ __typeof (mem) __atg17_memp = (mem); \
+ __typeof (*(mem)) __atg17_mask = (mask); \
\
do \
- __oldval = (*__memp); \
- while (__builtin_expect (atomic_compare_and_exchange_bool_acq (__memp, \
- __oldval \
- | __mask, \
- __oldval), \
- 0)); \
+ __atg17_old = (*__atg17_memp); \
+ while (__builtin_expect \
+ (atomic_compare_and_exchange_bool_acq (__atg17_memp, \
+ __atg17_old | __atg17_mask, \
+ __atg17_old), 0)); \
} while (0)
#endif
#ifndef catomic_or
# define catomic_or(mem, mask) \
do { \
- __typeof (*(mem)) __oldval; \
- __typeof (mem) __memp = (mem); \
- __typeof (*(mem)) __mask = (mask); \
+ __typeof (*(mem)) __atg18_old; \
+ __typeof (mem) __atg18_memp = (mem); \
+ __typeof (*(mem)) __atg18_mask = (mask); \
\
do \
- __oldval = (*__memp); \
- while (__builtin_expect (catomic_compare_and_exchange_bool_acq (__memp, \
- __oldval \
- | __mask, \
- __oldval),\
- 0)); \
+ __atg18_old = (*__atg18_memp); \
+ while (__builtin_expect \
+ (catomic_compare_and_exchange_bool_acq (__atg18_memp, \
+ __atg18_old | __atg18_mask,\
+ __atg18_old), 0)); \
} while (0)
#endif
/* Atomically *mem |= mask and return the old value of *mem. */
#ifndef atomic_or_val
# define atomic_or_val(mem, mask) \
- ({ __typeof (*(mem)) __oldval; \
- __typeof (mem) __memp = (mem); \
- __typeof (*(mem)) __mask = (mask); \
+ ({ __typeof (*(mem)) __atg19_old; \
+ __typeof (mem) __atg19_memp = (mem); \
+ __typeof (*(mem)) __atg19_mask = (mask); \
\
do \
- __oldval = (*__memp); \
- while (__builtin_expect (atomic_compare_and_exchange_bool_acq (__memp, \
- __oldval \
- | __mask, \
- __oldval),\
- 0)); \
+ __atg19_old = (*__atg19_memp); \
+ while (__builtin_expect \
+ (atomic_compare_and_exchange_bool_acq (__atg19_memp, \
+ __atg19_old | __atg19_mask,\
+ __atg19_old), 0)); \
\
- __oldval; })
+ __atg19_old; })
#endif
#ifndef atomic_full_barrier
--- libc/csu/tst-atomic.c.jj 2004-09-14 00:32:41.000000000 +0200
+++ libc/csu/tst-atomic.c 2006-10-17 12:15:44.000000000 +0200
@@ -1,5 +1,5 @@
/* Tests for atomic.h macros.
- Copyright (C) 2003, 2004 Free Software Foundation, Inc.
+ Copyright (C) 2003, 2004, 2006 Free Software Foundation, Inc.
This file is part of the GNU C Library.
Contributed by Jakub Jelinek <jakub@redhat.com>, 2003.
@@ -379,6 +379,117 @@ do_test (void)
}
#endif
+#ifdef catomic_compare_and_exchange_val_acq
+ mem = 24;
+ if (catomic_compare_and_exchange_val_acq (&mem, 35, 24) != 24
+ || mem != 35)
+ {
+ puts ("catomic_compare_and_exchange_val_acq test 1 failed");
+ ret = 1;
+ }
+
+ mem = 12;
+ if (catomic_compare_and_exchange_val_acq (&mem, 10, 15) != 12
+ || mem != 12)
+ {
+ puts ("catomic_compare_and_exchange_val_acq test 2 failed");
+ ret = 1;
+ }
+
+ mem = -15;
+ if (catomic_compare_and_exchange_val_acq (&mem, -56, -15) != -15
+ || mem != -56)
+ {
+ puts ("catomic_compare_and_exchange_val_acq test 3 failed");
+ ret = 1;
+ }
+
+ mem = -1;
+ if (catomic_compare_and_exchange_val_acq (&mem, 17, 0) != -1
+ || mem != -1)
+ {
+ puts ("catomic_compare_and_exchange_val_acq test 4 failed");
+ ret = 1;
+ }
+#endif
+
+ mem = 24;
+ if (catomic_compare_and_exchange_bool_acq (&mem, 35, 24)
+ || mem != 35)
+ {
+ puts ("catomic_compare_and_exchange_bool_acq test 1 failed");
+ ret = 1;
+ }
+
+ mem = 12;
+ if (! catomic_compare_and_exchange_bool_acq (&mem, 10, 15)
+ || mem != 12)
+ {
+ puts ("catomic_compare_and_exchange_bool_acq test 2 failed");
+ ret = 1;
+ }
+
+ mem = -15;
+ if (catomic_compare_and_exchange_bool_acq (&mem, -56, -15)
+ || mem != -56)
+ {
+ puts ("catomic_compare_and_exchange_bool_acq test 3 failed");
+ ret = 1;
+ }
+
+ mem = -1;
+ if (! catomic_compare_and_exchange_bool_acq (&mem, 17, 0)
+ || mem != -1)
+ {
+ puts ("catomic_compare_and_exchange_bool_acq test 4 failed");
+ ret = 1;
+ }
+
+ mem = 2;
+ if (catomic_exchange_and_add (&mem, 11) != 2
+ || mem != 13)
+ {
+ puts ("catomic_exchange_and_add test failed");
+ ret = 1;
+ }
+
+ mem = -21;
+ catomic_add (&mem, 22);
+ if (mem != 1)
+ {
+ puts ("catomic_add test failed");
+ ret = 1;
+ }
+
+ mem = -1;
+ catomic_increment (&mem);
+ if (mem != 0)
+ {
+ puts ("catomic_increment test failed");
+ ret = 1;
+ }
+
+ mem = 2;
+ if (catomic_increment_val (&mem) != 3)
+ {
+ puts ("catomic_increment_val test failed");
+ ret = 1;
+ }
+
+ mem = 17;
+ catomic_decrement (&mem);
+ if (mem != 16)
+ {
+ puts ("catomic_decrement test failed");
+ ret = 1;
+ }
+
+ if (catomic_decrement_val (&mem) != 15)
+ {
+ puts ("catomic_decrement_val test failed");
+ ret = 1;
+ }
+
return ret;
}
Jakub
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] Fix catomic_* macros
2006-10-17 11:36 [PATCH] Fix catomic_* macros Jakub Jelinek
@ 2006-10-18 19:04 ` Ulrich Drepper
0 siblings, 0 replies; 2+ messages in thread
From: Ulrich Drepper @ 2006-10-18 19:04 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Glibc hackers
Applied.
--
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA â–
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2006-10-18 19:04 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-10-17 11:36 [PATCH] Fix catomic_* macros Jakub Jelinek
2006-10-18 19:04 ` Ulrich Drepper
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).