Michael137 created this revision.
Michael137 added reviewers: aprantl, labath, jingham.
Herald added a project: All.
Michael137 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
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
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D135921
Files:
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
Index: lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
===================================================================
--- lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
+++ lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
@@ -143,7 +143,11 @@
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("func01"));
EXPECT_TRUE(reference_1.ContainsPath("bar::func01"));
EXPECT_TRUE(reference_1.ContainsPath("foo::bar::func01"));
@@ -160,10 +164,23 @@
EXPECT_FALSE(reference_3.ContainsPath("func"));
EXPECT_FALSE(reference_3.ContainsPath("bar::func01"));
+ 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("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("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) {
Index: lldb/test/API/functionalities/breakpoint/cpp/main.cpp
===================================================================
--- lldb/test/API/functionalities/breakpoint/cpp/main.cpp
+++ lldb/test/API/functionalities/breakpoint/cpp/main.cpp
@@ -82,6 +82,20 @@
};
}
+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 @@
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;
}
Index: lldb/test/API/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py
===================================================================
--- lldb/test/API/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py
+++ lldb/test/API/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py
@@ -54,6 +54,28 @@
{'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': 'func<ns::Foo<int>>', 'loc_names': ['auto ns::Foo<double>::func<ns::Foo<int>>()']}, # FIXME
+
+ {'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>>()']},
+
+ # FIXME: Currently doesn't work because 'import' is seen as a reserved keyword by the CPlusPlusNameParser
+ # {'name': 'import', 'loc_names': [auto ns::Foo<double>::import<ns::Foo<double>>()]},
+
+ {'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>)']},
+ # {'name': 'operator<<<ns::Foo<int>>', 'loc_names': ['void ns::Foo<double>::operator<<<ns::Foo<int>>(ns::Foo<int>)']}, # FIXME
]
for bp_dict in bp_dicts:
Index: lldb/test/API/functionalities/breakpoint/cpp/Makefile
===================================================================
--- lldb/test/API/functionalities/breakpoint/cpp/Makefile
+++ 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
Index: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
===================================================================
--- lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
+++ lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
@@ -58,6 +58,9 @@
bool ContainsPath(llvm::StringRef path);
+ private:
+ llvm::StringRef GetBasenameNoTemplateParameters();
+
protected:
void Parse();
bool TrySimplifiedParse();
Index: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
===================================================================
--- lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -269,9 +269,21 @@
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 {};
+}
+
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 @@
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;
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
