From: Tom Tromey <tromey@adacore.com>
To: gdb-patches@sourceware.org
Cc: Tom Tromey <tromey@adacore.com>
Subject: [PATCH v2 4/4] Fix parameter-less template regression in new DWARF reader
Date: Tue, 10 Jan 2023 11:33:38 -0700 [thread overview]
Message-ID: <20230110183338.2623088-5-tromey@adacore.com> (raw)
In-Reply-To: <20230110183338.2623088-1-tromey@adacore.com>
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
---
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(-)
create mode 100644 gdb/testsuite/gdb.cp/paramless.cc
create mode 100644 gdb/testsuite/gdb.cp/paramless.exp
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 @@ class cooked_index
/* 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 @@ class cooked_index_vector : public dwarf_scanner_base
/* 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 c3f246fedf7..44b54f77de9 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -18741,8 +18741,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..392b15f4256
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/paramless.cc
@@ -0,0 +1,46 @@
+/* Test case for template breakpoint test.
+
+ Copyright 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..4fc8fd9e015
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/paramless.exp
@@ -0,0 +1,41 @@
+# Copyright 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
--
2.38.1
next prev parent reply other threads:[~2023-01-10 18:33 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-10 18:33 [PATCH v2 0/4] Fix " Tom Tromey
2023-01-10 18:33 ` [PATCH v2 1/4] Avoid submitting empty tasks in parallel_for_each Tom Tromey
2023-01-14 6:03 ` Joel Brobecker
2023-01-10 18:33 ` [PATCH v2 2/4] Don't erase empty indices in DWARF reader Tom Tromey
2023-01-14 6:05 ` Joel Brobecker
2023-01-17 13:53 ` Tom Tromey
2023-01-10 18:33 ` [PATCH v2 3/4] Move hash_entry and eq_entry into cooked_index::do_finalize Tom Tromey
2023-01-14 6:06 ` Joel Brobecker
2023-01-10 18:33 ` Tom Tromey [this message]
2023-01-14 6:11 ` [PATCH v2 4/4] Fix parameter-less template regression in new DWARF reader Joel Brobecker
2023-01-17 13:54 ` Tom Tromey
2023-01-17 16:44 ` Tom de Vries
2023-01-17 18:46 ` Tom Tromey
2023-01-17 18:09 ` Simon Marchi
2023-01-17 19:39 ` Tom Tromey
2023-01-27 5:47 ` Simon Marchi
2023-01-27 10:15 ` Andrew Burgess
2023-01-27 14:30 ` Tom Tromey
2023-01-27 19:57 ` Tom Tromey
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230110183338.2623088-5-tromey@adacore.com \
--to=tromey@adacore.com \
--cc=gdb-patches@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).