Common: Redo assertions, purge DiagnosticOrigin

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))
+extern void pxOnAssertFail(const char* file, int line, const char* func, const char* msg);
-#define pxAssumeRel(cond, msg) ((void)((!likely(cond)) && (pxOnAssert(pxAssertSpot(cond), msg), false)))
+
+#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.

common/CrashHandler.cpp

@@ -46,11 +46,7 @@ CrashHandlerStackWalker::CrashHandlerStackWalker(HANDLE out_file)
 {
 }
 
-CrashHandlerStackWalker::~CrashHandlerStackWalker()
+CrashHandlerStackWalker::~CrashHandlerStackWalker() = default;
-{
-	if (m_out_file)
-		CloseHandle(m_out_file);
-}
 
 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()
 {
 }

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

common/Exceptions.cpp

@@ -16,51 +16,21 @@
 #include "Threading.h"
 #include "General.h"
 #include "Exceptions.h"
+#include "CrashHandler.h"
 
+#include <mutex>
 #include "fmt/core.h"
 
 #ifdef _WIN32
 #include "RedtapeWindows.h"
+#include <intrin.h>
+#include <tlhelp32.h>
 #endif
 
 #ifdef __UNIX__
 #include <signal.h>
 #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() );
+#if defined(_WIN32) && !defined(_UWP)
-	std::string full_msg(origin.ToString(msg));
+	HANDLE snapshot = CreateToolhelp32Snapshot(TH32CS_SNAPTHREAD, 0);
-#ifdef _WIN32
+	if (snapshot != INVALID_HANDLE_VALUE)
-	OutputDebugStringA(full_msg.c_str());
+	{
-	OutputDebugStringA("\n");
+		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<void*>(snapshot);
+#else
+	* handle = nullptr;
 #endif
-
-	std::fprintf(stderr, "%s\n", full_msg.c_str());
-
-	pxTrap();
-	return false;
 }
 
-
+static inline void ResumeThreads(void* handle)
-DEVASSERT_INLINE void pxOnAssert(const DiagnosticOrigin& origin, const char* msg)
 {
-	// wxWidgets doesn't come with debug builds on some Linux distros, and other distros make
+#if defined(_WIN32) && !defined(_UWP)
-	// it difficult to use the debug build (compilation failures).  To handle these I've had to
+	if (handle != INVALID_HANDLE_VALUE)
-	// 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)
 	{
-		// Note: Format uses MSVC's syntax for output window hotlinking.
+		THREADENTRY32 threadEntry;
-		trapit = pxAssertImpl_LogIt(origin, msg);
+		if (Thread32First(reinterpret_cast<HANDLE>(handle), &threadEntry))
-	}
+		{
-	else
+			do
-	{
+			{
-		trapit = pxDoAssert(origin, msg);
+				if (threadEntry.th32ThreadID == GetCurrentThreadId())
-	}
+					continue;
 
-	if (trapit)
+				HANDLE hThread = OpenThread(THREAD_SUSPEND_RESUME, FALSE, threadEntry.th32ThreadID);
-	{
+				if (hThread != nullptr)
-		pxTrap();
+				{
+					ResumeThread(hThread);
+					CloseHandle(hThread);
+				}
+			} while (Thread32Next(reinterpret_cast<HANDLE>(handle), &threadEntry));
+		}
+		CloseHandle(reinterpret_cast<HANDLE>(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<DWORD>(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);
 }
 
 // --------------------------------------------------------------------------------------

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
 // --------------------------------------------------------------------------------------

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()))
+	pxOnAssertFail(nfile.c_str(), line, nfunc.c_str(), nmsg.empty() ? ncond.c_str() : nmsg.c_str());
-	{
-		pxTrap();
-	}
 }
 
 #endif

pcsx2/gui/AppInit.cpp

@@ -388,8 +388,6 @@ bool Pcsx2App::OnInit()
 
 	InitCPUTicks();
 
-	pxDoAssert = AppDoAssert;
-
 	g_Conf = std::make_unique<AppConfig>();
 	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()

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;
 }

pcsx2/gui/PersistentThread.h

@@ -30,6 +30,30 @@
 #include <semaphore.h>
 #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;

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};