Author: Med Ismail Bennani Date: 2020-03-18T14:15:58+01:00 New Revision: db31e2e1e6cacb31f3fe5a9b0e9e52cd626b5ff2
URL: https://github.com/llvm/llvm-project/commit/db31e2e1e6cacb31f3fe5a9b0e9e52cd626b5ff2 DIFF: https://github.com/llvm/llvm-project/commit/db31e2e1e6cacb31f3fe5a9b0e9e52cd626b5ff2.diff LOG: [lldb/Target] Support more than 2 symbols in StackFrameRecognizer This patch changes the way the StackFrame Recognizers match a certain frame. Until now, recognizers could be registered with a function name but also an alternate symbol. This change is motivated by a test failure for the Assert frame recognizer on Linux. Depending the version of the libc, the abort function (triggered by an assertion), could have more than two signatures (i.e. `raise`, `__GI_raise` and `gsignal`). Instead of only checking the default symbol name and the alternate one, lldb will iterate over a list of symbols to match against. rdar://60386577 Differential Revision: https://reviews.llvm.org/D76188 Signed-off-by: Med Ismail Bennani <medismail.benn...@gmail.com> Added: Modified: lldb/include/lldb/Target/StackFrameRecognizer.h lldb/packages/Python/lldbsuite/test/lldbutil.py lldb/source/Commands/CommandObjectFrame.cpp lldb/source/Commands/Options.td lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp lldb/source/Target/AssertFrameRecognizer.cpp lldb/source/Target/StackFrameRecognizer.cpp lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py lldb/test/API/commands/frame/recognizer/main.m lldb/unittests/Target/StackFrameRecognizerTest.cpp Removed: ################################################################################ diff --git a/lldb/include/lldb/Target/StackFrameRecognizer.h b/lldb/include/lldb/Target/StackFrameRecognizer.h index 3e4d6abe7aeb..6be506391deb 100644 --- a/lldb/include/lldb/Target/StackFrameRecognizer.h +++ b/lldb/include/lldb/Target/StackFrameRecognizer.h @@ -101,8 +101,8 @@ class ScriptedStackFrameRecognizer : public StackFrameRecognizer { class StackFrameRecognizerManager { public: static void AddRecognizer(lldb::StackFrameRecognizerSP recognizer, - ConstString module, ConstString symbol, - ConstString alternate_symbol, + ConstString module, + llvm::ArrayRef<ConstString> symbols, bool first_instruction_only = true); static void AddRecognizer(lldb::StackFrameRecognizerSP recognizer, @@ -110,11 +110,11 @@ class StackFrameRecognizerManager { lldb::RegularExpressionSP symbol, bool first_instruction_only = true); - static void ForEach( - std::function<void(uint32_t recognizer_id, std::string recognizer_name, - std::string module, std::string symbol, - std::string alternate_symbol, 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); static bool RemoveRecognizerWithID(uint32_t recognizer_id); diff --git a/lldb/packages/Python/lldbsuite/test/lldbutil.py b/lldb/packages/Python/lldbsuite/test/lldbutil.py index d603091677f3..ef0df90c416e 100644 --- a/lldb/packages/Python/lldbsuite/test/lldbutil.py +++ b/lldb/packages/Python/lldbsuite/test/lldbutil.py @@ -797,7 +797,7 @@ def run_to_breakpoint_do_run(test, target, bkpt, launch_info = None, test.assertEqual(num_threads, 1, "Expected 1 thread to stop at breakpoint, %d did."%(num_threads)) else: test.assertGreater(num_threads, 0, "No threads stopped at breakpoint") - + thread = threads[0] return (target, process, thread, bkpt) diff --git a/lldb/source/Commands/CommandObjectFrame.cpp b/lldb/source/Commands/CommandObjectFrame.cpp index baf64e5c884d..4fef9f4847e0 100644 --- a/lldb/source/Commands/CommandObjectFrame.cpp +++ b/lldb/source/Commands/CommandObjectFrame.cpp @@ -746,7 +746,7 @@ class CommandObjectFrameRecognizerAdd : public CommandObjectParsed { m_module = std::string(option_arg); break; case 'n': - m_function = std::string(option_arg); + m_symbols.push_back(std::string(option_arg)); break; case 'x': m_regex = true; @@ -760,7 +760,7 @@ class CommandObjectFrameRecognizerAdd : public CommandObjectParsed { void OptionParsingStarting(ExecutionContext *execution_context) override { m_module = ""; - m_function = ""; + m_symbols.clear(); m_class_name = ""; m_regex = false; } @@ -772,7 +772,7 @@ class CommandObjectFrameRecognizerAdd : public CommandObjectParsed { // Instance variables to hold the values for command options. std::string m_class_name; std::string m_module; - std::string m_function; + std::vector<std::string> m_symbols; bool m_regex; }; @@ -854,9 +854,18 @@ bool CommandObjectFrameRecognizerAdd::DoExecute(Args &command, return false; } - if (m_options.m_function.empty()) { - result.AppendErrorWithFormat("%s needs a function name (-n argument).\n", - m_cmd_name.c_str()); + if (m_options.m_symbols.empty()) { + result.AppendErrorWithFormat( + "%s needs at least one symbol name (-n argument).\n", + m_cmd_name.c_str()); + result.SetStatus(eReturnStatusFailed); + return false; + } + + if (m_options.m_regex && m_options.m_symbols.size() > 1) { + result.AppendErrorWithFormat( + "%s needs only one symbol regular expression (-n argument).\n", + m_cmd_name.c_str()); result.SetStatus(eReturnStatusFailed); return false; } @@ -876,12 +885,13 @@ bool CommandObjectFrameRecognizerAdd::DoExecute(Args &command, auto module = RegularExpressionSP(new RegularExpression(m_options.m_module)); auto func = - RegularExpressionSP(new RegularExpression(m_options.m_function)); + RegularExpressionSP(new RegularExpression(m_options.m_symbols.front())); StackFrameRecognizerManager::AddRecognizer(recognizer_sp, module, func); } else { auto module = ConstString(m_options.m_module); - auto func = ConstString(m_options.m_function); - StackFrameRecognizerManager::AddRecognizer(recognizer_sp, module, func, {}); + std::vector<ConstString> symbols(m_options.m_symbols.begin(), + m_options.m_symbols.end()); + StackFrameRecognizerManager::AddRecognizer(recognizer_sp, module, symbols); } #endif @@ -958,9 +968,9 @@ class CommandObjectFrameRecognizerList : public CommandObjectParsed { bool DoExecute(Args &command, CommandReturnObject &result) override { bool any_printed = false; StackFrameRecognizerManager::ForEach( - [&result, &any_printed](uint32_t recognizer_id, std::string name, - std::string module, std::string symbol, - std::string alternate_symbol, bool regexp) { + [&result, &any_printed]( + uint32_t recognizer_id, std::string name, std::string module, + llvm::ArrayRef<ConstString> symbols, bool regexp) { Stream &stream = result.GetOutputStream(); if (name.empty()) @@ -969,10 +979,9 @@ class CommandObjectFrameRecognizerList : public CommandObjectParsed { stream << std::to_string(recognizer_id) << ": " << name; if (!module.empty()) stream << ", module " << module; - if (!symbol.empty()) - stream << ", function " << symbol; - if (!alternate_symbol.empty()) - stream << ", symbol " << alternate_symbol; + if (!symbols.empty()) + for (auto &symbol : symbols) + stream << ", symbol " << symbol; if (regexp) stream << " (regexp)"; diff --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td index fa8f5224cf53..b8a142678b28 100644 --- a/lldb/source/Commands/Options.td +++ b/lldb/source/Commands/Options.td @@ -389,7 +389,8 @@ let Command = "frame recognizer add" in { "to.">; def frame_recognizer_function : Option<"function", "n">, Arg<"Name">, Completion<"Symbol">, - Desc<"Name of the function that this recognizer applies to.">; + Desc<"Name of the function that this recognizer applies to. " + "Can be specified more than once except if -x|--regex is provided.">; def frame_recognizer_python_class : Option<"python-class", "l">, Group<2>, Arg<"PythonClass">, Desc<"Give the name of a Python class to use for this frame recognizer.">; diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp index cb1d808d0f1e..9b3dbb166b68 100644 --- a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp +++ b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp @@ -2685,9 +2685,10 @@ static void RegisterObjCExceptionRecognizer() { FileSpec module; ConstString function; std::tie(module, function) = AppleObjCRuntime::GetExceptionThrowLocation(); + std::vector<ConstString> symbols = {function}; StackFrameRecognizerManager::AddRecognizer( StackFrameRecognizerSP(new ObjCExceptionThrowFrameRecognizer()), - module.GetFilename(), function, /*alternate_symbol*/ {}, + module.GetFilename(), symbols, /*first_instruction_only*/ true); }); } diff --git a/lldb/source/Target/AssertFrameRecognizer.cpp b/lldb/source/Target/AssertFrameRecognizer.cpp index ebeff6c39391..d87459ac2fdd 100644 --- a/lldb/source/Target/AssertFrameRecognizer.cpp +++ b/lldb/source/Target/AssertFrameRecognizer.cpp @@ -21,8 +21,7 @@ namespace lldb_private { /// name. struct SymbolLocation { FileSpec module_spec; - ConstString symbol_name; - ConstString alternate_symbol_name; + std::vector<ConstString> symbols; }; /// Fetches the abort frame location depending on the current platform. @@ -39,12 +38,13 @@ bool GetAbortLocation(llvm::Triple::OSType os, SymbolLocation &location) { case llvm::Triple::Darwin: case llvm::Triple::MacOSX: location.module_spec = FileSpec("libsystem_kernel.dylib"); - location.symbol_name.SetString("__pthread_kill"); + location.symbols.push_back(ConstString("__pthread_kill")); break; case llvm::Triple::Linux: location.module_spec = FileSpec("libc.so.6"); - location.symbol_name.SetString("raise"); - location.alternate_symbol_name.SetString("__GI_raise"); + location.symbols.push_back(ConstString("raise")); + location.symbols.push_back(ConstString("__GI_raise")); + location.symbols.push_back(ConstString("gsignal")); break; default: Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_UNWIND)); @@ -69,12 +69,12 @@ bool GetAssertLocation(llvm::Triple::OSType os, SymbolLocation &location) { case llvm::Triple::Darwin: case llvm::Triple::MacOSX: location.module_spec = FileSpec("libsystem_c.dylib"); - location.symbol_name.SetString("__assert_rtn"); + location.symbols.push_back(ConstString("__assert_rtn")); break; case llvm::Triple::Linux: location.module_spec = FileSpec("libc.so.6"); - location.symbol_name.SetString("__assert_fail"); - location.alternate_symbol_name.SetString("__GI___assert_fail"); + location.symbols.push_back(ConstString("__assert_fail")); + location.symbols.push_back(ConstString("__GI___assert_fail")); break; default: Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_UNWIND)); @@ -97,8 +97,7 @@ void RegisterAssertFrameRecognizer(Process *process) { StackFrameRecognizerManager::AddRecognizer( StackFrameRecognizerSP(new AssertFrameRecognizer()), - location.module_spec.GetFilename(), ConstString(location.symbol_name), - ConstString(location.alternate_symbol_name), + location.module_spec.GetFilename(), location.symbols, /*first_instruction_only*/ false); }); } @@ -139,10 +138,7 @@ AssertFrameRecognizer::RecognizeFrame(lldb::StackFrameSP frame_sp) { ConstString func_name = sym_ctx.GetFunctionName(); - if (func_name == location.symbol_name || - (!location.alternate_symbol_name.IsEmpty() && - func_name == location.alternate_symbol_name)) { - + if (llvm::is_contained(location.symbols, func_name)) { // We go a frame beyond the assert location because the most relevant // frame for the user is the one in which the assert function was called. // If the assert location is the last frame fetched, then it is set as diff --git a/lldb/source/Target/StackFrameRecognizer.cpp b/lldb/source/Target/StackFrameRecognizer.cpp index e48401974627..7dc6e9d1e490 100644 --- a/lldb/source/Target/StackFrameRecognizer.cpp +++ b/lldb/source/Target/StackFrameRecognizer.cpp @@ -51,27 +51,25 @@ ScriptedStackFrameRecognizer::RecognizeFrame(lldb::StackFrameSP frame) { class StackFrameRecognizerManagerImpl { public: void AddRecognizer(StackFrameRecognizerSP recognizer, ConstString module, - ConstString symbol, ConstString alternate_symbol, + llvm::ArrayRef<ConstString> symbols, bool first_instruction_only) { m_recognizers.push_front({(uint32_t)m_recognizers.size(), false, recognizer, - false, module, RegularExpressionSP(), symbol, - alternate_symbol, RegularExpressionSP(), - first_instruction_only}); + false, module, RegularExpressionSP(), symbols, + RegularExpressionSP(), first_instruction_only}); } void 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, ConstString(), - ConstString(), symbol, first_instruction_only}); + m_recognizers.push_front( + {(uint32_t)m_recognizers.size(), false, recognizer, true, ConstString(), + module, std::vector<ConstString>(), symbol, first_instruction_only}); } - void ForEach( - std::function<void(uint32_t recognized_id, std::string recognizer_name, - std::string module, std::string symbol, - std::string alternate_symbol, bool regexp)> const - &callback) { + void ForEach(std::function< + void(uint32_t recognized_id, std::string recognizer_name, + std::string module, llvm::ArrayRef<ConstString> symbols, + bool regexp)> const &callback) { for (auto entry : m_recognizers) { if (entry.is_regexp) { std::string module_name; @@ -83,16 +81,11 @@ class StackFrameRecognizerManagerImpl { symbol_name = entry.symbol_regexp->GetText().str(); callback(entry.recognizer_id, entry.recognizer->GetName(), module_name, - symbol_name, {}, true); + llvm::makeArrayRef(ConstString(symbol_name)), true); } else { - std::string alternate_symbol; - if (!entry.alternate_symbol.IsEmpty()) - alternate_symbol.append(entry.alternate_symbol.GetCString()); - callback(entry.recognizer_id, entry.recognizer->GetName(), - entry.module.GetCString(), entry.symbol.GetCString(), - alternate_symbol, false); + entry.module.GetCString(), entry.symbols, false); } } } @@ -128,10 +121,8 @@ class StackFrameRecognizerManagerImpl { if (entry.module_regexp) if (!entry.module_regexp->Execute(module_name.GetStringRef())) continue; - if (entry.symbol) - if (entry.symbol != function_name && - (!entry.alternate_symbol || - entry.alternate_symbol != function_name)) + if (!entry.symbols.empty()) + if (!llvm::is_contained(entry.symbols, function_name)) continue; if (entry.symbol_regexp) @@ -160,8 +151,7 @@ class StackFrameRecognizerManagerImpl { bool is_regexp; ConstString module; RegularExpressionSP module_regexp; - ConstString symbol; - ConstString alternate_symbol; + std::vector<ConstString> symbols; RegularExpressionSP symbol_regexp; bool first_instruction_only; }; @@ -176,10 +166,10 @@ StackFrameRecognizerManagerImpl &GetStackFrameRecognizerManagerImpl() { } void StackFrameRecognizerManager::AddRecognizer( - StackFrameRecognizerSP recognizer, ConstString module, ConstString symbol, - ConstString alternate_symbol, bool first_instruction_only) { + StackFrameRecognizerSP recognizer, ConstString module, + llvm::ArrayRef<ConstString> symbols, bool first_instruction_only) { GetStackFrameRecognizerManagerImpl().AddRecognizer( - recognizer, module, symbol, alternate_symbol, first_instruction_only); + recognizer, module, symbols, first_instruction_only); } void StackFrameRecognizerManager::AddRecognizer( @@ -191,9 +181,8 @@ void StackFrameRecognizerManager::AddRecognizer( void StackFrameRecognizerManager::ForEach( std::function<void(uint32_t recognized_id, std::string recognizer_name, - std::string module, std::string symbol, - std::string alternate_symbol, bool regexp)> const - &callback) { + std::string module, llvm::ArrayRef<ConstString> symbols, + bool regexp)> const &callback) { GetStackFrameRecognizerManagerImpl().ForEach(callback); } diff --git a/lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py b/lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py index 683b2e8f7b48..04b940049496 100644 --- a/lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py +++ b/lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py @@ -18,9 +18,7 @@ class FrameRecognizerTestCase(TestBase): @skipUnlessDarwin def test_frame_recognizer_1(self): self.build() - - target = self.dbg.CreateTarget(self.getBuildArtifact("a.out")) - self.assertTrue(target, VALID_TARGET) + exe = self.getBuildArtifact("a.out") # Clear internal & plugins recognizers that get initialized at launch self.runCmd("frame recognizer clear") @@ -33,21 +31,21 @@ def test_frame_recognizer_1(self): self.runCmd("frame recognizer add -l recognizer.MyFrameRecognizer -s a.out -n foo") self.expect("frame recognizer list", - substrs=['0: recognizer.MyFrameRecognizer, module a.out, function foo']) + substrs=['0: recognizer.MyFrameRecognizer, module a.out, symbol foo']) self.runCmd("frame recognizer add -l recognizer.MyOtherFrameRecognizer -s a.out -n bar -x") self.expect( "frame recognizer list", substrs=[ - '1: recognizer.MyOtherFrameRecognizer, module a.out, function bar (regexp)', - '0: recognizer.MyFrameRecognizer, module a.out, function foo' + '1: recognizer.MyOtherFrameRecognizer, module a.out, symbol bar (regexp)', + '0: recognizer.MyFrameRecognizer, module a.out, symbol foo' ]) self.runCmd("frame recognizer delete 0") self.expect("frame recognizer list", - substrs=['1: recognizer.MyOtherFrameRecognizer, module a.out, function bar (regexp)']) + substrs=['1: recognizer.MyOtherFrameRecognizer, module a.out, symbol bar (regexp)']) self.runCmd("frame recognizer clear") @@ -56,19 +54,10 @@ def test_frame_recognizer_1(self): self.runCmd("frame recognizer add -l recognizer.MyFrameRecognizer -s a.out -n foo") - lldbutil.run_break_set_by_symbol(self, "foo") - self.runCmd("r") - - self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT, - substrs=['stopped', 'stop reason = breakpoint']) - - process = target.GetProcess() - thread = process.GetSelectedThread() + target, process, thread, _ = lldbutil.run_to_name_breakpoint(self, "foo", + exe_name = exe) frame = thread.GetSelectedFrame() - self.assertEqual(frame.GetSymbol().GetName(), "foo") - self.assertFalse(frame.GetLineEntry().IsValid()) - self.expect("frame variable", substrs=['(int) a = 42', '(int) b = 56']) @@ -110,8 +99,9 @@ def test_frame_recognizer_1(self): # FIXME: The following doesn't work yet, but should be fixed. """ - lldbutil.run_break_set_by_symbol(self, "bar") - self.runCmd("c") + target, process, thread, _ = lldbutil.run_to_name_breakpoint(self, "bar", + exe_name = exe) + frame = thread.GetSelectedFrame() self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT, substrs=['stopped', 'stop reason = breakpoint']) @@ -122,3 +112,35 @@ def test_frame_recognizer_1(self): self.expect("frame variable -t *a", substrs=['*a = 78']) """ + + @skipUnlessDarwin + def test_frame_recognizer_multi_symbol(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.']) + + self.runCmd("frame recognizer add -l recognizer.MyFrameRecognizer -s a.out -n foo -n bar") + + self.expect("frame recognizer list", + substrs=['recognizer.MyFrameRecognizer, module a.out, symbol foo, symbol bar']) + + target, process, thread, _ = lldbutil.run_to_name_breakpoint(self, "foo", + exe_name = exe) + frame = thread.GetSelectedFrame() + + self.expect("frame recognizer info 0", + substrs=['frame 0 is recognized by recognizer.MyFrameRecognizer']) + + target, process, thread, _ = lldbutil.run_to_name_breakpoint(self, "bar", + exe_name = exe) + frame = thread.GetSelectedFrame() + + self.expect("frame recognizer info 0", + substrs=['frame 0 is recognized by recognizer.MyFrameRecognizer']) diff --git a/lldb/test/API/commands/frame/recognizer/main.m b/lldb/test/API/commands/frame/recognizer/main.m index d903625369ec..5d064c2564dd 100644 --- a/lldb/test/API/commands/frame/recognizer/main.m +++ b/lldb/test/API/commands/frame/recognizer/main.m @@ -5,10 +5,7 @@ void foo(int a, int b) printf("%d %d\n", a, b); } -void bar(int *ptr) -{ - printf("%d\n", *ptr); -} +void bar(int *ptr) { printf("%d\n", *ptr); } int main (int argc, const char * argv[]) { diff --git a/lldb/unittests/Target/StackFrameRecognizerTest.cpp b/lldb/unittests/Target/StackFrameRecognizerTest.cpp index f7b7829e2bb8..067a56a19902 100644 --- a/lldb/unittests/Target/StackFrameRecognizerTest.cpp +++ b/lldb/unittests/Target/StackFrameRecognizerTest.cpp @@ -76,8 +76,7 @@ TEST_F(StackFrameRecognizerTest, NullModuleRegex) { bool any_printed = false; StackFrameRecognizerManager::ForEach( [&any_printed](uint32_t recognizer_id, std::string name, - std::string function, std::string symbol, - std::string alternate_symbol, + std::string function, llvm::ArrayRef<ConstString> symbols, bool regexp) { any_printed = true; }); EXPECT_TRUE(any_printed); _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
