[Lldb-commits] [PATCH] D135631: [lldb] Copy log files into diagnostic directory (RFC)

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: labath, DavidSpickett, clayborg, mib.
Herald added a project: All.
JDevlieghere requested review of this revision.

This patch copies over log files to the diagnostic directory. It's a bit of a straw man proposal as it has some shortcomings that might be acceptable, depending on whether we value completeness or performance more.

I see two ways to achieve having log files end up in the diagnostics:

1. The approach implemented here which piggybacks of the mapping kept by the debugger. The advantage is that it's free until you generate the diagnostics (at which point you pay the price of copying over the file). The obvious disadvantage is that this only works for log channels which are written to a file.
2. The alternative approach would be to have a log handler similar to our StreamTee that basically emits every log message to two log handlers. The first log handler would be the one the user chooses (this can be a file, the debugger output, etc) and the second log handler is a ring buffer managed by the diagnostics (similar to the always-on log). This ensures all the log messages are captured, but increases the performance hit of a log message and increases the debugger's memory usage.

This didn't seem like it was worth its own RFC on the mailing list so I rolled a dice and implemented one of the approaches so we can discuss it here.


https://reviews.llvm.org/D135631

Files:
  lldb/include/lldb/Utility/Diagnostics.h
  lldb/source/Core/Debugger.cpp
  lldb/source/Utility/Diagnostics.cpp
  lldb/test/Shell/Diagnostics/TestCopyLogs.test


Index: lldb/test/Shell/Diagnostics/TestCopyLogs.test
===================================================================
--- /dev/null
+++ lldb/test/Shell/Diagnostics/TestCopyLogs.test
@@ -0,0 +1,7 @@
+# RUN: rm -rf %t
+# RUN: mkdir -p %t
+# The "ll''db" below is not a typo but a way to prevent lit from substituting 'lldb'.
+# RUN: %lldb -o 'log enable ll''db commands -f %t/commands.log' -o 'diagnostics dump -d %t/diags'
+# RUN: cat %t/diags/commands.log | FileCheck %s
+
+# CHECK: Processing command: diagnostics dump
Index: lldb/source/Utility/Diagnostics.cpp
===================================================================
--- lldb/source/Utility/Diagnostics.cpp
+++ lldb/source/Utility/Diagnostics.cpp
@@ -30,6 +30,8 @@
   InstanceImpl().reset();
 }
 
+bool Diagnostics::Enabled() { return InstanceImpl().operator bool(); }
+
 Optional<Diagnostics> &Diagnostics::InstanceImpl() {
   static Optional<Diagnostics> g_diagnostics;
   return g_diagnostics;
Index: lldb/source/Core/Debugger.cpp
===================================================================
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -810,6 +810,22 @@
   if (!GetOutputFile().GetIsTerminalWithColors())
     SetUseColor(false);
 
+  if (Diagnostics::Enabled()) {
+    Diagnostics::Instance().AddCallback(
+        [&](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(entry.first(), 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.
Index: lldb/include/lldb/Utility/Diagnostics.h
===================================================================
--- lldb/include/lldb/Utility/Diagnostics.h
+++ lldb/include/lldb/Utility/Diagnostics.h
@@ -43,6 +43,7 @@
   static Diagnostics &Instance();
   static void Initialize();
   static void Terminate();
+  static bool Enabled();
 
   /// Create a unique diagnostic directory.
   static llvm::Expected<FileSpec> CreateUniqueDirectory();


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D135631.466667.patch
Type: text/x-patch
Size: 2493 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20221011/a90f693c/attachment.bin>