[lldb] Store StackFrameRecognizers in the target instead of a global list

Summary:

Currently the frame recognizers are stored in a global list (the list in the
StackFrameRecognizersManagerImpl singleton to be precise). All commands and
plugins that modify the list are just modifying that global list of recognizers
which is shared by all Target and Debugger instances.

This is clearly against the idea of LLDB being usable as a library and it also
leads to some very obscure errors as now multiple tests are sharing the used
frame recognizers. For example D83400 is currently failing as it reorders some
test_ functions which permanently changes the frame recognizers of all
debuggers/targets. As all frame recognizers are also initialized in a 'once'
guard, it's also impossible to every restore back the original frame recognizers
once they are deleted in a process.

This patch just moves the frame recognizers into the current target. This seems
the way everyone assumes the system works as for example the assert frame
recognizers is using the current target to find the function/so-name to look for
(which only works if the recognizers are stored in the target).

Reviewers: jingham, mib

Reviewed By: jingham, mib

Subscribers: MrHate, JDevlieghere

Differential Revision: https://reviews.llvm.org/D83757

Author: Teemperor

lldb/include/lldb/Target/StackFrameRecognizer.h

@@ -17,6 +17,8 @@
 #include "lldb/lldb-private-forward.h"
 #include "lldb/lldb-public.h"
 
