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

Around this time last year, I said on the mailing list [1] that I wanted to to 
transform the reproducers into something that resembles a sysdiagnose on Apple 
platforms: a collection of files containing a variety of information to help 
diagnose bugs or troubleshoot issues. This patch adds that framework. Based on 
lessons learned from the reproducers, I've intentionally tried to keep it small 
and simple. Different parts of LLDB can register callbacks (this is necessary 
for layering purposes) that will get called when the diagnostics should be 
generated.

For this patch, it's hooked up to Greg's statistics, but I have a bunch more 
things in the pipeline. With the minimal implementation in this patch, when the 
command line LLDB crashes, it dump the JSON statistics into a temp directory 
and ask the user to attach them to their bug report.

  LLDB diagnostics written to 
/var/folders/6v/6zkjmd45211_1vxrtbnl0dj00000gn/T/diagnostics-ea34d4
  Please include the directory content when filing a bug report

[1] https://lists.llvm.org/pipermail/lldb-dev/2021-September/017045.html


https://reviews.llvm.org/D134991

Files:
  lldb/include/lldb/API/SBDebugger.h
  lldb/include/lldb/Target/Statistics.h
  lldb/include/lldb/Utility/Diagnostics.h
  lldb/source/API/SBDebugger.cpp
  lldb/source/Core/Debugger.cpp
  lldb/source/Initialization/SystemInitializerCommon.cpp
  lldb/source/Target/Statistics.cpp
  lldb/source/Utility/Diagnostics.cpp
  lldb/tools/driver/Driver.cpp

Index: lldb/tools/driver/Driver.cpp
===================================================================
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -788,6 +788,10 @@
                        << '\n';
     return 1;
   }
+
+  // Setup LLDB signal handlers once the debugger has been initialized.
+  SBDebugger::PrintDiagnosticsOnError();
+
   SBHostOS::ThreadCreated("<lldb.driver.main-thread>");
 
   signal(SIGINT, sigint_handler);
Index: lldb/source/Utility/Diagnostics.cpp
===================================================================
--- lldb/source/Utility/Diagnostics.cpp
+++ lldb/source/Utility/Diagnostics.cpp
@@ -10,13 +10,13 @@
 #include "lldb/Utility/LLDBAssert.h"
 
 #include "llvm/Support/Error.h"
+#include "llvm/Support/FileSystem.h"
 #include "llvm/Support/raw_ostream.h"
 
 using namespace lldb_private;
+using namespace lldb;
 using namespace llvm;
 
-static const constexpr size_t g_log_messages = 100;
-
 void Diagnostics::Initialize() {
   lldbassert(!InstanceImpl() && "Already initialized.");
   InstanceImpl().emplace();
@@ -34,19 +34,48 @@
 
 Diagnostics &Diagnostics::Instance() { return *InstanceImpl(); }
 
-Diagnostics::Diagnostics()
-    : m_always_on_log_handler(
-          std::make_shared<RotatingLogHandler>(g_log_messages)) {
-  uint32_t log_options = LLDB_LOG_OPTION_PREPEND_THREAD_NAME;
-  Log::EnableLogChannel(m_always_on_log_handler, log_options, "lldb",
-                        {"always-on"}, llvm::outs());
+Diagnostics::Diagnostics() {}
+
+Diagnostics::~Diagnostics() {}
+
+void Diagnostics::AddCallback(Callback callback) {
+  std::lock_guard<std::mutex> guard(m_callbacks_mutex);
+  m_callbacks.push_back(callback);
 }
 
-Diagnostics::~Diagnostics() {
-  Log::DisableLogChannel("lldb", {"always-on"}, llvm::nulls());
+void Diagnostics::RemoveCallback(Callback callback) {
+  std::lock_guard<std::mutex> guard(m_callbacks_mutex);
+  m_callbacks.erase(
+      std::remove(m_callbacks.begin(), m_callbacks.end(), callback),
+      m_callbacks.end());
 }
 
-bool Diagnostics::Dump(llvm::raw_ostream &stream) {
-  m_always_on_log_handler->Dump(stream);
+bool Diagnostics::Dump(raw_ostream &stream) {
+  SmallString<128> diagnostics_dir;
+  std::error_code ec =
+      sys::fs::createUniqueDirectory("diagnostics", diagnostics_dir);
+  if (ec) {
+    stream << "unable to create diagnostic dir: "
+           << toString(errorCodeToError(ec)) << '\n';
+    return false;
+  }
+
+  Error error = Create(FileSpec(diagnostics_dir.str()));
+  if (error) {
+    stream << toString(std::move(error)) << '\n';
+    return false;
+  }
+
+  stream << "LLDB diagnostics written to " << diagnostics_dir << "\n";
+  stream << "Please include the directory content when filing a bug report\n";
+
   return true;
 }
+
+Error Diagnostics::Create(const FileSpec &dir) {
+  for (Callback c : m_callbacks) {
+    if (Error err = c(dir))
+      return err;
+  }
+  return Error::success();
+}
Index: lldb/source/Target/Statistics.cpp
===================================================================
--- lldb/source/Target/Statistics.cpp
+++ lldb/source/Target/Statistics.cpp
@@ -14,6 +14,7 @@
 #include "lldb/Target/Process.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Target/UnixSignals.h"
+#include "lldb/Utility/Diagnostics.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -275,3 +276,29 @@
   };
   return std::move(global_stats);
 }
+
+void Stats::Initialize() { Diagnostics::Instance().AddCallback(Stats::Dump); }
+
+void Stats::Terminate() { Diagnostics::Instance().RemoveCallback(Stats::Dump); }
+
+llvm::Error Stats::Dump(const FileSpec &dir) {
+  for (size_t debugger_idx = 0; debugger_idx < Debugger::GetNumDebuggers();
+       debugger_idx++) {
+    DebuggerSP debugger_sp(Debugger::GetDebuggerAtIndex(debugger_idx));
+    if (!debugger_sp)
+      continue;
+
+    std::string filename =
+        (Twine("debugger-") + Twine(debugger_idx) + Twine("-stats.json")).str();
+    FileSpec stat_file = dir.CopyByAppendingPathComponent(filename);
+
+    std::error_code ec;
+    raw_fd_ostream stats_stream(stat_file.GetPath(), ec, sys::fs::OF_None);
+    if (ec)
+      return errorCodeToError(ec);
+
+    stats_stream << DebuggerStats::ReportStatistics(*debugger_sp, nullptr);
+  }
+
+  return llvm::Error::success();
+}
Index: lldb/source/Initialization/SystemInitializerCommon.cpp
===================================================================
--- lldb/source/Initialization/SystemInitializerCommon.cpp
+++ lldb/source/Initialization/SystemInitializerCommon.cpp
@@ -12,6 +12,7 @@
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/Host.h"
 #include "lldb/Host/Socket.h"
+#include "lldb/Target/Statistics.h"
 #include "lldb/Utility/Diagnostics.h"
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/Timer.h"
@@ -65,6 +66,7 @@
   InitializeLldbChannel();
 
   Diagnostics::Initialize();
+  Stats::Initialize();
   FileSystem::Initialize();
   HostInfo::Initialize(m_shlib_dir_helper);
 
@@ -97,5 +99,6 @@
   HostInfo::Terminate();
   Log::DisableAllLogChannels();
   FileSystem::Terminate();
+  Stats::Terminate();
   Diagnostics::Terminate();
 }
Index: lldb/source/Core/Debugger.cpp
===================================================================
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -44,6 +44,7 @@
 #include "lldb/Target/Thread.h"
 #include "lldb/Target/ThreadList.h"
 #include "lldb/Utility/AnsiTerminal.h"
+#include "lldb/Utility/Diagnostics.h"
 #include "lldb/Utility/Event.h"
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/Listener.h"
Index: lldb/source/API/SBDebugger.cpp
===================================================================
--- lldb/source/API/SBDebugger.cpp
+++ lldb/source/API/SBDebugger.cpp
@@ -223,7 +223,7 @@
   Diagnostics::Instance().Dump(llvm::errs());
 }
 
