From d932a611d5c977ea608c9c9b975f22bcea376fe3 Mon Sep 17 00:00:00 2001 From: slipher Date: Mon, 31 Mar 2025 23:53:02 -0500 Subject: [PATCH 1/4] Fix fatal C++ exception handling for MSVC native exe It turns out that using SetUnhandledExceptionFilter clobbers the standard library's mechanism for calling std::terminate on unhandled exceptions. --- src/common/System.cpp | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/common/System.cpp b/src/common/System.cpp index bbafc9e90e..f41684e42d 100644 --- a/src/common/System.cpp +++ b/src/common/System.cpp @@ -312,17 +312,31 @@ static const char *WindowsExceptionString(DWORD code) return "Unknown exception"; } } + +#ifdef _MSC_VER +// The standard library implements calling std::terminate with an exception filter +static LPTOP_LEVEL_EXCEPTION_FILTER originalExceptionFilter; +#endif + ALIGN_STACK_FOR_MINGW static LONG WINAPI CrashHandler(PEXCEPTION_POINTERS ExceptionInfo) { // Reset handler so that any future errors cause a crash SetUnhandledExceptionFilter(nullptr); - // TODO: backtrace +#ifdef _MSC_VER + constexpr DWORD CPP_EXCEPTION = 0xe06d7363; + if (ExceptionInfo->ExceptionRecord->ExceptionCode == CPP_EXCEPTION && originalExceptionFilter) { + return originalExceptionFilter(ExceptionInfo); + } +#endif Sys::Error("Crashed with exception 0x%lx: %s", ExceptionInfo->ExceptionRecord->ExceptionCode, WindowsExceptionString(ExceptionInfo->ExceptionRecord->ExceptionCode)); } void SetupCrashHandler() { +#ifdef _MSC_VER + originalExceptionFilter = +#endif SetUnhandledExceptionFilter(CrashHandler); } #elif defined(__native_client__) From 9d5c3a47dc06629ec05b33110ef4a6c47569aef5 Mon Sep 17 00:00:00 2001 From: slipher Date: Tue, 1 Apr 2025 01:13:24 -0500 Subject: [PATCH 2/4] Produce NaCl crash dump upon C++ exception Now we can both produce a crash dump, and report the exception message with SendMsg (which sets the text in the UI error message). 6ebd8585 was supposed to do that, but I messed up the final version of the commit and called SendMsg too early, so the crash dump was not produced. --- src/shared/VMMain.cpp | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/src/shared/VMMain.cpp b/src/shared/VMMain.cpp index 5bed3f484a..df3e522ee3 100644 --- a/src/shared/VMMain.cpp +++ b/src/shared/VMMain.cpp @@ -73,7 +73,7 @@ static void CommonInit(Sys::OSHandle rootSocket) } } -static void LogFatalError(Str::StringRef message) +static void SendErrorMsg(Str::StringRef message) { // Only try sending an ErrorMsg once static std::atomic_flag errorEntered; @@ -89,6 +89,13 @@ static void LogFatalError(Str::StringRef message) } } +#ifdef __native_client__ +// HACK: when we get a fatal exception in the terminate handler and call abort() to trigger +// a crash dump (as there doesn't seem to be an API for requesting a minidump directly), +// the error message is passed through this variable. +static char realErrorMessage[256]; +#endif + void Sys::Error(Str::StringRef message) { if (!OnMainThread()) { @@ -101,7 +108,13 @@ void Sys::Error(Str::StringRef message) std::abort(); } - LogFatalError(message); +#ifdef __native_client__ + if (realErrorMessage[0]) { + message = realErrorMessage; + } +#endif + + SendErrorMsg(message); #ifdef BUILD_VM_IN_PROCESS // Then engine will close the root socket when it wants us to exit, which @@ -143,13 +156,22 @@ extern "C" DLLEXPORT ALIGN_STACK_FOR_MINGW void vmMain(Sys::OSHandle rootSocket) // traditional Unix core dump (for native exe), a debugger, etc. NORETURN static void TerminateHandler() { +#ifdef __native_client__ + // Using a lambda triggers -Wformat-security... +# define DispatchError(...) snprintf(realErrorMessage, sizeof(realErrorMessage), __VA_ARGS__) +#else + auto DispatchError = [](const char* msg, const auto&... fmtArgs) { + Sys::Error(msg, fmtArgs...); + }; +#endif + if (Sys::OnMainThread()) { try { throw; // A terminate handler is only called if there is an active exception } catch (std::exception& err) { - LogFatalError(Str::Format("Unhandled exception (%s): %s", typeid(err).name(), err.what())); + DispatchError("Unhandled exception (%s): %s", typeid(err).name(), err.what()); } catch (...) { - LogFatalError("Unhandled exception of unknown type"); + DispatchError("Unhandled exception of unknown type"); } } std::abort(); From 229bf73c9f4f53ddacfede3fd06615ff87df0a62 Mon Sep 17 00:00:00 2001 From: slipher Date: Tue, 1 Apr 2025 19:38:40 -0500 Subject: [PATCH 3/4] Add cvar to disable native exe crash handling So server owners who want to get Unix core dumps for native exe gamelogic could set sgame.nativeExeCrashHandler to 0. Doing this stops the crash handler from being installed. In case of a fatal C++ exception we still install the terminate handler so we can log the exception message, but afterward, if the cvar is off, we call abort() instead of shutting down with the ErrorMsg IPC. --- cmake/DaemonGame.cmake | 3 +++ src/shared/CommonProxies.cpp | 11 ++++++++++- src/shared/VMMain.cpp | 19 +++++++++++++++++-- src/shared/VMMain.h | 4 ++++ 4 files changed, 34 insertions(+), 3 deletions(-) diff --git a/cmake/DaemonGame.cmake b/cmake/DaemonGame.cmake index 596b47361f..33b19b7385 100644 --- a/cmake/DaemonGame.cmake +++ b/cmake/DaemonGame.cmake @@ -119,6 +119,9 @@ function(buildGameModule module_slug) set_target_properties(${module_target} PROPERTIES PREFIX "") set(GAMEMODULE_DEFINITIONS "${GAMEMODULE_DEFINITIONS};BUILD_VM_IN_PROCESS") else() + if (module_slug STREQUAL "native-exe") + set(GAMEMODULE_DEFINITIONS "${GAMEMODULE_DEFINITIONS};BUILD_VM_NATIVE_EXE") + endif() add_executable("${module_target}" ${module_target_args}) endif() diff --git a/src/shared/CommonProxies.cpp b/src/shared/CommonProxies.cpp index 24b9b3fa58..26dba204f6 100644 --- a/src/shared/CommonProxies.cpp +++ b/src/shared/CommonProxies.cpp @@ -437,8 +437,17 @@ namespace VM { void InitializeProxies(int milliseconds) { baseTime = Sys::SteadyClock::now() - std::chrono::milliseconds(milliseconds); - Cmd::InitializeProxy(); Cvar::InitializeProxy(); + +#ifdef BUILD_VM_NATIVE_EXE + // Set up the crash handler now that we have cvars to know if we want to use it. + // The handler wouldn't have worked before StaticInitMsg anyway since it needs IPC to log anything. + if (useNativeExeCrashHandler.Get()) { + Sys::SetupCrashHandler(); + } +#endif + + Cmd::InitializeProxy(); } static void HandleMiscSyscall(int minor, Util::Reader& reader, IPC::Channel& channel) { diff --git a/src/shared/VMMain.cpp b/src/shared/VMMain.cpp index df3e522ee3..dfa3a58bff 100644 --- a/src/shared/VMMain.cpp +++ b/src/shared/VMMain.cpp @@ -37,6 +37,13 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. IPC::Channel VM::rootChannel; +#ifdef BUILD_VM_NATIVE_EXE +// When using native exe, turn this off if you want to use core dumps (*nix) or attach a debugger +// upon crashing (Windows). Disabling it is like -nocrashhandler for Daemon. +Cvar::Cvar VM::useNativeExeCrashHandler( + VM_STRING_PREFIX "nativeExeCrashHandler", "catch and log crashes/fatal exceptions in native exe VM", Cvar::NONE, true); +#endif + #ifdef BUILD_VM_IN_PROCESS // Special exception type used to cleanly exit a thread for in-process VMs // Using an anonymous namespace so the compiler knows that the exception is @@ -159,9 +166,15 @@ NORETURN static void TerminateHandler() #ifdef __native_client__ // Using a lambda triggers -Wformat-security... # define DispatchError(...) snprintf(realErrorMessage, sizeof(realErrorMessage), __VA_ARGS__) -#else +#elif defined(BUILD_VM_NATIVE_EXE) auto DispatchError = [](const char* msg, const auto&... fmtArgs) { - Sys::Error(msg, fmtArgs...); + if (VM::useNativeExeCrashHandler.Get()) { + Sys::Error(msg, fmtArgs...); + } else { + Log::Warn(XSTRING(VM_NAME) " VM terminating:"); + Log::Warn(msg, fmtArgs...); + // fall through to abort() for core dump etc. + } }; #endif @@ -199,7 +212,9 @@ int main(int argc, char** argv) std::set_terminate(TerminateHandler); // Set up crash handling for this process. This will allow crashes to be // sent back to the engine and reported to the user. +#ifdef __native_client__ Sys::SetupCrashHandler(); +#endif try { CommonInit(rootSocket); diff --git a/src/shared/VMMain.h b/src/shared/VMMain.h index 85c24d159a..3b0da31ff6 100644 --- a/src/shared/VMMain.h +++ b/src/shared/VMMain.h @@ -38,6 +38,10 @@ namespace VM { // Root channel used to communicate with the engine extern IPC::Channel rootChannel; +#ifdef BUILD_VM_NATIVE_EXE + extern Cvar::Cvar useNativeExeCrashHandler; +#endif + // Functions each specific gamelogic should implement void VMInit(); void VMHandleSyscall(uint32_t id, Util::Reader reader); From 28d89c7146c882eb14609fa6c84f9554222a84d0 Mon Sep 17 00:00:00 2001 From: slipher Date: Tue, 1 Apr 2025 21:18:40 -0500 Subject: [PATCH 4/4] Don't abort for non-main thread NaCl crash --- src/common/System.cpp | 2 ++ src/shared/VMMain.cpp | 13 ++++++++----- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/common/System.cpp b/src/common/System.cpp index f41684e42d..4da9e244cc 100644 --- a/src/common/System.cpp +++ b/src/common/System.cpp @@ -342,6 +342,8 @@ void SetupCrashHandler() #elif defined(__native_client__) static void CrashHandler(const void* data, size_t n) { + // Note: this only works on the main thread. Otherwise we hit + // Sys::Error("SendMsg from non-main VM thread"); VM::CrashDump(static_cast(data), n); Sys::Error("Crashed with NaCl exception"); } diff --git a/src/shared/VMMain.cpp b/src/shared/VMMain.cpp index dfa3a58bff..0ca7bdb148 100644 --- a/src/shared/VMMain.cpp +++ b/src/shared/VMMain.cpp @@ -106,13 +106,16 @@ static char realErrorMessage[256]; void Sys::Error(Str::StringRef message) { if (!OnMainThread()) { - // On a non-main thread we can't rely on IPC, so we may not be able to communicate the - // error message. So try to trigger a crash dump instead (exiting with abort() triggers - // one but exiting with _exit() doesn't). This will give something to work with when - // debugging. (For the main thread case a message is usually enough to diagnose the problem - // so we don't generate a crash dump; those consume disk space after all.) + // On a non-main thread we can't use IPC, so we can't communicate the error message. Just exit. // Also note that throwing ExitException would only work as intended on the main thread. +#ifdef __native_client__ + // Don't abort, to avoid "nacl_loader.exe has stopped working" popup on Windows + _exit(222); +#else + // Trigger a core dump if enabled. Would give us something to work with since the + // error message can't be shown. std::abort(); +#endif } #ifdef __native_client__