diff --git a/.github/ISSUE_TEMPLATE/crash.yml b/.github/ISSUE_TEMPLATE/crash.yml index c8b4804..80fa827 100644 --- a/.github/ISSUE_TEMPLATE/crash.yml +++ b/.github/ISSUE_TEMPLATE/crash.yml @@ -1,4 +1,4 @@ -name: Crash Report +name: Crash Report (v1) description: Quickshell has crashed labels: ["bug", "crash"] body: diff --git a/.github/ISSUE_TEMPLATE/crash2.yml b/.github/ISSUE_TEMPLATE/crash2.yml new file mode 100644 index 0000000..84beef8 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/crash2.yml @@ -0,0 +1,49 @@ +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 8d19f58..7b8cbce 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -55,10 +55,11 @@ jobs: libpipewire \ cli11 \ polkit \ - jemalloc + jemalloc \ + libunwind \ + git # for cpptrace clone - name: Build - # breakpad is annoying to build in ci due to makepkg not running as root run: | - cmake -GNinja -B build -DCRASH_REPORTER=OFF + cmake -GNinja -B build -DVENDOR_CPPTRACE=ON cmake --build build diff --git a/BUILD.md b/BUILD.md index c9459b5..6a3f422 100644 --- a/BUILD.md +++ b/BUILD.md @@ -64,14 +64,18 @@ At least Qt 6.6 is required. All features are enabled by default and some have their own dependencies. -### Crash Reporter -The crash reporter catches crashes, restarts quickshell when it crashes, +### Crash Handler +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_REPORTER=OFF` +To disable: `-DCRASH_HANDLER=OFF` -Dependencies: `google-breakpad` (static library) +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. ### Jemalloc We recommend leaving Jemalloc enabled as it will mask memory fragmentation caused diff --git a/CMakeLists.txt b/CMakeLists.txt index 7633f4f..fabda0e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -47,12 +47,11 @@ 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 b9000c2..2083462 100644 --- a/changelog/next.md +++ b/changelog/next.md @@ -32,6 +32,7 @@ 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 @@ -52,5 +53,6 @@ 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). +- `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. diff --git a/default.nix b/default.nix index 7783774..59e68b0 100644 --- a/default.nix +++ b/default.nix @@ -10,7 +10,9 @@ ninja, spirv-tools, qt6, - breakpad, + cpptrace ? null, + libunwind, + libdwarf, jemalloc, cli11, wayland, @@ -49,6 +51,8 @@ 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"; @@ -74,7 +78,12 @@ cli11 ] ++ lib.optional withQtSvg qt6.qtsvg - ++ lib.optional withCrashReporter breakpad + ++ lib.optional withCrashHandler (cpptrace.overrideAttrs (prev: { + cmakeFlags = prev.cmakeFlags ++ [ + "-DCPPTRACE_UNWIND_WITH_LIBUNWIND=TRUE" + ]; + buildInputs = prev.buildInputs ++ [ libunwind ]; + })) ++ 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 ] @@ -91,7 +100,7 @@ (lib.cmakeFeature "INSTALL_QML_PREFIX" qt6.qtbase.qtQmlPrefix) (lib.cmakeBool "DISTRIBUTOR_DEBUGINFO_AVAILABLE" true) (lib.cmakeFeature "GIT_REVISION" gitRev) - (lib.cmakeBool "CRASH_REPORTER" withCrashReporter) + (lib.cmakeBool "CRASH_HANDLER" withCrashHandler) (lib.cmakeBool "USE_JEMALLOC" withJemalloc) (lib.cmakeBool "WAYLAND" withWayland) (lib.cmakeBool "SCREENCOPY" (libgbm != null)) diff --git a/quickshell.scm b/quickshell.scm index 3f82160..780bb96 100644 --- a/quickshell.scm +++ b/quickshell.scm @@ -56,8 +56,7 @@ #~(list "-GNinja" "-DDISTRIBUTOR=\"In-tree Guix channel\"" "-DDISTRIBUTOR_DEBUGINFO_AVAILABLE=NO" - ;; Breakpad is not currently packaged for Guix. - "-DCRASH_REPORTER=OFF") + "-DCRASH_HANDLER=OFF") #:phases #~(modify-phases %standard-phases (replace 'build (lambda _ (invoke "cmake" "--build" "."))) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index c95ecf7..4b13d45 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -12,7 +12,7 @@ add_subdirectory(io) add_subdirectory(widgets) add_subdirectory(ui) -if (CRASH_REPORTER) +if (CRASH_HANDLER) add_subdirectory(crash) endif() diff --git a/src/build/CMakeLists.txt b/src/build/CMakeLists.txt index bb35da9..62574d9 100644 --- a/src/build/CMakeLists.txt +++ b/src/build/CMakeLists.txt @@ -9,10 +9,10 @@ if (NOT DEFINED GIT_REVISION) ) endif() -if (CRASH_REPORTER) - set(CRASH_REPORTER_DEF 1) +if (CRASH_HANDLER) + set(CRASH_HANDLER_DEF 1) else() - set(CRASH_REPORTER_DEF 0) + set(CRASH_HANDLER_DEF 0) endif() if (DISTRIBUTOR_DEBUGINFO_AVAILABLE) diff --git a/src/build/build.hpp.in b/src/build/build.hpp.in index 66fb664..93e78a9 100644 --- a/src/build/build.hpp.in +++ b/src/build/build.hpp.in @@ -9,7 +9,7 @@ #define GIT_REVISION "@GIT_REVISION@" #define DISTRIBUTOR "@DISTRIBUTOR@" #define DISTRIBUTOR_DEBUGINFO_AVAILABLE @DEBUGINFO_AVAILABLE@ -#define CRASH_REPORTER @CRASH_REPORTER_DEF@ +#define CRASH_HANDLER @CRASH_HANDLER_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 d462f6e..977e4c2 100644 --- a/src/core/instanceinfo.hpp +++ b/src/core/instanceinfo.hpp @@ -35,6 +35,8 @@ 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 7fdd830..a891ee9 100644 --- a/src/crash/CMakeLists.txt +++ b/src/crash/CMakeLists.txt @@ -6,12 +6,51 @@ qt_add_library(quickshell-crash STATIC qs_pch(quickshell-crash SET large) -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) +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 () # quick linked for pch compat -target_link_libraries(quickshell-crash PRIVATE quickshell-build Qt::Quick Qt::Widgets) +target_link_libraries(quickshell-crash PRIVATE quickshell-build Qt::Quick Qt::Widgets cpptrace::cpptrace) target_link_libraries(quickshell PRIVATE quickshell-crash) diff --git a/src/crash/handler.cpp b/src/crash/handler.cpp index 0baa8e6..fd40f94 100644 --- a/src/crash/handler.cpp +++ b/src/crash/handler.cpp @@ -1,12 +1,12 @@ #include "handler.hpp" +#include #include +#include #include #include -#include -#include -#include -#include +#include +#include #include #include #include @@ -19,98 +19,60 @@ extern char** environ; // NOLINT -using namespace google_breakpad; - namespace qs::crash { namespace { + QS_LOGGING_CATEGORY(logCrashHandler, "quickshell.crashhandler", QtWarningMsg); -} -struct CrashHandlerPrivate { - ExceptionHandler* exceptionHandler = nullptr; - int minidumpFd = -1; - int infoFd = -1; +void writeEnvInt(char* buf, const char* name, int value) { + // NOLINTBEGIN (cppcoreguidelines-pro-bounds-pointer-arithmetic) + while (*name != '\0') *buf++ = *name++; + *buf++ = '='; - 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++ = '-'; + value = -value; } - 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."; + if (value == 0) { + *buf++ = '0'; + *buf = '\0'; return; } - QFile file; - - if (!file.open(this->d->infoFd, QFile::ReadWrite)) { - qCCritical( - logCrashHandler - ) << "Failed to open instance info memfd, crash recovery will not work."; + auto* start = buf; + while (value > 0) { + *buf++ = static_cast('0' + (value % 10)); + value /= 10; } - QDataStream ds(&file); - ds << info; - file.flush(); - - qCDebug(logCrashHandler) << "Stored instance info in memfd" << this->d->infoFd; + *buf = '\0'; + std::reverse(start, buf); + // NOLINTEND } -CrashHandler::~CrashHandler() { - delete this->d->exceptionHandler; - delete this->d; -} - -bool CrashHandlerPrivate::minidumpCallback( - const MinidumpDescriptor& /*descriptor*/, - void* context, - bool /*success*/ +void signalHandler( + int sig, + siginfo_t* /*info*/, // NOLINT (misc-include-cleaner) + void* /*context*/ ) { - // A fork that just dies to ensure the coredump is caught by the system. - auto coredumpPid = fork(); + if (CrashInfo::INSTANCE.traceFd != -1) { + auto traceBuffer = std::array(); + auto frameCount = cpptrace::safe_generate_raw_trace(traceBuffer.data(), traceBuffer.size(), 1); - if (coredumpPid == 0) { - return false; + for (size_t i = 0; i < static_cast(frameCount); i++) { + auto frame = cpptrace::safe_object_frame(); + cpptrace::get_safe_object_frame(traceBuffer[i], &frame); + write(CrashInfo::INSTANCE.traceFd, &frame, sizeof(cpptrace::safe_object_frame)); + } } - auto* self = static_cast(context); + auto coredumpPid = fork(); + if (coredumpPid == 0) { + raise(sig); + _exit(-1); + } auto exe = std::array(); if (readlink("/proc/self/exe", exe.data(), exe.size() - 1) == -1) { @@ -123,17 +85,19 @@ bool CrashHandlerPrivate::minidumpCallback( auto env = std::array(); auto envi = 0; - 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); + // dup to remove CLOEXEC + auto infoFdStr = std::array(); + writeEnvInt(infoFdStr.data(), "__QUICKSHELL_CRASH_INFO_FD", dup(CrashInfo::INSTANCE.infoFd)); env[envi++] = infoFdStr.data(); - auto corePidStr = std::array(); - memcpy(corePidStr.data(), "__QUICKSHELL_CRASH_DUMP_PID=-1" /*\0*/, 31); - if (coredumpPid != -1) my_uitos(&corePidStr[28], coredumpPid, 10); + auto corePidStr = std::array(); + writeEnvInt(corePidStr.data(), "__QUICKSHELL_CRASH_DUMP_PID", coredumpPid); 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) { @@ -145,30 +109,18 @@ bool CrashHandlerPrivate::minidumpCallback( 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"); - return false; + _exit(-1); } else if (pid == 0) { + // dup to remove CLOEXEC - // 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); + 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)); env[envi++] = dumpFdStr.data(); env[envi++] = logFdStr.data(); @@ -185,8 +137,83 @@ bool CrashHandlerPrivate::minidumpCallback( perror("Failed to relaunch quickshell.\n"); _exit(-1); } +} - return false; // should make sure it hits the system coredump handler +} // 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; } } // namespace qs::crash diff --git a/src/crash/handler.hpp b/src/crash/handler.hpp index 2a1d86f..9488d71 100644 --- a/src/crash/handler.hpp +++ b/src/crash/handler.hpp @@ -5,19 +5,10 @@ #include "../core/instanceinfo.hpp" namespace qs::crash { -struct CrashHandlerPrivate; - class CrashHandler { public: - explicit CrashHandler(); - ~CrashHandler(); - Q_DISABLE_COPY_MOVE(CrashHandler); - - void init(); - void setRelaunchInfo(const RelaunchInfo& info); - -private: - CrashHandlerPrivate* d; + static void init(); + static void setRelaunchInfo(const RelaunchInfo& info); }; } // namespace qs::crash diff --git a/src/crash/interface.cpp b/src/crash/interface.cpp index 326216a..a3422d3 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=crash.yml", + "https://github.com/quickshell-mirror/quickshell/issues/new?template=crash2.yml", this )); @@ -114,7 +114,7 @@ void CrashReporterGui::openFolder() { void CrashReporterGui::openReportUrl() { QDesktopServices::openUrl( - QUrl("https://github.com/outfoxxed/quickshell/issues/new?template=crash.yml") + QUrl("https://github.com/outfoxxed/quickshell/issues/new?template=crash2.yml") ); } diff --git a/src/crash/main.cpp b/src/crash/main.cpp index 6571660..c406ba6 100644 --- a/src/crash/main.cpp +++ b/src/crash/main.cpp @@ -1,7 +1,10 @@ #include "main.hpp" #include #include +#include +#include +#include #include #include #include @@ -13,13 +16,17 @@ #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" @@ -61,6 +68,76 @@ 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(); @@ -71,32 +148,25 @@ 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) << "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) << "Resolving stacktrace from fd" << dumpFd; + auto stacktrace = resolveStacktrace(dumpFd); - qCDebug(logCrashReporter) << "Saving log from fd" << logFd; - auto logDupStatus = tryDup(logFd, crashDir.filePath("log.qslog.log")); + 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")); 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("info.txt")); + auto extraInfoFile = QFile(crashDir.filePath("report.txt")); if (!extraInfoFile.open(QFile::WriteOnly)) { qCCritical(logCrashReporter) << "Failed to open crash info file for writing."; } else { @@ -111,16 +181,12 @@ 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"); @@ -140,6 +206,18 @@ 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 f269f61..ee7ca64 100644 --- a/src/launch/launch.cpp +++ b/src/launch/launch.cpp @@ -27,7 +27,7 @@ #include "build.hpp" #include "launch_p.hpp" -#if CRASH_REPORTER +#if CRASH_HANDLER #include "../crash/handler.hpp" #endif @@ -137,13 +137,12 @@ int launch(const LaunchArgs& args, char** argv, QCoreApplication* coreApplicatio .display = getDisplayConnection(), }; -#if CRASH_REPORTER - auto crashHandler = crash::CrashHandler(); - crashHandler.init(); +#if CRASH_HANDLER + crash::CrashHandler::init(); { auto* log = LogManager::instance(); - crashHandler.setRelaunchInfo({ + crash::CrashHandler::setRelaunchInfo({ .instance = InstanceInfo::CURRENT, .noColor = !log->colorLogs, .timestamp = log->timestampLogs, diff --git a/src/launch/main.cpp b/src/launch/main.cpp index 7a801fc..a324e09 100644 --- a/src/launch/main.cpp +++ b/src/launch/main.cpp @@ -16,7 +16,7 @@ #include "build.hpp" #include "launch_p.hpp" -#if CRASH_REPORTER +#if CRASH_HANDLER #include "../crash/main.hpp" #endif @@ -25,7 +25,7 @@ namespace qs::launch { namespace { void checkCrashRelaunch(char** argv, QCoreApplication* coreApplication) { -#if CRASH_REPORTER +#if CRASH_HANDLER 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_REPORTER +#if CRASH_HANDLER qsCheckCrash(argc, argv); #endif