* [PATCH] Fix pthread_condattr_setclock (BZ #7008)
@ 2008-11-12 9:55 Jakub Jelinek
0 siblings, 0 replies; only message in thread
From: Jakub Jelinek @ 2008-11-12 9:55 UTC (permalink / raw)
To: Ulrich Drepper; +Cc: Glibc hackers
Hi!
As can be seen on the attached testcase, pthread_condattr_setclock.c
is wrong, in addition to setting the desired clock it also sets pshared
to PTHREAD_PROCESS_PRIVATE. COND_NWAITERS_SHIFT is ATM 1, so
(*valuep & 1) is pshared bit and (*valuep & 2) >> 1 is the clock id bit.
So, to just set the clock id and not anything else,
pthread_condattr_setclock wants to do
*valuep = (*valuep & ~2) | (clock_id << 1),
but it actually does:
*valuep = (*valuep & ~4 & ~1) | (clock_id << 1)
i.e.
*valuep = (*valuep & ~5) | (clock_id << 1). Also, in
pthread_condattr_setclock (&ca, CLOCK_MONOTONIC);
pthread_condattr_setclock (&ca, CLOCK_REALTIME);
the second pthread_condattr_setclock will not do anything (except for
acting as pthread_condattr_setpshared (&ca, PTHREAD_PROCESS_PRIVATE);),
clock id will remain CLOCK_MONOTONIC.
The following patch uses
*valuep = (*valuep & ~(((1 << COND_NWAITERS_SHIFT) - 1) << 1)) | (clock_id << 1)
i.e.
*valuep = (*valuep & ~(((1 << 1) - 1) << 1)) | (clock_id << 1)
*valuep = (*valuep & ~2) | (clock_id << 1)
Alternatively, we could use:
*valuep = (*valuep & ~(((1 << (COND_NWAITERS_SHIFT + 1)) - 1) & ~1)) | (clock_id << 1)
which is also
*valuep = (*valuep & ~2) | (clock_id << 1)
in the end, but that's IMHO longer and less readable.
This patch also fixes pthread_cond_init, which has very weird expression,
which happens to be correct for COND_NWAITERS_SHIFT == 1, but is wrong for
all the other values of that macro.
2008-11-12 Jakub Jelinek <jakub@redhat.com>
[BZ #7008]
* pthread_condattr_setclock.c (pthread_condattr_setclock): Fix masking
of old value.
* pthread_cond_init.c (__pthread_cond_init): Fix
cond->__data.__nwaiters initialization.
* Makefile (tests): Add tst-cond23.
* tst-cond23.c: New test.
--- libc/nptl/pthread_condattr_setclock.c.jj 2007-06-04 08:42:05.000000000 +0200
+++ libc/nptl/pthread_condattr_setclock.c 2008-11-12 09:36:03.000000000 +0100
@@ -1,4 +1,4 @@
-/* Copyright (C) 2003, 2004, 2007 Free Software Foundation, Inc.
+/* Copyright (C) 2003, 2004, 2007, 2008 Free Software Foundation, Inc.
This file is part of the GNU C Library.
Contributed by Ulrich Drepper <drepper@redhat.com>, 2003.
@@ -66,7 +66,7 @@ pthread_condattr_setclock (attr, clock_i
int *valuep = &((struct pthread_condattr *) attr)->value;
- *valuep = ((*valuep & ~(1 << (COND_NWAITERS_SHIFT + 1)) & ~1)
+ *valuep = ((*valuep & ~(((1 << COND_NWAITERS_SHIFT) - 1) << 1))
| (clock_id << 1));
return 0;
--- libc/nptl/pthread_cond_init.c.jj 2007-08-01 10:50:19.000000000 +0200
+++ libc/nptl/pthread_cond_init.c 2008-11-12 09:15:34.000000000 +0100
@@ -1,4 +1,5 @@
-/* Copyright (C) 2002, 2003, 2004, 2005, 2007 Free Software Foundation, Inc.
+/* Copyright (C) 2002, 2003, 2004, 2005, 2007, 2008
+ Free Software Foundation, Inc.
This file is part of the GNU C Library.
Contributed by Ulrich Drepper <drepper@redhat.com>, 2002.
@@ -31,8 +32,9 @@ __pthread_cond_init (cond, cond_attr)
cond->__data.__lock = LLL_LOCK_INITIALIZER;
cond->__data.__futex = 0;
cond->__data.__nwaiters = (icond_attr != NULL
- && ((icond_attr->value
- & (COND_NWAITERS_SHIFT << 1)) >> 1));
+ ? ((icond_attr->value >> 1)
+ & ((1 << COND_NWAITERS_SHIFT) - 1))
+ : CLOCK_REALTIME);
cond->__data.__total_seq = 0;
cond->__data.__wakeup_seq = 0;
cond->__data.__woken_seq = 0;
--- libc/nptl/Makefile.jj 2008-07-08 12:57:27.000000000 +0200
+++ libc/nptl/Makefile 2008-11-12 10:19:46.000000000 +0100
@@ -205,7 +205,7 @@ tests = tst-typesizes \
tst-cond1 tst-cond2 tst-cond3 tst-cond4 tst-cond5 tst-cond6 tst-cond7 \
tst-cond8 tst-cond9 tst-cond10 tst-cond11 tst-cond12 tst-cond13 \
tst-cond14 tst-cond15 tst-cond16 tst-cond17 tst-cond18 tst-cond19 \
- tst-cond20 tst-cond21 tst-cond22 \
+ tst-cond20 tst-cond21 tst-cond22 tst-cond23 \
tst-robust1 tst-robust2 tst-robust3 tst-robust4 tst-robust5 \
tst-robust6 tst-robust7 tst-robust8 tst-robust9 \
tst-robustpi1 tst-robustpi2 tst-robustpi3 tst-robustpi4 tst-robustpi5 \
--- libc/nptl/tst-cond23.c.jj 2008-11-12 09:52:28.000000000 +0100
+++ libc/nptl/tst-cond23.c 2008-11-12 10:19:07.000000000 +0100
@@ -0,0 +1,184 @@
+/* Copyright (C) 2008 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+ Contributed by Jakub Jelinek <jakub@redhat.com>, 2008.
+
+ 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 <errno.h>
+#include <pthread.h>
+#include <stdio.h>
+#include <time.h>
+#include <unistd.h>
+
+
+#if defined _POSIX_CLOCK_SELECTION && _POSIX_CLOCK_SELECTION >= 0
+static int
+check (pthread_condattr_t *condattr, int pshared, clockid_t cl)
+{
+ clockid_t cl2;
+ if (pthread_condattr_getclock (condattr, &cl2) != 0)
+ {
+ puts ("condattr_getclock failed");
+ return 1;
+ }
+ if (cl != cl2)
+ {
+ printf ("condattr_getclock returned wrong value: %d, expected %d\n",
+ (int) cl2, (int) cl);
+ return 1;
+ }
+
+ int p;
+ if (pthread_condattr_getpshared (condattr, &p) != 0)
+ {
+ puts ("condattr_getpshared failed");
+ return 1;
+ }
+ else if (p != pshared)
+ {
+ printf ("condattr_getpshared returned wrong value: %d, expected %d\n",
+ p, pshared);
+ return 1;
+ }
+
+ return 0;
+}
+
+static int
+run_test (clockid_t cl)
+{
+ pthread_condattr_t condattr;
+
+ printf ("clock = %d\n", (int) cl);
+
+ if (pthread_condattr_init (&condattr) != 0)
+ {
+ puts ("condattr_init failed");
+ return 1;
+ }
+
+ if (check (&condattr, PTHREAD_PROCESS_PRIVATE, CLOCK_REALTIME))
+ return 1;
+
+ if (pthread_condattr_setpshared (&condattr, PTHREAD_PROCESS_SHARED) != 0)
+ {
+ puts ("1st condattr_setpshared failed");
+ return 1;
+ }
+
+ if (check (&condattr, PTHREAD_PROCESS_SHARED, CLOCK_REALTIME))
+ return 1;
+
+ if (pthread_condattr_setclock (&condattr, cl) != 0)
+ {
+ puts ("1st condattr_setclock failed");
+ return 1;
+ }
+
+ if (check (&condattr, PTHREAD_PROCESS_SHARED, cl))
+ return 1;
+
+ if (pthread_condattr_setpshared (&condattr, PTHREAD_PROCESS_PRIVATE) != 0)
+ {
+ puts ("2nd condattr_setpshared failed");
+ return 1;
+ }
+
+ if (check (&condattr, PTHREAD_PROCESS_PRIVATE, cl))
+ return 1;
+
+ if (pthread_condattr_setclock (&condattr, CLOCK_REALTIME) != 0)
+ {
+ puts ("2nd condattr_setclock failed");
+ return 1;
+ }
+
+ if (check (&condattr, PTHREAD_PROCESS_PRIVATE, CLOCK_REALTIME))
+ return 1;
+
+ if (pthread_condattr_setclock (&condattr, cl) != 0)
+ {
+ puts ("3rd condattr_setclock failed");
+ return 1;
+ }
+
+ if (check (&condattr, PTHREAD_PROCESS_PRIVATE, cl))
+ return 1;
+
+ if (pthread_condattr_setpshared (&condattr, PTHREAD_PROCESS_SHARED) != 0)
+ {
+ puts ("3rd condattr_setpshared failed");
+ return 1;
+ }
+
+ if (check (&condattr, PTHREAD_PROCESS_SHARED, cl))
+ return 1;
+
+ if (pthread_condattr_setclock (&condattr, CLOCK_REALTIME) != 0)
+ {
+ puts ("4th condattr_setclock failed");
+ return 1;
+ }
+
+ if (check (&condattr, PTHREAD_PROCESS_SHARED, CLOCK_REALTIME))
+ return 1;
+
+ if (pthread_condattr_destroy (&condattr) != 0)
+ {
+ puts ("condattr_destroy failed");
+ return 1;
+ }
+
+ return 0;
+}
+#endif
+
+
+static int
+do_test (void)
+{
+#if !defined _POSIX_CLOCK_SELECTION || _POSIX_CLOCK_SELECTION == -1
+
+ puts ("_POSIX_CLOCK_SELECTION not supported, test skipped");
+ return 0;
+
+#else
+
+ int res = run_test (CLOCK_REALTIME);
+
+# if defined _POSIX_MONOTONIC_CLOCK && _POSIX_MONOTONIC_CLOCK >= 0
+# if _POSIX_MONOTONIC_CLOCK == 0
+ int e = sysconf (_SC_MONOTONIC_CLOCK);
+ if (e < 0)
+ puts ("CLOCK_MONOTONIC not supported");
+ else if (e == 0)
+ {
+ puts ("sysconf (_SC_MONOTONIC_CLOCK) must not return 0");
+ res = 1;
+ }
+ else
+# endif
+ res |= run_test (CLOCK_MONOTONIC);
+# else
+ puts ("_POSIX_MONOTONIC_CLOCK not defined");
+# endif
+
+ return res;
+#endif
+}
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"
Jakub
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2008-11-12 9:55 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-11-12 9:55 [PATCH] Fix pthread_condattr_setclock (BZ #7008) Jakub Jelinek
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).