FEXCore: Moves TLS initialization for Alloc::OSAllocator

Alloc::OSAllocator uses a TLS variable of the thread object so it can use a forkable mutex plus a deferring signal section. This was setup when the FEXCore "ExecutionThread" function is called, which is a bit awkward and is an artifact from when the thread creation was mixed between the frontend and the backend. Instead let the frontend inform the backend when to install the TLS variable. This is one step required to make GdbServer work correctly again since the thread initialization and pausing is awkward today.
FEX-Emu · Nov 28, 2024 · 3ecc66f · 3ecc66f
1 parent ee592ba
commit 3ecc66f
Show file tree

Hide file tree

Showing 8 changed files with 31 additions and 42 deletions.
diff --git a/FEXCore/Source/Interface/Context/Context.h b/FEXCore/Source/Interface/Context/Context.h
@@ -124,7 +124,6 @@ class ContextImpl final : public FEXCore::Context::Context {
    *    - ExecutionThread(Thread); // Starts executing without creating another host thread
    *  Thunk callback executing guest code from native host thread
    *    - Thread = CreateThread(0, 0, NewState, PPID);
-   *    - InitializeThreadTLSData(Thread);
    *    - HandleCallback(Thread, RIP);
    */
 
@@ -139,7 +138,7 @@ class ContextImpl final : public FEXCore::Context::Context {
    *
    * @param Thread The internal FEX thread state object
    */
-  void DestroyThread(FEXCore::Core::InternalThreadState* Thread, bool NeedsTLSUninstall) override;
+  void DestroyThread(FEXCore::Core::InternalThreadState* Thread) override;
 
 #ifndef _WIN32
   void LockBeforeFork(FEXCore::Core::InternalThreadState* Thread) override;
@@ -303,13 +302,6 @@ class ContextImpl final : public FEXCore::Context::Context {
   uintptr_t CompileBlock(FEXCore::Core::CpuStateFrame* Frame, uint64_t GuestRIP, uint64_t MaxInst = 0);
 
   // Used for thread creation from syscalls
-  /**
-   * @brief Initializes TID, PID and TLS data for a thread
-   *
-   * @param Thread The internal FEX thread state object
-   */
-  void InitializeThreadTLSData(FEXCore::Core::InternalThreadState* Thread);
-
   void CopyMemoryMapping(FEXCore::Core::InternalThreadState* ParentThread, FEXCore::Core::InternalThreadState* ChildThread);
 
   uint8_t GetGPRSize() const {

diff --git a/FEXCore/Source/Interface/Core/Core.cpp b/FEXCore/Source/Interface/Core/Core.cpp
@@ -382,14 +382,6 @@ void ContextImpl::ExecuteThread(FEXCore::Core::InternalThreadState* Thread) {
   Dispatcher->ExecuteDispatch(Thread->CurrentFrame);
 }
 
-
-void ContextImpl::InitializeThreadTLSData(FEXCore::Core::InternalThreadState* Thread) {
-  // Let's do some initial bookkeeping here
-#ifndef _WIN32
-  Alloc::OSAllocator::RegisterTLSData(Thread);
-#endif
-}
-
 void ContextImpl::InitializeCompiler(FEXCore::Core::InternalThreadState* Thread) {
   Thread->OpDispatcher = fextl::make_unique<FEXCore::IR::OpDispatchBuilder>(this);
   Thread->OpDispatcher->SetMultiblock(Config.Multiblock);
@@ -443,13 +435,7 @@ ContextImpl::CreateThread(uint64_t InitialRIP, uint64_t StackPointer, const FEXC
   return Thread;
 }
 
-void ContextImpl::DestroyThread(FEXCore::Core::InternalThreadState* Thread, bool NeedsTLSUninstall) {
-  if (NeedsTLSUninstall) {
-#ifndef _WIN32
-    Alloc::OSAllocator::UninstallTLSData(Thread);
-#endif
-  }
-
+void ContextImpl::DestroyThread(FEXCore::Core::InternalThreadState* Thread) {
   FEXCore::Allocator::VirtualProtect(&Thread->InterruptFaultPage, sizeof(Thread->InterruptFaultPage),
                                      Allocator::ProtectOptions::Read | Allocator::ProtectOptions::Write);
   delete Thread;
@@ -889,8 +875,6 @@ uintptr_t ContextImpl::CompileBlock(FEXCore::Core::CpuStateFrame* Frame, uint64_
 void ContextImpl::ExecutionThread(FEXCore::Core::InternalThreadState* Thread) {
   Thread->ExitReason = FEXCore::Context::ExitReason::EXIT_WAITING;
 
-  InitializeThreadTLSData(Thread);
-
   // Now notify the thread that we are initialized
   Thread->ThreadWaiting.NotifyAll();
 
@@ -919,10 +903,6 @@ void ContextImpl::ExecutionThread(FEXCore::Core::InternalThreadState* Thread) {
 
   // If it is the parent thread that died then just leave
   FEX_TODO("This doesn't make sense when the parent thread doesn't outlive its children");
-
-#ifndef _WIN32
-  Alloc::OSAllocator::UninstallTLSData(Thread);
-#endif
 }
 
 static void InvalidateGuestThreadCodeRange(FEXCore::Core::InternalThreadState* Thread, uint64_t Start, uint64_t Length) {

diff --git a/FEXCore/Source/Utils/Allocator/64BitAllocator.cpp b/FEXCore/Source/Utils/Allocator/64BitAllocator.cpp
@@ -32,14 +32,6 @@ namespace Alloc::OSAllocator {
 
 thread_local FEXCore::Core::InternalThreadState* TLSThread {};
 
-void RegisterTLSData(FEXCore::Core::InternalThreadState* Thread) {
-  TLSThread = Thread;
-}
-
-void UninstallTLSData(FEXCore::Core::InternalThreadState* Thread) {
-  TLSThread = nullptr;
-}
-
 class OSAllocator_64Bit final : public Alloc::HostAllocator {
 public:
   OSAllocator_64Bit();
@@ -585,3 +577,13 @@ fextl::unique_ptr<Alloc::HostAllocator> Create64BitAllocator() {
   return fextl::make_unique<OSAllocator_64Bit>();
 }
 } // namespace Alloc::OSAllocator
+
+namespace FEXCore::Allocator {
+void RegisterTLSData(FEXCore::Core::InternalThreadState* Thread) {
+  Alloc::OSAllocator::TLSThread = Thread;
+}
+
+void UninstallTLSData(FEXCore::Core::InternalThreadState* Thread) {
+  Alloc::OSAllocator::TLSThread = nullptr;
+}
+} // namespace FEXCore::Allocator
diff --git a/FEXCore/Source/Utils/Allocator/HostAllocator.h b/FEXCore/Source/Utils/Allocator/HostAllocator.h
@@ -48,7 +48,5 @@ class GlobalAllocator {
 } // namespace Alloc
 
 namespace Alloc::OSAllocator {
-void RegisterTLSData(FEXCore::Core::InternalThreadState* Thread);
-void UninstallTLSData(FEXCore::Core::InternalThreadState* Thread);
 fextl::unique_ptr<Alloc::HostAllocator> Create64BitAllocator();
 } // namespace Alloc::OSAllocator
diff --git a/FEXCore/include/FEXCore/Core/Context.h b/FEXCore/include/FEXCore/Core/Context.h
@@ -169,7 +169,7 @@ class Context {
     uint64_t InitialRIP, uint64_t StackPointer, const FEXCore::Core::CPUState* NewThreadState = nullptr, uint64_t ParentTID = 0) = 0;
 
   FEX_DEFAULT_VISIBILITY virtual void ExecutionThread(FEXCore::Core::InternalThreadState* Thread) = 0;
-  FEX_DEFAULT_VISIBILITY virtual void DestroyThread(FEXCore::Core::InternalThreadState* Thread, bool NeedsTLSUninstall = false) = 0;
+  FEX_DEFAULT_VISIBILITY virtual void DestroyThread(FEXCore::Core::InternalThreadState* Thread) = 0;
 #ifndef _WIN32
   FEX_DEFAULT_VISIBILITY virtual void LockBeforeFork(FEXCore::Core::InternalThreadState* Thread) {}
   FEX_DEFAULT_VISIBILITY virtual void UnlockAfterFork(FEXCore::Core::InternalThreadState* Thread, bool Child) {}

diff --git a/FEXCore/include/FEXCore/Utils/Allocator.h b/FEXCore/include/FEXCore/Utils/Allocator.h
@@ -9,6 +9,10 @@
 #include <optional>
 #include <sys/types.h>
 
+namespace FEXCore::Core {
+struct InternalThreadState;
+}
+
 namespace FEXCore::Allocator {
 FEX_DEFAULT_VISIBILITY void SetupHooks();
 FEX_DEFAULT_VISIBILITY void ClearHooks();
@@ -83,4 +87,9 @@ FEX_DEFAULT_VISIBILITY void ReclaimMemoryRegion(const fextl::vector<MemoryRegion
 // Use this to reserve the top 128TB of VA so the guest never see it
 // Returns nullptr on host VA < 48bits
 FEX_DEFAULT_VISIBILITY fextl::vector<MemoryRegion> Steal48BitVA();
+
+#ifndef _WIN32
+FEX_DEFAULT_VISIBILITY void RegisterTLSData(FEXCore::Core::InternalThreadState* Thread);
+FEX_DEFAULT_VISIBILITY void UninstallTLSData(FEXCore::Core::InternalThreadState* Thread);
+#endif
 } // namespace FEXCore::Allocator
diff --git a/Source/Tools/LinuxEmulation/LinuxSyscalls/SignalDelegator.cpp b/Source/Tools/LinuxEmulation/LinuxSyscalls/SignalDelegator.cpp
@@ -956,6 +956,8 @@ SignalDelegator::~SignalDelegator() {
 }
 
 void SignalDelegator::RegisterTLSState(FEX::HLE::ThreadStateObject* Thread) {
+  FEXCore::Allocator::RegisterTLSData(Thread->Thread);
+
   Thread->SignalInfo.Delegator = this;
 
   // Set up our signal alternative stack
@@ -999,6 +1001,8 @@ void SignalDelegator::UninstallTLSState(FEX::HLE::ThreadStateObject* Thread) {
   if (Result == -1) {
     LogMan::Msg::EFmt("Failed to uninstall alternative signal stack {}", strerror(errno));
   }
+
+  FEXCore::Allocator::UninstallTLSData(Thread->Thread);
 }
 
 void SignalDelegator::FrontendRegisterHostSignalHandler(int Signal, bool Required) {

diff --git a/Source/Tools/LinuxEmulation/LinuxSyscalls/ThreadManager.cpp b/Source/Tools/LinuxEmulation/LinuxSyscalls/ThreadManager.cpp
@@ -62,7 +62,11 @@ void ThreadManager::HandleThreadDeletion(FEX::HLE::ThreadStateObject* Thread, bo
     }
   }
 
-  CTX->DestroyThread(Thread->Thread, NeedsTLSUninstall);
+  if (NeedsTLSUninstall) {
+    FEXCore::Allocator::UninstallTLSData(Thread->Thread);
+  }
+
+  CTX->DestroyThread(Thread->Thread);
   FEX::HLE::_SyscallHandler->SeccompEmulator.FreeSeccompFilters(Thread);
 
   delete Thread;