From d86a240aec6e0e26027c9f2b5444eb2e8a4b1301 Mon Sep 17 00:00:00 2001 From: Augustin Cavalier Date: Wed, 29 Mar 2023 11:24:17 -0400 Subject: [PATCH] kernel/thread: Refactor thread-local-storage initialization and updating. * TLS slots are not in any way architecture-specific; we do not even have a mechanism by which they could be made so at present. Thus, the initialization of them can be moved to common code, and out of the per-architecture implementations. * When dealing with a fork()ed thread, it will already have a TLS array with values set in it. Therefore, do not overwrite the whole array, but instead only update the specific values which have changed on fork. This fixes at least part of #17896. --- src/system/kernel/arch/arm/arch_thread.cpp | 17 +++---------- .../kernel/arch/riscv64/arch_thread.cpp | 16 +++--------- src/system/kernel/arch/x86/arch_thread.cpp | 23 +++-------------- src/system/kernel/thread.cpp | 25 ++++++++++++++++--- 4 files changed, 33 insertions(+), 48 deletions(-) diff --git a/src/system/kernel/arch/arm/arch_thread.cpp b/src/system/kernel/arch/arm/arch_thread.cpp index ff0ca0c36c..004ab26806 100644 --- a/src/system/kernel/arch/arm/arch_thread.cpp +++ b/src/system/kernel/arch/arm/arch_thread.cpp @@ -21,7 +21,6 @@ #include #include #include -#include #include #include #include @@ -117,20 +116,12 @@ arch_thread_init_kthread_stack(Thread* thread, void* _stack, void* _stackTop, status_t arch_thread_init_tls(Thread *thread) { - uint32 tls[TLS_FIRST_FREE_SLOT]; - - thread->user_local_storage = thread->user_stack_base - + thread->user_stack_size; - - // initialize default TLS fields - memset(tls, 0, sizeof(tls)); - tls[TLS_BASE_ADDRESS_SLOT] = thread->user_local_storage; - tls[TLS_THREAD_ID_SLOT] = thread->id; - tls[TLS_USER_THREAD_SLOT] = (addr_t)thread->user_thread; - - return user_memcpy((void *)thread->user_local_storage, tls, sizeof(tls)); + thread->user_local_storage = + thread->user_stack_base + thread->user_stack_size; + return B_OK; } + void arm_swap_pgdir(uint32_t pageDirectoryAddress) { diff --git a/src/system/kernel/arch/riscv64/arch_thread.cpp b/src/system/kernel/arch/riscv64/arch_thread.cpp index 21088c6eea..dd72296afa 100644 --- a/src/system/kernel/arch/riscv64/arch_thread.cpp +++ b/src/system/kernel/arch/riscv64/arch_thread.cpp @@ -13,7 +13,6 @@ #include #include #include -#include #include #include @@ -80,18 +79,9 @@ arch_thread_init_kthread_stack(Thread* thread, void* _stack, void* _stackTop, status_t arch_thread_init_tls(Thread *thread) { - addr_t tls[TLS_FIRST_FREE_SLOT]; - - thread->user_local_storage = thread->user_stack_base - + thread->user_stack_size; - - // initialize default TLS fields - memset(tls, 0, sizeof(tls)); - tls[TLS_BASE_ADDRESS_SLOT] = thread->user_local_storage; - tls[TLS_THREAD_ID_SLOT] = thread->id; - tls[TLS_USER_THREAD_SLOT] = (addr_t)thread->user_thread; - - return user_memcpy((void*)thread->user_local_storage, tls, sizeof(tls)); + thread->user_local_storage = + thread->user_stack_base + thread->user_stack_size; + return B_OK; } diff --git a/src/system/kernel/arch/x86/arch_thread.cpp b/src/system/kernel/arch/x86/arch_thread.cpp index f204693b3c..4af28e33a0 100644 --- a/src/system/kernel/arch/x86/arch_thread.cpp +++ b/src/system/kernel/arch/x86/arch_thread.cpp @@ -18,7 +18,6 @@ #include #include #include -#include #include #include @@ -184,26 +183,12 @@ arch_team_init_team_struct(Team* p, bool kernel) } -/*! Initializes the user-space TLS local storage pointer in - the thread structure, and the reserved TLS slots. - - Is called from _create_user_thread_kentry(). -*/ status_t -arch_thread_init_tls(Thread* thread) +arch_thread_init_tls(Thread *thread) { - addr_t tls[TLS_FIRST_FREE_SLOT]; - - thread->user_local_storage = thread->user_stack_base - + thread->user_stack_size; - - // initialize default TLS fields - memset(tls, 0, sizeof(tls)); - tls[TLS_BASE_ADDRESS_SLOT] = thread->user_local_storage; - tls[TLS_THREAD_ID_SLOT] = thread->id; - tls[TLS_USER_THREAD_SLOT] = (addr_t)thread->user_thread; - - return user_memcpy((void*)thread->user_local_storage, tls, sizeof(tls)); + thread->user_local_storage = + thread->user_stack_base + thread->user_stack_size; + return B_OK; } diff --git a/src/system/kernel/thread.cpp b/src/system/kernel/thread.cpp index c0851d3cb4..db0adcc4ee 100644 --- a/src/system/kernel/thread.cpp +++ b/src/system/kernel/thread.cpp @@ -659,9 +659,28 @@ enter_userspace(Thread* thread, UserThreadEntryArguments* args) userThread->pending_signals = 0; clear_ac(); - if (args->forkArgs != NULL) { - // This is a fork()ed thread. Copy the fork args onto the stack and - // free them. + // initialize default TLS fields + addr_t tls[TLS_FIRST_FREE_SLOT]; + memset(tls, 0, sizeof(tls)); + tls[TLS_BASE_ADDRESS_SLOT] = thread->user_local_storage; + tls[TLS_THREAD_ID_SLOT] = thread->id; + tls[TLS_USER_THREAD_SLOT] = (addr_t)thread->user_thread; + + if (args->forkArgs == NULL) { + if (user_memcpy((void*)thread->user_local_storage, tls, sizeof(tls)) != B_OK) + return B_BAD_ADDRESS; + } else { + // This is a fork()ed thread. + + // Update select TLS values, do not clear the whole array. + set_ac(); + addr_t* userTls = (addr_t*)thread->user_local_storage; + ASSERT(userTls[TLS_BASE_ADDRESS_SLOT] == thread->user_local_storage); + userTls[TLS_THREAD_ID_SLOT] = tls[TLS_THREAD_ID_SLOT]; + userTls[TLS_USER_THREAD_SLOT] = tls[TLS_USER_THREAD_SLOT]; + clear_ac(); + + // Copy the fork args onto the stack and free them. arch_fork_arg archArgs = *args->forkArgs; free(args->forkArgs);