Files
qelectrotech-source-mirror/tests/qttest/tst_crashhandler.cpp
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

150 lines
4.4 KiB
C++

/*
Copyright 2006-2026 The QElectroTech Team
This file is part of QElectroTech.
QElectroTech is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation, either version 2 of the License, or
(at your option) any later version.
QElectroTech is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.
You should have received a copy of the GNU General Public License
along with QElectroTech. If not, see <http://www.gnu.org/licenses/>.
*/
#include "logging/crashhandler.h"
#include <QTest>
#include <climits>
#include <csignal>
#include <cstring>
/**
@brief The tst_CrashHandler class
Covers CrashHandler::formatInt(), the decimal formatter the signal
handler uses to write "Signal: 11" into a crash dump.
It is worth a test out of proportion to its size. It cannot use
snprintf(), which is not on the POSIX async-signal-safe list, so it is
hand-rolled; it runs only inside a signal handler, where nothing can
assert and a fault produces no diagnostic; and it is exercised only
when the application is already crashing, so a defect here would
corrupt or truncate exactly the dumps that matter and would never be
noticed in ordinary use.
The negation is the interesting part: -value on INT_MIN is undefined
behaviour, so the implementation goes through unsigned.
*/
class tst_CrashHandler : public QObject
{
Q_OBJECT
private slots:
void formatsZero();
void formatsPositive();
void formatsTheHandledSignals();
void formatsNegative();
void formatsIntMinWithoutOverflow();
void formatsIntMax();
void truncatesRatherThanOverflowing();
void writesNothingWhenThereIsNoRoom();
private:
/// Formats into a buffer poisoned with a sentinel, and fails if
/// anything past the returned length was touched.
static QByteArray format(int value, int size = 32);
};
QByteArray tst_CrashHandler::format(int value, int size)
{
char buffer[64];
memset(buffer, '\xAB', sizeof(buffer));
const int len = CrashHandler::formatInt(buffer, size, value);
// Nothing may be written past what was reported, nor past `size`.
for (int i = qMax(len, 0) ; i < static_cast<int>(sizeof(buffer)) ; ++i) {
if (buffer[i] != '\xAB') {
return QByteArray("WROTE PAST END at ") + QByteArray::number(i);
}
}
if (len < 0 || len > size) {
return QByteArray("BAD LENGTH ") + QByteArray::number(len);
}
return QByteArray(buffer, len);
}
void tst_CrashHandler::formatsZero()
{
QCOMPARE(format(0), QByteArray("0"));
}
void tst_CrashHandler::formatsPositive()
{
QCOMPARE(format(1), QByteArray("1"));
QCOMPARE(format(9), QByteArray("9"));
QCOMPARE(format(10), QByteArray("10"));
QCOMPARE(format(1234567), QByteArray("1234567"));
}
/**
The values this actually sees in the field: kHandledSignals, as
written into the "Signal: N" line of every dump.
*/
void tst_CrashHandler::formatsTheHandledSignals()
{
QCOMPARE(format(SIGSEGV), QByteArray::number(SIGSEGV));
QCOMPARE(format(SIGABRT), QByteArray::number(SIGABRT));
QCOMPARE(format(SIGBUS), QByteArray::number(SIGBUS));
QCOMPARE(format(SIGFPE), QByteArray::number(SIGFPE));
QCOMPARE(format(SIGILL), QByteArray::number(SIGILL));
}
void tst_CrashHandler::formatsNegative()
{
QCOMPARE(format(-1), QByteArray("-1"));
QCOMPARE(format(-42), QByteArray("-42"));
}
/**
-INT_MIN is undefined behaviour; the implementation negates through
unsigned instead. A build that got this wrong would either trap under
-ftrapv/UBSan or silently print the wrong number.
*/
void tst_CrashHandler::formatsIntMinWithoutOverflow()
{
QCOMPARE(format(INT_MIN), QByteArray::number(INT_MIN));
}
void tst_CrashHandler::formatsIntMax()
{
QCOMPARE(format(INT_MAX), QByteArray::number(INT_MAX));
}
/**
A buffer too small must be filled and stopped at, never run past --
the handler passes a fixed 64-byte stack buffer and subtracts what it
has already used.
*/
void tst_CrashHandler::truncatesRatherThanOverflowing()
{
QCOMPARE(format(12345, 3), QByteArray("123"));
QCOMPARE(format(-12345, 3), QByteArray("-12"));
QCOMPARE(format(7, 1), QByteArray("7"));
}
void tst_CrashHandler::writesNothingWhenThereIsNoRoom()
{
QCOMPARE(format(123, 0), QByteArray());
QCOMPARE(format(0, 0), QByteArray());
QCOMPARE(format(-5, 0), QByteArray());
}
QTEST_APPLESS_MAIN(tst_CrashHandler)
#include "tst_crashhandler.moc"