Common: Tidy up signal handlers

cmake/SearchForStuff.cmake

@@ -69,7 +69,7 @@ else()
 			find_package(Wayland REQUIRED Egl)
 		endif()
 
-		find_package(Libbacktrace)
+		find_package(Libbacktrace REQUIRED)
 		find_package(PkgConfig REQUIRED)
 		pkg_check_modules(DBUS REQUIRED dbus-1)
 	endif()

common/CMakeLists.txt

@@ -44,15 +44,6 @@ target_sources(common PRIVATE
 	emitter/movs.cpp
 	emitter/simd.cpp
 	emitter/x86emitter.cpp
-	Darwin/DarwinThreads.cpp
-	Darwin/DarwinMisc.cpp
-	Darwin/DarwinSemaphore.cpp
-	Linux/LnxHostSys.cpp
-	Linux/LnxThreads.cpp
-	Linux/LnxMisc.cpp
-	Windows/WinThreads.cpp
-	Windows/WinHostSys.cpp
-	Windows/WinMisc.cpp
 )
 
 # x86emitter headers
@@ -124,50 +115,60 @@ target_sources(common PRIVATE
 	emitter/legacy_types.h
 	emitter/x86emitter.h
 	emitter/x86types.h
-	Darwin/DarwinMisc.h
 )
 
-set_source_files_properties(PrecompiledHeader.cpp PROPERTIES HEADER_FILE_ONLY TRUE)
-
-if(USE_VTUNE)
-	target_link_libraries(common PUBLIC Vtune::Vtune)
-endif()
-
 if(WIN32)
 	enable_language(ASM_MASM)
-	target_sources(common PRIVATE FastJmp.asm)
-	target_link_libraries(common PUBLIC WIL::WIL winmm pathcch)
 	target_sources(common PRIVATE
-		CrashHandler.cpp
-		CrashHandler.h
 		FastJmp.asm
 		HTTPDownloaderWinHTTP.cpp
 		HTTPDownloaderWinHTTP.h
 		StackWalker.cpp
 		StackWalker.h
+		Windows/WinThreads.cpp
+		Windows/WinHostSys.cpp
+		Windows/WinMisc.cpp
 	)
-endif()
-
-if(APPLE)
+	target_link_libraries(common PUBLIC
+		WIL::WIL
+		winmm
+		pathcch
+	)
+elseif(APPLE)
 	target_sources(common PRIVATE
 		CocoaTools.mm
 		CocoaTools.h
+		Darwin/DarwinThreads.cpp
+		Darwin/DarwinMisc.cpp
+		Darwin/DarwinMisc.h
 	)
 	target_compile_options(common PRIVATE -fobjc-arc)
 	target_link_options(common PRIVATE -fobjc-link-runtime)
 	target_link_libraries(common PRIVATE
 		"-framework Foundation"
 		"-framework IOKit"
 	)
+else()
+	target_sources(common PRIVATE
+		Linux/LnxHostSys.cpp
+		Linux/LnxThreads.cpp
+		Linux/LnxMisc.cpp
+	)
+	target_include_directories(common PRIVATE
+		${DBUS_INCLUDE_DIRS}
+	)
+	target_link_libraries(common PRIVATE
+		${DBUS_LINK_LIBRARIES}
+		libbacktrace::libbacktrace
+		X11::X11
+		X11::Xrandr
+	)
 endif()
 
-if(UNIX AND NOT APPLE)
-	target_include_directories(common PRIVATE ${DBUS_INCLUDE_DIRS})
-	target_link_libraries(common PRIVATE ${DBUS_LINK_LIBRARIES} X11::X11 X11::Xrandr)
-  if(TARGET libbacktrace::libbacktrace)
-		target_compile_definitions(common PRIVATE "HAS_LIBBACKTRACE=1")
-		target_link_libraries(common PRIVATE libbacktrace::libbacktrace)
-  endif()
+set_source_files_properties(PrecompiledHeader.cpp PROPERTIES HEADER_FILE_ONLY TRUE)
+
+if(USE_VTUNE)
+	target_link_libraries(common PUBLIC Vtune::Vtune)
 endif()
 
 if (USE_GCC AND CMAKE_INTERPROCEDURAL_OPTIMIZATION)
@@ -181,7 +182,9 @@ if(NOT WIN32)
 		HTTPDownloaderCurl.cpp
 		HTTPDownloaderCurl.h
 	)
