Merge pull request #481 from geode-sdk/breakpad

Use Breakpad for crash reports on Android

.github/actions/setup-dump-sym/action.yml

@@ -0,0 +1,42 @@
+name: Setup Breakpad Dump Symbols
+description: Sets up + builds the dump_syms utility from Breakpad
+
+inputs:
+  dump_syms_version:
+    description: "Revision of mozilla/dump_syms repository to fetch"
+    required: true
+    default: "v2.2.2"
+outputs:
+  binary-path:
+    description: "Path of the dump_syms utility, including the executable."
+    value: ${{ steps.get-path.outputs.binary-path }}
+
+runs:
+  using: "composite"
+  steps:
+  - uses: dtolnay/rust-toolchain@stable
+    id: toolchain
+
+  - uses: actions/cache@v4
+    id: binary-cache
+    with:
+      path: "./dump_syms/target/release/dump_syms"
+      key: dump_syms-${{ inputs.dump_syms_version }}-${{steps.toolchain.outputs.cachekey}}
+
+  - uses: actions/checkout@v4
+    if: steps.binary-cache.outputs.cache-hit != 'true'
+    with:
+      repository: "mozilla/dump_syms"
+      ref: ${{ inputs.dump_syms_version }}
+      path: "./dump_syms"
+
+  - run: cargo build --release
+    shell: bash
+    if: steps.binary-cache.outputs.cache-hit != 'true'
+    working-directory: "./dump_syms"
+
+  - run: echo "binary-path=$(realpath target/release/dump_syms)" >> "$GITHUB_OUTPUT"
+    shell: bash
+    id: get-path
+    working-directory: "./dump_syms"
+

.github/workflows/build.yml

@@ -130,6 +130,11 @@ jobs:
         sudo apt install ninja-build
       if: matrix.config.id == 'android32' || matrix.config.id == 'android64'
 
+    - name: Setup Breakpad Tools
+      uses: ./.github/actions/setup-dump-sym
+      id: breakpad-tools
+      if: matrix.config.id == 'android32' || matrix.config.id == 'android64'
+
     - name: Configure
       run: >
         cmake -B ${{ github.workspace }}/build
@@ -142,6 +147,15 @@ jobs:
         cmake --build . --config RelWithDebInfo --parallel
         rm ${{ github.workspace }}/bin/nightly/resources/.geode_cache
 
+    - name: Dump Symbols
+      run: "${{ steps.breakpad-tools.outputs.binary-path }} ./bin/nightly/Geode.${{ matrix.config.id }}.so -o ./bin/nightly/Geode.${{ matrix.config.id }}.so.sym"
+      if: matrix.config.id == 'android32' || matrix.config.id == 'android64'
+
+    # hardcoding toolchain path :(
+    - name: Strip Binaries
+      run: "$NDK_HOME/toolchains/llvm/prebuilt/linux-x86_64/bin/llvm-strip ./bin/nightly/Geode.${{ matrix.config.id }}.so"
+      if: matrix.config.id == 'android32' || matrix.config.id == 'android64'
+
     - name: Upload Artifacts
       uses: actions/upload-artifact@v3
       with:
@@ -200,13 +214,13 @@ jobs:
       - name: Zip Android32 Artifacts
         uses: vimtor/action-zip@v1.1
         with:
-          files: geode-android32/Geode.android32.so
+          files: geode-android32/Geode.android32.so geode-android32/Geode.android32.so.sym
           dest: geode-${{ steps.ref.outputs.hash }}-android32.zip
 
       - name: Zip Android64 Artifacts
         uses: vimtor/action-zip@v1.1
         with:
-          files: geode-android64/Geode.android64.so
+          files: geode-android64/Geode.android64.so geode-android64/Geode.android64.so.sym
           dest: geode-${{ steps.ref.outputs.hash }}-android64.zip
 
       - name: Zip Resources

CMakeLists.txt

@@ -10,6 +10,8 @@ if (GEODE_BUILDING_DOCS)
 	set(GEODE_DONT_BUILD_TEST_MODS On)
 endif()
 
