diff --git a/.github/ISSUE_TEMPLATE/crash.yml b/.github/ISSUE_TEMPLATE/crash.yml index 80fa827..c8b4804 100644 --- a/.github/ISSUE_TEMPLATE/crash.yml +++ b/.github/ISSUE_TEMPLATE/crash.yml @@ -1,4 +1,4 @@ -name: Crash Report (v1) +name: Crash Report description: Quickshell has crashed labels: ["bug", "crash"] body: diff --git a/.github/ISSUE_TEMPLATE/crash2.yml b/.github/ISSUE_TEMPLATE/crash2.yml deleted file mode 100644 index 84beef8..0000000 --- a/.github/ISSUE_TEMPLATE/crash2.yml +++ /dev/null @@ -1,49 +0,0 @@ -name: Crash Report (v2) -description: Quickshell has crashed -labels: ["bug", "crash"] -body: - - type: textarea - id: userinfo - attributes: - label: What caused the crash - description: | - Any information likely to help debug the crash. What were you doing when the crash occurred, - what changes did you make, can you get it to happen again? - - type: textarea - id: report - attributes: - label: Report file - description: Attach `report.txt` here. - validations: - required: true - - type: textarea - id: logs - attributes: - label: Log file - description: | - Attach `log.qslog.log` here. If it is too big to upload, compress it. - - You can preview the log if you'd like using `quickshell read-log `. - validations: - required: true - - type: textarea - id: config - attributes: - label: Configuration - description: | - Attach your configuration here, preferrably in full (not just one file). - Compress it into a zip, tar, etc. - - This will help us reproduce the crash ourselves. - - type: textarea - id: bt - attributes: - label: Backtrace - description: | - GDB usually produces better stacktraces than quickshell can. Consider attaching a gdb backtrace - following the instructions below. - - 1. Run `coredumpctl debug ` where `pid` is the number shown after "Crashed process ID" - in the crash reporter. - 2. Once it loads, type `bt -full` (then enter) - 3. Copy the output and attach it as a file or in a spoiler. diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 7b8cbce..8d19f58 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -55,11 +55,10 @@ jobs: libpipewire \ cli11 \ polkit \ - jemalloc \ - libunwind \ - git # for cpptrace clone + jemalloc - name: Build + # breakpad is annoying to build in ci due to makepkg not running as root run: | - cmake -GNinja -B build -DVENDOR_CPPTRACE=ON + cmake -GNinja -B build -DCRASH_REPORTER=OFF cmake --build build diff --git a/BUILD.md b/BUILD.md index 29aecac..c9459b5 100644 --- a/BUILD.md +++ b/BUILD.md @@ -15,7 +15,15 @@ Please make this descriptive enough to identify your specific package, for examp - `Nixpkgs` - `Fedora COPR (errornointernet/quickshell)` -Please leave at least symbol names attached to the binary for debugging purposes. +`-DDISTRIBUTOR_DEBUGINFO_AVAILABLE=YES/NO` + +If we can retrieve binaries and debug information for the package without actually running your +distribution (e.g. from an website), and you would like to strip the binary, please set this to `YES`. + +If we cannot retrieve debug information, please set this to `NO` and +**ensure you aren't distributing stripped (non debuggable) binaries**. + +In both cases you should build with `-DCMAKE_BUILD_TYPE=RelWithDebInfo` (then split or keep the debuginfo). ### QML Module dir Currently all QML modules are statically linked to quickshell, but this is where @@ -56,18 +64,14 @@ At least Qt 6.6 is required. All features are enabled by default and some have their own dependencies. -### Crash Handler -The crash reporter catches crashes, restarts Quickshell when it crashes, +### Crash Reporter +The crash reporter catches crashes, restarts quickshell when it crashes, and collects useful crash information in one place. Leaving this enabled will enable us to fix bugs far more easily. -To disable: `-DCRASH_HANDLER=OFF` +To disable: `-DCRASH_REPORTER=OFF` -Dependencies: `cpptrace` - -Note: `-DVENDOR_CPPTRACE=ON` can be set to vendor cpptrace using FetchContent. - -When using FetchContent, `libunwind` is required, and `libdwarf` can be provided by the package manager or fetched with FetchContent. +Dependencies: `google-breakpad` (static library) ### Jemalloc We recommend leaving Jemalloc enabled as it will mask memory fragmentation caused diff --git a/CMakeLists.txt b/CMakeLists.txt index d57e322..7633f4f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -40,17 +40,19 @@ string(APPEND QS_BUILD_OPTIONS " Distributor: ${DISTRIBUTOR}") message(STATUS "Quickshell configuration") message(STATUS " Distributor: ${DISTRIBUTOR}") +boption(DISTRIBUTOR_DEBUGINFO_AVAILABLE "Distributor provided debuginfo" NO) boption(NO_PCH "Disable precompild headers (dev)" OFF) boption(BUILD_TESTING "Build tests (dev)" OFF) boption(ASAN "ASAN (dev)" OFF) # note: better output with gcc than clang boption(FRAME_POINTERS "Keep Frame Pointers (dev)" ${ASAN}) if (CMAKE_SYSTEM_NAME STREQUAL "FreeBSD") + boption(CRASH_REPORTER "Crash Handling" OFF) boption(USE_JEMALLOC "Use jemalloc" OFF) else() + boption(CRASH_REPORTER "Crash Handling" ON) boption(USE_JEMALLOC "Use jemalloc" ON) endif() -boption(CRASH_HANDLER "Crash Handling" ON) boption(SOCKETS "Unix Sockets" ON) boption(WAYLAND "Wayland" ON) boption(WAYLAND_WLR_LAYERSHELL " Wlroots Layer-Shell" ON REQUIRES WAYLAND) diff --git a/changelog/next.md b/changelog/next.md index 0feffe1..b9000c2 100644 --- a/changelog/next.md +++ b/changelog/next.md @@ -32,7 +32,6 @@ set shell id. - FreeBSD is now partially supported. - IPC operations filter available instances to the current display connection by default. - PwNodeLinkTracker ignores sound level monitoring programs. -- Replaced breakpad with cpptrace. ## Bug Fixes @@ -53,7 +52,5 @@ set shell id. ## Packaging Changes -- `glib` and `polkit` have been added as dependencies when compiling with polkit agent support. -- `vulkan-headers` has been added as a build-time dependency for screencopy (Vulkan backend support). -- `breakpad` has been replaced by `cpptrace`, which is far easier to package, and the `CRASH_REPORTER` cmake variable has been replaced with `CRASH_HANDLER` to stop this from being easy to ignore. -- `DISTRIBUTOR_DEBUGINFO_AVAILABLE` was removed as it is no longer important without breakpad. +`glib` and `polkit` have been added as dependencies when compiling with polkit agent support. +`vulkan-headers` has been added as a build-time dependency for screencopy (Vulkan backend support). diff --git a/default.nix b/default.nix index 59e68b0..7783774 100644 --- a/default.nix +++ b/default.nix @@ -10,9 +10,7 @@ ninja, spirv-tools, qt6, - cpptrace ? null, - libunwind, - libdwarf, + breakpad, jemalloc, cli11, wayland, @@ -51,8 +49,6 @@ withPolkit ? true, withNetworkManager ? true, }: let - withCrashHandler = withCrashReporter && cpptrace != null && lib.strings.compareVersions cpptrace.version "0.7.2" >= 0; - unwrapped = stdenv.mkDerivation { pname = "quickshell${lib.optionalString debug "-debug"}"; version = "0.2.1"; @@ -78,12 +74,7 @@ cli11 ] ++ lib.optional withQtSvg qt6.qtsvg - ++ lib.optional withCrashHandler (cpptrace.overrideAttrs (prev: { - cmakeFlags = prev.cmakeFlags ++ [ - "-DCPPTRACE_UNWIND_WITH_LIBUNWIND=TRUE" - ]; - buildInputs = prev.buildInputs ++ [ libunwind ]; - })) + ++ lib.optional withCrashReporter breakpad ++ lib.optional withJemalloc jemalloc ++ lib.optional (withWayland && lib.strings.compareVersions qt6.qtbase.version "6.10.0" == -1) qt6.qtwayland ++ lib.optionals withWayland [ wayland wayland-protocols ] @@ -100,7 +91,7 @@ (lib.cmakeFeature "INSTALL_QML_PREFIX" qt6.qtbase.qtQmlPrefix) (lib.cmakeBool "DISTRIBUTOR_DEBUGINFO_AVAILABLE" true) (lib.cmakeFeature "GIT_REVISION" gitRev) - (lib.cmakeBool "CRASH_HANDLER" withCrashHandler) + (lib.cmakeBool "CRASH_REPORTER" withCrashReporter) (lib.cmakeBool "USE_JEMALLOC" withJemalloc) (lib.cmakeBool "WAYLAND" withWayland) (lib.cmakeBool "SCREENCOPY" (libgbm != null)) diff --git a/quickshell.scm b/quickshell.scm index 780bb96..3f82160 100644 --- a/quickshell.scm +++ b/quickshell.scm @@ -56,7 +56,8 @@ #~(list "-GNinja" "-DDISTRIBUTOR=\"In-tree Guix channel\"" "-DDISTRIBUTOR_DEBUGINFO_AVAILABLE=NO" - "-DCRASH_HANDLER=OFF") + ;; Breakpad is not currently packaged for Guix. + "-DCRASH_REPORTER=OFF") #:phases #~(modify-phases %standard-phases (replace 'build (lambda _ (invoke "cmake" "--build" "."))) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 4b13d45..c95ecf7 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -12,7 +12,7 @@ add_subdirectory(io) add_subdirectory(widgets) add_subdirectory(ui) -if (CRASH_HANDLER) +if (CRASH_REPORTER) add_subdirectory(crash) endif() diff --git a/src/build/CMakeLists.txt b/src/build/CMakeLists.txt index c1ffa59..bb35da9 100644 --- a/src/build/CMakeLists.txt +++ b/src/build/CMakeLists.txt @@ -9,10 +9,16 @@ if (NOT DEFINED GIT_REVISION) ) endif() -if (CRASH_HANDLER) - set(CRASH_HANDLER_DEF 1) +if (CRASH_REPORTER) + set(CRASH_REPORTER_DEF 1) else() - set(CRASH_HANDLER_DEF 0) + set(CRASH_REPORTER_DEF 0) +endif() + +if (DISTRIBUTOR_DEBUGINFO_AVAILABLE) + set(DEBUGINFO_AVAILABLE 1) +else() + set(DEBUGINFO_AVAILABLE 0) endif() configure_file(build.hpp.in build.hpp @ONLY ESCAPE_QUOTES) diff --git a/src/build/build.hpp.in b/src/build/build.hpp.in index 2ab2db2..66fb664 100644 --- a/src/build/build.hpp.in +++ b/src/build/build.hpp.in @@ -8,7 +8,8 @@ #define QS_UNRELEASED_FEATURES "@UNRELEASED_FEATURES@" #define GIT_REVISION "@GIT_REVISION@" #define DISTRIBUTOR "@DISTRIBUTOR@" -#define CRASH_HANDLER @CRASH_HANDLER_DEF@ +#define DISTRIBUTOR_DEBUGINFO_AVAILABLE @DEBUGINFO_AVAILABLE@ +#define CRASH_REPORTER @CRASH_REPORTER_DEF@ #define BUILD_TYPE "@CMAKE_BUILD_TYPE@" #define COMPILER "@CMAKE_CXX_COMPILER_ID@ (@CMAKE_CXX_COMPILER_VERSION@)" #define COMPILE_FLAGS "@CMAKE_CXX_FLAGS@" diff --git a/src/core/instanceinfo.hpp b/src/core/instanceinfo.hpp index 977e4c2..d462f6e 100644 --- a/src/core/instanceinfo.hpp +++ b/src/core/instanceinfo.hpp @@ -35,8 +35,6 @@ namespace qs::crash { struct CrashInfo { int logFd = -1; - int traceFd = -1; - int infoFd = -1; static CrashInfo INSTANCE; // NOLINT }; diff --git a/src/crash/CMakeLists.txt b/src/crash/CMakeLists.txt index a891ee9..7fdd830 100644 --- a/src/crash/CMakeLists.txt +++ b/src/crash/CMakeLists.txt @@ -6,51 +6,12 @@ qt_add_library(quickshell-crash STATIC qs_pch(quickshell-crash SET large) -if (VENDOR_CPPTRACE) - message(STATUS "Vendoring cpptrace...") - include(FetchContent) - - # For use without internet access see: https://cmake.org/cmake/help/latest/module/FetchContent.html#variable:FETCHCONTENT_SOURCE_DIR_%3CuppercaseName%3E - FetchContent_Declare( - cpptrace - GIT_REPOSITORY https://github.com/jeremy-rifkin/cpptrace.git - GIT_TAG v1.0.4 - ) - - set(CPPTRACE_UNWIND_WITH_LIBUNWIND TRUE) - FetchContent_MakeAvailable(cpptrace) -else () - find_package(cpptrace REQUIRED) - - # useful for cross after you have already checked cpptrace is built correctly - if (NOT DO_NOT_CHECK_CPPTRACE_USABILITY) - try_run(CPPTRACE_SIGNAL_SAFE_UNWIND CPPTRACE_SIGNAL_SAFE_UNWIND_COMP - SOURCE_FROM_CONTENT check.cxx " - #include - int main() { - return cpptrace::can_signal_safe_unwind() ? 0 : 1; - } - " - LOG_DESCRIPTION "Checking ${CPPTRACE_SIGNAL_SAFE_UNWIND}" - LINK_LIBRARIES cpptrace::cpptrace - COMPILE_OUTPUT_VARIABLE CPPTRACE_SIGNAL_SAFE_UNWIND_LOG - CXX_STANDARD 20 - CXX_STANDARD_REQUIRED ON - ) - - if (NOT CPPTRACE_SIGNAL_SAFE_UNWIND_COMP) - message(STATUS "${CPPTRACE_SIGNAL_SAFE_UNWIND_LOG}") - message(FATAL_ERROR "Failed to compile cpptrace signal safe unwind tester.") - endif() - - if (NOT CPPTRACE_SIGNAL_SAFE_UNWIND EQUAL 0) - message(STATUS "Cpptrace signal safe unwind test exited with: ${CPPTRACE_SIGNAL_SAFE_UNWIND}") - message(FATAL_ERROR "Cpptrace was built without CPPTRACE_UNWIND_WITH_LIBUNWIND set to true. Enable libunwind support in the package or set VENDOR_CPPTRACE to true when building Quickshell.") - endif() - endif () -endif () +find_package(PkgConfig REQUIRED) +pkg_check_modules(breakpad REQUIRED IMPORTED_TARGET breakpad) +# only need client?? take only includes from pkg config todo +target_link_libraries(quickshell-crash PRIVATE PkgConfig::breakpad -lbreakpad_client) # quick linked for pch compat -target_link_libraries(quickshell-crash PRIVATE quickshell-build Qt::Quick Qt::Widgets cpptrace::cpptrace) +target_link_libraries(quickshell-crash PRIVATE quickshell-build Qt::Quick Qt::Widgets) target_link_libraries(quickshell PRIVATE quickshell-crash) diff --git a/src/crash/handler.cpp b/src/crash/handler.cpp index c875c2e..0baa8e6 100644 --- a/src/crash/handler.cpp +++ b/src/crash/handler.cpp @@ -1,13 +1,12 @@ #include "handler.hpp" -#include #include -#include -#include #include #include -#include -#include +#include +#include +#include +#include #include #include #include @@ -20,71 +19,99 @@ extern char** environ; // NOLINT +using namespace google_breakpad; + namespace qs::crash { namespace { - QS_LOGGING_CATEGORY(logCrashHandler, "quickshell.crashhandler", QtWarningMsg); +} -void writeEnvInt(char* buf, const char* name, int value) { - // NOLINTBEGIN (cppcoreguidelines-pro-bounds-pointer-arithmetic) - while (*name != '\0') *buf++ = *name++; - *buf++ = '='; +struct CrashHandlerPrivate { + ExceptionHandler* exceptionHandler = nullptr; + int minidumpFd = -1; + int infoFd = -1; - if (value < 0) { - *buf++ = '-'; - value = -value; + static bool minidumpCallback(const MinidumpDescriptor& descriptor, void* context, bool succeeded); +}; + +CrashHandler::CrashHandler(): d(new CrashHandlerPrivate()) {} + +void CrashHandler::init() { + // MinidumpDescriptor has no move constructor and the copy constructor breaks fds. + auto createHandler = [this](const MinidumpDescriptor& desc) { + this->d->exceptionHandler = new ExceptionHandler( + desc, + nullptr, + &CrashHandlerPrivate::minidumpCallback, + this->d, + true, + -1 + ); + }; + + qCDebug(logCrashHandler) << "Starting crash handler..."; + + this->d->minidumpFd = memfd_create("quickshell:minidump", MFD_CLOEXEC); + + if (this->d->minidumpFd == -1) { + qCCritical( + logCrashHandler + ) << "Failed to allocate minidump memfd, minidumps will be saved in the working directory."; + createHandler(MinidumpDescriptor(".")); + } else { + qCDebug(logCrashHandler) << "Created memfd" << this->d->minidumpFd + << "for holding possible minidumps."; + createHandler(MinidumpDescriptor(this->d->minidumpFd)); } - if (value == 0) { - *buf++ = '0'; - *buf = '\0'; + qCInfo(logCrashHandler) << "Crash handler initialized."; +} + +void CrashHandler::setRelaunchInfo(const RelaunchInfo& info) { + this->d->infoFd = memfd_create("quickshell:instance_info", MFD_CLOEXEC); + + if (this->d->infoFd == -1) { + qCCritical( + logCrashHandler + ) << "Failed to allocate instance info memfd, crash recovery will not work."; return; } - auto* start = buf; - while (value > 0) { - *buf++ = static_cast('0' + (value % 10)); - value /= 10; + QFile file; + + if (!file.open(this->d->infoFd, QFile::ReadWrite)) { + qCCritical( + logCrashHandler + ) << "Failed to open instance info memfd, crash recovery will not work."; } - *buf = '\0'; - std::reverse(start, buf); - // NOLINTEND + QDataStream ds(&file); + ds << info; + file.flush(); + + qCDebug(logCrashHandler) << "Stored instance info in memfd" << this->d->infoFd; } -void signalHandler( - int sig, - siginfo_t* /*info*/, // NOLINT (misc-include-cleaner) - void* /*context*/ +CrashHandler::~CrashHandler() { + delete this->d->exceptionHandler; + delete this->d; +} + +bool CrashHandlerPrivate::minidumpCallback( + const MinidumpDescriptor& /*descriptor*/, + void* context, + bool /*success*/ ) { - if (CrashInfo::INSTANCE.traceFd != -1) { - auto traceBuffer = std::array(); - auto frameCount = cpptrace::safe_generate_raw_trace(traceBuffer.data(), traceBuffer.size(), 1); - - for (size_t i = 0; i < static_cast(frameCount); i++) { - auto frame = cpptrace::safe_object_frame(); - cpptrace::get_safe_object_frame(traceBuffer[i], &frame); - - auto* wptr = reinterpret_cast(&frame); - auto* end = wptr + sizeof(cpptrace::safe_object_frame); // NOLINT - while (wptr != end) { - auto r = write(CrashInfo::INSTANCE.traceFd, &frame, sizeof(cpptrace::safe_object_frame)); - if (r < 0 && errno == EINTR) continue; - if (r <= 0) goto fail; - wptr += r; // NOLINT - } - } - - fail:; - } - + // A fork that just dies to ensure the coredump is caught by the system. auto coredumpPid = fork(); + if (coredumpPid == 0) { - raise(sig); - _exit(-1); + return false; } + auto* self = static_cast(context); + auto exe = std::array(); if (readlink("/proc/self/exe", exe.data(), exe.size() - 1) == -1) { perror("Failed to find crash reporter executable.\n"); @@ -96,19 +123,17 @@ void signalHandler( auto env = std::array(); auto envi = 0; - // dup to remove CLOEXEC - auto infoFdStr = std::array(); - writeEnvInt(infoFdStr.data(), "__QUICKSHELL_CRASH_INFO_FD", dup(CrashInfo::INSTANCE.infoFd)); + auto infoFd = dup(self->infoFd); + auto infoFdStr = std::array(); + memcpy(infoFdStr.data(), "__QUICKSHELL_CRASH_INFO_FD=-1" /*\0*/, 30); + if (infoFd != -1) my_uitos(&infoFdStr[27], infoFd, 10); env[envi++] = infoFdStr.data(); - auto corePidStr = std::array(); - writeEnvInt(corePidStr.data(), "__QUICKSHELL_CRASH_DUMP_PID", coredumpPid); + auto corePidStr = std::array(); + memcpy(corePidStr.data(), "__QUICKSHELL_CRASH_DUMP_PID=-1" /*\0*/, 31); + if (coredumpPid != -1) my_uitos(&corePidStr[28], coredumpPid, 10); env[envi++] = corePidStr.data(); - auto sigStr = std::array(); - writeEnvInt(sigStr.data(), "__QUICKSHELL_CRASH_SIGNAL", sig); - env[envi++] = sigStr.data(); - auto populateEnv = [&]() { auto senvi = 0; while (envi != 4095) { @@ -120,18 +145,30 @@ void signalHandler( env[envi] = nullptr; }; + sigset_t sigset; + sigemptyset(&sigset); // NOLINT (include) + sigprocmask(SIG_SETMASK, &sigset, nullptr); // NOLINT + auto pid = fork(); if (pid == -1) { perror("Failed to fork and launch crash reporter.\n"); - _exit(-1); + return false; } else if (pid == 0) { - // dup to remove CLOEXEC - auto dumpFdStr = std::array(); - auto logFdStr = std::array(); - writeEnvInt(dumpFdStr.data(), "__QUICKSHELL_CRASH_DUMP_FD", dup(CrashInfo::INSTANCE.traceFd)); - writeEnvInt(logFdStr.data(), "__QUICKSHELL_CRASH_LOG_FD", dup(CrashInfo::INSTANCE.logFd)); + // if already -1 will return -1 + auto dumpFd = dup(self->minidumpFd); + auto logFd = dup(CrashInfo::INSTANCE.logFd); + + // allow up to 10 digits, which should never happen + auto dumpFdStr = std::array(); + auto logFdStr = std::array(); + + memcpy(dumpFdStr.data(), "__QUICKSHELL_CRASH_DUMP_FD=-1" /*\0*/, 30); + memcpy(logFdStr.data(), "__QUICKSHELL_CRASH_LOG_FD=-1" /*\0*/, 29); + + if (dumpFd != -1) my_uitos(&dumpFdStr[27], dumpFd, 10); + if (logFd != -1) my_uitos(&logFdStr[26], logFd, 10); env[envi++] = dumpFdStr.data(); env[envi++] = logFdStr.data(); @@ -148,82 +185,8 @@ void signalHandler( perror("Failed to relaunch quickshell.\n"); _exit(-1); } -} -} // namespace - -void CrashHandler::init() { - qCDebug(logCrashHandler) << "Starting crash handler..."; - - CrashInfo::INSTANCE.traceFd = memfd_create("quickshell:trace", MFD_CLOEXEC); - - if (CrashInfo::INSTANCE.traceFd == -1) { - qCCritical(logCrashHandler) << "Failed to allocate trace memfd, stack traces will not be " - "available in crash reports."; - } else { - qCDebug(logCrashHandler) << "Created memfd" << CrashInfo::INSTANCE.traceFd - << "for holding possible stack traces."; - } - - { - // Preload anything dynamically linked to avoid malloc etc in the dynamic loader. - // See cpptrace documentation for more information. - auto buffer = std::array(); - cpptrace::safe_generate_raw_trace(buffer.data(), buffer.size()); - auto frame = cpptrace::safe_object_frame(); - cpptrace::get_safe_object_frame(buffer[0], &frame); - } - - // NOLINTBEGIN (misc-include-cleaner) - - // Set up alternate signal stack for stack overflow handling - auto ss = stack_t(); - ss.ss_sp = new char[SIGSTKSZ]; - ss.ss_size = SIGSTKSZ; - ss.ss_flags = 0; - sigaltstack(&ss, nullptr); - - // Install signal handlers - struct sigaction sa {}; - sa.sa_sigaction = &signalHandler; - sa.sa_flags = SA_SIGINFO | SA_ONSTACK | SA_RESETHAND; - sigemptyset(&sa.sa_mask); - - sigaction(SIGSEGV, &sa, nullptr); - sigaction(SIGABRT, &sa, nullptr); - sigaction(SIGFPE, &sa, nullptr); - sigaction(SIGILL, &sa, nullptr); - sigaction(SIGBUS, &sa, nullptr); - sigaction(SIGTRAP, &sa, nullptr); - - // NOLINTEND (misc-include-cleaner) - - qCInfo(logCrashHandler) << "Crash handler initialized."; -} - -void CrashHandler::setRelaunchInfo(const RelaunchInfo& info) { - CrashInfo::INSTANCE.infoFd = memfd_create("quickshell:instance_info", MFD_CLOEXEC); - - if (CrashInfo::INSTANCE.infoFd == -1) { - qCCritical( - logCrashHandler - ) << "Failed to allocate instance info memfd, crash recovery will not work."; - return; - } - - QFile file; - - if (!file.open(CrashInfo::INSTANCE.infoFd, QFile::ReadWrite)) { - qCCritical( - logCrashHandler - ) << "Failed to open instance info memfd, crash recovery will not work."; - } - - QDataStream ds(&file); - ds << info; - file.flush(); - - qCDebug(logCrashHandler) << "Stored instance info in memfd" << CrashInfo::INSTANCE.infoFd; + return false; // should make sure it hits the system coredump handler } } // namespace qs::crash diff --git a/src/crash/handler.hpp b/src/crash/handler.hpp index 9488d71..2a1d86f 100644 --- a/src/crash/handler.hpp +++ b/src/crash/handler.hpp @@ -5,10 +5,19 @@ #include "../core/instanceinfo.hpp" namespace qs::crash { +struct CrashHandlerPrivate; + class CrashHandler { public: - static void init(); - static void setRelaunchInfo(const RelaunchInfo& info); + explicit CrashHandler(); + ~CrashHandler(); + Q_DISABLE_COPY_MOVE(CrashHandler); + + void init(); + void setRelaunchInfo(const RelaunchInfo& info); + +private: + CrashHandlerPrivate* d; }; } // namespace qs::crash diff --git a/src/crash/interface.cpp b/src/crash/interface.cpp index a3422d3..326216a 100644 --- a/src/crash/interface.cpp +++ b/src/crash/interface.cpp @@ -78,7 +78,7 @@ CrashReporterGui::CrashReporterGui(QString reportFolder, int pid) mainLayout->addWidget(new ReportLabel( "Github:", - "https://github.com/quickshell-mirror/quickshell/issues/new?template=crash2.yml", + "https://github.com/quickshell-mirror/quickshell/issues/new?template=crash.yml", this )); @@ -114,7 +114,7 @@ void CrashReporterGui::openFolder() { void CrashReporterGui::openReportUrl() { QDesktopServices::openUrl( - QUrl("https://github.com/outfoxxed/quickshell/issues/new?template=crash2.yml") + QUrl("https://github.com/outfoxxed/quickshell/issues/new?template=crash.yml") ); } diff --git a/src/crash/main.cpp b/src/crash/main.cpp index c406ba6..6571660 100644 --- a/src/crash/main.cpp +++ b/src/crash/main.cpp @@ -1,10 +1,7 @@ #include "main.hpp" #include #include -#include -#include -#include #include #include #include @@ -16,17 +13,13 @@ #include #include #include -#include #include #include -#include #include "../core/instanceinfo.hpp" #include "../core/logcat.hpp" #include "../core/logging.hpp" -#include "../core/logging_p.hpp" #include "../core/paths.hpp" -#include "../core/ringbuf.hpp" #include "build.hpp" #include "interface.hpp" @@ -68,76 +61,6 @@ int tryDup(int fd, const QString& path) { return 0; } -QString readRecentLogs(int logFd, int maxLines, qint64 maxAgeSecs) { - QFile file; - if (!file.open(logFd, QFile::ReadOnly, QFile::AutoCloseHandle)) { - return QStringLiteral("(failed to open log fd)\n"); - } - - file.seek(0); - - qs::log::EncodedLogReader reader; - reader.setDevice(&file); - - bool readable = false; - quint8 logVersion = 0; - quint8 readerVersion = 0; - if (!reader.readHeader(&readable, &logVersion, &readerVersion) || !readable) { - return QStringLiteral("(failed to read log header)\n"); - } - - // Read all messages, keeping last maxLines in a ring buffer - auto tail = RingBuffer(maxLines); - qs::log::LogMessage message; - while (reader.read(&message)) { - tail.emplace(message); - } - - if (tail.size() == 0) { - return QStringLiteral("(no logs)\n"); - } - - // Filter to only messages within maxAgeSecs of the newest message - auto cutoff = tail.at(0).time.addSecs(-maxAgeSecs); - - QString result; - auto stream = QTextStream(&result); - for (auto i = tail.size() - 1; i != -1; i--) { - if (tail.at(i).time < cutoff) continue; - qs::log::LogMessage::formatMessage(stream, tail.at(i), false, true); - stream << '\n'; - } - - if (result.isEmpty()) { - return QStringLiteral("(no recent logs)\n"); - } - - return result; -} - -cpptrace::stacktrace resolveStacktrace(int dumpFd) { - QFile sourceFile; - if (!sourceFile.open(dumpFd, QFile::ReadOnly, QFile::AutoCloseHandle)) { - qCCritical(logCrashReporter) << "Failed to open trace memfd."; - return {}; - } - - sourceFile.seek(0); - auto data = sourceFile.readAll(); - - auto frameCount = static_cast(data.size()) / sizeof(cpptrace::safe_object_frame); - if (frameCount == 0) return {}; - - const auto* frames = reinterpret_cast(data.constData()); - - cpptrace::object_trace objectTrace; - for (size_t i = 0; i < frameCount; i++) { - objectTrace.frames.push_back(frames[i].resolve()); // NOLINT - } - - return objectTrace.resolve(); -} - void recordCrashInfo(const QDir& crashDir, const InstanceInfo& instance) { qCDebug(logCrashReporter) << "Recording crash information at" << crashDir.path(); @@ -148,25 +71,32 @@ void recordCrashInfo(const QDir& crashDir, const InstanceInfo& instance) { } auto crashProc = qEnvironmentVariable("__QUICKSHELL_CRASH_DUMP_PID").toInt(); - auto crashSignal = qEnvironmentVariable("__QUICKSHELL_CRASH_SIGNAL").toInt(); auto dumpFd = qEnvironmentVariable("__QUICKSHELL_CRASH_DUMP_FD").toInt(); auto logFd = qEnvironmentVariable("__QUICKSHELL_CRASH_LOG_FD").toInt(); - qCDebug(logCrashReporter) << "Resolving stacktrace from fd" << dumpFd; - auto stacktrace = resolveStacktrace(dumpFd); + qCDebug(logCrashReporter) << "Saving minidump from fd" << dumpFd; + auto dumpDupStatus = tryDup(dumpFd, crashDir.filePath("minidump.dmp.log")); + if (dumpDupStatus != 0) { + qCCritical(logCrashReporter) << "Failed to write minidump:" << dumpDupStatus; + } - qCDebug(logCrashReporter) << "Reading recent log lines from fd" << logFd; - auto logDupFd = dup(logFd); - auto recentLogs = readRecentLogs(logFd, 100, 10); - - qCDebug(logCrashReporter) << "Saving log from fd" << logDupFd; - auto logDupStatus = tryDup(logDupFd, crashDir.filePath("log.qslog.log")); + qCDebug(logCrashReporter) << "Saving log from fd" << logFd; + auto logDupStatus = tryDup(logFd, crashDir.filePath("log.qslog.log")); if (logDupStatus != 0) { qCCritical(logCrashReporter) << "Failed to save log:" << logDupStatus; } + auto copyBinStatus = 0; + if (!DISTRIBUTOR_DEBUGINFO_AVAILABLE) { + qCDebug(logCrashReporter) << "Copying binary to crash folder"; + if (!QFile(QCoreApplication::applicationFilePath()).copy(crashDir.filePath("executable.txt"))) { + copyBinStatus = 1; + qCCritical(logCrashReporter) << "Failed to copy binary."; + } + } + { - auto extraInfoFile = QFile(crashDir.filePath("report.txt")); + auto extraInfoFile = QFile(crashDir.filePath("info.txt")); if (!extraInfoFile.open(QFile::WriteOnly)) { qCCritical(logCrashReporter) << "Failed to open crash info file for writing."; } else { @@ -181,12 +111,16 @@ void recordCrashInfo(const QDir& crashDir, const InstanceInfo& instance) { stream << "\n===== Runtime Information =====\n"; stream << "Runtime Qt Version: " << qVersion() << '\n'; - stream << "Signal: " << strsignal(crashSignal) << " (" << crashSignal << ")\n"; // NOLINT stream << "Crashed process ID: " << crashProc << '\n'; stream << "Run ID: " << instance.instanceId << '\n'; stream << "Shell ID: " << instance.shellId << '\n'; stream << "Config Path: " << instance.configPath << '\n'; + stream << "\n===== Report Integrity =====\n"; + stream << "Minidump save status: " << dumpDupStatus << '\n'; + stream << "Log save status: " << logDupStatus << '\n'; + stream << "Binary copy status: " << copyBinStatus << '\n'; + stream << "\n===== System Information =====\n\n"; stream << "/etc/os-release:"; auto osReleaseFile = QFile("/etc/os-release"); @@ -206,18 +140,6 @@ void recordCrashInfo(const QDir& crashDir, const InstanceInfo& instance) { stream << "FAILED TO OPEN\n"; } - stream << "\n===== Stacktrace =====\n"; - if (stacktrace.empty()) { - stream << "(no trace available)\n"; - } else { - auto formatter = cpptrace::formatter().header(std::string()); - auto traceStr = formatter.format(stacktrace); - stream << QString::fromStdString(traceStr) << '\n'; - } - - stream << "\n===== Log Tail =====\n"; - stream << recentLogs; - extraInfoFile.close(); } } diff --git a/src/launch/launch.cpp b/src/launch/launch.cpp index ee7ca64..f269f61 100644 --- a/src/launch/launch.cpp +++ b/src/launch/launch.cpp @@ -27,7 +27,7 @@ #include "build.hpp" #include "launch_p.hpp" -#if CRASH_HANDLER +#if CRASH_REPORTER #include "../crash/handler.hpp" #endif @@ -137,12 +137,13 @@ int launch(const LaunchArgs& args, char** argv, QCoreApplication* coreApplicatio .display = getDisplayConnection(), }; -#if CRASH_HANDLER - crash::CrashHandler::init(); +#if CRASH_REPORTER + auto crashHandler = crash::CrashHandler(); + crashHandler.init(); { auto* log = LogManager::instance(); - crash::CrashHandler::setRelaunchInfo({ + crashHandler.setRelaunchInfo({ .instance = InstanceInfo::CURRENT, .noColor = !log->colorLogs, .timestamp = log->timestampLogs, diff --git a/src/launch/main.cpp b/src/launch/main.cpp index a324e09..7a801fc 100644 --- a/src/launch/main.cpp +++ b/src/launch/main.cpp @@ -16,7 +16,7 @@ #include "build.hpp" #include "launch_p.hpp" -#if CRASH_HANDLER +#if CRASH_REPORTER #include "../crash/main.hpp" #endif @@ -25,7 +25,7 @@ namespace qs::launch { namespace { void checkCrashRelaunch(char** argv, QCoreApplication* coreApplication) { -#if CRASH_HANDLER +#if CRASH_REPORTER auto lastInfoFdStr = qEnvironmentVariable("__QUICKSHELL_CRASH_INFO_FD"); if (!lastInfoFdStr.isEmpty()) { @@ -104,7 +104,7 @@ void exitDaemon(int code) { int main(int argc, char** argv) { QCoreApplication::setApplicationName("quickshell"); -#if CRASH_HANDLER +#if CRASH_REPORTER qsCheckCrash(argc, argv); #endif