Script 'mail_helper' called by obssrc Hello community, here is the log from the commit of package kcrash for openSUSE:Factory checked in at 2021-06-16 20:33:46 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ Comparing /work/SRC/openSUSE:Factory/kcrash (Old) and /work/SRC/openSUSE:Factory/.kcrash.new.32437 (New) ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Package is "kcrash" Wed Jun 16 20:33:46 2021 rev:92 rq:899721 version:5.83.0 Changes: -------- --- /work/SRC/openSUSE:Factory/kcrash/kcrash.changes 2021-05-10 15:36:54.162151052 +0200 +++ /work/SRC/openSUSE:Factory/.kcrash.new.32437/kcrash.changes 2021-06-16 20:35:15.483162890 +0200 @@ -1,0 +2,15 @@ +Sat Jun 5 11:58:38 UTC 2021 - Christophe Giboudeaux <[email protected]> + +- Update to 5.83.0 + * New feature release + * For more details please see: + * https://kde.org/announcements/frameworks/5/5.83.0 +- Changes since 5.82.0: + * Use target_sources() to add sources to targets + * Add missing include <cerrno> + * write metadata to a cache file when applicable + * Bump required CMake version to 3.16 + * Pass Bugzilla product name to DrKonqi when explicitly set in the app + * init rlp struct and don't cast rlim_t to int + +------------------------------------------------------------------- Old: ---- kcrash-5.82.0.tar.xz kcrash-5.82.0.tar.xz.sig New: ---- kcrash-5.83.0.tar.xz kcrash-5.83.0.tar.xz.sig ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ Other differences: ------------------ ++++++ kcrash.spec ++++++ --- /var/tmp/diff_new_pack.NDBE3A/_old 2021-06-16 20:35:16.327164330 +0200 +++ /var/tmp/diff_new_pack.NDBE3A/_new 2021-06-16 20:35:16.331164337 +0200 @@ -17,7 +17,7 @@ %define lname libKF5Crash5 -%define _tar_path 5.82 +%define _tar_path 5.83 # Full KF5 version (e.g. 5.33.0) %{!?_kf5_version: %global _kf5_version %{version}} # Last major and minor KF5 version (e.g. 5.33) @@ -25,7 +25,7 @@ # Only needed for the package signature condition %bcond_without lang Name: kcrash -Version: 5.82.0 +Version: 5.83.0 Release: 0 Summary: An application crash handler License: LGPL-2.1-or-later ++++++ kcrash-5.82.0.tar.xz -> kcrash-5.83.0.tar.xz ++++++ diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/kcrash-5.82.0/.gitignore new/kcrash-5.83.0/.gitignore --- old/kcrash-5.82.0/.gitignore 2021-05-01 11:39:00.000000000 +0200 +++ new/kcrash-5.83.0/.gitignore 2021-06-05 10:52:09.000000000 +0200 @@ -25,3 +25,4 @@ .clangd .idea /cmake-build* +.cache diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/kcrash-5.82.0/CMakeLists.txt new/kcrash-5.83.0/CMakeLists.txt --- old/kcrash-5.82.0/CMakeLists.txt 2021-05-01 11:39:00.000000000 +0200 +++ new/kcrash-5.83.0/CMakeLists.txt 2021-06-05 10:52:09.000000000 +0200 @@ -1,11 +1,11 @@ -cmake_minimum_required(VERSION 3.5) +cmake_minimum_required(VERSION 3.16) -set(KF_VERSION "5.82.0") # handled by release scripts -set(KF_DEP_VERSION "5.82.0") # handled by release scripts +set(KF_VERSION "5.83.0") # handled by release scripts +set(KF_DEP_VERSION "5.83.0") # handled by release scripts project(KCrash VERSION ${KF_VERSION}) include(FeatureSummary) -find_package(ECM 5.82.0 NO_MODULE) +find_package(ECM 5.83.0 NO_MODULE) set_package_properties(ECM PROPERTIES TYPE REQUIRED DESCRIPTION "Extra CMake Modules." URL "https://commits.kde.org/extra-cmake-modules";) feature_summary(WHAT REQUIRED_PACKAGES_NOT_FOUND FATAL_ON_MISSING_REQUIRED_PACKAGES) @@ -52,15 +52,8 @@ KCRASH_CORE_PATTERN_RAISE "Raising signals to kernel core patterns (iff the pattern is a process). You may wish to not install drkonqi if this can cause a UI conflict.") -if (EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/po) - include(ECMPoQmTools) - ecm_install_po_files_as_qm(po) -endif() - - add_definitions(-DQT_DISABLE_DEPRECATED_BEFORE=0x050f00) add_definitions(-DKF_DISABLE_DEPRECATED_BEFORE_AND_AT=0x055100) -add_definitions(-DQT_NO_FOREACH) add_subdirectory(src) if (BUILD_TESTING) add_subdirectory(autotests) diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/kcrash-5.82.0/autotests/CMakeLists.txt new/kcrash-5.83.0/autotests/CMakeLists.txt --- old/kcrash-5.82.0/autotests/CMakeLists.txt 2021-05-01 11:39:00.000000000 +0200 +++ new/kcrash-5.83.0/autotests/CMakeLists.txt 2021-06-05 10:52:09.000000000 +0200 @@ -38,3 +38,8 @@ coreconfigtest.cpp LINK_LIBRARIES Qt5::Core Qt5::Test ) + +ecm_add_tests( + metadatatest.cpp + LINK_LIBRARIES Qt5::Core Qt5::Test +) diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/kcrash-5.82.0/autotests/core_patterns/exec-apport new/kcrash-5.83.0/autotests/core_patterns/exec-apport --- old/kcrash-5.82.0/autotests/core_patterns/exec-apport 1970-01-01 01:00:00.000000000 +0100 +++ new/kcrash-5.83.0/autotests/core_patterns/exec-apport 2021-06-05 10:52:09.000000000 +0200 @@ -0,0 +1 @@ +|/usr/share/apport/apport %p %s %c %d %P %E diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/kcrash-5.82.0/autotests/core_patterns/exec-apport.license new/kcrash-5.83.0/autotests/core_patterns/exec-apport.license --- old/kcrash-5.82.0/autotests/core_patterns/exec-apport.license 1970-01-01 01:00:00.000000000 +0100 +++ new/kcrash-5.83.0/autotests/core_patterns/exec-apport.license 2021-06-05 10:52:09.000000000 +0200 @@ -0,0 +1,2 @@ +SPDX-License-Identifier: CC0-1.0 +SPDX-FileCopyrightText: none diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/kcrash-5.82.0/autotests/coreconfigtest.cpp new/kcrash-5.83.0/autotests/coreconfigtest.cpp --- old/kcrash-5.82.0/autotests/coreconfigtest.cpp 2021-05-01 11:39:00.000000000 +0200 +++ new/kcrash-5.83.0/autotests/coreconfigtest.cpp 2021-06-05 10:52:09.000000000 +0200 @@ -1,5 +1,5 @@ /* - SPDX-FileCopyrightText: 2016 Harald Sitter <[email protected]> + SPDX-FileCopyrightText: 2016-2021 Harald Sitter <[email protected]> SPDX-License-Identifier: LGPL-2.0-or-later */ @@ -21,11 +21,24 @@ KCrash::CoreConfig c(QFINDTESTDATA("core_patterns/exec")); #ifdef KCRASH_CORE_PATTERN_RAISE QCOMPARE(c.isProcess(), true); + QCOMPARE(c.isCoredumpd(), true); #else QCOMPARE(c.isProcess(), false); + QCOMPARE(c.isCoredumpd(), false); #endif } + void testExecNot() + { +#ifndef KCRASH_CORE_PATTERN_RAISE + QSKIP("Not useful when opting out of re-raising.") +#endif + + KCrash::CoreConfig c(QFINDTESTDATA("core_patterns/exec-apport")); + QCOMPARE(c.isProcess(), true); + QCOMPARE(c.isCoredumpd(), false); + } + void testNoFile() { KCrash::CoreConfig c("/meow/kitteh/meow"); diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/kcrash-5.82.0/autotests/metadatatest.cpp new/kcrash-5.83.0/autotests/metadatatest.cpp --- old/kcrash-5.82.0/autotests/metadatatest.cpp 1970-01-01 01:00:00.000000000 +0100 +++ new/kcrash-5.83.0/autotests/metadatatest.cpp 2021-06-05 10:52:09.000000000 +0200 @@ -0,0 +1,63 @@ +/* + SPDX-License-Identifier: LGPL-2.0-or-later + SPDX-FileCopyrightText: 2021 Harald Sitter <[email protected]> +*/ + +#include <QTest> + +#include "../src/metadata.cpp" + +using namespace KCrash; + +class MetadataTest : public QObject +{ + Q_OBJECT +private Q_SLOTS: + void testEverything() + { + QTemporaryDir tmpDir; + QVERIFY(tmpDir.isValid()); + + const QString iniFile = QStringLiteral("%1/foo.ini").arg(tmpDir.path()); + MetadataWriter *writer = nullptr; +#ifdef Q_OS_LINUX + MetadataINIWriter iniWriter(iniFile.toLocal8Bit()); + writer = &iniWriter; +#endif + Metadata data("BEFEHL", writer); + data.add("--ABC", "FOO"); + data.addBool("--Meow"); + data.close(); + const int argc = data.argc; + QCOMPARE(argc, 4); + QCOMPARE(data.argv.at(0), QStringLiteral("BEFEHL")); // make sure we do stringy comparision + QCOMPARE(data.argv.at(1), QStringLiteral("--ABC")); + QCOMPARE(data.argv.at(2), QStringLiteral("FOO")); + QCOMPARE(data.argv.at(3), QStringLiteral("--Meow")); + QCOMPARE(data.argv.at(4), nullptr); // list should be null terminated + +#ifdef Q_OS_LINUX + QFile::exists(iniFile); + QFile ini(iniFile); + QVERIFY(ini.open(QFile::ReadOnly)); + QCOMPARE(ini.readLine(), "[KCrash]\n"); + QCOMPARE(ini.readLine(), "ABC=FOO\n"); + QCOMPARE(ini.readLine(), "Meow=true\n"); + QVERIFY(ini.atEnd()); // nothing after final newline +#endif + } + + void testNoFile() + { + // Doesn't explode without writer + Metadata data("BEFEHL", nullptr); + data.add("--ABC", "FOO"); + data.close(); + const int argc = data.argc; + QCOMPARE(argc, 3); + } +}; + +QTEST_MAIN(MetadataTest) + +#include "metadatatest.moc" diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/kcrash-5.82.0/src/CMakeLists.txt new/kcrash-5.83.0/src/CMakeLists.txt --- old/kcrash-5.82.0/src/CMakeLists.txt 2021-05-01 11:39:00.000000000 +0200 +++ new/kcrash-5.83.0/src/CMakeLists.txt 2021-06-05 10:52:09.000000000 +0200 @@ -1,8 +1,10 @@ +add_library(KF5Crash) +add_library(KF5::Crash ALIAS KF5Crash) -set(kcrash_SRCS +target_sources(KF5Crash PRIVATE kcrash.cpp coreconfig.cpp - ${kcrash_QM_LOADER} + metadata.cpp ) configure_file( @@ -18,9 +20,7 @@ EXPORT KCRASH ) -add_library(KF5Crash ${kcrash_SRCS}) generate_export_header(KF5Crash BASE_NAME KCrash) -add_library(KF5::Crash ALIAS KF5Crash) target_include_directories(KF5Crash INTERFACE "$<INSTALL_INTERFACE:${KDE_INSTALL_INCLUDEDIR_KF5}/KCrash>") @@ -42,7 +42,7 @@ target_include_directories(KF5Crash PRIVATE ${X11_X11_INCLUDE_PATH}) endif() -set_target_properties(KF5Crash PROPERTIES VERSION ${KCrash_VERSION_STRING} +set_target_properties(KF5Crash PROPERTIES VERSION ${KCrash_VERSION} SOVERSION ${KCrash_SOVERSION} EXPORT_NAME Crash ) diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/kcrash-5.82.0/src/coreconfig.cpp new/kcrash-5.83.0/src/coreconfig.cpp --- old/kcrash-5.82.0/src/coreconfig.cpp 2021-05-01 11:39:00.000000000 +0200 +++ new/kcrash-5.83.0/src/coreconfig.cpp 2021-06-05 10:52:09.000000000 +0200 @@ -1,5 +1,5 @@ /* - SPDX-FileCopyrightText: 2016 Harald Sitter <[email protected]> + SPDX-FileCopyrightText: 2016-2021 Harald Sitter <[email protected]> SPDX-License-Identifier: LGPL-2.0-or-later */ @@ -9,7 +9,6 @@ #include <QFile> #include <config-kcrash.h> - namespace KCrash { CoreConfig::CoreConfig(const QString &path) @@ -27,6 +26,10 @@ } m_supported = true; m_process = first == '|'; + + if (file.readLine().contains(QByteArrayLiteral("systemd-coredump"))) { + m_coredumpd = true; + } } bool CoreConfig::isProcess() const @@ -34,4 +37,9 @@ return m_supported && m_process; } +bool CoreConfig::isCoredumpd() const +{ + return m_coredumpd; +} + } // namespace KCrash diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/kcrash-5.82.0/src/coreconfig_p.h new/kcrash-5.83.0/src/coreconfig_p.h --- old/kcrash-5.82.0/src/coreconfig_p.h 2021-05-01 11:39:00.000000000 +0200 +++ new/kcrash-5.83.0/src/coreconfig_p.h 2021-06-05 10:52:09.000000000 +0200 @@ -1,5 +1,5 @@ /* - SPDX-FileCopyrightText: 2016 Harald Sitter <[email protected]> + SPDX-FileCopyrightText: 2016-2021 Harald Sitter <[email protected]> SPDX-License-Identifier: LGPL-2.0-or-later */ @@ -17,10 +17,13 @@ CoreConfig(const QString &path = QStringLiteral("/proc/sys/kernel/core_pattern")); bool isProcess() const; + // should this need expansion please refactor to enum. could also store cmdline and compare in kcrash.cpp + bool isCoredumpd() const; private: bool m_supported = false; bool m_process = false; + bool m_coredumpd = false; }; } // namespace KCrash diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/kcrash-5.82.0/src/kcrash.cpp new/kcrash-5.83.0/src/kcrash.cpp --- old/kcrash-5.82.0/src/kcrash.cpp 2021-05-01 11:39:00.000000000 +0200 +++ new/kcrash-5.83.0/src/kcrash.cpp 2021-06-05 10:52:09.000000000 +0200 @@ -4,6 +4,7 @@ SPDX-FileCopyrightText: 2000 Tom Braun <[email protected]> SPDX-FileCopyrightText: 2010 George Kiagiadakis <[email protected]> SPDX-FileCopyrightText: 2009 KDE e.V. <[email protected]> + SPDX-FileCopyrightText: 2021 Harald Sitter <[email protected]> SPDX-FileContributor: 2009 Adriaan de Groot <[email protected]> SPDX-License-Identifier: LGPL-2.0-or-later @@ -39,6 +40,7 @@ #include <memory> #include <QDebug> +#include <QDir> #include <QFile> #include <QGuiApplication> #include <QLibraryInfo> @@ -57,6 +59,7 @@ #endif #include "coreconfig_p.h" +#include "metadata_p.h" // Copy from klauncher_cmds typedef struct { @@ -120,13 +123,15 @@ static KCrash::HandlerType s_emergencySaveFunction = nullptr; static KCrash::HandlerType s_crashHandler = nullptr; -static std::unique_ptr<char[]> s_appName; -static std::unique_ptr<char[]> s_appPath; +static std::unique_ptr<char[]> s_appFilePath; // this is the actual QCoreApplication::applicationFilePath +static std::unique_ptr<char[]> s_appName; // the binary name (may be altered by the application) +static std::unique_ptr<char[]> s_appPath; // the binary dir path (may be altered by the application) static Args s_autoRestartCommandLine; static std::unique_ptr<char[]> s_drkonqiPath; static KCrash::CrashFlags s_flags = KCrash::CrashFlags(); static int s_launchDrKonqi = -1; // -1=initial value 0=disabled 1=enabled static int s_originalSignal = -1; +static QByteArray s_metadataPath; static std::unique_ptr<char[]> s_kcrashErrorMessage; Q_GLOBAL_STATIC(KCrash::CoreConfig, s_coreConfig) @@ -169,6 +174,18 @@ } Q_COREAPP_STARTUP_FUNCTION(kcrashInitialize) +static QStringList libexecPaths() +{ + // Static since we only need to evaluate once. + static QStringList list = QFile::decodeName(qgetenv("LIBEXEC_PATH")).split(QLatin1Char(':'), Qt::SkipEmptyParts) // env var is used first + + QStringList{ + QCoreApplication::applicationDirPath(), // then look where our application binary is located + QLibraryInfo::location(QLibraryInfo::LibraryExecutablesPath), // look where libexec path is (can be set in qt.conf) + QFile::decodeName(KDE_INSTALL_FULL_LIBEXECDIR) // look at our installation location + }; + return list; +} + namespace KCrash { void setApplicationFilePath(const QString &filePath); @@ -179,6 +196,19 @@ #endif } +static bool shouldWriteMetadataToDisk() +{ +#ifdef Q_OS_LINUX + // NB: The daemon being currently running must not be a condition here. If something crashes during logout + // the daemon may already be gone but we'll still want to deal with the crash on next login! + // Similar reasoning applies to not checking the presence of the launcher socket. + const bool drkonqiCoredumpHelper = !QStandardPaths::findExecutable(QStringLiteral("drkonqi-coredump-processor"), libexecPaths()).isEmpty(); + return s_coreConfig()->isCoredumpd() && drkonqiCoredumpHelper && !qEnvironmentVariableIsSet("KCRASH_NO_METADATA"); +#else + return false; +#endif +} + void KCrash::initialize() { if (s_launchDrKonqi == 0) { // disabled by the program itself @@ -187,7 +217,8 @@ const QStringList args = QCoreApplication::arguments(); if (!qEnvironmentVariableIsSet("KDE_DEBUG") // && !qEnvironmentVariableIsSet("KCRASH_AUTO_RESTARTED") // - && !qEnvironmentVariableIntValue("RUNNING_UNDER_RR")) { + && !qEnvironmentVariableIntValue("RUNNING_UNDER_RR") // + && qEnvironmentVariableIntValue("KCRASH_DUMP_ONLY") == 0) { // enable drkonqi KCrash::setDrKonqiEnabled(true); } else { @@ -196,18 +227,37 @@ } if (QCoreApplication::instance()) { - KCrash::setApplicationFilePath(QCoreApplication::applicationFilePath()); + const QString path = QCoreApplication::applicationFilePath(); + s_appFilePath.reset(qstrdup(qPrintable(path))); // This intentionally cannot be changed by the application! + KCrash::setApplicationFilePath(path); } else { qWarning() << "This process needs a QCoreApplication instance in order to use KCrash"; } #ifdef Q_OS_LINUX // Create socket path to transfer ptrace scope and open connection - s_socketpath = QFile::encodeName(QStringLiteral("%1/kcrash_%2") // - .arg(QStandardPaths::writableLocation(QStandardPaths::RuntimeLocation)) - .arg(getpid())); + s_socketpath = QFile::encodeName(QStringLiteral("%1/kcrash_%2").arg(QStandardPaths::writableLocation(QStandardPaths::RuntimeLocation)).arg(getpid())); #endif + if (shouldWriteMetadataToDisk()) { + // We do not actively clean up metadata via KCrash but some other service. This potentially means we litter + // a lot -> put the metadata in a subdir. + const QString metadataDir = QStandardPaths::writableLocation(QStandardPaths::GenericCacheLocation) + QStringLiteral("/kcrash-metadata"); + if (QDir().mkpath(metadataDir)) { + s_metadataPath = QFile::encodeName(metadataDir + QStringLiteral("/%1.ini").arg(QCoreApplication::applicationPid())); + } + if (!s_crashHandler) { + // Always enable the default handler. We cannot create the metadata ahead of time since we do not know + // when the application metadata is "complete". + // TODO: kf6 maybe change the way init works and have the users run it when their done with kaboutdata etc.? + // the problem with delayed writing is that our crash handler (or any crash handler really) introduces a delay + // in dumping and this in turn increases the risk of another stepping into a puddle (SEGV only runs on the + // faulting thread; all other threads continue running!). therefore it'd be greatly preferred if we + // were able to write the metadata during initial app setup instead of when a crash occurs + setCrashHandler(defaultCrashHandler); + } + } // empty s_metadataPath disables writing + s_coreConfig(); // Initialize. } @@ -300,15 +350,9 @@ } s_launchDrKonqi = launchDrKonqi; if (s_launchDrKonqi && !s_drkonqiPath) { - // search paths - const QStringList paths = - QStringList() << QFile::decodeName(qgetenv("LIBEXEC_PATH")).split(QLatin1Char(':'), Qt::SkipEmptyParts) // env var is used first - << QCoreApplication::applicationDirPath() // then look where our application binary is located - << QLibraryInfo::location(QLibraryInfo::LibraryExecutablesPath) // look where libexec path is (can be set in qt.conf) - << QFile::decodeName(KDE_INSTALL_FULL_LIBEXECDIR); // look at our installation location - const QString exec = QStandardPaths::findExecutable(QStringLiteral("drkonqi"), paths); + const QString exec = QStandardPaths::findExecutable(QStringLiteral("drkonqi"), libexecPaths()); if (exec.isEmpty()) { - qCDebug(LOG_KCRASH) << "Could not find drkonqi in search paths:" << paths; + qCDebug(LOG_KCRASH) << "Could not find drkonqi in search paths:" << libexecPaths(); s_launchDrKonqi = 0; } else { s_drkonqiPath.reset(qstrdup(qPrintable(exec))); @@ -381,9 +425,9 @@ static void closeAllFDs() { // Close all remaining file descriptors except for stdin/stdout/stderr - struct rlimit rlp; + struct rlimit rlp = {}; getrlimit(RLIMIT_NOFILE, &rlp); - for (int i = 3; i < (int)rlp.rlim_cur; i++) { + for (rlim_t i = 3; i < rlp.rlim_cur; i++) { close(i); } } @@ -419,142 +463,149 @@ crashRecursionCounter++; } -#if !defined(Q_OS_WIN) && !defined(Q_OS_OSX) - if (!(s_flags & KeepFDs)) { - // This tries to prevent problems where applications fail to release resources that drkonqi might need. - // Specifically this was introduced to ensure that an application that had grabbed the X11 cursor would - // forcefully have it removed upon crash to ensure it is ungrabbed by the time drkonqi makes an appearance. - // This is also the point in time when, for example, dbus services are lost. Closing the socket indicates - // to dbus-daemon that the process has disappeared and it will forcefully reclaim the registered service names. - closeAllFDs(); - } -#if HAVE_X11 - else if (QX11Info::display()) { - close(ConnectionNumber(QX11Info::display())); - } -#endif -#endif - if (crashRecursionCounter < 3) { -#ifndef NDEBUG - fprintf(stderr, "KCrash: crashing... crashRecursionCounter = %d\n", crashRecursionCounter); - fprintf(stderr, - "KCrash: Application Name = %s path = %s pid = %lld\n", - s_appName ? s_appName.get() : "<unknown>", - s_appPath ? s_appPath.get() : "<unknown>", - QCoreApplication::applicationPid()); - fprintf(stderr, "KCrash: Arguments: "); - for (int i = 0; i < s_autoRestartCommandLine.argc; ++i) { - fprintf(stderr, "%s ", s_autoRestartCommandLine.argv[i]); - } - fprintf(stderr, "\n"); -#else - fprintf(stderr, "KCrash: Application '%s' crashing...\n", s_appName ? s_appName.get() : "<unknown>"); -#endif - - if (s_launchDrKonqi != 1) { - setCrashHandler(nullptr); -#if !defined(Q_OS_WIN) - raise(sig); // dump core, or whatever is the default action for this signal. -#endif - return; - } - // If someone is telling me to stop while I'm already crashing, then I should resume crashing signal(SIGTERM, &crashOnSigTerm); - const char *argv[29]; // don't forget to update this - int i = 0; - - // argument 0 has to be drkonqi - argv[i++] = s_drkonqiPath.get(); + // NB: metadata writing ought to happen before closing FDs to reduce synchronization problems with dbus. + MetadataWriter *writer = nullptr; +#ifdef Q_OS_LINUX + if (!s_metadataPath.isEmpty()) { + MetadataINIWriter ini(s_metadataPath); + // Add the canonical exe path so the coredump daemon has more data points to map metdata to journald entry. + ini.add("--exe", s_appFilePath.get(), MetadataWriter::BoolValue::No); + writer = &ini; + } +#endif + // WARNING: do not forget to increase Metadata::argv's size when adding more potential arguments! + Metadata data(s_drkonqiPath.get(), writer); const QByteArray platformName = QGuiApplication::platformName().toUtf8(); if (!platformName.isEmpty()) { - argv[i++] = "-platform"; - argv[i++] = platformName.constData(); + data.add("--platform", platformName.constData()); } #if HAVE_X11 if (platformName == QByteArrayLiteral("xcb")) { // start up on the correct display - argv[i++] = "-display"; + char *display = nullptr; if (QX11Info::display()) { - argv[i++] = XDisplayString(QX11Info::display()); + display = XDisplayString(QX11Info::display()); } else { - argv[i++] = getenv("DISPLAY"); + display = getenv("DISPLAY"); } + data.add("--display", display); } #endif - argv[i++] = "--appname"; - argv[i++] = s_appName ? s_appName.get() : "<unknown>"; + data.add("--appname", s_appName ? s_appName.get() : "<unknown>"); if (loadedByKdeinit) { - argv[i++] = "--kdeinit"; + data.addBool("--kdeinit"); } // only add apppath if it's not NULL if (s_appPath && s_appPath[0]) { - argv[i++] = "--apppath"; - argv[i++] = s_appPath.get(); + data.add("--apppath", s_appPath.get()); } // signal number -- will never be NULL char sigtxt[10]; sprintf(sigtxt, "%d", sig); - argv[i++] = "--signal"; - argv[i++] = sigtxt; + data.add("--signal", sigtxt); char pidtxt[20]; sprintf(pidtxt, "%lld", QCoreApplication::applicationPid()); - argv[i++] = "--pid"; - argv[i++] = pidtxt; + data.add("--pid", pidtxt); const KAboutData *about = KAboutData::applicationDataPointer(); if (about) { if (about->internalVersion()) { - argv[i++] = "--appversion"; - argv[i++] = about->internalVersion(); + data.add("--appversion", about->internalVersion()); } if (about->internalProgramName()) { - argv[i++] = "--programname"; - argv[i++] = about->internalProgramName(); + data.add("--programname", about->internalProgramName()); } if (about->internalBugAddress()) { - argv[i++] = "--bugaddress"; - argv[i++] = about->internalBugAddress(); + data.add("--bugaddress", about->internalBugAddress()); + } + + if (about->internalProductName()) { + data.add("--productname", about->internalProductName()); } } // make sure the constData() pointer remains valid when we call startProcess by making a copy QByteArray startupId = KStartupInfo::startupId(); if (!startupId.isNull()) { - argv[i++] = "--startupid"; - argv[i++] = startupId.constData(); + data.add("--startupid", startupId.constData()); } if (s_flags & SaferDialog) { - argv[i++] = "--safer"; + data.addBool("--safer"); } if ((s_flags & AutoRestart) && s_autoRestartCommandLine) { - argv[i++] = "--restarted"; // tell drkonqi if the app has been restarted + data.addBool("--restarted"); } #if defined(Q_OS_WIN) char threadId[8] = {0}; sprintf(threadId, "%d", GetCurrentThreadId()); - argv[i++] = "--thread"; - argv[i++] = threadId; + data.add("--thread", threadId); +#endif + + data.close(); + const int argc = data.argc; + const char **argv = data.argv.data(); + +#ifndef NDEBUG + fprintf(stderr, "KCrash: crashing... crashRecursionCounter = %d\n", crashRecursionCounter); + fprintf(stderr, + "KCrash: Application Name = %s path = %s pid = %lld\n", + s_appName ? s_appName.get() : "<unknown>", + s_appPath ? s_appPath.get() : "<unknown>", + QCoreApplication::applicationPid()); + fprintf(stderr, "KCrash: Arguments: "); + for (int i = 0; i < s_autoRestartCommandLine.argc; ++i) { + fprintf(stderr, "%s ", s_autoRestartCommandLine.argv[i]); + } + fprintf(stderr, "\n"); +#else + fprintf(stderr, "KCrash: Application '%s' crashing...\n", s_appName ? s_appName.get() : "<unknown>"); +#endif + +#if !defined(Q_OS_WIN) && !defined(Q_OS_OSX) + if (!(s_flags & KeepFDs)) { + // This tries to prevent problems where applications fail to release resources that drkonqi might need. + // Specifically this was introduced to ensure that an application that had grabbed the X11 cursor would + // forcefully have it removed upon crash to ensure it is ungrabbed by the time drkonqi makes an appearance. + // This is also the point in time when, for example, dbus services are lost. Closing the socket indicates + // to dbus-daemon that the process has disappeared and it will forcefully reclaim the registered service names. + // + // Once we close our socket we lose potential dbus names and if we were running as a systemd service anchored to a name, + // the daemon may decide to jump at us with a TERM signal. We'll want to have finished the metadata by now and + // be near our tracing/raise(). + closeAllFDs(); + } +#if HAVE_X11 + else if (QX11Info::display()) { + close(ConnectionNumber(QX11Info::display())); + } +#endif #endif - // NULL terminated list - argv[i] = nullptr; + if (s_launchDrKonqi != 1) { + setCrashHandler(nullptr); +#if !defined(Q_OS_WIN) + raise(sig); // dump core, or whatever is the default action for this signal. +#endif + return; + } - startProcess(i, argv, true); + startProcess(argc, argv, true); } if (crashRecursionCounter < 4) { diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/kcrash-5.82.0/src/metadata.cpp new/kcrash-5.83.0/src/metadata.cpp --- old/kcrash-5.82.0/src/metadata.cpp 1970-01-01 01:00:00.000000000 +0100 +++ new/kcrash-5.83.0/src/metadata.cpp 2021-06-05 10:52:09.000000000 +0200 @@ -0,0 +1,111 @@ +/* + SPDX-License-Identifier: LGPL-2.0-or-later + SPDX-FileCopyrightText: 2021 Harald Sitter <[email protected]> +*/ + +#include "metadata_p.h" + +#include <QByteArray> +#include <cerrno> + +#ifdef Q_OS_LINUX +#include <cstring> +#include <fcntl.h> +#include <sys/stat.h> +#include <sys/types.h> +#include <unistd.h> +#endif + +namespace KCrash +{ +#ifdef Q_OS_LINUX +MetadataINIWriter::MetadataINIWriter(const QByteArray &path) + : MetadataWriter() + , fd(::open(path.constData(), O_WRONLY | O_CREAT | O_NONBLOCK | O_TRUNC | O_CLOEXEC, S_IRUSR | S_IWUSR)) +{ + if (fd == -1) { + fprintf(stderr, "Failed to open metadata file: %s\n", strerror(errno)); + } else { + const char *header = "[KCrash]\n"; + write(fd, header, strlen(header)); + } +} + +void MetadataINIWriter::close() +{ + if (fd >= 0 && ::close(fd) == -1) { + fprintf(stderr, "Failed to close metadata file: %s\n", strerror(errno)); + } +} + +void MetadataINIWriter::add(const char *key, const char *value, BoolValue boolValue) +{ + Q_ASSERT(key); + Q_ASSERT(value); + Q_ASSERT(key[0] == '-' && key[1] == '-'); // well-formed '--' prefix. This is important. MetadataWriter presume this + Q_UNUSED(boolValue); // value is a bool string but we don't care, we always write the value anyway + + if (fd < 0) { + return; + } + const int ret = snprintf(iniLine.data(), iniLine.max_size(), "%s=%s\n", key + 2 /** skip the leading -- */, value); + if (ret < 0) { + fprintf(stderr, "Failed to generate metadata line for '%s', '%s'\n", key, value); + return; + } + // Cannot be negative anymore. + const std::make_unsigned<decltype(ret)>::type lineLength = ret; + fprintf(stderr, "%d -- %s", lineLength, iniLine.data()); + Q_ASSERT(lineLength <= iniLine.max_size()); // is not truncated41 + write(fd, iniLine.data(), lineLength); +} +#endif + +Metadata::Metadata(const char *cmd, MetadataWriter *writer) + : MetadataWriter() + , m_writer(writer) +{ + // NB: cmd may be null! Just because we create metadata doesn't mean we'll execute drkonqi (we may only need the + // backing writers) + Q_ASSERT(argc == 0); + argv.at(argc++) = cmd; +} + +void Metadata::add(const char *key, const char *value) +{ + add(key, value, BoolValue::No); +} + +void Metadata::addBool(const char *key) +{ + add(key, "true", BoolValue::Yes); +} + +void Metadata::close() +{ + // NULL terminated list + argv.at(argc) = nullptr; + + if (m_writer) { + m_writer->close(); + } +} + +void Metadata::add(const char *key, const char *value, BoolValue boolValue) +{ + Q_ASSERT(key); + Q_ASSERT(value); + Q_ASSERT(key[0] == '-' && key[1] == '-'); // well-formed '--' prefix. This is important. MetadataWriter presume this + Q_ASSERT(argc < argv.max_size()); // argv has a static max size. guard against exhaustion + + argv.at(argc++) = key; + if (!boolValue) { + argv.at(argc++) = value; + } + + if (m_writer) { + m_writer->add(key, value, boolValue); + } +} + +} // namespace KCrash diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/kcrash-5.82.0/src/metadata_p.h new/kcrash-5.83.0/src/metadata_p.h --- old/kcrash-5.82.0/src/metadata_p.h 1970-01-01 01:00:00.000000000 +0100 +++ new/kcrash-5.83.0/src/metadata_p.h 2021-06-05 10:52:09.000000000 +0200 @@ -0,0 +1,83 @@ +/* + SPDX-License-Identifier: LGPL-2.0-or-later + SPDX-FileCopyrightText: 2021 Harald Sitter <[email protected]> +*/ + +#ifndef KCRASH_METADATA_H +#define KCRASH_METADATA_H + +#include <QtGlobal> + +#include <array> + +class QByteArray; + +namespace KCrash +{ +// A metadata writer interface. +class MetadataWriter +{ +public: + enum BoolValue { No = false, Yes = true }; + + virtual void add(const char *key, const char *value, BoolValue boolValue) = 0; + virtual void close() = 0; + +protected: + MetadataWriter() = default; + virtual ~MetadataWriter() = default; + +private: + Q_DISABLE_COPY_MOVE(MetadataWriter) +}; + +#ifdef Q_OS_LINUX +// This writes the metdata file. Only really useful on Linux for now as this needs +// cleanup by a helper daemon later. Also, this is only ever useful when coredump is in use. +class MetadataINIWriter : public MetadataWriter +{ +public: + explicit MetadataINIWriter(const QByteArray &path); + ~MetadataINIWriter() override = default; + + void add(const char *key, const char *value, BoolValue boolValue) override; + void close() override; + +private: + int fd = -1; + std::array<char, 1024> iniLine{}; // arbitrary max size + + Q_DISABLE_COPY_MOVE(MetadataINIWriter) +}; +#endif + +// Compile the crash metadata. These are the primary ARGV metadata, but additional +// metadata writers may be added behind it to (e.g.) write the data to a file as well. +// man 7 signal-safety +class Metadata : public MetadataWriter +{ +public: + Metadata(const char *cmd, MetadataWriter *writer); + ~Metadata() override = default; + + void add(const char *key, const char *value); + void addBool(const char *key); + + // Also closes the backing writer. + void close() override; + + std::array<const char *, 31> argv{}; + std::size_t argc = 0; + +private: + void add(const char *key, const char *value, BoolValue boolValue) override; + + // Obviously if we should ever need more writers, refactor to std::initializer_list or something similar. + MetadataWriter *m_writer = nullptr; + + Q_DISABLE_COPY_MOVE(Metadata) +}; + +} // namespace KCrash + +#endif // KCRASH_METADATA_H
