From 486d961f48bdf6da5a93f84c20ed3f2ffe3b3f24 Mon Sep 17 00:00:00 2001 From: Dale Glass Date: Mon, 18 May 2020 17:58:28 +0200 Subject: [PATCH 01/10] Add support for crashpad on Linux --- cmake/externals/crashpad/CMakeLists.txt | 22 ++++++++++++++++++++-- cmake/macros/AddCrashpad.cmake | 6 +++--- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/cmake/externals/crashpad/CMakeLists.txt b/cmake/externals/crashpad/CMakeLists.txt index e519ead0ee..b20b579152 100644 --- a/cmake/externals/crashpad/CMakeLists.txt +++ b/cmake/externals/crashpad/CMakeLists.txt @@ -35,6 +35,25 @@ elseif (APPLE) ExternalProject_Get_Property(${EXTERNAL_NAME} SOURCE_DIR) + set(BIN_RELEASE_PATH "${SOURCE_DIR}/out/Release") + set(BIN_EXT "") + set(LIB_RELEASE_PATH "${SOURCE_DIR}/out/Release/lib") + set(LIB_DEBUG_PATH "${SOURCE_DIR}/out/Debug/lib") + set(LIB_PREFIX "lib") + set(LIB_EXT "a") +elseif (UNIX) + ExternalProject_Add( + ${EXTERNAL_NAME} + URL "${EXTERNAL_BUILD_ASSETS}/dependencies/crashpad_linux_c7d1d2a1.tar.bz2" + URL_MD5 356bee858e7338e7b06a351bce92db36 + CONFIGURE_COMMAND "" + BUILD_COMMAND "" + INSTALL_COMMAND "" + LOG_DOWNLOAD 1 + ) + + ExternalProject_Get_Property(${EXTERNAL_NAME} SOURCE_DIR) + set(BIN_RELEASE_PATH "${SOURCE_DIR}/out/Release") set(BIN_EXT "") set(LIB_RELEASE_PATH "${SOURCE_DIR}/out/Release/lib") @@ -43,8 +62,7 @@ elseif (APPLE) set(LIB_EXT "a") endif () -if (WIN32 OR APPLE) - +if (WIN32 OR APPLE OR UNIX) set(${EXTERNAL_NAME_UPPER}_INCLUDE_DIRS ${SOURCE_DIR}/include CACHE PATH "List of Crashpad include directories") set(${EXTERNAL_NAME_UPPER}_LIBRARY_RELEASE ${LIB_RELEASE_PATH}/${LIB_PREFIX}crashpad_client.${LIB_EXT} CACHE FILEPATH "Path to Crashpad release library") diff --git a/cmake/macros/AddCrashpad.cmake b/cmake/macros/AddCrashpad.cmake index bc070e057b..f30e260563 100644 --- a/cmake/macros/AddCrashpad.cmake +++ b/cmake/macros/AddCrashpad.cmake @@ -23,12 +23,12 @@ macro(add_crashpad) set(CMAKE_BACKTRACE_TOKEN $ENV{CMAKE_BACKTRACE_TOKEN}) endif() - if ((WIN32 OR APPLE) AND USE_CRASHPAD) + if (USE_CRASHPAD) get_property(CRASHPAD_CHECKED GLOBAL PROPERTY CHECKED_FOR_CRASHPAD_ONCE) if (NOT CRASHPAD_CHECKED) add_dependency_external_projects(crashpad) - find_package(crashpad REQUIRED) + find_package(Crashpad REQUIRED) set_property(GLOBAL PROPERTY CHECKED_FOR_CRASHPAD_ONCE TRUE) endif() @@ -38,7 +38,7 @@ macro(add_crashpad) add_definitions(-DCMAKE_BACKTRACE_TOKEN=\"${CMAKE_BACKTRACE_TOKEN}\") target_include_directories(${TARGET_NAME} PRIVATE ${CRASHPAD_INCLUDE_DIRS}) - target_link_libraries(${TARGET_NAME} ${CRASHPAD_LIBRARY} ${CRASHPAD_BASE_LIBRARY} ${CRASHPAD_UTIL_LIBRARY}) + target_link_libraries(${TARGET_NAME} ${CRASHPAD_LIBRARY} ${CRASHPAD_UTIL_LIBRARY} ${CRASHPAD_BASE_LIBRARY}) if (WIN32) set_target_properties(${TARGET_NAME} PROPERTIES LINK_FLAGS "/ignore:4099") From 86e2da729e2d1d6a34c8e20a95cec52905eddf81 Mon Sep 17 00:00:00 2001 From: Dale Glass Date: Mon, 18 May 2020 20:27:27 +0200 Subject: [PATCH 02/10] Try library built with SSL --- cmake/externals/crashpad/CMakeLists.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmake/externals/crashpad/CMakeLists.txt b/cmake/externals/crashpad/CMakeLists.txt index b20b579152..57fcc5ec01 100644 --- a/cmake/externals/crashpad/CMakeLists.txt +++ b/cmake/externals/crashpad/CMakeLists.txt @@ -44,8 +44,8 @@ elseif (APPLE) elseif (UNIX) ExternalProject_Add( ${EXTERNAL_NAME} - URL "${EXTERNAL_BUILD_ASSETS}/dependencies/crashpad_linux_c7d1d2a1.tar.bz2" - URL_MD5 356bee858e7338e7b06a351bce92db36 + URL "${EXTERNAL_BUILD_ASSETS}/dependencies/crashpad_linux_ssl_c7d1d2a1.tar.bz2" + URL_MD5 f016faf1dea7dcb3af4b2ebffc7586cc CONFIGURE_COMMAND "" BUILD_COMMAND "" INSTALL_COMMAND "" From 9a5c1ac24a91396916d33609b9ec27373ecc35c0 Mon Sep 17 00:00:00 2001 From: Dale Glass Date: Sun, 11 Apr 2021 20:47:29 +0200 Subject: [PATCH 03/10] Use latest crashpad build. This one uses libcurl, which may work better. --- cmake/externals/crashpad/CMakeLists.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmake/externals/crashpad/CMakeLists.txt b/cmake/externals/crashpad/CMakeLists.txt index 57fcc5ec01..ae6972cab1 100644 --- a/cmake/externals/crashpad/CMakeLists.txt +++ b/cmake/externals/crashpad/CMakeLists.txt @@ -44,8 +44,8 @@ elseif (APPLE) elseif (UNIX) ExternalProject_Add( ${EXTERNAL_NAME} - URL "${EXTERNAL_BUILD_ASSETS}/dependencies/crashpad_linux_ssl_c7d1d2a1.tar.bz2" - URL_MD5 f016faf1dea7dcb3af4b2ebffc7586cc + URL "${EXTERNAL_BUILD_ASSETS}/dependencies/crashpad_linux_f1943fcb.tar.bz2" + URL_MD5 e0949e5988905471c63c399833879482 CONFIGURE_COMMAND "" BUILD_COMMAND "" INSTALL_COMMAND "" From 567b0d84efbb701d897cfaba3d6d09ae1b3033e8 Mon Sep 17 00:00:00 2001 From: Dale Glass Date: Mon, 28 Jun 2021 20:02:46 +0200 Subject: [PATCH 04/10] Update crash handler to latest build --- cmake/externals/crashpad/CMakeLists.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmake/externals/crashpad/CMakeLists.txt b/cmake/externals/crashpad/CMakeLists.txt index ae6972cab1..cdcd4fd5ed 100644 --- a/cmake/externals/crashpad/CMakeLists.txt +++ b/cmake/externals/crashpad/CMakeLists.txt @@ -6,8 +6,8 @@ string(TOUPPER ${EXTERNAL_NAME} EXTERNAL_NAME_UPPER) if (WIN32) ExternalProject_Add( ${EXTERNAL_NAME} - URL "${EXTERNAL_BUILD_ASSETS}/dependencies/crashpad_062317.1.zip" - URL_MD5 9c84b77f5f23daf939da1371825ed2b1 + URL "${EXTERNAL_BUILD_ASSETS}/dependencies/crashpad_linux_20210628_cdee8004.tar.bz2" + URL_SHA256 2e84b253b27f37224f5f9f86f955b23befcf00cd3300e66de42a863c82ec2787 CONFIGURE_COMMAND "" BUILD_COMMAND "" INSTALL_COMMAND "" From 186e784d89aca5fc181b716bc1af7cc8ff413a59 Mon Sep 17 00:00:00 2001 From: Dale Glass Date: Thu, 1 Jul 2021 01:43:06 +0200 Subject: [PATCH 05/10] Rework crashpad initialization * Don't abort on startup if we can't find our own path * Try to locate our own path by looking in /proc in Linux -- should fix AppImage issues * Add extensive logging --- interface/src/CrashHandler.h | 5 +- interface/src/CrashHandler_Crashpad.cpp | 64 +++++++++++++++++++++++-- 2 files changed, 65 insertions(+), 4 deletions(-) diff --git a/interface/src/CrashHandler.h b/interface/src/CrashHandler.h index 5b7e3b4438..27c5d76e1c 100644 --- a/interface/src/CrashHandler.h +++ b/interface/src/CrashHandler.h @@ -13,7 +13,10 @@ #define hifi_CrashHandler_h #include -class QCoreApplication; +#include +#include + +Q_DECLARE_LOGGING_CATEGORY(crash_handler) bool startCrashHandler(std::string appPath); void setCrashAnnotation(std::string name, std::string value); diff --git a/interface/src/CrashHandler_Crashpad.cpp b/interface/src/CrashHandler_Crashpad.cpp index a8195ce6e8..f2ec0c2ff8 100644 --- a/interface/src/CrashHandler_Crashpad.cpp +++ b/interface/src/CrashHandler_Crashpad.cpp @@ -13,6 +13,8 @@ #include "CrashHandler.h" +Q_LOGGING_CATEGORY(crash_handler, "vircadia.crash_handler") + #include #include @@ -141,6 +143,10 @@ static const QString CRASHPAD_HANDLER_NAME{ "crashpad_handler.exe" }; static const QString CRASHPAD_HANDLER_NAME{ "crashpad_handler" }; #endif +#ifdef Q_OS_LINUX +#include +#endif + #ifdef Q_OS_WIN // ------------------------------------------------------------------------------------------------ // The area within this #ifdef is specific to the Microsoft C++ compiler @@ -311,8 +317,38 @@ void checkUnhandledExceptionHook() { // ------------------------------------------------------------------------------------------------ #endif // Q_OS_WIN +// Locate the full path to the binary's directory +static QString findBinaryDir() { + // Normally we'd just use QCoreApplication::applicationDirPath(), but we can't. + // That function needs the QApplication to be created first, and Crashpad is initialized as early as possible, + // which is well before QApplication, so that function throws out a warning and returns ".". + // + // So we must do things the hard way here. In particular this is needed to correctly handle things in AppImage + // on Linux. On Windows and MacOS falling back to argv[0] should be fine. + +#ifdef Q_OS_LINUX + // Find outselves by looking at /proc//exe + pid_t ourPid = getpid(); + QString exeLink = QString("/proc/%1/exe").arg(ourPid); + qCDebug(crash_handler) << "Looking at" << exeLink; + + QFileInfo exeLinkInfo(exeLink); + if (exeLinkInfo.isSymLink()) { + QFileInfo exeInfo( exeLinkInfo.symLinkTarget() ); + qCDebug(crash_handler) << "exe symlink points at" << exeInfo; + return exeInfo.absoluteDir().absolutePath(); + } else { + qCWarning(crash_handler) << exeLink << "isn't a symlink. /proc not mounted?"; + } + +#endif + + return QString(); +} + bool startCrashHandler(std::string appPath) { if (BACKTRACE_URL.empty() || BACKTRACE_TOKEN.empty()) { + qCCritical(crash_handler) << "Backtrace URL or token not set, crash handler disabled."; return false; } @@ -338,11 +374,29 @@ bool startCrashHandler(std::string appPath) { const auto crashpadDbPath = crashpadDbDir.toStdString() + "/" + crashpadDbName; // Locate Crashpad handler - const QFileInfo interfaceBinary{ QString::fromStdString(appPath) }; - const QDir interfaceDir = interfaceBinary.dir(); - assert(interfaceDir.exists(CRASHPAD_HANDLER_NAME)); + QString binaryDir = findBinaryDir(); + QDir interfaceDir; + + if ( !binaryDir.isEmpty() ) { + // Locating ourselves by argv[0] fails in the case of AppImage on Linux, as we get the AppImage + // itself in there. If we have a platform-specific method, and it succeeds, we use that instead + // of argv. + qCDebug(crash_handler) << "Locating own directory by platform-specific method"; + interfaceDir.setPath(binaryDir); + } else { + qCDebug(crash_handler) << "Locating own directory by argv[0]"; + interfaceDir.setPath(QString::fromStdString(appPath)); + } + + if (!interfaceDir.exists(CRASHPAD_HANDLER_NAME)) { + qCCritical(crash_handler) << "Failed to find" << CRASHPAD_HANDLER_NAME << "in" << interfaceDir << ", can't start crash handler"; + return false; + } + const std::string CRASHPAD_HANDLER_PATH = interfaceDir.filePath(CRASHPAD_HANDLER_NAME).toStdString(); + qCDebug(crash_handler) << "Crashpad handler found at" << QString::fromStdString(CRASHPAD_HANDLER_PATH); + // Setup different file paths base::FilePath::StringType dbPath; base::FilePath::StringType handlerPath; @@ -352,8 +406,10 @@ bool startCrashHandler(std::string appPath) { base::FilePath db(dbPath); base::FilePath handler(handlerPath); + qCDebug(crash_handler) << "Opening crashpad database" << QString::fromStdString(crashpadDbPath); auto database = crashpad::CrashReportDatabase::Initialize(db); if (database == nullptr || database->GetSettings() == nullptr) { + qCCritical(crash_handler) << "Failed to open crashpad database" << QString::fromStdString(crashpadDbPath); return false; } @@ -361,6 +417,7 @@ bool startCrashHandler(std::string appPath) { database->GetSettings()->SetUploadsEnabled(true); if (!client->StartHandler(handler, db, db, BACKTRACE_URL, annotations, arguments, true, true)) { + qCCritical(crash_handler) << "Failed to start crashpad handler"; return false; } @@ -369,6 +426,7 @@ bool startCrashHandler(std::string appPath) { gl_crashpadUnhandledExceptionFilter = SetUnhandledExceptionFilter(unhandledExceptionHandler); #endif + qCInfo(crash_handler) << "Crashpad initialized"; return true; } From c4f3e1e263573a70a3604605f38d6a2e74b64554 Mon Sep 17 00:00:00 2001 From: Dale Glass Date: Thu, 1 Jul 2021 19:19:07 +0200 Subject: [PATCH 06/10] Fix cmake warning about crashpad name --- cmake/modules/FindCrashpad.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmake/modules/FindCrashpad.cmake b/cmake/modules/FindCrashpad.cmake index 283058336d..c93ab9a0be 100644 --- a/cmake/modules/FindCrashpad.cmake +++ b/cmake/modules/FindCrashpad.cmake @@ -38,4 +38,4 @@ select_library_configurations(CRASHPAD_BASE) select_library_configurations(CRASHPAD_UTIL) include(FindPackageHandleStandardArgs) -find_package_handle_standard_args(CRASHPAD DEFAULT_MSG CRASHPAD_INCLUDE_DIRS CRASHPAD_LIBRARY CRASHPAD_BASE_LIBRARY CRASHPAD_UTIL_LIBRARY) +find_package_handle_standard_args(Crashpad DEFAULT_MSG CRASHPAD_INCLUDE_DIRS CRASHPAD_LIBRARY CRASHPAD_BASE_LIBRARY CRASHPAD_UTIL_LIBRARY) From e2eab15469fa51aa1fa721a20012b1527ba9af42 Mon Sep 17 00:00:00 2001 From: daleglass <51060919+daleglass@users.noreply.github.com> Date: Fri, 2 Jul 2021 00:58:27 +0200 Subject: [PATCH 07/10] Whitespace review fix Co-authored-by: David Rowe --- interface/src/CrashHandler_Crashpad.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/interface/src/CrashHandler_Crashpad.cpp b/interface/src/CrashHandler_Crashpad.cpp index f2ec0c2ff8..a079b6cd4a 100644 --- a/interface/src/CrashHandler_Crashpad.cpp +++ b/interface/src/CrashHandler_Crashpad.cpp @@ -377,7 +377,7 @@ bool startCrashHandler(std::string appPath) { QString binaryDir = findBinaryDir(); QDir interfaceDir; - if ( !binaryDir.isEmpty() ) { + if (!binaryDir.isEmpty()) { // Locating ourselves by argv[0] fails in the case of AppImage on Linux, as we get the AppImage // itself in there. If we have a platform-specific method, and it succeeds, we use that instead // of argv. From 15dfa04c0719902f28e5c06d28f1b97fabfb805b Mon Sep 17 00:00:00 2001 From: daleglass <51060919+daleglass@users.noreply.github.com> Date: Fri, 2 Jul 2021 00:58:35 +0200 Subject: [PATCH 08/10] Whitespace review fix Co-authored-by: David Rowe --- interface/src/CrashHandler_Crashpad.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/interface/src/CrashHandler_Crashpad.cpp b/interface/src/CrashHandler_Crashpad.cpp index a079b6cd4a..8c8ceed4b3 100644 --- a/interface/src/CrashHandler_Crashpad.cpp +++ b/interface/src/CrashHandler_Crashpad.cpp @@ -334,7 +334,7 @@ static QString findBinaryDir() { QFileInfo exeLinkInfo(exeLink); if (exeLinkInfo.isSymLink()) { - QFileInfo exeInfo( exeLinkInfo.symLinkTarget() ); + QFileInfo exeInfo(exeLinkInfo.symLinkTarget()); qCDebug(crash_handler) << "exe symlink points at" << exeInfo; return exeInfo.absoluteDir().absolutePath(); } else { From 52add8827331de8c6b888ad68e83f1495206ee56 Mon Sep 17 00:00:00 2001 From: Dale Glass Date: Sat, 3 Jul 2021 19:54:57 +0200 Subject: [PATCH 09/10] Undo accidental change that broke reporting on Windows --- cmake/externals/crashpad/CMakeLists.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmake/externals/crashpad/CMakeLists.txt b/cmake/externals/crashpad/CMakeLists.txt index cdcd4fd5ed..ae6972cab1 100644 --- a/cmake/externals/crashpad/CMakeLists.txt +++ b/cmake/externals/crashpad/CMakeLists.txt @@ -6,8 +6,8 @@ string(TOUPPER ${EXTERNAL_NAME} EXTERNAL_NAME_UPPER) if (WIN32) ExternalProject_Add( ${EXTERNAL_NAME} - URL "${EXTERNAL_BUILD_ASSETS}/dependencies/crashpad_linux_20210628_cdee8004.tar.bz2" - URL_SHA256 2e84b253b27f37224f5f9f86f955b23befcf00cd3300e66de42a863c82ec2787 + URL "${EXTERNAL_BUILD_ASSETS}/dependencies/crashpad_062317.1.zip" + URL_MD5 9c84b77f5f23daf939da1371825ed2b1 CONFIGURE_COMMAND "" BUILD_COMMAND "" INSTALL_COMMAND "" From e3004af61aad58ff306d7182ca82cce445b581fd Mon Sep 17 00:00:00 2001 From: Dale Glass Date: Sat, 10 Jul 2021 01:42:10 +0200 Subject: [PATCH 10/10] More logging for crash reporting * Make it clear in cmake if crash reporting is going to be used * Log warning on startup if there's no crash reporting --- cmake/macros/AddCrashpad.cmake | 5 +++++ interface/src/CrashHandler_None.cpp | 5 ++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/cmake/macros/AddCrashpad.cmake b/cmake/macros/AddCrashpad.cmake index f30e260563..684c842ea2 100644 --- a/cmake/macros/AddCrashpad.cmake +++ b/cmake/macros/AddCrashpad.cmake @@ -11,19 +11,24 @@ macro(add_crashpad) set (USE_CRASHPAD TRUE) + message(STATUS "Checking crashpad config") + if ("$ENV{CMAKE_BACKTRACE_URL}" STREQUAL "") + message(STATUS "Checking crashpad config - CMAKE_BACKTRACE_URL is not set, disabled.") set(USE_CRASHPAD FALSE) else() set(CMAKE_BACKTRACE_URL $ENV{CMAKE_BACKTRACE_URL}) endif() if ("$ENV{CMAKE_BACKTRACE_TOKEN}" STREQUAL "") + message(STATUS "Checking crashpad config - CMAKE_BACKTRACE_TOKEN is not set, disabled.") set(USE_CRASHPAD FALSE) else() set(CMAKE_BACKTRACE_TOKEN $ENV{CMAKE_BACKTRACE_TOKEN}) endif() if (USE_CRASHPAD) + message(STATUS "Checking crashpad config - enabled.") get_property(CRASHPAD_CHECKED GLOBAL PROPERTY CHECKED_FOR_CRASHPAD_ONCE) if (NOT CRASHPAD_CHECKED) diff --git a/interface/src/CrashHandler_None.cpp b/interface/src/CrashHandler_None.cpp index 4fe56cd042..2e86472705 100644 --- a/interface/src/CrashHandler_None.cpp +++ b/interface/src/CrashHandler_None.cpp @@ -17,8 +17,11 @@ #include + +Q_LOGGING_CATEGORY(crash_handler, "vircadia.crash_handler") + bool startCrashHandler(std::string appPath) { - qDebug() << "No crash handler available."; + qCWarning(crash_handler) << "No crash handler available."; return false; }