From 808d1bb424f72f9b4c9a5db1404b248201f4662d Mon Sep 17 00:00:00 2001 From: Exzap <13877693+Exzap@users.noreply.github.com> Date: Thu, 15 Jun 2023 22:18:53 +0200 Subject: [PATCH] Add debug asserts for invalid MEMPTR Also fixed some corruptions this uncovered --- .gitignore | 1 + src/Cafe/HW/MMU/MMU.h | 4 +- src/Cafe/OS/libs/coreinit/coreinit_Alarm.cpp | 1 + src/Cafe/OS/libs/coreinit/coreinit_MEM.cpp | 2 +- src/Cafe/OS/libs/coreinit/coreinit_Thread.cpp | 23 ++- src/Cafe/OS/libs/coreinit/coreinit_Thread.h | 12 ++ src/Cafe/OS/libs/erreula/erreula.cpp | 2 +- src/Cafe/OS/libs/nn_boss/nn_boss.cpp | 2 +- src/Cafe/OS/libs/padscore/padscore.cpp | 2 +- src/Cafe/OS/libs/vpad/vpad.cpp | 2 +- src/Common/MemPtr.h | 12 +- src/Common/precompiled.h | 107 ++++++----- .../DebugPPCThreadsWindow.cpp | 179 +++++++++--------- 13 files changed, 193 insertions(+), 156 deletions(-) diff --git a/.gitignore b/.gitignore index 51700872..e7f104d2 100644 --- a/.gitignore +++ b/.gitignore @@ -38,6 +38,7 @@ bin/debugger/* bin/sdcard/* bin/screenshots/* bin/dump/* +bin/cafeLibs/* !bin/shaderCache/info.txt bin/shaderCache/* diff --git a/src/Cafe/HW/MMU/MMU.h b/src/Cafe/HW/MMU/MMU.h index 03238aaa..222e8c0e 100644 --- a/src/Cafe/HW/MMU/MMU.h +++ b/src/Cafe/HW/MMU/MMU.h @@ -263,4 +263,6 @@ namespace MMU uint16 ReadMMIO_32(PAddr address); uint16 ReadMMIO_16(PAddr address); -} \ No newline at end of file +} + +#define MMU_IsInPPCMemorySpace(__ptr) ((const uint8*)(__ptr) >= memory_base && (const uint8*)(__ptr) < (memory_base + 0x100000000)) \ No newline at end of file diff --git a/src/Cafe/OS/libs/coreinit/coreinit_Alarm.cpp b/src/Cafe/OS/libs/coreinit/coreinit_Alarm.cpp index 50fc66df..afc13919 100644 --- a/src/Cafe/OS/libs/coreinit/coreinit_Alarm.cpp +++ b/src/Cafe/OS/libs/coreinit/coreinit_Alarm.cpp @@ -211,6 +211,7 @@ namespace coreinit void __OSInitiateAlarm(OSAlarm_t* alarm, uint64 startTime, uint64 period, MPTR handlerFunc, bool isPeriodic) { + cemu_assert_debug(MMU_IsInPPCMemorySpace(alarm)); cemu_assert_debug(__OSHasSchedulerLock()); uint64 nextTime = startTime; diff --git a/src/Cafe/OS/libs/coreinit/coreinit_MEM.cpp b/src/Cafe/OS/libs/coreinit/coreinit_MEM.cpp index 6586416f..d8cc500a 100644 --- a/src/Cafe/OS/libs/coreinit/coreinit_MEM.cpp +++ b/src/Cafe/OS/libs/coreinit/coreinit_MEM.cpp @@ -50,7 +50,7 @@ namespace coreinit MEMList g_list3; std::array gHeapFillValues{ 0xC3C3C3C3, 0xF3F3F3F3, 0xD3D3D3D3 }; - OSSpinLock gHeapGlobalLock; + SysAllocator gHeapGlobalLock; MEMHeapBase* gDefaultHeap; bool MEMHeapTable_Add(MEMHeapBase* heap) diff --git a/src/Cafe/OS/libs/coreinit/coreinit_Thread.cpp b/src/Cafe/OS/libs/coreinit/coreinit_Thread.cpp index 6b3dedc1..34a614bc 100644 --- a/src/Cafe/OS/libs/coreinit/coreinit_Thread.cpp +++ b/src/Cafe/OS/libs/coreinit/coreinit_Thread.cpp @@ -263,9 +263,7 @@ namespace coreinit thread = (OSThread_t*)memory_getPointerFromVirtualOffset(coreinit_allocFromSysArea(sizeof(OSThread_t), 32)); memset(thread, 0x00, sizeof(OSThread_t)); // init signatures - thread->context.magic0 = OS_CONTEXT_MAGIC_0; - thread->context.magic1 = OS_CONTEXT_MAGIC_1; - thread->magic = 'tHrD'; + thread->SetMagic(); thread->type = threadType; thread->state = (entryPoint != MPTR_NULL) ? OSThread_t::THREAD_STATE::STATE_READY : OSThread_t::THREAD_STATE::STATE_NONE; thread->entrypoint = _swapEndianU32(entryPoint); @@ -563,7 +561,10 @@ namespace coreinit // adds the thread to each core's run queue if in runable state void __OSAddReadyThreadToRunQueue(OSThread_t* thread) { + cemu_assert_debug(MMU_IsInPPCMemorySpace(thread)); + cemu_assert_debug(thread->IsValidMagic()); cemu_assert_debug(__OSHasSchedulerLock()); + if (thread->state != OSThread_t::THREAD_STATE::STATE_READY) return; if (thread->suspendCounter != 0) @@ -703,10 +704,18 @@ namespace coreinit } else if (prevAffinityMask != affinityMask) { - __OSRemoveThreadFromRunQueues(thread); - thread->attr = (thread->attr & ~7) | (affinityMask & 7); - thread->context.setAffinity(affinityMask); - __OSAddReadyThreadToRunQueue(thread); + if(thread->state != OSThread_t::THREAD_STATE::STATE_NONE) + { + __OSRemoveThreadFromRunQueues(thread); + thread->attr = (thread->attr & ~7) | (affinityMask & 7); + thread->context.setAffinity(affinityMask); + __OSAddReadyThreadToRunQueue(thread); + } + else + { + thread->attr = (thread->attr & ~7) | (affinityMask & 7); + thread->context.setAffinity(affinityMask); + } } __OSUnlockScheduler(); return true; diff --git a/src/Cafe/OS/libs/coreinit/coreinit_Thread.h b/src/Cafe/OS/libs/coreinit/coreinit_Thread.h index f1c39bd4..a2fa2a80 100644 --- a/src/Cafe/OS/libs/coreinit/coreinit_Thread.h +++ b/src/Cafe/OS/libs/coreinit/coreinit_Thread.h @@ -404,6 +404,18 @@ struct OSThread_t return 0; } + void SetMagic() + { + context.magic0 = OS_CONTEXT_MAGIC_0; + context.magic1 = OS_CONTEXT_MAGIC_1; + magic = 'tHrD'; + } + + bool IsValidMagic() const + { + return magic == 'tHrD' && context.magic0 == OS_CONTEXT_MAGIC_0 && context.magic1 == OS_CONTEXT_MAGIC_1; + } + /* +0x000 */ OSContext_t context; /* +0x320 */ uint32be magic; // 'tHrD' /* +0x324 */ betype state; diff --git a/src/Cafe/OS/libs/erreula/erreula.cpp b/src/Cafe/OS/libs/erreula/erreula.cpp index e8b82c2f..c95816b6 100644 --- a/src/Cafe/OS/libs/erreula/erreula.cpp +++ b/src/Cafe/OS/libs/erreula/erreula.cpp @@ -82,7 +82,7 @@ namespace erreula struct ErrEula_t { - coreinit::OSMutex mutex; + SysAllocator mutex; uint32 regionType; uint32 langType; MEMPTR fsClient; diff --git a/src/Cafe/OS/libs/nn_boss/nn_boss.cpp b/src/Cafe/OS/libs/nn_boss/nn_boss.cpp index c2a35341..fed77bcc 100644 --- a/src/Cafe/OS/libs/nn_boss/nn_boss.cpp +++ b/src/Cafe/OS/libs/nn_boss/nn_boss.cpp @@ -23,7 +23,7 @@ memset(bossRequest, 0, sizeof(iosuBossCemuRequest_t)); \ memset(bossBufferVector, 0, sizeof(ioBufferVector_t)); \ bossBufferVector->buffer = (uint8*)bossRequest; - coreinit::OSMutex g_mutex; + SysAllocator g_mutex; sint32 g_initCounter = 0; bool g_isInitialized = false; diff --git a/src/Cafe/OS/libs/padscore/padscore.cpp b/src/Cafe/OS/libs/padscore/padscore.cpp index 5c1203bf..661af5b0 100644 --- a/src/Cafe/OS/libs/padscore/padscore.cpp +++ b/src/Cafe/OS/libs/padscore/padscore.cpp @@ -54,7 +54,7 @@ namespace padscore WPADState_t g_wpad_state = kWPADStateMaster; struct { - coreinit::OSAlarm_t alarm; + SysAllocator alarm; bool kpad_initialized = false; struct WPADData diff --git a/src/Cafe/OS/libs/vpad/vpad.cpp b/src/Cafe/OS/libs/vpad/vpad.cpp index d869eef1..94bb0ca2 100644 --- a/src/Cafe/OS/libs/vpad/vpad.cpp +++ b/src/Cafe/OS/libs/vpad/vpad.cpp @@ -163,7 +163,7 @@ namespace vpad struct { - coreinit::OSAlarm_t alarm; + SysAllocator alarm; struct { diff --git a/src/Common/MemPtr.h b/src/Common/MemPtr.h index 7fa0ff18..dc1ecd36 100644 --- a/src/Common/MemPtr.h +++ b/src/Common/MemPtr.h @@ -37,7 +37,10 @@ public: if (ptr == nullptr) m_value = 0; else - m_value = (uint32)((uintptr_t)ptr - (uintptr_t)memory_base); + { + cemu_assert_debug((uint8*)ptr >= memory_base && (uint8*)ptr <= memory_base + 0x100000000); + m_value = (uint32)((uintptr_t)ptr - (uintptr_t)memory_base); + } } constexpr MEMPTR(const MEMPTR& memptr) @@ -63,10 +66,13 @@ public: MEMPTR& operator=(T* ptr) { - if (ptr == nullptr) + if (ptr == nullptr) m_value = 0; else - m_value = (uint32)((uintptr_t)ptr - (uintptr_t)memory_base); + { + cemu_assert_debug((uint8*)ptr >= memory_base && (uint8*)ptr <= memory_base + 0x100000000); + m_value = (uint32)((uintptr_t)ptr - (uintptr_t)memory_base); + } return *this; } diff --git a/src/Common/precompiled.h b/src/Common/precompiled.h index 939e3ec4..56f31a03 100644 --- a/src/Common/precompiled.h +++ b/src/Common/precompiled.h @@ -296,6 +296,61 @@ inline unsigned char _addcarry_u64(unsigned char carry, unsigned long long a, un #endif +// asserts + + +inline void cemu_assert(bool _condition) +{ + if ((_condition) == false) + { + DEBUG_BREAK; + } +} + +#ifndef CEMU_DEBUG_ASSERT +//#define cemu_assert_debug(__cond) -> Forcing __cond not to be evaluated currently has unexpected side-effects + +inline void cemu_assert_debug(bool _condition) +{ +} + +inline void cemu_assert_unimplemented() +{ +} + +inline void cemu_assert_suspicious() +{ +} + +inline void cemu_assert_error() +{ + DEBUG_BREAK; +} +#else +inline void cemu_assert_debug(bool _condition) +{ + if ((_condition) == false) + DEBUG_BREAK; +} + +inline void cemu_assert_unimplemented() +{ + DEBUG_BREAK; +} + +inline void cemu_assert_suspicious() +{ + DEBUG_BREAK; +} + +inline void cemu_assert_error() +{ + DEBUG_BREAK; +} +#endif + +#define assert_dbg() DEBUG_BREAK // old style unconditional generic assert + // MEMPTR #include "Common/MemPtr.h" @@ -380,58 +435,6 @@ bool match_any_of(T1 value, T2 compareTo, Types&&... others) #endif } -inline void cemu_assert(bool _condition) -{ - if ((_condition) == false) - { - DEBUG_BREAK; - } -} - -#ifndef CEMU_DEBUG_ASSERT -//#define cemu_assert_debug(__cond) -> Forcing __cond not to be evaluated currently has unexpected side-effects - -inline void cemu_assert_debug(bool _condition) -{ -} - -inline void cemu_assert_unimplemented() -{ -} - -inline void cemu_assert_suspicious() -{ -} - -inline void cemu_assert_error() -{ - DEBUG_BREAK; -} -#else -inline void cemu_assert_debug(bool _condition) -{ - if ((_condition) == false) - DEBUG_BREAK; -} - -inline void cemu_assert_unimplemented() -{ - DEBUG_BREAK; -} - -inline void cemu_assert_suspicious() -{ - DEBUG_BREAK; -} - -inline void cemu_assert_error() -{ - DEBUG_BREAK; -} -#endif - -#define assert_dbg() DEBUG_BREAK // old style unconditional generic assert - // Some string conversion helpers because C++20 std::u8string is too cumbersome to use in practice // mixing string types generally causes loads of issues and many of the libraries we use dont expose interfaces for u8string diff --git a/src/gui/windows/PPCThreadsViewer/DebugPPCThreadsWindow.cpp b/src/gui/windows/PPCThreadsViewer/DebugPPCThreadsWindow.cpp index 0c10f325..0f3a8aec 100644 --- a/src/gui/windows/PPCThreadsViewer/DebugPPCThreadsWindow.cpp +++ b/src/gui/windows/PPCThreadsViewer/DebugPPCThreadsWindow.cpp @@ -173,100 +173,103 @@ void DebugPPCThreadsWindow::RefreshThreadList() const int scrollPos = m_thread_list->GetScrollPos(0); m_thread_list->DeleteAllItems(); - __OSLockScheduler(); - srwlock_activeThreadList.LockWrite(); - for (sint32 i = 0; i < activeThreadCount; i++) - { - MPTR threadItrMPTR = activeThread[i]; - OSThread_t* cafeThread = (OSThread_t*)memory_getPointerFromVirtualOffset(threadItrMPTR); + if(activeThreadCount > 0) + { + __OSLockScheduler(); + srwlock_activeThreadList.LockWrite(); + for (sint32 i = 0; i < activeThreadCount; i++) + { + MPTR threadItrMPTR = activeThread[i]; + OSThread_t* cafeThread = (OSThread_t*)memory_getPointerFromVirtualOffset(threadItrMPTR); - char tempStr[512]; - sprintf(tempStr, "%08X", threadItrMPTR); + char tempStr[512]; + sprintf(tempStr, "%08X", threadItrMPTR); - wxListItem item; - item.SetId(i); - item.SetText(tempStr); - m_thread_list->InsertItem(item); - m_thread_list->SetItemData(item, (long)threadItrMPTR); - // entry point - sprintf(tempStr, "%08X", _swapEndianU32(cafeThread->entrypoint)); - m_thread_list->SetItem(i, 1, tempStr); - // stack base (low) - sprintf(tempStr, "%08X - %08X", _swapEndianU32(cafeThread->stackEnd), _swapEndianU32(cafeThread->stackBase)); - m_thread_list->SetItem(i, 2, tempStr); - // pc - RPLStoredSymbol* symbol = rplSymbolStorage_getByAddress(cafeThread->context.srr0); - if (symbol) - sprintf(tempStr, "%s (0x%08x)", (const char*)symbol->symbolName, cafeThread->context.srr0); - else - sprintf(tempStr, "%08X", cafeThread->context.srr0); - m_thread_list->SetItem(i, 3, tempStr); - // lr - sprintf(tempStr, "%08X", _swapEndianU32(cafeThread->context.lr)); - m_thread_list->SetItem(i, 4, tempStr); - // state - OSThread_t::THREAD_STATE threadState = cafeThread->state; - wxString threadStateStr = "UNDEFINED"; - if (cafeThread->suspendCounter != 0) - threadStateStr = "SUSPENDED"; - else if (threadState == OSThread_t::THREAD_STATE::STATE_NONE) - threadStateStr = "NONE"; - else if (threadState == OSThread_t::THREAD_STATE::STATE_READY) - threadStateStr = "READY"; - else if (threadState == OSThread_t::THREAD_STATE::STATE_RUNNING) - threadStateStr = "RUNNING"; - else if (threadState == OSThread_t::THREAD_STATE::STATE_WAITING) - threadStateStr = "WAITING"; - else if (threadState == OSThread_t::THREAD_STATE::STATE_MORIBUND) - threadStateStr = "MORIBUND"; - m_thread_list->SetItem(i, 5, threadStateStr); - // affinity - uint8 affinity = cafeThread->attr&7; - uint8 affinityReal = cafeThread->context.affinity; - if(affinity != affinityReal) - sprintf(tempStr, "(!) %d%d%d real: %d%d%d", (affinity >> 0) & 1, (affinity >> 1) & 1, (affinity >> 2) & 1, (affinityReal >> 0) & 1, (affinityReal >> 1) & 1, (affinityReal >> 2) & 1); - else - sprintf(tempStr, "%d%d%d", (affinity >> 0) & 1, (affinity >> 1) & 1, (affinity >> 2) & 1); - m_thread_list->SetItem(i, 6, tempStr); - // priority - sint32 effectivePriority = cafeThread->effectivePriority; - sprintf(tempStr, "%d", effectivePriority); - m_thread_list->SetItem(i, 7, tempStr); - // last awake in cycles - uint64 lastWakeUpTime = cafeThread->wakeUpTime; - sprintf(tempStr, "%" PRIu64, lastWakeUpTime); - m_thread_list->SetItem(i, 8, tempStr); - // awake time in cycles - uint64 awakeTime = cafeThread->totalCycles; - sprintf(tempStr, "%" PRIu64, awakeTime); - m_thread_list->SetItem(i, 9, tempStr); - // thread name - const char* threadName = "NULL"; - if (!cafeThread->threadName.IsNull()) - threadName = cafeThread->threadName.GetPtr(); - m_thread_list->SetItem(i, 10, threadName); - // GPR - sprintf(tempStr, "r3 %08x r4 %08x r5 %08x r6 %08x r7 %08x", _r(3), _r(4), _r(5), _r(6), _r(7)); - m_thread_list->SetItem(i, 11, tempStr); - // waiting condition / extra info - coreinit::OSMutex* mutex = cafeThread->waitingForMutex; - if (mutex) - sprintf(tempStr, "Mutex 0x%08x (Held by thread 0x%08X Lock-Count: %d)", memory_getVirtualOffsetFromPointer(mutex), mutex->owner.GetMPTR(), (uint32)mutex->lockCount); - else - sprintf(tempStr, ""); + wxListItem item; + item.SetId(i); + item.SetText(tempStr); + m_thread_list->InsertItem(item); + m_thread_list->SetItemData(item, (long)threadItrMPTR); + // entry point + sprintf(tempStr, "%08X", _swapEndianU32(cafeThread->entrypoint)); + m_thread_list->SetItem(i, 1, tempStr); + // stack base (low) + sprintf(tempStr, "%08X - %08X", _swapEndianU32(cafeThread->stackEnd), _swapEndianU32(cafeThread->stackBase)); + m_thread_list->SetItem(i, 2, tempStr); + // pc + RPLStoredSymbol* symbol = rplSymbolStorage_getByAddress(cafeThread->context.srr0); + if (symbol) + sprintf(tempStr, "%s (0x%08x)", (const char*)symbol->symbolName, cafeThread->context.srr0); + else + sprintf(tempStr, "%08X", cafeThread->context.srr0); + m_thread_list->SetItem(i, 3, tempStr); + // lr + sprintf(tempStr, "%08X", _swapEndianU32(cafeThread->context.lr)); + m_thread_list->SetItem(i, 4, tempStr); + // state + OSThread_t::THREAD_STATE threadState = cafeThread->state; + wxString threadStateStr = "UNDEFINED"; + if (cafeThread->suspendCounter != 0) + threadStateStr = "SUSPENDED"; + else if (threadState == OSThread_t::THREAD_STATE::STATE_NONE) + threadStateStr = "NONE"; + else if (threadState == OSThread_t::THREAD_STATE::STATE_READY) + threadStateStr = "READY"; + else if (threadState == OSThread_t::THREAD_STATE::STATE_RUNNING) + threadStateStr = "RUNNING"; + else if (threadState == OSThread_t::THREAD_STATE::STATE_WAITING) + threadStateStr = "WAITING"; + else if (threadState == OSThread_t::THREAD_STATE::STATE_MORIBUND) + threadStateStr = "MORIBUND"; + m_thread_list->SetItem(i, 5, threadStateStr); + // affinity + uint8 affinity = cafeThread->attr&7; + uint8 affinityReal = cafeThread->context.affinity; + if(affinity != affinityReal) + sprintf(tempStr, "(!) %d%d%d real: %d%d%d", (affinity >> 0) & 1, (affinity >> 1) & 1, (affinity >> 2) & 1, (affinityReal >> 0) & 1, (affinityReal >> 1) & 1, (affinityReal >> 2) & 1); + else + sprintf(tempStr, "%d%d%d", (affinity >> 0) & 1, (affinity >> 1) & 1, (affinity >> 2) & 1); + m_thread_list->SetItem(i, 6, tempStr); + // priority + sint32 effectivePriority = cafeThread->effectivePriority; + sprintf(tempStr, "%d", effectivePriority); + m_thread_list->SetItem(i, 7, tempStr); + // last awake in cycles + uint64 lastWakeUpTime = cafeThread->wakeUpTime; + sprintf(tempStr, "%" PRIu64, lastWakeUpTime); + m_thread_list->SetItem(i, 8, tempStr); + // awake time in cycles + uint64 awakeTime = cafeThread->totalCycles; + sprintf(tempStr, "%" PRIu64, awakeTime); + m_thread_list->SetItem(i, 9, tempStr); + // thread name + const char* threadName = "NULL"; + if (!cafeThread->threadName.IsNull()) + threadName = cafeThread->threadName.GetPtr(); + m_thread_list->SetItem(i, 10, threadName); + // GPR + sprintf(tempStr, "r3 %08x r4 %08x r5 %08x r6 %08x r7 %08x", _r(3), _r(4), _r(5), _r(6), _r(7)); + m_thread_list->SetItem(i, 11, tempStr); + // waiting condition / extra info + coreinit::OSMutex* mutex = cafeThread->waitingForMutex; + if (mutex) + sprintf(tempStr, "Mutex 0x%08x (Held by thread 0x%08X Lock-Count: %d)", memory_getVirtualOffsetFromPointer(mutex), mutex->owner.GetMPTR(), (uint32)mutex->lockCount); + else + sprintf(tempStr, ""); - // OSSetThreadCancelState - if (cafeThread->requestFlags & OSThread_t::REQUEST_FLAG_CANCEL) - strcat(tempStr, "[Cancel requested]"); + // OSSetThreadCancelState + if (cafeThread->requestFlags & OSThread_t::REQUEST_FLAG_CANCEL) + strcat(tempStr, "[Cancel requested]"); - m_thread_list->SetItem(i, 12, tempStr); + m_thread_list->SetItem(i, 12, tempStr); - if(selected_thread != 0 && selected_thread == (long)threadItrMPTR) - m_thread_list->SetItemState(i, wxLIST_STATE_FOCUSED | wxLIST_STATE_SELECTED, wxLIST_STATE_FOCUSED | wxLIST_STATE_SELECTED); - } - srwlock_activeThreadList.UnlockWrite(); - __OSUnlockScheduler(); + if(selected_thread != 0 && selected_thread == (long)threadItrMPTR) + m_thread_list->SetItemState(i, wxLIST_STATE_FOCUSED | wxLIST_STATE_SELECTED, wxLIST_STATE_FOCUSED | wxLIST_STATE_SELECTED); + } + srwlock_activeThreadList.UnlockWrite(); + __OSUnlockScheduler(); + } m_thread_list->SetScrollPos(0, scrollPos, true); }