From 0bfc5b241199997cfbe0a12286f0b120b520624a Mon Sep 17 00:00:00 2001 From: krzaq Date: Tue, 16 May 2023 14:31:52 +0200 Subject: [PATCH 1/3] Crash History: initial implementation. This feature heavily leverages the old feature "Crash Loop Detection", but offers a more robust API, underlying implementation and is named more appropriately to its expected usage. --- client/crashpad_client.h | 4 +- client/crashpad_client_linux.cc | 22 +-- examples/CMakeLists.txt | 2 +- examples/linux/crash_history/CMakeLists.txt | 10 ++ .../README.md | 9 +- .../crash_history.cpp} | 2 +- .../linux/crash_loop_detection/CMakeLists.txt | 10 -- .../linux/crash_report_exception_handler.cc | 8 +- util/CMakeLists.txt | 4 +- util/backtrace/crash_history.cc | 134 ++++++++++++++++++ ...crash_loop_detection.h => crash_history.h} | 26 +++- util/backtrace/crash_loop_detection.cc | 125 ---------------- 12 files changed, 188 insertions(+), 168 deletions(-) create mode 100644 examples/linux/crash_history/CMakeLists.txt rename examples/linux/{crash_loop_detection => crash_history}/README.md (61%) rename examples/linux/{crash_loop_detection/crash_loop_detection.cpp => crash_history/crash_history.cpp} (98%) delete mode 100644 examples/linux/crash_loop_detection/CMakeLists.txt create mode 100644 util/backtrace/crash_history.cc rename util/backtrace/{crash_loop_detection.h => crash_history.h} (64%) delete mode 100644 util/backtrace/crash_loop_detection.cc diff --git a/client/crashpad_client.h b/client/crashpad_client.h index f48faab5..620d5427 100644 --- a/client/crashpad_client.h +++ b/client/crashpad_client.h @@ -142,7 +142,7 @@ class CrashpadClient { //! This function must be called prior to `StartHandler()` //! //! \return `true` on success. Otherwise `false` with a message logged. - bool EnableCrashLoopDetection(); + bool EnableCrashHistory(); //! \brief Detects if safe mode should be enabled //! @@ -853,7 +853,7 @@ class CrashpadClient { std::wstring ipc_pipe_; ScopedKernelHANDLE handler_start_thread_; #elif BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_CHROMEOS) || BUILDFLAG(IS_ANDROID) - bool crash_loop_detection_ = false; + bool crash_history_ = false; UUID run_uuid_; std::set unhandled_signals_; #endif // BUILDFLAG(IS_APPLE) diff --git a/client/crashpad_client_linux.cc b/client/crashpad_client_linux.cc index e78149ed..5bf87ead 100644 --- a/client/crashpad_client_linux.cc +++ b/client/crashpad_client_linux.cc @@ -36,7 +36,7 @@ #include "build/buildflag.h" #include "client/client_argv_handling.h" #include "third_party/lss/lss.h" -#include "util/backtrace/crash_loop_detection.h" +#include "util/backtrace/crash_history.h" #include "util/file/file_io.h" #include "util/file/filesystem.h" #include "util/linux/exception_handler_client.h" @@ -471,9 +471,9 @@ bool CrashpadClient::StartHandler( argv.push_back(FormatArgumentInt("initial-client-fd", handler_sock.get())); argv.push_back("--shared-client-connection"); - if (crash_loop_detection_) { - namespace clc = backtrace::crash_loop_detection; - DCHECK(clc::CrashLoopDetectionAppend(database, run_uuid_)); + if (crash_history_) { + namespace ch = backtrace::crash_history; + DCHECK(ch::Append(database, run_uuid_)); argv.push_back("--annotation=run-uuid=" + run_uuid_.ToString()); } @@ -498,10 +498,10 @@ bool CrashpadClient::StartHandler( } #if BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_ANDROID) || DOXYGEN -bool CrashpadClient::EnableCrashLoopDetection() +bool CrashpadClient::EnableCrashHistory() { - crash_loop_detection_ = run_uuid_.InitializeWithNew(); - return crash_loop_detection_; + crash_history_ = run_uuid_.InitializeWithNew(); + return crash_history_; } bool CrashpadClient::IsSafeModeRequired(const base::FilePath& database) @@ -511,7 +511,7 @@ bool CrashpadClient::IsSafeModeRequired(const base::FilePath& database) int CrashpadClient::ConsecutiveCrashesCount(const base::FilePath& database) { - namespace clc = backtrace::crash_loop_detection; + namespace clc = backtrace::crash_history; return clc::ConsecutiveCrashesCount(database); } #endif @@ -741,9 +741,9 @@ bool CrashpadClient::StartHandlerAtCrash( backtrace::android_cert_store::create(database); #endif - if (crash_loop_detection_) { - namespace clc = backtrace::crash_loop_detection; - bool ok = clc::CrashLoopDetectionAppend(database, run_uuid_); + if (crash_history_) { + namespace ch = backtrace::crash_history; + bool ok = ch::Append(database, run_uuid_); DCHECK(ok); argv.push_back("--annotation=run-uuid=" + run_uuid_.ToString()); } diff --git a/examples/CMakeLists.txt b/examples/CMakeLists.txt index d864d40d..a8642184 100644 --- a/examples/CMakeLists.txt +++ b/examples/CMakeLists.txt @@ -14,5 +14,5 @@ endif (WIN32) if (LINUX) add_subdirectory(linux/demo) - add_subdirectory(linux/crash_loop_detection) + add_subdirectory(linux/crash_history) endif (LINUX) diff --git a/examples/linux/crash_history/CMakeLists.txt b/examples/linux/crash_history/CMakeLists.txt new file mode 100644 index 00000000..1c1faba2 --- /dev/null +++ b/examples/linux/crash_history/CMakeLists.txt @@ -0,0 +1,10 @@ +cmake_minimum_required(VERSION 3.10) +project(crash_history_linux LANGUAGES CXX) + +if (CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR) + add_subdirectory(crashpad) +endif () + +add_executable(crash_history_linux crash_history.cpp) +target_compile_features(crash_history_linux PRIVATE cxx_std_14) +target_link_libraries(crash_history_linux PRIVATE client) diff --git a/examples/linux/crash_loop_detection/README.md b/examples/linux/crash_history/README.md similarity index 61% rename from examples/linux/crash_loop_detection/README.md rename to examples/linux/crash_history/README.md index 5ec68e3c..a1f9c681 100644 --- a/examples/linux/crash_loop_detection/README.md +++ b/examples/linux/crash_history/README.md @@ -1,18 +1,17 @@ -# Linux demo app for crash loop detection +# Linux demo app for Crash History This demo app immediately crashes and sends report to the Backtrace universe -selected by the user. This demo enables Backtrace's Safe Mode/crash loop -detection feature. +selected by the user. This demo enables Backtrace's Crash History feature. # Build instructions Assumptions: - crashpad is checked out to `~/crashpad` -- the demo app has been copied to `~/crash_loop_detection` +- the demo app has been copied to `~/crash_history` 1. Update your handler path: ```cpp -std::string handler_path("/home/myusername/crash_loop_detection/build/crashpad/handler/handler"); +std::string handler_path("/home/myusername/crash_history/build/crashpad/handler/handler"); ``` 2. Update your upload URL ```cpp diff --git a/examples/linux/crash_loop_detection/crash_loop_detection.cpp b/examples/linux/crash_history/crash_history.cpp similarity index 98% rename from examples/linux/crash_loop_detection/crash_loop_detection.cpp rename to examples/linux/crash_history/crash_history.cpp index 8cb69434..6063118f 100644 --- a/examples/linux/crash_loop_detection/crash_loop_detection.cpp +++ b/examples/linux/crash_history/crash_history.cpp @@ -54,7 +54,7 @@ startCrashHandler(std::string const& url, std::string const& handler_path, auto client = CrashpadClient{}; - client.EnableCrashLoopDetection(); + client.EnableCrashHistory(); std::cout << "Last Runs crashed: " << CrashpadClient::ConsecutiveCrashesCount(db) << "\n"; diff --git a/examples/linux/crash_loop_detection/CMakeLists.txt b/examples/linux/crash_loop_detection/CMakeLists.txt deleted file mode 100644 index 51a928cd..00000000 --- a/examples/linux/crash_loop_detection/CMakeLists.txt +++ /dev/null @@ -1,10 +0,0 @@ -cmake_minimum_required(VERSION 3.10) -project(crash_loop_detection_linux LANGUAGES CXX) - -if (CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR) - add_subdirectory(crashpad) -endif () - -add_executable(crash_loop_detection_linux crash_loop_detection.cpp) -target_compile_features(crash_loop_detection_linux PRIVATE cxx_std_14) -target_link_libraries(crash_loop_detection_linux PRIVATE client) diff --git a/handler/linux/crash_report_exception_handler.cc b/handler/linux/crash_report_exception_handler.cc index cad1e7a5..37e6e384 100644 --- a/handler/linux/crash_report_exception_handler.cc +++ b/handler/linux/crash_report_exception_handler.cc @@ -24,7 +24,7 @@ #include "minidump/minidump_file_writer.h" #include "snapshot/linux/process_snapshot_linux.h" #include "snapshot/sanitized/process_snapshot_sanitized.h" -#include "util/backtrace/crash_loop_detection.h" +#include "util/backtrace/crash_history.h" #include "util/file/file_helper.h" #include "util/file/file_reader.h" #include "util/file/output_stream_file_writer.h" @@ -132,12 +132,12 @@ bool CrashReportExceptionHandler::HandleException( { auto it = process_annotations_->find("run-uuid"); if (it != process_annotations_->cend()) { - namespace cld = backtrace::crash_loop_detection; + namespace ch = backtrace::crash_history; UUID uuid; uuid.InitializeFromString(it->second); - auto success = cld::CrashLoopDetectionSetCrashed(database_->DatabasePath(), uuid); + auto success = ch::SetCrashed(database_->DatabasePath(), uuid); if (!success) - LOG(ERROR) << "Failed to set crash loop detection data for '" << it->second + LOG(ERROR) << "Failed to set crash history data for '" << it->second << "'"; } } diff --git a/util/CMakeLists.txt b/util/CMakeLists.txt index dffeaacc..d958a11d 100644 --- a/util/CMakeLists.txt +++ b/util/CMakeLists.txt @@ -1,6 +1,6 @@ set(CRASHPAD_UTIL_LIBRARY_FILES - ./backtrace/crash_loop_detection.cc - ./backtrace/crash_loop_detection.h + ./backtrace/crash_history.cc + ./backtrace/crash_history.h ./file/delimited_file_reader.cc ./file/delimited_file_reader.h ./file/directory_reader.h diff --git a/util/backtrace/crash_history.cc b/util/backtrace/crash_history.cc new file mode 100644 index 00000000..222c8cb0 --- /dev/null +++ b/util/backtrace/crash_history.cc @@ -0,0 +1,134 @@ +#include "crash_history.h" + +#include + +#include +#include +#include +#include +#include +#include +#include + +#include "build/build_config.h" +#include "util/string/split_string.h" + +namespace crashpad { +namespace backtrace { +namespace crash_history { + +static auto CsvFileName(const base::FilePath& database) +{ +#if BUILDFLAG(IS_WIN) + return database.value() + L"/crash_history.csv"; +#else + return database.value() + "/crash_history.csv"; +#endif +} + +static std::vector +LoadCrashData(const base::FilePath& database, int max_entries = std::numeric_limits::max()) +{ + std::vector entries; + auto csv_file = CsvFileName(database); + std::ifstream f(csv_file); + + for (std::string line; std::getline(f, line);) { + if (line.size() && line.back() == '\n') + line.pop_back(); + auto split = SplitString(line, ','); + if (split.size() < 3) + continue; + RunEntry entry; + if (!entry.uuid.InitializeFromString(split[0])) + continue; + entry.crashed = split[1] == "1"; + try { + entry.start_time = std::stoll(split[2]); + if (split.size() > 3) { + entry.crash_time = std::stoll(split[3]); + } + } catch (const std::invalid_argument& arg) { + continue; + } + entries.push_back(std::move(entry)); + } + + if (entries.size() > max_entries) + entries.erase(entries.begin(), entries.end() - max_entries); + return entries; +} + +static bool WriteCrashData(const base::FilePath& database, + const std::vector& data) +{ + std::deque lines; + std::ofstream f(CsvFileName(database), std::ios::trunc); + + if (!f) + return false; + + for (const auto& entry : data) { + std::stringstream str; + str << entry.uuid.ToString() << ',' + << (entry.crashed ? "1" : "0") << ',' + << entry.start_time; + if (entry.crash_time) + str << ',' << entry.crash_time.value(); + str << '\n'; + + auto line = str.str(); + if (line.size()) { + if (&entry == &data.back()) + line.pop_back(); + } + f << line; + } + + return true; +} + +bool Append(const base::FilePath& database, UUID uuid, int max_entries) +{ + auto entries = LoadCrashData(database, max_entries - 1); + + time_t current_time = time(nullptr); + RunEntry entry = {uuid, false, current_time, std::nullopt}; + entries.push_back(std::move(entry)); + + return WriteCrashData(database, entries); +} + +bool SetCrashed(const base::FilePath& database, UUID uuid) +{ + auto entries = LoadCrashData(database); + bool success{}; + std::string uuid_str = uuid.ToString(); + + for (auto& entry : entries) { + if (entry.uuid == uuid) { + success = true; + entry.crashed = true; + entry.crash_time = time(nullptr); + break; + } + } + + return success && WriteCrashData(database, entries); +} + +int ConsecutiveCrashesCount(const base::FilePath& database) +{ + auto entries = LoadCrashData(database); + + auto is_crashing = [](const auto& entry) { + return !entry.crashed; + }; + + auto it = std::find_if(entries.rbegin(), entries.rend(), is_crashing); + return std::distance(entries.rbegin(), it); +} + +} // namespace crash_history +} // namespace backtrace +} // namespace crashpad diff --git a/util/backtrace/crash_loop_detection.h b/util/backtrace/crash_history.h similarity index 64% rename from util/backtrace/crash_loop_detection.h rename to util/backtrace/crash_history.h index fdc47705..af88b062 100644 --- a/util/backtrace/crash_loop_detection.h +++ b/util/backtrace/crash_history.h @@ -14,23 +14,35 @@ #pragma once +#include +#include +#include + #include "base/files/file_path.h" #include "util/misc/uuid.h" namespace crashpad { namespace backtrace { -namespace crash_loop_detection { +namespace crash_history { + +struct RunEntry +{ + UUID uuid; + bool crashed; + time_t start_time; + std::optional crash_time; +}; -static constexpr int crash_loop_detection_max_entries = 10; +static constexpr int default_max_entries = 10; -bool CrashLoopDetectionAppend(const base::FilePath& database, - UUID uuid, - int max_entries = crash_loop_detection_max_entries); +bool Append(const base::FilePath& database, + UUID uuid, + int max_entries = default_max_entries); -bool CrashLoopDetectionSetCrashed(const base::FilePath& database, UUID uuid); +bool SetCrashed(const base::FilePath& database, UUID uuid); int ConsecutiveCrashesCount(const base::FilePath& database); -} // crash_loop_detection +} // crash_history } // backtrace } // crashpad diff --git a/util/backtrace/crash_loop_detection.cc b/util/backtrace/crash_loop_detection.cc deleted file mode 100644 index 7c5a74a5..00000000 --- a/util/backtrace/crash_loop_detection.cc +++ /dev/null @@ -1,125 +0,0 @@ -#include "crash_loop_detection.h" - -#include - -#include -#include -#include -#include -#include -#include - -#include "build/build_config.h" -#include "util/string/split_string.h" - -namespace crashpad { -namespace backtrace { -namespace crash_loop_detection { - -static auto CsvFileName(const base::FilePath& database) -{ -#if BUILDFLAG(IS_WIN) - return database.value() + L"/crash_loop_detection.csv"; -#else - return database.value() + "/crash_loop_detection.csv"; -#endif -} - -using CrashData = std::deque>; - -static CrashData LoadCrashData(const base::FilePath& database, - int max_entries = std::numeric_limits::max()) -{ - CrashData lines; - auto csv_file = CsvFileName(database); - std::ifstream f(csv_file); - - for (std::string line; std::getline(f, line);) { - if (line.size() && line.back() == '\n') - line.pop_back(); - lines.push_back(SplitString(line, ',')); - if (lines.size() > max_entries) - lines.pop_front(); - } - return lines; -} - -static bool WriteCrashData(const base::FilePath& database, - const CrashData& data) -{ - std::deque lines; - std::ofstream f(CsvFileName(database), std::ios::trunc); - - if (!f) - return false; - - for (const auto& vec : data) { - std::stringstream str; - for (const auto& el : vec) - str << el << ','; - auto line = str.str(); - if (line.size()) { - if (&vec == &data.back()) - line.pop_back(); - else - line.back() = '\n'; - } - f << line; - } - - return true; -} - -bool CrashLoopDetectionAppend(const base::FilePath& database, - UUID uuid, - int max_entries) -{ - auto lines = LoadCrashData(database, max_entries - 1); - - time_t current_time = time(nullptr); - std::vector new_line = { - std::to_string(current_time), uuid.ToString(), "0"}; - lines.push_back(std::move(new_line)); - - return WriteCrashData(database, lines); -} - -bool CrashLoopDetectionSetCrashed(const base::FilePath& database, UUID uuid) -{ - auto lines = LoadCrashData(database); - bool success{}; - std::string uuid_str = uuid.ToString(); - - for (auto& line : lines) { - if (line.size() < 3) - continue; - if (line[1] == uuid_str) { - success = true; - line[2] = "1"; - if (line.size() > 3) - line[3] = std::to_string(time(nullptr)); - else - line.push_back(std::to_string(time(nullptr))); - } - } - - return success && WriteCrashData(database, lines); -} - -int ConsecutiveCrashesCount(const base::FilePath& database) -{ - auto lines = LoadCrashData(database); - - auto is_crashing = [](const auto& line) { - if (line.size() >= 3 && line[2] != "0") - return false; - return true; - }; - - auto it = std::find_if(lines.rbegin(), lines.rend(), is_crashing); - return std::distance(lines.rbegin(), it); -} - -} // namespace crash_loop_detection -} // namespace backtrace -} // namespace crashpad From 7d7f38c362eda7b4bb3733630be8dc1bfdc114e0 Mon Sep 17 00:00:00 2001 From: krzaq Date: Mon, 22 May 2023 13:11:16 +0200 Subject: [PATCH 2/3] fix test names --- backtrace/test/test_linux.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/backtrace/test/test_linux.rb b/backtrace/test/test_linux.rb index a9960537..f6e9f967 100755 --- a/backtrace/test/test_linux.rb +++ b/backtrace/test/test_linux.rb @@ -15,8 +15,8 @@ def test_crashpad_uploads assert_equal payload['format'], 'minidump' end - def test_crash_loop_detection - exe = 'examples/linux/crash_loop_detection/crash_loop_detection_linux' + def test_crash_history + exe = 'examples/linux/crash_history/crash_history_linux' ce = Crashpad::Execution.new executable: exe Dir.mktmpdir do |dir| From 1065cce1ec51b43f6c3db477e6e3af79e08bb2e3 Mon Sep 17 00:00:00 2001 From: krzaq Date: Mon, 22 May 2023 23:27:36 +0200 Subject: [PATCH 3/3] missed rename. --- client/crashpad_client_linux.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client/crashpad_client_linux.cc b/client/crashpad_client_linux.cc index 5bf87ead..cf3b40f8 100644 --- a/client/crashpad_client_linux.cc +++ b/client/crashpad_client_linux.cc @@ -511,8 +511,8 @@ bool CrashpadClient::IsSafeModeRequired(const base::FilePath& database) int CrashpadClient::ConsecutiveCrashesCount(const base::FilePath& database) { - namespace clc = backtrace::crash_history; - return clc::ConsecutiveCrashesCount(database); + namespace ch = backtrace::crash_history; + return ch::ConsecutiveCrashesCount(database); } #endif