diff --git a/common/Assertions.h b/common/Assertions.h index a27387a1da..c31225ce64 100644 --- a/common/Assertions.h +++ b/common/Assertions.h @@ -27,35 +27,6 @@ #endif #endif -// -------------------------------------------------------------------------------------- -// DiagnosticOrigin -// -------------------------------------------------------------------------------------- -struct DiagnosticOrigin -{ - const char* srcfile; - const char* function; - const char* condition; - int line; - - DiagnosticOrigin(const char* _file, int _line, const char* _func, const char* _cond = nullptr) - : srcfile(_file) - , function(_func) - , condition(_cond) - , line(_line) - { - } - - std::string ToString(const char* msg = nullptr) const; -}; - -// Returns ture if the assertion is to trap into the debugger, or false if execution -// of the program should continue unimpeded. -typedef bool pxDoAssertFnType(const DiagnosticOrigin& origin, const char* msg); - -extern pxDoAssertFnType pxAssertImpl_LogIt; - -extern pxDoAssertFnType* pxDoAssert; - // ---------------------------------------------------------------------------------------- // pxAssert / pxAssertDev // ---------------------------------------------------------------------------------------- @@ -93,16 +64,15 @@ extern pxDoAssertFnType* pxDoAssert; // it can lead to the compiler optimizing out code and leading to crashes in dev/release // builds. To have code optimized, explicitly use pxAssume(false) or pxAssumeDev(false,msg); -#define pxDiagSpot DiagnosticOrigin(__FILE__, __LINE__, __pxFUNCTION__) -#define pxAssertSpot(cond) DiagnosticOrigin(__FILE__, __LINE__, __pxFUNCTION__, #cond) - // pxAssertRel -> // Special release-mode assertion. Limited use since stack traces in release mode builds // (especially with LTCG) are highly suspect. But when troubleshooting crashes that only // rear ugly heads in optimized builds, this is one of the few tools we have. -#define pxAssertRel(cond, msg) ((likely(cond)) || (pxOnAssert(pxAssertSpot(cond), msg), false)) -#define pxAssumeRel(cond, msg) ((void)((!likely(cond)) && (pxOnAssert(pxAssertSpot(cond), msg), false))) +extern void pxOnAssertFail(const char* file, int line, const char* func, const char* msg); + +#define pxAssertRel(cond, msg) ((likely(cond)) || (pxOnAssertFail(__FILE__, __LINE__, __pxFUNCTION__, msg), false)) +#define pxAssumeRel(cond, msg) ((void)((!likely(cond)) && (pxOnAssertFail(__FILE__, __LINE__, __pxFUNCTION__, msg), false))) #define pxFailRel(msg) pxAssertRel(false, msg) #if defined(PCSX2_DEBUG) @@ -168,8 +138,6 @@ extern pxDoAssertFnType* pxDoAssert; #define pxAssertRelease(cond, msg) -extern void pxOnAssert(const DiagnosticOrigin& origin, const char* msg); - // -------------------------------------------------------------------------------------- // jNO_DEFAULT -- disables the default case in a switch, which improves switch optimization // under MSVC. diff --git a/common/CrashHandler.cpp b/common/CrashHandler.cpp index 988831b009..70269efa8b 100644 --- a/common/CrashHandler.cpp +++ b/common/CrashHandler.cpp @@ -46,11 +46,7 @@ CrashHandlerStackWalker::CrashHandlerStackWalker(HANDLE out_file) { } -CrashHandlerStackWalker::~CrashHandlerStackWalker() -{ - if (m_out_file) - CloseHandle(m_out_file); -} +CrashHandlerStackWalker::~CrashHandlerStackWalker() = default; void CrashHandlerStackWalker::OnOutput(LPCSTR szText) { @@ -105,33 +101,8 @@ static void GenerateCrashFilename(wchar_t* buf, size_t len, const wchar_t* prefi st.wYear, st.wMonth, st.wDay, st.wHour, st.wMinute, st.wSecond, st.wMilliseconds, extension); } -static LONG NTAPI ExceptionHandler(PEXCEPTION_POINTERS exi) +static void WriteMinidumpAndCallstack(PEXCEPTION_POINTERS exi) { - if (s_in_crash_handler) - return EXCEPTION_CONTINUE_SEARCH; - - switch (exi->ExceptionRecord->ExceptionCode) - { - case EXCEPTION_ACCESS_VIOLATION: - case EXCEPTION_BREAKPOINT: - case EXCEPTION_ARRAY_BOUNDS_EXCEEDED: - case EXCEPTION_INT_DIVIDE_BY_ZERO: - case EXCEPTION_INT_OVERFLOW: - case EXCEPTION_PRIV_INSTRUCTION: - case EXCEPTION_ILLEGAL_INSTRUCTION: - case EXCEPTION_NONCONTINUABLE_EXCEPTION: - case EXCEPTION_STACK_OVERFLOW: - case EXCEPTION_GUARD_PAGE: - break; - - default: - return EXCEPTION_CONTINUE_SEARCH; - } - - // if the debugger is attached, let it take care of it. - if (IsDebuggerPresent()) - return EXCEPTION_CONTINUE_SEARCH; - s_in_crash_handler = true; wchar_t filename[1024] = {}; @@ -139,7 +110,7 @@ static LONG NTAPI ExceptionHandler(PEXCEPTION_POINTERS exi) // might fail HANDLE hFile = CreateFileW(filename, GENERIC_WRITE, 0, nullptr, CREATE_ALWAYS, 0, nullptr); - if (hFile != INVALID_HANDLE_VALUE) + if (exi && hFile != INVALID_HANDLE_VALUE) { char line[1024]; DWORD written; @@ -169,11 +140,40 @@ static LONG NTAPI ExceptionHandler(PEXCEPTION_POINTERS exi) CloseHandle(hMinidumpFile); CrashHandlerStackWalker sw(hFile); - sw.ShowCallstack(GetCurrentThread(), exi->ContextRecord); + sw.ShowCallstack(GetCurrentThread(), exi ? exi->ContextRecord : nullptr); if (hFile != INVALID_HANDLE_VALUE) CloseHandle(hFile); +} +static LONG NTAPI ExceptionHandler(PEXCEPTION_POINTERS exi) +{ + if (s_in_crash_handler) + return EXCEPTION_CONTINUE_SEARCH; + + switch (exi->ExceptionRecord->ExceptionCode) + { + case EXCEPTION_ACCESS_VIOLATION: + case EXCEPTION_BREAKPOINT: + case EXCEPTION_ARRAY_BOUNDS_EXCEEDED: + case EXCEPTION_INT_DIVIDE_BY_ZERO: + case EXCEPTION_INT_OVERFLOW: + case EXCEPTION_PRIV_INSTRUCTION: + case EXCEPTION_ILLEGAL_INSTRUCTION: + case EXCEPTION_NONCONTINUABLE_EXCEPTION: + case EXCEPTION_STACK_OVERFLOW: + case EXCEPTION_GUARD_PAGE: + break; + + default: + return EXCEPTION_CONTINUE_SEARCH; + } + + // if the debugger is attached, let it take care of it. + if (IsDebuggerPresent()) + return EXCEPTION_CONTINUE_SEARCH; + + WriteMinidumpAndCallstack(exi); return EXCEPTION_CONTINUE_SEARCH; } @@ -195,6 +195,11 @@ void CrashHandler::SetWriteDirectory(const std::string_view& dump_directory) s_write_directory = StringUtil::UTF8StringToWideString(dump_directory); } +void CrashHandler::WriteDumpForCaller() +{ + WriteMinidumpAndCallstack(nullptr); +} + void CrashHandler::Uninstall() { if (s_veh_handle) @@ -221,6 +226,10 @@ void CrashHandler::SetWriteDirectory(const std::string_view& dump_directory) { } +void CrashHandler::WriteDumpForCaller() +{ +} + void CrashHandler::Uninstall() { } diff --git a/common/CrashHandler.h b/common/CrashHandler.h index f783125f31..281c1b60e7 100644 --- a/common/CrashHandler.h +++ b/common/CrashHandler.h @@ -19,5 +19,6 @@ namespace CrashHandler { bool Install(); void SetWriteDirectory(const std::string_view& dump_directory); + void WriteDumpForCaller(); void Uninstall(); } // namespace CrashHandler diff --git a/common/Exceptions.cpp b/common/Exceptions.cpp index e1dcfa345c..96eccf6353 100644 --- a/common/Exceptions.cpp +++ b/common/Exceptions.cpp @@ -16,51 +16,21 @@ #include "Threading.h" #include "General.h" #include "Exceptions.h" +#include "CrashHandler.h" +#include #include "fmt/core.h" #ifdef _WIN32 #include "RedtapeWindows.h" +#include +#include #endif #ifdef __UNIX__ #include #endif -// ------------------------------------------------------------------------ -// Force DevAssert to *not* inline for devel builds (allows using breakpoints to trap assertions, -// and force it to inline for release builds (optimizes it out completely since IsDevBuild is a -// const false). -// -#ifdef PCSX2_DEVBUILD -#define DEVASSERT_INLINE __noinline -#else -#define DEVASSERT_INLINE __fi -#endif - -pxDoAssertFnType* pxDoAssert = pxAssertImpl_LogIt; - -// make life easier for people using VC++ IDE by using this format, which allows double-click -// response times from the Output window... -std::string DiagnosticOrigin::ToString(const char* msg) const -{ - std::string message; - - fmt::format_to(std::back_inserter(message), "{}({}) : assertion failed:\n", srcfile, line); - - if (function) - fmt::format_to(std::back_inserter(message), " Function: {}\n", function); - - if (condition) - fmt::format_to(std::back_inserter(message), " Condition: {}\n", condition); - - if (msg) - fmt::format_to(std::back_inserter(message), " Message: {}\n", msg); - - return message; -} - - // Because wxTrap isn't available on Linux builds of wxWidgets (non-Debug, typically) void pxTrap() { @@ -73,46 +43,105 @@ void pxTrap() #endif } +static std::mutex s_assertion_failed_mutex; -bool pxAssertImpl_LogIt(const DiagnosticOrigin& origin, const char* msg) +static inline void FreezeThreads(void** handle) { - //wxLogError( L"%s", origin.ToString( msg ).c_str() ); - std::string full_msg(origin.ToString(msg)); -#ifdef _WIN32 - OutputDebugStringA(full_msg.c_str()); - OutputDebugStringA("\n"); +#if defined(_WIN32) && !defined(_UWP) + HANDLE snapshot = CreateToolhelp32Snapshot(TH32CS_SNAPTHREAD, 0); + if (snapshot != INVALID_HANDLE_VALUE) + { + THREADENTRY32 threadEntry; + if (Thread32First(snapshot, &threadEntry)) + { + do + { + if (threadEntry.th32ThreadID == GetCurrentThreadId()) + continue; + + HANDLE hThread = OpenThread(THREAD_SUSPEND_RESUME, FALSE, threadEntry.th32ThreadID); + if (hThread != nullptr) + { + SuspendThread(hThread); + CloseHandle(hThread); + } + } while (Thread32Next(snapshot, &threadEntry)); + } + } + + *handle = static_cast(snapshot); +#else + * handle = nullptr; #endif - - std::fprintf(stderr, "%s\n", full_msg.c_str()); - - pxTrap(); - return false; } - -DEVASSERT_INLINE void pxOnAssert(const DiagnosticOrigin& origin, const char* msg) +static inline void ResumeThreads(void* handle) { - // wxWidgets doesn't come with debug builds on some Linux distros, and other distros make - // it difficult to use the debug build (compilation failures). To handle these I've had to - // bypass the internal wxWidgets assertion handler entirely, since it may not exist even if - // PCSX2 itself is compiled in debug mode (assertions enabled). - - bool trapit; - - if (pxDoAssert == NULL) +#if defined(_WIN32) && !defined(_UWP) + if (handle != INVALID_HANDLE_VALUE) { - // Note: Format uses MSVC's syntax for output window hotlinking. - trapit = pxAssertImpl_LogIt(origin, msg); - } - else - { - trapit = pxDoAssert(origin, msg); - } + THREADENTRY32 threadEntry; + if (Thread32First(reinterpret_cast(handle), &threadEntry)) + { + do + { + if (threadEntry.th32ThreadID == GetCurrentThreadId()) + continue; - if (trapit) - { - pxTrap(); + HANDLE hThread = OpenThread(THREAD_SUSPEND_RESUME, FALSE, threadEntry.th32ThreadID); + if (hThread != nullptr) + { + ResumeThread(hThread); + CloseHandle(hThread); + } + } while (Thread32Next(reinterpret_cast(handle), &threadEntry)); + } + CloseHandle(reinterpret_cast(handle)); } +#else +#endif +} + +void pxOnAssertFail(const char* file, int line, const char* func, const char* msg) +{ + std::unique_lock guard(s_assertion_failed_mutex); + + void* handle; + FreezeThreads(&handle); + + char full_msg[512]; + std::snprintf(full_msg, sizeof(full_msg), "%s:%d: assertion failed in function %s: %s\n", file, line, func, msg); + +#if defined(_WIN32) && !defined(_UWP) + HANDLE error_handle = GetStdHandle(STD_ERROR_HANDLE); + if (error_handle != INVALID_HANDLE_VALUE) + WriteConsoleA(GetStdHandle(STD_ERROR_HANDLE), full_msg, static_cast(std::strlen(full_msg)), NULL, NULL); + OutputDebugStringA(full_msg); + + std::snprintf( + full_msg, sizeof(full_msg), + "Assertion failed in function %s (%s:%d):\n\n%s\n\nPress Abort to exit, Retry to break to debugger, or Ignore to attempt to continue.", + func, file, line, msg); + + int result = MessageBoxA(NULL, full_msg, NULL, MB_ABORTRETRYIGNORE | MB_ICONERROR); + if (result == IDRETRY) + { + __debugbreak(); + } + else if (result != IDIGNORE) + { + // try to save a crash dump before exiting + CrashHandler::WriteDumpForCaller(); + TerminateProcess(GetCurrentProcess(), 0xBAADC0DE); + } +#else + fputs(full_msg, stderr); + fputs("\nAborting application.\n", stderr); + fflush(stderr); + abort(); +#endif + + ResumeThreads(handle); } // -------------------------------------------------------------------------------------- diff --git a/pcsx2/gui/App.h b/pcsx2/gui/App.h index 3f194d92ae..144efde07b 100644 --- a/pcsx2/gui/App.h +++ b/pcsx2/gui/App.h @@ -728,8 +728,6 @@ int AppOpenModalDialog(wxString panel_name, wxWindow* parent = NULL) return DialogType(parent).ShowModal(); } -extern pxDoAssertFnType AppDoAssert; - // -------------------------------------------------------------------------------------- // External App-related Globals and Shortcuts // -------------------------------------------------------------------------------------- diff --git a/pcsx2/gui/AppAssert.cpp b/pcsx2/gui/AppAssert.cpp index 017ef25d21..92d4c7c2e8 100644 --- a/pcsx2/gui/AppAssert.cpp +++ b/pcsx2/gui/AppAssert.cpp @@ -114,10 +114,7 @@ void Pcsx2App::OnAssertFailure( const wxChar *file, int line, const wxChar *func std::string nfunc(StringUtil::wxStringToUTF8String(func)); std::string ncond(StringUtil::wxStringToUTF8String(cond)); std::string nmsg(StringUtil::wxStringToUTF8String(msg)); - if( AppDoAssert( DiagnosticOrigin( nfile.c_str(), line, nfunc.c_str(), ncond.c_str()), nmsg.c_str())) - { - pxTrap(); - } + pxOnAssertFail(nfile.c_str(), line, nfunc.c_str(), nmsg.empty() ? ncond.c_str() : nmsg.c_str()); } #endif diff --git a/pcsx2/gui/AppInit.cpp b/pcsx2/gui/AppInit.cpp index bbf0d50e6c..913954ab8b 100644 --- a/pcsx2/gui/AppInit.cpp +++ b/pcsx2/gui/AppInit.cpp @@ -388,8 +388,6 @@ bool Pcsx2App::OnInit() InitCPUTicks(); - pxDoAssert = AppDoAssert; - g_Conf = std::make_unique(); wxInitAllImageHandlers(); @@ -623,7 +621,6 @@ void Pcsx2App::CleanupOnExit() // FIXME: performing a wxYield() here may fix that problem. -- air - pxDoAssert = pxAssertImpl_LogIt; Console_SetActiveHandler(ConsoleWriter_Stdout); } @@ -732,7 +729,6 @@ Pcsx2App::Pcsx2App() Pcsx2App::~Pcsx2App() { - pxDoAssert = pxAssertImpl_LogIt; } void Pcsx2App::CleanUp() diff --git a/pcsx2/gui/PersistentThread.cpp b/pcsx2/gui/PersistentThread.cpp index adf5c6d801..5c1e1b66fe 100644 --- a/pcsx2/gui/PersistentThread.cpp +++ b/pcsx2/gui/PersistentThread.cpp @@ -107,6 +107,26 @@ static void unmake_curthread_key() curthread_key = 0; } +// make life easier for people using VC++ IDE by using this format, which allows double-click +// response times from the Output window... +std::string DiagnosticOrigin::ToString(const char* msg) const +{ + std::string message; + + fmt::format_to(std::back_inserter(message), "{}({}) : assertion failed:\n", srcfile, line); + + if (function) + fmt::format_to(std::back_inserter(message), " Function: {}\n", function); + + if (condition) + fmt::format_to(std::back_inserter(message), " Condition: {}\n", condition); + + if (msg) + fmt::format_to(std::back_inserter(message), " Message: {}\n", msg); + + return message; +} + void Threading::pxTestCancel() { pthread_testcancel(); @@ -204,7 +224,7 @@ bool Threading::pxThread::AffinityAssert_AllowFromSelf(const DiagnosticOrigin& o return true; if (IsDevBuild) - pxOnAssert(origin, pxsFmt(L"Thread affinity violation: Call allowed from '%s' thread only.", WX_STR(GetName())).ToUTF8().data()); + pxOnAssertFail(origin.srcfile, origin.line, origin.function, pxsFmt(L"Thread affinity violation: Call allowed from '%s' thread only.", WX_STR(GetName())).ToUTF8().data()); return false; } @@ -215,7 +235,7 @@ bool Threading::pxThread::AffinityAssert_DisallowFromSelf(const DiagnosticOrigin return true; if (IsDevBuild) - pxOnAssert(origin, pxsFmt(L"Thread affinity violation: Call is *not* allowed from '%s' thread.", WX_STR(GetName())).ToUTF8().data()); + pxOnAssertFail(origin.srcfile, origin.line, origin.function, pxsFmt(L"Thread affinity violation: Call is *not* allowed from '%s' thread.", WX_STR(GetName())).ToUTF8().data()); return false; } diff --git a/pcsx2/gui/PersistentThread.h b/pcsx2/gui/PersistentThread.h index aae4ccee64..309db3dfb0 100644 --- a/pcsx2/gui/PersistentThread.h +++ b/pcsx2/gui/PersistentThread.h @@ -30,6 +30,30 @@ #include #endif +// -------------------------------------------------------------------------------------- +// DiagnosticOrigin +// -------------------------------------------------------------------------------------- +struct DiagnosticOrigin +{ + const char* srcfile; + const char* function; + const char* condition; + int line; + + DiagnosticOrigin(const char* _file, int _line, const char* _func, const char* _cond = nullptr) + : srcfile(_file) + , function(_func) + , condition(_cond) + , line(_line) + { + } + + std::string ToString(const char* msg = nullptr) const; +}; + +#define pxDiagSpot DiagnosticOrigin(__FILE__, __LINE__, __pxFUNCTION__) +#define pxAssertSpot(cond) DiagnosticOrigin(__FILE__, __LINE__, __pxFUNCTION__, #cond) + namespace Threading { class pxThread; diff --git a/tests/ctest/x86emitter/codegen_tests.cpp b/tests/ctest/x86emitter/codegen_tests.cpp index f110c02838..fc22ae88ba 100644 --- a/tests/ctest/x86emitter/codegen_tests.cpp +++ b/tests/ctest/x86emitter/codegen_tests.cpp @@ -22,20 +22,7 @@ using namespace x86Emitter; thread_local const char *currentTest; -static void assertHandlerInternal(const DiagnosticOrigin& origin, const char* msg) { - FAIL() << "Assertion failed: " << msg - << "\n at " << origin.srcfile << ":" << origin.line << "" - << "\n when trying to assemble " << currentTest; -} - -static bool assertHandler(const DiagnosticOrigin& origin, const char* msg) { - assertHandlerInternal(origin, msg); - return false; -} - void runCodegenTest(void (*exec)(void *base), const char* description, const char* expected) { - pxDoAssert = assertHandler; - u8 code[4096]; memset(code, 0xcc, sizeof(code)); char str[4096] = {0};