-	target_link_libraries(common PRIVATE CURL::libcurl)
+	target_link_libraries(common PRIVATE
+		CURL::libcurl
+	)
 endif()
 
 target_link_libraries(common PRIVATE

common/CrashHandler.cpp

@@ -56,23 +56,31 @@ static bool WriteMinidump(HMODULE hDbgHelp, HANDLE hFile, HANDLE hProcess, DWORD
 			PMINIDUMP_EXCEPTION_INFORMATION ExceptionParam, PMINIDUMP_USER_STREAM_INFORMATION UserStreamParam,
 			PMINIDUMP_CALLBACK_INFORMATION CallbackParam);
 
-	PFNMINIDUMPWRITEDUMP minidump_write_dump = hDbgHelp ?
-												   reinterpret_cast<PFNMINIDUMPWRITEDUMP>(GetProcAddress(hDbgHelp, "MiniDumpWriteDump")) :
-                                                   nullptr;
+	PFNMINIDUMPWRITEDUMP minidump_write_dump =
+		hDbgHelp ? reinterpret_cast<PFNMINIDUMPWRITEDUMP>(GetProcAddress(hDbgHelp, "MiniDumpWriteDump")) : nullptr;
 	if (!minidump_write_dump)
 		return false;
 
-	MINIDUMP_EXCEPTION_INFORMATION mei;
-	PMINIDUMP_EXCEPTION_INFORMATION mei_ptr = nullptr;
+	MINIDUMP_EXCEPTION_INFORMATION mei = {};
 	if (exception)
 	{
 		mei.ThreadId = thread_id;
 		mei.ExceptionPointers = exception;
 		mei.ClientPointers = FALSE;
-		mei_ptr = &mei;
+		return minidump_write_dump(hProcess, process_id, hFile, type, &mei, nullptr, nullptr);
 	}
 
-	return minidump_write_dump(hProcess, process_id, hFile, type, mei_ptr, nullptr, nullptr);
+	__try
+	{
+		RaiseException(EXCEPTION_INVALID_HANDLE, 0, 0, nullptr);
+	}
+	__except (WriteMinidump(hDbgHelp, hFile, GetCurrentProcess(), GetCurrentProcessId(), GetCurrentThreadId(),
+				  GetExceptionInformation(), type),
+		EXCEPTION_EXECUTE_HANDLER)
+	{
+	}
+
+	return true;
 }
 
 static std::wstring s_write_directory;
@@ -168,7 +176,7 @@ void CrashHandler::WriteDumpForCaller()
 	WriteMinidumpAndCallstack(nullptr);
 }
 
-#elif defined(HAS_LIBBACKTRACE)
+#elif !defined(__APPLE__)
 
 #include "FileSystem.h"
 
@@ -194,16 +202,13 @@ namespace CrashHandler
 	static void FreeBuffer(BacktraceBuffer* buf);
 	static void AppendToBuffer(BacktraceBuffer* buf, const char* format, ...);
 	static int BacktraceFullCallback(void* data, uintptr_t pc, const char* filename, int lineno, const char* function);
-	static void CallExistingSignalHandler(int signal, siginfo_t* siginfo, void* ctx);
-	static void CrashSignalHandler(int signal, siginfo_t* siginfo, void* ctx);
+	static void LogCallstack(int signal, const void* exception_pc);
 
 	static std::recursive_mutex s_crash_mutex;
 	static bool s_in_signal_handler = false;
 
 	static backtrace_state* s_backtrace_state = nullptr;
-	static struct sigaction s_old_sigbus_action;
-	static struct sigaction s_old_sigsegv_action;
-}
+} // namespace CrashHandler
 
 const char* CrashHandler::GetSignalName(int signal_no)
 {
@@ -214,7 +219,7 @@ const char* CrashHandler::GetSignalName(int signal_no)
 		case SIGSEGV: return "SIGSEGV";
 		case SIGBUS: return "SIGBUS";
 		default: return "UNKNOWN";
-		// clang-format on
+			// clang-format on
 	}
 }
 
@@ -253,42 +258,45 @@ void CrashHandler::AppendToBuffer(BacktraceBuffer* buf, const char* format, ...)
 	va_end(ap);
 }
 
-int CrashHandler::BacktraceFullCallback(void* data, uintptr_t pc, const char* filename, int lineno, const char* function)
+int CrashHandler::BacktraceFullCallback(void* data, uintptr_t pc, const char* filename, int lineno,
+	const char* function)
 {
 	BacktraceBuffer* buf = static_cast<BacktraceBuffer*>(data);
 	AppendToBuffer(buf, "  %016p", pc);
 	if (function)
 		AppendToBuffer(buf, " %s", function);
 	if (filename)
 		AppendToBuffer(buf, " [%s:%d]", filename, lineno);
-	
+
 	AppendToBuffer(buf, "\n");
 	return 0;
 }
 
