[lldb] Emit diagnostic events in the diagnostic dump

[JDevlieghere]

Summary

Emit diagnostic events (errors and warnings) in the diagnostic dump. This patch also adds an "information" diagnostic event, which doesn't get broadcast, but ends up in the diagnostics as well.

[labath]

(Unsurprisingly, I guess), this looks much better to me. And I'm totally open to automatically logging these diagnostics somewhere as well (to a dedicated log channel/category, or by passing the log as an argument, or something else).

lldb/source/Utility/Diagnostics.cpp

94

Maybe call this DumpDiangosticsLog ?

[clayborg]

lldb/include/lldb/Core/Debugger.h

431

This comment seems wrong? The code below asserts and returns in PrivateReportDiagnostic(). Do we want to add a eBroadcastBitInfo?

lldb/include/lldb/Core/Debugger.h

public:
   /// \param [in] once
   ///   If a pointer is passed to a std::once_flag, then it will be used to
   ///   ensure the given error is only broadcast once.
   static void
   ReportError(std::string message,
               llvm::Optional<lldb::user_id_t> debugger_id = llvm::None,
               std::once_flag *once = nullptr);
 
+  /// Report info events.
+  ///
+  /// Unlike warning and error events, info events are not broadcast but are
+  /// logged for diagnostic purposes.
+  ///
+  /// \param[in] message
+  ///   The info message to be reported.
+  ///
+  /// \param [in] debugger_id
+  ///   If this optional parameter has a value, it indicates this diagnostic is
+  ///   associated with a unique debugger instance.
+  ///
+  /// \param [in] once
+  ///   If a pointer is passed to a std::once_flag, then it will be used to
+  ///   ensure the given info is only logged once.
+  static void
+  ReportInfo(std::string message,
+             llvm::Optional<lldb::user_id_t> debugger_id = llvm::None,
+             std::once_flag *once = nullptr);
+
   static void ReportSymbolChange(const ModuleSpec &module_spec);
 
 protected:
   friend class CommandInterpreter;
   friend class REPL;
   friend class Progress;
 
   /// Report progress events.

lldb/include/lldb/Core/DebuggerEvents.h

private:
   const bool m_debugger_specific;
   ProgressEventData(const ProgressEventData &) = delete;
   const ProgressEventData &operator=(const ProgressEventData &) = delete;
 };
 
 class DiagnosticEventData : public EventData {
 public:
   enum class Type {
+    Info,
     Warning,
     Error,
   };
   DiagnosticEventData(Type type, std::string message, bool debugger_specific)
       : m_message(std::move(message)), m_type(type),
         m_debugger_specific(debugger_specific) {}
   ~DiagnosticEventData() override = default;
 

lldb/include/lldb/Utility/Diagnostics.h

public:
   llvm::Error Create(const FileSpec &dir);
 
   /// Gather diagnostics and print a message to the given output stream.
   /// @{
   bool Dump(llvm::raw_ostream &stream);
   bool Dump(llvm::raw_ostream &stream, const FileSpec &dir);
   /// @}
 
+  void Report(llvm::StringRef message);
+
   using Callback = std::function<llvm::Error(const FileSpec &)>;
 
   void AddCallback(Callback callback);
 
   static Diagnostics &Instance();
+
+  static bool Enabled();
   static void Initialize();
   static void Terminate();
 
   /// Create a unique diagnostic directory.
   static llvm::Expected<FileSpec> CreateUniqueDirectory();
 
 private:
   static llvm::Optional<Diagnostics> &InstanceImpl();
 
+  llvm::Error DumpDiangosticsLog(const FileSpec &dir) const;
+
+  RotatingLogHandler m_log_handler;
+
   llvm::SmallVector<Callback, 4> m_callbacks;
   std::mutex m_callbacks_mutex;
 };
 
 } // namespace lldb_private
 
 #endif

