[Lldb-commits] [lldb] [lldb-dap] Ensure logging statements are written as a single chunk. (PR #131916)

John Harrison via lldb-commits lldb-commits at lists.llvm.org
Wed Mar 19 08:52:09 PDT 2025


https://github.com/ashgti updated https://github.com/llvm/llvm-project/pull/131916

>From 6aa311afe9832bbd4a4118874e411243b05e490d Mon Sep 17 00:00:00 2001
From: John Harrison <harjohn at google.com>
Date: Tue, 18 Mar 2025 14:03:12 -0700
Subject: [PATCH 1/6] [lldb-dap] Ensure logging statements are written as a
 single atomic chunk.

Previously, it was possible for a log statement to have two messages interspersed since the timestamp and log statement were two different writes to the log stream.

Instead, combine the logging into a single buffer first before printing.
---
 lldb/tools/lldb-dap/DAPLog.h | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/lldb/tools/lldb-dap/DAPLog.h b/lldb/tools/lldb-dap/DAPLog.h
index 75a0a7d63a4f1..2574506739fea 100644
--- a/lldb/tools/lldb-dap/DAPLog.h
+++ b/lldb/tools/lldb-dap/DAPLog.h
@@ -23,8 +23,12 @@
     if (log_private) {                                                         \
       ::std::chrono::duration<double> now{                                     \
           ::std::chrono::system_clock::now().time_since_epoch()};              \
-      *log_private << ::llvm::formatv("{0:f9} ", now.count()).str()            \
-                   << ::llvm::formatv(__VA_ARGS__).str() << std::endl;         \
+      ::std::string out;                                                       \
+      ::llvm::raw_string_ostream os(out);                                      \
+      os << ::llvm::formatv("{0:f9} ", now.count()).str()                      \
+         << ::llvm::formatv(__VA_ARGS__).str() << "\n";                        \
+      *log_private << out;                                                     \
+      log_private->flush();                                                    \
     }                                                                          \
   } while (0)
 
@@ -37,10 +41,13 @@
     if (log_private && error_private) {                                        \
       ::std::chrono::duration<double> now{                                     \
           std::chrono::system_clock::now().time_since_epoch()};                \
-      *log_private << ::llvm::formatv("{0:f9} ", now.count()).str()            \
-                   << ::lldb_dap::FormatError(::std::move(error_private),      \
-                                              __VA_ARGS__)                     \
-                   << std::endl;                                               \
+      ::std::string out;                                                       \
+      ::llvm::raw_string_ostream os(out);                                      \
+      os << ::llvm::formatv("{0:f9} ", now.count()).str()                      \
+         << ::lldb_dap::FormatError(::std::move(error_private), __VA_ARGS__)   \
+         << "\n";                                                              \
+      *log_private << out;                                                     \
+      log_private->flush();                                                    \
     } else                                                                     \
       ::llvm::consumeError(::std::move(error_private));                        \
   } while (0)

>From d4a03fa5be456436518f35f0ef88561da6b1ffcf Mon Sep 17 00:00:00 2001
From: John Harrison <harjohn at google.com>
Date: Tue, 18 Mar 2025 16:39:27 -0700
Subject: [PATCH 2/6] Using a small `class Log` wrapper around a mutex +
 ofstream to protect log statements.

---
 lldb/tools/lldb-dap/DAP.cpp         |  2 +-
 lldb/tools/lldb-dap/DAP.h           |  6 +++---
 lldb/tools/lldb-dap/DAPLog.h        | 27 +++++++++++++++++++++------
 lldb/tools/lldb-dap/EventHelper.cpp | 12 +++++-------
 lldb/tools/lldb-dap/Transport.cpp   |  2 +-
 lldb/tools/lldb-dap/Transport.h     |  8 ++++----
 lldb/tools/lldb-dap/lldb-dap.cpp    |  6 +++---
 7 files changed, 38 insertions(+), 25 deletions(-)

diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index a1e2187288768..202c80e19baac 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -68,7 +68,7 @@ const char DEV_NULL[] = "/dev/null";
 
 namespace lldb_dap {
 
-DAP::DAP(llvm::StringRef path, std::ofstream *log,
+DAP::DAP(llvm::StringRef path, Log *log,
          const ReplMode default_repl_mode,
          std::vector<std::string> pre_init_commands, Transport &transport)
     : debug_adapter_path(path), log(log), transport(transport),
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index 4c57f9fef3d89..dae7ea20b9f5c 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -144,7 +144,7 @@ struct SendEventRequestHandler : public lldb::SBCommandPluginInterface {
 
 struct DAP {
   llvm::StringRef debug_adapter_path;
-  std::ofstream *log;
+  Log *log;
   Transport &transport;
   lldb::SBFile in;
   OutputRedirector out;
@@ -211,14 +211,14 @@ struct DAP {
   /// \param[in] path
   ///     Path to the lldb-dap binary.
   /// \param[in] log
-  ///     Log file stream, if configured.
+  ///     Log stream, if configured.
   /// \param[in] default_repl_mode
   ///     Default repl mode behavior, as configured by the binary.
   /// \param[in] pre_init_commands
   ///     LLDB commands to execute as soon as the debugger instance is allocaed.
   /// \param[in] transport
   ///     Transport for this debug session.
-  DAP(llvm::StringRef path, std::ofstream *log,
+  DAP(llvm::StringRef path, Log *log,
       const ReplMode default_repl_mode,
       std::vector<std::string> pre_init_commands, Transport &transport);
 
diff --git a/lldb/tools/lldb-dap/DAPLog.h b/lldb/tools/lldb-dap/DAPLog.h
index 2574506739fea..fbb88a038ca2f 100644
--- a/lldb/tools/lldb-dap/DAPLog.h
+++ b/lldb/tools/lldb-dap/DAPLog.h
@@ -9,17 +9,19 @@
 #ifndef LLDB_TOOLS_LLDB_DAP_DAPLOG_H
 #define LLDB_TOOLS_LLDB_DAP_DAPLOG_H
 
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/FormatAdapters.h"
 #include "llvm/Support/FormatVariadic.h"
 #include <chrono>
 #include <fstream>
+#include <mutex>
 #include <string>
 
 // Write a message to log, if logging is enabled.
 #define DAP_LOG(log, ...)                                                      \
   do {                                                                         \
-    ::std::ofstream *log_private = (log);                                      \
+    ::lldb_dap::Log *log_private = (log);                                      \
     if (log_private) {                                                         \
       ::std::chrono::duration<double> now{                                     \
           ::std::chrono::system_clock::now().time_since_epoch()};              \
@@ -27,8 +29,7 @@
       ::llvm::raw_string_ostream os(out);                                      \
       os << ::llvm::formatv("{0:f9} ", now.count()).str()                      \
          << ::llvm::formatv(__VA_ARGS__).str() << "\n";                        \
-      *log_private << out;                                                     \
-      log_private->flush();                                                    \
+      log_private->WriteMessage(out);                                          \
     }                                                                          \
   } while (0)
 
@@ -36,7 +37,7 @@
 // with {0}. Error is cleared regardless of whether logging is enabled.
 #define DAP_LOG_ERROR(log, error, ...)                                         \
   do {                                                                         \
-    ::std::ofstream *log_private = (log);                                      \
+    ::lldb_dap::Log *log_private = (log);                                      \
     ::llvm::Error error_private = (error);                                     \
     if (log_private && error_private) {                                        \
       ::std::chrono::duration<double> now{                                     \
@@ -46,14 +47,28 @@
       os << ::llvm::formatv("{0:f9} ", now.count()).str()                      \
          << ::lldb_dap::FormatError(::std::move(error_private), __VA_ARGS__)   \
          << "\n";                                                              \
-      *log_private << out;                                                     \
-      log_private->flush();                                                    \
+      log_private->WriteMessage(out);                                          \
     } else                                                                     \
       ::llvm::consumeError(::std::move(error_private));                        \
   } while (0)
 
 namespace lldb_dap {
 
+class Log {
+public:
+  Log(std::ofstream stream) : m_stream(std::move(stream)) {}
+
+  void WriteMessage(llvm::StringRef message) {
+    std::scoped_lock<std::mutex> lock(m_mutex);
+    m_stream << message.str();
+    m_stream.flush();
+  }
+
+private:
+  std::mutex m_mutex;
+  std::ofstream m_stream;
+};
+
 template <typename... Args>
 inline auto FormatError(llvm::Error error, const char *format, Args &&...args) {
   return llvm::formatv(format, llvm::toString(std::move(error)),
diff --git a/lldb/tools/lldb-dap/EventHelper.cpp b/lldb/tools/lldb-dap/EventHelper.cpp
index 705eb0a457d9c..758e3b7618e55 100644
--- a/lldb/tools/lldb-dap/EventHelper.cpp
+++ b/lldb/tools/lldb-dap/EventHelper.cpp
@@ -178,15 +178,13 @@ void SendThreadStoppedEvent(DAP &dap) {
           SendThreadExitedEvent(dap, tid);
       }
     } else {
-      if (dap.log)
-        *dap.log << "error: SendThreadStoppedEvent() when process"
-                    " isn't stopped ("
-                 << lldb::SBDebugger::StateAsCString(state) << ')' << std::endl;
+      DAP_LOG(
+          dap.log,
+          "error: SendThreadStoppedEvent() when process isn't stopped ({0})",
+          lldb::SBDebugger::StateAsCString(state));
     }
   } else {
-    if (dap.log)
-      *dap.log << "error: SendThreadStoppedEvent() invalid process"
-               << std::endl;
+    DAP_LOG(dap.log, "error: SendThreadStoppedEvent() invalid process");
   }
   dap.RunStopCommands();
 }
diff --git a/lldb/tools/lldb-dap/Transport.cpp b/lldb/tools/lldb-dap/Transport.cpp
index 4500e7cf909ba..010f04f39f9ed 100644
--- a/lldb/tools/lldb-dap/Transport.cpp
+++ b/lldb/tools/lldb-dap/Transport.cpp
@@ -63,7 +63,7 @@ static constexpr StringLiteral kHeaderSeparator = "\r\n\r\n";
 
 namespace lldb_dap {
 
-Transport::Transport(StringRef client_name, std::ofstream *log,
+Transport::Transport(StringRef client_name, Log *log,
                      IOObjectSP input, IOObjectSP output)
     : m_client_name(client_name), m_log(log), m_input(std::move(input)),
       m_output(std::move(output)) {}
diff --git a/lldb/tools/lldb-dap/Transport.h b/lldb/tools/lldb-dap/Transport.h
index e77bb5dd05e65..6cb580ae86c24 100644
--- a/lldb/tools/lldb-dap/Transport.h
+++ b/lldb/tools/lldb-dap/Transport.h
@@ -14,11 +14,11 @@
 #ifndef LLDB_TOOLS_LLDB_DAP_TRANSPORT_H
 #define LLDB_TOOLS_LLDB_DAP_TRANSPORT_H
 
+#include "DAPLog.h"
 #include "Protocol/ProtocolBase.h"
 #include "lldb/lldb-forward.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
-#include <fstream>
 #include <optional>
 
 namespace lldb_dap {
@@ -27,8 +27,8 @@ namespace lldb_dap {
 /// with the client.
 class Transport {
 public:
-  Transport(llvm::StringRef client_name, std::ofstream *log,
-            lldb::IOObjectSP input, lldb::IOObjectSP output);
+  Transport(llvm::StringRef client_name, Log *log, lldb::IOObjectSP input,
+            lldb::IOObjectSP output);
   ~Transport() = default;
 
   /// Transport is not copyable.
@@ -51,7 +51,7 @@ class Transport {
 
 private:
   llvm::StringRef m_client_name;
-  std::ofstream *m_log;
+  Log *m_log;
   lldb::IOObjectSP m_input;
   lldb::IOObjectSP m_output;
 };
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index 59e31cf8e2cc8..660f8379cbf4a 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -281,7 +281,7 @@ validateConnection(llvm::StringRef conn) {
 
 static llvm::Error
 serveConnection(const Socket::SocketProtocol &protocol, const std::string &name,
-                std::ofstream *log, llvm::StringRef program_path,
+                Log *log, llvm::StringRef program_path,
                 const ReplMode default_repl_mode,
                 const std::vector<std::string> &pre_init_commands) {
   Status status;
@@ -484,10 +484,10 @@ int main(int argc, char *argv[]) {
   }
 #endif
 
-  std::unique_ptr<std::ofstream> log = nullptr;
+  std::unique_ptr<Log> log = nullptr;
   const char *log_file_path = getenv("LLDBDAP_LOG");
   if (log_file_path)
-    log = std::make_unique<std::ofstream>(log_file_path);
+    log = std::make_unique<Log>(std::ofstream(log_file_path));
 
   // Initialize LLDB first before we do anything.
   lldb::SBError error = lldb::SBDebugger::InitializeWithErrorHandling();

>From 8d66eb274184c575c77b84750c9bec96af4531f5 Mon Sep 17 00:00:00 2001
From: John Harrison <harjohn at google.com>
Date: Tue, 18 Mar 2025 16:55:16 -0700
Subject: [PATCH 3/6] Adding some documentation to lldb_dap::Log and moving the
 impl out of the header.

---
 lldb/tools/lldb-dap/CMakeLists.txt |  1 +
 lldb/tools/lldb-dap/DAP.cpp        |  3 +--
 lldb/tools/lldb-dap/DAP.h          |  3 +--
 lldb/tools/lldb-dap/DAPLog.cpp     | 26 ++++++++++++++++++++++++++
 lldb/tools/lldb-dap/DAPLog.h       | 19 ++++++++++++-------
 lldb/tools/lldb-dap/Transport.cpp  |  4 ++--
 lldb/tools/lldb-dap/lldb-dap.cpp   |  2 +-
 7 files changed, 44 insertions(+), 14 deletions(-)
 create mode 100644 lldb/tools/lldb-dap/DAPLog.cpp

diff --git a/lldb/tools/lldb-dap/CMakeLists.txt b/lldb/tools/lldb-dap/CMakeLists.txt
index adad75a79fa7a..545212d8cfb74 100644
--- a/lldb/tools/lldb-dap/CMakeLists.txt
+++ b/lldb/tools/lldb-dap/CMakeLists.txt
@@ -23,6 +23,7 @@ add_lldb_tool(lldb-dap
   Breakpoint.cpp
   BreakpointBase.cpp
   DAP.cpp
+  DAPLog.cpp
   EventHelper.cpp
   ExceptionBreakpoint.cpp
   FifoFiles.cpp
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index 202c80e19baac..0179faad599ef 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -68,8 +68,7 @@ const char DEV_NULL[] = "/dev/null";
 
 namespace lldb_dap {
 
-DAP::DAP(llvm::StringRef path, Log *log,
-         const ReplMode default_repl_mode,
+DAP::DAP(llvm::StringRef path, Log *log, const ReplMode default_repl_mode,
          std::vector<std::string> pre_init_commands, Transport &transport)
     : debug_adapter_path(path), log(log), transport(transport),
       broadcaster("lldb-dap"), exception_breakpoints(),
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index dae7ea20b9f5c..b20ee046a9ff6 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -218,8 +218,7 @@ struct DAP {
   ///     LLDB commands to execute as soon as the debugger instance is allocaed.
   /// \param[in] transport
   ///     Transport for this debug session.
-  DAP(llvm::StringRef path, Log *log,
-      const ReplMode default_repl_mode,
+  DAP(llvm::StringRef path, Log *log, const ReplMode default_repl_mode,
       std::vector<std::string> pre_init_commands, Transport &transport);
 
   ~DAP();
diff --git a/lldb/tools/lldb-dap/DAPLog.cpp b/lldb/tools/lldb-dap/DAPLog.cpp
new file mode 100644
index 0000000000000..ebe14494e5675
--- /dev/null
+++ b/lldb/tools/lldb-dap/DAPLog.cpp
@@ -0,0 +1,26 @@
+//===-- DAPLog.cpp --------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "DAPLog.h"
+#include "llvm/ADT/StringRef.h"
+#include <fstream>
+#include <mutex>
+
+using namespace llvm;
+
+namespace lldb_dap {
+
+Log::Log(StringRef filename) : m_stream(std::ofstream(filename.str())) {}
+
+void Log::WriteMessage(StringRef message) {
+  std::scoped_lock<std::mutex> lock(m_mutex);
+  m_stream << message.str();
+  m_stream.flush();
+}
+
+} // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/DAPLog.h b/lldb/tools/lldb-dap/DAPLog.h
index fbb88a038ca2f..c4e6328cd56ff 100644
--- a/lldb/tools/lldb-dap/DAPLog.h
+++ b/lldb/tools/lldb-dap/DAPLog.h
@@ -54,15 +54,20 @@
 
 namespace lldb_dap {
 
-class Log {
+/// Log manages the lldb-dap log file, used with the corresponding `DAP_LOG` and
+/// `DAP_LOG_ERROR` helpers.
+class Log final {
 public:
-  Log(std::ofstream stream) : m_stream(std::move(stream)) {}
+  /// Creates a log file with the given filename.
+  Log(llvm::StringRef filename);
 
-  void WriteMessage(llvm::StringRef message) {
-    std::scoped_lock<std::mutex> lock(m_mutex);
-    m_stream << message.str();
-    m_stream.flush();
-  }
+  /// Log is not copyable.
+  /// @{
+  Log(const Log &) = delete;
+  void operator=(const Log &) = delete;
+  /// @}
+
+  void WriteMessage(llvm::StringRef message);
 
 private:
   std::mutex m_mutex;
diff --git a/lldb/tools/lldb-dap/Transport.cpp b/lldb/tools/lldb-dap/Transport.cpp
index 010f04f39f9ed..50f5a4399782a 100644
--- a/lldb/tools/lldb-dap/Transport.cpp
+++ b/lldb/tools/lldb-dap/Transport.cpp
@@ -63,8 +63,8 @@ static constexpr StringLiteral kHeaderSeparator = "\r\n\r\n";
 
 namespace lldb_dap {
 
-Transport::Transport(StringRef client_name, Log *log,
-                     IOObjectSP input, IOObjectSP output)
+Transport::Transport(StringRef client_name, Log *log, IOObjectSP input,
+                     IOObjectSP output)
     : m_client_name(client_name), m_log(log), m_input(std::move(input)),
       m_output(std::move(output)) {}
 
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index 660f8379cbf4a..bd71e1bb533d8 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -487,7 +487,7 @@ int main(int argc, char *argv[]) {
   std::unique_ptr<Log> log = nullptr;
   const char *log_file_path = getenv("LLDBDAP_LOG");
   if (log_file_path)
-    log = std::make_unique<Log>(std::ofstream(log_file_path));
+    log = std::make_unique<Log>(log_file_path);
 
   // Initialize LLDB first before we do anything.
   lldb::SBError error = lldb::SBDebugger::InitializeWithErrorHandling();

>From 2b370221216725a457638df1050be46e72cec0b5 Mon Sep 17 00:00:00 2001
From: John Harrison <harjohn at google.com>
Date: Tue, 18 Mar 2025 17:22:45 -0700
Subject: [PATCH 4/6] With the helper type we can move the timestamp logic into
 the Log::WriteMessage call instead of having it in the macro.

---
 lldb/tools/lldb-dap/DAPLog.cpp   |  9 +++++++--
 lldb/tools/lldb-dap/DAPLog.h     | 32 +++++++-------------------------
 lldb/tools/lldb-dap/lldb-dap.cpp | 12 ++++++++++--
 3 files changed, 24 insertions(+), 29 deletions(-)

diff --git a/lldb/tools/lldb-dap/DAPLog.cpp b/lldb/tools/lldb-dap/DAPLog.cpp
index ebe14494e5675..77a92fc12f85e 100644
--- a/lldb/tools/lldb-dap/DAPLog.cpp
+++ b/lldb/tools/lldb-dap/DAPLog.cpp
@@ -8,18 +8,23 @@
 
 #include "DAPLog.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/raw_ostream.h"
+#include <chrono>
 #include <fstream>
 #include <mutex>
+#include <system_error>
 
 using namespace llvm;
 
 namespace lldb_dap {
 
-Log::Log(StringRef filename) : m_stream(std::ofstream(filename.str())) {}
+Log::Log(StringRef filename, std::error_code &EC) : m_stream(filename, EC) {}
 
 void Log::WriteMessage(StringRef message) {
   std::scoped_lock<std::mutex> lock(m_mutex);
-  m_stream << message.str();
+  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.flush();
 }
 
diff --git a/lldb/tools/lldb-dap/DAPLog.h b/lldb/tools/lldb-dap/DAPLog.h
index c4e6328cd56ff..484001a9b1628 100644
--- a/lldb/tools/lldb-dap/DAPLog.h
+++ b/lldb/tools/lldb-dap/DAPLog.h
@@ -13,23 +13,17 @@
 #include "llvm/Support/Error.h"
 #include "llvm/Support/FormatAdapters.h"
 #include "llvm/Support/FormatVariadic.h"
-#include <chrono>
-#include <fstream>
+#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) {                                                         \
-      ::std::chrono::duration<double> now{                                     \
-          ::std::chrono::system_clock::now().time_since_epoch()};              \
-      ::std::string out;                                                       \
-      ::llvm::raw_string_ostream os(out);                                      \
-      os << ::llvm::formatv("{0:f9} ", now.count()).str()                      \
-         << ::llvm::formatv(__VA_ARGS__).str() << "\n";                        \
-      log_private->WriteMessage(out);                                          \
+      log_private->WriteMessage(::llvm::formatv(__VA_ARGS__).str());           \
     }                                                                          \
   } while (0)
 
@@ -40,14 +34,8 @@
     ::lldb_dap::Log *log_private = (log);                                      \
     ::llvm::Error error_private = (error);                                     \
     if (log_private && error_private) {                                        \
-      ::std::chrono::duration<double> now{                                     \
-          std::chrono::system_clock::now().time_since_epoch()};                \
-      ::std::string out;                                                       \
-      ::llvm::raw_string_ostream os(out);                                      \
-      os << ::llvm::formatv("{0:f9} ", now.count()).str()                      \
-         << ::lldb_dap::FormatError(::std::move(error_private), __VA_ARGS__)   \
-         << "\n";                                                              \
-      log_private->WriteMessage(out);                                          \
+      log_private->WriteMessage(                                               \
+          ::lldb_dap::FormatError(::std::move(error_private), __VA_ARGS__));   \
     } else                                                                     \
       ::llvm::consumeError(::std::move(error_private));                        \
   } while (0)
@@ -59,19 +47,13 @@ namespace lldb_dap {
 class Log final {
 public:
   /// Creates a log file with the given filename.
-  Log(llvm::StringRef filename);
-
-  /// Log is not copyable.
-  /// @{
-  Log(const Log &) = delete;
-  void operator=(const Log &) = delete;
-  /// @}
+  Log(llvm::StringRef filename, std::error_code &EC);
 
   void WriteMessage(llvm::StringRef message);
 
 private:
   std::mutex m_mutex;
-  std::ofstream m_stream;
+  llvm::raw_fd_ostream m_stream;
 };
 
 template <typename... Args>
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index bd71e1bb533d8..062c3a5f989f3 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -48,6 +48,7 @@
 #include <memory>
 #include <mutex>
 #include <string>
+#include <system_error>
 #include <thread>
 #include <utility>
 #include <vector>
@@ -486,8 +487,15 @@ int main(int argc, char *argv[]) {
 
   std::unique_ptr<Log> log = nullptr;
   const char *log_file_path = getenv("LLDBDAP_LOG");
-  if (log_file_path)
-    log = std::make_unique<Log>(log_file_path);
+  if (log_file_path) {
+    std::error_code EC;
+    log = std::make_unique<Log>(log_file_path, EC);
+    if (EC) {
+      llvm::logAllUnhandledErrors(llvm::errorCodeToError(EC), llvm::errs(),
+                                  "Failed to create log file: ");
+      return EXIT_FAILURE;
+    }
+  }
 
   // Initialize LLDB first before we do anything.
   lldb::SBError error = lldb::SBDebugger::InitializeWithErrorHandling();

>From 9fec66ade3aff6524601408727d001d76df5be4a Mon Sep 17 00:00:00 2001
From: John Harrison <harjohn at google.com>
Date: Tue, 18 Mar 2025 18:00:12 -0700
Subject: [PATCH 5/6] Adding lldb_dap::Log to the DAPForward.h.

---
 lldb/tools/lldb-dap/DAPForward.h    | 1 +
 lldb/tools/lldb-dap/EventHelper.cpp | 1 +
 lldb/tools/lldb-dap/Transport.h     | 2 +-
 3 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/lldb/tools/lldb-dap/DAPForward.h b/lldb/tools/lldb-dap/DAPForward.h
index 667aef23abd0f..58e034ed1cc77 100644
--- a/lldb/tools/lldb-dap/DAPForward.h
+++ b/lldb/tools/lldb-dap/DAPForward.h
@@ -19,6 +19,7 @@ struct SourceBreakpoint;
 struct Watchpoint;
 struct InstructionBreakpoint;
 struct DAP;
+class Log;
 class BaseRequestHandler;
 class ResponseHandler;
 } // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/EventHelper.cpp b/lldb/tools/lldb-dap/EventHelper.cpp
index 758e3b7618e55..2c659f39f4b66 100644
--- a/lldb/tools/lldb-dap/EventHelper.cpp
+++ b/lldb/tools/lldb-dap/EventHelper.cpp
@@ -8,6 +8,7 @@
 
 #include "EventHelper.h"
 #include "DAP.h"
+#include "DAPLog.h"
 #include "JSONUtils.h"
 #include "LLDBUtils.h"
 #include "lldb/API/SBFileSpec.h"
diff --git a/lldb/tools/lldb-dap/Transport.h b/lldb/tools/lldb-dap/Transport.h
index 6cb580ae86c24..78b1a3fb23832 100644
--- a/lldb/tools/lldb-dap/Transport.h
+++ b/lldb/tools/lldb-dap/Transport.h
@@ -14,7 +14,7 @@
 #ifndef LLDB_TOOLS_LLDB_DAP_TRANSPORT_H
 #define LLDB_TOOLS_LLDB_DAP_TRANSPORT_H
 
-#include "DAPLog.h"
+#include "DAPForward.h"
 #include "Protocol/ProtocolBase.h"
 #include "lldb/lldb-forward.h"
 #include "llvm/ADT/StringRef.h"

>From 279cefbb742390fd1f919af90851a2a646cafdbb Mon Sep 17 00:00:00 2001
From: John Harrison <harjohn at google.com>
Date: Wed, 19 Mar 2025 08:51:44 -0700
Subject: [PATCH 6/6] Using lock_guard instead of scoped_lock

---
 lldb/tools/lldb-dap/DAPLog.cpp | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lldb/tools/lldb-dap/DAPLog.cpp b/lldb/tools/lldb-dap/DAPLog.cpp
index 77a92fc12f85e..f66784e197518 100644
--- a/lldb/tools/lldb-dap/DAPLog.cpp
+++ b/lldb/tools/lldb-dap/DAPLog.cpp
@@ -10,7 +10,6 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/raw_ostream.h"
 #include <chrono>
-#include <fstream>
 #include <mutex>
 #include <system_error>
 
@@ -21,7 +20,7 @@ namespace lldb_dap {
 Log::Log(StringRef filename, std::error_code &EC) : m_stream(filename, EC) {}
 
 void Log::WriteMessage(StringRef message) {
-  std::scoped_lock<std::mutex> lock(m_mutex);
+  std::lock_guard<std::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";



More information about the lldb-commits mailing list