From 5411269fb29dbe4e2d550ffc0668269eed3f6bc4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tam=C3=A1s=20B=C3=A1lint=20Misius?= Date: Tue, 2 Jan 2024 08:53:21 +0100 Subject: [PATCH] Fix bluescreen stack trace not showing exception frames --- src/PowderToy.cpp | 193 +++++++++++--------- src/common/platform/Platform.h | 7 +- src/common/platform/stacktrace/Execinfo.cpp | 2 +- src/common/platform/stacktrace/Null.cpp | 2 +- src/common/platform/stacktrace/Windows.cpp | 39 +--- src/common/platform/stacktrace/meson.build | 4 +- 6 files changed, 115 insertions(+), 132 deletions(-) diff --git a/src/PowderToy.cpp b/src/PowderToy.cpp index cd246fb3b..d138571ba 100644 --- a/src/PowderToy.cpp +++ b/src/PowderToy.cpp @@ -29,6 +29,8 @@ #include #include #include +#include +#include void LoadWindowPosition() { @@ -97,7 +99,7 @@ void TickClient() Client::Ref().Tick(); } -void BlueScreen(String detailMessage, std::optional> stackTrace) +static void BlueScreen(String detailMessage, std::optional> stackTrace) { auto &engine = ui::Engine::Ref(); engine.g->BlendFilledRect(engine.g->Size().OriginRect(), 0x1172A9_rgb .WithAlpha(0xD2)); @@ -140,16 +142,29 @@ void BlueScreen(String detailMessage, std::optional> stackTr //Death loop SDL_Event event; - while(true) + auto running = true; + while (running) { while (SDL_PollEvent(&event)) - if(event.type == SDL_QUIT) - exit(-1); // Don't use Platform::Exit, we're practically zombies at this point anyway. + { + if (event.type == SDL_QUIT) + { + running = false; + } + } blit(engine.g->Data()); } + + // Don't use Platform::Exit, we're practically zombies at this point anyway. +#if defined(__MINGW32__) || defined(__APPLE__) + // Come on... + exit(-1); +#else + quick_exit(-1); +#endif } -struct +static struct { int sig; const char *message; @@ -161,7 +176,7 @@ struct { 0, nullptr }, }; -void SigHandler(int signal) +static void SigHandler(int signal) { const char *message = "Unknown signal"; for (auto *msg = signalMessages; msg->message; ++msg) @@ -172,7 +187,29 @@ void SigHandler(int signal) break; } } - BlueScreen(ByteString(message).FromUtf8(), Platform::StackTrace(Platform::stackTraceFromHere)); + BlueScreen(ByteString(message).FromUtf8(), Platform::StackTrace()); +} + +static void TerminateHandler() +{ + ByteString err = "std::terminate called without a current exception"; + auto eptr = std::current_exception(); + try + { + if (eptr) + { + std::rethrow_exception(eptr); + } + } + catch (const std::exception &e) + { + err = "unhandled exception: " + ByteString(e.what()); + } + catch (...) + { + err = "unhandled exception not derived from std::exception, cannot determine reason"; + } + BlueScreen(err.FromUtf8(), Platform::StackTrace()); } constexpr int SCALE_MAXIMUM = 10; @@ -431,6 +468,7 @@ int Main(int argc, char *argv[]) { signal(msg->sig, SigHandler); } + std::set_terminate(TerminateHandler); } if constexpr (X86) @@ -438,114 +476,97 @@ int Main(int argc, char *argv[]) X86KillDenormals(); } - auto wrapWithBluescreen = [&]() { - explicitSingletons->simulationData = std::make_unique(); - explicitSingletons->gameController = std::make_unique(); - auto *gameController = explicitSingletons->gameController.get(); - engine.ShowWindow(gameController->GetView()); + explicitSingletons->simulationData = std::make_unique(); + explicitSingletons->gameController = std::make_unique(); + auto *gameController = explicitSingletons->gameController.get(); + engine.ShowWindow(gameController->GetView()); - auto openArg = arguments["open"]; - if (openArg.has_value()) + auto openArg = arguments["open"]; + if (openArg.has_value()) + { + if constexpr (DEBUG) { - if constexpr (DEBUG) - { - std::cout << "Loading " << openArg.value() << std::endl; - } - if (Platform::FileExists(openArg.value())) - { - try - { - std::vector gameSaveData; - if (!Platform::ReadFile(gameSaveData, openArg.value())) - { - new ErrorMessage("Error", "Could not read file"); - } - else - { - auto newFile = std::make_unique(openArg.value()); - auto newSave = std::make_unique(std::move(gameSaveData)); - newFile->SetGameSave(std::move(newSave)); - gameController->LoadSaveFile(std::move(newFile)); - } - - } - catch (std::exception & e) - { - new ErrorMessage("Error", "Could not open save file:\n" + ByteString(e.what()).FromUtf8()) ; - } - } - else - { - new ErrorMessage("Error", "Could not open file"); - } + std::cout << "Loading " << openArg.value() << std::endl; } - - auto ptsaveArg = arguments["ptsave"]; - if (ptsaveArg.has_value()) + if (Platform::FileExists(openArg.value())) { - engine.g->Clear(); - engine.g->DrawRect(RectSized(engine.g->Size() / 2 - Vec2(100, 25), Vec2(200, 50)), 0xB4B4B4_rgb); - String loadingText = "Loading save..."; - engine.g->BlendText(engine.g->Size() / 2 - Vec2((Graphics::TextSize(loadingText).X - 1) / 2, 5), loadingText, style::Colour::InformationTitle); - - blit(engine.g->Data()); try { - ByteString saveIdPart; - if (ByteString::Split split = ptsaveArg.value().SplitBy(':')) + std::vector gameSaveData; + if (!Platform::ReadFile(gameSaveData, openArg.value())) { - if (split.Before() != "ptsave") - throw std::runtime_error("Not a ptsave link"); - saveIdPart = split.After().SplitBy('#').Before(); + new ErrorMessage("Error", "Could not read file"); } else - throw std::runtime_error("Invalid save link"); + { + auto newFile = std::make_unique(openArg.value()); + auto newSave = std::make_unique(std::move(gameSaveData)); + newFile->SetGameSave(std::move(newSave)); + gameController->LoadSaveFile(std::move(newFile)); + } - if (!saveIdPart.size()) - throw std::runtime_error("No Save ID"); - if constexpr (DEBUG) - { - std::cout << "Got Ptsave: id: " << saveIdPart << std::endl; - } - ByteString saveHistoryPart = "0"; - if (auto split = saveIdPart.SplitBy('@')) - { - saveHistoryPart = split.After(); - saveIdPart = split.Before(); - } - int saveId = saveIdPart.ToNumber(); - int saveHistory = saveHistoryPart.ToNumber(); - gameController->OpenSavePreview(saveId, saveHistory, savePreviewUrl); } catch (std::exception & e) { - new ErrorMessage("Error", ByteString(e.what()).FromUtf8()); - Platform::MarkPresentable(); + new ErrorMessage("Error", "Could not open save file:\n" + ByteString(e.what()).FromUtf8()) ; } } else { - Platform::MarkPresentable(); + new ErrorMessage("Error", "Could not open file"); } + } - MainLoop(); - }; - - if (enableBluescreen) + auto ptsaveArg = arguments["ptsave"]; + if (ptsaveArg.has_value()) { + engine.g->Clear(); + engine.g->DrawRect(RectSized(engine.g->Size() / 2 - Vec2(100, 25), Vec2(200, 50)), 0xB4B4B4_rgb); + String loadingText = "Loading save..."; + engine.g->BlendText(engine.g->Size() / 2 - Vec2((Graphics::TextSize(loadingText).X - 1) / 2, 5), loadingText, style::Colour::InformationTitle); + + blit(engine.g->Data()); try { - wrapWithBluescreen(); + ByteString saveIdPart; + if (ByteString::Split split = ptsaveArg.value().SplitBy(':')) + { + if (split.Before() != "ptsave") + throw std::runtime_error("Not a ptsave link"); + saveIdPart = split.After().SplitBy('#').Before(); + } + else + throw std::runtime_error("Invalid save link"); + + if (!saveIdPart.size()) + throw std::runtime_error("No Save ID"); + if constexpr (DEBUG) + { + std::cout << "Got Ptsave: id: " << saveIdPart << std::endl; + } + ByteString saveHistoryPart = "0"; + if (auto split = saveIdPart.SplitBy('@')) + { + saveHistoryPart = split.After(); + saveIdPart = split.Before(); + } + int saveId = saveIdPart.ToNumber(); + int saveHistory = saveHistoryPart.ToNumber(); + gameController->OpenSavePreview(saveId, saveHistory, savePreviewUrl); } - catch (const std::exception &e) + catch (std::exception & e) { - BlueScreen(ByteString(e.what()).FromUtf8(), Platform::StackTrace(Platform::stackTraceFromException)); + new ErrorMessage("Error", ByteString(e.what()).FromUtf8()); + Platform::MarkPresentable(); } } else { - wrapWithBluescreen(); + Platform::MarkPresentable(); } + + MainLoop(); + Platform::Exit(0); return 0; } diff --git a/src/common/platform/Platform.h b/src/common/platform/Platform.h index 2b0d2e773..610c3b442 100644 --- a/src/common/platform/Platform.h +++ b/src/common/platform/Platform.h @@ -69,12 +69,7 @@ namespace Platform int InvokeMain(int argc, char *argv[]); - enum StackTraceType - { - stackTraceFromHere, - stackTraceFromException, - }; - std::optional> StackTrace(StackTraceType StackTraceType); + std::optional> StackTrace(); void MarkPresentable(); } diff --git a/src/common/platform/stacktrace/Execinfo.cpp b/src/common/platform/stacktrace/Execinfo.cpp index eb42a0de3..041b54fa6 100644 --- a/src/common/platform/stacktrace/Execinfo.cpp +++ b/src/common/platform/stacktrace/Execinfo.cpp @@ -6,7 +6,7 @@ namespace Platform { -std::optional> StackTrace(StackTraceType) +std::optional> StackTrace() { std::array buf; auto used = backtrace(&buf[0], buf.size()); diff --git a/src/common/platform/stacktrace/Null.cpp b/src/common/platform/stacktrace/Null.cpp index 35dfff12b..0cc46fb54 100644 --- a/src/common/platform/stacktrace/Null.cpp +++ b/src/common/platform/stacktrace/Null.cpp @@ -2,7 +2,7 @@ namespace Platform { -std::optional> StackTrace(StackTraceType) +std::optional> StackTrace() { return std::nullopt; } diff --git a/src/common/platform/stacktrace/Windows.cpp b/src/common/platform/stacktrace/Windows.cpp index 224ab0ac6..f23ab803e 100644 --- a/src/common/platform/stacktrace/Windows.cpp +++ b/src/common/platform/stacktrace/Windows.cpp @@ -8,23 +8,9 @@ #include #include -#if defined(_MSC_VER) && _MSC_VER >= 1900 -extern "C" void **__cdecl __current_exception_context(); -#endif - -static CONTEXT *CurrentExceptionContext() -{ - // TODO: find the corresponding hack for mingw and older msvc; stack traces printed for exceptions are broken without it - CONTEXT **pctx = nullptr; -#if defined(_MSC_VER) && _MSC_VER >= 1900 - pctx = (CONTEXT **)__current_exception_context(); -#endif - return pctx ? *pctx : nullptr; -} - namespace Platform { -std::optional> StackTrace(StackTraceType stackTraceType) +std::optional> StackTrace() { static std::mutex mx; std::unique_lock lk(mx); @@ -38,27 +24,8 @@ std::optional> StackTrace(StackTraceType stackTraceType) CONTEXT context; memset(&context, 0, sizeof(context)); - switch (stackTraceType) - { - case stackTraceFromException: - if (auto *ec = CurrentExceptionContext()) - { - if (ec->ContextFlags) - { - context = *ec; - } - } - if (!context.ContextFlags) - { - return std::nullopt; - } - break; - - default: - context.ContextFlags = CONTEXT_FULL; - RtlCaptureContext(&context); - break; - } + context.ContextFlags = CONTEXT_FULL; + RtlCaptureContext(&context); STACKFRAME64 frame; memset(&frame, 0, sizeof(frame)); diff --git a/src/common/platform/stacktrace/meson.build b/src/common/platform/stacktrace/meson.build index 0676a480d..5e7ccc1b1 100644 --- a/src/common/platform/stacktrace/meson.build +++ b/src/common/platform/stacktrace/meson.build @@ -7,8 +7,8 @@ if host_platform == 'windows' endif stacktrace_files = files('Windows.cpp') elif host_platform == 'darwin' - # stacktrace_files = files('Execinfo.cpp') # macos has this but it's useless >_> - stacktrace_files = files('Null.cpp') + # TODO: good impl; current one is only slightly better than nothing + stacktrace_files = files('Execinfo.cpp') elif host_platform == 'linux' # TODO: again, this is more like "posix" than "linux" stacktrace_files = files('Execinfo.cpp')