JDevlieghere updated this revision to Diff 415283.
JDevlieghere added a comment.

Address code review:

- Decentralize tracking whether a warning/error has been emitted.
- Support diagnostics that are not tied to a single debugger.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121511/new/

https://reviews.llvm.org/D121511

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/include/lldb/Core/DebuggerEvents.h
  lldb/source/Core/Debugger.cpp
  lldb/source/Core/DebuggerEvents.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
  lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h

Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
===================================================================
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
@@ -433,7 +433,8 @@
   std::unique_ptr<NonPointerISACache> m_non_pointer_isa_cache_up;
   std::unique_ptr<TaggedPointerVendor> m_tagged_pointer_vendor_up;
   EncodingToTypeSP m_encoding_to_type_sp;
-  bool m_noclasses_warning_emitted;
+  std::once_flag m_no_classes_cached_warning;
+  std::once_flag m_no_expanded_cache_warning;
   llvm::Optional<std::pair<lldb::addr_t, lldb::addr_t>> m_CFBoolean_values;
   uint64_t m_realized_class_generation_count;
 };
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===================================================================
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -21,6 +21,7 @@
 
 #include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include "lldb/Core/Debugger.h"
+#include "lldb/Core/DebuggerEvents.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/Section.h"
@@ -665,8 +666,8 @@
       m_loaded_objc_opt(false), m_non_pointer_isa_cache_up(),
       m_tagged_pointer_vendor_up(
           TaggedPointerVendorV2::CreateInstance(*this, objc_module_sp)),
-      m_encoding_to_type_sp(), m_noclasses_warning_emitted(false),
-      m_CFBoolean_values(), m_realized_class_generation_count(0) {
+      m_encoding_to_type_sp(), m_CFBoolean_values(),
+      m_realized_class_generation_count(0) {
   static const ConstString g_gdb_object_getClass("gdb_object_getClass");
   m_has_object_getClass = HasSymbol(g_gdb_object_getClass);
   static const ConstString g_objc_copyRealizedClassList(
@@ -2330,32 +2331,27 @@
 
 void AppleObjCRuntimeV2::WarnIfNoClassesCached(
     SharedCacheWarningReason reason) {
-  if (m_noclasses_warning_emitted)
-    return;
-
   if (GetProcess() && !DoesProcessHaveSharedCache(*GetProcess())) {
     // Simulators do not have the objc_opt_ro class table so don't actually
     // complain to the user
-    m_noclasses_warning_emitted = true;
     return;
   }
 
   Debugger &debugger(GetProcess()->GetTarget().GetDebugger());
-  if (auto stream = debugger.GetAsyncOutputStream()) {
-    switch (reason) {
-    case SharedCacheWarningReason::eNotEnoughClassesRead:
-      stream->PutCString("warning: could not find Objective-C class data in "
-                         "the process. This may reduce the quality of type "
-                         "information available.\n");
-      m_noclasses_warning_emitted = true;
-      break;
-    case SharedCacheWarningReason::eExpressionExecutionFailure:
-      stream->PutCString("warning: could not execute support code to read "
-                         "Objective-C class data in the process. This may "
-                         "reduce the quality of type information available.\n");
-      m_noclasses_warning_emitted = true;
-      break;
-    }
+  switch (reason) {
+  case SharedCacheWarningReason::eNotEnoughClassesRead:
+    Debugger::ReportWarning("warning: could not find Objective-C class data in "
+                            "the process. This may reduce the quality of type "
+                            "information available.\n",
+                            debugger.GetID(), &m_no_classes_cached_warning);
+    break;
+  case SharedCacheWarningReason::eExpressionExecutionFailure:
+    Debugger::ReportWarning(
+        "warning: could not execute support code to read "
+        "Objective-C class data in the process. This may "
+        "reduce the quality of type information available.\n",
+        debugger.GetID(), &m_no_classes_cached_warning);
+    break;
   }
 }
 
@@ -2372,17 +2368,25 @@
 
   Target &target = GetProcess()->GetTarget();
   Debugger &debugger = target.GetDebugger();
-  if (auto stream = debugger.GetAsyncOutputStream()) {
-    const char *msg = "read from the shared cache";
-    if (PlatformSP platform_sp = target.GetPlatform())
-      msg = platform_sp->IsHost()
-                ? "read from the host's in-memory shared cache"
-                : "find the on-disk shared cache for this device";
-    stream->Printf("warning: libobjc.A.dylib is being read from process "
-                   "memory. This indicates that LLDB could not %s. This will "
-                   "likely reduce debugging performance.\n",
-                   msg);
+
+  std::string buffer;
+  llvm::raw_string_ostream os(buffer);
+
+  os << "warning: libobjc.A.dylib is being read from process memory. This "
+        "indicates that LLDB could not ";
+  if (PlatformSP platform_sp = target.GetPlatform()) {
+    if (platform_sp->IsHost()) {
+      os << "read from the host's in-memory shared cache";
+    } else {
+      os << "find the on-disk shared cache for this device";
+    }
+  } else {
+    os << "read from the shared cache";
   }
+  os << ". This will likely reduce debugging performance.\n";
+
+  Debugger::ReportWarning(os.str(), debugger.GetID(),
+                          &m_no_expanded_cache_warning);
 }
 
 DeclVendor *AppleObjCRuntimeV2::GetDeclVendor() {
Index: lldb/source/Core/DebuggerEvents.cpp
===================================================================
--- lldb/source/Core/DebuggerEvents.cpp
+++ lldb/source/Core/DebuggerEvents.cpp
@@ -10,6 +10,15 @@
 
 using namespace lldb_private;
 
+template <typename T>
+static const T *GetEventDataFromEventImpl(const Event *event_ptr) {
+  if (event_ptr)
+    if (const EventData *event_data = event_ptr->GetData())
+      if (event_data->GetFlavor() == T::GetFlavorString())
+        return static_cast<const T *>(event_ptr->GetData());
+  return nullptr;
+}
+
 ConstString ProgressEventData::GetFlavorString() {
   static ConstString g_flavor("ProgressEventData");
   return g_flavor;
@@ -33,9 +42,33 @@
 
 const ProgressEventData *
 ProgressEventData::GetEventDataFromEvent(const Event *event_ptr) {
-  if (event_ptr)
-    if (const EventData *event_data = event_ptr->GetData())
-      if (event_data->GetFlavor() == ProgressEventData::GetFlavorString())
-        return static_cast<const ProgressEventData *>(event_ptr->GetData());
-  return nullptr;
+  return GetEventDataFromEventImpl<ProgressEventData>(event_ptr);
+}
+
+ConstString WarningEventData::GetFlavorString() {
+  static ConstString g_flavor("WarningEventData");
+  return g_flavor;
+}
+
+ConstString WarningEventData::GetFlavor() const {
+  return WarningEventData::GetFlavorString();
+}
+
+const WarningEventData *
+WarningEventData::GetEventDataFromEvent(const Event *event_ptr) {
+  return GetEventDataFromEventImpl<WarningEventData>(event_ptr);
+}
+
+ConstString ErrorEventData::GetFlavorString() {
+  static ConstString g_flavor("ErrorEventData");
+  return g_flavor;
+}
+
+ConstString ErrorEventData::GetFlavor() const {
+  return ErrorEventData::GetFlavorString();
+}
+
+const ErrorEventData *
+ErrorEventData::GetEventDataFromEvent(const Event *event_ptr) {
+  return GetEventDataFromEventImpl<ErrorEventData>(event_ptr);
 }
Index: lldb/source/Core/Debugger.cpp
===================================================================
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -1326,6 +1326,63 @@
   }
 }
 
+template <typename T>
+static void PrivateReportDiagnostic(Debugger &debugger, uint32_t event_type,
+                                    std::string message,
+                                    bool debugger_specific) {
+  Broadcaster &broadcaster = debugger.GetBroadcaster();
+  if (!broadcaster.EventTypeHasListeners(event_type))
+    return;
+  EventSP event_sp = std::make_shared<Event>(
+      event_type, new T(std::move(message), debugger_specific));
+  broadcaster.BroadcastEvent(event_sp);
+}
+
+template <typename T>
+void Debugger::ReportDiagnosticImpl(uint32_t event_type, std::string message,
+                                    llvm::Optional<lldb::user_id_t> debugger_id,
+                                    std::once_flag *once) {
+  auto ReportDiagnosticLambda = [&]() {
+    // Check if this progress 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<T>(*debugger_sp, event_type, std::move(message),
+                                   true);
+      return;
+    }
+    // The progress event is not debugger specific, iterate over all debuggers
+    // and deliver a progress event to each one.
+    if (g_debugger_list_ptr && g_debugger_list_mutex_ptr) {
+      std::lock_guard<std::recursive_mutex> guard(*g_debugger_list_mutex_ptr);
+      for (const auto &debugger : *g_debugger_list_ptr)
+        PrivateReportDiagnostic<T>(*debugger, event_type, message, false);
+    }
+  };
+
+  if (once)
+    std::call_once(*once, ReportDiagnosticLambda);
+  else
+    ReportDiagnosticLambda();
+}
+
+void Debugger::ReportWarning(std::string message,
+                             llvm::Optional<lldb::user_id_t> debugger_id,
+                             std::once_flag *once) {
+  ReportDiagnosticImpl<WarningEventData>(Debugger::eBroadcastBitWarning,
+                                         std::move(message), debugger_id, once);
+}
+
+void Debugger::ReportError(std::string message,
+                           llvm::Optional<lldb::user_id_t> debugger_id,
+                           std::once_flag *once) {
+
+  ReportDiagnosticImpl<ErrorEventData>(Debugger::eBroadcastBitError,
+                                       std::move(message), debugger_id, once);
+}
+
 bool Debugger::EnableLog(llvm::StringRef channel,
                          llvm::ArrayRef<const char *> categories,
                          llvm::StringRef log_file, uint32_t log_options,
@@ -1607,6 +1664,10 @@
 
   listener_sp->StartListeningForEvents(&m_broadcaster,
                                        Debugger::eBroadcastBitProgress);
+  listener_sp->StartListeningForEvents(&m_broadcaster,
+                                       Debugger::eBroadcastBitWarning);
+  listener_sp->StartListeningForEvents(&m_broadcaster,
+                                       Debugger::eBroadcastBitError);
 
   // Let the thread that spawned us know that we have started up and that we
   // are now listening to all required events so no events get missed
@@ -1660,6 +1721,10 @@
           } else if (broadcaster == &m_broadcaster) {
             if (event_type & Debugger::eBroadcastBitProgress)
               HandleProgressEvent(event_sp);
+            else if (event_type & Debugger::eBroadcastBitWarning)
+              HandleWarningEvent(event_sp);
+            else if (event_type & Debugger::eBroadcastBitError)
+              HandleErrorEvent(event_sp);
           }
         }
 
@@ -1793,6 +1858,24 @@
   output->Flush();
 }
 
+void Debugger::HandleWarningEvent(const lldb::EventSP &event_sp) {
+  auto *data = WarningEventData::GetEventDataFromEvent(event_sp.get());
+  if (!data)
+    return;
+
+  Stream &stream = *GetAsyncErrorStream();
+  stream << "warning: " << data->GetMessage() << '\n';
+}
+
+void Debugger::HandleErrorEvent(const lldb::EventSP &event_sp) {
+  auto *data = ErrorEventData::GetEventDataFromEvent(event_sp.get());
+  if (!data)
+    return;
+
+  Stream &stream = *GetAsyncErrorStream();
+  stream << "error: " << data->GetMessage() << '\n';
+}
+
 bool Debugger::HasIOHandlerThread() { return m_io_handler_thread.IsJoinable(); }
 
 bool Debugger::StartIOHandlerThread() {
Index: lldb/include/lldb/Core/DebuggerEvents.h
===================================================================
--- lldb/include/lldb/Core/DebuggerEvents.h
+++ lldb/include/lldb/Core/DebuggerEvents.h
@@ -46,6 +46,54 @@
   ProgressEventData(const ProgressEventData &) = delete;
   const ProgressEventData &operator=(const ProgressEventData &) = delete;
 };
+
+class DiagnosticEventData : public EventData {
+public:
+  enum class Type {
+    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() {}
+
+  const std::string &GetMessage() const { return m_message; }
+  Type GetType() const { return m_type; }
+
+protected:
+  std::string m_message;
+  Type m_type;
+  const bool m_debugger_specific;
+
+  DiagnosticEventData(const DiagnosticEventData &) = delete;
+  const DiagnosticEventData &operator=(const DiagnosticEventData &) = delete;
+};
+
+class WarningEventData : public DiagnosticEventData {
+public:
+  WarningEventData(std::string message, bool debugger_specific)
+      : DiagnosticEventData(DiagnosticEventData::Type::Warning,
+                            std::move(message), debugger_specific) {}
+
+  static ConstString GetFlavorString();
+  ConstString GetFlavor() const override;
+
+  static const WarningEventData *GetEventDataFromEvent(const Event *event_ptr);
+};
+
+class ErrorEventData : public DiagnosticEventData {
+public:
+  ErrorEventData(std::string message, bool debugger_specific)
+      : DiagnosticEventData(DiagnosticEventData::Type::Error,
+                            std::move(message), debugger_specific) {}
+
+  static ConstString GetFlavorString();
+  ConstString GetFlavor() const override;
+
+  static const ErrorEventData *GetEventDataFromEvent(const Event *event_ptr);
+};
+
 } // namespace lldb_private
 
 #endif // LLDB_CORE_DEBUGGER_EVENTS_H
Index: lldb/include/lldb/Core/Debugger.h
===================================================================
--- lldb/include/lldb/Core/Debugger.h
+++ lldb/include/lldb/Core/Debugger.h
@@ -14,6 +14,7 @@
 #include <memory>
 #include <vector>
 
+#include "lldb/Core/DebuggerEvents.h"
 #include "lldb/Core/FormatEntity.h"
 #include "lldb/Core/IOHandler.h"
 #include "lldb/Core/SourceManager.h"
@@ -57,7 +58,6 @@
 class Stream;
 class SymbolContext;
 class Target;
-class ProgressEventData;
 
 namespace repro {
 class DataRecorder;
@@ -77,6 +77,8 @@
   /// Broadcaster event bits definitions.
   enum {
     eBroadcastBitProgress = (1 << 0),
+    eBroadcastBitWarning = (1 << 1),
+    eBroadcastBitError = (1 << 2),
   };
 
   static ConstString GetStaticBroadcasterClass();
@@ -375,6 +377,35 @@
     return m_broadcaster_manager_sp;
   }
 
+  /// Report warning events.
+  ///
+  /// Progress events will be delivered to any debuggers that have listeners
+  /// for the eBroadcastBitError.
+  ///
+  /// \param[in] message
+  ///   The warning message to be reported.
+  ///
+  /// \param [in] debugger_id
+  ///   If this optional parameter has a value, it indicates the unique
+  ///   debugger identifier that this progress should be delivered to. If this
+  ///   optional parameter does not have a value, the progress will be
+  ///   delivered to all debuggers.
+  static void ReportWarning(std::string messsage,
+                            llvm::Optional<lldb::user_id_t> debugger_id,
+                            std::once_flag *once = nullptr);
+
+  /// Report error events.
+  ///
+  /// Progress events will be delivered to any debuggers that have listeners
+  /// for the eBroadcastBitError.
+  ///
+  /// \param[in] message
+  ///   The error message to be reported.
+  ///
+  static void ReportError(std::string messsage,
+                          llvm::Optional<lldb::user_id_t> debugger_id,
+                          std::once_flag *once = nullptr);
+
 protected:
   friend class CommandInterpreter;
   friend class REPL;
@@ -413,6 +444,11 @@
                              uint64_t completed, uint64_t total,
                              llvm::Optional<lldb::user_id_t> debugger_id);
 
+  template <typename T>
+  static void ReportDiagnosticImpl(uint32_t event_type, std::string message,
+                                   llvm::Optional<lldb::user_id_t> debugger_id,
+                                   std::once_flag *once);
+
   void PrintProgress(const ProgressEventData &data);
 
   bool StartEventHandlerThread();
@@ -444,6 +480,10 @@
 
   void HandleProgressEvent(const lldb::EventSP &event_sp);
 
+  void HandleWarningEvent(const lldb::EventSP &event_sp);
+
+  void HandleErrorEvent(const lldb::EventSP &event_sp);
+
   // Ensures two threads don't attempt to flush process output in parallel.
   std::mutex m_output_flush_mutex;
   void FlushProcessOutput(Process &process, bool flush_stdout,
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to