[Lldb-commits] [PATCH] D84404: [lldb] Make deleting frame recognizers actually work

[Raphael Isemann via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG5477fbc29446: [lldb] Make deleting frame recognizers 
actually work (authored by teemperor).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84404

Files:
  lldb/include/lldb/Target/StackFrameRecognizer.h
  lldb/source/Commands/CommandObjectFrame.cpp
  lldb/source/Target/StackFrameRecognizer.cpp
  lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py


Index: lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py
===================================================================
--- lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py
+++ lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py
@@ -44,8 +44,24 @@
 
         self.runCmd("frame recognizer delete 0")
 
+        # Test that it deleted the recognizer with id 0.
         self.expect("frame recognizer list",
                     substrs=['1: recognizer.MyOtherFrameRecognizer, module a.out, symbol bar (regexp)'])
+        self.expect("frame recognizer list", matching=False,
+                    substrs=['MyFrameRecognizer'])
+
+        # Test that an invalid index and deleting the same index again
+        # is an error and doesn't do any changes.
+        self.expect("frame recognizer delete 2", error=True,
+                    substrs=["error: '2' is not a valid recognizer id."])
+        self.expect("frame recognizer delete 0", error=True,
+                    substrs=["error: '0' is not a valid recognizer id."])
+        # Recognizers should have the same state as above.
+        self.expect("frame recognizer list",
+                    substrs=['1: recognizer.MyOtherFrameRecognizer, module a.out, symbol bar (regexp)'])
+        self.expect("frame recognizer list", matching=False,
+                    substrs=['MyFrameRecognizer'])
+
 
         self.runCmd("frame recognizer clear")
 
Index: lldb/source/Target/StackFrameRecognizer.cpp
===================================================================
--- lldb/source/Target/StackFrameRecognizer.cpp
+++ lldb/source/Target/StackFrameRecognizer.cpp
@@ -50,17 +50,17 @@
 void StackFrameRecognizerManager::AddRecognizer(
     StackFrameRecognizerSP recognizer, ConstString module,
     llvm::ArrayRef<ConstString> symbols, bool first_instruction_only) {
-  m_recognizers.push_front({(uint32_t)m_recognizers.size(), false, recognizer,
-                            false, module, RegularExpressionSP(), symbols,
+  m_recognizers.push_front({(uint32_t)m_recognizers.size(), recognizer, false,
+                            module, RegularExpressionSP(), symbols,
                             RegularExpressionSP(), first_instruction_only});
 }
 
 void StackFrameRecognizerManager::AddRecognizer(
     StackFrameRecognizerSP recognizer, RegularExpressionSP module,
     RegularExpressionSP symbol, bool first_instruction_only) {
-  m_recognizers.push_front(
-      {(uint32_t)m_recognizers.size(), false, recognizer, true, ConstString(),
-       module, std::vector<ConstString>(), symbol, first_instruction_only});
+  m_recognizers.push_front({(uint32_t)m_recognizers.size(), recognizer, true,
+                            ConstString(), module, std::vector<ConstString>(),
+                            symbol, first_instruction_only});
 }
 
 void StackFrameRecognizerManager::ForEach(
@@ -90,9 +90,13 @@
     uint32_t recognizer_id) {
   if (recognizer_id >= m_recognizers.size())
     return false;
-  if (m_recognizers[recognizer_id].deleted)
+  auto found =
+      llvm::find_if(m_recognizers, [recognizer_id](const RegisteredEntry &e) {
+        return e.recognizer_id == recognizer_id;
+      });
+  if (found == m_recognizers.end())
     return false;
-  m_recognizers[recognizer_id].deleted = true;
+  m_recognizers.erase(found);
   return true;
 }
 
@@ -116,8 +120,6 @@
   Address current_addr = frame->GetFrameCodeAddress();
 
   for (auto entry : m_recognizers) {
-    if (entry.deleted)
-      continue;
     if (entry.module)
       if (entry.module != module_name)
         continue;
Index: lldb/source/Commands/CommandObjectFrame.cpp
===================================================================
--- lldb/source/Commands/CommandObjectFrame.cpp
+++ lldb/source/Commands/CommandObjectFrame.cpp
@@ -999,9 +999,14 @@
       return false;
     }
 
-    GetSelectedOrDummyTarget()
-        .GetFrameRecognizerManager()
-        .RemoveRecognizerWithID(recognizer_id);
+    if (!GetSelectedOrDummyTarget()
+             .GetFrameRecognizerManager()
+             .RemoveRecognizerWithID(recognizer_id)) {
+      result.AppendErrorWithFormat("'%s' is not a valid recognizer id.\n",
+                                   command.GetArgumentAtIndex(0));
+      result.SetStatus(eReturnStatusFailed);
+      return false;
+    }
     result.SetStatus(eReturnStatusSuccessFinishResult);
     return result.Succeeded();
   }
Index: lldb/include/lldb/Target/StackFrameRecognizer.h
===================================================================
--- lldb/include/lldb/Target/StackFrameRecognizer.h
+++ lldb/include/lldb/Target/StackFrameRecognizer.h
@@ -125,7 +125,6 @@
 private:
   struct RegisteredEntry {
     uint32_t recognizer_id;
-    bool deleted;
     lldb::StackFrameRecognizerSP recognizer;
     bool is_regexp;
     ConstString module;

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits