From patchwork Mon Oct 10 18:03:32 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Adhemerval Zanella Netto X-Patchwork-Id: 77451 Delivered-To: patch@linaro.org Received: by 10.140.97.247 with SMTP id m110csp33633qge; Mon, 10 Oct 2016 11:04:01 -0700 (PDT) X-Received: by 10.66.166.238 with SMTP id zj14mr55813520pab.205.1476122641711; Mon, 10 Oct 2016 11:04:01 -0700 (PDT) Return-Path: Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id l5si5574575pan.265.2016.10.10.11.04.01 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 10 Oct 2016 11:04:01 -0700 (PDT) Received-SPF: pass (google.com: domain of libc-alpha-return-73830-patch=linaro.org@sourceware.org designates 209.132.180.131 as permitted sender) client-ip=209.132.180.131; Authentication-Results: mx.google.com; dkim=pass header.i=@sourceware.org; spf=pass (google.com: domain of libc-alpha-return-73830-patch=linaro.org@sourceware.org designates 209.132.180.131 as permitted sender) smtp.mailfrom=libc-alpha-return-73830-patch=linaro.org@sourceware.org; dmarc=fail (p=NONE dis=NONE) header.from=linaro.org DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:subject:to:references:from:message-id:date :mime-version:in-reply-to:content-type :content-transfer-encoding; q=dns; s=default; b=WhuGH+Dg8A8dW96m 9uX1iwTvHZ6AfRJ+mwA/bVX8WPPmUTcgdWynHcEUC2EsPBYVTl3RT+DJPA1gmWWa 6tMu6y6bdHXqv4famkCyKK3eNhPJTiuckCaDrmXwyEzp5sg98JNXOTCS78L2VxlJ H5ed5SETFW4XarCd3+wAbOQyTuA= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:subject:to:references:from:message-id:date :mime-version:in-reply-to:content-type :content-transfer-encoding; s=default; bh=ryPRttUEhjROHCWATWkvoC mYBRo=; b=CgRYFWGcS20GQzBg499I+ZeD0vkPE2WtiUOz2dboF1nyLASzPa1g5x lKwKq3+CywHeHabGXLHyqV2L1KoWjU9pQF63JU0meu83Rtc71AZ1piJPuq2r0gGj QAgkp/2lGFJFGSvIjwZFHBZaBhUn9ivLnYpGOf3eKOq8NnsUZ0iyA= Received: (qmail 120476 invoked by alias); 10 Oct 2016 18:03:51 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 120467 invoked by uid 89); 10 Oct 2016 18:03:50 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.4 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_SPAM, SPF_PASS autolearn=no version=3.3.2 spammy=circumvent, Safety, sysdeps, H*r:10.0.0 X-HELO: mail-vk0-f42.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=D6+zSljINn8Q3GCnPwfPRaP2g+iSuN1g5M+relVa3BE=; b=APnhgDnTrt3tDsO9mgeyngpZQr+LCC2lEq2AVyGNt2+EuKOgwapFJ1yvZOOaWxGMzc jK7ZVebm9UPsGt+Gc+QO0K/jtwSRrLmL3Y//IBZwvkGtDFWMq8dedJoF4EbmMCkQb6Mz wKrRjUMG9sTijp42pfvVmZs4Pt14oo0+R4bLKUwEdl7xR1vgsEt5OH939JqgKoa1tfjS mCnxn8h5gWFkrQq+ulh2uC1ypZulL2Qns3X5A+SgcxNBrbmp65znxy4CqmnvAZ5tF1vd fvW2IYYvh0UAqxxY8wnhl0aDTrRS0e7WlUFddO9Wf7xHUWW7JgiDXpJdAccBWCDSHTdG akJA== X-Gm-Message-State: AA6/9RlAUsYRi7qM4HA8vJRslqW3vRcLR+XIz0UAKwbIOaJJIB9q0GX/Pb7/869WxHfXRhMR X-Received: by 10.31.108.70 with SMTP id h67mr13182883vkc.163.1476122618350; Mon, 10 Oct 2016 11:03:38 -0700 (PDT) Subject: Re: Caching of PID/TID after fork To: libc-alpha@sourceware.org References: <87y4202blm.fsf@mid.deneb.enyo.de> <87mvidj0qw.fsf@mid.deneb.enyo.de> From: Adhemerval Zanella Message-ID: Date: Mon, 10 Oct 2016 15:03:32 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 MIME-Version: 1.0 In-Reply-To: On 09/10/2016 11:19, Robert Święcki wrote: >>> Though, one useful feature which could be potentially useful for some >>> porjects would be to create a detached process (for which the kernel >>> doesn't send termination signal - usually SIGCHLD - to the parent >>> process). And this cannot be solved by ORing custom flags with fork >>> ones, because SIGCHLD is already there. But, it's just a guess. >> >> Yes, I know of some cases where this would have helped as well. >> >> If we go the fork-with-flags route, we should translate glibc-specific >> flags to kernel flags anyway because the kernel might introduce new >> flags which break the interface in subtle ways (see O_TMPFILE and mode >> argument handling). > > An initial stab at the interface; it should avoid the problem you've > described above? I don't think additional rewrite of clone flags would > be required with such interface, even with future changes in the > kernel - in any case, using it requires good understanding of the > underlying kernel's clone() interface. > > /* fork with flags */ > pid_t ffork(int mode, unsigned long flags); > > mode: > FFORK_FLAG_SET - set flags directly > FFORK_FLAG_OR - append flags to whatever fork uses internally > > flags: > as with clone() > > ret val / errno: > as with fork() > I really do not think adding a another GLIBC/GNU specific extension to circumvent a possible misused implementation detail should be way to handle this specific issue. For the pid/tid caching problem I see it is feasible to just get rid of pid caching and adjust the implementation that rely on this. The cached pid is used basically in tgkill interface on both setxid, sigcancel checking, pthread_{kill,cancel,sigqueue}, and getpid. And for all implementation it used basically as an optimization to avoid the syscall, while 'fork' has a lot of internal control to avoid invalidate its value. We can just replace the cached value with a direct syscall with some less performance (the getpid syscall), while both the {v}fork arch defined implementation would be simpler (including getpid). Also, fork won't require any set/reset the cached value, which also only adds complexity. It also simplifies the nptl_db code as well, since no extra check would require to validate the pid cached value between forks. Now, about the stack argument checking, I also think it is now feasible to remove since clone itself won't require to operate in any thread implicit data. What I am not sure if this semantic change would require a compatibility symbol. Below is a RFC patch to implement the first options (pid caching removal) only for x86_64. I think I covered all cases of caching PID field (I basically renamed its field and covered all the build failures). It shows no regression on x86_64. diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c index 3016a2e..98a0ea2 100644 --- a/nptl/allocatestack.c +++ b/nptl/allocatestack.c @@ -438,9 +438,6 @@ allocate_stack (const struct pthread_attr *attr, struct pthread **pdp, SETUP_THREAD_SYSINFO (pd); #endif - /* The process ID is also the same as that of the caller. */ - pd->pid = THREAD_GETMEM (THREAD_SELF, pid); - /* Don't allow setxid until cloned. */ pd->setxid_futex = -1; @@ -577,9 +574,6 @@ allocate_stack (const struct pthread_attr *attr, struct pthread **pdp, /* Don't allow setxid until cloned. */ pd->setxid_futex = -1; - /* The process ID is also the same as that of the caller. */ - pd->pid = THREAD_GETMEM (THREAD_SELF, pid); - /* Allocate the DTV for this thread. */ if (_dl_allocate_tls (TLS_TPADJ (pd)) == NULL) { @@ -873,9 +867,6 @@ __reclaim_stacks (void) /* This marks the stack as free. */ curp->tid = 0; - /* The PID field must be initialized for the new process. */ - curp->pid = self->pid; - /* Account for the size of the stack. */ stack_cache_actsize += curp->stackblock_size; @@ -901,13 +892,6 @@ __reclaim_stacks (void) } } - /* Reset the PIDs in any cached stacks. */ - list_for_each (runp, &stack_cache) - { - struct pthread *curp = list_entry (runp, struct pthread, list); - curp->pid = self->pid; - } - /* Add the stack of all running threads to the cache. */ list_splice (&stack_used, &stack_cache); @@ -1052,9 +1036,9 @@ setxid_signal_thread (struct xid_command *cmdp, struct pthread *t) return 0; int val; + pid_t pid = __getpid (); INTERNAL_SYSCALL_DECL (err); - val = INTERNAL_SYSCALL (tgkill, err, 3, THREAD_GETMEM (THREAD_SELF, pid), - t->tid, SIGSETXID); + val = INTERNAL_SYSCALL_CALL (tgkill, err, pid, t->tid, SIGSETXID); /* If this failed, it must have had not started yet or else exited. */ if (!INTERNAL_SYSCALL_ERROR_P (val, err)) diff --git a/nptl/descr.h b/nptl/descr.h index 8e4938d..17a2c9f 100644 --- a/nptl/descr.h +++ b/nptl/descr.h @@ -167,7 +167,7 @@ struct pthread therefore stack) used' flag. */ pid_t tid; - /* Process ID - thread group ID in kernel speak. */ + /* Ununsed. */ pid_t pid; /* List of robust mutexes the thread is holding. */ diff --git a/nptl/nptl-init.c b/nptl/nptl-init.c index bdbdfed..48fab50 100644 --- a/nptl/nptl-init.c +++ b/nptl/nptl-init.c @@ -184,18 +184,12 @@ __nptl_set_robust (struct pthread *self) static void sigcancel_handler (int sig, siginfo_t *si, void *ctx) { - /* Determine the process ID. It might be negative if the thread is - in the middle of a fork() call. */ - pid_t pid = THREAD_GETMEM (THREAD_SELF, pid); - if (__glibc_unlikely (pid < 0)) - pid = -pid; - /* Safety check. It would be possible to call this function for other signals and send a signal from another process. This is not correct and might even be a security problem. Try to catch as many incorrect invocations as possible. */ if (sig != SIGCANCEL - || si->si_pid != pid + || si->si_pid != __getpid() || si->si_code != SI_TKILL) return; @@ -243,19 +237,14 @@ struct xid_command *__xidcmd attribute_hidden; static void sighandler_setxid (int sig, siginfo_t *si, void *ctx) { - /* Determine the process ID. It might be negative if the thread is - in the middle of a fork() call. */ - pid_t pid = THREAD_GETMEM (THREAD_SELF, pid); int result; - if (__glibc_unlikely (pid < 0)) - pid = -pid; /* Safety check. It would be possible to call this function for other signals and send a signal from another process. This is not correct and might even be a security problem. Try to catch as many incorrect invocations as possible. */ if (sig != SIGSETXID - || si->si_pid != pid + || si->si_pid != __getpid () || si->si_code != SI_TKILL) return; diff --git a/nptl/pthread_cancel.c b/nptl/pthread_cancel.c index 1419baf..89d02e1 100644 --- a/nptl/pthread_cancel.c +++ b/nptl/pthread_cancel.c @@ -22,7 +22,7 @@ #include "pthreadP.h" #include #include - +#include int pthread_cancel (pthread_t th) @@ -66,19 +66,11 @@ pthread_cancel (pthread_t th) #ifdef SIGCANCEL /* The cancellation handler will take care of marking the thread as canceled. */ - INTERNAL_SYSCALL_DECL (err); - - /* One comment: The PID field in the TCB can temporarily be - changed (in fork). But this must not affect this code - here. Since this function would have to be called while - the thread is executing fork, it would have to happen in - a signal handler. But this is no allowed, pthread_cancel - is not guaranteed to be async-safe. */ - int val; - val = INTERNAL_SYSCALL (tgkill, err, 3, - THREAD_GETMEM (THREAD_SELF, pid), pd->tid, - SIGCANCEL); + pid_t pid = getpid (); + INTERNAL_SYSCALL_DECL (err); + int val = INTERNAL_SYSCALL_CALL (tgkill, err, pid, pd->tid, + SIGCANCEL); if (INTERNAL_SYSCALL_ERROR_P (val, err)) result = INTERNAL_SYSCALL_ERRNO (val, err); #else diff --git a/nptl/pthread_getattr_np.c b/nptl/pthread_getattr_np.c index fb906f0..32d7484 100644 --- a/nptl/pthread_getattr_np.c +++ b/nptl/pthread_getattr_np.c @@ -68,7 +68,6 @@ pthread_getattr_np (pthread_t thread_id, pthread_attr_t *attr) { /* No stack information available. This must be for the initial thread. Get the info in some magical way. */ - assert (abs (thread->pid) == thread->tid); /* Stack size limit. */ struct rlimit rl; diff --git a/nptl_db/td_ta_thr_iter.c b/nptl_db/td_ta_thr_iter.c index a990fed..9e50599 100644 --- a/nptl_db/td_ta_thr_iter.c +++ b/nptl_db/td_ta_thr_iter.c @@ -76,48 +76,28 @@ iterate_thread_list (td_thragent_t *ta, td_thr_iter_f *callback, if (ps_pdread (ta->ph, addr, copy, ta->ta_sizeof_pthread) != PS_OK) return TD_ERR; - /* Verify that this thread's pid field matches the child PID. - If its pid field is negative, it's about to do a fork or it - is the sole thread in a fork child. */ - psaddr_t pid; - err = DB_GET_FIELD_LOCAL (pid, ta, copy, pthread, pid, 0); - if (err == TD_OK && (pid_t) (uintptr_t) pid < 0) - { - if (-(pid_t) (uintptr_t) pid == match_pid) - /* It is about to do a fork, but is really still the parent PID. */ - pid = (psaddr_t) (uintptr_t) match_pid; - else - /* It must be a fork child, whose new PID is in the tid field. */ - err = DB_GET_FIELD_LOCAL (pid, ta, copy, pthread, tid, 0); - } + err = DB_GET_FIELD_LOCAL (schedpolicy, ta, copy, pthread, + schedpolicy, 0); if (err != TD_OK) break; + err = DB_GET_FIELD_LOCAL (schedprio, ta, copy, pthread, + schedparam_sched_priority, 0); + if (err != TD_OK) + break; + + /* Now test whether this thread matches the specified conditions. */ - if ((pid_t) (uintptr_t) pid == match_pid) + /* Only if the priority level is as high or higher. */ + int descr_pri = ((uintptr_t) schedpolicy == SCHED_OTHER + ? 0 : (uintptr_t) schedprio); + if (descr_pri >= ti_pri) { - err = DB_GET_FIELD_LOCAL (schedpolicy, ta, copy, pthread, - schedpolicy, 0); - if (err != TD_OK) - break; - err = DB_GET_FIELD_LOCAL (schedprio, ta, copy, pthread, - schedparam_sched_priority, 0); - if (err != TD_OK) - break; - - /* Now test whether this thread matches the specified conditions. */ - - /* Only if the priority level is as high or higher. */ - int descr_pri = ((uintptr_t) schedpolicy == SCHED_OTHER - ? 0 : (uintptr_t) schedprio); - if (descr_pri >= ti_pri) - { - /* Yep, it matches. Call the callback function. */ - td_thrhandle_t th; - th.th_ta_p = (td_thragent_t *) ta; - th.th_unique = addr; - if (callback (&th, cbdata_p) != 0) - return TD_DBERR; - } + /* Yep, it matches. Call the callback function. */ + td_thrhandle_t th; + th.th_ta_p = (td_thragent_t *) ta; + th.th_unique = addr; + if (callback (&th, cbdata_p) != 0) + return TD_DBERR; } /* Get the pointer to the next element. */ diff --git a/nptl_db/td_thr_validate.c b/nptl_db/td_thr_validate.c index f3c8a7b..9b89fec 100644 --- a/nptl_db/td_thr_validate.c +++ b/nptl_db/td_thr_validate.c @@ -80,28 +80,5 @@ td_thr_validate (const td_thrhandle_t *th) err = TD_OK; } - if (err == TD_OK) - { - /* Verify that this is not a stale element in a fork child. */ - pid_t match_pid = ps_getpid (th->th_ta_p->ph); - psaddr_t pid; - err = DB_GET_FIELD (pid, th->th_ta_p, th->th_unique, pthread, pid, 0); - if (err == TD_OK && (pid_t) (uintptr_t) pid < 0) - { - /* This was a thread that was about to fork, or it is the new sole - thread in a fork child. In the latter case, its tid was stored - via CLONE_CHILD_SETTID and so is already the proper child PID. */ - if (-(pid_t) (uintptr_t) pid == match_pid) - /* It is about to do a fork, but is really still the parent PID. */ - pid = (psaddr_t) (uintptr_t) match_pid; - else - /* It must be a fork child, whose new PID is in the tid field. */ - err = DB_GET_FIELD (pid, th->th_ta_p, th->th_unique, - pthread, tid, 0); - } - if (err == TD_OK && (pid_t) (uintptr_t) pid != match_pid) - err = TD_NOTHR; - } - return err; } diff --git a/sysdeps/nptl/fork.c b/sysdeps/nptl/fork.c index ea135f8..7294c08 100644 --- a/sysdeps/nptl/fork.c +++ b/sysdeps/nptl/fork.c @@ -132,15 +132,10 @@ __libc_fork (void) } #ifndef NDEBUG - pid_t ppid = THREAD_GETMEM (THREAD_SELF, tid); + INTERNAL_SYSCALL_DECL (err); + pid_t ppid = INTERNAL_SYSCALL_CALL (gettid, err); #endif - /* We need to prevent the getpid() code to update the PID field so - that, if a signal arrives in the child very early and the signal - handler uses getpid(), the value returned is correct. */ - pid_t parentpid = THREAD_GETMEM (THREAD_SELF, pid); - THREAD_SETMEM (THREAD_SELF, pid, -parentpid); - #ifdef ARCH_FORK pid = ARCH_FORK (); #else @@ -159,9 +154,6 @@ __libc_fork (void) if (__fork_generation_pointer != NULL) *__fork_generation_pointer += __PTHREAD_ONCE_FORK_GEN_INCR; - /* Adjust the PID field for the new process. */ - THREAD_SETMEM (self, pid, THREAD_GETMEM (self, tid)); - #if HP_TIMING_AVAIL /* The CPU clock of the thread and process have to be set to zero. */ hp_timing_t now; @@ -231,10 +223,7 @@ __libc_fork (void) } else { - assert (THREAD_GETMEM (THREAD_SELF, tid) == ppid); - - /* Restore the PID value. */ - THREAD_SETMEM (THREAD_SELF, pid, parentpid); + assert (THREAD_GETMEM (THREAD_SELF, tid) == 0); /* Release acquired locks in the multi-threaded case. */ if (multiple_threads) diff --git a/sysdeps/unix/sysv/linux/createthread.c b/sysdeps/unix/sysv/linux/createthread.c index 6d32cec..ec86f50 100644 --- a/sysdeps/unix/sysv/linux/createthread.c +++ b/sysdeps/unix/sysv/linux/createthread.c @@ -128,10 +128,10 @@ create_thread (struct pthread *pd, const struct pthread_attr *attr, /* The operation failed. We have to kill the thread. We let the normal cancellation mechanism do the work. */ + pid_t pid = __getpid (); INTERNAL_SYSCALL_DECL (err2); - (void) INTERNAL_SYSCALL (tgkill, err2, 3, - THREAD_GETMEM (THREAD_SELF, pid), - pd->tid, SIGCANCEL); + (void) INTERNAL_SYSCALL_CALL (tgkill, err2, pid, pd->tid, + SIGCANCEL); return INTERNAL_SYSCALL_ERRNO (res, err); } diff --git a/sysdeps/unix/sysv/linux/getpid.c b/sysdeps/unix/sysv/linux/getpid.c index 1124549..2bfafed 100644 --- a/sysdeps/unix/sysv/linux/getpid.c +++ b/sysdeps/unix/sysv/linux/getpid.c @@ -20,43 +20,11 @@ #include #include - -#if IS_IN (libc) -static inline __attribute__((always_inline)) pid_t really_getpid (pid_t oldval); - -static inline __attribute__((always_inline)) pid_t -really_getpid (pid_t oldval) -{ - if (__glibc_likely (oldval == 0)) - { - pid_t selftid = THREAD_GETMEM (THREAD_SELF, tid); - if (__glibc_likely (selftid != 0)) - 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 - pid_t __getpid (void) { -#if !IS_IN (libc) INTERNAL_SYSCALL_DECL (err); - pid_t result = INTERNAL_SYSCALL (getpid, err, 0); -#else - pid_t result = THREAD_GETMEM (THREAD_SELF, pid); - if (__glibc_unlikely (result <= 0)) - result = really_getpid (result); -#endif - return result; + return INTERNAL_SYSCALL_CALL (getpid, err); } libc_hidden_def (__getpid) diff --git a/sysdeps/unix/sysv/linux/pthread-pids.h b/sysdeps/unix/sysv/linux/pthread-pids.h index d42bba0..618a5b1 100644 --- a/sysdeps/unix/sysv/linux/pthread-pids.h +++ b/sysdeps/unix/sysv/linux/pthread-pids.h @@ -26,5 +26,5 @@ static inline void __pthread_initialize_pids (struct pthread *pd) { INTERNAL_SYSCALL_DECL (err); - pd->pid = pd->tid = INTERNAL_SYSCALL (set_tid_address, err, 1, &pd->tid); + pd->tid = INTERNAL_SYSCALL_CALL (set_tid_address, err, &pd->tid); } diff --git a/sysdeps/unix/sysv/linux/pthread_kill.c b/sysdeps/unix/sysv/linux/pthread_kill.c index bcb3009..15c9ba6 100644 --- a/sysdeps/unix/sysv/linux/pthread_kill.c +++ b/sysdeps/unix/sysv/linux/pthread_kill.c @@ -21,6 +21,7 @@ #include #include #include +#include int @@ -49,14 +50,15 @@ __pthread_kill (pthread_t threadid, int signo) /* We have a special syscall to do the work. */ INTERNAL_SYSCALL_DECL (err); + pid_t pid = getpid (); + /* One comment: The PID field in the TCB can temporarily be changed (in fork). But this must not affect this code here. Since this function would have to be called while the thread is executing fork, it would have to happen in a signal handler. But this is no allowed, pthread_kill is not guaranteed to be async-safe. */ int val; - val = INTERNAL_SYSCALL (tgkill, err, 3, THREAD_GETMEM (THREAD_SELF, pid), - tid, signo); + val = INTERNAL_SYSCALL_CALL (tgkill, err, pid, tid, signo); return (INTERNAL_SYSCALL_ERROR_P (val, err) ? INTERNAL_SYSCALL_ERRNO (val, err) : 0); diff --git a/sysdeps/unix/sysv/linux/pthread_sigqueue.c b/sysdeps/unix/sysv/linux/pthread_sigqueue.c index 7694d54..642366b 100644 --- a/sysdeps/unix/sysv/linux/pthread_sigqueue.c +++ b/sysdeps/unix/sysv/linux/pthread_sigqueue.c @@ -49,12 +49,14 @@ pthread_sigqueue (pthread_t threadid, int signo, const union sigval value) if (signo == SIGCANCEL || signo == SIGTIMER || signo == SIGSETXID) return EINVAL; + pid_t pid = getpid (); + /* Set up the siginfo_t structure. */ siginfo_t info; memset (&info, '\0', sizeof (siginfo_t)); info.si_signo = signo; info.si_code = SI_QUEUE; - info.si_pid = THREAD_GETMEM (THREAD_SELF, pid); + info.si_pid = pid; info.si_uid = getuid (); info.si_value = value; @@ -66,9 +68,8 @@ pthread_sigqueue (pthread_t threadid, int signo, const union sigval value) function would have to be called while the thread is executing fork, it would have to happen in a signal handler. But this is no allowed, pthread_sigqueue is not guaranteed to be async-safe. */ - int val = INTERNAL_SYSCALL (rt_tgsigqueueinfo, err, 4, - THREAD_GETMEM (THREAD_SELF, pid), - tid, signo, &info); + int val = INTERNAL_SYSCALL_CALL (rt_tgsigqueueinfo, err, pid, tid, signo, + &info); return (INTERNAL_SYSCALL_ERROR_P (val, err) ? INTERNAL_SYSCALL_ERRNO (val, err) : 0); diff --git a/sysdeps/unix/sysv/linux/tst-clone2.c b/sysdeps/unix/sysv/linux/tst-clone2.c index 68a7e6d..2d6369e 100644 --- a/sysdeps/unix/sysv/linux/tst-clone2.c +++ b/sysdeps/unix/sysv/linux/tst-clone2.c @@ -28,8 +28,14 @@ #include #include #include +#include -#include /* for THREAD_* macros. */ +#include /* For _STACK_GROWS_{UP,DOWN}. */ + +static int do_test (void); + +#define TEST_FUNCTION do_test () +#include static int sig; static int pipefd[2]; @@ -39,9 +45,12 @@ f (void *a) { close (pipefd[0]); - pid_t pid = THREAD_GETMEM (THREAD_SELF, pid); - pid_t tid = THREAD_GETMEM (THREAD_SELF, tid); + pid_t ppid = getppid (); + pid_t pid = getpid (); + pid_t tid = syscall (__NR_gettid); + while (write (pipefd[1], &ppid, sizeof ppid) < 0) + continue; while (write (pipefd[1], &pid, sizeof pid) < 0) continue; while (write (pipefd[1], &tid, sizeof tid) < 0) @@ -52,26 +61,19 @@ f (void *a) static int -clone_test (int clone_flags) +do_test (void) { sig = SIGRTMIN; sigset_t ss; sigemptyset (&ss); sigaddset (&ss, sig); if (sigprocmask (SIG_BLOCK, &ss, NULL) != 0) - { - printf ("sigprocmask failed: %m\n"); - return 1; - } + FAIL_EXIT1 ("sigprocmask failed: %m"); if (pipe2 (pipefd, O_CLOEXEC)) - { - printf ("sigprocmask failed: %m\n"); - return 1; - } - - pid_t ppid = getpid (); + FAIL_EXIT1 ("pipe failed: %m"); + int clone_flags = 0; #ifdef __ia64__ extern int __clone2 (int (*__fn) (void *__arg), void *__child_stack_base, size_t __child_stack_size, int __flags, @@ -88,61 +90,47 @@ clone_test (int clone_flags) #error "Define either _STACK_GROWS_DOWN or _STACK_GROWS_UP" #endif #endif + close (pipefd[1]); if (p == -1) + FAIL_EXIT1("clone failed: %m"); + + pid_t ppid, pid, tid; + if (read (pipefd[0], &ppid, sizeof pid) != sizeof pid) { - printf ("clone failed: %m\n"); - return 1; + kill (p, SIGKILL); + FAIL_EXIT1 ("read ppid failed: %m"); } - - pid_t pid, tid; if (read (pipefd[0], &pid, sizeof pid) != sizeof pid) { - printf ("read pid failed: %m\n"); kill (p, SIGKILL); - return 1; + FAIL_EXIT1 ("read pid failed: %m"); } if (read (pipefd[0], &tid, sizeof tid) != sizeof tid) { - printf ("read pid failed: %m\n"); kill (p, SIGKILL); - return 1; + FAIL_EXIT1 ("read tid failed: %m"); } close (pipefd[0]); int ret = 0; - /* For CLONE_VM glibc clone implementation does not change the pthread - pid/tid field. */ - if ((clone_flags & CLONE_VM) == CLONE_VM) - { - if ((ppid != pid) || (ppid != tid)) - { - printf ("parent pid (%i) != received pid/tid (%i/%i)\n", - (int)ppid, (int)pid, (int)tid); - ret = 1; - } - } - /* For any other flag clone updates the new pthread pid and tid with - the clone return value. */ - else - { - if ((p != pid) || (p != tid)) - { - printf ("child pid (%i) != received pid/tid (%i/%i)\n", - (int)p, (int)pid, (int)tid); - ret = 1; - } - } + pid_t own_pid = getpid (); + pid_t own_tid = syscall (__NR_gettid); + + /* Some sanity checks for clone syscall: returned ppid should be currernt + pid and both returned tid/pid should be different from current one. */ + if ((ppid != own_pid) || (pid == own_pid) || (tid == own_tid)) + FAIL_RET ("ppd=%i pid=%i tid=%i | own_pid=%i own_tid=%i", + (int)ppid, (int)pid, (int)tid, (int)own_pid, (int)own_tid); int e; if (waitpid (p, &e, __WCLONE) != p) { - puts ("waitpid failed"); kill (p, SIGKILL); - return 1; + FAIL_EXIT1 ("waitpid failed"); } if (!WIFEXITED (e)) { @@ -150,29 +138,10 @@ clone_test (int clone_flags) printf ("died from signal %s\n", strsignal (WTERMSIG (e))); else puts ("did not terminate correctly"); - return 1; + exit (EXIT_FAILURE); } if (WEXITSTATUS (e) != 0) - { - printf ("exit code %d\n", WEXITSTATUS (e)); - return 1; - } + FAIL_EXIT1 ("exit code %d", WEXITSTATUS (e)); return ret; } - -int -do_test (void) -{ - /* First, check that the clone implementation, without any flag, updates - the struct pthread to contain the new PID and TID. */ - int ret = clone_test (0); - /* Second, check that with CLONE_VM the struct pthread PID and TID fields - remain unmodified after the clone. Any modifications would cause problem - for the parent as described in bug 19957. */ - ret += clone_test (CLONE_VM); - return ret; -} - -#define TEST_FUNCTION do_test () -#include "../test-skeleton.c" diff --git a/sysdeps/unix/sysv/linux/x86_64/clone.S b/sysdeps/unix/sysv/linux/x86_64/clone.S index 66f4b11..5629aed 100644 --- a/sysdeps/unix/sysv/linux/x86_64/clone.S +++ b/sysdeps/unix/sysv/linux/x86_64/clone.S @@ -91,14 +91,6 @@ L(thread_start): the outermost frame obviously. */ xorl %ebp, %ebp - andq $CLONE_VM, %rdi - jne 1f - movl $SYS_ify(getpid), %eax - syscall - movl %eax, %fs:PID - movl %eax, %fs:TID -1: - /* Set up arguments for the function call. */ popq %rax /* Function to call. */ popq %rdi /* Argument. */ diff --git a/sysdeps/unix/sysv/linux/x86_64/vfork.S b/sysdeps/unix/sysv/linux/x86_64/vfork.S index 8332ade..cdd2dea 100644 --- a/sysdeps/unix/sysv/linux/x86_64/vfork.S +++ b/sysdeps/unix/sysv/linux/x86_64/vfork.S @@ -34,16 +34,6 @@ ENTRY (__vfork) cfi_adjust_cfa_offset(-8) cfi_register(%rip, %rdi) - /* Save the TCB-cached PID away in %esi, and then negate the TCB - field. But if it's zero, set it to 0x80000000 instead. See - raise.c for the logic that relies on this value. */ - movl %fs:PID, %esi - movl $0x80000000, %ecx - movl %esi, %edx - negl %edx - cmove %ecx, %edx - movl %edx, %fs:PID - /* Stuff the syscall number in RAX and enter into the kernel. */ movl $SYS_ify (vfork), %eax syscall @@ -52,14 +42,6 @@ ENTRY (__vfork) pushq %rdi cfi_adjust_cfa_offset(8) - /* Restore the original value of the TCB cache of the PID, if we're - the parent. But in the child (syscall return value equals zero), - leave things as they are. */ - testq %rax, %rax - je 1f - movl %esi, %fs:PID -1: - cmpl $-4095, %eax jae SYSCALL_ERROR_LABEL /* Branch forward if it failed. */ diff --git a/sysdeps/x86_64/nptl/tcb-offsets.sym b/sysdeps/x86_64/nptl/tcb-offsets.sym index aeb7526..8a25c48 100644 --- a/sysdeps/x86_64/nptl/tcb-offsets.sym +++ b/sysdeps/x86_64/nptl/tcb-offsets.sym @@ -4,7 +4,6 @@ RESULT offsetof (struct pthread, result) TID offsetof (struct pthread, tid) -PID offsetof (struct pthread, pid) CANCELHANDLING offsetof (struct pthread, cancelhandling) CLEANUP_JMP_BUF offsetof (struct pthread, cleanup_jmp_buf) CLEANUP offsetof (struct pthread, cleanup)