-void SBDebugger::DumpDiagnosticsOnError() {
+void SBDebugger::PrintDiagnosticsOnError() {
   LLDB_INSTRUMENT();
 
   llvm::sys::AddSignalHandler(&DumpDiagnostics, nullptr);
Index: lldb/include/lldb/Utility/Diagnostics.h
===================================================================
--- lldb/include/lldb/Utility/Diagnostics.h
+++ lldb/include/lldb/Utility/Diagnostics.h
@@ -6,21 +6,40 @@
 //
 //===----------------------------------------------------------------------===//
 
-#ifndef LLDB_UTILITY_REPRODUCER_H
-#define LLDB_UTILITY_REPRODUCER_H
+#ifndef LLDB_UTILITY_DIAGNOSTICS_H
+#define LLDB_UTILITY_DIAGNOSTICS_H
 
+#include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/Log.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringSet.h"
+#include "llvm/Support/Error.h"
+
+#include <functional>
+#include <mutex>
+#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.
 class Diagnostics {
 public:
   Diagnostics();
   ~Diagnostics();
 
+  /// Gather diagnostics in the given directory.
+  llvm::Error Create(const FileSpec &dir);
+
+  /// Gather diagnostics and print a message to the given output stream.
   bool Dump(llvm::raw_ostream &stream);
 
+  using Callback = llvm::Error (*)(const FileSpec &);
+
+  void AddCallback(Callback callback);
+  void RemoveCallback(Callback callback);
+
   static Diagnostics &Instance();
   static void Initialize();
   static void Terminate();
@@ -28,7 +47,8 @@
 private:
   static llvm::Optional<Diagnostics> &InstanceImpl();
 
-  std::shared_ptr<RotatingLogHandler> m_always_on_log_handler;
+  std::vector<Callback> m_callbacks;
+  std::mutex m_callbacks_mutex;
 };
 
 } // namespace lldb_private
Index: lldb/include/lldb/Target/Statistics.h
===================================================================
--- lldb/include/lldb/Target/Statistics.h
+++ lldb/include/lldb/Target/Statistics.h
@@ -12,6 +12,7 @@
 #include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/Stream.h"
 #include "lldb/lldb-forward.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/JSON.h"
 #include <atomic>
 #include <chrono>
@@ -176,6 +177,13 @@
   static bool g_collecting_stats;
 };
 
+class Stats {
+public:
+  static void Initialize();
+  static void Terminate();
+  static llvm::Error Dump(const FileSpec &dir);
+};
+
 } // namespace lldb_private
 
 #endif // LLDB_TARGET_STATISTICS_H
Index: lldb/include/lldb/API/SBDebugger.h
===================================================================
--- lldb/include/lldb/API/SBDebugger.h
+++ lldb/include/lldb/API/SBDebugger.h
@@ -94,7 +94,7 @@
 
   static void PrintStackTraceOnError();
 
-  static void DumpDiagnosticsOnError();
+  static void PrintDiagnosticsOnError();
 
   static void Terminate();
 
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to