-void CrashHandler::CallExistingSignalHandler(int signal, siginfo_t* siginfo, void* ctx)
+void CrashHandler::LogCallstack(int signal, const void* exception_pc)
 {
-	const struct sigaction& sa = (signal == SIGBUS) ? s_old_sigbus_action : s_old_sigsegv_action;
-	if (sa.sa_flags & SA_SIGINFO)
-	{
-		sa.sa_sigaction(signal, siginfo, ctx);
-	}
-	else if (sa.sa_handler == SIG_DFL)
-	{
-		// Re-raising the signal would just queue it, and since we'd restore the handler back to us,
-		// we'd end up right back here again. So just abort, because that's probably what it'd do anyway.
-		abort();
-	}
-	else if (sa.sa_handler != SIG_IGN)
-	{
-		sa.sa_handler(signal);
-	}
+	BacktraceBuffer buf;
+	AllocateBuffer(&buf);
+	if (signal != 0 || exception_pc)
+		AppendToBuffer(&buf, "*************** Unhandled %s at %p ***************\n", GetSignalName(signal), exception_pc);
+	else
+		AppendToBuffer(&buf, "*******************************************************************\n");
+
+	const int rc = backtrace_full(s_backtrace_state, 0, BacktraceFullCallback, nullptr, &buf);
+	if (rc != 0)
+		AppendToBuffer(&buf, "  backtrace_full() failed: %d\n");
+
+	AppendToBuffer(&buf, "*******************************************************************\n");
+
+	if (buf.used > 0)
+		write(STDERR_FILENO, buf.buffer, buf.used);
+
+	FreeBuffer(&buf);
 }
 
 void CrashHandler::CrashSignalHandler(int signal, siginfo_t* siginfo, void* ctx)
 {
 	std::unique_lock lock(s_crash_mutex);
-	
+
 	// If we crash somewhere in libbacktrace, don't bother trying again.
 	if (!s_in_signal_handler)
 	{
@@ -304,27 +312,17 @@ void CrashHandler::CrashSignalHandler(int signal, siginfo_t* siginfo, void* ctx)
 		void* const exception_pc = nullptr;
 #endif
 
-		BacktraceBuffer buf;
-		AllocateBuffer(&buf);
-		AppendToBuffer(&buf, "*************** Unhandled %s at %p ***************\n", GetSignalName(signal), exception_pc);
-
-		const int rc = backtrace_full(s_backtrace_state, 0, BacktraceFullCallback, nullptr, &buf);
-		if (rc != 0)
-			AppendToBuffer(&buf, "  backtrace_full() failed: %d\n");
-
-		AppendToBuffer(&buf, "*******************************************************************\n");
-
-		if (buf.used > 0)
-			write(STDERR_FILENO, buf.buffer, buf.used);
-
-		FreeBuffer(&buf);
+		LogCallstack(signal, exception_pc);
 
 		s_in_signal_handler = false;
 	}
 
-	// Chances are we're not going to have anything else to call, but just in case.
 	lock.unlock();
-	CallExistingSignalHandler(signal, siginfo, ctx);
+
+	// We can't continue from here. Just bail out and dump core.
+	std::fputs("Aborting application.\n", stderr);
+	std::fflush(stderr);
+	std::abort();
 }
 
 bool CrashHandler::Install()
@@ -333,15 +331,15 @@ bool CrashHandler::Install()
 	s_backtrace_state = backtrace_create_state(progpath.empty() ? nullptr : progpath.c_str(), 0, nullptr, nullptr);
 	if (!s_backtrace_state)
 		return false;
-	
+
 	struct sigaction sa;
 
 	sigemptyset(&sa.sa_mask);
 	sa.sa_flags = SA_SIGINFO | SA_NODEFER;
 	sa.sa_sigaction = CrashSignalHandler;
-	if (sigaction(SIGBUS, &sa, &s_old_sigbus_action) != 0)
+	if (sigaction(SIGBUS, &sa, nullptr) != 0)
 		return false;
-	if (sigaction(SIGSEGV, &sa, &s_old_sigsegv_action) != 0)
+	if (sigaction(SIGSEGV, &sa, nullptr) != 0)
 		return false;
 
 	return true;
@@ -353,6 +351,7 @@ void CrashHandler::SetWriteDirectory(std::string_view dump_directory)
 
 void CrashHandler::WriteDumpForCaller()
 {
+	LogCallstack(0, nullptr);
 }
 
 #else
@@ -370,4 +369,12 @@ void CrashHandler::WriteDumpForCaller()
 {
 }
 
+void CrashHandler::CrashSignalHandler(int signal, siginfo_t* siginfo, void* ctx)
+{
+	// We can't continue from here. Just bail out and dump core.
+	std::fputs("Aborting application.\n", stderr);
+	std::fflush(stderr);
+	std::abort();
+}
+
 #endif

common/CrashHandler.h

@@ -3,9 +3,18 @@
 
 #include <string_view>
 
+#ifndef _WIN32
+#include <csignal>
+#endif
+
 namespace CrashHandler
 {
 	bool Install();
 	void SetWriteDirectory(std::string_view dump_directory);
 	void WriteDumpForCaller();
+
+#ifndef _WIN32
+	// Allow crash handler to be invoked from a signal.
+	void CrashSignalHandler(int signal, siginfo_t* siginfo, void* ctx);
+#endif
 } // namespace CrashHandler