This is the mail archive of the libc-hacker@sources.redhat.com mailing list for the glibc project.
Note that libc-hacker is a closed list. You may look at the archives of this list, but subscription and posting are not open.
| Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
|---|---|---|
| Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
| Other format: | [Raw text] | |
Hi!
I found a couple of issues with various atomic.h headers and decide
to write a testcase for all the atomic.h macros (it doesn't test if the
operations are actually atomic, just whether they do what they are supposed
to do). This revealed a bunch of other bugs.
It is unclear to me what the semantics of a couple of macros should be,
as the include/atomic.h version is different from i486 and x86-64 version.
The testcase passes on IA-64 and i386 (using sysdeps/generic/bits/atomic.h),
but there are failures on i686.
atomic_increment_and_test test 1 failed
atomic_decrement_and_test test 1 failed
atomic_add_negative test 1 failed
atomic_add_zero test 2 failed
IMHO every macro in include/atomic.h should have a comment explaining
what it exactly does and then tst-atomic.c should be changed to match that.
E.g. atomic_increment_and_test seems to test the value after increment
while generic version tests the value before increment.
Another problem are the nested statement expressions. We must ensure
we never use the same names for local variables in them if they can be
nested. This patch fixes one where things would expand into
__typeof(something) __oldvalue = (__oldvalue);
and another one where similar thing happens with __memp.
I wonder whether we shouldn't use some unique prefixes for these local
variables, say __ followed by first letters of word in macro name __
and descriptive name or something.
2003-03-21 Jakub Jelinek <jakub at redhat dot com>
* include/atomic.h (atomic_compare_and_exchange_val_acq): Add comment.
Don't define if __arch_compare_and_exchange_val_32_acq is not defined.
(atomic_compare_and_exchange_bool_acq): Add comment. Don't use
__oldval variable in the macro, since it might be macro argument.
(atomic_decrement_if_positive): Initialize __memp, remove setting
of non-existent variable.
(atomic_bit_test_set): Cast 1 to __typeof (*mem) before shifting.
* sysdeps/ia64/bits/atomic.h (atomic_exchange_and_add): Implement
using atomic_compare_and_exchange_val_acq.
(atomic_decrement_if_positive, atomic_bit_test_set): Define.
* sysdeps/s390/bits/atomic.h (__arch_compare_and_exchange_val_8_acq):
Renamed from...
(__arch_compare_and_exchange_bool_8_acq): ... this.
(__arch_compare_and_exchange_val_16_acq): Renamed from...
(__arch_compare_and_exchange_bool_16_acq): ... this.
(__arch_compare_and_exchange_val_32_acq): Return old value. Renamed
from...
(__arch_compare_and_exchange_bool_32_acq): ... this.
(__arch_compare_and_exchange_val_64_acq): Return old value. Renamed
from...
(__arch_compare_and_exchange_bool_64_acq): ... this.
* sysdeps/generic/bits/atomic.h (arch_compare_and_exchange_acq):
Remove.
(atomic_compare_and_exchange_val_acq,
atomic_compare_and_exchange_bool_acq): Define.
* sysdeps/x86_64/bits/atomic.h (atomic_bit_set): Shift 1L instead
of 1 up in the 64-bit case.
* csu/tst-atomic.c: New test.
* csu/tst-atomic-long.c: New test.
* csu/Makefile (tests): Add tst-atomic and tst-atomic-long.
* malloc/memusagestat.c (main): Kill warning if uint64_t is ulong.
* sysdeps/s390/Versions: Add trailing newline.
* sysdeps/unix/sysv/linux/sysconf.c (__sysconf): Kill warning
if INTERNAL_SYSCALL_ERROR_P doesn't use its first argument.
--- libc/csu/tst-atomic.c.jj 2003-03-21 10:09:42.000000000 -0500
+++ libc/csu/tst-atomic.c 2003-03-21 11:41:50.000000000 -0500
@@ -0,0 +1,276 @@
+/* Copyright (C) 2003 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+ Contributed by Jakub Jelinek <jakub at redhat dot com>, 2003.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, write to the Free
+ Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
+ 02111-1307 USA. */
+
+#include <stdio.h>
+#include <atomic.h>
+
+#ifndef atomic_t
+# define atomic_t int
+#endif
+
+/* Test various atomic.h macros. */
+static int
+do_test (void)
+{
+ atomic_t mem;
+ int ret = 0;
+
+#ifdef atomic_compare_and_exchange_val_acq
+ mem = 24;
+ if (atomic_compare_and_exchange_val_acq (&mem, 35, 24) != 24
+ || mem != 35)
+ {
+ puts ("atomic_compare_and_exchange_val_acq test 1 failed");
+ ret = 1;
+ }
+
+ mem = 12;
+ if (atomic_compare_and_exchange_val_acq (&mem, 10, 15) != 12
+ || mem != 12)
+ {
+ puts ("atomic_compare_and_exchange_val_acq test 2 failed");
+ ret = 1;
+ }
+#endif
+
+ mem = 24;
+ if (atomic_compare_and_exchange_bool_acq (&mem, 35, 24)
+ || mem != 35)
+ {
+ puts ("atomic_compare_and_exchange_bool_acq test 1 failed");
+ ret = 1;
+ }
+
+ mem = 12;
+ if (! atomic_compare_and_exchange_bool_acq (&mem, 10, 15)
+ || mem != 12)
+ {
+ puts ("atomic_compare_and_exchange_bool_acq test 2 failed");
+ ret = 1;
+ }
+
+ mem = 64;
+ if (atomic_exchange (&mem, 31) != 64
+ || mem != 31)
+ {
+ puts ("atomic_exchange test failed");
+ ret = 1;
+ }
+
+ mem = 2;
+ if (atomic_exchange_and_add (&mem, 11) != 2
+ || mem != 13)
+ {
+ puts ("atomic_exchange_and_add test failed");
+ ret = 1;
+ }
+
+ mem = -21;
+ atomic_add (&mem, 22);
+ if (mem != 1)
+ {
+ puts ("atomic_add test failed");
+ ret = 1;
+ }
+
+ mem = -1;
+ atomic_increment (&mem);
+ if (mem != 0)
+ {
+ puts ("atomic_increment test failed");
+ ret = 1;
+ }
+
+ mem = 0;
+ if (! atomic_increment_and_test (&mem)
+ || mem != 1)
+ {
+ puts ("atomic_increment_and_test test 1 failed");
+ ret = 1;
+ }
+
+ mem = 35;
+ if (atomic_increment_and_test (&mem)
+ || mem != 36)
+ {
+ puts ("atomic_increment_and_test test 2 failed");
+ ret = 1;
+ }
+
+ mem = 17;
+ atomic_decrement (&mem);
+ if (mem != 16)
+ {
+ puts ("atomic_decrement test failed");
+ ret = 1;
+ }
+
+ mem = 0;
+ if (! atomic_decrement_and_test (&mem)
+ || mem != -1)
+ {
+ puts ("atomic_decrement_and_test test 1 failed");
+ ret = 1;
+ }
+
+ mem = 15;
+ if (atomic_decrement_and_test (&mem)
+ || mem != 14)
+ {
+ puts ("atomic_decrement_and_test test 2 failed");
+ ret = 1;
+ }
+
+ mem = 1;
+ if (atomic_decrement_if_positive (&mem) != 1
+ || mem != 0)
+ {
+ puts ("atomic_decrement_if_positive test 1 failed");
+ ret = 1;
+ }
+
+ mem = 0;
+ if (atomic_decrement_if_positive (&mem) != 0
+ || mem != 0)
+ {
+ puts ("atomic_decrement_if_positive test 2 failed");
+ ret = 1;
+ }
+
+ mem = -1;
+ if (atomic_decrement_if_positive (&mem) != -1
+ || mem != -1)
+ {
+ puts ("atomic_decrement_if_positive test 3 failed");
+ ret = 1;
+ }
+
+ mem = -10;
+ if (! atomic_add_negative (&mem, 12)
+ || mem != 2)
+ {
+ puts ("atomic_add_negative test 1 failed");
+ ret = 1;
+ }
+
+ mem = 0;
+ if (atomic_add_negative (&mem, 100)
+ || mem != 100)
+ {
+ puts ("atomic_add_negative test 2 failed");
+ ret = 1;
+ }
+
+ mem = 15;
+ if (atomic_add_negative (&mem, -10)
+ || mem != 5)
+ {
+ puts ("atomic_add_negative test 3 failed");
+ ret = 1;
+ }
+
+ mem = -34;
+ if (atomic_add_zero (&mem, 31)
+ || mem != -3)
+ {
+ puts ("atomic_add_zero test 1 failed");
+ ret = 1;
+ }
+
+ mem = 0;
+ if (! atomic_add_zero (&mem, 36)
+ || mem != 36)
+ {
+ puts ("atomic_add_zero test 2 failed");
+ ret = 1;
+ }
+
+ mem = 113;
+ if (atomic_add_zero (&mem, -13)
+ || mem != 100)
+ {
+ puts ("atomic_add_zero test 3 failed");
+ ret = 1;
+ }
+
+ mem = 0;
+ atomic_bit_set (&mem, 1);
+ if (mem != 2)
+ {
+ puts ("atomic_bit_set test 1 failed");
+ ret = 1;
+ }
+
+ mem = 8;
+ atomic_bit_set (&mem, 3);
+ if (mem != 8)
+ {
+ puts ("atomic_bit_set test 2 failed");
+ ret = 1;
+ }
+
+#ifdef TEST_ATOMIC64
+ mem = 16;
+ atomic_bit_set (&mem, 35);
+ if (mem != 0x800000010LL)
+ {
+ puts ("atomic_bit_set test 3 failed");
+ ret = 1;
+ }
+#endif
+
+ mem = 0;
+ if (atomic_bit_test_set (&mem, 1)
+ || mem != 2)
+ {
+ puts ("atomic_bit_test_set test 1 failed");
+ ret = 1;
+ }
+
+ mem = 8;
+ if (! atomic_bit_test_set (&mem, 3)
+ || mem != 8)
+ {
+ puts ("atomic_bit_test_set test 2 failed");
+ ret = 1;
+ }
+
+#ifdef TEST_ATOMIC64
+ mem = 16;
+ if (atomic_bit_test_set (&mem, 35)
+ || mem != 0x800000010LL)
+ {
+ puts ("atomic_bit_test_set test 3 failed");
+ ret = 1;
+ }
+
+ mem = 0x100000000LL;
+ if (! atomic_bit_test_set (&mem, 32)
+ || mem != 0x100000000LL)
+ {
+ puts ("atomic_bit_test_set test 4 failed");
+ ret = 1;
+ }
+#endif
+
+ return 0;
+}
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"
--- libc/csu/Makefile.jj 2002-12-31 21:33:13.000000000 -0500
+++ libc/csu/Makefile 2003-03-21 11:23:40.000000000 -0500
@@ -1,5 +1,5 @@
# Makefile for csu code for GNU C library.
-# Copyright (C) 1995,96,97,98,99,2000,01,2002 Free Software Foundation, Inc.
+# Copyright (C) 1995,96,97,98,99,2000,01,02,2003 Free Software Foundation, Inc.
# This file is part of the GNU C Library.
# The GNU C Library is free software; you can redistribute it and/or
@@ -42,6 +42,8 @@ distribute = initfini.c gmon-start.c sta
generated = version-info.h
before-compile = $(objpfx)version-info.h
+tests := tst-atomic tst-atomic-long
+
all: # Make this the default target; it will be defined in Rules.
include ../Makeconfig
--- libc/csu/tst-atomic-long.c.jj 2003-03-21 11:17:56.000000000 -0500
+++ libc/csu/tst-atomic-long.c 2003-03-21 11:21:33.000000000 -0500
@@ -0,0 +1,27 @@
+/* Copyright (C) 2003 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+ Contributed by Jakub Jelinek <jakub at redhat dot com>, 2003.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, write to the Free
+ Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
+ 02111-1307 USA. */
+
+#include <bits/wordsize.h>
+
+#define atomic_t long
+#if __WORDSIZE == 64
+# define TEST_ATOMIC64 1
+#endif
+
+#include "tst-atomic.c"
--- libc/include/atomic.h.jj 2003-03-21 05:17:25.000000000 -0500
+++ libc/include/atomic.h 2003-03-21 11:45:04.000000000 -0500
@@ -25,7 +25,10 @@
#include <bits/atomic.h>
-#ifndef atomic_compare_and_exchange_val_acq
+/* Atomically store NEWVAL in *MEM if *MEM is equal to OLDVAL.
+ Return the old *MEM value. */
+#if !defined atomic_compare_and_exchange_val_acq \
+ && defined __arch_compare_and_exchange_val_32_acq
# define atomic_compare_and_exchange_val_acq(mem, newval, oldval) \
({ __typeof (*mem) __result; \
if (sizeof (*mem) == 1) \
@@ -48,6 +51,8 @@
#endif
+/* Atomically store NEWVAL in *MEM if *MEM is equal to OLDVAL.
+ Return zero if *MEM was changed or non-zero if no exchange happened. */
#ifndef atomic_compare_and_exchange_bool_acq
# ifdef __arch_compare_and_exchange_bool_32_acq
# define atomic_compare_and_exchange_bool_acq(mem, newval, oldval) \
@@ -69,8 +74,10 @@
__result; })
# else
# define atomic_compare_and_exchange_bool_acq(mem, newval, oldval) \
- ({ __typeof (oldval) __oldval = (oldval); \
- atomic_compare_and_exchange_val_acq (mem, newval, __oldval) != __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; \
})
# endif
#endif
@@ -150,9 +157,8 @@
#ifndef atomic_decrement_if_positive
# define atomic_decrement_if_positive(mem) \
({ __typeof (*mem) __oldval; \
- __typeof (mem) __memp; \
+ __typeof (mem) __memp = (mem); \
\
- __val = *__memp; \
do \
{ \
__oldval = *__memp; \
@@ -190,7 +196,7 @@
# define atomic_bit_test_set(mem, bit) \
({ __typeof (*mem) __oldval; \
__typeof (mem) __memp = (mem); \
- __typeof (*mem) __mask = (1 << (bit)); \
+ __typeof (*mem) __mask = ((__typeof (*mem)) 1 << (bit)); \
\
do \
__oldval = (*__memp); \
--- libc/malloc/memusagestat.c.jj 2001-08-23 12:48:21.000000000 -0400
+++ libc/malloc/memusagestat.c 2003-03-21 09:29:53.000000000 -0500
@@ -405,7 +405,7 @@ main (int argc, char *argv[])
}
- snprintf (buf, sizeof (buf), "%llu", total);
+ snprintf (buf, sizeof (buf), "%llu", (unsigned long long) total);
gdImageString (im_out, gdFontSmall, xsize - 50, ysize - 14, buf, blue);
if (!time_based)
--- libc/sysdeps/generic/bits/atomic.h.jj 2003-03-20 01:56:50.000000000 -0500
+++ libc/sysdeps/generic/bits/atomic.h 2003-03-21 12:32:06.000000000 -0500
@@ -25,7 +25,19 @@
up with real definitions. */
/* The only basic operation needed is compare and exchange. */
-#define arch_compare_and_exchange_acq(mem, newval, oldval) \
- ({ *(mem) == (oldval) ? 1 : (*(mem) = (newval), 0); })
+#define atomic_compare_and_exchange_val_acq(mem, newval, oldval) \
+ ({ __typeof (mem) __gmemp = (mem); \
+ __typeof (*mem) __gret = *__gmemp; \
+ __typeof (*mem) __gnewval = (newval); \
+ \
+ if (__gret == (oldval)) \
+ *__gmemp = __gnewval; \
+ __gret; })
+
+#define atomic_compare_and_exchange_bool_acq(mem, newval, oldval) \
+ ({ __typeof (mem) __gmemp = (mem); \
+ __typeof (*mem) __gnewval = (newval); \
+ \
+ *__gmemp == (oldval) ? (*__gmemp = __gnewval, 0) : 1; })
#endif /* bits/atomic.h */
--- libc/sysdeps/ia64/bits/atomic.h.jj 2003-03-21 05:17:32.000000000 -0500
+++ libc/sysdeps/ia64/bits/atomic.h 2003-03-21 11:13:16.000000000 -0500
@@ -78,30 +78,49 @@ typedef uintmax_t uatomic_max_t;
__sync_lock_test_and_set_si (mem, value)
#define atomic_exchange_and_add(mem, value) \
- ({ \
- __typeof (*mem) __oldval, __val; \
- __typeof (mem) __memp = (mem); \
- __typeof (*mem) __value = (value); \
+ ({ __typeof (*mem) __oldval, __val; \
+ __typeof (mem) __memp = (mem); \
+ __typeof (*mem) __value = (value); \
\
- __val = *__memp; \
- if (sizeof (*mem) == 4) \
- do \
- { \
- __oldval = __val; \
- __val = __arch_compare_and_exchange_32_val_acq (__memp, \
- __oldval + __value, \
- __oldval); \
- } \
- while (__builtin_expect (__val != __oldval, 0)); \
- else if (sizeof (*mem) == 8) \
- do \
- { \
- __oldval = __val; \
- __val = __arch_compare_and_exchange_64_val_acq (__memp, \
- __oldval + __value, \
- __oldval); \
- } \
- while (__builtin_expect (__val != __oldval, 0)); \
- else \
- abort (); \
- __oldval + __value; })
+ __val = (*__memp); \
+ do \
+ { \
+ __oldval = __val; \
+ __val = atomic_compare_and_exchange_val_acq (__memp, \
+ __oldval + __value, \
+ __oldval); \
+ } \
+ while (__builtin_expect (__val != __oldval, 0)); \
+ __oldval; })
+
+#define atomic_decrement_if_positive(mem) \
+ ({ __typeof (*mem) __oldval, __val; \
+ __typeof (mem) __memp = (mem); \
+ \
+ __val = (*__memp); \
+ do \
+ { \
+ __oldval = __val; \
+ if (__builtin_expect (__val <= 0, 0)) \
+ break; \
+ __val = atomic_compare_and_exchange_val_acq (__memp, __oldval - 1, \
+ __oldval); \
+ } \
+ while (__builtin_expect (__val != __oldval, 0)); \
+ __oldval; })
+
+#define atomic_bit_test_set(mem, bit) \
+ ({ __typeof (*mem) __oldval, __val; \
+ __typeof (mem) __memp = (mem); \
+ __typeof (*mem) __mask = ((__typeof (*mem)) 1 << (bit)); \
+ \
+ __val = (*__memp); \
+ do \
+ { \
+ __oldval = __val; \
+ __val = atomic_compare_and_exchange_val_acq (__memp, \
+ __oldval | __mask, \
+ __oldval); \
+ } \
+ while (__builtin_expect (__val != __oldval, 0)); \
+ __oldval & __mask; })
--- libc/sysdeps/s390/bits/atomic.h.jj 2003-03-21 05:17:34.000000000 -0500
+++ libc/sysdeps/s390/bits/atomic.h 2003-03-21 08:23:32.000000000 -0500
@@ -45,34 +45,32 @@ typedef intmax_t atomic_max_t;
typedef uintmax_t uatomic_max_t;
-#define __arch_compare_and_exchange_bool_8_acq(mem, newval, oldval) \
+#define __arch_compare_and_exchange_val_8_acq(mem, newval, oldval) \
(abort (), 0)
-#define __arch_compare_and_exchange_bool_16_acq(mem, newval, oldval) \
+#define __arch_compare_and_exchange_val_16_acq(mem, newval, oldval) \
(abort (), 0)
-#define __arch_compare_and_exchange_bool_32_acq(mem, newval, oldval) \
+#define __arch_compare_and_exchange_val_32_acq(mem, newval, oldval) \
({ unsigned int *__mem = (unsigned int *) (mem); \
unsigned int __old = (unsigned int) (oldval); \
- unsigned int __cmp = __old; \
__asm __volatile ("cs %0,%2,%1" \
: "+d" (__old), "=Q" (*__mem) \
: "d" (newval), "m" (*__mem) : "cc" ); \
- __cmp != __old; })
+ __old; })
#ifdef __s390x__
-# define __arch_compare_and_exchange_bool_64_acq(mem, newval, oldval) \
+# define __arch_compare_and_exchange_val_64_acq(mem, newval, oldval) \
({ unsigned long int *__mem = (unsigned long int *) (mem); \
unsigned long int __old = (unsigned long int) (oldval); \
- unsigned long int __cmp = __old; \
__asm __volatile ("csg %0,%2,%1" \
: "+d" (__old), "=Q" (*__mem) \
: "d" (newval), "m" (*__mem) : "cc" ); \
- __cmp != __old; })
+ __old; })
#else
/* For 31 bit we do not really need 64-bit compare-and-exchange. We can
implement them by use of the csd instruction. The straightforward
implementation causes warnings so we skip the definition for now. */
-# define __arch_compare_and_exchange_bool_64_acq(mem, newval, oldval) \
+# define __arch_compare_and_exchange_val_64_acq(mem, newval, oldval) \
(abort (), 0)
#endif
--- libc/sysdeps/s390/Versions.jj 2003-01-28 05:32:49.000000000 -0500
+++ libc/sysdeps/s390/Versions 2003-03-21 09:24:20.000000000 -0500
@@ -3,4 +3,4 @@ ld {
# runtime interface to TLS
__tls_get_offset;
}
-}
\ No newline at end of file
+}
--- libc/sysdeps/unix/sysv/linux/sysconf.c.jj 2003-03-21 09:33:38.000000000 -0500
+++ libc/sysdeps/unix/sysv/linux/sysconf.c 2003-03-21 09:36:34.000000000 -0500
@@ -40,7 +40,8 @@ __sysconf (int name)
{
struct timespec ts;
INTERNAL_SYSCALL_DECL (err);
- int r = INTERNAL_SYSCALL (clock_getres, err, 2, CLOCK_MONOTONIC, &ts);
+ int r;
+ r = INTERNAL_SYSCALL (clock_getres, err, 2, CLOCK_MONOTONIC, &ts);
return INTERNAL_SYSCALL_ERROR_P (r, err) ? 0 : 1;
}
#endif
--- libc/sysdeps/x86_64/bits/atomic.h.jj 2003-03-21 05:17:40.000000000 -0500
+++ libc/sysdeps/x86_64/bits/atomic.h 2003-03-21 10:00:50.000000000 -0500
@@ -291,7 +291,7 @@ typedef uintmax_t uatomic_max_t;
else \
__asm __volatile (LOCK "orq %q2, %0" \
: "=m" (*mem) \
- : "m" (*mem), "i" (1 << (bit))); \
+ : "m" (*mem), "i" (1L << (bit))); \
})
Jakub
| Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
|---|---|---|
| Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |