From 12f7d42d32511955ee27875d42b6e8e3cda9e523 Mon Sep 17 00:00:00 2001 From: lat9nq Date: Sat, 30 Jul 2022 10:23:14 -0400 Subject: mini_dump: Address review feedback Uses fmt::print as opposed to std::fprintf. Adds a missing return. static's a single-use function. Initializes structs as opposed to std::memset where possible. Fixes CMake linkage. Co-authored-by: Lioncash mini_dump: Use a namespace Co-authored-by: Lioncash --- src/yuzu/CMakeLists.txt | 2 +- src/yuzu/main.cpp | 5 +- src/yuzu/mini_dump.cpp | 122 +++++++++++++++++++++++++----------------------- src/yuzu/mini_dump.h | 5 +- 4 files changed, 71 insertions(+), 63 deletions(-) (limited to 'src') diff --git a/src/yuzu/CMakeLists.txt b/src/yuzu/CMakeLists.txt index df0f64b83..29d506c47 100644 --- a/src/yuzu/CMakeLists.txt +++ b/src/yuzu/CMakeLists.txt @@ -214,7 +214,7 @@ if (WIN32 AND YUZU_CRASH_DUMPS) mini_dump.h ) - target_link_libraries(yuzu PUBLIC ${DBGHELP_LIBRARY}) + target_link_libraries(yuzu PRIVATE ${DBGHELP_LIBRARY}) target_compile_definitions(yuzu PRIVATE -DYUZU_DBGHELP) endif() diff --git a/src/yuzu/main.cpp b/src/yuzu/main.cpp index ff59c64c3..bda9986e1 100644 --- a/src/yuzu/main.cpp +++ b/src/yuzu/main.cpp @@ -4096,10 +4096,11 @@ int main(int argc, char* argv[]) { #ifdef YUZU_DBGHELP PROCESS_INFORMATION pi; - if (!is_child && Settings::values.create_crash_dumps.GetValue() && SpawnDebuggee(argv[0], pi)) { + if (!is_child && Settings::values.create_crash_dumps.GetValue() && + MiniDump::SpawnDebuggee(argv[0], pi)) { // Delete the config object so that it doesn't save when the program exits config.reset(nullptr); - DebugDebuggee(pi); + MiniDump::DebugDebuggee(pi); return 0; } #endif diff --git a/src/yuzu/mini_dump.cpp b/src/yuzu/mini_dump.cpp index b25067c10..a34dc6a9c 100644 --- a/src/yuzu/mini_dump.cpp +++ b/src/yuzu/mini_dump.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include "yuzu/mini_dump.h" #include "yuzu/startup_checks.h" @@ -12,6 +13,8 @@ // dbghelp.h must be included after windows.h #include +namespace MiniDump { + void CreateMiniDump(HANDLE process_handle, DWORD process_id, MINIDUMP_EXCEPTION_INFORMATION* info, EXCEPTION_POINTERS* pep) { char file_name[255]; @@ -23,7 +26,7 @@ void CreateMiniDump(HANDLE process_handle, DWORD process_id, MINIDUMP_EXCEPTION_ CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr); if (file_handle == nullptr || file_handle == INVALID_HANDLE_VALUE) { - std::fprintf(stderr, "CreateFileA failed. Error: %d\n", GetLastError()); + fmt::print(stderr, "CreateFileA failed. Error: {}", GetLastError()); return; } @@ -34,9 +37,9 @@ void CreateMiniDump(HANDLE process_handle, DWORD process_id, MINIDUMP_EXCEPTION_ dump_type, (pep != 0) ? info : 0, 0, 0); if (write_dump_status) { - std::fprintf(stderr, "MiniDump created: %s\n", file_name); + fmt::print(stderr, "MiniDump created: {}", file_name); } else { - std::fprintf(stderr, "MiniDumpWriteDump failed. Error: %d\n", GetLastError()); + fmt::print(stderr, "MiniDumpWriteDump failed. Error: {}", GetLastError()); } // Close the file @@ -48,15 +51,15 @@ void DumpFromDebugEvent(DEBUG_EVENT& deb_ev, PROCESS_INFORMATION& pi) { HANDLE thread_handle = OpenThread(THREAD_GET_CONTEXT, false, deb_ev.dwThreadId); if (thread_handle == nullptr) { - std::fprintf(stderr, "OpenThread failed (%d)\n", GetLastError()); + fmt::print(stderr, "OpenThread failed ({})", GetLastError()); + return; } // Get child process context - CONTEXT context; - std::memset(&context, 0, sizeof(context)); + CONTEXT context = {}; context.ContextFlags = CONTEXT_ALL; if (!GetThreadContext(thread_handle, &context)) { - std::fprintf(stderr, "GetThreadContext failed (%d)\n", GetLastError()); + fmt::print(stderr, "GetThreadContext failed ({})", GetLastError()); return; } @@ -73,7 +76,7 @@ void DumpFromDebugEvent(DEBUG_EVENT& deb_ev, PROCESS_INFORMATION& pi) { CreateMiniDump(pi.hProcess, pi.dwProcessId, &info, &ep); if (CloseHandle(thread_handle) == 0) { - std::fprintf(stderr, "error: CloseHandle(thread_handle) failed (%d)\n", GetLastError()); + fmt::print(stderr, "error: CloseHandle(thread_handle) failed ({})", GetLastError()); } } @@ -86,67 +89,22 @@ bool SpawnDebuggee(const char* arg0, PROCESS_INFORMATION& pi) { } if (!SpawnChild(arg0, &pi, 0)) { - std::fprintf(stderr, "warning: continuing without crash dumps\n"); + fmt::print(stderr, "warning: continuing without crash dumps"); return false; } const bool can_debug = DebugActiveProcess(pi.dwProcessId); if (!can_debug) { - std::fprintf(stderr, - "warning: DebugActiveProcess failed (%d), continuing without crash dumps\n", - GetLastError()); + fmt::print(stderr, + "warning: DebugActiveProcess failed ({}), continuing without crash dumps", + GetLastError()); return false; } return true; } -void DebugDebuggee(PROCESS_INFORMATION& pi) { - DEBUG_EVENT deb_ev; - std::memset(&deb_ev, 0, sizeof(deb_ev)); - - while (deb_ev.dwDebugEventCode != EXIT_PROCESS_DEBUG_EVENT) { - const bool wait_success = WaitForDebugEvent(&deb_ev, INFINITE); - if (!wait_success) { - std::fprintf(stderr, "error: WaitForDebugEvent failed (%d)\n", GetLastError()); - return; - } - - switch (deb_ev.dwDebugEventCode) { - case OUTPUT_DEBUG_STRING_EVENT: - case CREATE_PROCESS_DEBUG_EVENT: - case CREATE_THREAD_DEBUG_EVENT: - case EXIT_PROCESS_DEBUG_EVENT: - case EXIT_THREAD_DEBUG_EVENT: - case LOAD_DLL_DEBUG_EVENT: - case RIP_EVENT: - case UNLOAD_DLL_DEBUG_EVENT: - // Continue on all other debug events - ContinueDebugEvent(deb_ev.dwProcessId, deb_ev.dwThreadId, DBG_CONTINUE); - break; - case EXCEPTION_DEBUG_EVENT: - EXCEPTION_RECORD& record = deb_ev.u.Exception.ExceptionRecord; - - // We want to generate a crash dump if we are seeing the same exception again. - if (!deb_ev.u.Exception.dwFirstChance) { - std::fprintf(stderr, "Creating MiniDump on ExceptionCode: 0x%08x %s\n", - record.ExceptionCode, ExceptionName(record.ExceptionCode)); - DumpFromDebugEvent(deb_ev, pi); - } - - // Continue without handling the exception. - // Lets the debuggee use its own exception handler. - // - If one does not exist, we will see the exception once more where we make a minidump - // for. Then when it reaches here again, yuzu will probably crash. - // - DBG_CONTINUE on an exception that the debuggee does not handle can set us up for an - // infinite loop of exceptions. - ContinueDebugEvent(deb_ev.dwProcessId, deb_ev.dwThreadId, DBG_EXCEPTION_NOT_HANDLED); - break; - } - } -} - -const char* ExceptionName(DWORD exception) { +static const char* ExceptionName(DWORD exception) { switch (exception) { case EXCEPTION_ACCESS_VIOLATION: return "EXCEPTION_ACCESS_VIOLATION"; @@ -193,6 +151,52 @@ const char* ExceptionName(DWORD exception) { case EXCEPTION_INVALID_HANDLE: return "EXCEPTION_INVALID_HANDLE"; default: - return nullptr; + return "unknown exception type"; } } + +void DebugDebuggee(PROCESS_INFORMATION& pi) { + DEBUG_EVENT deb_ev = {}; + + while (deb_ev.dwDebugEventCode != EXIT_PROCESS_DEBUG_EVENT) { + const bool wait_success = WaitForDebugEvent(&deb_ev, INFINITE); + if (!wait_success) { + fmt::print(stderr, "error: WaitForDebugEvent failed ({})", GetLastError()); + return; + } + + switch (deb_ev.dwDebugEventCode) { + case OUTPUT_DEBUG_STRING_EVENT: + case CREATE_PROCESS_DEBUG_EVENT: + case CREATE_THREAD_DEBUG_EVENT: + case EXIT_PROCESS_DEBUG_EVENT: + case EXIT_THREAD_DEBUG_EVENT: + case LOAD_DLL_DEBUG_EVENT: + case RIP_EVENT: + case UNLOAD_DLL_DEBUG_EVENT: + // Continue on all other debug events + ContinueDebugEvent(deb_ev.dwProcessId, deb_ev.dwThreadId, DBG_CONTINUE); + break; + case EXCEPTION_DEBUG_EVENT: + EXCEPTION_RECORD& record = deb_ev.u.Exception.ExceptionRecord; + + // We want to generate a crash dump if we are seeing the same exception again. + if (!deb_ev.u.Exception.dwFirstChance) { + fmt::print(stderr, "Creating MiniDump on ExceptionCode: 0x{:08x} {}\n", + record.ExceptionCode, ExceptionName(record.ExceptionCode)); + DumpFromDebugEvent(deb_ev, pi); + } + + // Continue without handling the exception. + // Lets the debuggee use its own exception handler. + // - If one does not exist, we will see the exception once more where we make a minidump + // for. Then when it reaches here again, yuzu will probably crash. + // - DBG_CONTINUE on an exception that the debuggee does not handle can set us up for an + // infinite loop of exceptions. + ContinueDebugEvent(deb_ev.dwProcessId, deb_ev.dwThreadId, DBG_EXCEPTION_NOT_HANDLED); + break; + } + } +} + +} // namespace MiniDump diff --git a/src/yuzu/mini_dump.h b/src/yuzu/mini_dump.h index 2052e5248..d6b6cca84 100644 --- a/src/yuzu/mini_dump.h +++ b/src/yuzu/mini_dump.h @@ -7,10 +7,13 @@ #include +namespace MiniDump { + void CreateMiniDump(HANDLE process_handle, DWORD process_id, MINIDUMP_EXCEPTION_INFORMATION* info, EXCEPTION_POINTERS* pep); void DumpFromDebugEvent(DEBUG_EVENT& deb_ev, PROCESS_INFORMATION& pi); bool SpawnDebuggee(const char* arg0, PROCESS_INFORMATION& pi); void DebugDebuggee(PROCESS_INFORMATION& pi); -const char* ExceptionName(DWORD exception); + +} // namespace MiniDump -- cgit v1.2.3