Author: Jim Ingham Date: 2022-05-12T12:39:28-07:00 New Revision: 3339000e0bda696c2e29173d15958c0a4978a143
URL: https://github.com/llvm/llvm-project/commit/3339000e0bda696c2e29173d15958c0a4978a143 DIFF: https://github.com/llvm/llvm-project/commit/3339000e0bda696c2e29173d15958c0a4978a143.diff LOG: We don't require users to type out the full context of a function, for symbol name matches. Instead, we extract the incoming path's base name, look up all the symbols with that base name, and then compare the rest of the context that the user provided to make sure it matches. However, we do this comparison using just a strstr. So for instance: break set -n foo::bar will match not only "a::foo::bar" but "notherfoo::bar". The former is pretty clearly the user's intent, but I don't think the latter is, and results in breakpoints picking up too many matches. This change adds a Language::DemangledNameContainsPath API which can do a language aware match against the path provided. If the language doesn't provide this we fall back to the strstr (though that's changed to StringRef::contains in the patch). Differential Revision: https://reviews.llvm.org/D124579 Added: Modified: lldb/include/lldb/Target/Language.h lldb/source/Core/Module.cpp lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h lldb/source/Target/Language.cpp lldb/test/API/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py lldb/test/API/functionalities/breakpoint/cpp/main.cpp lldb/test/API/functionalities/return-value/TestReturnValue.py lldb/test/API/macosx/indirect_symbol/TestIndirectSymbols.py lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp Removed: ################################################################################ diff --git a/lldb/include/lldb/Target/Language.h b/lldb/include/lldb/Target/Language.h index ce2d273a82770..fa79aaee05748 100644 --- a/lldb/include/lldb/Target/Language.h +++ b/lldb/include/lldb/Target/Language.h @@ -217,6 +217,17 @@ class Language : public PluginInterface { std::string &prefix, std::string &suffix); + // When looking up functions, we take a user provided string which may be a + // partial match to the full demangled name and compare it to the actual + // demangled name to see if it matches as much as the user specified. An + // example of this is if the user provided A::my_function, but the + // symbol was really B::A::my_function. We want that to be + // a match. But we wouldn't want this to match AnotherA::my_function. The + // user is specifying a truncated path, not a truncated set of characters. + // This function does a language-aware comparison for those purposes. + virtual bool DemangledNameContainsPath(llvm::StringRef path, + ConstString demangled) const; + // if a language has a custom format for printing variable declarations that // it wants LLDB to honor it should return an appropriate closure here virtual DumpValueObjectOptions::DeclPrintingHelper GetDeclPrintingHelper(); diff --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp index e89d5ec401788..7160c2386efd6 100644 --- a/lldb/source/Core/Module.cpp +++ b/lldb/source/Core/Module.cpp @@ -739,13 +739,25 @@ void Module::LookupInfo::Prune(SymbolContextList &sc_list, while (i < sc_list.GetSize()) { if (!sc_list.GetContextAtIndex(i, sc)) break; - ConstString full_name(sc.GetFunctionName()); - if (full_name && - ::strstr(full_name.GetCString(), m_name.GetCString()) == nullptr) { - sc_list.RemoveContextAtIndex(i); - } else { - ++i; + + llvm::StringRef user_name = m_name.GetStringRef(); + bool keep_it = true; + Language *language = Language::FindPlugin(sc.GetLanguage()); + // If the symbol has a language, then let the language make the match. + // Otherwise just check that the demangled name contains the user name. + if (language) + keep_it = language->DemangledNameContainsPath(m_name.GetStringRef(), + sc.GetFunctionName()); + else { + llvm::StringRef full_name = sc.GetFunctionName().GetStringRef(); + // We always keep unnamed symbols: + if (!full_name.empty()) + keep_it = full_name.contains(user_name); } + if (keep_it) + ++i; + else + sc_list.RemoveContextAtIndex(i); } } diff --git a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp index 56daf75ee0faf..08b6d89e55f7a 100644 --- a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp +++ b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp @@ -268,6 +268,41 @@ std::string CPlusPlusLanguage::MethodName::GetScopeQualifiedName() { return res; } +bool CPlusPlusLanguage::MethodName::ContainsPath(llvm::StringRef path) { + if (!m_parsed) + Parse(); + // If we can't parse the incoming name, then just check that it contains path. + if (m_parse_error) + return m_full.GetStringRef().contains(path); + + llvm::StringRef identifier; + llvm::StringRef context; + std::string path_str = path.str(); + bool success + = CPlusPlusLanguage::ExtractContextAndIdentifier(path_str.c_str(), + context, + identifier); + if (!success) + return m_full.GetStringRef().contains(path); + + if (identifier != GetBasename()) + return false; + // Incoming path only had an identifier, so we match. + if (context.empty()) + return true; + // Incoming path has context but this method does not, no match. + if (m_context.empty()) + return false; + + llvm::StringRef haystack = m_context; + if (!haystack.consume_back(context)) + return false; + if (haystack.empty() || !isalnum(haystack.back())) + return true; + + return false; +} + bool CPlusPlusLanguage::IsCPPMangledName(llvm::StringRef name) { // FIXME!! we should really run through all the known C++ Language plugins // and ask each one if this is a C++ mangled name @@ -280,6 +315,12 @@ bool CPlusPlusLanguage::IsCPPMangledName(llvm::StringRef name) { return true; } +bool CPlusPlusLanguage::DemangledNameContainsPath(llvm::StringRef path, + ConstString demangled) const { + MethodName demangled_name(demangled); + return demangled_name.ContainsPath(path); +} + bool CPlusPlusLanguage::ExtractContextAndIdentifier( const char *name, llvm::StringRef &context, llvm::StringRef &identifier) { if (MSVCUndecoratedNameParser::IsMSVCUndecoratedName(name)) diff --git a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h index 5547864a37639..53a01cfc4799d 100644 --- a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h +++ b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h @@ -55,6 +55,8 @@ class CPlusPlusLanguage : public Language { llvm::StringRef GetArguments(); llvm::StringRef GetQualifiers(); + + bool ContainsPath(llvm::StringRef path); protected: void Parse(); @@ -105,6 +107,9 @@ class CPlusPlusLanguage : public Language { static llvm::StringRef GetPluginNameStatic() { return "cplusplus"; } bool SymbolNameFitsToLanguage(Mangled mangled) const override; + + bool DemangledNameContainsPath(llvm::StringRef path, + ConstString demangled) const override; ConstString GetDemangledFunctionNameWithoutArguments(Mangled mangled) const override; diff --git a/lldb/source/Target/Language.cpp b/lldb/source/Target/Language.cpp index eee1ff1512d95..6df36aeeb7b72 100644 --- a/lldb/source/Target/Language.cpp +++ b/lldb/source/Target/Language.cpp @@ -428,6 +428,14 @@ bool Language::GetFormatterPrefixSuffix(ValueObject &valobj, return false; } +bool Language::DemangledNameContainsPath(llvm::StringRef path, + ConstString demangled) const { + // The base implementation does a simple contains comparision: + if (path.empty()) + return false; + return demangled.GetStringRef().contains(path); +} + DumpValueObjectOptions::DeclPrintingHelper Language::GetDeclPrintingHelper() { return nullptr; } diff --git a/lldb/test/API/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py b/lldb/test/API/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py index be21c6eea138b..8edd852cd8ece 100644 --- a/lldb/test/API/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py +++ b/lldb/test/API/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py @@ -28,7 +28,7 @@ def verify_breakpoint_locations(self, target, bp_dict): self.assertEquals( bp.GetNumLocations(), len(names), - "Make sure we find the right number of breakpoint locations") + "Make sure we find the right number of breakpoint locations for {}".format(name)) bp_loc_names = list() for bp_loc in bp: @@ -47,9 +47,9 @@ def breakpoint_id_tests(self): target = self.dbg.CreateTarget(exe) self.assertTrue(target, VALID_TARGET) bp_dicts = [ - {'name': 'func1', 'loc_names': ['a::c::func1()', 'b::c::func1()']}, - {'name': 'func2', 'loc_names': ['a::c::func2()', 'c::d::func2()']}, - {'name': 'func3', 'loc_names': ['a::c::func3()', 'b::c::func3()', 'c::d::func3()']}, + {'name': 'func1', 'loc_names': ['a::c::func1()', 'aa::cc::func1()', 'b::c::func1()']}, + {'name': 'func2', 'loc_names': ['a::c::func2()', 'aa::cc::func2()', 'c::d::func2()']}, + {'name': 'func3', 'loc_names': ['a::c::func3()', 'aa::cc::func3()', 'b::c::func3()', 'c::d::func3()']}, {'name': 'c::func1', 'loc_names': ['a::c::func1()', 'b::c::func1()']}, {'name': 'c::func2', 'loc_names': ['a::c::func2()']}, {'name': 'c::func3', 'loc_names': ['a::c::func3()', 'b::c::func3()']}, diff --git a/lldb/test/API/functionalities/breakpoint/cpp/main.cpp b/lldb/test/API/functionalities/breakpoint/cpp/main.cpp index f352084e74447..088e33c6a7c73 100644 --- a/lldb/test/API/functionalities/breakpoint/cpp/main.cpp +++ b/lldb/test/API/functionalities/breakpoint/cpp/main.cpp @@ -24,6 +24,29 @@ namespace a { c::~c() {} } +namespace aa { + class cc { + public: + cc(); + ~cc(); + void func1() + { + puts (__PRETTY_FUNCTION__); + } + void func2() + { + puts (__PRETTY_FUNCTION__); + } + void func3() + { + puts (__PRETTY_FUNCTION__); + } + }; + + cc::cc() {} + cc::~cc() {} +} + namespace b { class c { public: @@ -62,11 +85,15 @@ namespace c { int main (int argc, char const *argv[]) { a::c ac; + aa::cc aac; b::c bc; c::d cd; ac.func1(); ac.func2(); ac.func3(); + aac.func1(); + aac.func2(); + aac.func3(); bc.func1(); bc.func3(); cd.func2(); diff --git a/lldb/test/API/functionalities/return-value/TestReturnValue.py b/lldb/test/API/functionalities/return-value/TestReturnValue.py index f2f50385b8bd7..517ed46a4fe9f 100644 --- a/lldb/test/API/functionalities/return-value/TestReturnValue.py +++ b/lldb/test/API/functionalities/return-value/TestReturnValue.py @@ -237,7 +237,7 @@ def return_and_test_struct_value(self, func_name): # Set the breakpoint, run to it, finish out. bkpt = self.target.BreakpointCreateByName(func_name) - self.assertTrue(bkpt.GetNumResolvedLocations() > 0) + self.assertTrue(bkpt.GetNumResolvedLocations() > 0, "Got wrong number of locations for {0}".format(func_name)) self.process.Continue() diff --git a/lldb/test/API/macosx/indirect_symbol/TestIndirectSymbols.py b/lldb/test/API/macosx/indirect_symbol/TestIndirectSymbols.py index 5c76d7aabf1bc..e2cebf0209104 100644 --- a/lldb/test/API/macosx/indirect_symbol/TestIndirectSymbols.py +++ b/lldb/test/API/macosx/indirect_symbol/TestIndirectSymbols.py @@ -98,7 +98,7 @@ def test_with_python_api(self): # make sure we are again in out target function. break_reexported = target.BreakpointCreateByName( "reexport_to_indirect") - self.assertTrue(break_reexported, VALID_BREAKPOINT) + self.assertEqual(break_reexported.GetNumLocations(), 1, VALID_BREAKPOINT) # Now continue should take us to the second call through the indirect # symbol: diff --git a/lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp b/lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp index 20abd5a87e1b4..bf887fb0777c5 100644 --- a/lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp +++ b/lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp @@ -123,6 +123,37 @@ TEST(CPlusPlusLanguage, MethodNameParsing) { } } +TEST(CPlusPlusLanguage, ContainsPath) { + CPlusPlusLanguage::MethodName + reference_1(ConstString("int foo::bar::func01(int a, double b)")); + CPlusPlusLanguage::MethodName + reference_2(ConstString("int foofoo::bar::func01(std::string a, int b)")); + CPlusPlusLanguage::MethodName reference_3(ConstString("int func01()")); + CPlusPlusLanguage::MethodName + reference_4(ConstString("bar::baz::operator bool()")); + + EXPECT_TRUE(reference_1.ContainsPath("func01")); + EXPECT_TRUE(reference_1.ContainsPath("bar::func01")); + EXPECT_TRUE(reference_1.ContainsPath("foo::bar::func01")); + EXPECT_FALSE(reference_1.ContainsPath("func")); + EXPECT_FALSE(reference_1.ContainsPath("baz::func01")); + EXPECT_FALSE(reference_1.ContainsPath("::bar::func01")); + EXPECT_FALSE(reference_1.ContainsPath("::foo::baz::func01")); + EXPECT_FALSE(reference_1.ContainsPath("foo::bar::baz::func01")); + + EXPECT_TRUE(reference_2.ContainsPath("foofoo::bar::func01")); + EXPECT_FALSE(reference_2.ContainsPath("foo::bar::func01")); + + EXPECT_TRUE(reference_3.ContainsPath("func01")); + EXPECT_FALSE(reference_3.ContainsPath("func")); + EXPECT_FALSE(reference_3.ContainsPath("bar::func01")); + + EXPECT_TRUE(reference_4.ContainsPath("operator bool")); + EXPECT_TRUE(reference_4.ContainsPath("baz::operator bool")); + EXPECT_TRUE(reference_4.ContainsPath("bar::baz::operator bool")); + EXPECT_FALSE(reference_4.ContainsPath("az::operator bool")); +} + TEST(CPlusPlusLanguage, ExtractContextAndIdentifier) { struct TestCase { std::string input; _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
