From d8d252fe96877f9fdbf6b0b6c19da543e796bb84 Mon Sep 17 00:00:00 2001 From: Adrian Vogelsgesang Date: Fri, 20 Sep 2024 22:17:42 +0200 Subject: [PATCH] [lldb] Add support for disabling frame recognizers (#109219) Sometimes you only want to temporarily disable a frame recognizer instead of deleting it. In particular, when dealing with one of the builtin frame recognizers, which cannot be restored after deletion. To be able to write test cases for this functionality, I also changed `lldb/test/API/commands/frame/recognizer` to use normal C instead of Objective-C --- .../lldb/Target/StackFrameRecognizer.h | 13 +- lldb/source/Commands/CommandObjectFrame.cpp | 151 +++++++++++++----- lldb/source/Target/StackFrameRecognizer.cpp | 38 +++-- .../API/commands/frame/recognizer/Makefile | 2 +- .../frame/recognizer/TestFrameRecognizer.py | 70 ++++++-- .../API/commands/frame/recognizer/categories | 1 - .../frame/recognizer/{main.m => main.c} | 2 +- .../Target/StackFrameRecognizerTest.cpp | 4 +- 8 files changed, 205 insertions(+), 76 deletions(-) delete mode 100644 lldb/test/API/commands/frame/recognizer/categories rename lldb/test/API/commands/frame/recognizer/{main.m => main.c} (93%) diff --git a/lldb/include/lldb/Target/StackFrameRecognizer.h b/lldb/include/lldb/Target/StackFrameRecognizer.h index 617b1617d404..6c67a7fb4f68 100644 --- a/lldb/include/lldb/Target/StackFrameRecognizer.h +++ b/lldb/include/lldb/Target/StackFrameRecognizer.h @@ -124,12 +124,14 @@ public: Mangled::NamePreference symbol_mangling, bool first_instruction_only = true); - void ForEach(std::function< - void(uint32_t recognizer_id, std::string recognizer_name, - std::string module, llvm::ArrayRef symbols, - Mangled::NamePreference name_reference, bool regexp)> const - &callback); + void + ForEach(std::function symbols, + Mangled::NamePreference name_preference, + bool regexp)> const &callback); + bool SetEnabledForID(uint32_t recognizer_id, bool enabled); bool RemoveRecognizerWithID(uint32_t recognizer_id); void RemoveAllRecognizers(); @@ -155,6 +157,7 @@ private: lldb::RegularExpressionSP symbol_regexp; Mangled::NamePreference symbol_mangling; bool first_instruction_only; + bool enabled; }; std::deque m_recognizers; diff --git a/lldb/source/Commands/CommandObjectFrame.cpp b/lldb/source/Commands/CommandObjectFrame.cpp index d8091e8993fd..142f96946ed3 100644 --- a/lldb/source/Commands/CommandObjectFrame.cpp +++ b/lldb/source/Commands/CommandObjectFrame.cpp @@ -930,10 +930,13 @@ protected: }; static void -PrintRecognizerDetails(Stream &strm, const std::string &name, +PrintRecognizerDetails(Stream &strm, const std::string &name, bool enabled, const std::string &module, llvm::ArrayRef symbols, Mangled::NamePreference symbol_mangling, bool regexp) { + if (!enabled) + strm << "[disabled] "; + strm << name << ", "; if (!module.empty()) @@ -957,17 +960,18 @@ PrintRecognizerDetails(Stream &strm, const std::string &name, llvm::interleaveComma(symbols, strm); } -class CommandObjectFrameRecognizerDelete : public CommandObjectParsed { +// Base class for commands which accept a single frame recognizer as an argument +class CommandObjectWithFrameRecognizerArg : public CommandObjectParsed { public: - CommandObjectFrameRecognizerDelete(CommandInterpreter &interpreter) - : CommandObjectParsed(interpreter, "frame recognizer delete", - "Delete an existing frame recognizer by id.", - nullptr) { + CommandObjectWithFrameRecognizerArg(CommandInterpreter &interpreter, + const char *name, + const char *help = nullptr, + const char *syntax = nullptr, + uint32_t flags = 0) + : CommandObjectParsed(interpreter, name, help, syntax, flags) { AddSimpleArgumentList(eArgTypeRecognizerID); } - ~CommandObjectFrameRecognizerDelete() override = default; - void HandleArgumentCompletion(CompletionRequest &request, OptionElementVector &opt_element_vector) override { @@ -975,41 +979,25 @@ public: return; GetTarget().GetFrameRecognizerManager().ForEach( - [&request](uint32_t rid, std::string rname, std::string module, + [&request](uint32_t rid, bool enabled, std::string rname, + std::string module, llvm::ArrayRef symbols, Mangled::NamePreference symbol_mangling, bool regexp) { StreamString strm; if (rname.empty()) rname = "(internal)"; - PrintRecognizerDetails(strm, rname, module, symbols, symbol_mangling, - regexp); + PrintRecognizerDetails(strm, rname, enabled, module, symbols, + symbol_mangling, regexp); request.TryCompleteCurrentArg(std::to_string(rid), strm.GetString()); }); } -protected: + virtual void DoExecuteWithId(CommandReturnObject &result, + uint32_t recognizer_id) = 0; + void DoExecute(Args &command, CommandReturnObject &result) override { - if (command.GetArgumentCount() == 0) { - if (!m_interpreter.Confirm( - "About to delete all frame recognizers, do you want to do that?", - true)) { - result.AppendMessage("Operation cancelled..."); - return; - } - - GetTarget().GetFrameRecognizerManager().RemoveAllRecognizers(); - result.SetStatus(eReturnStatusSuccessFinishResult); - return; - } - - if (command.GetArgumentCount() != 1) { - result.AppendErrorWithFormat("'%s' takes zero or one arguments.\n", - m_cmd_name.c_str()); - return; - } - uint32_t recognizer_id; if (!llvm::to_integer(command.GetArgumentAtIndex(0), recognizer_id)) { result.AppendErrorWithFormat("'%s' is not a valid recognizer id.\n", @@ -1017,10 +1005,79 @@ protected: return; } - if (!GetTarget().GetFrameRecognizerManager().RemoveRecognizerWithID( - recognizer_id)) { - result.AppendErrorWithFormat("'%s' is not a valid recognizer id.\n", - command.GetArgumentAtIndex(0)); + DoExecuteWithId(result, recognizer_id); + } +}; + +class CommandObjectFrameRecognizerEnable + : public CommandObjectWithFrameRecognizerArg { +public: + CommandObjectFrameRecognizerEnable(CommandInterpreter &interpreter) + : CommandObjectWithFrameRecognizerArg( + interpreter, "frame recognizer enable", + "Enable a frame recognizer by id.", nullptr) { + AddSimpleArgumentList(eArgTypeRecognizerID); + } + + ~CommandObjectFrameRecognizerEnable() override = default; + +protected: + void DoExecuteWithId(CommandReturnObject &result, + uint32_t recognizer_id) override { + auto &recognizer_mgr = GetTarget().GetFrameRecognizerManager(); + if (!recognizer_mgr.SetEnabledForID(recognizer_id, true)) { + result.AppendErrorWithFormat("'%u' is not a valid recognizer id.\n", + recognizer_id); + return; + } + result.SetStatus(eReturnStatusSuccessFinishResult); + } +}; + +class CommandObjectFrameRecognizerDisable + : public CommandObjectWithFrameRecognizerArg { +public: + CommandObjectFrameRecognizerDisable(CommandInterpreter &interpreter) + : CommandObjectWithFrameRecognizerArg( + interpreter, "frame recognizer disable", + "Disable a frame recognizer by id.", nullptr) { + AddSimpleArgumentList(eArgTypeRecognizerID); + } + + ~CommandObjectFrameRecognizerDisable() override = default; + +protected: + void DoExecuteWithId(CommandReturnObject &result, + uint32_t recognizer_id) override { + auto &recognizer_mgr = GetTarget().GetFrameRecognizerManager(); + if (!recognizer_mgr.SetEnabledForID(recognizer_id, false)) { + result.AppendErrorWithFormat("'%u' is not a valid recognizer id.\n", + recognizer_id); + return; + } + result.SetStatus(eReturnStatusSuccessFinishResult); + } +}; + +class CommandObjectFrameRecognizerDelete + : public CommandObjectWithFrameRecognizerArg { +public: + CommandObjectFrameRecognizerDelete(CommandInterpreter &interpreter) + : CommandObjectWithFrameRecognizerArg( + interpreter, "frame recognizer delete", + "Delete an existing frame recognizer by id.", nullptr) { + AddSimpleArgumentList(eArgTypeRecognizerID); + } + + ~CommandObjectFrameRecognizerDelete() override = default; + +protected: + void DoExecuteWithId(CommandReturnObject &result, + uint32_t recognizer_id) override { + auto &recognizer_mgr = GetTarget().GetFrameRecognizerManager(); + if (!recognizer_mgr.RemoveRecognizerWithID(recognizer_id)) { + result.AppendErrorWithFormat("'%u' is not a valid recognizer id.\n", + recognizer_id); return; } result.SetStatus(eReturnStatusSuccessFinishResult); @@ -1041,7 +1098,7 @@ protected: bool any_printed = false; GetTarget().GetFrameRecognizerManager().ForEach( [&result, - &any_printed](uint32_t recognizer_id, std::string name, + &any_printed](uint32_t recognizer_id, bool enabled, std::string name, std::string module, llvm::ArrayRef symbols, Mangled::NamePreference symbol_mangling, bool regexp) { Stream &stream = result.GetOutputStream(); @@ -1050,8 +1107,8 @@ protected: name = "(internal)"; stream << std::to_string(recognizer_id) << ": "; - PrintRecognizerDetails(stream, name, module, symbols, symbol_mangling, - regexp); + PrintRecognizerDetails(stream, name, enabled, module, symbols, + symbol_mangling, regexp); stream.EOL(); stream.Flush(); @@ -1135,18 +1192,24 @@ public: interpreter, "frame recognizer", "Commands for editing and viewing frame recognizers.", "frame recognizer [] ") { + LoadSubCommand("info", CommandObjectSP(new CommandObjectFrameRecognizerInfo( + interpreter))); + LoadSubCommand("list", CommandObjectSP(new CommandObjectFrameRecognizerList( + interpreter))); LoadSubCommand("add", CommandObjectSP(new CommandObjectFrameRecognizerAdd( interpreter))); LoadSubCommand( - "clear", - CommandObjectSP(new CommandObjectFrameRecognizerClear(interpreter))); + "enable", + CommandObjectSP(new CommandObjectFrameRecognizerEnable(interpreter))); + LoadSubCommand( + "disable", + CommandObjectSP(new CommandObjectFrameRecognizerDisable(interpreter))); LoadSubCommand( "delete", CommandObjectSP(new CommandObjectFrameRecognizerDelete(interpreter))); - LoadSubCommand("list", CommandObjectSP(new CommandObjectFrameRecognizerList( - interpreter))); - LoadSubCommand("info", CommandObjectSP(new CommandObjectFrameRecognizerInfo( - interpreter))); + LoadSubCommand( + "clear", + CommandObjectSP(new CommandObjectFrameRecognizerClear(interpreter))); } ~CommandObjectFrameRecognizer() override = default; diff --git a/lldb/source/Target/StackFrameRecognizer.cpp b/lldb/source/Target/StackFrameRecognizer.cpp index fa24253320a3..d23c1fa1a928 100644 --- a/lldb/source/Target/StackFrameRecognizer.cpp +++ b/lldb/source/Target/StackFrameRecognizer.cpp @@ -67,7 +67,7 @@ void StackFrameRecognizerManager::AddRecognizer( m_recognizers.push_front({(uint32_t)m_recognizers.size(), recognizer, false, module, RegularExpressionSP(), symbols, RegularExpressionSP(), symbol_mangling, - first_instruction_only}); + first_instruction_only, true}); BumpGeneration(); } @@ -77,14 +77,15 @@ void StackFrameRecognizerManager::AddRecognizer( bool first_instruction_only) { m_recognizers.push_front({(uint32_t)m_recognizers.size(), recognizer, true, ConstString(), module, std::vector(), - symbol, symbol_mangling, first_instruction_only}); + symbol, symbol_mangling, first_instruction_only, + true}); BumpGeneration(); } void StackFrameRecognizerManager::ForEach( - const std::function< - void(uint32_t, std::string, std::string, llvm::ArrayRef, - Mangled::NamePreference name_reference, bool)> &callback) { + const std::function, + Mangled::NamePreference name_preference, bool)> &callback) { for (auto entry : m_recognizers) { if (entry.is_regexp) { std::string module_name; @@ -95,22 +96,32 @@ void StackFrameRecognizerManager::ForEach( if (entry.symbol_regexp) symbol_name = entry.symbol_regexp->GetText().str(); - callback(entry.recognizer_id, entry.recognizer->GetName(), module_name, - llvm::ArrayRef(ConstString(symbol_name)), entry.symbol_mangling, - true); - + callback(entry.recognizer_id, entry.enabled, entry.recognizer->GetName(), + module_name, llvm::ArrayRef(ConstString(symbol_name)), + entry.symbol_mangling, true); } else { - callback(entry.recognizer_id, entry.recognizer->GetName(), + callback(entry.recognizer_id, entry.enabled, entry.recognizer->GetName(), entry.module.GetCString(), entry.symbols, entry.symbol_mangling, false); } } } +bool StackFrameRecognizerManager::SetEnabledForID(uint32_t recognizer_id, + bool enabled) { + 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; + found->enabled = enabled; + BumpGeneration(); + return true; +} + bool StackFrameRecognizerManager::RemoveRecognizerWithID( uint32_t recognizer_id) { - if (recognizer_id >= m_recognizers.size()) - return false; auto found = llvm::find_if(m_recognizers, [recognizer_id](const RegisteredEntry &e) { return e.recognizer_id == recognizer_id; @@ -142,6 +153,9 @@ StackFrameRecognizerManager::GetRecognizerForFrame(StackFrameSP frame) { Address current_addr = frame->GetFrameCodeAddress(); for (auto entry : m_recognizers) { + if (!entry.enabled) + continue; + if (entry.module) if (entry.module != module_name) continue; diff --git a/lldb/test/API/commands/frame/recognizer/Makefile b/lldb/test/API/commands/frame/recognizer/Makefile index 09f6bd59a2fb..796767e24253 100644 --- a/lldb/test/API/commands/frame/recognizer/Makefile +++ b/lldb/test/API/commands/frame/recognizer/Makefile @@ -1,4 +1,4 @@ -OBJC_SOURCES := main.m +C_SOURCES := main.c CFLAGS_EXTRAS := -g0 # No debug info. MAKE_DSYM := NO diff --git a/lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py b/lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py index 24d6a67c0ccd..aa2a44808743 100644 --- a/lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py +++ b/lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py @@ -14,10 +14,13 @@ import recognizer class FrameRecognizerTestCase(TestBase): NO_DEBUG_INFO_TESTCASE = True - @skipUnlessDarwin def test_frame_recognizer_1(self): self.build() exe = self.getBuildArtifact("a.out") + target, process, thread, _ = lldbutil.run_to_name_breakpoint( + self, "foo", exe_name=exe + ) + frame = thread.GetSelectedFrame() # Clear internal & plugins recognizers that get initialized at launch self.runCmd("frame recognizer clear") @@ -96,11 +99,6 @@ class FrameRecognizerTestCase(TestBase): "frame recognizer add -l recognizer.MyFrameRecognizer -s a.out -n foo" ) - target, process, thread, _ = lldbutil.run_to_name_breakpoint( - self, "foo", exe_name=exe - ) - frame = thread.GetSelectedFrame() - self.expect("frame variable", substrs=["(int) a = 42", "(int) b = 56"]) # Recognized arguments don't show up by default... @@ -164,7 +162,6 @@ class FrameRecognizerTestCase(TestBase): substrs=['*a = 78']) """ - @skipUnlessDarwin def test_frame_recognizer_hiding(self): self.build() @@ -204,7 +201,6 @@ class FrameRecognizerTestCase(TestBase): frame = thread.GetSelectedFrame() self.assertIn("main", frame.name) - @skipUnlessDarwin def test_frame_recognizer_multi_symbol(self): self.build() exe = self.getBuildArtifact("a.out") @@ -250,7 +246,6 @@ class FrameRecognizerTestCase(TestBase): substrs=["frame 0 is recognized by recognizer.MyFrameRecognizer"], ) - @skipUnlessDarwin def test_frame_recognizer_target_specific(self): self.build() exe = self.getBuildArtifact("a.out") @@ -318,7 +313,6 @@ class FrameRecognizerTestCase(TestBase): substrs=["frame 0 is recognized by recognizer.MyFrameRecognizer"], ) - @skipUnlessDarwin def test_frame_recognizer_not_only_first_instruction(self): self.build() exe = self.getBuildArtifact("a.out") @@ -395,6 +389,62 @@ class FrameRecognizerTestCase(TestBase): variables.GetValueAtIndex(1).GetValueType(), lldb.eValueTypeVariableArgument ) + def test_frame_recognizer_disable(self): + self.build() + exe = self.getBuildArtifact("a.out") + target, process, thread, _ = lldbutil.run_to_name_breakpoint( + self, "foo", exe_name=exe + ) + + # Clear internal & plugins recognizers that get initialized at launch. + self.runCmd("frame recognizer clear") + + self.runCmd( + "command script import " + + os.path.join(self.getSourceDir(), "recognizer.py") + ) + + # Add a frame recognizer in that target. + self.runCmd( + "frame recognizer add -l recognizer.MyFrameRecognizer -s a.out -n foo -n bar" + ) + + # The frame is recognized + self.expect( + "frame recognizer info 0", + substrs=["frame 0 is recognized by recognizer.MyFrameRecognizer"], + ) + + # Disable the recognizer + self.runCmd("frame recognizer disable 0") + + self.expect( + "frame recognizer list", + substrs=[ + "0: [disabled] recognizer.MyFrameRecognizer, module a.out, demangled symbol foo" + ], + ) + + self.expect( + "frame recognizer info 0", + substrs=["frame 0 not recognized by any recognizer"], + ) + + # Re-enable the recognizer + self.runCmd("frame recognizer enable 0") + + self.expect( + "frame recognizer list", + substrs=[ + "0: recognizer.MyFrameRecognizer, module a.out, demangled symbol foo" + ], + ) + + self.expect( + "frame recognizer info 0", + substrs=["frame 0 is recognized by recognizer.MyFrameRecognizer"], + ) + @no_debug_info_test def test_frame_recognizer_delete_invalid_arg(self): self.expect( diff --git a/lldb/test/API/commands/frame/recognizer/categories b/lldb/test/API/commands/frame/recognizer/categories deleted file mode 100644 index 72cf07c1efea..000000000000 --- a/lldb/test/API/commands/frame/recognizer/categories +++ /dev/null @@ -1 +0,0 @@ -objc diff --git a/lldb/test/API/commands/frame/recognizer/main.m b/lldb/test/API/commands/frame/recognizer/main.c similarity index 93% rename from lldb/test/API/commands/frame/recognizer/main.m rename to lldb/test/API/commands/frame/recognizer/main.c index 74d219f1fff4..aa3089478c21 100644 --- a/lldb/test/API/commands/frame/recognizer/main.m +++ b/lldb/test/API/commands/frame/recognizer/main.c @@ -1,4 +1,4 @@ -#import +#include void foo(int a, int b) { printf("%d %d\n", a, b); } diff --git a/lldb/unittests/Target/StackFrameRecognizerTest.cpp b/lldb/unittests/Target/StackFrameRecognizerTest.cpp index df4458e3138c..55bae3e30157 100644 --- a/lldb/unittests/Target/StackFrameRecognizerTest.cpp +++ b/lldb/unittests/Target/StackFrameRecognizerTest.cpp @@ -69,8 +69,8 @@ TEST_F(StackFrameRecognizerTest, NullModuleRegex) { RegisterDummyStackFrameRecognizer(manager); bool any_printed = false; - manager.ForEach([&any_printed](uint32_t recognizer_id, std::string name, - std::string function, + manager.ForEach([&any_printed](uint32_t recognizer_id, bool enabled, + std::string name, std::string function, llvm::ArrayRef symbols, Mangled::NamePreference symbol_mangling, bool regexp) { any_printed = true; });