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

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();

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);
     }
   }
 

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

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;

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;
 }

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()']},

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();

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

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:

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;