From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16616 invoked by alias); 17 Oct 2006 11:36:50 -0000 Received: (qmail 16598 invoked by uid 22791); 17 Oct 2006 11:36:49 -0000 X-Spam-Check-By: sourceware.org Received: from sunsite.ms.mff.cuni.cz (HELO sunsite.mff.cuni.cz) (195.113.15.26) by sourceware.org (qpsmtpd/0.31) with ESMTP; Tue, 17 Oct 2006 11:36:40 +0000 Received: from sunsite.mff.cuni.cz (sunsite.mff.cuni.cz [127.0.0.1]) by sunsite.mff.cuni.cz (8.13.1/8.13.1) with ESMTP id k9HBa6UU016878; Tue, 17 Oct 2006 13:36:06 +0200 Received: (from jj@localhost) by sunsite.mff.cuni.cz (8.13.1/8.13.1/Submit) id k9HBa5eQ016877; Tue, 17 Oct 2006 13:36:05 +0200 Date: Tue, 17 Oct 2006 11:36:00 -0000 From: Jakub Jelinek To: Ulrich Drepper Cc: Glibc hackers Subject: [PATCH] Fix catomic_* macros Message-ID: <20061017113605.GC5868@sunsite.mff.cuni.cz> Reply-To: Jakub Jelinek Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.4.1i Mailing-List: contact libc-hacker-help@sourceware.org; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-hacker-owner@sourceware.org X-SW-Source: 2006-10/txt/msg00008.txt.bz2 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 * 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 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 @@ -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 , 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