[binutils-gdb/gdb-13-branch] Fix parameter-less template regression in new DWARF reader

[binutils-gdb/gdb-13-branch] Fix parameter-less template regression in new DWARF reader

[Tom Tromey]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=83d3152401cce0c6fd5cd2d4a140bbf5d89a5d9c

commit 83d3152401cce0c6fd5cd2d4a140bbf5d89a5d9c
Author: Tom Tromey <tromey@adacore.com>
Date:   Wed Dec 14 14:37:41 2022 -0700

    Fix parameter-less template regression in new DWARF reader
    
    PR c++/29896 points out a regression in the new DWARF reader.  It does
    not properly handle a case like "break fn", where "fn" is a template
    function.
    
    This happens because the new index uses strncasecmp to compare.
    However, to make this work correctly, we need a custom function that
    ignores template parameters.
    
    This patch adds a custom comparison function and fixes the bug.  A new
    test case is included.
    
    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29896
    
    (cherry picked from commit ac37b79cc440e37fc704d425a6e450afb3c7ee89)

Diff:
---
 gdb/dwarf2/cooked-index.c          | 143 ++++++++++++++++++++++++++++++++-----
 gdb/dwarf2/cooked-index.h          |  15 ++--
 gdb/dwarf2/read.c                  |   3 +-
 gdb/testsuite/gdb.cp/paramless.cc  |  46 ++++++++++++
 gdb/testsuite/gdb.cp/paramless.exp |  41 +++++++++++
 5 files changed, 226 insertions(+), 22 deletions(-)

diff --git a/gdb/dwarf2/cooked-index.c b/gdb/dwarf2/cooked-index.c
index c711e3b9b2a..09b3fd70b26 100644
--- a/gdb/dwarf2/cooked-index.c
+++ b/gdb/dwarf2/cooked-index.c
@@ -25,6 +25,114 @@
 #include "ada-lang.h"
 #include "split-name.h"
 #include <algorithm>
+#include "safe-ctype.h"
+#include "gdbsupport/selftest.h"
+
+/* See cooked-index.h.  */
+
+bool
+cooked_index_entry::compare (const char *stra, const char *strb,
+			     bool completing)
+{
+  /* If we've ever matched "<" in both strings, then we disable the
+     special template parameter handling.  */
+  bool seen_lt = false;
+
+  while (*stra != '\0'
+	 && *strb != '\0'
+	 && (TOLOWER ((unsigned char) *stra)
+	     == TOLOWER ((unsigned char ) *strb)))
+    {
+      if (*stra == '<')
+	seen_lt = true;
+      ++stra;
+      ++strb;
+    }
+
+  unsigned c1 = TOLOWER ((unsigned char) *stra);
+  unsigned c2 = TOLOWER ((unsigned char) *strb);
+
+  if (completing)
+    {
+      /* When completing, if one string ends earlier than the other,
+	 consider them as equal.  Also, completion mode ignores the
+	 special '<' handling.  */
+      if (c1 == '\0' || c2 == '\0')
+	return false;
+      /* Fall through to the generic case.  */
+    }
+  else if (seen_lt)
+    {
+      /* Fall through to the generic case.  */
+    }
+  else if (c1 == '\0' || c1 == '<')
+    {
+      /* Maybe they both end at the same spot.  */
+      if (c2 == '\0' || c2 == '<')
+	return false;
+      /* First string ended earlier.  */
+      return true;
+    }
+  else if (c2 == '\0' || c2 == '<')
+    {
+      /* Second string ended earlier.  */
+      return false;
+    }
+
+  return c1 < c2;
+}
+
+#if GDB_SELF_TEST
+
+namespace {
+
+void
+test_compare ()
+{
+  SELF_CHECK (!cooked_index_entry::compare ("abcd", "abcd", false));
+  SELF_CHECK (!cooked_index_entry::compare ("abcd", "abcd", false));
+  SELF_CHECK (!cooked_index_entry::compare ("abcd", "abcd", true));
+  SELF_CHECK (!cooked_index_entry::compare ("abcd", "abcd", true));
+
+  SELF_CHECK (cooked_index_entry::compare ("abcd", "ABCDE", false));
+  SELF_CHECK (!cooked_index_entry::compare ("ABCDE", "abcd", false));
+  SELF_CHECK (!cooked_index_entry::compare ("abcd", "ABCDE", true));
+  SELF_CHECK (!cooked_index_entry::compare ("ABCDE", "abcd", true));
+
+  SELF_CHECK (!cooked_index_entry::compare ("name", "name<>", false));
+  SELF_CHECK (!cooked_index_entry::compare ("name<>", "name", false));
+  SELF_CHECK (!cooked_index_entry::compare ("name", "name<>", true));
+  SELF_CHECK (!cooked_index_entry::compare ("name<>", "name", true));
+
+  SELF_CHECK (!cooked_index_entry::compare ("name<arg>", "name<arg>", false));
+  SELF_CHECK (!cooked_index_entry::compare ("name<arg>", "name<arg>", false));
+  SELF_CHECK (!cooked_index_entry::compare ("name<arg>", "name<arg>", true));
+  SELF_CHECK (!cooked_index_entry::compare ("name<arg>", "name<ag>", true));
+
+  SELF_CHECK (!cooked_index_entry::compare ("name<arg<more>>",
+					    "name<arg<more>>", false));
+
+  SELF_CHECK (!cooked_index_entry::compare ("name", "name<arg<more>>", false));
+  SELF_CHECK (!cooked_index_entry::compare ("name<arg<more>>", "name", false));
+  SELF_CHECK (cooked_index_entry::compare ("name<arg<", "name<arg<more>>",
+					   false));
+  SELF_CHECK (!cooked_index_entry::compare ("name<arg<",
+					    "name<arg<more>>",
+					    true));
+  SELF_CHECK (!cooked_index_entry::compare ("name<arg<more>>", "name<arg<",
+					    false));
+  SELF_CHECK (!cooked_index_entry::compare ("name<arg<more>>", "name<arg<",
+					    true));
+
+  SELF_CHECK (cooked_index_entry::compare ("", "abcd", false));
+  SELF_CHECK (!cooked_index_entry::compare ("", "abcd", true));
+  SELF_CHECK (!cooked_index_entry::compare ("abcd", "", false));
+  SELF_CHECK (!cooked_index_entry::compare ("abcd", "", true));
+}
+
+} /* anonymous namespace */
+
+#endif /* GDB_SELF_TEST */
 
 /* See cooked-index.h.  */
 
