From e11d7d9ce0ef799697779cfa89660a8671071a0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=97=B1=20PixelyIon?= Date: Fri, 22 Nov 2019 20:29:50 +0530 Subject: [PATCH] Fix threading implementation & Fix SVC logging This commit fixes the threading implementation and fixes errors in SVC logging and improves them in general. --- app/src/main/cpp/skyline/common.h | 29 ++++++++++--------- app/src/main/cpp/skyline/kernel/svc.cpp | 28 ++++++------------ .../cpp/skyline/kernel/types/KProcess.cpp | 8 +++-- .../main/cpp/skyline/kernel/types/KThread.cpp | 10 ++++++- .../main/cpp/skyline/kernel/types/KThread.h | 10 +++++-- app/src/main/cpp/skyline/os.cpp | 18 ++++++------ app/src/main/cpp/skyline/os.h | 3 +- 7 files changed, 57 insertions(+), 49 deletions(-) diff --git a/app/src/main/cpp/skyline/common.h b/app/src/main/cpp/skyline/common.h index 7081d0be..c8796105 100644 --- a/app/src/main/cpp/skyline/common.h +++ b/app/src/main/cpp/skyline/common.h @@ -17,18 +17,17 @@ #include namespace skyline { - // Global typedefs - typedef __uint128_t u128; - typedef __uint64_t u64; - typedef __uint32_t u32; - typedef __uint16_t u16; - typedef __uint8_t u8; - typedef __int128_t i128; - typedef __int64_t i64; - typedef __int32_t i32; - typedef __int16_t i16; - typedef __int8_t i8; - typedef u32 handle_t; //!< The type of an handle + using u128 = __uint128_t; //!< Unsigned 128-bit integer + using u64 = __uint64_t; //!< Unsigned 64-bit integer + using u32 = __uint32_t; //!< Unsigned 32-bit integer + using u16 = __uint16_t; //!< Unsigned 16-bit integer + using u8 = __uint8_t; //!< Unsigned 8-bit integer + using i128 = __int128_t; //!< Signed 128-bit integer + using i64 = __int64_t; //!< Signed 64-bit integer + using i32 = __int32_t; //!< Signed 32-bit integer + using i16 = __int16_t; //!< Signed 16-bit integer + using i8 = __int8_t; //!< Signed 8-bit integer + using handle_t = u32; //!< The type of a kernel handle namespace constant { // Memory @@ -74,6 +73,9 @@ namespace skyline { constexpr u32 DockedResolutionW = 1920; //!< The width component of the docked resolution constexpr u32 DockedResolutionH = 1080; //!< The height component of the docked resolution constexpr u32 TokenLength = 0x50; //!< The length of the token on BufferQueue parcels + constexpr u32 GobHeight = 0x8; //!< The height of a blocklinear GOB + constexpr u32 GobStride = 0x40; //!< The stride of a blocklinear GOB + constexpr u32 GobSize = GobHeight * GobStride; //!< The size of a blocklinear GOB // Status codes namespace status { constexpr u32 Success = 0x0; //!< "Success" @@ -94,7 +96,7 @@ namespace skyline { namespace instr { /** - * @brief A bit-field struct that encapsulates a BRK instruction. It can be used to generate as well as parse the instruction's opcode. See https://developer.arm.com/docs/ddi0596/latest/base-instructions-alphabetic-order/brk-breakpoint-instruction. + * @brief A bit-field struct that encapsulates a BRK instruction. See https://developer.arm.com/docs/ddi0596/latest/base-instructions-alphabetic-order/brk-breakpoint-instruction. */ struct Brk { /** @@ -328,7 +330,6 @@ namespace skyline { return static_cast(std::chrono::duration_cast(std::chrono::high_resolution_clock::now().time_since_epoch()).count()); } - // Predeclare some classes here as we use them in DeviceState class NCE; namespace gpu { class GPU; diff --git a/app/src/main/cpp/skyline/kernel/svc.cpp b/app/src/main/cpp/skyline/kernel/svc.cpp index af167aee..a66510ba 100644 --- a/app/src/main/cpp/skyline/kernel/svc.cpp +++ b/app/src/main/cpp/skyline/kernel/svc.cpp @@ -26,13 +26,13 @@ namespace skyline::kernel::svc { void SetMemoryAttribute(DeviceState &state) { const u64 addr = state.nce->GetRegister(Xreg::X0); - if((addr & (PAGE_SIZE - 1))) { + if((addr & (PAGE_SIZE - 1U))) { state.nce->SetRegister(Wreg::W0, constant::status::InvAddress); state.logger->Warn("svcSetMemoryAttribute: 'address' not page aligned: {}", addr); return; } const u64 size = state.nce->GetRegister(Xreg::X1); - if((size & (PAGE_SIZE - 1)) || !size) { + if((size & (PAGE_SIZE - 1U)) || !size) { state.nce->SetRegister(Wreg::W0, constant::status::InvSize); state.logger->Warn("svcSetMemoryAttribute: 'size' {}: {}", size ? "not page aligned" : "is zero", size); return; @@ -127,22 +127,22 @@ namespace skyline::kernel::svc { u64 entryArg = state.nce->GetRegister(Xreg::X2); u64 stackTop = state.nce->GetRegister(Xreg::X3); u8 priority = static_cast(state.nce->GetRegister(Wreg::W4)); - if(priority >= constant::PriorityNin.first && priority <= constant::PriorityNin.second) { + if((priority < constant::PriorityNin.first) && (priority > constant::PriorityNin.second)) { // NOLINT(misc-redundant-expression) state.nce->SetRegister(Wreg::W0, constant::status::InvAddress); - state.logger->Warn("svcSetHeapSize: 'priority' invalid: {}", priority); + state.logger->Warn("svcCreateThread: 'priority' invalid: {}", priority); return; } auto thread = state.thisProcess->CreateThread(entryAddr, entryArg, stackTop, priority); - state.nce->SetRegister(Wreg::W0, constant::status::Success); + state.logger->Debug("svcCreateThread: Created thread with handle 0x{:X} (Entry Point: 0x{:X}, Argument: 0x{:X}, Stack Pointer: 0x{:X}, Priority: {}, PID: {})", thread->handle, entryAddr, entryArg, stackTop, priority, thread->pid); state.nce->SetRegister(Wreg::W1, thread->handle); - state.logger->Info("svcCreateThread: Created thread with handle 0x{:X}", thread->handle); + state.nce->SetRegister(Wreg::W0, constant::status::Success); } void StartThread(DeviceState &state) { auto handle = state.nce->GetRegister(Wreg::W0); try { auto thread = state.thisProcess->GetHandle(handle); - state.logger->Debug("svcStartThread: Starting thread: 0x{:X}, {}", handle, thread->pid); + state.logger->Debug("svcStartThread: Starting thread: 0x{:X}, PID: {}", handle, thread->pid); thread->Start(); } catch (const std::exception&) { state.logger->Warn("svcStartThread: 'handle' invalid: 0x{:X}", handle); @@ -257,17 +257,7 @@ namespace skyline::kernel::svc { void CloseHandle(DeviceState &state) { auto handle = static_cast(state.nce->GetRegister(Wreg::W0)); try { - auto &object = state.thisProcess->handleTable.at(handle); - switch (object->objectType) { - case (type::KType::KThread): - state.os->KillThread(std::static_pointer_cast(object)->pid); - break; - case (type::KType::KProcess): - state.os->KillThread(std::static_pointer_cast(object)->mainThread); - break; - default: - state.thisProcess->handleTable.erase(handle); - } + state.thisProcess->handleTable.erase(handle); state.logger->Debug("svcCloseHandle: Closing handle: 0x{:X}", handle); state.nce->SetRegister(Wreg::W0, constant::status::Success); } catch(const std::exception&) { @@ -408,7 +398,7 @@ namespace skyline::kernel::svc { auto count = state.nce->GetRegister(Wreg::W1); state.nce->SetRegister(Wreg::W0, constant::status::Success); if (!state.thisProcess->condVarMap.count(address)) { - state.logger->Warn("svcSignalProcessWideKey: No Conditional-Variable at 0x{:X}", address); + state.logger->Debug("svcSignalProcessWideKey: No Conditional-Variable at 0x{:X}", address); return; } auto &cvarVec = state.thisProcess->condVarMap.at(address); diff --git a/app/src/main/cpp/skyline/kernel/types/KProcess.cpp b/app/src/main/cpp/skyline/kernel/types/KProcess.cpp index 9dfe0686..7374a44d 100644 --- a/app/src/main/cpp/skyline/kernel/types/KProcess.cpp +++ b/app/src/main/cpp/skyline/kernel/types/KProcess.cpp @@ -1,5 +1,6 @@ #include "KProcess.h" #include +#include #include #include @@ -71,9 +72,10 @@ namespace skyline::kernel::type { auto pid = static_cast(fregs.regs[0]); if (pid == -1) throw exception("Cannot create thread: Address: 0x{:X}, Stack Top: 0x{:X}", entryPoint, stackTop); - threadMap[pid] = NewHandle(pid, entryPoint, entryArg, stackTop, GetTlsSlot(), priority, this).item; - state.logger->Debug("A new thread was created: EP: 0x{:X}, EA: 0x{:X}, STP: 0x{:X}, PR: 0x{:X}, TLS: {}", entryPoint, entryArg, stackTop, priority, threadMap[pid]->tls); - return threadMap[pid]; + auto process = NewHandle(pid, entryPoint, entryArg, stackTop, GetTlsSlot(), priority, this).item; + threadMap[pid] = process; + state.os->processMap[pid] = state.os->processMap[mainThread]; + return process; } void KProcess::ReadMemory(void *destination, u64 offset, size_t size) const { diff --git a/app/src/main/cpp/skyline/kernel/types/KThread.cpp b/app/src/main/cpp/skyline/kernel/types/KThread.cpp index 87d70366..3537ee35 100644 --- a/app/src/main/cpp/skyline/kernel/types/KThread.cpp +++ b/app/src/main/cpp/skyline/kernel/types/KThread.cpp @@ -9,7 +9,7 @@ namespace skyline::kernel::type { } KThread::~KThread() { - kill(pid, SIGKILL); + Kill(); } void KThread::Start() { @@ -21,6 +21,14 @@ namespace skyline::kernel::type { } } + void KThread::Kill() { + if(status != Status::Dead) { + status = Status::Dead; + kill(pid, SIGKILL); + Signal(); + } + } + void KThread::UpdatePriority(u8 priority) { this->priority = priority; auto liPriority = static_cast(constant::PriorityAn.first + ((static_cast(constant::PriorityAn.second - constant::PriorityAn.first) / static_cast(constant::PriorityNin.second - constant::PriorityNin.first)) * (static_cast(priority) - constant::PriorityNin.first))); // Resize range PriorityNin (Nintendo Priority) to PriorityAn (Android Priority) diff --git a/app/src/main/cpp/skyline/kernel/types/KThread.h b/app/src/main/cpp/skyline/kernel/types/KThread.h index 39227e5b..261d5445 100644 --- a/app/src/main/cpp/skyline/kernel/types/KThread.h +++ b/app/src/main/cpp/skyline/kernel/types/KThread.h @@ -20,7 +20,8 @@ namespace skyline::kernel::type { WaitSync, //!< The thread is waiting for a KSyncObject signal WaitMutex, //!< The thread is waiting on a Mutex WaitCondVar, //!< The thread is waiting on a Conditional Variable - Runnable //!< The thread is ready to run after waiting + Runnable, //!< The thread is ready to run after waiting + Dead //!< The thread is dead and not running } status = Status::Created; //!< The state of the thread std::vector> waitObjects; //!< A vector holding the objects this thread is waiting for u64 timeout{}; //!< The end of a timeout for svcWaitSynchronization or the end of the sleep period for svcSleepThread @@ -49,10 +50,15 @@ namespace skyline::kernel::type { ~KThread(); /** - * @brief This starts this thread + * @brief This starts this thread process */ void Start(); + /** + * @brief This kills the thread + */ + void Kill(); + /** * @brief This causes this thread to sleep indefinitely (no-op if thread is already sleeping) */ diff --git a/app/src/main/cpp/skyline/os.cpp b/app/src/main/cpp/skyline/os.cpp index 8e1601b0..98918899 100644 --- a/app/src/main/cpp/skyline/os.cpp +++ b/app/src/main/cpp/skyline/os.cpp @@ -19,8 +19,8 @@ namespace skyline::kernel { } /** - * Function executed by all child processes after cloning - */ + * Function executed by all child processes after cloning + */ int ExecuteChild(void *) { ptrace(PTRACE_TRACEME); asm volatile("Brk #0xFF"); // BRK #constant::brkRdy (So we know when the thread/process is ready) @@ -48,15 +48,15 @@ namespace skyline::kernel { void OS::KillThread(pid_t pid) { auto process = processMap.at(pid); if (process->mainThread == pid) { - state.logger->Debug("Exiting process with PID: {}", pid); - // Erasing all shared_ptr instances to the process will call the destructor - // However, in the case these are not all instances of it we wouldn't want to call the destructor - for (auto&[key, value]: process->threadMap) + state.logger->Debug("Killing process with PID: {}", pid); + for (auto&[key, value]: process->threadMap) { + value->Kill(); processMap.erase(key); + } processVec.erase(std::remove(processVec.begin(), processVec.end(), pid), processVec.end()); } else { - state.logger->Debug("Exiting thread with TID: {}", pid); - process->handleTable.erase(process->threadMap[pid]->handle); + state.logger->Debug("Killing thread with TID: {}", pid); + process->threadMap.at(pid)->Kill(); process->threadMap.erase(pid); processMap.erase(pid); } @@ -70,7 +70,7 @@ namespace skyline::kernel { } else throw exception("Unimplemented SVC 0x{:X}", svc); } catch(const exception& e) { - throw exception("{} (SVC: {})", e.what(), svc); + throw exception("{} (SVC: 0x{:X})", e.what(), svc); } } } diff --git a/app/src/main/cpp/skyline/os.h b/app/src/main/cpp/skyline/os.h index 2f233683..9add81ce 100644 --- a/app/src/main/cpp/skyline/os.h +++ b/app/src/main/cpp/skyline/os.h @@ -28,11 +28,12 @@ namespace skyline::kernel { /** * @param logger An instance of the Logger class * @param settings An instance of the Settings class + * @param window The ANativeWindow object to draw the screen to */ OS(std::shared_ptr &logger, std::shared_ptr &settings, ANativeWindow *window); /** - * @brief Execute a particular ROM file. This launches a the main processes and calls the NCE class to handle execution. + * @brief Execute a particular ROM file. This launches the main process and calls the NCE class to handle execution. * @param romFile The path to the ROM file to execute */ void Execute(const std::string &romFile);