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

@@ -17,18 +17,17 @@
 #include <android/native_window.h>
 
 namespace skyline {
-    // Global typedefs
+    using u128 = __uint128_t; //!< Unsigned 128-bit integer
-    typedef __uint128_t u128;
+    using u64 = __uint64_t; //!< Unsigned 64-bit integer
-    typedef __uint64_t u64;
+    using u32 = __uint32_t; //!< Unsigned 32-bit integer
-    typedef __uint32_t u32;
+    using u16 = __uint16_t; //!< Unsigned 16-bit integer
-    typedef __uint16_t u16;
+    using u8 = __uint8_t; //!< Unsigned 8-bit integer
-    typedef __uint8_t u8;
+    using i128 = __int128_t; //!< Signed 128-bit integer
-    typedef __int128_t i128;
+    using i64 = __int64_t; //!< Signed 64-bit integer
-    typedef __int64_t i64;
+    using i32 = __int32_t; //!< Signed 32-bit integer
-    typedef __int32_t i32;
+    using i16 = __int16_t; //!< Signed 16-bit integer
-    typedef __int16_t i16;
+    using i8 = __int8_t; //!< Signed 8-bit integer
-    typedef __int8_t i8;
+    using handle_t = u32; //!< The type of a kernel handle
-    typedef u32 handle_t; //!< The type of an 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<u64>(std::chrono::duration_cast<std::chrono::nanoseconds>(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;

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<u8>(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<type::KThread>(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<handle_t>(state.nce->GetRegister(Wreg::W0));
         try {
-            auto &object = state.thisProcess->handleTable.at(handle);
+            state.thisProcess->handleTable.erase(handle);
-            switch (object->objectType) {
-                case (type::KType::KThread):
-                    state.os->KillThread(std::static_pointer_cast<type::KThread>(object)->pid);
-                    break;
-                case (type::KType::KProcess):
-                    state.os->KillThread(std::static_pointer_cast<type::KProcess>(object)->mainThread);
-                    break;
-                default:
-                    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);

app/src/main/cpp/skyline/kernel/types/KProcess.cpp

@@ -1,5 +1,6 @@
 #include "KProcess.h"
 #include <nce.h>
+#include <os.h>
 #include <fcntl.h>
 #include <unistd.h>
 
@@ -71,9 +72,10 @@ namespace skyline::kernel::type {
         auto pid = static_cast<pid_t>(fregs.regs[0]);
         if (pid == -1)
             throw exception("Cannot create thread: Address: 0x{:X}, Stack Top: 0x{:X}", entryPoint, stackTop);
-        threadMap[pid] = NewHandle<KThread>(pid, entryPoint, entryArg, stackTop, GetTlsSlot(), priority, this).item;
+        auto process = NewHandle<KThread>(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);
+        threadMap[pid] = process;
-        return threadMap[pid];
+        state.os->processMap[pid] = state.os->processMap[mainThread];
+        return process;
     }
 
     void KProcess::ReadMemory(void *destination, u64 offset, size_t size) const {

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<int8_t>(constant::PriorityAn.first + ((static_cast<float>(constant::PriorityAn.second - constant::PriorityAn.first) / static_cast<float>(constant::PriorityNin.second - constant::PriorityNin.first)) * (static_cast<float>(priority) - constant::PriorityNin.first))); // Resize range PriorityNin (Nintendo Priority) to PriorityAn (Android Priority)

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<std::shared_ptr<KSyncObject>> 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)
          */

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);
+            state.logger->Debug("Killing process with PID: {}", pid);
-            // Erasing all shared_ptr instances to the process will call the destructor
+            for (auto&[key, value]: process->threadMap) {
-            // However, in the case these are not all instances of it we wouldn't want to call the destructor
+                value->Kill();
-            for (auto&[key, value]: process->threadMap)
                 processMap.erase(key);
+            }
             processVec.erase(std::remove(processVec.begin(), processVec.end(), pid), processVec.end());
         } else {
-            state.logger->Debug("Exiting thread with TID: {}", pid);
+            state.logger->Debug("Killing thread with TID: {}", pid);
-            process->handleTable.erase(process->threadMap[pid]->handle);
+            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);
         }
     }
 }

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> &logger, std::shared_ptr<Settings> &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);