diff --git a/sources/logging/crashhandler.cpp b/sources/logging/crashhandler.cpp index 1a66e3a0d..860dc9b88 100644 --- a/sources/logging/crashhandler.cpp +++ b/sources/logging/crashhandler.cpp @@ -36,6 +36,10 @@ #include #include #include +#if __has_include() +#include +#define QET_CRASH_BACKTRACE 1 +#endif #endif namespace { @@ -64,6 +68,39 @@ char g_altstack[65536]; const int kHandledSignals[] = {SIGSEGV, SIGABRT, SIGBUS, SIGFPE, SIGILL}; +#ifdef QET_CRASH_BACKTRACE +// Preallocated here for the same reason as everything else in this block: +// backtrace() fills a caller-supplied array, so it needs no heap of its +// own, and backtrace_symbols_fd() writes straight to the fd (unlike +// backtrace_symbols(), which mallocs and is therefore unusable here). +void *g_backtrace_frames[64]; +#endif + +// Async-signal-safe decimal formatting: write() takes a buffer, and there +// is no snprintf on the POSIX async-signal-safe list. Writes into a +// caller-owned buffer (stack, not heap) and returns the length used. +int formatInt(char *buffer, int size, int value) +{ + if (size <= 0) return 0; + if (value == 0) { + buffer[0] = '0'; + return 1; + } + char scratch[16]; + int n = 0; + bool negative = value < 0; + unsigned int v = negative ? static_cast(-(value + 1)) + 1u + : static_cast(value); + while (v > 0 && n < static_cast(sizeof(scratch))) { + scratch[n++] = static_cast('0' + (v % 10)); + v /= 10; + } + int len = 0; + if (negative && len < size) buffer[len++] = '-'; + while (n > 0 && len < size) buffer[len++] = scratch[--n]; + return len; +} + void restoreDefaultAndReraise(int sig) { struct sigaction sa {}; @@ -85,13 +122,46 @@ void signalHandler(int sig) return; } - // open/write/close are all on the POSIX async-signal-safe function - // list; nothing else is called here. + // open/write/close, and backtrace_symbols_fd, are all on the POSIX + // async-signal-safe function list; nothing else is called here. const int fd = ::open(g_dump_path, O_WRONLY | O_CREAT | O_TRUNC, 0600); if (fd >= 0) { if (g_header_len > 0) { ::write(fd, g_header, static_cast(g_header_len)); } + + // Which signal killed it. The header is built once at install() + // and is therefore identical for every crash, so without this the + // dump never said what actually happened -- SIGSEGV and SIGABRT + // point at very different bugs. + char line[64]; + int len = 0; + const char kSignalLabel[] = "Signal: "; + for (unsigned i = 0 ; i < sizeof(kSignalLabel) - 1 ; ++i) { + line[len++] = kSignalLabel[i]; + } + len += formatInt(line + len, static_cast(sizeof(line)) - len - 1, sig); + line[len++] = '\n'; + ::write(fd, line, static_cast(len)); + +#ifdef QET_CRASH_BACKTRACE + // The log ring says what the program was doing; this says where it + // was when it died. backtrace() is warmed in install() so its + // first-call lazy resolution cannot allocate here, and + // backtrace_symbols_fd() writes to the fd without allocating -- + // unlike backtrace_symbols(), which mallocs and must not be used. + const char kBacktraceLabel[] = "--- backtrace ---\n"; + ::write(fd, kBacktraceLabel, sizeof(kBacktraceLabel) - 1); + const int frames = ::backtrace(g_backtrace_frames, + static_cast(sizeof(g_backtrace_frames) + / sizeof(g_backtrace_frames[0]))); + if (frames > 0) { + ::backtrace_symbols_fd(g_backtrace_frames, frames, fd); + } + const char kRingLabel[] = "--- log ---\n"; + ::write(fd, kRingLabel, sizeof(kRingLabel) - 1); +#endif + if (g_ring) { g_ring->dumpToFd(fd); } @@ -159,6 +229,15 @@ void CrashHandler::install(const LogRing *ring, const QString &dump_path) ss.ss_flags = 0; sigaltstack(&ss, nullptr); +#ifdef QET_CRASH_BACKTRACE + // Warm the unwinder. backtrace()'s *first* call resolves dynamic + // linker state and may allocate; every call after that does not. Doing + // it here, in normal context, is what lets the handler call it without + // breaking invariant 2. The result is deliberately discarded. + void *warmup[4]; + (void) ::backtrace(warmup, 4); +#endif + struct sigaction sa {}; sa.sa_handler = signalHandler; sigemptyset(&sa.sa_mask); diff --git a/sources/logging/qetlogger.cpp b/sources/logging/qetlogger.cpp index 88bf4d4d9..610a4cc21 100644 --- a/sources/logging/qetlogger.cpp +++ b/sources/logging/qetlogger.cpp @@ -21,6 +21,7 @@ #include "../qetapp.h" #include "../qetversion.h" +#include #include #include #include @@ -104,12 +105,81 @@ void QetLogger::installCrashHandler() if (m_disabled) { return; } - CrashHandler::install(&m_ring, crashDumpPath()); + //Fixed for the life of the process: the handler copies it into a + //preallocated buffer, and pendingCrashDumpFiles() needs to know + //which file is this run's own so it doesn't offer it back. + m_crash_dump_path = buildCrashDumpPath(); + CrashHandler::install(&m_ring, m_crash_dump_path); } -QString QetLogger::crashDumpPath() const +/** + @brief QetLogger::crashDumpDir + @return the directory holding crash dumps, created if missing. + + A directory rather than a single file, because dumps are per-run and + several can be waiting at once. +*/ +QString QetLogger::crashDumpDir() const { - return m_log_dir % QStringLiteral("/crash_dump.log"); + const QString dir = m_log_dir % QStringLiteral("/crashes"); + QDir().mkpath(dir); + return dir; +} + +/** + @brief QetLogger::buildCrashDumpPath + @return where this run would write a crash dump. + + One file per run, rather than a single fixed crash_dump.log. That old + scheme opened one path with O_TRUNC, so a second crash overwrote the + first: someone who crashed ten times still ended up with exactly one + dump, the most recent. Reported on #898 -- "the report appeared only + once despite there being 10 or more crashes" -- where losing the + earlier dumps mattered as much as never being shown them. + + Built here in normal context and handed to CrashHandler::install(), + which copies it into a preallocated buffer, so the handler still + writes to one fixed path and its no-allocation invariant is untouched. +*/ +QString QetLogger::buildCrashDumpPath() const +{ + return crashDumpDir() + % QStringLiteral("/crash_") + % QDateTime::currentDateTime().toString(QStringLiteral("yyyyMMdd-hhmmss")) + % QStringLiteral("_") + % QString::number(QCoreApplication::applicationPid()) + % QStringLiteral(".log"); +} + +/** + @brief QetLogger::pendingCrashDumpFiles + @return dumps left by previous runs, newest first. + + This run's own path is excluded: it does not exist yet unless this run + is itself crashing, and a handler mid-crash is in no position to be + offered a dialog. +*/ +QStringList QetLogger::pendingCrashDumpFiles() const +{ + QDir dir(crashDumpDir()); + dir.setNameFilters({QStringLiteral("crash_*.log")}); + dir.setFilter(QDir::Files); + dir.setSorting(QDir::Time); + + QStringList files; + const QFileInfoList entries = dir.entryInfoList(); + for (const QFileInfo &info : entries) + { + if (info.size() <= 0) { + continue; + } + if (!m_crash_dump_path.isEmpty() + && info.absoluteFilePath() == QFileInfo(m_crash_dump_path).absoluteFilePath()) { + continue; + } + files << info.absoluteFilePath(); + } + return files; } QString QetLogger::currentLogFilePath() const @@ -362,22 +432,62 @@ bool QetLogger::hasPendingCrashDump() const if (m_disabled) { return false; } - const QFileInfo info(crashDumpPath()); - return info.exists() && info.isFile() && info.size() > 0; + return !pendingCrashDumpFiles().isEmpty(); } +/** + @brief QetLogger::pendingCrashDumpContents + @return every pending dump, newest first, concatenated. + + All of them rather than only the latest: a crash that repeats is the + case where the earlier dumps are most worth having, since the + difference between them is the evidence. They are separated by a + banner so a reader can tell where one ends and the next begins, and + the whole thing is redacted as a single pass. +*/ QByteArray QetLogger::pendingCrashDumpContents() const { - QFile file(crashDumpPath()); - if (!file.open(QIODevice::ReadOnly)) { + const QStringList files = pendingCrashDumpFiles(); + if (files.isEmpty()) { return QByteArray(); } - return redact(file.readAll()); + + QByteArray all; + if (files.size() > 1) { + all += QByteArray("QET: ") + QByteArray::number(files.size()) + + " crash dumps pending, newest first.\n\n"; + } + + for (const QString &path : files) + { + QFile file(path); + if (!file.open(QIODevice::ReadOnly)) { + continue; + } + all += "===== " + QFileInfo(path).fileName().toUtf8() + " =====\n"; + all += file.readAll(); + if (!all.endsWith('\n')) { + all += '\n'; + } + all += '\n'; + } + + return redact(all); } +/** + @brief QetLogger::clearPendingCrashDump + Drop the dumps that have just been offered. + + Only those: a dump written by a run that crashed after this list was + taken would otherwise be deleted without ever being seen. +*/ void QetLogger::clearPendingCrashDump() { - QFile::remove(crashDumpPath()); + const QStringList files = pendingCrashDumpFiles(); + for (const QString &path : files) { + QFile::remove(path); + } } QByteArray QetLogger::buildDiagnosticsReport() const diff --git a/sources/logging/qetlogger.h b/sources/logging/qetlogger.h index a4596096a..a72c9a0e6 100644 --- a/sources/logging/qetlogger.h +++ b/sources/logging/qetlogger.h @@ -136,7 +136,9 @@ class QetLogger void rotateLocked(); void writeToFile(const QByteArray &line, QtMsgType type); QString rotatedPath(int index) const; - QString crashDumpPath() const; + QString crashDumpDir() const; + QString buildCrashDumpPath() const; + QStringList pendingCrashDumpFiles() const; QString currentLogFilePath() const; static QByteArray sanitize(const QByteArray &input); @@ -147,6 +149,10 @@ class QetLogger QString m_log_dir; QString m_base_name; // e.g. "20260803", resolved once in init() + /// This run's own dump path, fixed at installCrashHandler(): + /// the handler writes here, and it is excluded when collecting + /// dumps left by previous runs. + QString m_crash_dump_path; QMutex m_file_mutex; QFile m_file;