[PATCH] Fix getpid/raise
Jakub Jelinek
jakub@redhat.com
Tue Dec 23 16:53:00 GMT 2003
Hi!
The attached testcase fails with current CVS (when not linked against
-lpthread):
tgkill(32233, 32233, SIGUSR1) = 0
--- SIGUSR1 (User defined signal 1) @ 0 (0) ---
sigreturn() = ? (mask now [])
tgkill(0, 32233, SIGUSR1) = -1 EINVAL (Invalid argument)
Don't know where to put the tst-raise1.c testcase, signal/ or somewhere
else (nptl/ is bad, since it should not be linked against -lpthread)?
Additionally, I think there is a race. If fork () is called when getpid ()
has never been called before in the program, then after the self->pid =
-parentpid; in fork.c but before the clone () syscall a signal is received
which calls getpid () and another signal is received in the child before
self->pid = self->tid setting, getpid () in that signal handler will return
incorrect value.
The attached patch should cure both. Performance wise, I tried:
#include <stdio.h>
#include <dlfcn.h>
int main (void)
{
int i;
unsigned long long st, en, mi = ~0ULL;
for (i = 0; i < 100; ++i)
{
asm volatile ("rdtsc" : "=A" (st));
getpid ();
asm volatile ("rdtsc" : "=A" (en));
if (en - st < mi)
mi = en - st;
}
printf ("%lld ticks\n", mi);
dlopen ("libpthread.so.0", RTLD_LAZY);
mi = ~0ULL;
for (i = 0; i < 100; ++i)
{
asm volatile ("rdtsc" : "=A" (st));
getpid ();
asm volatile ("rdtsc" : "=A" (en));
if (en - st < mi)
mi = en - st;
}
printf ("%lld ticks\n", mi);
}
on my PIII, against the installed glibc which doesn't yet have any of the
getpid changes it prints
356 ticks
357 ticks
against CVS libc.so:
37 ticks
39 ticks
and against CVS+this patch:
41 ticks
38 ticks
I've moved pid field closer to tid, so that they share a cache line when
they are used so often together.
Jakub
-------------- next part --------------
2003-12-23 Jakub Jelinek <jakub@redhat.com>
* sysdeps/unix/sysv/linux/raise.c (raise): Protect pid = selftid
setting with __ASSUME_TGKILL || defined __NR_tgkill.
If pid is 0, set it to selftid.
* sysdeps/unix/sysv/linux/getpid.c (really_getpid): Make inline.
Don't set self->pid but self->tid. If self->pid == 0 and self->tid
!= 0, return self->tid without doing a syscall.
* descr.h (struct pthread): Move pid field after tid.
--- libc/nptl/sysdeps/unix/sysv/linux/raise.c.jj 2003-12-21 12:55:41.000000000 +0100
+++ libc/nptl/sysdeps/unix/sysv/linux/raise.c 2003-12-23 15:11:10.000000000 +0100
@@ -44,17 +44,19 @@ raise (sig)
#endif
THREAD_SETMEM (pd, tid, selftid);
+#if __ASSUME_TGKILL || defined __NR_tgkill
/* We do not set the PID field in the TID here since we might be
called from a signal handler while the thread executes fork. */
pid = selftid;
+#endif
}
#if __ASSUME_TGKILL || defined __NR_tgkill
else
/* raise is an async-safe function. It could be called while the
fork function temporarily invalidated the PID field. Adjust for
that. */
- if (__builtin_expect (pid < 0, 0))
- pid = -pid;
+ if (__builtin_expect (pid <= 0, 0))
+ pid = pid == 0 ? selftid : -pid;
#endif
#if __ASSUME_TGKILL
--- libc/nptl/sysdeps/unix/sysv/linux/getpid.c.jj 2003-12-21 12:55:41.000000000 +0100
+++ libc/nptl/sysdeps/unix/sysv/linux/getpid.c 2003-12-23 16:07:57.000000000 +0100
@@ -23,32 +23,42 @@
#ifndef NOT_IN_libc
-static pid_t really_getpid (pid_t oldval);
-#endif
-
+static inline __attribute__((always_inline)) pid_t really_getpid (pid_t oldval);
-pid_t
-__getpid (void)
+static inline __attribute__((always_inline)) pid_t
+really_getpid (pid_t oldval)
{
-#ifndef NOT_IN_libc
- pid_t result = THREAD_GETMEM (THREAD_SELF, pid);
- if (__builtin_expect (result <= 0, 0))
- result = really_getpid (result);
+ pid_t selftid;
+ if (__builtin_expect (oldval == 0
+ && ((selftid = THREAD_GETMEM (THREAD_SELF, tid))
+ != 0), 1))
+ return selftid;
+
+ INTERNAL_SYSCALL_DECL (err);
+ pid_t result = INTERNAL_SYSCALL (getpid, err, 0);
+
+ /* We do not set the PID field in the TID here since we might be
+ called from a signal handler while the thread executes fork. */
+ if (oldval == 0)
+ THREAD_SETMEM (THREAD_SELF, tid, result);
return result;
}
+#endif
-static pid_t
-really_getpid (pid_t oldval)
+pid_t
+__getpid (void)
{
-#endif
+#ifdef NOT_IN_libc
INTERNAL_SYSCALL_DECL (err);
pid_t result = INTERNAL_SYSCALL (getpid, err, 0);
-#ifndef NOT_IN_libc
- if (oldval == 0)
- THREAD_SETMEM (THREAD_SELF, pid, result);
+#else
+ pid_t result = THREAD_GETMEM (THREAD_SELF, pid);
+ if (__builtin_expect (result <= 0, 0))
+ result = really_getpid (result);
#endif
return result;
}
+
libc_hidden_def (__getpid)
weak_alias (__getpid, getpid)
libc_hidden_def (getpid)
--- libc/nptl/descr.h.jj 2003-07-23 01:04:00.000000000 +0200
+++ libc/nptl/descr.h 2003-12-23 16:17:55.000000000 +0100
@@ -121,6 +121,9 @@ struct pthread
therefore stack) used' flag. */
pid_t tid;
+ /* Process ID - thread group ID in kernel speak. */
+ pid_t pid;
+
/* List of cleanup buffers. */
struct _pthread_cleanup_buffer *cleanup;
@@ -178,9 +181,6 @@ struct pthread
/* Two-level array for the thread-specific data. */
struct pthread_key_data *specific[PTHREAD_KEY_1STLEVEL_SIZE];
- /* Process ID - thread group ID in kernel speak. */
- pid_t pid;
-
/* True if events must be reported. */
bool report_events;
-------------- next part --------------
#include <errno.h>
#include <error.h>
#include <signal.h>
#include <stdlib.h>
volatile int count;
void
sh (int sig)
{
++count;
}
int
main (void)
{
struct sigaction sa;
sa.sa_handler = sh;
sigemptyset (&sa.sa_mask);
sa.sa_flags = 0;
if (sigaction (SIGUSR1, &sa, NULL) < 0)
{
printf ("sigaction failed: %m\n");
exit (1);
}
if (raise (SIGUSR1) < 0)
{
printf ("first raise failed: %m\n");
exit (1);
}
if (raise (SIGUSR1) < 0)
{
printf ("second raise failed: %m\n");
exit (1);
}
if (count != 2)
{
printf ("signal handler not called 2 times\n");
exit (1);
}
exit (0);
}
More information about the Libc-hacker
mailing list