[lldb] Support simplified template names when looking up functions

[aeubanks]

Summary

This makes setting breakpoints work with -gsimple-template-names.

Assume that callers handle false positives. For example, Module::LookupInfo::Prune removes wrong template instantiations when setting a breakpoint.

[aeubanks]

(I don't understand why this works yet, it should be giving us false positives?)

[aeubanks]

updated description with why this doesn't produce false positives with breakpoints

this doesn't support regex function name lookup, not sure if we care enough about the interaction between regexes/function names under simple template names. if we do, we could instead add template parameters to the names we put in the manual index. I did take a quick look at doing that but it'd require more work

[dblaikie]

In D137098#3897289, @aeubanks wrote:

updated description with why this doesn't produce false positives with breakpoints

this doesn't support regex function name lookup, not sure if we care enough about the interaction between regexes/function names under simple template names. if we do, we could instead add template parameters to the names we put in the manual index. I did take a quick look at doing that but it'd require more work

Presumably that'd then lead to divergence between manual and automatic index - which would be bad. So if this behavior is the same between automatic and manual index with simplified template names, that's probably good.

[labath]

Why is it that the other indexes don't need an equivalent fix? Could it be that you just haven't tried those code paths?

If they do need it, then it'd be good if we could make the fix in a single place. Possibly by putting the retry logic at a higher level?

In D137098#3897322, @dblaikie wrote:

In D137098#3897289, @aeubanks wrote:

updated description with why this doesn't produce false positives with breakpoints

this doesn't support regex function name lookup, not sure if we care enough about the interaction between regexes/function names under simple template names. if we do, we could instead add template parameters to the names we put in the manual index. I did take a quick look at doing that but it'd require more work

Presumably that'd then lead to divergence between manual and automatic index - which would be bad. So if this behavior is the same between automatic and manual index with simplified template names, that's probably good.

In addition to the divergence, the building the manual index is possibly the most performance-critical part of lldb, so doing all this extra work to reconstruct the templated names is probably a non-starter. If we wanted to support that, then we'd have to come up with a completely different way to achieve implement this functionality.

lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp

450 ↗

(On Diff #472081)

Are you sure this doesn't need to be repeated for eFunctionNameTypeBase? that's where non-member functions end up...

[aeubanks]

move retry into SymbolFileDWARF
add tests for non-member functions

[aeubanks]

In D137098#3902140, @labath wrote:

Why is it that the other indexes don't need an equivalent fix? Could it be that you just haven't tried those code paths?

If they do need it, then it'd be good if we could make the fix in a single place. Possibly by putting the retry logic at a higher level?

Other indexes as in the other manual index indexes? Or as in other higher level indexes?

lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp

450 ↗

(On Diff #472081)

you're right, but I've moved this up into SymbolFileDWARF

[labath]

In D137098#3903547, @aeubanks wrote:

In D137098#3902140, @labath wrote:

Why is it that the other indexes don't need an equivalent fix? Could it be that you just haven't tried those code paths?

If they do need it, then it'd be good if we could make the fix in a single place. Possibly by putting the retry logic at a higher level?

Other indexes as in the other manual index indexes? Or as in other higher level indexes?

I meant the compiler-generated indexes (implementations in AppleDWARFIndex and DebugNamesDWARFIndex).

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

void SymbolFileDWARF::FindFunctions(const Module::LookupInfo &lookup_info,
 
   llvm::DenseSet<const DWARFDebugInfoEntry *> resolved_dies;
 
   m_index->GetFunctions(lookup_info, *this, parent_decl_ctx, [&](DWARFDIE die) {
     if (resolved_dies.insert(die.GetDIE()).second)
       ResolveFunction(die, include_inlines, sc_list);
     return true;
   });
+  // With -gsimple-template-names, a templated type's DW_AT_name will not
+  // contain the template parameters. Try again stripping '<' and anything
+  // after, filtering out entries with template parameters that don't match.
+  {
+    const llvm::StringRef name_ref = name.GetStringRef();
+    auto it = name_ref.find('<');
+    if (it != llvm::StringRef::npos) {
+      const llvm::StringRef name_no_template_params = name_ref.slice(0, it);
+
+      Module::LookupInfo no_tp_lookup_info(lookup_info);
+      no_tp_lookup_info.SetLookupName(ConstString(name_no_template_params));
+      m_index->GetFunctions(no_tp_lookup_info, *this, parent_decl_ctx, [&](DWARFDIE die) {
+        if (resolved_dies.insert(die.GetDIE()).second)
+          ResolveFunction(die, include_inlines, sc_list);
+        return true;
+      });
+    }
+  }
 
   // Return the number of variable that were appended to the list
   const uint32_t num_matches = sc_list.GetSize() - original_size;
 
   if (log && num_matches > 0) {
     GetObjectFile()->GetModule()->LogMessage(
         log,
         "SymbolFileDWARF::FindFunctions (name=\"%s\", "

lldb/test/API/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py

 """
 Test lldb breakpoint ids.
 """
 
 import lldb
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
 
 class TestCPPBreakpointLocations(TestBase):
 
     @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24764")
     def test(self):
-        self.build()
+        self.do_test(dict())
+
+    @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24764")
+    @skipIf(compiler=no_match("clang"))
+    @skipIf(compiler_version=["<", "15.0"])
+    def test_simple_template_names(self):
+        self.do_test(dict(CFLAGS_EXTRAS="-gsimple-template-names"))
+
+    def do_test(self, debug_flags):
+        self.build(dictionary=debug_flags)
         self.breakpoint_id_tests()
 
     def verify_breakpoint_locations(self, target, bp_dict):
 
         name = bp_dict['name']
         names = bp_dict['loc_names']
         bp = target.BreakpointCreateByName(name)
         self.assertEquals(
def breakpoint_id_tests(self):
             {'name': 'b::c::func1', 'loc_names': ['b::c::func1()']},
             {'name': 'c::d::func2', 'loc_names': ['c::d::func2()']},
             {'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': 'Foo::func<float>', 'loc_names': []},
+            {'name': 'ns::Foo::func<float>', 'loc_names': []},
             {'name': 'func<int>', 'loc_names': ['auto ns::Foo<double>::func<int>()']},
+            {'name': 'Foo<double>::func<int>', 'loc_names': ['auto ns::Foo<double>::func<int>()']},
+            {'name': 'ns::Foo<double>::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>)']},
+
+            {'name': 'g<float>', 'loc_names': []},
+            {'name': 'g<int>', 'loc_names': ['void ns::g<int>()']},
+            {'name': 'g<char>', 'loc_names': ['void ns::g<char>()']},
+            {'name': 'g', 'loc_names': ['void ns::g<int>()', 'void ns::g<char>()']},
+            {'name': 'ns::g<float>', 'loc_names': []},
+            {'name': 'ns::g<int>', 'loc_names': ['void ns::g<int>()']},
+            {'name': 'ns::g<char>', 'loc_names': ['void ns::g<char>()']},
+            {'name': 'ns::g', 'loc_names': ['void ns::g<int>()', 'void ns::g<char>()']},
         ]
 
         for bp_dict in bp_dicts:
             self.verify_breakpoint_locations(target, bp_dict)
 
     @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24764")
     def test_destructors(self):
         self.build()

lldb/test/API/functionalities/breakpoint/cpp/main.cpp

template <typename Type> struct Foo {
   template <typename T> auto func() {}
 
   operator bool() { return true; }
 
   template <typename T> operator T() { return {}; }
 
   template <typename T> void operator<<(T t) {}
 };
+
+template <typename Type> void g() {}
 } // namespace ns
 
 int main (int argc, char const *argv[])
 {
     a::c ac;
     aa::cc aac;
     b::c bc;
     c::d cd;
int main (int argc, char const *argv[])
     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>>({});
 
+    ns::g<int>();
+    ns::g<char>();
+
     return 0;
 }