[lldb] Port Process::PrintWarning* to use the new diagnostic events

[JDevlieghere]

Summary

Port the two Process::PrintWarning functions to use the new diagnostic events through Debugger::ReportWarning. I kept the wrapper function in the process, but delegated the work to the Module. Consistent with the current code, the Module ensures the warning is only printed once per module.

[JDevlieghere]

Update tests to check stderr

[kastiglione]

lldb/include/lldb/Core/Module.h

820–825

In this diff, it seems the debugger_id is always given. Is the Optionalality for future uses?

[JDevlieghere]

lldb/include/lldb/Core/Module.h

820–825

Yeah, my thinking was that modules are rarely tied to a single debugger, but these particular function are only called from the process. I'll make them non-optional.

lldb/include/lldb/Core/Module.h

public:
   void ReportError(const char *format, ...)
       __attribute__((format(printf, 2, 3)));
 
   // Only report an error once when the module is first detected to be modified
   // so we don't spam the console with many messages.
   void ReportErrorIfModifyDetected(const char *format, ...)
       __attribute__((format(printf, 2, 3)));
 
+  void ReportWarningOptimization(llvm::Optional<lldb::user_id_t> debugger_id);
+
+  void
+  ReportWarningUnsupportedLanguage(lldb::LanguageType language,
+                                   llvm::Optional<lldb::user_id_t> debugger_id);
+
   // Return true if the file backing this module has changed since the module
   // was originally created  since we saved the initial file modification time
   // when the module first gets created.
   bool FileHasChanged() const;
 
   // SymbolFile and ObjectFile member objects should lock the
   // module mutex to avoid deadlocks.
   std::recursive_mutex &GetMutex() const { return m_mutex; }
protected:
   /// an object file and a symbol file which both have symbol tables. The parse
   /// time for the symbol tables can be aggregated here.
   StatsDuration m_symtab_parse_time;
   /// We store a symbol named index time duration here because we might have
   /// an object file and a symbol file which both have symbol tables. The parse
   /// time for the symbol tables can be aggregated here.
   StatsDuration m_symtab_index_time;
 
+  std::once_flag m_optimization_warning;
+  std::once_flag m_language_warning;
+
   /// Resolve a file or load virtual address.
   ///
   /// Tries to resolve \a vm_addr as a file address (if \a
   /// vm_addr_is_file_addr is true) or as a load address if \a
   /// vm_addr_is_file_addr is false) in the symbol vendor. \a resolve_scope
   /// indicates what clients wish to resolve and can be used to limit the
   /// scope of what is parsed.
   ///

lldb/include/lldb/Target/Process.h

public:
   };
 
   enum {
     eBroadcastInternalStateControlStop = (1 << 0),
     eBroadcastInternalStateControlPause = (1 << 1),
     eBroadcastInternalStateControlResume = (1 << 2)
   };
 
-  /// Process warning types.
-  enum Warnings { eWarningsOptimization = 1, eWarningsUnsupportedLanguage = 2 };
-
   typedef Range<lldb::addr_t, lldb::addr_t> LoadRange;
   // We use a read/write lock to allow on or more clients to access the process
   // state while the process is stopped (reader). We lock the write lock to
   // control access to the process while it is running (readers, or clients
   // that want the process stopped can block waiting for the process to stop,
   // or just try to lock it to see if they can immediately access the stopped
   // process. If the try read lock fails, then the process is running.
   typedef ProcessRunLock::ProcessRunLocker StopLocker;
protected:
   ///
   /// \return
   ///     An Status object describing the success or failure of the resume.
   Status PrivateResume();
 
   // Called internally
   void CompleteAttach();
 
-  /// Print a user-visible warning one time per Process
-  ///
-  /// A facility for printing a warning to the user once per repeat_key.
-  ///
-  /// warning_type is from the Process::Warnings enums. repeat_key is a
-  /// pointer value that will be used to ensure that the warning message is
-  /// not printed multiple times.  For instance, with a warning about a
-  /// function being optimized, you can pass the CompileUnit pointer to have
-  /// the warning issued for only the first function in a CU, or the Function
-  /// pointer to have it issued once for every function, or a Module pointer
-  /// to have it issued once per Module.
-  ///
-  /// Classes outside Process should call a specific PrintWarning method so
-  /// that the warning strings are all centralized in Process, instead of
-  /// calling PrintWarning() directly.
-  ///
-  /// \param [in] warning_type
-  ///     One of the types defined in Process::Warnings.
-  ///
-  /// \param [in] repeat_key
-  ///     A pointer value used to ensure that the warning is only printed once.
-  ///     May be nullptr, indicating that the warning is printed unconditionally
-  ///     every time.
-  ///
-  /// \param [in] fmt
-  ///     printf style format string
-  void PrintWarning(uint64_t warning_type, const void *repeat_key,
-                    const char *fmt, ...) __attribute__((format(printf, 4, 5)));
-
   // NextEventAction provides a way to register an action on the next event
   // that is delivered to this process.  There is currently only one next event
   // action allowed in the process at one time.  If a new "NextEventAction" is
   // added while one is already present, the old action will be discarded (with
   // HandleBeingUnshipped called after it is discarded.)
   //
   // If you want to resume the process as a result of a resume action, call
   // RequestResume, don't call Resume directly.
