From 3c7ad57650fa0321d776a75c5de103a084838b9f Mon Sep 17 00:00:00 2001 From: Kawe Mazidjatari <48657826+Mauler125@users.noreply.github.com> Date: Wed, 28 Dec 2022 21:38:47 +0100 Subject: [PATCH] CCrashHandler improvements * Move large module instance array to the heap. * Add ability to query whether or not an exception was handled. --- r5dev/public/utility/crashhandler.cpp | 45 ++++++++++++++++++++------- r5dev/public/utility/crashhandler.h | 13 ++++++-- 2 files changed, 43 insertions(+), 15 deletions(-) diff --git a/r5dev/public/utility/crashhandler.cpp b/r5dev/public/utility/crashhandler.cpp index e552c574..89f546a5 100644 --- a/r5dev/public/utility/crashhandler.cpp +++ b/r5dev/public/utility/crashhandler.cpp @@ -9,6 +9,24 @@ #include "public/utility/binstream.h" #include "public/utility/crashhandler.h" +//----------------------------------------------------------------------------- +// Purpose: +//----------------------------------------------------------------------------- +void CCrashHandler::Start() +{ + Lock(); + m_bExceptionHandled = true; +} + +//----------------------------------------------------------------------------- +// Purpose: +//----------------------------------------------------------------------------- +void CCrashHandler::End() +{ + m_bExceptionHandled = false; + Unlock(); +} + //----------------------------------------------------------------------------- // Purpose: formats the crasher (module, address and exception) //----------------------------------------------------------------------------- @@ -108,12 +126,12 @@ void CCrashHandler::FormatModules() { m_svBuffer.append("modules:\n{\n"); - HMODULE hModule[4096]; + std::unique_ptr hModule(new HMODULE[4096]); HANDLE hProcess = GetCurrentProcess(); DWORD cbNeeded; - BOOL result = K32EnumProcessModulesEx(hProcess, hModule, sizeof(hModule), &cbNeeded, LIST_MODULES_ALL); - if (result && cbNeeded <= sizeof(hModule) && cbNeeded >> 3) + BOOL result = K32EnumProcessModulesEx(hProcess, &*hModule.get(), 4096, &cbNeeded, LIST_MODULES_ALL); + if (result && cbNeeded <= 4096 && cbNeeded >> 3) { CHAR szModuleName[512]; LPSTR pszModuleName; @@ -121,10 +139,10 @@ void CCrashHandler::FormatModules() for (DWORD i = 0, j = cbNeeded >> 3; j; i++, j--) { - DWORD m = GetModuleFileNameA(hModule[i], szModuleName, sizeof(szModuleName)); + DWORD m = GetModuleFileNameA(&*hModule.get()[i], szModuleName, sizeof(szModuleName)); if ((m - 1) > (sizeof(szModuleName) - 2)) // Too small for buffer. { - snprintf(szModuleName, sizeof(szModuleName), "module@%p", hModule[i]); + snprintf(szModuleName, sizeof(szModuleName), "module@%p", &*hModule.get()[i]); pszModuleName = szModuleName; } else @@ -132,7 +150,7 @@ void CCrashHandler::FormatModules() pszModuleName = strrchr(szModuleName, '\\') + 1; } - K32GetModuleInformation(hProcess, hModule[i], &modInfo, sizeof(modInfo)); + K32GetModuleInformation(hProcess, &*hModule.get()[i], &modInfo, sizeof(modInfo)); m_svBuffer.append(fmt::format("\t{:15s}: [0x{:016X}, 0x{:016X}]\n", pszModuleName, reinterpret_cast(modInfo.lpBaseOfDll), @@ -515,29 +533,30 @@ void CCrashHandler::CreateMessageProcess() //----------------------------------------------------------------------------- long __stdcall ExceptionFilter(EXCEPTION_POINTERS* pExceptionInfo) { - g_CrashHandler->Lock(); - + g_CrashHandler->Start(); g_CrashHandler->SetExceptionPointers(pExceptionInfo); + + // Let the higher level exception handlers deal with this particular exception. if (g_CrashHandler->ExceptionToString() == g_CrashHandler->ExceptionToString(-1)) { - g_CrashHandler->Unlock(); + g_CrashHandler->End(); return EXCEPTION_CONTINUE_SEARCH; } // Don't run when a debugger is present. if (IsDebuggerPresent()) { - g_CrashHandler->Unlock(); + g_CrashHandler->End(); return EXCEPTION_CONTINUE_SEARCH; } g_CrashHandler->GetCallStack(); - // Don't run when exception return address is in whitelist. + // Don't run filter when exception return address is in whitelist. // This is useful for when we want to use a different exception handler instead. if (g_CrashHandler->HasWhitelist()) { - g_CrashHandler->Unlock(); + g_CrashHandler->End(); return EXCEPTION_CONTINUE_SEARCH; } @@ -556,6 +575,7 @@ long __stdcall ExceptionFilter(EXCEPTION_POINTERS* pExceptionInfo) g_CrashHandler->WriteFile(); g_CrashHandler->CreateMessageProcess(); // Display the message to the user. + // Don't end, just unlock the mutex so the next call kills the process. g_CrashHandler->Unlock(); return EXCEPTION_EXECUTE_HANDLER; @@ -570,6 +590,7 @@ CCrashHandler::CCrashHandler() , m_nCapturedFrames(0) , m_nCrashMsgFlags(0) , m_bCallState(false) + , m_bExceptionHandled(false) , m_bCrashMsgCreated(false) { m_hExceptionHandler = AddVectoredExceptionHandler(TRUE, ExceptionFilter); diff --git a/r5dev/public/utility/crashhandler.h b/r5dev/public/utility/crashhandler.h index 20961a76..76d201b9 100644 --- a/r5dev/public/utility/crashhandler.h +++ b/r5dev/public/utility/crashhandler.h @@ -13,11 +13,17 @@ public: //------------------------------------------------------------------------- // Inlines: //------------------------------------------------------------------------- + void Start(); + void End(); + void Lock() const { m_Mutex.lock(); }; void Unlock() const { m_Mutex.unlock(); }; - bool GetState() const { return m_bCallState; }; + void SetState(bool bState) { m_bCallState = bState; }; + bool GetState() const { return m_bCallState; }; + bool IsValid() const { return m_hExceptionHandler != nullptr; }; + bool Handled() const { return m_bExceptionHandled; }; //------------------------------------------------------------------------- // Formatters: @@ -75,8 +81,9 @@ private: string m_svCrashMsgInfo; uint8_t m_nCrashMsgFlags; - bool m_bCallState; // Set when called to prevent recursive calls. - bool m_bCrashMsgCreated; // Set when crashmsg.exe is created to prevent recursive messages. + bool m_bCallState; // Set when called to prevent recursive calls. + bool m_bCrashMsgCreated; // Set when crashmsg.exe is created to prevent recursive messages. + bool m_bExceptionHandled; // Set on filter entry, unset within the same lock if exception was not handled, never unset if handled. std::set m_WhiteList; mutable std::mutex m_Mutex;