@@ -247,30 +355,24 @@ cooked_index::do_finalize ()
 /* See cooked-index.h.  */
 
 cooked_index::range
-cooked_index::find (gdb::string_view name, bool completing)
+cooked_index::find (const std::string &name, bool completing)
 {
   wait ();
 
-  auto lower = std::lower_bound (m_entries.begin (), m_entries.end (),
-				 name,
+  auto lower = std::lower_bound (m_entries.begin (), m_entries.end (), name,
 				 [=] (const cooked_index_entry *entry,
-				      const gdb::string_view &n)
+				      const std::string &n)
   {
-    int cmp = strncasecmp (entry->canonical, n.data (), n.length ());
-    if (cmp != 0 || completing)
-      return cmp < 0;
-    return strlen (entry->canonical) < n.length ();
+    return cooked_index_entry::compare (entry->canonical, n.c_str (),
+					completing);
   });
 
-  auto upper = std::upper_bound (m_entries.begin (), m_entries.end (),
-				 name,
-				 [=] (const gdb::string_view &n,
+  auto upper = std::upper_bound (m_entries.begin (), m_entries.end (), name,
+				 [=] (const std::string &n,
 				      const cooked_index_entry *entry)
   {
-    int cmp = strncasecmp (n.data (), entry->canonical, n.length ());
-    if (cmp != 0 || completing)
-      return cmp < 0;
-    return n.length () < strlen (entry->canonical);
+    return cooked_index_entry::compare (n.c_str (), entry->canonical,
+					completing);
   });
 
   return range (lower, upper);
@@ -311,7 +413,7 @@ cooked_index_vector::get_addrmaps ()
 /* See cooked-index.h.  */
 
 cooked_index_vector::range
-cooked_index_vector::find (gdb::string_view name, bool completing)
+cooked_index_vector::find (const std::string &name, bool completing)
 {
   std::vector<cooked_index::range> result_range;
   result_range.reserve (m_vector.size ());
@@ -339,3 +441,12 @@ cooked_index_vector::get_main () const
 
   return result;
 }
+
+void _initialize_cooked_index ();
+void
+_initialize_cooked_index ()
+{
+#if GDB_SELF_TEST
+  selftests::register_test ("cooked_index_entry::compare", test_compare);
+#endif
+}
diff --git a/gdb/dwarf2/cooked-index.h b/gdb/dwarf2/cooked-index.h
index 607e07745f9..2b34a6534e8 100644
--- a/gdb/dwarf2/cooked-index.h
+++ b/gdb/dwarf2/cooked-index.h
@@ -143,11 +143,16 @@ struct cooked_index_entry : public allocate_on_obstack
      STORAGE.  */
   const char *full_name (struct obstack *storage) const;
 
-  /* Entries must be sorted case-insensitively; this compares two
-     entries.  */
+  /* Compare two strings, case-insensitively.  Return true if STRA is
+     less than STRB.  If one string has template parameters, but the
+     other does not, then they are considered to be equal; so for
+     example "t<x>" == "t<x>", "t<x>" < "t<y>", but "t" == "t<x>".  */
+  static bool compare (const char *stra, const char *strb, bool completing);
+
+  /* Compare two entries by canonical name.  */
   bool operator< (const cooked_index_entry &other) const
   {
-    return strcasecmp (canonical, other.canonical) < 0;
+    return compare (canonical, other.canonical, false);
   }
 
   /* The name as it appears in DWARF.  This always points into one of
@@ -232,7 +237,7 @@ public:
   /* Look up an entry by name.  Returns a range of all matching
      results.  If COMPLETING is true, then a larger range, suitable
      for completion, will be returned.  */
-  range find (gdb::string_view name, bool completing);
+  range find (const std::string &name, bool completing);
 
 private:
 
@@ -335,7 +340,7 @@ public:
   /* Look up an entry by name.  Returns a range of all matching
      results.  If COMPLETING is true, then a larger range, suitable
      for completion, will be returned.  */
-  range find (gdb::string_view name, bool completing);
+  range find (const std::string &name, bool completing);
 
   /* Return a range of all the entries.  */
   range all_entries ()
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 23d69f45e24..32de64e9529 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -18724,8 +18724,9 @@ cooked_index_functions::expand_symtabs_matching
     {
       std::vector<gdb::string_view> name_vec
 	= lookup_name_without_params.split_name (lang);
+      std::string last_name = gdb::to_string (name_vec.back ());
 
-      for (const cooked_index_entry *entry : table->find (name_vec.back (),
+      for (const cooked_index_entry *entry : table->find (last_name,
 							  completing))
 	{
 	  QUIT;
diff --git a/gdb/testsuite/gdb.cp/paramless.cc b/gdb/testsuite/gdb.cp/paramless.cc
new file mode 100644
index 00000000000..82924391c24
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/paramless.cc
@@ -0,0 +1,46 @@
+/* Test case for template breakpoint test.
+
+   Copyright 2022-2023 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+template<typename T>
+struct outer
+{
+  template<typename Q = int>
+  int fn (int x)
+  {
+    return x + 23;
+  }
+};
+
+template<typename T = int>
+int toplev (int y)
+{
+  return y;
+}
+
+outer<int> outer1;
+outer<char> outer2;
+
+int main ()
+{
+  int x1 = outer1.fn (0);
+  int x2 = outer2.fn<short> (-46);
+  int x3 = toplev<char> (0);
+  int x4 = toplev (0);
+  return x1 + x2;
+}
diff --git a/gdb/testsuite/gdb.cp/paramless.exp b/gdb/testsuite/gdb.cp/paramless.exp
new file mode 100644
index 00000000000..79529de49f5
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/paramless.exp
@@ -0,0 +1,41 @@
+# Copyright 2022-2023 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This file is part of the gdb testsuite.
+
+# Test template breakpoints without parameters.
+
+if { [skip_cplus_tests] } { continue }
+
+standard_testfile .cc
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++}]} {
+    return -1
+}
+
+gdb_breakpoint "outer<int>::fn" message
+delete_breakpoints
+
+gdb_breakpoint "outer<char>::fn<short>" message
+delete_breakpoints
+
+gdb_test "break outer::fn" "Breakpoint $decimal at .*2 locations."
+delete_breakpoints
+
+gdb_test "break toplev" "Breakpoint $decimal at .*2 locations."
+delete_breakpoints
+
+gdb_breakpoint "toplev<char>" message
+delete_breakpoints