lldb/source/Core/Debugger.cpp

 }
 
 static void PrivateReportDiagnostic(Debugger &debugger,
                                     DiagnosticEventData::Type type,
                                     std::string message,
                                     bool debugger_specific) {
   uint32_t event_type = 0;
   switch (type) {
+  case DiagnosticEventData::Type::Info:
+    assert(false && "DiagnosticEventData::Type::Info should not be broadcast");
+    return;
   case DiagnosticEventData::Type::Warning:
     event_type = Debugger::eBroadcastBitWarning;
     break;
   case DiagnosticEventData::Type::Error:
     event_type = Debugger::eBroadcastBitError;
     break;
   }
 
static void PrivateReportDiagnostic(Debugger &debugger,
   broadcaster.BroadcastEvent(event_sp);
 }
 
 void Debugger::ReportDiagnosticImpl(DiagnosticEventData::Type type,
                                     std::string message,
                                     llvm::Optional<lldb::user_id_t> debugger_id,
                                     std::once_flag *once) {
   auto ReportDiagnosticLambda = [&]() {
+    // The diagnostic subsystem is optional but we still want to broadcast
+    // events when it's disabled.
+    if (Diagnostics::Enabled())
+      Diagnostics::Instance().Report(message);
+
+    // We don't broadcast info events.
+    if (type == DiagnosticEventData::Type::Info)
+      return;
+
     // Check if this diagnostic is for a specific debugger.
     if (debugger_id) {
       // It is debugger specific, grab it and deliver the event if the debugger
       // still exists.
       DebuggerSP debugger_sp = FindDebuggerWithID(*debugger_id);
       if (debugger_sp)
         PrivateReportDiagnostic(*debugger_sp, type, std::move(message), true);
       return;

 
 void Debugger::ReportError(std::string message,
                            llvm::Optional<lldb::user_id_t> debugger_id,
                            std::once_flag *once) {
   ReportDiagnosticImpl(DiagnosticEventData::Type::Error, std::move(message),
                        debugger_id, once);
 }
 
+void Debugger::ReportInfo(std::string message,
+                          llvm::Optional<lldb::user_id_t> debugger_id,
+                          std::once_flag *once) {
+  ReportDiagnosticImpl(DiagnosticEventData::Type::Info, std::move(message),
+                       debugger_id, once);
+}
+
 void Debugger::ReportSymbolChange(const ModuleSpec &module_spec) {
   if (g_debugger_list_ptr && g_debugger_list_mutex_ptr) {
     std::lock_guard<std::recursive_mutex> guard(*g_debugger_list_mutex_ptr);
     for (DebuggerSP debugger_sp : *g_debugger_list_ptr) {
       EventSP event_sp = std::make_shared<Event>(
           Debugger::eBroadcastSymbolChange,
           new SymbolChangeEventData(debugger_sp, module_spec));
       debugger_sp->GetBroadcaster().BroadcastEvent(event_sp);

lldb/source/Core/DebuggerEvents.cpp

 
 const ProgressEventData *
 ProgressEventData::GetEventDataFromEvent(const Event *event_ptr) {
   return GetEventDataFromEventImpl<ProgressEventData>(event_ptr);
 }
 
 llvm::StringRef DiagnosticEventData::GetPrefix() const {
   switch (m_type) {
+  case Type::Info:
+    return "info";
   case Type::Warning:
     return "warning";
   case Type::Error:
     return "error";
   }
   llvm_unreachable("Fully covered switch above!");
 }
 

lldb/source/Utility/Diagnostics.cpp

 #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 constexpr size_t g_num_log_messages = 100;
+
 void Diagnostics::Initialize() {
   lldbassert(!InstanceImpl() && "Already initialized.");
   InstanceImpl().emplace();
 }
 
 void Diagnostics::Terminate() {
   lldbassert(InstanceImpl() && "Already terminated.");
   InstanceImpl().reset();
 }
 
+bool Diagnostics::Enabled() { return InstanceImpl().operator bool(); }
+
 Optional<Diagnostics> &Diagnostics::InstanceImpl() {
   static Optional<Diagnostics> g_diagnostics;
   return g_diagnostics;
 }
 
 Diagnostics &Diagnostics::Instance() { return *InstanceImpl(); }
 
-Diagnostics::Diagnostics() {}
+Diagnostics::Diagnostics() : m_log_handler(g_num_log_messages) {}
 
 Diagnostics::~Diagnostics() {}
 
 void Diagnostics::AddCallback(Callback callback) {
   std::lock_guard<std::mutex> guard(m_callbacks_mutex);
   m_callbacks.push_back(callback);
 }
 
 bool Diagnostics::Dump(raw_ostream &stream) {
   Expected<FileSpec> diagnostics_dir = CreateUniqueDirectory();
   if (!diagnostics_dir) {
     stream << "unable to create diagnostic dir: "
            << toString(diagnostics_dir.takeError()) << '\n';
     return false;
   }
 
   return Dump(stream, *diagnostics_dir);
 }
 
 bool Diagnostics::Dump(raw_ostream &stream, const FileSpec &dir) {
   stream << "LLDB diagnostics will be written to " << dir.GetPath() << "\n";
   stream << "Please include the directory content when filing a bug report\n";
 
-  Error error = Create(dir);
-  if (error) {
+  if (Error error = Create(dir)) {
     stream << toString(std::move(error)) << '\n';
     return false;
   }
 
   return true;
 }
 
 llvm::Expected<FileSpec> Diagnostics::CreateUniqueDirectory() {
   SmallString<128> diagnostics_dir;
   std::error_code ec =
       sys::fs::createUniqueDirectory("diagnostics", diagnostics_dir);
   if (ec)
     return errorCodeToError(ec);
   return FileSpec(diagnostics_dir.str());
 }
 
 Error Diagnostics::Create(const FileSpec &dir) {
+  if (Error err = DumpDiangosticsLog(dir))
+    return err;
+
   for (Callback c : m_callbacks) {
     if (Error err = c(dir))
       return err;
   }
+
+  return Error::success();
+}
+
+llvm::Error Diagnostics::DumpDiangosticsLog(const FileSpec &dir) const {
+  FileSpec log_file = dir.CopyByAppendingPathComponent("diagnostics.log");
+  std::error_code ec;
+  llvm::raw_fd_ostream stream(log_file.GetPath(), ec, llvm::sys::fs::OF_None);
+  if (ec)
+    return errorCodeToError(ec);
+  m_log_handler.Dump(stream);
   return Error::success();
 }
+
+void Diagnostics::Report(llvm::StringRef message) {
+  m_log_handler.Emit(message);
+}

lldb/test/Shell/Diagnostics/TestDiagnosticsLog.test

+# RUN: yaml2obj %s -o %t
+# RUN: mkdir -p %t.diags
+# RUN: %lldb -c %t -o 'diagnostics dump -d %t.diags' 2> %t.stderr
+
+# RUN: cat %t.stderr | FileCheck %s --check-prefix ERROR
+# RUN: cat %t.diags/diagnostics.log | FileCheck %s --check-prefix ERROR
+
+# ERROR: unable to retrieve process ID from minidump file, setting process ID to 1
+
+--- !minidump
+Streams:
+  - Type:            ThreadList
+    Threads:
+      - Thread Id:       0x00003E81
+        Context:         0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000B0010000000000033000000000000000000000006020100000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000010A234EBFC7F000010A234EBFC7F00000000000000000000F09C34EBFC7F0000C0A91ABCE97F00000000000000000000A0163FBCE97F00004602000000000000921C40000000000030A434EBFC7F000000000000000000000000000000000000C61D4000000000007F0300000000000000000000000000000000000000000000801F0000FFFF0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000FFFF00FFFFFFFFFFFFFF00FFFFFFFF25252525252525252525252525252525000000000000000000000000000000000000000000000000000000000000000000FFFF00FFFFFFFFFFFFFF00FFFFFFFF0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000FF00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
+        Stack:
+          Start of Memory Range: 0x00007FFCEB34A000
+          Content:         ''
+  - Type:            ModuleList
+    Modules:
+      - Base of Image:   0x0000000000400000
+        Size of Image:   0x00017000
+        Module Name:     'a.out'
+        CodeView Record: ''
+  - Type:            SystemInfo
+    Processor Arch:  AMD64
+    Platform ID:     Linux
+    CSD Version:     'Linux 3.13'
+    CPU:
+      Vendor ID:       GenuineIntel
+      Version Info:    0x00000000
+      Feature Info:    0x00000000
+...