Author: Jonas Devlieghere Date: 2026-06-26T10:49:55-07:00 New Revision: 9f34f1caf558c27c3d5bf879d7514f3c7fab13f8
URL: https://github.com/llvm/llvm-project/commit/9f34f1caf558c27c3d5bf879d7514f3c7fab13f8 DIFF: https://github.com/llvm/llvm-project/commit/9f34f1caf558c27c3d5bf879d7514f3c7fab13f8.diff LOG: [lldb] Remove the Diagnostics callback mechanism (#206132) The Diagnostics framework had a callback registry (AddCallback / RemoveCallback) so subsystems could contribute files to a diagnostics directory, intended to also run during crash handling. That crash-time path never materialized, and the sole registered callback was the Debugger copying its file-backed logs. If you had no logging enabled, the directory would be empty, confusing the users. Remove the registry and the callback loop in Diagnostics::Create (which now just writes the in-memory log), and expose the log copying as Debugger::CopyLogFilesToDirectory, which "diagnostics dump" calls directly. The dump command now copies the invoking debugger's logs rather than every debugger's, which is the more useful behavior I want to double down on. Added: Modified: lldb/include/lldb/Core/Debugger.h lldb/include/lldb/Utility/Diagnostics.h lldb/source/Commands/CommandObjectDiagnostics.cpp lldb/source/Core/Debugger.cpp lldb/source/Utility/Diagnostics.cpp Removed: ################################################################################ diff --git a/lldb/include/lldb/Core/Debugger.h b/lldb/include/lldb/Core/Debugger.h index 179ef0f38d940..b7d66f5795d43 100644 --- a/lldb/include/lldb/Core/Debugger.h +++ b/lldb/include/lldb/Core/Debugger.h @@ -266,6 +266,10 @@ class Debugger : public std::enable_shared_from_this<Debugger>, void SetLoggingCallback(lldb::LogOutputCallback log_callback, void *baton); + /// Copy this debugger's file-backed log files into the given directory, for + /// inclusion in a diagnostics bundle. Best-effort; failures are skipped. + void CopyLogFilesToDirectory(const FileSpec &dir); + Status SetPropertyValue(const ExecutionContext *exe_ctx, VarSetOperationType op, llvm::StringRef property_path, llvm::StringRef value) override; @@ -805,7 +809,6 @@ class Debugger : public std::enable_shared_from_this<Debugger>, lldb::ListenerSP m_forward_listener_sp; llvm::once_flag m_clear_once; lldb::TargetSP m_dummy_target_sp; - Diagnostics::CallbackID m_diagnostics_callback_id; /// Bookkeeping for command line progress events. /// @{ diff --git a/lldb/include/lldb/Utility/Diagnostics.h b/lldb/include/lldb/Utility/Diagnostics.h index c2e4b44350a37..e32ffa5233f11 100644 --- a/lldb/include/lldb/Utility/Diagnostics.h +++ b/lldb/include/lldb/Utility/Diagnostics.h @@ -11,29 +11,25 @@ #include "lldb/Utility/FileSpec.h" #include "lldb/Utility/Log.h" -#include "llvm/ADT/SmallVector.h" -#include "llvm/ADT/StringSet.h" #include "llvm/Support/Error.h" -#include <functional> -#include <mutex> #include <optional> -#include <vector> namespace lldb_private { -/// Diagnostics are a collection of files to help investigate bugs and -/// troubleshoot issues. Any part of the debugger can register itself with the -/// help of a callback to emit one or more files into the diagnostic directory. +/// Diagnostics maintain an always-on, in-memory log of recent diagnostic +/// messages that can be written out to help investigate bugs and troubleshoot +/// issues. class Diagnostics { public: Diagnostics(); ~Diagnostics(); - /// Gather diagnostics in the given directory. + /// Write the in-memory diagnostic log into the given directory. llvm::Error Create(const FileSpec &dir); - /// Gather diagnostics and print a message to the given output stream. + /// Write the diagnostic log into a directory and print a message to the given + /// output stream. /// @{ bool Dump(llvm::raw_ostream &stream); bool Dump(llvm::raw_ostream &stream, const FileSpec &dir); @@ -41,12 +37,6 @@ class Diagnostics { void Report(llvm::StringRef message); - using Callback = std::function<llvm::Error(const FileSpec &)>; - using CallbackID = uint64_t; - - CallbackID AddCallback(Callback callback); - void RemoveCallback(CallbackID id); - static Diagnostics &Instance(); static bool Enabled(); @@ -62,23 +52,6 @@ class Diagnostics { llvm::Error DumpDiangosticsLog(const FileSpec &dir) const; RotatingLogHandler m_log_handler; - - struct CallbackEntry { - CallbackEntry(CallbackID id, Callback callback) - : id(id), callback(std::move(callback)) {} - CallbackID id; - Callback callback; - }; - - /// Monotonically increasing callback identifier. Unique per Diagnostic - /// instance. - CallbackID m_callback_id; - - /// List of callback entries. - llvm::SmallVector<CallbackEntry, 4> m_callbacks; - - /// Mutex to protect callback list and callback identifier. - std::mutex m_callbacks_mutex; }; } // namespace lldb_private diff --git a/lldb/source/Commands/CommandObjectDiagnostics.cpp b/lldb/source/Commands/CommandObjectDiagnostics.cpp index b565e16e76b53..3e942c98c8180 100644 --- a/lldb/source/Commands/CommandObjectDiagnostics.cpp +++ b/lldb/source/Commands/CommandObjectDiagnostics.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "CommandObjectDiagnostics.h" +#include "lldb/Core/Debugger.h" #include "lldb/Host/OptionParser.h" #include "lldb/Interpreter/CommandOptionArgumentTable.h" #include "lldb/Interpreter/CommandReturnObject.h" @@ -93,6 +94,10 @@ class CommandObjectDiagnosticsDump : public CommandObjectParsed { return; } + // Copy this debugger's file-backed logs into the directory. This used to be + // done by a Diagnostics callback registered by the Debugger. + GetDebugger().CopyLogFilesToDirectory(*directory); + result.GetOutputStream() << "diagnostics written to " << *directory << '\n'; result.SetStatus(eReturnStatusSuccessFinishResult); diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp index 9a75c5eb407c5..b2e6fe66dc29e 100644 --- a/lldb/source/Core/Debugger.cpp +++ b/lldb/source/Core/Debugger.cpp @@ -1103,22 +1103,6 @@ Debugger::Debugger(lldb::LogOutputCallback log_callback, void *baton) if (!GetOutputFileSP()->GetIsTerminalWithColors()) disable_color(); - if (Diagnostics::Enabled()) { - m_diagnostics_callback_id = Diagnostics::Instance().AddCallback( - [this](const FileSpec &dir) -> llvm::Error { - for (auto &entry : m_stream_handlers) { - llvm::StringRef log_path = entry.first(); - llvm::StringRef file_name = llvm::sys::path::filename(log_path); - FileSpec destination = dir.CopyByAppendingPathComponent(file_name); - std::error_code ec = - llvm::sys::fs::copy_file(log_path, destination.GetPath()); - if (ec) - return llvm::errorCodeToError(ec); - } - return llvm::Error::success(); - }); - } - #if defined(_WIN32) && defined(ENABLE_VIRTUAL_TERMINAL_PROCESSING) // Enabling use of ANSI color codes because LLDB is using them to highlight // text. @@ -1164,9 +1148,6 @@ void Debugger::Clear() { GetInputFile().Close(); m_command_interpreter_up->Clear(); - - if (Diagnostics::Enabled()) - Diagnostics::Instance().RemoveCallback(m_diagnostics_callback_id); }); } @@ -1692,6 +1673,16 @@ void Debugger::SetLoggingCallback(lldb::LogOutputCallback log_callback, std::make_shared<CallbackLogHandler>(log_callback, baton); } +void Debugger::CopyLogFilesToDirectory(const FileSpec &dir) { + for (auto &entry : m_stream_handlers) { + llvm::StringRef log_path = entry.first(); + llvm::StringRef file_name = llvm::sys::path::filename(log_path); + FileSpec destination = dir.CopyByAppendingPathComponent(file_name); + // Best-effort: skip logs that can't be copied rather than aborting. + llvm::sys::fs::copy_file(log_path, destination.GetPath()); + } +} + void Debugger::SetDestroyCallback( lldb_private::DebuggerDestroyCallback destroy_callback, void *baton) { std::lock_guard<std::mutex> guard(m_destroy_callback_mutex); diff --git a/lldb/source/Utility/Diagnostics.cpp b/lldb/source/Utility/Diagnostics.cpp index b2a08165dd6ca..175011783ead2 100644 --- a/lldb/source/Utility/Diagnostics.cpp +++ b/lldb/source/Utility/Diagnostics.cpp @@ -43,19 +43,6 @@ Diagnostics::Diagnostics() : m_log_handler(g_num_log_messages) {} Diagnostics::~Diagnostics() {} -Diagnostics::CallbackID Diagnostics::AddCallback(Callback callback) { - std::lock_guard<std::mutex> guard(m_callbacks_mutex); - CallbackID id = m_callback_id++; - m_callbacks.emplace_back(id, callback); - return id; -} - -void Diagnostics::RemoveCallback(CallbackID id) { - std::lock_guard<std::mutex> guard(m_callbacks_mutex); - llvm::erase_if(m_callbacks, - [id](const CallbackEntry &e) { return e.id == id; }); -} - bool Diagnostics::Dump(raw_ostream &stream) { Expected<FileSpec> diagnostics_dir = CreateUniqueDirectory(); if (!diagnostics_dir) { @@ -92,11 +79,6 @@ Error Diagnostics::Create(const FileSpec &dir) { if (Error err = DumpDiangosticsLog(dir)) return err; - for (CallbackEntry e : m_callbacks) { - if (Error err = e.callback(dir)) - return err; - } - return Error::success(); } _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