+option(GEODE_USE_BREAKPAD "Enables the use of the Breakpad library for crash dumps." ON)
+
 # Read version
 file(READ VERSION GEODE_VERSION)
 string(STRIP "${GEODE_VERSION}" GEODE_VERSION)

loader/CMakeLists.txt

@@ -125,6 +125,62 @@ if (WIN32)
 	set(SOURCES ${SOURCES} ${CMAKE_CURRENT_BINARY_DIR}/info.rc)
 endif()
 
+
+if (ANDROID AND GEODE_USE_BREAKPAD)
+	CPMAddPackage(
+		NAME "Breakpad"
+		VERSION "2023.06.01"
+		GIT_REPOSITORY "https://chromium.googlesource.com/breakpad/breakpad.git"
+	)
+
+	if (Breakpad_ADDED)
+		# uses fetchcontent to store the library in an arbitrary path (the binary dir)
+		include(FetchContent)
+
+		FetchContent_Declare(lss
+			GIT_REPOSITORY https://chromium.googlesource.com/linux-syscall-support.git
+			GIT_TAG v2022.10.12
+			SOURCE_DIR ${Breakpad_BINARY_DIR}/src/third_party/lss
+		)
+
+		FetchContent_MakeAvailable(lss)
+
+		add_library(Breakpad STATIC
+			${Breakpad_SOURCE_DIR}/src/client/linux/crash_generation/crash_generation_client.cc
+			${Breakpad_SOURCE_DIR}/src/client/linux/dump_writer_common/thread_info.cc
+			${Breakpad_SOURCE_DIR}/src/client/linux/dump_writer_common/ucontext_reader.cc
+			${Breakpad_SOURCE_DIR}/src/client/linux/handler/exception_handler.cc
+			${Breakpad_SOURCE_DIR}/src/client/linux/handler/minidump_descriptor.cc
+			${Breakpad_SOURCE_DIR}/src/client/linux/log/log.cc
+			${Breakpad_SOURCE_DIR}/src/client/linux/microdump_writer/microdump_writer.cc
+			${Breakpad_SOURCE_DIR}/src/client/linux/minidump_writer/linux_dumper.cc
+			${Breakpad_SOURCE_DIR}/src/client/linux/minidump_writer/linux_ptrace_dumper.cc
+			${Breakpad_SOURCE_DIR}/src/client/linux/minidump_writer/minidump_writer.cc
+			${Breakpad_SOURCE_DIR}/src/client/linux/minidump_writer/pe_file.cc
+			${Breakpad_SOURCE_DIR}/src/client/minidump_file_writer.cc
+			${Breakpad_SOURCE_DIR}/src/common/convert_UTF.cc
+			${Breakpad_SOURCE_DIR}/src/common/md5.cc
+			${Breakpad_SOURCE_DIR}/src/common/string_conversion.cc
+			${Breakpad_SOURCE_DIR}/src/common/linux/breakpad_getcontext.S
+			${Breakpad_SOURCE_DIR}/src/common/linux/elfutils.cc
+			${Breakpad_SOURCE_DIR}/src/common/linux/file_id.cc
+			${Breakpad_SOURCE_DIR}/src/common/linux/guid_creator.cc
+			${Breakpad_SOURCE_DIR}/src/common/linux/linux_libc_support.cc
+			${Breakpad_SOURCE_DIR}/src/common/linux/memory_mapped_file.cc
+			${Breakpad_SOURCE_DIR}/src/common/linux/safe_readlink.cc
+		)
+
+		target_include_directories(Breakpad PUBLIC
+			${Breakpad_SOURCE_DIR}/src/common/android/include/
+			${Breakpad_SOURCE_DIR}/src/
+			${Breakpad_SOURCE_DIR}/src/common/android/include/
+
+			# add lss headers
+			${Breakpad_BINARY_DIR}/src/
+		)
+	endif()
+endif()
+
 add_library(${PROJECT_NAME} SHARED ${SOURCES})
 
 target_compile_features(${PROJECT_NAME} PUBLIC cxx_std_20)
@@ -170,6 +226,12 @@ endif()
 if (ANDROID)
 	# needed to define some opengl functions
 	target_link_libraries(${PROJECT_NAME} EGL)
