[Lldb-commits] [lldb] d4a55ad - [lldb][Breakpoint] Fix setting breakpoints on templates by basename

Michael Buch via lldb-commits lldb-commits at lists.llvm.org
Fri Oct 14 15:51:30 PDT 2022


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) {


        


More information about the lldb-commits mailing list