+#include <vector>
+
 namespace lldb_private {
 
 /// \class RecognizedStackFrame
@@ -95,37 +97,45 @@ class ScriptedStackFrameRecognizer : public StackFrameRecognizer {
   operator=(const ScriptedStackFrameRecognizer &) = delete;
 };
 
-/// \class StackFrameRecognizerManager
-///
-/// Static class that provides a registry of known stack frame recognizers.
-/// Has static methods to add, enumerate, remove, query and invoke recognizers.
-
+/// Class that provides a registry of known stack frame recognizers.
 class StackFrameRecognizerManager {
 public:
-  static void AddRecognizer(lldb::StackFrameRecognizerSP recognizer,
-                            ConstString module,
-                            llvm::ArrayRef<ConstString> symbols,
-                            bool first_instruction_only = true);
+  void AddRecognizer(lldb::StackFrameRecognizerSP recognizer,
+                     ConstString module, llvm::ArrayRef<ConstString> symbols,
+                     bool first_instruction_only = true);
+
+  void AddRecognizer(lldb::StackFrameRecognizerSP recognizer,
+                     lldb::RegularExpressionSP module,
+                     lldb::RegularExpressionSP symbol,
+                     bool first_instruction_only = true);
 
-  static void AddRecognizer(lldb::StackFrameRecognizerSP recognizer,
-                            lldb::RegularExpressionSP module,
-                            lldb::RegularExpressionSP symbol,
-                            bool first_instruction_only = true);
+  void ForEach(std::function<
+               void(uint32_t recognizer_id, std::string recognizer_name,
+                    std::string module, llvm::ArrayRef<ConstString> symbols,
+                    bool regexp)> const &callback);
 
-  static void
-  ForEach(std::function<void(uint32_t recognizer_id,
-                             std::string recognizer_name, std::string module,
-                             llvm::ArrayRef<ConstString> symbols,
-                             bool regexp)> const &callback);
+  bool RemoveRecognizerWithID(uint32_t recognizer_id);
 
-  static bool RemoveRecognizerWithID(uint32_t recognizer_id);
+  void RemoveAllRecognizers();
 
-  static void RemoveAllRecognizers();
+  lldb::StackFrameRecognizerSP GetRecognizerForFrame(lldb::StackFrameSP frame);
 
-  static lldb::StackFrameRecognizerSP GetRecognizerForFrame(
-      lldb::StackFrameSP frame);
+  lldb::RecognizedStackFrameSP RecognizeFrame(lldb::StackFrameSP frame);
+
+private:
+  struct RegisteredEntry {
+    uint32_t recognizer_id;
+    bool deleted;
+    lldb::StackFrameRecognizerSP recognizer;
+    bool is_regexp;
+    ConstString module;
+    lldb::RegularExpressionSP module_regexp;
+    std::vector<ConstString> symbols;
+    lldb::RegularExpressionSP symbol_regexp;
+    bool first_instruction_only;
+  };
 
-  static lldb::RecognizedStackFrameSP RecognizeFrame(lldb::StackFrameSP frame);
+  std::deque<RegisteredEntry> m_recognizers;
 };
 
 /// \class ValueObjectRecognizerSynthesizedValue

lldb/include/lldb/Target/Target.h

@@ -1251,6 +1251,10 @@ class Target : public std::enable_shared_from_this<Target>,
 
   void SetREPL(lldb::LanguageType language, lldb::REPLSP repl_sp);
 
+  StackFrameRecognizerManager &GetFrameRecognizerManager() {
+    return *m_frame_recognizer_manager_up;
+  }
+
 protected:
   /// Implementing of ModuleList::Notifier.
 
@@ -1325,6 +1329,8 @@ class Target : public std::enable_shared_from_this<Target>,
   bool m_suppress_stop_hooks;
   bool m_is_dummy_target;
   unsigned m_next_persistent_variable_index = 0;
+  /// Stores the frame recognizers of this target.
+  lldb::StackFrameRecognizerManagerUP m_frame_recognizer_manager_up;
 
   static void ImageSearchPathsChanged(const PathMappingList &path_list,
                                       void *baton);

lldb/include/lldb/lldb-forward.h

@@ -402,6 +402,8 @@ typedef std::weak_ptr<lldb_private::StackFrame> StackFrameWP;
 typedef std::shared_ptr<lldb_private::StackFrameList> StackFrameListSP;
 typedef std::shared_ptr<lldb_private::StackFrameRecognizer>
     StackFrameRecognizerSP;
+typedef std::unique_ptr<lldb_private::StackFrameRecognizerManager>
+    StackFrameRecognizerManagerUP;
 typedef std::shared_ptr<lldb_private::StopInfo> StopInfoSP;
 typedef std::shared_ptr<lldb_private::StoppointLocation> StoppointLocationSP;
 typedef std::shared_ptr<lldb_private::Stream> StreamSP;

lldb/source/Commands/CommandObjectFrame.cpp

@@ -898,12 +898,14 @@ bool CommandObjectFrameRecognizerAdd::DoExecute(Args &command,
         RegularExpressionSP(new RegularExpression(m_options.m_module));
     auto func =
         RegularExpressionSP(new RegularExpression(m_options.m_symbols.front()));
-    StackFrameRecognizerManager::AddRecognizer(recognizer_sp, module, func);
+    GetSelectedOrDummyTarget().GetFrameRecognizerManager().AddRecognizer(
+        recognizer_sp, module, func);
   } else {
     auto module = ConstString(m_options.m_module);
     std::vector<ConstString> symbols(m_options.m_symbols.begin(),
                                      m_options.m_symbols.end());
-    StackFrameRecognizerManager::AddRecognizer(recognizer_sp, module, symbols);
+    GetSelectedOrDummyTarget().GetFrameRecognizerManager().AddRecognizer(
+        recognizer_sp, module, symbols);
   }
 #endif
 
@@ -921,7 +923,9 @@ class CommandObjectFrameRecognizerClear : public CommandObjectParsed {
 
 protected:
   bool DoExecute(Args &command, CommandReturnObject &result) override {
-    StackFrameRecognizerManager::RemoveAllRecognizers();
+    GetSelectedOrDummyTarget()
+        .GetFrameRecognizerManager()
+        .RemoveAllRecognizers();
     result.SetStatus(eReturnStatusSuccessFinishResult);
     return result.Succeeded();
   }
@@ -941,7 +945,7 @@ class CommandObjectFrameRecognizerDelete : public CommandObjectParsed {
     if (request.GetCursorIndex() != 0)
       return;
 
-    StackFrameRecognizerManager::ForEach(
+    GetSelectedOrDummyTarget().GetFrameRecognizerManager().ForEach(
         [&request](uint32_t rid, std::string rname, std::string module,
                    llvm::ArrayRef<lldb_private::ConstString> symbols,
                    bool regexp) {
@@ -973,7 +977,9 @@ class CommandObjectFrameRecognizerDelete : public CommandObjectParsed {
         return false;
       }
 
-      StackFrameRecognizerManager::RemoveAllRecognizers();
+      GetSelectedOrDummyTarget()
+          .GetFrameRecognizerManager()
+          .RemoveAllRecognizers();
       result.SetStatus(eReturnStatusSuccessFinishResult);
       return result.Succeeded();
     }
@@ -993,7 +999,9 @@ class CommandObjectFrameRecognizerDelete : public CommandObjectParsed {
       return false;
     }
 
-    StackFrameRecognizerManager::RemoveRecognizerWithID(recognizer_id);
+    GetSelectedOrDummyTarget()
+        .GetFrameRecognizerManager()
+        .RemoveRecognizerWithID(recognizer_id);
     result.SetStatus(eReturnStatusSuccessFinishResult);
     return result.Succeeded();
   }
@@ -1011,7 +1019,7 @@ class CommandObjectFrameRecognizerList : public CommandObjectParsed {
 protected:
   bool DoExecute(Args &command, CommandReturnObject &result) override {
     bool any_printed = false;
-    StackFrameRecognizerManager::ForEach(
+    GetSelectedOrDummyTarget().GetFrameRecognizerManager().ForEach(
         [&result, &any_printed](
             uint32_t recognizer_id, std::string name, std::string module,
             llvm::ArrayRef<ConstString> symbols, bool regexp) {
@@ -1106,8 +1114,9 @@ class CommandObjectFrameRecognizerInfo : public CommandObjectParsed {
       return false;
     }
 
-    auto recognizer =
-        StackFrameRecognizerManager::GetRecognizerForFrame(frame_sp);
+    auto recognizer = GetSelectedOrDummyTarget()
+                          .GetFrameRecognizerManager()
+                          .GetRecognizerForFrame(frame_sp);
 
     Stream &output_stream = result.GetOutputStream();
     output_stream.Printf("frame %d ", frame_index);

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp

@@ -407,7 +407,7 @@ ExtractRuntimeGlobalSymbol(Process *process, ConstString name,
   }
 }
 
-static void RegisterObjCExceptionRecognizer();
+static void RegisterObjCExceptionRecognizer(Process *process);
 
 AppleObjCRuntimeV2::AppleObjCRuntimeV2(Process *process,
                                        const ModuleSP &objc_module_sp)
@@ -429,7 +429,7 @@ AppleObjCRuntimeV2::AppleObjCRuntimeV2(Process *process,
   m_has_object_getClass =
       (objc_module_sp->FindFirstSymbolWithNameAndType(
            g_gdb_object_getClass, eSymbolTypeCode) != nullptr);
-  RegisterObjCExceptionRecognizer();
+  RegisterObjCExceptionRecognizer(process);
 }
 
 bool AppleObjCRuntimeV2::GetDynamicTypeAndAddress(
@@ -2711,16 +2711,14 @@ class ObjCExceptionThrowFrameRecognizer : public StackFrameRecognizer {
   };
 };
 
-static void RegisterObjCExceptionRecognizer() {
-  static llvm::once_flag g_once_flag;
-  llvm::call_once(g_once_flag, []() {
-    FileSpec module;
-    ConstString function;
-    std::tie(module, function) = AppleObjCRuntime::GetExceptionThrowLocation();
-    std::vector<ConstString> symbols = {function};
-    StackFrameRecognizerManager::AddRecognizer(
-        StackFrameRecognizerSP(new ObjCExceptionThrowFrameRecognizer()),
-        module.GetFilename(), symbols,
-        /*first_instruction_only*/ true);
-  });
+static void RegisterObjCExceptionRecognizer(Process *process) {
+  FileSpec module;
+  ConstString function;
+  std::tie(module, function) = AppleObjCRuntime::GetExceptionThrowLocation();
+  std::vector<ConstString> symbols = {function};
+
+  process->GetTarget().GetFrameRecognizerManager().AddRecognizer(
+      StackFrameRecognizerSP(new ObjCExceptionThrowFrameRecognizer()),
+      module.GetFilename(), symbols,
+      /*first_instruction_only*/ true);
 }

lldb/source/Target/AssertFrameRecognizer.cpp

@@ -86,20 +86,17 @@ bool GetAssertLocation(llvm::Triple::OSType os, SymbolLocation &location) {
 }
 
 void RegisterAssertFrameRecognizer(Process *process) {
-  static llvm::once_flag g_once_flag;
-  llvm::call_once(g_once_flag, [process]() {
-    Target &target = process->GetTarget();
-    llvm::Triple::OSType os = target.GetArchitecture().GetTriple().getOS();
-    SymbolLocation location;
-
-    if (!GetAbortLocation(os, location))
-      return;
-
-    StackFrameRecognizerManager::AddRecognizer(
-        StackFrameRecognizerSP(new AssertFrameRecognizer()),
-        location.module_spec.GetFilename(), location.symbols,
-        /*first_instruction_only*/ false);
-  });
+  Target &target = process->GetTarget();
+  llvm::Triple::OSType os = target.GetArchitecture().GetTriple().getOS();
+  SymbolLocation location;
+
+  if (!GetAbortLocation(os, location))
+    return;
+
+  target.GetFrameRecognizerManager().AddRecognizer(
+      StackFrameRecognizerSP(new AssertFrameRecognizer()),
+      location.module_spec.GetFilename(), location.symbols,
+      /*first_instruction_only*/ false);
 }
 
 } // namespace lldb_private

lldb/source/Target/StackFrame.cpp

@@ -1956,8 +1956,11 @@ bool StackFrame::GetStatus(Stream &strm, bool show_frame_info, bool show_source,
 
 RecognizedStackFrameSP StackFrame::GetRecognizedFrame() {
   if (!m_recognized_frame_sp) {
-    m_recognized_frame_sp =
-        StackFrameRecognizerManager::RecognizeFrame(CalculateStackFrame());
+    m_recognized_frame_sp = GetThread()
+                                ->GetProcess()
+                                ->GetTarget()
+                                .GetFrameRecognizerManager()
+                                .RecognizeFrame(CalculateStackFrame());
   }
   return m_recognized_frame_sp;
 }