Author: Michael Buch Date: 2022-10-14T23:51:00+01:00 New Revision: d4a55ad346514b2478762cbc198942c72347e81e
URL: https://github.com/llvm/llvm-project/commit/d4a55ad346514b2478762cbc198942c72347e81e DIFF: https://github.com/llvm/llvm-project/commit/d4a55ad346514b2478762cbc198942c72347e81e.diff LOG: [lldb][Breakpoint] Fix setting breakpoints on templates by basename This patch fixes a regression with setting breakpoints on template functions by name. E.g.,: ``` $ cat main.cpp template<typename T> struct Foo { template<typename U> void func() {} }; int main() { Foo<int> f; f.func<double>(); } (lldb) br se -n func ``` This has regressed since `3339000e0bda696c2e29173d15958c0a4978a143` where we started using the `CPlusPlusNameParser` for getting the basename of the function symbol and match it exactly against the name in the breakpoint command. The parser will include template parameters in the basename, so the exact match will always fail **Testing** * Added API tests * Added unit-tests Differential Revision: https://reviews.llvm.org/D135921 Added: Modified: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h lldb/test/API/functionalities/breakpoint/cpp/Makefile lldb/test/API/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py lldb/test/API/functionalities/breakpoint/cpp/main.cpp lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp Removed: ################################################################################ diff --git a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp index 0339d38904759..d4dfc4c0e1d2e 100644 --- a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp +++ b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp @@ -269,9 +269,21 @@ std::string CPlusPlusLanguage::MethodName::GetScopeQualifiedName() { return res; } +llvm::StringRef +CPlusPlusLanguage::MethodName::GetBasenameNoTemplateParameters() { + llvm::StringRef basename = GetBasename(); + size_t arg_start, arg_end; + llvm::StringRef parens("<>", 2); + if (ReverseFindMatchingChars(basename, parens, arg_start, arg_end)) + return basename.substr(0, arg_start); + + return basename; +} + 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); @@ -286,8 +298,23 @@ bool CPlusPlusLanguage::MethodName::ContainsPath(llvm::StringRef path) { if (!success) return m_full.GetStringRef().contains(path); - if (identifier != GetBasename()) + // Basename may include template arguments. + // E.g., + // GetBaseName(): func<int> + // identifier : func + // + // ...but we still want to account for identifiers with template parameter + // lists, e.g., when users set breakpoints on template specializations. + // + // E.g., + // GetBaseName(): func<uint32_t> + // identifier : func<int32_t*> + // + // Try to match the basename with or without template parameters. + if (GetBasename() != identifier && + GetBasenameNoTemplateParameters() != identifier) return false; + // Incoming path only had an identifier, so we match. if (context.empty()) return true; diff --git a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h index 53a01cfc4799d..4d4226accd02b 100644 --- a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h +++ b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h @@ -58,6 +58,21 @@ class CPlusPlusLanguage : public Language { bool ContainsPath(llvm::StringRef path); + private: + /// Returns the Basename of this method without a template parameter + /// list, if any. + /// + // Examples: + // + // +--------------------------------+---------+ + // | MethodName | Returns | + // +--------------------------------+---------+ + // | void func() | func | + // | void func<int>() | func | + // | void func<std::vector<int>>() | func | + // +--------------------------------+---------+ + llvm::StringRef GetBasenameNoTemplateParameters(); + protected: void Parse(); bool TrySimplifiedParse(); diff --git a/lldb/test/API/functionalities/breakpoint/cpp/Makefile b/lldb/test/API/functionalities/breakpoint/cpp/Makefile index 2c00681fa2280..66108b79e7fe0 100644 --- a/lldb/test/API/functionalities/breakpoint/cpp/Makefile +++ b/lldb/test/API/functionalities/breakpoint/cpp/Makefile @@ -1,4 +1,5 @@ CXX_SOURCES := main.cpp +CXXFLAGS_EXTRAS := -std=c++14 ifneq (,$(findstring icc,$(CC))) CXXFLAGS_EXTRAS := -debug inline-debug-info diff --git a/lldb/test/API/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py b/lldb/test/API/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py index 8296ae62877c6..6c86f5016a606 100644 --- a/lldb/test/API/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py +++ b/lldb/test/API/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py @@ -54,6 +54,23 @@ def breakpoint_id_tests(self): {'name': 'a::c::func1()', 'loc_names': ['a::c::func1()']}, {'name': 'b::c::func1()', 'loc_names': ['b::c::func1()']}, {'name': 'c::d::func2()', 'loc_names': ['c::d::func2()']}, + + # Template cases + {'name': 'func<float>', 'loc_names': []}, + {'name': 'func<int>', 'loc_names': ['auto ns::Foo<double>::func<int>()']}, + {'name': 'func', 'loc_names': ['auto ns::Foo<double>::func<int>()', + 'auto ns::Foo<double>::func<ns::Foo<int>>()']}, + + {'name': 'operator', 'loc_names': []}, + {'name': 'ns::Foo<double>::operator bool', 'loc_names': ['ns::Foo<double>::operator bool()']}, + + {'name': 'operator a::c', 'loc_names': ['ns::Foo<double>::operator a::c<a::c>()']}, + {'name': 'operator ns::Foo<int>', 'loc_names': ['ns::Foo<double>::operator ns::Foo<int><ns::Foo<int>>()']}, + + {'name': 'operator<<<a::c>', 'loc_names': []}, + {'name': 'operator<<<int>', 'loc_names': ['void ns::Foo<double>::operator<<<int>(int)']}, + {'name': 'ns::Foo<double>::operator<<', 'loc_names': ['void ns::Foo<double>::operator<<<int>(int)', + 'void ns::Foo<double>::operator<<<ns::Foo<int>>(ns::Foo<int>)']}, ] for bp_dict in bp_dicts: diff --git a/lldb/test/API/functionalities/breakpoint/cpp/main.cpp b/lldb/test/API/functionalities/breakpoint/cpp/main.cpp index 088e33c6a7c73..7ee61e92ffd57 100644 --- a/lldb/test/API/functionalities/breakpoint/cpp/main.cpp +++ b/lldb/test/API/functionalities/breakpoint/cpp/main.cpp @@ -82,6 +82,20 @@ namespace c { }; } +namespace ns { +template <typename Type> struct Foo { + template <typename T> void import() {} + + template <typename T> auto func() {} + + operator bool() { return true; } + + template <typename T> operator T() { return {}; } + + template <typename T> void operator<<(T t) {} +}; +} // namespace ns + int main (int argc, char const *argv[]) { a::c ac; @@ -98,5 +112,16 @@ int main (int argc, char const *argv[]) bc.func3(); cd.func2(); cd.func3(); + + ns::Foo<double> f; + f.import <int>(); + f.func<int>(); + f.func<ns::Foo<int>>(); + f.operator bool(); + f.operator a::c(); + f.operator ns::Foo<int>(); + f.operator<<(5); + f.operator<< <ns::Foo<int>>({}); + return 0; } diff --git a/lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp b/lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp index 9a7e9a792f5b9..85cf8081e4270 100644 --- a/lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp +++ b/lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp @@ -143,7 +143,12 @@ TEST(CPlusPlusLanguage, ContainsPath) { CPlusPlusLanguage::MethodName reference_3(ConstString("int func01()")); CPlusPlusLanguage::MethodName reference_4(ConstString("bar::baz::operator bool()")); - + CPlusPlusLanguage::MethodName reference_5( + ConstString("bar::baz::operator bool<int, Type<double>>()")); + CPlusPlusLanguage::MethodName reference_6(ConstString( + "bar::baz::operator<<<Type<double>, Type<std::vector<double>>>()")); + + EXPECT_TRUE(reference_1.ContainsPath("")); EXPECT_TRUE(reference_1.ContainsPath("func01")); EXPECT_TRUE(reference_1.ContainsPath("bar::func01")); EXPECT_TRUE(reference_1.ContainsPath("foo::bar::func01")); @@ -153,17 +158,35 @@ TEST(CPlusPlusLanguage, ContainsPath) { EXPECT_FALSE(reference_1.ContainsPath("::foo::baz::func01")); EXPECT_FALSE(reference_1.ContainsPath("foo::bar::baz::func01")); + EXPECT_TRUE(reference_2.ContainsPath("")); EXPECT_TRUE(reference_2.ContainsPath("foofoo::bar::func01")); EXPECT_FALSE(reference_2.ContainsPath("foo::bar::func01")); + EXPECT_TRUE(reference_3.ContainsPath("")); EXPECT_TRUE(reference_3.ContainsPath("func01")); EXPECT_FALSE(reference_3.ContainsPath("func")); EXPECT_FALSE(reference_3.ContainsPath("bar::func01")); + EXPECT_TRUE(reference_4.ContainsPath("")); + EXPECT_TRUE(reference_4.ContainsPath("operator")); 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")); + + EXPECT_TRUE(reference_5.ContainsPath("")); + EXPECT_TRUE(reference_5.ContainsPath("operator")); + EXPECT_TRUE(reference_5.ContainsPath("operator bool")); + EXPECT_TRUE(reference_5.ContainsPath("operator bool<int, Type<double>>")); + EXPECT_FALSE(reference_5.ContainsPath("operator bool<int, double>")); + EXPECT_FALSE(reference_5.ContainsPath("operator bool<int, Type<int>>")); + + EXPECT_TRUE(reference_6.ContainsPath("")); + EXPECT_TRUE(reference_6.ContainsPath("operator")); + EXPECT_TRUE(reference_6.ContainsPath("operator<<")); + EXPECT_TRUE(reference_6.ContainsPath( + "bar::baz::operator<<<Type<double>, Type<std::vector<double>>>()")); + EXPECT_FALSE(reference_6.ContainsPath("operator<<<Type<double>>")); } TEST(CPlusPlusLanguage, ExtractContextAndIdentifier) { _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits