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

Report warnings and errors through events instead of printing directly the to 
the debugger's error stream. Diagnostic events are handled by the default event 
loop. This allows IDEs such as Xcode to report these issues in the UI instead 
of having them show up in the debugger console.


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,6 @@
   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;
   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,31 @@
 
 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;
   }
 
+  constexpr const bool once = true;
+
   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(DebuggerDiagnostic::SharedCacheNotEnoughClassesRead,
+                           "warning: could not find Objective-C class data in "
+                           "the process. This may reduce the quality of type "
+                           "information available.\n",
+                           once);
+    break;
+  case SharedCacheWarningReason::eExpressionExecutionFailure:
+    debugger.ReportWarning(
+        DebuggerDiagnostic::SharedCacheExpressionExecutionFailure,
+        "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",
+        once);
+    break;
   }
 }
 
@@ -2372,17 +2372,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(DebuggerDiagnostic::NoExpandedSharedCache, os.str(),
+                         true);
 }
 
 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
@@ -1323,6 +1323,26 @@
   }
 }
 
+void Debugger::ReportWarning(DebuggerDiagnostic diagnostic, std::string message,
+                             bool once) {
+  const uint32_t event_type = Debugger::eBroadcastBitWarning;
+  if (!GetBroadcaster().EventTypeHasListeners(event_type))
+    return;
+  EventSP event_sp = std::make_shared<Event>(
+      event_type, new WarningEventData(diagnostic, std::move(message), once));
+  GetBroadcaster().BroadcastEvent(event_sp);
+}
+
+void Debugger::ReportError(DebuggerDiagnostic diagnostic, std::string message,
+                           bool once) {
+  const uint32_t event_type = Debugger::eBroadcastBitError;
+  if (!GetBroadcaster().EventTypeHasListeners(event_type))
+    return;
+  EventSP event_sp = std::make_shared<Event>(
+      event_type, new ErrorEventData(diagnostic, std::move(message), once));
+  GetBroadcaster().BroadcastEvent(event_sp);
+}
+
 bool Debugger::EnableLog(llvm::StringRef channel,
                          llvm::ArrayRef<const char *> categories,
                          llvm::StringRef log_file, uint32_t log_options,
@@ -1604,6 +1624,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
@@ -1657,6 +1681,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);
           }
         }
 
@@ -1788,6 +1816,32 @@
   output.Flush();
 }
 
+void Debugger::HandleWarningEvent(const lldb::EventSP &event_sp) {
+  auto *data = WarningEventData::GetEventDataFromEvent(event_sp.get());
+  if (!data)
+    return;
+
+  if (data->GetReportOnce())
+    if (!m_reported_diagnostics.insert(data->GetDiagnostic()).second)
+      return;
+
+  StreamFile &output = GetOutputStream();
+  output << "warning: " << data->GetMessage() << '\n';
+}
+
+void Debugger::HandleErrorEvent(const lldb::EventSP &event_sp) {
+  auto *data = ErrorEventData::GetEventDataFromEvent(event_sp.get());
+  if (!data)
+    return;
+
+  if (data->GetReportOnce())
+    if (!m_reported_diagnostics.insert(data->GetDiagnostic()).second)
+      return;
+
+  StreamFile &output = GetOutputStream();
+  output << "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,69 @@
   ProgressEventData(const ProgressEventData &) = delete;
   const ProgressEventData &operator=(const ProgressEventData &) = delete;
 };
+
+enum class DebuggerDiagnostic {
+  SharedCacheNotEnoughClassesRead,
+  SharedCacheExpressionExecutionFailure,
+  NoExpandedSharedCache,
+};
+
+class DiagnosticEventData : public EventData {
+public:
+  enum class Type {
+    Warning,
+    Error,
+  };
+  DiagnosticEventData(Type type, DebuggerDiagnostic diagnostic,
+                      std::string message, bool once)
+      : m_message(std::move(message)), m_diagnostic(diagnostic), m_type(type) {}
+  ~DiagnosticEventData() {}
+
+  const std::string &GetMessage() const { return m_message; }
+  Type GetType() const { return m_type; }
+  DebuggerDiagnostic GetDiagnostic() const { return m_diagnostic; }
+  bool GetReportOnce() const { return m_report_once; }
+
+  bool operator==(const DiagnosticEventData &RHS) {
+    return GetDiagnostic() == RHS.GetDiagnostic();
+  }
+
+protected:
+  std::string m_message;
+  DebuggerDiagnostic m_diagnostic;
+  Type m_type;
+  bool m_report_once;
+
+  DiagnosticEventData(const DiagnosticEventData &) = delete;
+  const DiagnosticEventData &operator=(const DiagnosticEventData &) = delete;
+};
+
+class WarningEventData : public DiagnosticEventData {
+public:
+  WarningEventData(DebuggerDiagnostic diagnostic, std::string message,
+                   bool once = false)
+      : DiagnosticEventData(DiagnosticEventData::Type::Warning, diagnostic,
+                            std::move(message), once){};
+
+  static ConstString GetFlavorString();
+  ConstString GetFlavor() const override;
+
+  static const WarningEventData *GetEventDataFromEvent(const Event *event_ptr);
+};
+
+class ErrorEventData : public DiagnosticEventData {
+public:
+  ErrorEventData(DebuggerDiagnostic diagnostic, std::string message,
+                 bool once = false)
+      : DiagnosticEventData(DiagnosticEventData::Type::Error, diagnostic,
+                            std::move(message), once) {}
+
+  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"
@@ -37,6 +38,7 @@
 #include "lldb/lldb-types.h"
 
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/DynamicLibrary.h"
@@ -57,7 +59,6 @@
 class Stream;
 class SymbolContext;
 class Target;
-class ProgressEventData;
 
 namespace repro {
 class DataRecorder;
@@ -77,6 +78,8 @@
   /// Broadcaster event bits definitions.
   enum {
     eBroadcastBitProgress = (1 << 0),
+    eBroadcastBitWarning = (1 << 1),
+    eBroadcastBitError = (1 << 2),
   };
 
   static ConstString GetStaticBroadcasterClass();
@@ -375,6 +378,26 @@
     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.
+  void ReportWarning(DebuggerDiagnostic diagnostic, std::string messsage,
+                     bool once = false);
+
+  /// 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.
+  void ReportError(DebuggerDiagnostic diagnostic, std::string messsage,
+                   bool once = false);
+
 protected:
   friend class CommandInterpreter;
   friend class REPL;
@@ -444,6 +467,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,
@@ -494,6 +521,8 @@
 
   llvm::Optional<uint64_t> m_current_event_id;
 
+  llvm::SmallSet<DebuggerDiagnostic, 3> m_reported_diagnostics;
+
   llvm::StringMap<std::weak_ptr<llvm::raw_ostream>> m_log_streams;
   std::shared_ptr<llvm::raw_ostream> m_log_callback_stream_sp;
   ConstString m_instance_name;
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to