virtual Status DoWriteMemoryTags(lldb::addr_t addr, size_t len, int32_t type,
     status.SetErrorStringWithFormatv("{0} does not support writing memory tags",
                                      GetPluginName());
     return status;
   }
 
   // Type definitions
   typedef std::map<lldb::LanguageType, lldb::LanguageRuntimeSP>
       LanguageRuntimeCollection;
-  typedef std::unordered_set<const void *> WarningsPointerSet;
-  typedef std::map<uint64_t, WarningsPointerSet> WarningsCollection;
 
   struct PreResumeCallbackAndBaton {
     bool (*callback)(void *);
     void *baton;
     PreResumeCallbackAndBaton(PreResumeActionCallback in_callback,
                               void *in_baton)
         : callback(in_callback), baton(in_baton) {}
     bool operator== (const PreResumeCallbackAndBaton &rhs) {
protected:
 
   bool m_clear_thread_plans_on_stop;
   bool m_force_next_event_delivery;
   lldb::StateType m_last_broadcast_state; /// This helps with the Public event
                                           /// coalescing in
                                           /// ShouldBroadcastEvent.
   std::map<lldb::addr_t, lldb::addr_t> m_resolved_indirect_addresses;
   bool m_destroy_in_process;
-  bool m_can_interpret_function_calls;  // Some targets, e.g the OSX kernel,
-                                        // don't support the ability to modify
-                                        // the stack.
-  WarningsCollection m_warnings_issued; // A set of object pointers which have
-                                        // already had warnings printed
+  bool m_can_interpret_function_calls; // Some targets, e.g the OSX kernel,
   std::mutex m_run_thread_plan_lock;
   StructuredDataPluginMap m_structured_data_plugin_map;
 
   enum { eCanJITDontKnow = 0, eCanJITYes, eCanJITNo } m_can_jit;
 
   std::unique_ptr<UtilityFunction> m_dlopen_utility_func_up;
   llvm::once_flag m_dlopen_utility_func_flag_once;
 

lldb/source/Core/Module.cpp

bool Module::FileHasChanged() const {
   if (m_data_sp)
     return false;
   if (!m_file_has_changed)
     m_file_has_changed =
         (FileSystem::Instance().GetModificationTime(m_file) != m_mod_time);
   return m_file_has_changed;
 }
 
+void Module::ReportWarningOptimization(
+    llvm::Optional<lldb::user_id_t> debugger_id) {
+  ConstString file_name = GetFileSpec().GetFilename();
+  if (file_name.IsEmpty())
+    return;
+
+  StreamString ss;
+  ss << file_name.GetStringRef()
+     << " was compiled with optimization - stepping may behave "
+        "oddly; variables may not be available.";
+  Debugger::ReportWarning(std::string(ss.GetString()), debugger_id,
+                          &m_optimization_warning);
+}
+
+void Module::ReportWarningUnsupportedLanguage(
+    LanguageType language, llvm::Optional<lldb::user_id_t> debugger_id) {
+  StreamString ss;
+  ss << "This version of LLDB has no plugin for the language \""
+     << Language::GetNameForLanguageType(language)
+     << "\". "
+        "Inspection of frame variables will be limited.";
+  Debugger::ReportWarning(std::string(ss.GetString()), debugger_id,
+                          &m_language_warning);
+}
+
 void Module::ReportErrorIfModifyDetected(const char *format, ...) {
   if (!m_first_file_changed_log) {
     if (FileHasChanged()) {
       m_first_file_changed_log = true;
       if (format) {
         StreamString strm;
         strm.PutCString("error: the object file ");
         GetDescription(strm.AsRawOstream(), lldb::eDescriptionLevelFull);

lldb/source/Target/Process.cpp

: ProcessProperties(this),
       m_stdio_communication("process.stdio"), m_stdio_communication_mutex(),
       m_stdin_forward(false), m_stdout_data(), m_stderr_data(),
       m_profile_data_comm_mutex(), m_profile_data(), m_iohandler_sync(0),
       m_memory_cache(*this), m_allocated_memory_cache(*this),
       m_should_detach(false), m_next_event_action_up(), m_public_run_lock(),
       m_private_run_lock(), m_finalizing(false),
       m_clear_thread_plans_on_stop(false), m_force_next_event_delivery(false),
       m_last_broadcast_state(eStateInvalid), m_destroy_in_process(false),
-      m_can_interpret_function_calls(false), m_warnings_issued(),
-      m_run_thread_plan_lock(), m_can_jit(eCanJITDontKnow) {
+      m_can_interpret_function_calls(false), m_run_thread_plan_lock(),
+      m_can_jit(eCanJITDontKnow) {
   CheckInWithManager();
 
   Log *log = GetLog(LLDBLog::Object);
   LLDB_LOGF(log, "%p Process::Process()", static_cast<void *>(this));
 
   if (!m_unix_signals_sp)
     m_unix_signals_sp = std::make_shared<UnixSignals>();
 
void Process::ModulesDidLoad(ModuleList &module_list) {
 
   // Inform the structured-data plugins of the modified modules.
   for (auto pair : m_structured_data_plugin_map) {
     if (pair.second)
       pair.second->ModulesDidLoad(*this, module_list);
   }
 }
 
-void Process::PrintWarning(uint64_t warning_type, const void *repeat_key,
-                           const char *fmt, ...) {
-  bool print_warning = true;
-
-  StreamSP stream_sp = GetTarget().GetDebugger().GetAsyncOutputStream();
-  if (!stream_sp)
-    return;
-
-  if (repeat_key != nullptr) {
-    WarningsCollection::iterator it = m_warnings_issued.find(warning_type);
-    if (it == m_warnings_issued.end()) {
-      m_warnings_issued[warning_type] = WarningsPointerSet();
-      m_warnings_issued[warning_type].insert(repeat_key);
-    } else {
-      if (it->second.find(repeat_key) != it->second.end()) {
-        print_warning = false;
-      } else {
-        it->second.insert(repeat_key);
-      }
-    }
-  }
-
-  if (print_warning) {
-    va_list args;
-    va_start(args, fmt);
-    stream_sp->PrintfVarArg(fmt, args);
-    va_end(args);
-  }
-}
-
 void Process::PrintWarningOptimization(const SymbolContext &sc) {
   if (!GetWarningsOptimization())
     return;
-  if (!sc.module_sp)
+  if (!sc.module_sp || !sc.function || !sc.function->GetIsOptimized())
     return;
-  if (!sc.module_sp->GetFileSpec().GetFilename().IsEmpty() && sc.function &&
-      sc.function->GetIsOptimized()) {
-    PrintWarning(Process::Warnings::eWarningsOptimization, sc.module_sp.get(),
-                 "%s was compiled with optimization - stepping may behave "
-                 "oddly; variables may not be available.\n",
-                 sc.module_sp->GetFileSpec().GetFilename().GetCString());
-  }
+  sc.module_sp->ReportWarningOptimization(GetTarget().GetDebugger().GetID());
 }
 
 void Process::PrintWarningUnsupportedLanguage(const SymbolContext &sc) {
   if (!GetWarningsUnsupportedLanguage())
     return;
   if (!sc.module_sp)
     return;
   LanguageType language = sc.GetLanguage();
   if (language == eLanguageTypeUnknown)
     return;
   LanguageSet plugins =
       PluginManager::GetAllTypeSystemSupportedLanguagesForTypes();
-  if (!plugins[language]) {
-    PrintWarning(Process::Warnings::eWarningsUnsupportedLanguage,
-                 sc.module_sp.get(),
-                 "This version of LLDB has no plugin for the language \"%s\". "
-                 "Inspection of frame variables will be limited.\n",
-                 Language::GetNameForLanguageType(language));
-  }
+  if (plugins[language])
+    return;
+  sc.module_sp->ReportWarningUnsupportedLanguage(
+      language, GetTarget().GetDebugger().GetID());
 }
 
 bool Process::GetProcessInfo(ProcessInstanceInfo &info) {
   info.Clear();
 
   PlatformSP platform_sp = GetTarget().GetPlatform();
   if (!platform_sp)
     return false;

lldb/test/Shell/Process/Optimization.test

 Test warnings.
 REQUIRES: shell, system-darwin
 RUN: %clang_host -O3 %S/Inputs/true.c -std=c99 -g -o %t.exe
-RUN: %lldb -o "b main" -o r -o q -b %t.exe | FileCheck %s
+RUN: %lldb -o "b main" -o r -o q -b %t.exe 2>&1 | FileCheck %s
 
 CHECK: compiled with optimization

lldb/test/Shell/Process/UnsupportedLanguage.test

 Test warnings.
 REQUIRES: shell
 RUN: %clang_host %S/Inputs/true.c -std=c99 -g -c -S -emit-llvm -o - \
 RUN:   | sed -e 's/DW_LANG_C99/DW_LANG_Mips_Assembler/g' >%t.ll
 RUN: %clang_host %t.ll -g -o %t.exe
-RUN: %lldb -o "b main" -o r -o q -b %t.exe | FileCheck %s
+RUN: %lldb -o "b main" -o r -o q -b %t.exe 2>&1 | FileCheck %s
 
 CHECK: This version of LLDB has no plugin for the language "assembler"