From c2588403c0b8cf198f13f903f626851c7e94266c Mon Sep 17 00:00:00 2001 From: Yuri Kunde Schlesner Date: Thu, 23 Oct 2014 01:20:01 -0200 Subject: HLE: Revamp error handling throrough the HLE code All service calls in the CTR OS return result codes indicating the success or failure of the call. Previous to this commit, Citra's HLE emulation of services and the kernel universally either ignored errors or returned dummy -1 error codes. This commit makes an initial effort to provide an infrastructure for error reporting and propagation which can be use going forward to make HLE calls accurately return errors as the original system. A few parts of the code have been updated to use the new system where applicable. One part of this effort is the definition of the `ResultCode` type, which provides facilities for constructing and parsing error codes in the structured format used by the CTR. The `ResultVal` type builds on `ResultCode` by providing a container for values returned by function that can report errors. It enforces that correct error checking will be done on function returns by preventing the use of the return value if the function returned an error code. Currently this change is mostly internal since errors are still suppressed on the ARM<->HLE border, as a temporary compatibility hack. As functionality is implemented and tested this hack can be eventually removed. --- src/core/hle/kernel/thread.cpp | 51 +++++++++++++++++++++++++----------------- 1 file changed, 30 insertions(+), 21 deletions(-) (limited to 'src/core/hle/kernel/thread.cpp') diff --git a/src/core/hle/kernel/thread.cpp b/src/core/hle/kernel/thread.cpp index cc70cbca7..b01779f2e 100644 --- a/src/core/hle/kernel/thread.cpp +++ b/src/core/hle/kernel/thread.cpp @@ -11,10 +11,11 @@ #include "common/thread_queue_list.h" #include "core/core.h" -#include "core/mem_map.h" #include "core/hle/hle.h" #include "core/hle/kernel/kernel.h" #include "core/hle/kernel/thread.h" +#include "core/hle/result.h" +#include "core/mem_map.h" namespace Kernel { @@ -38,16 +39,17 @@ public: * @param wait Boolean wait set if current thread should wait as a result of sync operation * @return Result of operation, 0 on success, otherwise error code */ - Result WaitSynchronization(bool* wait) override { + ResultVal WaitSynchronization() override { if (status != THREADSTATUS_DORMANT) { Handle thread = GetCurrentThreadHandle(); if (std::find(waiting_threads.begin(), waiting_threads.end(), thread) == waiting_threads.end()) { waiting_threads.push_back(thread); } WaitCurrentThread(WAITTYPE_THREADEND, this->GetHandle()); - *wait = true; + return MakeResult(true); + } else { + return MakeResult(false); } - return 0; } ThreadContext context; @@ -144,9 +146,9 @@ void ChangeReadyState(Thread* t, bool ready) { } /// Verify that a thread has not been released from waiting -inline bool VerifyWait(const Handle& handle, WaitType type, Handle wait_handle) { - Thread* thread = g_object_pool.GetFast(handle); - _assert_msg_(KERNEL, (thread != nullptr), "called, but thread is nullptr!"); +inline bool VerifyWait(Handle handle, WaitType type, Handle wait_handle) { + Thread* thread = g_object_pool.Get(handle); + _dbg_assert_(KERNEL, thread != nullptr); if (type != thread->wait_type || wait_handle != thread->wait_handle) return false; @@ -155,9 +157,9 @@ inline bool VerifyWait(const Handle& handle, WaitType type, Handle wait_handle) } /// Stops the current thread -void StopThread(Handle handle, const char* reason) { - Thread* thread = g_object_pool.GetFast(handle); - _assert_msg_(KERNEL, (thread != nullptr), "called, but thread is nullptr!"); +ResultCode StopThread(Handle handle, const char* reason) { + Thread* thread = g_object_pool.Get(handle); + if (thread == nullptr) return InvalidHandle(ErrorModule::Kernel); ChangeReadyState(thread, false); thread->status = THREADSTATUS_DORMANT; @@ -172,6 +174,8 @@ void StopThread(Handle handle, const char* reason) { // Stopped threads are never waiting. thread->wait_type = WAITTYPE_NONE; thread->wait_handle = 0; + + return RESULT_SUCCESS; } /// Changes a threads state @@ -201,7 +205,9 @@ Handle ArbitrateHighestPriorityThread(u32 arbiter, u32 address) { if (!VerifyWait(handle, WAITTYPE_ARB, arbiter)) continue; - Thread* thread = g_object_pool.GetFast(handle); + Thread* thread = g_object_pool.Get(handle); + if (thread == nullptr) + continue; // TODO(yuriks): Thread handle will hang around forever. Should clean up. if(thread->current_priority <= priority) { highest_priority_thread = handle; priority = thread->current_priority; @@ -272,7 +278,7 @@ Thread* NextThread() { if (next == 0) { return nullptr; } - return Kernel::g_object_pool.GetFast(next); + return Kernel::g_object_pool.Get(next); } /** @@ -289,8 +295,7 @@ void WaitCurrentThread(WaitType wait_type, Handle wait_handle) { /// Resumes a thread from waiting by marking it as "ready" void ResumeThreadFromWait(Handle handle) { - u32 error; - Thread* thread = Kernel::g_object_pool.Get(handle, error); + Thread* thread = Kernel::g_object_pool.Get(handle); if (thread) { thread->status &= ~THREADSTATUS_WAIT; if (!(thread->status & (THREADSTATUS_WAITSUSPEND | THREADSTATUS_DORMANT | THREADSTATUS_DEAD))) { @@ -378,19 +383,23 @@ Handle CreateThread(const char* name, u32 entry_point, s32 priority, u32 arg, s3 } /// Get the priority of the thread specified by handle -u32 GetThreadPriority(const Handle handle) { - Thread* thread = g_object_pool.GetFast(handle); - _assert_msg_(KERNEL, (thread != nullptr), "called, but thread is nullptr!"); - return thread->current_priority; +ResultVal GetThreadPriority(const Handle handle) { + Thread* thread = g_object_pool.Get(handle); + if (thread == nullptr) return InvalidHandle(ErrorModule::Kernel); + + return MakeResult(thread->current_priority); } /// Set the priority of the thread specified by handle -Result SetThreadPriority(Handle handle, s32 priority) { +ResultCode SetThreadPriority(Handle handle, s32 priority) { Thread* thread = nullptr; if (!handle) { thread = GetCurrentThread(); // TODO(bunnei): Is this correct behavior? } else { - thread = g_object_pool.GetFast(handle); + thread = g_object_pool.Get(handle); + if (thread == nullptr) { + return InvalidHandle(ErrorModule::Kernel); + } } _assert_msg_(KERNEL, (thread != nullptr), "called, but thread is nullptr!"); @@ -417,7 +426,7 @@ Result SetThreadPriority(Handle handle, s32 priority) { thread_ready_queue.push_back(thread->current_priority, handle); } - return 0; + return RESULT_SUCCESS; } /// Sets up the primary application thread -- cgit v1.2.3 From 22c86824a4e4fe8953d7104f2fbdf853d3d30c60 Mon Sep 17 00:00:00 2001 From: Yuri Kunde Schlesner Date: Fri, 31 Oct 2014 21:19:20 -0200 Subject: Remove duplicated docs/update them for changed parameters. --- src/core/hle/kernel/thread.cpp | 5 ----- 1 file changed, 5 deletions(-) (limited to 'src/core/hle/kernel/thread.cpp') diff --git a/src/core/hle/kernel/thread.cpp b/src/core/hle/kernel/thread.cpp index b01779f2e..2521f883d 100644 --- a/src/core/hle/kernel/thread.cpp +++ b/src/core/hle/kernel/thread.cpp @@ -34,11 +34,6 @@ public: inline bool IsWaiting() const { return (status & THREADSTATUS_WAIT) != 0; } inline bool IsSuspended() const { return (status & THREADSTATUS_SUSPEND) != 0; } - /** - * Wait for kernel object to synchronize - * @param wait Boolean wait set if current thread should wait as a result of sync operation - * @return Result of operation, 0 on success, otherwise error code - */ ResultVal WaitSynchronization() override { if (status != THREADSTATUS_DORMANT) { Handle thread = GetCurrentThreadHandle(); -- cgit v1.2.3 From 8189593255df8ab4abb699082f2c48baa3b0656b Mon Sep 17 00:00:00 2001 From: Yuri Kunde Schlesner Date: Mon, 24 Nov 2014 15:51:50 -0200 Subject: Use pointers instead of passing handles around in some functions. --- src/core/hle/kernel/thread.cpp | 34 +++++++++++++++------------------- 1 file changed, 15 insertions(+), 19 deletions(-) (limited to 'src/core/hle/kernel/thread.cpp') diff --git a/src/core/hle/kernel/thread.cpp b/src/core/hle/kernel/thread.cpp index 2521f883d..f3f54a4e9 100644 --- a/src/core/hle/kernel/thread.cpp +++ b/src/core/hle/kernel/thread.cpp @@ -35,16 +35,16 @@ public: inline bool IsSuspended() const { return (status & THREADSTATUS_SUSPEND) != 0; } ResultVal WaitSynchronization() override { - if (status != THREADSTATUS_DORMANT) { + const bool wait = status != THREADSTATUS_DORMANT; + if (wait) { Handle thread = GetCurrentThreadHandle(); if (std::find(waiting_threads.begin(), waiting_threads.end(), thread) == waiting_threads.end()) { waiting_threads.push_back(thread); } WaitCurrentThread(WAITTYPE_THREADEND, this->GetHandle()); - return MakeResult(true); - } else { - return MakeResult(false); } + + return MakeResult(wait); } ThreadContext context; @@ -141,14 +141,9 @@ void ChangeReadyState(Thread* t, bool ready) { } /// Verify that a thread has not been released from waiting -inline bool VerifyWait(Handle handle, WaitType type, Handle wait_handle) { - Thread* thread = g_object_pool.Get(handle); +inline bool VerifyWait(const Thread* thread, WaitType type, Handle wait_handle) { _dbg_assert_(KERNEL, thread != nullptr); - - if (type != thread->wait_type || wait_handle != thread->wait_handle) - return false; - - return true; + return type == thread->wait_type && wait_handle == thread->wait_handle; } /// Stops the current thread @@ -158,10 +153,10 @@ ResultCode StopThread(Handle handle, const char* reason) { ChangeReadyState(thread, false); thread->status = THREADSTATUS_DORMANT; - for (size_t i = 0; i < thread->waiting_threads.size(); ++i) { - const Handle waiting_thread = thread->waiting_threads[i]; + for (Handle waiting_handle : thread->waiting_threads) { + Thread* waiting_thread = g_object_pool.Get(waiting_handle); if (VerifyWait(waiting_thread, WAITTYPE_THREADEND, handle)) { - ResumeThreadFromWait(waiting_thread); + ResumeThreadFromWait(waiting_handle); } } thread->waiting_threads.clear(); @@ -194,13 +189,13 @@ Handle ArbitrateHighestPriorityThread(u32 arbiter, u32 address) { s32 priority = THREADPRIO_LOWEST; // Iterate through threads, find highest priority thread that is waiting to be arbitrated... - for (const auto& handle : thread_queue) { + for (Handle handle : thread_queue) { + Thread* thread = g_object_pool.Get(handle); // TODO(bunnei): Verify arbiter address... - if (!VerifyWait(handle, WAITTYPE_ARB, arbiter)) + if (!VerifyWait(thread, WAITTYPE_ARB, arbiter)) continue; - Thread* thread = g_object_pool.Get(handle); if (thread == nullptr) continue; // TODO(yuriks): Thread handle will hang around forever. Should clean up. if(thread->current_priority <= priority) { @@ -219,10 +214,11 @@ Handle ArbitrateHighestPriorityThread(u32 arbiter, u32 address) { void ArbitrateAllThreads(u32 arbiter, u32 address) { // Iterate through threads, find highest priority thread that is waiting to be arbitrated... - for (const auto& handle : thread_queue) { + for (Handle handle : thread_queue) { + Thread* thread = g_object_pool.Get(handle); // TODO(bunnei): Verify arbiter address... - if (VerifyWait(handle, WAITTYPE_ARB, arbiter)) + if (VerifyWait(thread, WAITTYPE_ARB, arbiter)) ResumeThreadFromWait(handle); } } -- cgit v1.2.3