diff mbox series

[4/6] nptl: Initialize tid earlier

Message ID 20201204180944.3774769-4-adhemerval.zanella@linaro.org
State Superseded
Headers show
Series [1/6] nptl: Move Linux pthread_kill to nptl | expand

Commit Message

Adhemerval Zanella Dec. 4, 2020, 6:09 p.m. UTC
It sets the PD tid value as once pthread is initialized, the idea is
to allow raise being implemented by pthread_kill and avoid a gettid
call.

Checked on x86_64-linux-gnu.
---
 csu/libc-tls.c                                   |  3 +++
 elf/rtld.c                                       |  3 +++
 nptl/nptl-init.c                                 |  5 ++---
 {nptl => sysdeps/htl}/pthread-pids.h             | 13 +++----------
 sysdeps/{unix/sysv/linux => nptl}/pthread-pids.h | 11 +++++++----
 5 files changed, 18 insertions(+), 17 deletions(-)
 rename {nptl => sysdeps/htl}/pthread-pids.h (61%)
 rename sysdeps/{unix/sysv/linux => nptl}/pthread-pids.h (70%)

Comments

Florian Weimer Dec. 4, 2020, 6:22 p.m. UTC | #1
* Adhemerval Zanella via Libc-alpha:

> It sets the PD tid value as once pthread is initialized, the idea is
> to allow raise being implemented by pthread_kill and avoid a gettid
> call.

I don't think this is possible.  I expect that some applications
assume that raise works after vfork (certainly via abort), and the TID
cache is wrong at that point.  Sorry.
Adhemerval Zanella Dec. 4, 2020, 6:30 p.m. UTC | #2
On 04/12/2020 15:22, Florian Weimer wrote:
> * Adhemerval Zanella via Libc-alpha:
> 
>> It sets the PD tid value as once pthread is initialized, the idea is
>> to allow raise being implemented by pthread_kill and avoid a gettid
>> call.
> 
> I don't think this is possible.  I expect that some applications
> assume that raise works after vfork (certainly via abort), and the TID
> cache is wrong at that point.  Sorry.
> 

Sign, I would consider this undefined behavior but I don't see why break
it anyway.  I withdraw this patch.
diff mbox series

Patch

diff --git a/csu/libc-tls.c b/csu/libc-tls.c
index c3589f0a7d..54fd085420 100644
--- a/csu/libc-tls.c
+++ b/csu/libc-tls.c
@@ -25,6 +25,7 @@ 
 #include <sys/param.h>
 #include <array_length.h>
 #include <list.h>
+#include <pthread-pids.h>
 
 #ifdef SHARED
  #error makefile bug, this file is for static only
@@ -219,4 +220,6 @@  __libc_setup_tls (void)
 #endif
 
   init_static_tls (memsz, MAX (TLS_TCB_ALIGN, max_align));
+
+  pthread_initialize_pids (THREAD_SELF);
 }
diff --git a/elf/rtld.c b/elf/rtld.c
index c4ffc8d4b7..d01ba55a57 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -49,6 +49,7 @@ 
 #include <libc-early-init.h>
 #include <dl-main.h>
 #include <list.h>
+#include <pthread-pids.h>
 
 #include <assert.h>
 
@@ -805,6 +806,8 @@  cannot allocate TLS data structures for initial thread\n");
 #endif
   tls_init_tp_called = true;
 
+  pthread_initialize_pids (THREAD_SELF);
+
   return tcbp;
 }
 
diff --git a/nptl/nptl-init.c b/nptl/nptl-init.c
index 53b817715d..489023dee6 100644
--- a/nptl/nptl-init.c
+++ b/nptl/nptl-init.c
@@ -36,7 +36,6 @@ 
 #include <futex-internal.h>
 #include <kernel-features.h>
 #include <libc-pointer-arith.h>
-#include <pthread-pids.h>
 #include <pthread_mutex_conf.h>
 
 #ifndef TLS_MULTIPLE_THREADS_IN_TCB
@@ -225,9 +224,9 @@  static bool __nptl_initial_report_events __attribute_used__;
 void
 __pthread_initialize_minimal_internal (void)
 {
-  /* Minimal initialization of the thread descriptor.  */
+  /* Minimal initialization of the thread descriptor.
+     pd->tid was set during TLS initialization.  */
   struct pthread *pd = THREAD_SELF;
-  __pthread_initialize_pids (pd);
   THREAD_SETMEM (pd, specific[0], &pd->specific_1stblock[0]);
   THREAD_SETMEM (pd, user_stack, true);
 
diff --git a/nptl/pthread-pids.h b/sysdeps/htl/pthread-pids.h
similarity index 61%
rename from nptl/pthread-pids.h
rename to sysdeps/htl/pthread-pids.h
index b2a8349d0e..54c9cbca2e 100644
--- a/nptl/pthread-pids.h
+++ b/sysdeps/htl/pthread-pids.h
@@ -1,5 +1,5 @@ 
-/* Initialize pid and tid fields of struct pthread.  Stub version.
-   Copyright (C) 2015-2020 Free Software Foundation, Inc.
+/* Initialize pid and tid fields of struct pthread.  Hurd version.
+   Copyright (C) 2020 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
@@ -16,14 +16,7 @@ 
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
-#include <pthreadP.h>
-
-/* Initialize PD->pid and PD->tid for the initial thread.  If there is
-   setup required to arrange that __exit_thread causes PD->tid to be
-   cleared and futex-woken, then this function should do that as well.  */
 static inline void
-__pthread_initialize_pids (struct pthread *pd)
+pthread_initialize_pids (tcbhead_t *pd)
 {
-#error "sysdeps pthread-pids.h file required"
-  pd->pid = pd->tid = -1;
 }
diff --git a/sysdeps/unix/sysv/linux/pthread-pids.h b/sysdeps/nptl/pthread-pids.h
similarity index 70%
rename from sysdeps/unix/sysv/linux/pthread-pids.h
rename to sysdeps/nptl/pthread-pids.h
index 0b8ca4ec06..32fb2d937d 100644
--- a/sysdeps/unix/sysv/linux/pthread-pids.h
+++ b/sysdeps/nptl/pthread-pids.h
@@ -16,14 +16,17 @@ 
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
-#include <pthreadP.h>
 #include <sysdep.h>
 
-/* Initialize PD->pid and PD->tid for the initial thread.  If there is
-   setup required to arrange that __exit_thread causes PD->tid to be
+/* Initialize only as much of the initial thread's descriptor as is necessary
+   even when libpthread is not loaded.  More will be done by
+   __pthread_initialize_minimal if libpthread is loaded.  This needs to happen
+   before anything that could possibly call raise, but cannot happen before
+   TLS is initialized.
+   If there is setup required to that __exit_thread causes PD->tid to be
    cleared and futex-woken, then this function should do that as well.  */
 static inline void
-__pthread_initialize_pids (struct pthread *pd)
+pthread_initialize_pids (struct pthread *pd)
 {
   pd->tid = INTERNAL_SYSCALL_CALL (set_tid_address, &pd->tid);
 }