[PATCH] Fix catomic_* macros

Jakub Jelinek jakub@redhat.com
Tue Oct 17 11:36:00 GMT 2006


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



More information about the Libc-hacker mailing list