From aa8430db9cd3e793086dbdb1c262c3301e5f0d6b Mon Sep 17 00:00:00 2001 From: Ryan Houdek Date: Tue, 10 Dec 2024 13:51:56 -0800 Subject: [PATCH 1/6] LinuxSyscalls: Give it a pointer to the gdbserver --- Source/Tools/FEXLoader/FEXLoader.cpp | 1 + Source/Tools/LinuxEmulation/LinuxSyscalls/Syscalls.h | 10 ++++++++++ 2 files changed, 11 insertions(+) diff --git a/Source/Tools/FEXLoader/FEXLoader.cpp b/Source/Tools/FEXLoader/FEXLoader.cpp index 60a8c49936..11a5324a74 100644 --- a/Source/Tools/FEXLoader/FEXLoader.cpp +++ b/Source/Tools/FEXLoader/FEXLoader.cpp @@ -587,6 +587,7 @@ int main(int argc, char** argv, char** const envp) { fextl::unique_ptr DebugServer; if (GdbServer) { DebugServer = fextl::make_unique(CTX.get(), SignalDelegation.get(), SyscallHandler.get()); + SyscallHandler->SetGdbServer(DebugServer.get()); } if (!CTX->InitCore()) { diff --git a/Source/Tools/LinuxEmulation/LinuxSyscalls/Syscalls.h b/Source/Tools/LinuxEmulation/LinuxSyscalls/Syscalls.h index db8d5363c0..eaa08c60ac 100644 --- a/Source/Tools/LinuxEmulation/LinuxSyscalls/Syscalls.h +++ b/Source/Tools/LinuxEmulation/LinuxSyscalls/Syscalls.h @@ -53,6 +53,7 @@ desc: Glue logic, STRACE magic namespace FEX { class CodeLoader; +class GdbServer; } namespace FEXCore { @@ -282,6 +283,14 @@ class SyscallHandler : public FEXCore::HLE::SyscallHandler, FEXCore::HLE::Source constexpr static uint64_t TASK_MAX_64BIT = (1ULL << 48); + void SetGdbServer(FEX::GdbServer* Server) { + GdbServer = Server; + } + + FEX::GdbServer* GetGdbServer() { + return GdbServer; + } + protected: SyscallHandler(FEXCore::Context::Context* _CTX, FEX::HLE::SignalDelegator* _SignalDelegation, FEX::HLE::ThunkHandler* ThunkHandler); @@ -308,6 +317,7 @@ class SyscallHandler : public FEXCore::HLE::SyscallHandler, FEXCore::HLE::Source FEX::HLE::SignalDelegator* SignalDelegation; FEX::HLE::ThunkHandler* ThunkHandler; + FEX::GdbServer* GdbServer {}; std::mutex FutexMutex; std::mutex SyscallMutex; From a3aaaa79a61cb6a4b812d4aa8f17891b84b5d947 Mon Sep 17 00:00:00 2001 From: Ryan Houdek Date: Tue, 10 Dec 2024 13:52:44 -0800 Subject: [PATCH 2/6] FEXLoader: Ensure thread TLS state is registered before tracking the thread --- Source/Tools/FEXLoader/FEXLoader.cpp | 2 +- Source/Tools/LinuxEmulation/LinuxSyscalls/Syscalls/Thread.cpp | 4 ++-- Source/Tools/TestHarnessRunner/TestHarnessRunner.cpp | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Source/Tools/FEXLoader/FEXLoader.cpp b/Source/Tools/FEXLoader/FEXLoader.cpp index 11a5324a74..bf621bf393 100644 --- a/Source/Tools/FEXLoader/FEXLoader.cpp +++ b/Source/Tools/FEXLoader/FEXLoader.cpp @@ -595,9 +595,9 @@ int main(int argc, char** argv, char** const envp) { } auto ParentThread = SyscallHandler->TM.CreateThread(Loader.DefaultRIP(), Loader.GetStackPointer()); - SyscallHandler->TM.TrackThread(ParentThread); SignalDelegation->RegisterTLSState(ParentThread); ThunkHandler->RegisterTLSState(ParentThread); + SyscallHandler->TM.TrackThread(ParentThread); // Pass in our VDSO thunks ThunkHandler->AppendThunkDefinitions(FEX::VDSO::GetVDSOThunkDefinitions(Loader.Is64BitMode())); diff --git a/Source/Tools/LinuxEmulation/LinuxSyscalls/Syscalls/Thread.cpp b/Source/Tools/LinuxEmulation/LinuxSyscalls/Syscalls/Thread.cpp index 3c93f76935..7eed5b5ec3 100644 --- a/Source/Tools/LinuxEmulation/LinuxSyscalls/Syscalls/Thread.cpp +++ b/Source/Tools/LinuxEmulation/LinuxSyscalls/Syscalls/Thread.cpp @@ -230,12 +230,12 @@ uint64_t HandleNewClone(FEX::HLE::ThreadStateObject* Thread, FEXCore::Context::C Thread->ThreadInfo.PID = ::getpid(); FEX::HLE::_SyscallHandler->FM.UpdatePID(Thread->ThreadInfo.PID); + FEX::HLE::_SyscallHandler->RegisterTLSState(Thread); + if (CreatedNewThreadObject) { FEX::HLE::_SyscallHandler->TM.TrackThread(Thread); } - FEX::HLE::_SyscallHandler->RegisterTLSState(Thread); - // Start exuting the thread directly // Our host clone starts in a new stack space, so it can't return back to the JIT space CTX->ExecuteThread(Thread->Thread); diff --git a/Source/Tools/TestHarnessRunner/TestHarnessRunner.cpp b/Source/Tools/TestHarnessRunner/TestHarnessRunner.cpp index 11edc6453f..7e6467e13e 100644 --- a/Source/Tools/TestHarnessRunner/TestHarnessRunner.cpp +++ b/Source/Tools/TestHarnessRunner/TestHarnessRunner.cpp @@ -323,8 +323,8 @@ int main(int argc, char** argv, char** const envp) { return 1; } auto ParentThread = SyscallHandler->TM.CreateThread(Loader.DefaultRIP(), Loader.GetStackPointer()); - SyscallHandler->TM.TrackThread(ParentThread); SignalDelegation->RegisterTLSState(ParentThread); + SyscallHandler->TM.TrackThread(ParentThread); if (!ParentThread) { return 1; From f47dfeb5c69d64d284909613784545564f4d9151 Mon Sep 17 00:00:00 2001 From: Ryan Houdek Date: Tue, 10 Dec 2024 13:54:27 -0800 Subject: [PATCH 3/6] ThreadManager: Store off the syscallHandler --- Source/Tools/LinuxEmulation/LinuxSyscalls/Syscalls.cpp | 2 +- Source/Tools/LinuxEmulation/LinuxSyscalls/ThreadManager.h | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/Source/Tools/LinuxEmulation/LinuxSyscalls/Syscalls.cpp b/Source/Tools/LinuxEmulation/LinuxSyscalls/Syscalls.cpp index ac3c8fe781..d1161802a0 100644 --- a/Source/Tools/LinuxEmulation/LinuxSyscalls/Syscalls.cpp +++ b/Source/Tools/LinuxEmulation/LinuxSyscalls/Syscalls.cpp @@ -744,7 +744,7 @@ void SyscallHandler::DefaultProgramBreak(uint64_t Base, uint64_t Size) { } SyscallHandler::SyscallHandler(FEXCore::Context::Context* _CTX, FEX::HLE::SignalDelegator* _SignalDelegation, FEX::HLE::ThunkHandler* ThunkHandler) - : TM {_CTX, _SignalDelegation} + : TM {_CTX, this, _SignalDelegation} , SeccompEmulator {this, _SignalDelegation} , FM {_CTX} , CTX {_CTX} diff --git a/Source/Tools/LinuxEmulation/LinuxSyscalls/ThreadManager.h b/Source/Tools/LinuxEmulation/LinuxSyscalls/ThreadManager.h index 2401a88357..516b1d065a 100644 --- a/Source/Tools/LinuxEmulation/LinuxSyscalls/ThreadManager.h +++ b/Source/Tools/LinuxEmulation/LinuxSyscalls/ThreadManager.h @@ -99,8 +99,9 @@ struct ThreadStateObject : public FEXCore::Allocator::FEXAllocOperators { class ThreadManager final { public: - ThreadManager(FEXCore::Context::Context* CTX, FEX::HLE::SignalDelegator* SignalDelegation) + ThreadManager(FEXCore::Context::Context* CTX, FEX::HLE::SyscallHandler* SyscallHandler, FEX::HLE::SignalDelegator* SignalDelegation) : CTX {CTX} + , SyscallHandler {SyscallHandler} , SignalDelegation {SignalDelegation} {} ~ThreadManager(); @@ -175,6 +176,7 @@ class ThreadManager final { private: FEXCore::Context::Context* CTX; + FEX::HLE::SyscallHandler* SyscallHandler; FEX::HLE::SignalDelegator* SignalDelegation; FEXCore::ForkableUniqueMutex ThreadCreationMutex; From 0a1c476a06d602eb28838390a7c49f2d1587bf39 Mon Sep 17 00:00:00 2001 From: Ryan Houdek Date: Tue, 10 Dec 2024 13:57:00 -0800 Subject: [PATCH 4/6] GdbServer: Have Break consume the ThreadObject directly --- Source/Tools/LinuxEmulation/LinuxSyscalls/GdbServer.cpp | 5 ++--- Source/Tools/LinuxEmulation/LinuxSyscalls/GdbServer.h | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/Source/Tools/LinuxEmulation/LinuxSyscalls/GdbServer.cpp b/Source/Tools/LinuxEmulation/LinuxSyscalls/GdbServer.cpp index 4265a562b2..03039ae5ef 100644 --- a/Source/Tools/LinuxEmulation/LinuxSyscalls/GdbServer.cpp +++ b/Source/Tools/LinuxEmulation/LinuxSyscalls/GdbServer.cpp @@ -62,13 +62,12 @@ desc: Provides a gdb interface to the guest state namespace FEX { #ifndef _WIN32 -void GdbServer::Break(FEXCore::Core::InternalThreadState* Thread, int signal) { +void GdbServer::Break(FEX::HLE::ThreadStateObject* ThreadObject, int signal) { std::lock_guard lk(sendMutex); if (!CommsStream) { return; } - auto ThreadObject = FEX::HLE::ThreadManager::GetStateObjectFromFEXCoreThread(Thread); // Current debugging thread switches to the thread that is breaking. CurrentDebuggingThread = ThreadObject->ThreadInfo.TID.load(); @@ -119,7 +118,7 @@ GdbServer::GdbServer(FEXCore::Context::Context* ctx, FEX::HLE::SignalDelegator* ThreadObject->GdbInfo->PState = ArchHelpers::Context::GetArmPState(ucontext); // Let GDB know that we have a signal - this->Break(Thread, Signal); + this->Break(ThreadObject, Signal); WaitForThreadWakeup(); ThreadObject->GdbInfo.reset(); diff --git a/Source/Tools/LinuxEmulation/LinuxSyscalls/GdbServer.h b/Source/Tools/LinuxEmulation/LinuxSyscalls/GdbServer.h index af749d8f89..0b4cc726c3 100644 --- a/Source/Tools/LinuxEmulation/LinuxSyscalls/GdbServer.h +++ b/Source/Tools/LinuxEmulation/LinuxSyscalls/GdbServer.h @@ -36,7 +36,7 @@ class GdbServer { } private: - void Break(FEXCore::Core::InternalThreadState* Thread, int signal); + void Break(FEX::HLE::ThreadStateObject* ThreadObject, int signal); void OpenListenSocket(); void CloseListenSocket(); From 2b2acf82f99a960da614416d2ab27e7ae7828804 Mon Sep 17 00:00:00 2001 From: Ryan Houdek Date: Tue, 10 Dec 2024 14:05:20 -0800 Subject: [PATCH 5/6] GdbServer: Changes how thread addition and pausing works This is enough to get GdbServer working for my simple test case again. Some things of note: - There is now a callback which signals to GdbServer when a thread is created. - Thread sleeping is now a single uint32_t futex word for knowing when a thread is sleeping and if we need to signal it to wake up. - GdbServer thread sleeping versus ThreadManager thread sleeping distinction has been removed. GdbServer now uses ThreadManager to put a thread to sleep. This still only gets the attach at startup scheme of gdbserver working. I haven't yet dived back in to getting arbitrary attach working. Primarily the work necessary here is actually getting thread creation at the start to stop and wait for gdbserver to attach. Then actually sleeping correctly while gdbserver is communicating with gdb. --- .../LinuxSyscalls/GdbServer.cpp | 23 +++-- .../LinuxEmulation/LinuxSyscalls/GdbServer.h | 5 +- .../LinuxSyscalls/SignalDelegator.cpp | 2 +- .../LinuxEmulation/LinuxSyscalls/Syscalls.h | 2 +- .../LinuxSyscalls/ThreadManager.cpp | 30 ++++-- .../LinuxSyscalls/ThreadManager.h | 96 +++++++++++++++++-- 6 files changed, 130 insertions(+), 28 deletions(-) diff --git a/Source/Tools/LinuxEmulation/LinuxSyscalls/GdbServer.cpp b/Source/Tools/LinuxEmulation/LinuxSyscalls/GdbServer.cpp index 03039ae5ef..0c83cda56e 100644 --- a/Source/Tools/LinuxEmulation/LinuxSyscalls/GdbServer.cpp +++ b/Source/Tools/LinuxEmulation/LinuxSyscalls/GdbServer.cpp @@ -10,6 +10,7 @@ desc: Provides a gdb interface to the guest state #include "GdbServer/Info.h" #include "LinuxSyscalls/NetStream.h" +#include "LinuxSyscalls/SignalDelegator.h" #include #include @@ -75,11 +76,6 @@ void GdbServer::Break(FEX::HLE::ThreadStateObject* ThreadObject, int signal) { SendPacket(*CommsStream, str); } -void GdbServer::WaitForThreadWakeup() { - // Wait for gdbserver to tell us to wake up - ThreadBreakEvent.Wait(); -} - GdbServer::~GdbServer() { CloseListenSocket(); CoreShuttingDown = true; @@ -120,7 +116,7 @@ GdbServer::GdbServer(FEXCore::Context::Context* ctx, FEX::HLE::SignalDelegator* // Let GDB know that we have a signal this->Break(ThreadObject, Signal); - WaitForThreadWakeup(); + this->SyscallHandler->TM.SleepThread(this->CTX, ThreadObject); ThreadObject->GdbInfo.reset(); return true; @@ -604,8 +600,14 @@ GdbServer::HandledPacketType GdbServer::handleProgramOffsets() { GdbServer::HandledPacketType GdbServer::ThreadAction(char action, uint32_t tid) { switch (action) { case 'c': { + { + std::lock_guard lk(*SyscallHandler->TM.GetThreadsCreationMutex()); + auto Threads = SyscallHandler->TM.GetThreads(); + for (auto& Thread : *Threads) { + Thread->ThreadSleeping.NotifyOne(); + } + } SyscallHandler->TM.Run(); - ThreadBreakEvent.NotifyAll(); SyscallHandler->TM.WaitForThreadsToRun(); return {"", HandledPacketType::TYPE_ONLYACK}; } @@ -1450,6 +1452,13 @@ void GdbServer::StartThread() { FEXCore::Threads::SetSignalMask(OldMask); } +void GdbServer::OnThreadCreated(FEX::HLE::ThreadStateObject* ThreadObject) { + if (SyscallHandler->TM.GetThreadCount() == 1) { + // Sleep the first thread created. This is because FEX only supports attaching at startup currently. + SyscallHandler->TM.SleepThread(CTX, ThreadObject); + } +} + void GdbServer::OpenListenSocket() { const auto GdbUnixPath = fextl::fmt::format("{}/FEX_gdbserver/", FEXServerClient::GetTempFolder()); if (FHU::Filesystem::CreateDirectory(GdbUnixPath) == FHU::Filesystem::CreateDirectoryResult::ERROR) { diff --git a/Source/Tools/LinuxEmulation/LinuxSyscalls/GdbServer.h b/Source/Tools/LinuxEmulation/LinuxSyscalls/GdbServer.h index 0b4cc726c3..425eabb7ee 100644 --- a/Source/Tools/LinuxEmulation/LinuxSyscalls/GdbServer.h +++ b/Source/Tools/LinuxEmulation/LinuxSyscalls/GdbServer.h @@ -35,6 +35,8 @@ class GdbServer { LibraryMapChanged = true; } + void OnThreadCreated(FEX::HLE::ThreadStateObject* ThreadObject); + private: void Break(FEX::HLE::ThreadStateObject* ThreadObject, int signal); @@ -52,9 +54,6 @@ class GdbServer { void SendACK(std::ostream& stream, bool NACK); - Event ThreadBreakEvent {}; - void WaitForThreadWakeup(); - struct HandledPacketType { fextl::string Response {}; enum ResponseType { diff --git a/Source/Tools/LinuxEmulation/LinuxSyscalls/SignalDelegator.cpp b/Source/Tools/LinuxEmulation/LinuxSyscalls/SignalDelegator.cpp index 51abf11619..94920b4d45 100644 --- a/Source/Tools/LinuxEmulation/LinuxSyscalls/SignalDelegator.cpp +++ b/Source/Tools/LinuxEmulation/LinuxSyscalls/SignalDelegator.cpp @@ -476,7 +476,7 @@ bool SignalDelegator::HandleSignalPause(FEXCore::Core::InternalThreadState* Thre // We need to be a little bit careful here // If we were already paused (due to GDB) and we are immediately stopping (due to gdb kill) // Then we need to ensure we don't double decrement our idle thread counter - if (ThreadObject->ThreadSleeping) { + if (ThreadObject->ThreadSleeping.HasWaiter()) { // If the thread was sleeping then its idle counter was decremented // Reincrement it here to not break logic FEX::HLE::_SyscallHandler->TM.IncrementIdleRefCount(); diff --git a/Source/Tools/LinuxEmulation/LinuxSyscalls/Syscalls.h b/Source/Tools/LinuxEmulation/LinuxSyscalls/Syscalls.h index eaa08c60ac..20ec01f75a 100644 --- a/Source/Tools/LinuxEmulation/LinuxSyscalls/Syscalls.h +++ b/Source/Tools/LinuxEmulation/LinuxSyscalls/Syscalls.h @@ -54,7 +54,7 @@ desc: Glue logic, STRACE magic namespace FEX { class CodeLoader; class GdbServer; -} +} // namespace FEX namespace FEXCore { namespace Context { diff --git a/Source/Tools/LinuxEmulation/LinuxSyscalls/ThreadManager.cpp b/Source/Tools/LinuxEmulation/LinuxSyscalls/ThreadManager.cpp index 106e5a4cb1..1532f5d501 100644 --- a/Source/Tools/LinuxEmulation/LinuxSyscalls/ThreadManager.cpp +++ b/Source/Tools/LinuxEmulation/LinuxSyscalls/ThreadManager.cpp @@ -1,5 +1,6 @@ // SPDX-License-Identifier: MIT +#include "LinuxSyscalls/GdbServer.h" #include "LinuxSyscalls/Syscalls.h" #include "LinuxSyscalls/SignalDelegator.h" @@ -29,6 +30,18 @@ FEX::HLE::ThreadStateObject* ThreadManager::CreateThread(uint64_t InitialRIP, ui return ThreadStateObject; } +void ThreadManager::TrackThread(FEX::HLE::ThreadStateObject* Thread) { + { + std::lock_guard lk(ThreadCreationMutex); + Threads.emplace_back(Thread); + } + + auto GdbServer = SyscallHandler->GetGdbServer(); + if (GdbServer) { + GdbServer->OnThreadCreated(Thread); + } +} + void ThreadManager::DestroyThread(FEX::HLE::ThreadStateObject* Thread, bool NeedsTLSUninstall) { { std::lock_guard lk(ThreadCreationMutex); @@ -79,7 +92,10 @@ void ThreadManager::NotifyPause() { // Tell all the threads that they should pause std::lock_guard lk(ThreadCreationMutex); for (auto& Thread : Threads) { - SignalDelegation->SignalThread(Thread->Thread, SignalEvent::Pause); + if (!Thread->ThreadSleeping.HasWaiter()) { + // Only signal if it isn't already sleeping. + SignalDelegation->SignalThread(Thread->Thread, SignalEvent::Pause); + } } } @@ -182,8 +198,7 @@ void ThreadManager::Stop(bool IgnoreCurrentThread) { } } -void ThreadManager::SleepThread(FEXCore::Context::Context* CTX, FEXCore::Core::CpuStateFrame* Frame) { - auto ThreadObject = FEX::HLE::ThreadManager::GetStateObjectFromCPUState(Frame); +void ThreadManager::SleepThread(FEXCore::Context::Context* CTX, FEX::HLE::ThreadStateObject* ThreadObject) { #if defined(ASSERTIONS_ENABLED) && ASSERTIONS_ENABLED // Sanity check. This can only be called from the owning thread. { @@ -197,19 +212,16 @@ void ThreadManager::SleepThread(FEXCore::Context::Context* CTX, FEXCore::Core::C --IdleWaitRefCount; IdleWaitCV.notify_all(); - ThreadObject->ThreadSleeping = true; - - // Go to sleep - ThreadObject->ThreadPaused.Wait(); + // Go to sleep. + ThreadObject->ThreadSleeping.Wait(); ++IdleWaitRefCount; - ThreadObject->ThreadSleeping = false; IdleWaitCV.notify_all(); } void ThreadManager::UnpauseThread(FEX::HLE::ThreadStateObject* Thread) { - Thread->ThreadPaused.NotifyOne(); + Thread->ThreadSleeping.NotifyOne(); } void ThreadManager::UnlockAfterFork(FEXCore::Core::InternalThreadState* LiveThread, bool Child) { diff --git a/Source/Tools/LinuxEmulation/LinuxSyscalls/ThreadManager.h b/Source/Tools/LinuxEmulation/LinuxSyscalls/ThreadManager.h index 516b1d065a..d18d26118e 100644 --- a/Source/Tools/LinuxEmulation/LinuxSyscalls/ThreadManager.h +++ b/Source/Tools/LinuxEmulation/LinuxSyscalls/ThreadManager.h @@ -22,6 +22,79 @@ namespace FEX::HLE { class SyscallHandler; class SignalDelegator; +// A latch that can be inspected to see if there is a waiter currently active. +// This allows us to remove the race condition between a thread trying to go asleep and something else telling it to go to sleep or wake up. +// +// Only one thread can ever wait on a latch, while another thread signals it. +class InspectableLatch final { +public: + bool Wait(struct timespec* Timeout = nullptr) { + while (true) { + uint32_t Expected = HAS_NO_WAITER; + const uint32_t Desired = HAS_WAITER; + + if (Mutex.compare_exchange_strong(Expected, Desired)) { + // We have latched, now futex. + constexpr int Op = FUTEX_WAIT | FUTEX_PRIVATE_FLAG; + // WAIT will keep sleeping on the futex word while it is `val` + int Result = ::syscall(SYS_futex, &Mutex, Op, + Desired, // val + Timeout, // Timeout/val2 + nullptr, // Addr2 + 0); // val3 + + if (Timeout && Result == -1 && errno == ETIMEDOUT) { + return false; + } + } else if (Expected == HAS_SIGNALED) { + // Reset the latch once signaled + Mutex.store(HAS_NO_WAITER); + return true; + } + } + } + + template + bool WaitFor(const std::chrono::duration& time) { + struct timespec Timeout {}; + auto SecondsDuration = std::chrono::duration_cast(time); + Timeout.tv_sec = SecondsDuration.count(); + Timeout.tv_nsec = std::chrono::duration_cast(time - SecondsDuration).count(); + return Wait(&Timeout); + } + + void NotifyOne() { + DoNotify(1); + } + + bool HasWaiter() const { + return Mutex.load() == HAS_WAITER; + } + +private: + std::atomic Mutex {}; + constexpr static uint32_t HAS_NO_WAITER = 0; + constexpr static uint32_t HAS_WAITER = 1; + constexpr static uint32_t HAS_SIGNALED = 2; + + void DoNotify(int Waiters) { + uint32_t Expected = HAS_WAITER; + const uint32_t Desired = HAS_SIGNALED; + + // If the mutex is in a waiting state and we have CAS exchanged it to HAS_SIGNALED, then execute a futex syscall. + // otherwise just leave since nothing was waiting. + if (Mutex.compare_exchange_strong(Expected, Desired)) { + constexpr int Op = FUTEX_WAKE | FUTEX_PRIVATE_FLAG; + + ::syscall(SYS_futex, &Mutex, Op, + Waiters, // val - Number of waiters to wake + 0, // val2 + &Mutex, // Addr2 - Mutex to do the operation on + 0); // val3 + } + } +}; + enum class SignalEvent : uint32_t { Nothing, // If the guest uses our signal we need to know it was errant on our end Pause, @@ -81,8 +154,7 @@ struct ThreadStateObject : public FEXCore::Allocator::FEXAllocOperators { std::atomic SignalReason {SignalEvent::Nothing}; // Thread pause handling - std::atomic_bool ThreadSleeping {false}; - FEXCore::InterruptableConditionVariable ThreadPaused; + InspectableLatch ThreadSleeping; // GDB signal information struct GdbInfoStruct { @@ -117,10 +189,7 @@ class ThreadManager final { FEX::HLE::ThreadStateObject* CreateThread(uint64_t InitialRIP, uint64_t StackPointer, const FEXCore::Core::CPUState* NewThreadState = nullptr, uint64_t ParentTID = 0, FEX::HLE::ThreadStateObject* InheritThread = nullptr); - void TrackThread(FEX::HLE::ThreadStateObject* Thread) { - std::lock_guard lk(ThreadCreationMutex); - Threads.emplace_back(Thread); - } + void TrackThread(FEX::HLE::ThreadStateObject* Thread); void DestroyThread(FEX::HLE::ThreadStateObject* Thread, bool NeedsTLSUninstall = false); void StopThread(FEX::HLE::ThreadStateObject* Thread); @@ -135,7 +204,11 @@ class ThreadManager final { void WaitForIdleWithTimeout(); void WaitForThreadsToRun(); - void SleepThread(FEXCore::Context::Context* CTX, FEXCore::Core::CpuStateFrame* Frame); + void SleepThread(FEXCore::Context::Context* CTX, FEX::HLE::ThreadStateObject* ThreadObject); + void SleepThread(FEXCore::Context::Context* CTX, FEXCore::Core::CpuStateFrame* Frame) { + auto ThreadObject = FEX::HLE::ThreadManager::GetStateObjectFromCPUState(Frame); + SleepThread(CTX, ThreadObject); + } void UnlockAfterFork(FEXCore::Core::InternalThreadState* Thread, bool Child); @@ -170,10 +243,19 @@ class ThreadManager final { } } + FEXCore::ForkableUniqueMutex* GetThreadsCreationMutex() { + return &ThreadCreationMutex; + } + const fextl::vector* GetThreads() const { return &Threads; } + size_t GetThreadCount() { + std::lock_guard lk(ThreadCreationMutex); + return Threads.size(); + } + private: FEXCore::Context::Context* CTX; FEX::HLE::SyscallHandler* SyscallHandler; From a0159e14f565eb21049f4e55e19bc648dd83016f Mon Sep 17 00:00:00 2001 From: Ryan Houdek Date: Wed, 18 Dec 2024 11:27:31 -0800 Subject: [PATCH 6/6] Review comments --- .../LinuxSyscalls/GdbServer.cpp | 2 +- .../LinuxSyscalls/ThreadManager.h | 19 +++++++++++-------- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/Source/Tools/LinuxEmulation/LinuxSyscalls/GdbServer.cpp b/Source/Tools/LinuxEmulation/LinuxSyscalls/GdbServer.cpp index 0c83cda56e..32d1cddcbf 100644 --- a/Source/Tools/LinuxEmulation/LinuxSyscalls/GdbServer.cpp +++ b/Source/Tools/LinuxEmulation/LinuxSyscalls/GdbServer.cpp @@ -601,7 +601,7 @@ GdbServer::HandledPacketType GdbServer::ThreadAction(char action, uint32_t tid) switch (action) { case 'c': { { - std::lock_guard lk(*SyscallHandler->TM.GetThreadsCreationMutex()); + std::lock_guard lk(SyscallHandler->TM.GetThreadsCreationMutex()); auto Threads = SyscallHandler->TM.GetThreads(); for (auto& Thread : *Threads) { Thread->ThreadSleeping.NotifyOne(); diff --git a/Source/Tools/LinuxEmulation/LinuxSyscalls/ThreadManager.h b/Source/Tools/LinuxEmulation/LinuxSyscalls/ThreadManager.h index d18d26118e..466706d040 100644 --- a/Source/Tools/LinuxEmulation/LinuxSyscalls/ThreadManager.h +++ b/Source/Tools/LinuxEmulation/LinuxSyscalls/ThreadManager.h @@ -23,18 +23,18 @@ class SyscallHandler; class SignalDelegator; // A latch that can be inspected to see if there is a waiter currently active. -// This allows us to remove the race condition between a thread trying to go asleep and something else telling it to go to sleep or wake up. -// -// Only one thread can ever wait on a latch, while another thread signals it. +// In contrast to std::condition_variable, an InspectableLatch may be signaled *before* being waited on without change of effect. class InspectableLatch final { public: bool Wait(struct timespec* Timeout = nullptr) { + LOGMAN_THROW_A_FMT(Mutex.load() != HAS_SIGNALED, "Stale signal mutex!"); + LOGMAN_THROW_A_FMT(Mutex.load() != HAS_WAITER, "Can't have multiple waiters on a single InspectableLatch!"); while (true) { uint32_t Expected = HAS_NO_WAITER; const uint32_t Desired = HAS_WAITER; if (Mutex.compare_exchange_strong(Expected, Desired)) { - // We have latched, now futex. + // We have latched, sleep using a futex syscall until signaled. constexpr int Op = FUTEX_WAIT | FUTEX_PRIVATE_FLAG; // WAIT will keep sleeping on the futex word while it is `val` int Result = ::syscall(SYS_futex, &Mutex, Op, @@ -64,6 +64,9 @@ class InspectableLatch final { } void NotifyOne() { +#if defined(ASSERTIONS_ENABLED) && ASSERTIONS_ENABLED + LOGMAN_THROW_A_FMT(Mutex.load() != HAS_SIGNALED, "Trying to double signal!"); +#endif DoNotify(1); } @@ -81,8 +84,8 @@ class InspectableLatch final { uint32_t Expected = HAS_WAITER; const uint32_t Desired = HAS_SIGNALED; - // If the mutex is in a waiting state and we have CAS exchanged it to HAS_SIGNALED, then execute a futex syscall. - // otherwise just leave since nothing was waiting. + // If the mutex is in a waiting state and we have CAS exchanged it to HAS_SIGNALED, then signal the thread to wake up with a futex + // syscall. otherwise just leave since nothing was waiting. if (Mutex.compare_exchange_strong(Expected, Desired)) { constexpr int Op = FUTEX_WAKE | FUTEX_PRIVATE_FLAG; @@ -243,8 +246,8 @@ class ThreadManager final { } } - FEXCore::ForkableUniqueMutex* GetThreadsCreationMutex() { - return &ThreadCreationMutex; + FEXCore::ForkableUniqueMutex& GetThreadsCreationMutex() { + return ThreadCreationMutex; } const fextl::vector* GetThreads() const {