+
+	if (GEODE_USE_BREAKPAD)
+		target_compile_definitions(${PROJECT_NAME} PRIVATE -DGEODE_USE_BREAKPAD)
+		enable_language(ASM)
+		target_link_libraries(${PROJECT_NAME} Breakpad)
+	endif()
 endif()
 
 target_include_directories(${PROJECT_NAME} PRIVATE

loader/src/loader/LoaderImpl.cpp

@@ -78,12 +78,17 @@ Result<> Loader::Impl::setup() {
         log::popNest();
     }
 
-    log::debug("Setting up crash handler");
-    log::pushNest();
-    if (!crashlog::setupPlatformHandler()) {
-        log::debug("Failed to set up crash handler");
+    // on some platforms, using the crash handler overrides more convenient native handlers
+    if (!this->getLaunchFlag("disable-crash-handler")) {
+        log::debug("Setting up crash handler");
+        log::pushNest();
+        if (!crashlog::setupPlatformHandler()) {
+            log::debug("Failed to set up crash handler");
+        }
+        log::popNest();
+    } else {
+        log::debug("Crash handler setup skipped");
     }
-    log::popNest();
 
     log::debug("Loading hooks");
     log::pushNest();

loader/src/platform/android/crashlog.cpp

@@ -1,5 +1,56 @@
 #include <crashlog.hpp>
 
+static bool s_lastLaunchCrashed = false;
+
+#ifdef GEODE_USE_BREAKPAD
+
+#include <memory>
+
+#include <client/linux/handler/exception_handler.h>
+#include <client/linux/handler/minidump_descriptor.h>
+
+namespace {
+    // this object must be kept alive
+    auto s_exceptionHandler = std::unique_ptr<google_breakpad::ExceptionHandler>(nullptr);
+
+    bool crashCallback(
+        google_breakpad::MinidumpDescriptor const& descriptor, void* /* context */, bool succeeded
+    ) {
+        // jumping into unsafe territory :fish:
+        // create a file that indicates a crash did happen (which is then cleared on next launch)
+        auto crashIndicatorPath = crashlog::getCrashLogDirectory() / "lastSessionDidCrash";
+        auto indicatorString = crashIndicatorPath.string();
+
+        sys_open(indicatorString.c_str(), O_CREAT | O_WRONLY | O_TRUNC, 0);
+
+        return succeeded;
+    }
+}
+
+bool crashlog::setupPlatformHandler() {
+    auto logDirectory = crashlog::getCrashLogDirectory();
+
+    (void)geode::utils::file::createDirectoryAll(logDirectory);
+
+    google_breakpad::MinidumpDescriptor descriptor(logDirectory.string() + "/");
+
+    s_exceptionHandler = std::make_unique<google_breakpad::ExceptionHandler>(
+        descriptor, nullptr, crashCallback, nullptr, true, -1
+    );
+
+    auto crashIndicatorPath = crashlog::getCrashLogDirectory() / "lastSessionDidCrash";
+    if (ghc::filesystem::exists(crashIndicatorPath)) {
+        s_lastLaunchCrashed = true;
+        ghc::filesystem::remove(crashIndicatorPath);
+    }
+
+    return true;
+}
+
+void crashlog::setupPlatformHandlerPost() { }
+
+#else
+
 using namespace geode::prelude;
 
 #include <Geode/utils/string.hpp>
@@ -188,12 +239,6 @@ static void handlerThread() {
     log::debug("Notified");
 }
 
-static bool s_lastLaunchCrashed = false;
-
-ghc::filesystem::path crashlog::getCrashLogDirectory() {
-    return dirs::getGeodeDir() / "crashlogs";
-}
-
 int writeAndGetPid() {
     auto pidFile = crashlog::getCrashLogDirectory() / "last-pid";
 
@@ -307,6 +352,12 @@ void crashlog::setupPlatformHandlerPost() {
     actualFile.close();
 }
 
+#endif
+
+ghc::filesystem::path crashlog::getCrashLogDirectory() {
+    return geode::dirs::getGeodeDir() / "crashlogs";
+}
+
 bool crashlog::didLastLaunchCrash() {
     return s_lastLaunchCrashed;
 }