[Lldb-commits] [lldb] [lldb-dap] Improving logging support and formatting. (PR #170731)
via lldb-commits
lldb-commits at lists.llvm.org
Thu Dec 4 11:51:41 PST 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: John Harrison (ashgti)
<details>
<summary>Changes</summary>
Adjusting logs in a few ways:
* The `DAP_LOG` and `DAP_LOG_ERROR` macros now include the file/line of the log statement.
* Added support for creating a log with a prefix. This simplifies how we create logs for the `lldb_dap::DAP` instance and `lldb_dap::Transport` instance, allowing us to not have to pass the client name around as much.
* Updated logging usage to take the `lldb_dap::Log` as a reference but it now defaults to `/dev/null` if not configured. This ensures more uniform access to the logger, even if its not written anywhere.
---
Patch is 29.05 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/170731.diff
15 Files Affected:
- (modified) lldb/tools/lldb-dap/DAP.cpp (+15-26)
- (modified) lldb/tools/lldb-dap/DAP.h (+3-6)
- (modified) lldb/tools/lldb-dap/DAPLog.cpp (+15-5)
- (modified) lldb/tools/lldb-dap/DAPLog.h (+29-16)
- (modified) lldb/tools/lldb-dap/DAPSessionManager.cpp (+2-1)
- (modified) lldb/tools/lldb-dap/EventHelper.cpp (+7-7)
- (modified) lldb/tools/lldb-dap/EventHelper.h (+1-10)
- (modified) lldb/tools/lldb-dap/Handler/CompletionsHandler.cpp (+1-1)
- (modified) lldb/tools/lldb-dap/Transport.cpp (+5-5)
- (modified) lldb/tools/lldb-dap/Transport.h (+3-4)
- (modified) lldb/tools/lldb-dap/tool/lldb-dap.cpp (+26-25)
- (modified) lldb/unittests/DAP/CMakeLists.txt (+1)
- (added) lldb/unittests/DAP/DAPLogTest.cpp (+46)
- (modified) lldb/unittests/DAP/TestBase.cpp (+5-4)
- (modified) lldb/unittests/DAP/TestBase.h (+3)
``````````diff
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index 6971bfea5c128..58c9922214583 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -122,7 +122,7 @@ static std::string capitalize(llvm::StringRef str) {
llvm::StringRef DAP::debug_adapter_path = "";
-DAP::DAP(Log *log, const ReplMode default_repl_mode,
+DAP::DAP(Log &log, const ReplMode default_repl_mode,
std::vector<std::string> pre_init_commands, bool no_lldbinit,
llvm::StringRef client_name, DAPTransport &transport, MainLoop &loop)
: log(log), transport(transport), broadcaster("lldb-dap"),
@@ -134,8 +134,6 @@ DAP::DAP(Log *log, const ReplMode default_repl_mode,
RegisterRequests();
}
-DAP::~DAP() = default;
-
void DAP::PopulateExceptionBreakpoints() {
if (lldb::SBDebugger::SupportsLanguage(lldb::eLanguageTypeC_plus_plus)) {
exception_breakpoints.emplace_back(*this, "cpp_catch", "C++ Catch",
@@ -263,8 +261,7 @@ void DAP::SendJSON(const llvm::json::Value &json) {
Message message;
llvm::json::Path::Root root;
if (!fromJSON(json, message, root)) {
- DAP_LOG_ERROR(log, root.getError(), "({1}) encoding failed: {0}",
- m_client_name);
+ DAP_LOG_ERROR(log, root.getError(), "encoding failed: {0}");
return;
}
Send(message);
@@ -285,15 +282,13 @@ Id DAP::Send(const Message &message) {
if (const protocol::Event *event = std::get_if<protocol::Event>(&msg)) {
if (llvm::Error err = transport.Send(*event))
- DAP_LOG_ERROR(log, std::move(err), "({0}) sending event failed",
- m_client_name);
+ DAP_LOG_ERROR(log, std::move(err), "sending event failed: {0}");
return event->seq;
}
if (const Request *req = std::get_if<Request>(&msg)) {
if (llvm::Error err = transport.Send(*req))
- DAP_LOG_ERROR(log, std::move(err), "({0}) sending request failed",
- m_client_name);
+ DAP_LOG_ERROR(log, std::move(err), "sending request failed: {0}");
return req->seq;
}
@@ -313,8 +308,7 @@ Id DAP::Send(const Message &message) {
})
: transport.Send(*resp);
if (err)
- DAP_LOG_ERROR(log, std::move(err), "({0}) sending response failed",
- m_client_name);
+ DAP_LOG_ERROR(log, std::move(err), "sending response failed: {0}");
return resp->seq;
}
@@ -857,8 +851,7 @@ bool DAP::HandleObject(const Message &M) {
dispatcher.Set("error",
llvm::Twine("unhandled-command:" + req->command).str());
- DAP_LOG(log, "({0}) error: unhandled command '{1}'", m_client_name,
- req->command);
+ DAP_LOG(log, "error: unhandled command '{0}'", req->command);
return false; // Fail
}
@@ -1004,35 +997,33 @@ void DAP::Received(const protocol::Request &request) {
// effort attempt to interrupt.
std::lock_guard<std::mutex> guard(m_active_request_mutex);
if (m_active_request && cancel_args->requestId == m_active_request->seq) {
- DAP_LOG(log, "({0}) interrupting inflight request (command={1} seq={2})",
- m_client_name, m_active_request->command, m_active_request->seq);
+ DAP_LOG(log, "interrupting inflight request (command={0} seq={1})",
+ m_active_request->command, m_active_request->seq);
debugger.RequestInterrupt();
}
}
std::lock_guard<std::mutex> guard(m_queue_mutex);
- DAP_LOG(log, "({0}) queued (command={1} seq={2})", m_client_name,
- request.command, request.seq);
+ DAP_LOG(log, "queued (command={0} seq={1})", request.command, request.seq);
m_queue.push_back(request);
m_queue_cv.notify_one();
}
void DAP::Received(const protocol::Response &response) {
std::lock_guard<std::mutex> guard(m_queue_mutex);
- DAP_LOG(log, "({0}) queued (command={1} seq={2})", m_client_name,
- response.command, response.request_seq);
+ DAP_LOG(log, "queued (command={0} seq={1})", response.command,
+ response.request_seq);
m_queue.push_back(response);
m_queue_cv.notify_one();
}
void DAP::OnError(llvm::Error error) {
- DAP_LOG_ERROR(log, std::move(error), "({1}) received error: {0}",
- m_client_name);
+ DAP_LOG_ERROR(log, std::move(error), "transport error: {0}");
TerminateLoop(/*failed=*/true);
}
void DAP::OnClosed() {
- DAP_LOG(log, "({0}) received EOF", m_client_name);
+ DAP_LOG(log, "transport closed");
TerminateLoop();
}
@@ -1058,16 +1049,14 @@ void DAP::TransportHandler() {
auto handle = transport.RegisterMessageHandler(m_loop, *this);
if (!handle) {
DAP_LOG_ERROR(log, handle.takeError(),
- "({1}) registering message handler failed: {0}",
- m_client_name);
+ "registering message handler failed: {0}");
std::lock_guard<std::mutex> guard(m_queue_mutex);
m_error_occurred = true;
return;
}
if (Status status = m_loop.Run(); status.Fail()) {
- DAP_LOG_ERROR(log, status.takeError(), "({1}) MainLoop run failed: {0}",
- m_client_name);
+ DAP_LOG_ERROR(log, status.takeError(), "MainLoop run failed: {0}");
std::lock_guard<std::mutex> guard(m_queue_mutex);
m_error_occurred = true;
return;
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index b5f2a57d9dc5f..231eb0b1cf2d1 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -23,7 +23,6 @@
#include "Transport.h"
#include "Variables.h"
#include "lldb/API/SBBroadcaster.h"
-#include "lldb/API/SBCommandInterpreter.h"
#include "lldb/API/SBDebugger.h"
#include "lldb/API/SBError.h"
#include "lldb/API/SBFile.h"
@@ -33,11 +32,9 @@
#include "lldb/API/SBTarget.h"
#include "lldb/API/SBThread.h"
#include "lldb/Host/MainLoop.h"
-#include "lldb/Utility/Status.h"
#include "lldb/lldb-types.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/DenseSet.h"
-#include "llvm/ADT/FunctionExtras.h"
#include "llvm/ADT/SmallSet.h"
#include "llvm/ADT/StringMap.h"
#include "llvm/ADT/StringRef.h"
@@ -88,7 +85,7 @@ struct DAP final : public DAPTransport::MessageHandler {
/// Path to the lldb-dap binary itself.
static llvm::StringRef debug_adapter_path;
- Log *log;
+ Log &log;
DAPTransport &transport;
lldb::SBFile in;
OutputRedirector out;
@@ -194,12 +191,12 @@ struct DAP final : public DAPTransport::MessageHandler {
/// Transport for this debug session.
/// \param[in] loop
/// Main loop associated with this instance.
- DAP(Log *log, const ReplMode default_repl_mode,
+ DAP(Log &log, const ReplMode default_repl_mode,
std::vector<std::string> pre_init_commands, bool no_lldbinit,
llvm::StringRef client_name, DAPTransport &transport,
lldb_private::MainLoop &loop);
- ~DAP();
+ ~DAP() override = default;
/// DAP is not copyable.
/// @{
diff --git a/lldb/tools/lldb-dap/DAPLog.cpp b/lldb/tools/lldb-dap/DAPLog.cpp
index f66784e197518..8907d02d3e9f4 100644
--- a/lldb/tools/lldb-dap/DAPLog.cpp
+++ b/lldb/tools/lldb-dap/DAPLog.cpp
@@ -8,22 +8,32 @@
#include "DAPLog.h"
#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Path.h"
#include "llvm/Support/raw_ostream.h"
#include <chrono>
#include <mutex>
-#include <system_error>
using namespace llvm;
namespace lldb_dap {
-Log::Log(StringRef filename, std::error_code &EC) : m_stream(filename, EC) {}
+void Log::Emit(StringRef message) {
+ std::lock_guard<Log::Mutex> lock(m_mutex);
+ std::chrono::duration<double> now{
+ std::chrono::system_clock::now().time_since_epoch()};
+ m_stream << formatv("{0:f9} ", now.count()).str() << m_prefix << message
+ << "\n";
+ m_stream.flush();
+}
-void Log::WriteMessage(StringRef message) {
- std::lock_guard<std::mutex> lock(m_mutex);
+void Log::Emit(StringRef file, size_t line, StringRef message) {
+ std::lock_guard<Log::Mutex> lock(m_mutex);
std::chrono::duration<double> now{
std::chrono::system_clock::now().time_since_epoch()};
- m_stream << formatv("{0:f9} ", now.count()).str() << message << "\n";
+ m_stream << formatv("{0:f9} {1}:{2} ", now.count(), sys::path::filename(file),
+ line)
+ .str()
+ << m_prefix << message << "\n";
m_stream.flush();
}
diff --git a/lldb/tools/lldb-dap/DAPLog.h b/lldb/tools/lldb-dap/DAPLog.h
index 484001a9b1628..05cca7d4669a9 100644
--- a/lldb/tools/lldb-dap/DAPLog.h
+++ b/lldb/tools/lldb-dap/DAPLog.h
@@ -11,33 +11,28 @@
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Error.h"
-#include "llvm/Support/FormatAdapters.h"
#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/raw_ostream.h"
#include <mutex>
#include <string>
-#include <system_error>
// Write a message to log, if logging is enabled.
#define DAP_LOG(log, ...) \
do { \
- ::lldb_dap::Log *log_private = (log); \
- if (log_private) { \
- log_private->WriteMessage(::llvm::formatv(__VA_ARGS__).str()); \
- } \
+ ::lldb_dap::Log &log_private = (log); \
+ log_private.Emit(__FILE__, __LINE__, ::llvm::formatv(__VA_ARGS__).str()); \
} while (0)
// Write message to log, if error is set. In the log message refer to the error
// with {0}. Error is cleared regardless of whether logging is enabled.
#define DAP_LOG_ERROR(log, error, ...) \
do { \
- ::lldb_dap::Log *log_private = (log); \
+ ::lldb_dap::Log &log_private = (log); \
::llvm::Error error_private = (error); \
- if (log_private && error_private) { \
- log_private->WriteMessage( \
+ if (error_private) \
+ log_private.Emit( \
+ __FILE__, __LINE__, \
::lldb_dap::FormatError(::std::move(error_private), __VA_ARGS__)); \
- } else \
- ::llvm::consumeError(::std::move(error_private)); \
} while (0)
namespace lldb_dap {
@@ -46,14 +41,32 @@ namespace lldb_dap {
/// `DAP_LOG_ERROR` helpers.
class Log final {
public:
- /// Creates a log file with the given filename.
- Log(llvm::StringRef filename, std::error_code &EC);
+ using Mutex = std::recursive_mutex;
- void WriteMessage(llvm::StringRef message);
+ Log(llvm::raw_ostream &stream, Mutex &mutex)
+ : m_stream(stream), m_mutex(mutex) {}
+ Log(llvm::StringRef prefix, const Log &log)
+ : m_prefix(prefix), m_stream(log.m_stream), m_mutex(log.m_mutex) {}
+
+ /// Retuns a new Log instance with the associated prefix for all messages.
+ inline Log WithPrefix(llvm::StringRef prefix) const {
+ std::string full_prefix =
+ m_prefix.empty() ? prefix.str() : m_prefix + " " + prefix.str();
+ full_prefix += " ";
+ return Log(full_prefix, *this);
+ }
+
+ /// Emit writes a message to the underlying stream.
+ void Emit(llvm::StringRef message);
+
+ /// Emit writes a message to the underlying stream, including the file and
+ /// line the message originated from.
+ void Emit(llvm::StringRef file, size_t line, llvm::StringRef message);
private:
- std::mutex m_mutex;
- llvm::raw_fd_ostream m_stream;
+ std::string m_prefix;
+ llvm::raw_ostream &m_stream;
+ Mutex &m_mutex;
};
template <typename... Args>
diff --git a/lldb/tools/lldb-dap/DAPSessionManager.cpp b/lldb/tools/lldb-dap/DAPSessionManager.cpp
index d5440ffd64597..3fbdc26fc4cda 100644
--- a/lldb/tools/lldb-dap/DAPSessionManager.cpp
+++ b/lldb/tools/lldb-dap/DAPSessionManager.cpp
@@ -110,7 +110,8 @@ DAPSessionManager::GetEventThreadForDebugger(lldb::SBDebugger debugger,
auto new_thread_sp = std::make_shared<ManagedEventThread>(
requesting_dap->broadcaster,
std::thread(EventThread, debugger, requesting_dap->broadcaster,
- requesting_dap->m_client_name, requesting_dap->log));
+ requesting_dap->m_client_name,
+ std::ref(requesting_dap->log)));
m_debugger_event_threads[debugger_id] = new_thread_sp;
return new_thread_sp;
}
diff --git a/lldb/tools/lldb-dap/EventHelper.cpp b/lldb/tools/lldb-dap/EventHelper.cpp
index bdb6bb55fe168..5fd13925be209 100644
--- a/lldb/tools/lldb-dap/EventHelper.cpp
+++ b/lldb/tools/lldb-dap/EventHelper.cpp
@@ -324,8 +324,8 @@ void SendMemoryEvent(DAP &dap, lldb::SBValue variable) {
// the original DAP::Handle*Event pattern while supporting multi-session
// debugging.
-void HandleProcessEvent(const lldb::SBEvent &event, bool &process_exited,
- Log *log) {
+static void HandleProcessEvent(const lldb::SBEvent &event, bool &process_exited,
+ Log &log) {
lldb::SBProcess process = lldb::SBProcess::GetProcessFromEvent(event);
// Find the DAP instance that owns this process's target.
@@ -393,7 +393,7 @@ void HandleProcessEvent(const lldb::SBEvent &event, bool &process_exited,
}
}
-void HandleTargetEvent(const lldb::SBEvent &event, Log *log) {
+static void HandleTargetEvent(const lldb::SBEvent &event, Log &log) {
lldb::SBTarget target = lldb::SBTarget::GetTargetFromEvent(event);
// Find the DAP instance that owns this target.
@@ -480,7 +480,7 @@ void HandleTargetEvent(const lldb::SBEvent &event, Log *log) {
}
}
-void HandleBreakpointEvent(const lldb::SBEvent &event, Log *log) {
+void HandleBreakpointEvent(const lldb::SBEvent &event, Log &log) {
const uint32_t event_mask = event.GetType();
if (!(event_mask & lldb::SBTarget::eBroadcastBitBreakpointChanged))
return;
@@ -529,7 +529,7 @@ void HandleBreakpointEvent(const lldb::SBEvent &event, Log *log) {
}
}
-void HandleThreadEvent(const lldb::SBEvent &event, Log *log) {
+static void HandleThreadEvent(const lldb::SBEvent &event, Log &log) {
uint32_t event_type = event.GetType();
if (!(event_type & lldb::SBThread::eBroadcastBitStackChanged))
@@ -550,7 +550,7 @@ void HandleThreadEvent(const lldb::SBEvent &event, Log *log) {
thread.GetThreadID());
}
-void HandleDiagnosticEvent(const lldb::SBEvent &event, Log *log) {
+static void HandleDiagnosticEvent(const lldb::SBEvent &event, Log &log) {
// Global debugger events - send to all DAP instances.
std::vector<DAP *> active_instances =
DAPSessionManager::GetInstance().GetActiveSessions();
@@ -588,7 +588,7 @@ void HandleDiagnosticEvent(const lldb::SBEvent &event, Log *log) {
// them prevent multiple threads from writing simultaneously so no locking
// is required.
void EventThread(lldb::SBDebugger debugger, lldb::SBBroadcaster broadcaster,
- llvm::StringRef client_name, Log *log) {
+ llvm::StringRef client_name, Log &log) {
llvm::set_thread_name("lldb.DAP.client." + client_name + ".event_handler");
lldb::SBListener listener = debugger.GetListener();
broadcaster.AddListener(listener, eBroadcastBitStopEventThread);
diff --git a/lldb/tools/lldb-dap/EventHelper.h b/lldb/tools/lldb-dap/EventHelper.h
index 3beba2629b2e3..b46d5aef3581f 100644
--- a/lldb/tools/lldb-dap/EventHelper.h
+++ b/lldb/tools/lldb-dap/EventHelper.h
@@ -51,16 +51,7 @@ void SendMemoryEvent(DAP &dap, lldb::SBValue variable);
/// \param client_name The client name for thread naming/logging purposes.
/// \param log The log instance for logging.
void EventThread(lldb::SBDebugger debugger, lldb::SBBroadcaster broadcaster,
- llvm::StringRef client_name, Log *log);
-
-/// Event handler functions called by EventThread.
-/// These handlers extract the necessary objects from events and find the
-/// appropriate DAP instance to handle them.
-void HandleProcessEvent(const lldb::SBEvent &event, bool &done, Log *log);
-void HandleTargetEvent(const lldb::SBEvent &event, Log *log);
-void HandleBreakpointEvent(const lldb::SBEvent &event, Log *log);
-void HandleThreadEvent(const lldb::SBEvent &event, Log *log);
-void HandleDiagnosticEvent(const lldb::SBEvent &event, Log *log);
+ llvm::StringRef client_name, Log &log);
} // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/Handler/CompletionsHandler.cpp b/lldb/tools/lldb-dap/Handler/CompletionsHandler.cpp
index de9a15dcb73f4..27ac01a37e4de 100644
--- a/lldb/tools/lldb-dap/Handler/CompletionsHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/CompletionsHandler.cpp
@@ -7,11 +7,11 @@
//===----------------------------------------------------------------------===//
#include "DAP.h"
-#include "JSONUtils.h"
#include "Protocol/ProtocolRequests.h"
#include "Protocol/ProtocolTypes.h"
#include "RequestHandler.h"
#include "lldb/API/SBStringList.h"
+#include "lldb/SBCommandInterpreter.h"
using namespace llvm;
using namespace lldb_dap;
diff --git a/lldb/tools/lldb-dap/Transport.cpp b/lldb/tools/lldb-dap/Transport.cpp
index 8f71f88cae1f7..b3512385d6579 100644
--- a/lldb/tools/lldb-dap/Transport.cpp
+++ b/lldb/tools/lldb-dap/Transport.cpp
@@ -17,13 +17,13 @@ using namespace lldb_private;
namespace lldb_dap {
-Transport::Transport(llvm::StringRef client_name, lldb_dap::Log *log,
- lldb::IOObjectSP input, lldb::IOObjectSP output)
- : HTTPDelimitedJSONTransport(input, output), m_client_name(client_name),
- m_log(log) {}
+Transport::Transport(lldb_dap::Log &log, lldb::IOObjectSP input,
+ lldb::IOObjectSP output)
+ : HTTPDelimitedJSONTransport(input, output), m_log(log) {}
void Transport::Log(llvm::StringRef message) {
- DAP_LOG(m_log, "({0}) {1}", m_client_name, message);
+ // Emit the message directly, since this log was forwarded.
+ m_log.Emit(message);
}
} // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/Transport.h b/lldb/tools/lldb-dap/Transport.h
index 58c48c133f9cb..b20a93475d2dd 100644
--- a/lldb/tools/lldb-dap/Transport.h
+++ b/lldb/tools/lldb-dap/Transport.h
@@ -35,15 +35,14 @@ class Transport final
: public lldb_private::transport::HTTPDelimitedJSONTransport<
ProtocolDescriptor> {
public:
- Transport(llvm::StringRef client_name, lldb_dap::Log *log,
- lldb::IOObjectSP input, lldb::IOObjectSP output);
+ Transport(lldb_dap::Log &log, lldb::IOObjectSP input,
+ lldb::IOObjectSP output);
virtual ~Transport() = default;
void Log(llvm::StringRef message) override;
private:
- llvm::StringRef m_client_name;
- lldb_dap::Log *m_log;
+ lldb_dap::Log &m_log;
};
} // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/tool/lldb-dap.cpp b/lldb/tools/lldb-dap/tool/lldb-dap.cpp
index 27516b2a25678..a427ecdef5080 100644
--- a/lldb/tools/lldb-dap/tool/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/tool/lldb-dap.cpp
@@ -11,6 +11,7 @@
#include "DAPLog.h"
#include "EventHelper.h"
#include "Handler/RequestHandler.h"
+#include "Handler/ResponseHandler.h"
#include "RunInTerminal.h"
#include "Transport.h"
#include "lldb/API/SBDebugger.h"
@@ -50,9 +51,7 @@
#include <cstddef>
#include <cstdio>
#include <cstdlib>
-#include <exception>
#include <fcntl.h>
-#include <map>
#include <memory>
#include <mutex>
#include <string>
@@ -410,7 +409,7 @@ validateConnection(llvm::StringRef conn) {
}
static llvm::Error serveConnection(
- const Socket::SocketProtocol &protocol, const std::string &name, Log *log,
+ const Socket::SocketProtocol &protocol, const std::string &name, Log &log,
const ReplMode default_repl_mode,
const std::vector<std::string> &pre_init_commands, bool no_lldbinit,
std::optional<std::chrono::seconds> connection_timeout_seconds) {
@@ -446,7 +445,7 @@ static llvm::Error serveConnection(
connection_timeout_seconds.value());
std::condition_variable dap_sessions_condition;
unsigned int clientCount = 0;
- auto handle = listener->Accept(g_loop, [=, &clientCo...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/170731
More information about the lldb-commits
mailing list