llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Adrian Vogelsgesang (vogelsgesang) <details> <summary>Changes</summary> 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 --- Patch is 22.70 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/109219.diff 7 Files Affected: - (modified) lldb/include/lldb/Target/StackFrameRecognizer.h (+9-6) - (modified) lldb/source/Commands/CommandObjectFrame.cpp (+110-38) - (modified) lldb/source/Target/StackFrameRecognizer.cpp (+26-12) - (modified) lldb/test/API/commands/frame/recognizer/Makefile (+1-1) - (modified) lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py (+136-10) - (removed) lldb/test/API/commands/frame/recognizer/categories (-1) - (renamed) lldb/test/API/commands/frame/recognizer/main.c (+1-1) ``````````diff diff --git a/lldb/include/lldb/Target/StackFrameRecognizer.h b/lldb/include/lldb/Target/StackFrameRecognizer.h index 617b1617d404a1..6c67a7fb4f68dd 100644 --- a/lldb/include/lldb/Target/StackFrameRecognizer.h +++ b/lldb/include/lldb/Target/StackFrameRecognizer.h @@ -124,12 +124,14 @@ class StackFrameRecognizerManager { 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<ConstString> symbols, - Mangled::NamePreference name_reference, bool regexp)> const - &callback); - + void + ForEach(std::function<void(uint32_t recognizer_id, bool enabled, + std::string recognizer_name, std::string module, + llvm::ArrayRef<ConstString> 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 @@ class StackFrameRecognizerManager { lldb::RegularExpressionSP symbol_regexp; Mangled::NamePreference symbol_mangling; bool first_instruction_only; + bool enabled; }; std::deque<RegisteredEntry> m_recognizers; diff --git a/lldb/source/Commands/CommandObjectFrame.cpp b/lldb/source/Commands/CommandObjectFrame.cpp index d8091e8993fde1..d42cbeaba15f22 100644 --- a/lldb/source/Commands/CommandObjectFrame.cpp +++ b/lldb/source/Commands/CommandObjectFrame.cpp @@ -31,8 +31,10 @@ #include "lldb/Target/Thread.h" #include "lldb/Utility/Args.h" +#include <iostream> #include <memory> #include <optional> +#include <ostream> #include <string> using namespace lldb; @@ -930,10 +932,13 @@ class CommandObjectFrameRecognizerClear : public CommandObjectParsed { }; static void -PrintRecognizerDetails(Stream &strm, const std::string &name, +PrintRecognizerDetails(Stream &strm, const std::string &name, bool enabled, const std::string &module, llvm::ArrayRef<lldb_private::ConstString> symbols, Mangled::NamePreference symbol_mangling, bool regexp) { + if (!enabled) + strm << "[disabled] "; + strm << name << ", "; if (!module.empty()) @@ -957,53 +962,45 @@ 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 { + std::cerr << request.GetCursorIndex() << std::endl; if (request.GetCursorIndex() != 0) 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<lldb_private::ConstString> 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: - 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; - } + virtual void DoExecuteWithId(CommandReturnObject &result, + uint32_t recognizer_id) = 0; + void DoExecute(Args &command, CommandReturnObject &result) override { if (command.GetArgumentCount() != 1) { result.AppendErrorWithFormat("'%s' takes zero or one arguments.\n", m_cmd_name.c_str()); @@ -1017,10 +1014,79 @@ class CommandObjectFrameRecognizerDelete : public CommandObjectParsed { 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 +1107,7 @@ class CommandObjectFrameRecognizerList : public CommandObjectParsed { 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<ConstString> symbols, Mangled::NamePreference symbol_mangling, bool regexp) { Stream &stream = result.GetOutputStream(); @@ -1050,8 +1116,8 @@ class CommandObjectFrameRecognizerList : public CommandObjectParsed { 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 +1201,24 @@ class CommandObjectFrameRecognizer : public CommandObjectMultiword { interpreter, "frame recognizer", "Commands for editing and viewing frame recognizers.", "frame recognizer [<sub-command-options>] ") { + 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 fa24253320a3f2..d23c1fa1a928be 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<ConstString>(), - 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<ConstString>, - Mangled::NamePreference name_reference, bool)> &callback) { + const std::function<void( + uint32_t, bool, std::string, std::string, llvm::ArrayRef<ConstString>, + 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 09f6bd59a2fb32..796767e2425343 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 24d6a67c0ccd48..17396fb2c5b37f 100644 --- a/lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py +++ b/lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py @@ -14,10 +14,13 @@ 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 @@ def test_frame_recognizer_1(self): "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 @@ def test_frame_recognizer_1(self): substrs=['*a = 78']) """ - @skipUnlessDarwin def test_frame_recognizer_hiding(self): self.build() @@ -204,7 +201,6 @@ def test_frame_recognizer_hiding(self): 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 @@ def test_frame_recognizer_multi_symbol(self): 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,138 @@ def test_frame_recognizer_target_specific(self): 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") + + # 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") + ) + + self.expect("frame recognizer list", substrs=["no matching results found."]) + + # Create a target. + target, process, thread, _ = lldbutil.run_to_name_breakpoint( + self, "foo", exe_name=exe + ) + + # Move the PC one instruction further. + self.runCmd("next") + + # Add a frame recognizer in that target. + self.runCmd( + "frame recognizer add -l recognizer.MyFrameRecognizer -s a.out -n foo -n bar" + ) + + # It's not applied to foo(), because frame's PC is not at the first instruction of the function. + self.expect( + "frame recognizer info 0", + substrs=["frame 0 not recognized by any recognizer"], + ) + + # Add a frame recognizer with --first-instruction-only=true. + self.runCmd("frame recognizer clear") + + self.runCmd( + "frame recognizer add -l recognizer.MyFrameRecognizer -s a.out -n foo -n bar --first-instruction-only=true" + ) + + # It's not applied to foo(), because frame's PC is not at the first instruction of the function. + self.expect( + "frame recognizer info 0", + substrs=["frame 0 not recognized by any recognizer"], + ) + + # Now add a frame recognizer with --first-instruction-only=false. + self.runCmd("frame recognizer clear") + + self.runCmd( + "frame recognizer add -l recognizer.MyFrameRecognizer -s a.out -n foo -n bar --first-instruction-only=false" + ) + + # This time it should recognize the frame. + self.expect( + "frame recognizer info 0", + substrs=["frame 0 is recognized by recognizer.MyFrameRecognizer"], + ) + + opts = lldb.SBVariablesOptions() + opts.SetIncludeRecognizedArguments(True) + frame = thread.GetSelectedFrame() + variables = frame.GetVariables(opts) + + self.assertEqual(variables.GetSize(), 2) + self.assertEqual(variables.GetValueAtIndex(0).name, "a") + self.assertEqual(variables.GetValueAtIndex(0).signed, 42) + self.assertEqual( + variables.GetValueAtIndex(0).GetValueType(), lldb.eValueTypeVariableArgument + ) + self.assertEqual(variables.GetValueAtIndex(1).name, "b") + self.assertEqual(variables.GetValueAtIndex(1).signed, 56) + self.assertEqual( + ... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/109219 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
