From f4968793b81b00a727288b918dcb992c62aea0fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=97=B1=20PixelyIon?= Date: Thu, 2 Apr 2020 12:09:24 +0530 Subject: [PATCH] Fix minor inaccuracy with VMM and mutexes This commit fixes a tiny inaccuracy with the VMM which was a problem with block insertion, The mutexes on the other hand had a minor issue regarding owner checks. --- app/src/main/cpp/skyline/kernel/memory.cpp | 20 ++++--------------- app/src/main/cpp/skyline/kernel/svc.cpp | 2 +- .../cpp/skyline/kernel/types/KProcess.cpp | 18 ++++++++++------- .../java/emu/skyline/utility/LicenseDialog.kt | 1 - 4 files changed, 16 insertions(+), 25 deletions(-) diff --git a/app/src/main/cpp/skyline/kernel/memory.cpp b/app/src/main/cpp/skyline/kernel/memory.cpp index b9ef2932..cedc2c53 100644 --- a/app/src/main/cpp/skyline/kernel/memory.cpp +++ b/app/src/main/cpp/skyline/kernel/memory.cpp @@ -93,6 +93,9 @@ namespace skyline::kernel { } void MemoryManager::InsertBlock(ChunkDescriptor *chunk, const BlockDescriptor block) { + if(chunk->address + chunk->size < block.address + block.size) + throw exception("InsertBlock: Inserting block past chunk end is not allowed"); + for (auto iter = chunk->blockList.begin(); iter != chunk->blockList.end(); iter++) { if (iter->address <= block.address) { if ((iter->address + iter->size) > block.address) { @@ -107,23 +110,8 @@ namespace skyline::kernel { iter->size = iter->address - block.address; chunk->blockList.insert(std::next(iter), {block, endBlock}); } - } else if (std::next(iter) != chunk->blockList.end()) { - auto nextIter = std::next(iter); - auto nextEnd = nextIter->address + nextIter->size; - - if(nextEnd > block.address) { - iter->size = block.address - iter->address; - nextIter->address = block.address + block.size; - nextIter->size = nextEnd - nextIter->address; - - chunk->blockList.insert(nextIter, block); - } else { - throw exception("InsertBlock: Inserting block across more than one block is not allowed"); - } - } else { - throw exception("InsertBlock: Inserting block with end past chunk end is not allowed"); + return; } - return; } } diff --git a/app/src/main/cpp/skyline/kernel/svc.cpp b/app/src/main/cpp/skyline/kernel/svc.cpp index bea69e9d..70a90132 100644 --- a/app/src/main/cpp/skyline/kernel/svc.cpp +++ b/app/src/main/cpp/skyline/kernel/svc.cpp @@ -503,7 +503,7 @@ namespace skyline::kernel::svc { if (state.process->MutexLock(address, ownerHandle)) state.logger->Debug("svcArbitrateLock: Locked mutex at 0x{:X}", address); else - state.logger->Debug("svcArbitrateLock: Owner handle did not match current owner for mutex at 0x{:X}", address); + state.logger->Debug("svcArbitrateLock: Owner handle did not match current owner for mutex or didn't have waiter flag at 0x{:X}", address); state.ctx->registers.w0 = constant::status::Success; } diff --git a/app/src/main/cpp/skyline/kernel/types/KProcess.cpp b/app/src/main/cpp/skyline/kernel/types/KProcess.cpp index 4ef773f1..6db5d7d1 100644 --- a/app/src/main/cpp/skyline/kernel/types/KProcess.cpp +++ b/app/src/main/cpp/skyline/kernel/types/KProcess.cpp @@ -85,7 +85,7 @@ namespace skyline::kernel::type { Registers fregs{ .x0 = CLONE_THREAD | CLONE_SIGHAND | CLONE_PTRACE | CLONE_FS | CLONE_VM | CLONE_FILES | CLONE_IO, .x1 = stackTop, - .x3 = tlsMem->Map(0, size, memory::Permission{ true, true, false }), + .x3 = tlsMem->Map(0, size, memory::Permission{true, true, false}), .x8 = __NR_clone, .x5 = reinterpret_cast(&guest::GuestEntry), .x6 = entryPoint, @@ -203,10 +203,13 @@ namespace skyline::kernel::type { auto mtx = GetPointer(address); auto &mtxWaiters = mutexes[address]; - u32 mtxExpected = 0; - if (__atomic_compare_exchange_n(mtx, &mtxExpected, (constant::MtxOwnerMask & state.thread->handle) | (mtxWaiters.empty() ? 0 : ~constant::MtxOwnerMask), false, __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST)) - return true; - if (owner && (__atomic_load_n(mtx, __ATOMIC_SEQ_CST) != (owner | ~constant::MtxOwnerMask))) + if (mtxWaiters.empty()) { + u32 mtxExpected = 0; + if (__atomic_compare_exchange_n(mtx, &mtxExpected, (constant::MtxOwnerMask & state.thread->handle), false, __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST)) + return true; + } + + if (__atomic_load_n(mtx, __ATOMIC_SEQ_CST) != (owner | ~constant::MtxOwnerMask)) return false; std::shared_ptr status; @@ -245,7 +248,7 @@ namespace skyline::kernel::type { u32 mtxExpected = (constant::MtxOwnerMask & state.thread->handle) | ~constant::MtxOwnerMask; if (!__atomic_compare_exchange_n(mtx, &mtxExpected, mtxDesired, false, __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST)) { - mtxExpected = constant::MtxOwnerMask & state.thread->handle; + mtxExpected &= constant::MtxOwnerMask; if (!__atomic_compare_exchange_n(mtx, &mtxExpected, mtxDesired, false, __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST)) return false; @@ -277,14 +280,15 @@ namespace skyline::kernel::type { } lock.unlock(); - bool timedOut{}; + bool timedOut{}; auto start = utils::GetTimeNs(); while (!status->flag) if ((utils::GetTimeNs() - start) >= timeout) timedOut = true; lock.lock(); + if (!status->flag) timedOut = false; else diff --git a/app/src/main/java/emu/skyline/utility/LicenseDialog.kt b/app/src/main/java/emu/skyline/utility/LicenseDialog.kt index 137be584..32f09ed3 100644 --- a/app/src/main/java/emu/skyline/utility/LicenseDialog.kt +++ b/app/src/main/java/emu/skyline/utility/LicenseDialog.kt @@ -30,6 +30,5 @@ class LicenseDialog : DialogFragment() { license_url.text = libraryUrl license_content.text = libraryLicense - license_content.movementMethod = ScrollingMovementMethod() } }