From f120d2beb8cbbc34ac4d1e7312a656182cf3f006 Mon Sep 17 00:00:00 2001 From: Patric Stout Date: Sun, 20 Aug 2023 17:16:08 +0200 Subject: [PATCH] Add: use breakpad to create crash.dmp on MacOS / Linux too (#11202) Normally only the Windows platform could create a crash.dmp, making analysing crash-reports from MacOS / Linux rather tricky. --- .github/workflows/ci-build.yml | 8 ++- .github/workflows/release-linux.yml | 1 + .github/workflows/release-macos.yml | 2 + .github/workflows/release-windows.yml | 1 + CMakeLists.txt | 9 ++++ COMPILING.md | 6 ++- cmake/FindLZO.cmake | 2 +- cmake/LinkPackage.cmake | 3 ++ src/crashlog.cpp | 11 ++++- src/crashlog.h | 6 --- src/os/macosx/crashlog_osx.cpp | 25 +++++++++- src/os/unix/crashlog_unix.cpp | 22 +++++++++ src/os/windows/crashlog_win.cpp | 70 ++++++++++----------------- 13 files changed, 107 insertions(+), 59 deletions(-) diff --git a/.github/workflows/ci-build.yml b/.github/workflows/ci-build.yml index 4f84a69ef3..f7cd10c17e 100644 --- a/.github/workflows/ci-build.yml +++ b/.github/workflows/ci-build.yml @@ -121,6 +121,10 @@ jobs: ${{ matrix.libraries }} \ zlib1g-dev \ # EOF + + sudo vcpkg install \ + breakpad \ + # EOF echo "::endgroup::" env: DEBIAN_FRONTEND: noninteractive @@ -149,7 +153,7 @@ jobs: cd build echo "::group::CMake" - cmake .. ${{ matrix.extra-cmake-parameters }} + cmake .. -DCMAKE_TOOLCHAIN_FILE=/usr/local/share/vcpkg/scripts/buildsystems/vcpkg.cmake ${{ matrix.extra-cmake-parameters }} echo "::endgroup::" echo "::group::Build" @@ -205,6 +209,7 @@ jobs: - name: Prepare vcpkg run: | vcpkg install --triplet=${{ matrix.arch }}-osx \ + breakpad \ curl \ liblzma \ libpng \ @@ -290,6 +295,7 @@ jobs: shell: bash run: | vcpkg install --triplet=${{ matrix.arch }}-windows-static \ + breakpad \ liblzma \ libpng \ lzo \ diff --git a/.github/workflows/release-linux.yml b/.github/workflows/release-linux.yml index acfe45c2cf..247cd0b0c3 100644 --- a/.github/workflows/release-linux.yml +++ b/.github/workflows/release-linux.yml @@ -108,6 +108,7 @@ jobs: ln -sf $(pwd)/installed/x64-linux/tools/python3/python3.[0-9][0-9] /usr/bin/python3 ./vcpkg install \ + breakpad \ curl[http2] \ fontconfig \ freetype \ diff --git a/.github/workflows/release-macos.yml b/.github/workflows/release-macos.yml index ec82f8653e..7741064bf7 100644 --- a/.github/workflows/release-macos.yml +++ b/.github/workflows/release-macos.yml @@ -53,6 +53,8 @@ jobs: - name: Prepare vcpkg run: | vcpkg install \ + breakpad:x64-osx \ + breakpad:arm64-osx \ curl:x64-osx \ curl:arm64-osx \ liblzma:x64-osx \ diff --git a/.github/workflows/release-windows.yml b/.github/workflows/release-windows.yml index c55a4f8d4a..612abc4f72 100644 --- a/.github/workflows/release-windows.yml +++ b/.github/workflows/release-windows.yml @@ -65,6 +65,7 @@ jobs: shell: bash run: | vcpkg install --triplet=${{ matrix.arch }}-windows-static \ + breakpad \ liblzma \ libpng \ lzo \ diff --git a/CMakeLists.txt b/CMakeLists.txt index 1fc0fe1d16..e699c4d56d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -134,6 +134,11 @@ else() find_package(CURL) endif() +# Breakpad doesn't support emscripten. +if(NOT EMSCRIPTEN) + find_package(unofficial-breakpad) +endif() + if(NOT OPTION_DEDICATED) if(NOT WIN32) find_package(Allegro) @@ -310,6 +315,10 @@ if(NOT WIN32 AND NOT EMSCRIPTEN) link_package(CURL ENCOURAGED) endif() +if(NOT EMSCRIPTEN) + link_package(unofficial-breakpad TARGET unofficial::breakpad::libbreakpad_client ENCOURAGED) +endif() + if(NOT OPTION_DEDICATED) link_package(Fluidsynth) link_package(SDL) diff --git a/COMPILING.md b/COMPILING.md index be1bad79ab..ef9589366d 100644 --- a/COMPILING.md +++ b/COMPILING.md @@ -5,6 +5,7 @@ OpenTTD makes use of the following external libraries: - (encouraged) nlohmann-json: JSON handling +- (encouraged) breakpad: creates minidumps on crash - (encouraged) zlib: (de)compressing of old (0.3.0-1.0.5) savegames, content downloads, heightmaps - (encouraged) liblzma: (de)compressing of savegames (1.1.0 and later) @@ -50,6 +51,7 @@ by following the `Quick Start` instructions of their After this, you can install the dependencies OpenTTD needs. We advise to use the `static` versions, and OpenTTD currently needs the following dependencies: +- breakpad - liblzma - libpng - lzo @@ -59,8 +61,8 @@ the `static` versions, and OpenTTD currently needs the following dependencies: To install both the x64 (64bit) and x86 (32bit) variants (though only one is necessary), you can use: ```ps -.\vcpkg install liblzma:x64-windows-static libpng:x64-windows-static lzo:x64-windows-static nlohmann-json:x64-windows-static zlib:x64-windows-static -.\vcpkg install liblzma:x86-windows-static libpng:x86-windows-static lzo:x86-windows-static nlohmann-json:x86-windows-static zlib:x86-windows-static +.\vcpkg install breakpad:x64-windows-static liblzma:x64-windows-static libpng:x64-windows-static lzo:x64-windows-static nlohmann-json:x64-windows-static zlib:x64-windows-static +.\vcpkg install breakpad:x86-windows-static liblzma:x86-windows-static libpng:x86-windows-static lzo:x86-windows-static nlohmann-json:x86-windows-static zlib:x86-windows-static ``` You can open the folder (as a CMake project). CMake will be detected, and you can compile from there. diff --git a/cmake/FindLZO.cmake b/cmake/FindLZO.cmake index dacd9387d9..20ea4c1b57 100644 --- a/cmake/FindLZO.cmake +++ b/cmake/FindLZO.cmake @@ -55,7 +55,7 @@ find_library(LZO_LIBRARY # name as the optimized file. This is not always the case, but so far # experiences has shown that in those case vcpkg CMake files do the right # thing. -if(VCPKG_TOOLCHAIN AND LZO_LIBRARY) +if(VCPKG_TOOLCHAIN AND LZO_LIBRARY AND LZO_LIBRARY MATCHES "${VCPKG_INSTALLED_DIR}") if(LZO_LIBRARY MATCHES "/debug/") set(LZO_LIBRARY_DEBUG ${LZO_LIBRARY}) string(REPLACE "/debug/lib/" "/lib/" LZO_LIBRARY_RELEASE ${LZO_LIBRARY}) diff --git a/cmake/LinkPackage.cmake b/cmake/LinkPackage.cmake index afbd921cf7..9fb2cf5910 100644 --- a/cmake/LinkPackage.cmake +++ b/cmake/LinkPackage.cmake @@ -3,6 +3,9 @@ function(link_package NAME) if(${NAME}_FOUND) string(TOUPPER "${NAME}" UCNAME) + # Some libraries have a dash, which is not allowed in a preprocessor macro. + string(REPLACE "-" "_" UCNAME "${UCNAME}") + add_definitions(-DWITH_${UCNAME}) # Some libraries' cmake packages (looking at you, SDL2) leave trailing whitespace in the link commands, # which (later) cmake considers to be an error. Work around this with by stripping the incoming string. diff --git a/src/crashlog.cpp b/src/crashlog.cpp index 72d7c23d12..1692f32c64 100644 --- a/src/crashlog.cpp +++ b/src/crashlog.cpp @@ -369,9 +369,14 @@ bool CrashLog::WriteCrashLog() return len == written; } +/** + * Write the (crash) dump to a file. + * + * @note Sets \c crashdump_filename when there is a successful return. + * @return 1 iff the crashdump was successfully created, -1 if it failed, 0 if not implemented. + */ /* virtual */ int CrashLog::WriteCrashDump() { - /* Stub implementation; not all OSes support this. */ return 0; } @@ -452,13 +457,15 @@ bool CrashLog::MakeCrashLog() ret = false; } - /* Don't mention writing crash dumps because not all platforms support it. */ + fmt::print("Writing crash dump to disk...\n"); int dret = this->WriteCrashDump(); if (dret < 0) { fmt::print("Writing crash dump failed.\n\n"); ret = false; } else if (dret > 0) { fmt::print("Crash dump written to {}. Please add this file to any bug reports.\n\n", this->crashdump_filename); + } else { + fmt::print("Skipped; missing dependency to create crash dump.\n"); } fmt::print("Writing crash savegame...\n"); diff --git a/src/crashlog.h b/src/crashlog.h index 8a1a925b21..c310689d72 100644 --- a/src/crashlog.h +++ b/src/crashlog.h @@ -73,12 +73,6 @@ public: void FillCrashLog(std::back_insert_iterator &output_iterator) const; bool WriteCrashLog(); - /** - * Write the (crash) dump to a file. - * @note Sets \c crashdump_filename when there is a successful return. - * @return if less than 0, error. If 0 no dump is made, otherwise the dump - * was successful (not all OSes support dumping files). - */ virtual int WriteCrashDump(); bool WriteSavegame(); bool WriteScreenshot(); diff --git a/src/os/macosx/crashlog_osx.cpp b/src/os/macosx/crashlog_osx.cpp index 8c9c0ebcaf..72ebff316c 100644 --- a/src/os/macosx/crashlog_osx.cpp +++ b/src/os/macosx/crashlog_osx.cpp @@ -9,6 +9,7 @@ #include "../../stdafx.h" #include "../../crashlog.h" +#include "../../fileio_func.h" #include "../../string_func.h" #include "../../gamelog.h" #include "../../saveload/saveload.h" @@ -20,6 +21,10 @@ #include #include +#ifdef WITH_UNOFFICIAL_BREAKPAD +# include +#endif + #include "../../safeguards.h" @@ -139,6 +144,22 @@ class CrashLogOSX : public CrashLog { fmt::format_to(output_iterator, "\n"); } +#ifdef WITH_UNOFFICIAL_BREAKPAD + static bool MinidumpCallback(const char* dump_dir, const char* minidump_id, void* context, bool succeeded) + { + CrashLogOSX *crashlog = reinterpret_cast(context); + + crashlog->crashdump_filename = crashlog->CreateFileName(".dmp"); + std::rename(fmt::format("{}/{}.dmp", dump_dir, minidump_id).c_str(), crashlog->crashdump_filename.c_str()); + return succeeded; + } + + int WriteCrashDump() override + { + return google_breakpad::ExceptionHandler::WriteMinidump(_personal_dir, MinidumpCallback, this) ? 1 : -1; + } +#endif + public: /** * A crash log is always generated by signal. @@ -155,8 +176,8 @@ public: std::string message = fmt::format( "Please send the generated crash information and the last (auto)save to the developers. " "This will greatly help debugging. The correct place to do this is https://github.com/OpenTTD/OpenTTD/issues.\n\n" - "Generated file(s):\n{}\n{}\n{}", - this->crashlog_filename, this->savegame_filename, this->screenshot_filename); + "Generated file(s):\n{}\n{}\n{}\n{}", + this->crashlog_filename, this->crashdump_filename, this->savegame_filename, this->screenshot_filename); ShowMacDialog(crash_title, message.c_str(), "Quit"); } diff --git a/src/os/unix/crashlog_unix.cpp b/src/os/unix/crashlog_unix.cpp index 691a9a841b..5f8de4cec4 100644 --- a/src/os/unix/crashlog_unix.cpp +++ b/src/os/unix/crashlog_unix.cpp @@ -9,6 +9,7 @@ #include "../../stdafx.h" #include "../../crashlog.h" +#include "../../fileio_func.h" #include "../../string_func.h" #include "../../gamelog.h" #include "../../saveload/saveload.h" @@ -25,6 +26,10 @@ #include #endif +#ifdef WITH_UNOFFICIAL_BREAKPAD +# include +#endif + #include "../../safeguards.h" /** @@ -84,6 +89,23 @@ class CrashLogUnix : public CrashLog { #endif fmt::format_to(output_iterator, "\n"); } + +#ifdef WITH_UNOFFICIAL_BREAKPAD + static bool MinidumpCallback(const google_breakpad::MinidumpDescriptor& descriptor, void* context, bool succeeded) + { + CrashLogUnix *crashlog = reinterpret_cast(context); + + crashlog->crashdump_filename = crashlog->CreateFileName(".dmp"); + std::rename(descriptor.path(), crashlog->crashdump_filename.c_str()); + return succeeded; + } + + int WriteCrashDump() override + { + return google_breakpad::ExceptionHandler::WriteMinidump(_personal_dir, MinidumpCallback, this) ? 1 : -1; + } +#endif + public: /** * A crash log is always generated by signal. diff --git a/src/os/windows/crashlog_win.cpp b/src/os/windows/crashlog_win.cpp index a9fc02209c..786c7cb9b4 100644 --- a/src/os/windows/crashlog_win.cpp +++ b/src/os/windows/crashlog_win.cpp @@ -23,6 +23,14 @@ #include #include +#if defined(_MSC_VER) +# include +#endif + +#ifdef WITH_UNOFFICIAL_BREAKPAD +# include +#endif + #include "../../safeguards.h" /** @@ -37,8 +45,24 @@ class CrashLogWindows : public CrashLog { void LogStacktrace(std::back_insert_iterator &output_iterator) const override; void LogModules(std::back_insert_iterator &output_iterator) const override; public: + +#ifdef WITH_UNOFFICIAL_BREAKPAD + static bool MinidumpCallback(const wchar_t *dump_dir, const wchar_t *minidump_id, void *context, EXCEPTION_POINTERS *exinfo, MDRawAssertionInfo *assertion, bool succeeded) + { + CrashLogWindows *crashlog = reinterpret_cast(context); + + crashlog->crashdump_filename = crashlog->CreateFileName(".dmp"); + std::rename(fmt::format("{}/{}.dmp", FS2OTTD(dump_dir), FS2OTTD(minidump_id)).c_str(), crashlog->crashdump_filename.c_str()); + return succeeded; + } + + int WriteCrashDump() override + { + return google_breakpad::ExceptionHandler::WriteMinidump(OTTD2FS(_personal_dir), MinidumpCallback, this) ? 1 : -1; + } +#endif + #if defined(_MSC_VER) - int WriteCrashDump() override; void AppendDecodedStacktrace(std::back_insert_iterator &output_iterator) const; #else void AppendDecodedStacktrace(std::back_insert_iterator &output_iterator) const {} @@ -205,10 +229,6 @@ static void PrintModuleInfo(std::back_insert_iterator &output_itera static const uint MAX_SYMBOL_LEN = 512; static const uint MAX_FRAMES = 64; -#pragma warning(disable:4091) -#include -#pragma warning(default:4091) - void CrashLogWindows::AppendDecodedStacktrace(std::back_insert_iterator &output_iterator) const { DllLoader dbghelp(L"dbghelp.dll"); @@ -321,46 +341,6 @@ void CrashLogWindows::AppendDecodedStacktrace(std::back_insert_iteratorcrashdump_filename = this->CreateFileName(".dmp"); - HANDLE file = CreateFile(OTTD2FS(this->crashdump_filename).c_str(), GENERIC_WRITE, 0, nullptr, CREATE_ALWAYS, 0, 0); - HANDLE proc = GetCurrentProcess(); - DWORD procid = GetCurrentProcessId(); - MINIDUMP_EXCEPTION_INFORMATION mdei; - MINIDUMP_USER_STREAM userstream; - MINIDUMP_USER_STREAM_INFORMATION musi; - - userstream.Type = LastReservedStream + 1; - userstream.Buffer = (void*)this->crashlog.data(); - userstream.BufferSize = (ULONG)this->crashlog.size() + 1; - - musi.UserStreamCount = 1; - musi.UserStreamArray = &userstream; - - mdei.ThreadId = GetCurrentThreadId(); - mdei.ExceptionPointers = ep; - mdei.ClientPointers = false; - - funcMiniDumpWriteDump(proc, procid, file, MiniDumpWithDataSegs, &mdei, &musi, nullptr); - ret = 1; - } else { - ret = -1; - } - } - return ret; -} #endif /* _MSC_VER */ extern bool CloseConsoleLogIfActive();