3 Commits

Author	SHA1	Message	Date
ispyisail	f8c1b5206a	Address review on #905: offered-list semantics, dump ordering, FreeBSD, legacy dumps ... Five things raised in review, plus tests for the parts that were only described in prose. clearPendingCrashDump() did not do what its comment said. It called pendingCrashDumpFiles() again at clear time, so it deleted whatever was in the directory then, not what had been offered. The offer sits inside a modal dialog that stays open as long as the user reads it, and SingleApplication keys its socket on the binary path, so a second QElectroTech build running alongside is a separate process that can crash and write a dump in that window. Re-listing deleted that dump unseen -- the exact failure this change exists to fix. The list is now taken once in QETApp::checkCrashDump() and passed to both pendingCrashDumpContents() and clearPendingCrashDump(). The ring is now written before the backtrace. backtrace() unwinds through libgcc, which calls dl_iterate_phdr and takes the loader lock; warming it in install() removes the allocation but not the lock. Crashing inside dlopen() (Qt plugin loading), or on a corrupted stack, could therefore hang or re-fault the handler at the backtrace and lose the ring with it. Order is now header, signal, ring, backtrace, so the cheapest and most valuable part is already on disk before anything that can block. The class comment claimed the handler takes no locks; that was not strictly true and now says so. QET_CRASH_BACKTRACE comes from find_package(Backtrace) rather than __has_include(<execinfo.h>). The header exists on FreeBSD but backtrace() lives in libexecinfo there, so the probe compiled and the link failed. A crash_dump.log left by a pre-#905 version is migrated into crashes/ at startup, named from its own mtime. Otherwise upgrading stranded it: the new code never looks at that path, so the dump from the crash that prompted the upgrade would sit there unoffered forever. Also from the review: dumps are capped at the 10 newest, so a crash loop cannot fill the log directory before any dialog is shown; crashDumpDir() no longer creates the directory as a side effect of a const getter (ensureCrashDumpDir() does that for the callers that write); and redact() now masks an AppImage's per-run /tmp/.mount_XXXXXX prefix, which backtrace_symbols_fd() writes into every frame. Two test executables, both of which were checked to fail against the behaviour they replace: - tst_crashhandler covers CrashHandler::formatInt(), which had no coverage at all despite running only inside a signal handler, where nothing can assert: zero, negatives, INT_MIN (negated through unsigned, since -INT_MIN is UB), INT_MAX, truncation and a zero-sized buffer, each checked against a sentinel-filled buffer so a write past the reported length fails. - tst_crashdumps covers the bookkeeping: ordering, empty dumps, the exclusion of this run's own path, the cap, concatenation of every offered dump, that clearing deletes only what was offered, and what redact() masks. qetlogger.cpp needs exactly one symbol from the application, QETApp::dataDir(), which the test supplies itself. Not addressed here: the timestamp in crash_<timestamp>_<pid> is the launch time, not the crash time -- correct as observed, and the commit message that implied otherwise was the thing that was wrong. Resolvable QET frames for AppImage/Flatpak/Snap/Debian need -rdynamic and archived debug symbols, which is a packaging discussion, not this change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-18 14:31:49 +12:00
ispyisail	0646f9ca4f	Harden the crash reporter: keep every dump, and say what crashed ... Three weaknesses, all visible in ChuckNr11's report on #898 -- "the report appeared only once despite there being 10 or more crashes". One dump per run instead of one per install ------------------------------------------- crashDumpPath() was a single fixed crash_dump.log, and the handler opens it O_TRUNC, so each crash destroyed the evidence from the one before. Ten crashes left one dump. Dumps now go to a crashes/ directory named crash_<timestamp>_<pid>.log, and every pending one is offered together, newest first, with a banner saying how many there are. A crash that repeats is exactly the case where the earlier dumps matter, because the difference between them is the evidence. The name is built in normal context and handed to CrashHandler::install(), which copies it into a preallocated buffer as before -- the handler still writes to one fixed path, so its no-allocation invariant is untouched. The dump now says which signal fired ------------------------------------ The header is built once at install(), so every dump looked identical no matter what killed the process -- and SIGSEGV and SIGABRT point at very different bugs. Written with an async-signal-safe integer formatter into a stack buffer, since snprintf is not on the POSIX safe list. ...and where it was ------------------- The ring said what the program was doing; nothing said where it died. The dump now carries a backtrace. backtrace() is warmed once in install() so its first-call lazy resolution cannot allocate inside the handler, and backtrace_symbols_fd() writes straight to the fd -- unlike backtrace_symbols(), which mallocs and must never be used here. Guarded on __has_include(<execinfo.h>) so platforms without it are unaffected. QET's own frames currently resolve as offsets rather than names, since the binary does not export its dynamic symbols. They are still resolvable offline: the header records the exact git SHA. Building with -rdynamic would give names directly, but that is a build-flag decision for its own change. Deliberately unchanged: the four invariants in crashhandler.h. Nothing added here allocates, blocks, takes a lock, or swallows the crash. Verified: three consecutive SIGSEGVs now leave three separate dumps, each carrying "Signal: 11" and a backtrace with resolved Qt frames; launching afterwards offers all three in one dialog, newest first, and clears them once shown. ctest 8/8. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-17 22:44:59 +12:00
ispyisail	5dec36cb29	Add crash-time ring flush and a diagnostics export UI (discussion #644, steps 4-5) ... Stacked on the steps 1-3 branch (feature-diagnostic-logging, PR #646). Kept as its own PR rather than folded into that one, matching the discussion's own framing: step 4 is explicitly "the highest-risk piece ... lands last, behind its own switch." ## Step 4 -- crash-time ring flush (CrashHandler) Installs a handler for SIGSEGV/SIGABRT/SIGBUS/SIGFPE/SIGILL (POSIX) / SetUnhandledExceptionFilter (Windows) that flushes the in-memory ring to a fixed crash_dump.log before the process dies. This required reworking LogRing (step 3) to be genuinely lock-free, not just mutex-protected: a signal handler that blocks on a lock the crashing thread (or another thread) already holds turns a clean crash into a hang -- no ring dump *and* no core dump, worse than doing nothing. append() now claims a slot with a single atomic fetch-add; dumpToFd() reads the preallocated entries directly and writes them with write(2) only, looping on EINTR/short writes. Accepted tradeoff: at most one entry can be read torn if a crash lands mid-append into that exact slot -- documented in logring.h, and the alternative (a seqlock to detect and retry) wasn't judged worth the complexity for that window. Other invariants implemented per the discussion: - sigaltstack with a static 64 KiB buffer, SA_ONSTACK -- a stack- overflow SIGSEGV has no usable stack for a handler without one. - Nothing under the actual handler touches Qt, QString or the allocator: the dump path and a small header (version/git/OS/Qt) are precomputed into fixed char buffers by install(), which runs once at startup in normal context. - Atomic test-and-set so only the first crash writes a dump; a second concurrent/nested fault goes straight to restore-and-re-raise. - After writing, the handler restores SIG_DFL and re-raises (POSIX) / returns EXCEPTION_CONTINUE_SEARCH (Windows) so the OS's own crash path -- core dump, Windows Error Reporting -- still runs. A handler that "fixed" the crash by swallowing the signal would destroy exactly the post-mortem evidence this whole design exists to preserve. Tested in this environment: POSIX/Linux only, all five signals. Sent each directly to a running process and confirmed (a) crash_dump.log is written with the correct header and ring contents, mode 0600, and (b) the process still terminates via the signal with the kernel's own "core dumped" flag set (exit code 128+signal, confirmed for all five). The Windows path is implemented per the discussion's guidance but is untested -- no Windows build available in this sandbox. ## Step 5 -- getting the data back out - QETApp::checkCrashDump(), called from checkBackupFiles() only when there's no stale project file to recover this run (so the two prompts never both show, per the discussion), offers an unretrieved crash dump via DiagnosticsReportDialog and then deletes it regardless of the user's choice -- offered exactly once. - A new "Aide > Enregistrer un rapport de diagnostic..." action (QETMainWindow) builds the same kind of report from the *current* session (QetLogger::buildDiagnosticsReport(): header + this session's log file) for a manual "attach this to a bug report" flow, not tied to a crash. - Both go through QetLogger::redact() before ever reaching the user: the one redaction implemented is a literal replace of the home directory with "~", since an absolute path under it leaks the account name. The discussion's fancier "optionally redact project filenames too" isn't attempted -- reliably telling a project path apart from arbitrary log text is a much fuzzier problem than a literal prefix match. - DiagnosticsReportDialog shows the full (already-redacted) content before saving, per the discussion: "the user is about to attach this to a public tracker." Verified in a real GUI session (Xvfb): triggered a SIGSEGV, relaunched, confirmed the crash-report dialog appears with the right header/content, confirmed it does not reappear on a second relaunch, and confirmed the manual "Save report" action produces a correctly-formatted report and saves it to a chosen path. Built clean, no new warnings. ## Build systems Registered in both: cmake/qet_compilation_vars.cmake, and qelectrotech.pro. The .pro needed explicit globs for the new sources/logging/ui/ subfolder -- sources/logging/*.{h,cpp} was already globbed, but unlike the other ui/ subfolders that one had no entry of its own, so diagnosticsreportdialog.{h,cpp} would not have been built under qmake.	2026-08-06 23:04:27 +12:00