From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-x32f.google.com (mail-wm1-x32f.google.com [IPv6:2a00:1450:4864:20::32f]) by sourceware.org (Postfix) with ESMTPS id D23C03858D33 for ; Sat, 7 Jan 2023 11:37:33 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org D23C03858D33 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=adacore.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=adacore.com Received: by mail-wm1-x32f.google.com with SMTP id k26-20020a05600c1c9a00b003d972646a7dso5267181wms.5 for ; Sat, 07 Jan 2023 03:37:33 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=adacore.com; s=google; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=kxwvw2hjjwMBo30EYgebSmASLcexdQVWq2ErgFcPFgg=; b=InalaAwR63VH4udmjSgkd5jxRSEHSk6Vla2tL/iiGIgioTEy62gKV6LqIBdKSlqaqH dq1fTfShM1+swqgO+CG3hRUN/AD27KKGThbOtfGnWNkIKALBFD9u2pgb2X7mL0BjMApb vEeusMeSq0CZLxycwgtd45QFaeDtrpZIIaB2E0f4L6AYG++FubaM9aNilnnG3Ybspeoo qI5y696CV72Bke7p6EOODqY3NgQQWUMyvIXeVu8lqolXDGazqpezHy5E3PRVI7JhmFI0 6EgzpVhI/EQJYLFFgPXLiu7Rs+ur6U9R8ECowQbeVag0o/RV5SU5ojF8FU3HC6vFlOwZ PBfQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=kxwvw2hjjwMBo30EYgebSmASLcexdQVWq2ErgFcPFgg=; b=gebb62SusKMy0iIjFhIy6dJiisGxYF/WukT+34y3gFI260KZl8exjQLYZmFPn06+oo 5EtlmigU1ytmDMjsrA4AnfkS7UFkr9Wbx4KS9asoGAfpaxYaM5OYZUv/u/yPqSn7fvkt h4wTuIpQUHM2qkHb9wZbgziGPw6qhFqvAo4K9N2eEHPa39C1S2cdMvgCYodPU+1IB4EB HPMMaEFjj2dUkxILJuo0r4OQpbTXje1vasfZrEd4DAzBsuwKBSzaZKVB5sH4lWRhYe7r yrvsakzneEXHbw26NCV9w5pnYTFdE7G83CPZl4j+1JLcmTF8ksTX/Eq4PeSmlOnNZW/B CTaA== X-Gm-Message-State: AFqh2koF/2t6wCRr2d4sGZYqtofJQb6+p0vP6Q3Yae9pNGWiGtUzXme5 VjKdvW+ywkycQ/GVsIcjF8KJOrkDLOGhGQrjtqH3 X-Google-Smtp-Source: AMrXdXveq/+eVAJcQI3V2+T5ArRxoMHU1lnT/OqXvGObZQU8WYhFE6zbL8e3qOnL66z1X9VhNgD3SA== X-Received: by 2002:a05:600c:3ca8:b0:3d9:e8b3:57fa with SMTP id bg40-20020a05600c3ca800b003d9e8b357famr169429wmb.30.1673091452560; Sat, 07 Jan 2023 03:37:32 -0800 (PST) Received: from takamaka.gnat.com (lfbn-reu-1-488-54.w92-130.abo.wanadoo.fr. [92.130.77.54]) by smtp.gmail.com with ESMTPSA id l27-20020a05600c2cdb00b003a84375d0d1sm10107277wmc.44.2023.01.07.03.37.31 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 07 Jan 2023 03:37:32 -0800 (PST) Received: by takamaka.gnat.com (Postfix, from userid 1000) id CB59F8288F; Sat, 7 Jan 2023 15:37:29 +0400 (+04) Date: Sat, 7 Jan 2023 15:37:29 +0400 From: Joel Brobecker To: Tom Tromey via Gdb-patches Cc: Tom Tromey , Joel Brobecker Subject: Re: [PATCH 4/4] Fix parameter-less template regression in new DWARF reader Message-ID: References: <20221215190759.2494095-1-tromey@adacore.com> <20221215190759.2494095-5-tromey@adacore.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20221215190759.2494095-5-tromey@adacore.com> X-Spam-Status: No, score=-10.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Hi Tom, On Thu, Dec 15, 2022 at 12:07:59PM -0700, Tom Tromey via Gdb-patches wrote: > 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 Thanks for this patch! Everything in it makes sense to me, so you can go ahead and push. But before doing so, you'll need to adjust the copyright year in the new files you're adding ;-). Thanks again! > --- > 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 f3688a351c9..fb874035965 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 > +#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", "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>", > + "name>", false)); > + > + 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>", > + false)); > + SELF_CHECK (!cooked_index_entry::compare ("name + "name>", > + true)); > + SELF_CHECK (!cooked_index_entry::compare ("name>", "name + false)); > + SELF_CHECK (!cooked_index_entry::compare ("name>", "name + 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 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 2ea32781be5..a1efbf0e573 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" == "t", "t" < "t", but "t" == "t". */ > + 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 a3f4ef351eb..c35113d4c5f 100644 > --- a/gdb/dwarf2/read.c > +++ b/gdb/dwarf2/read.c > @@ -18714,8 +18714,9 @@ cooked_index_functions::expand_symtabs_matching > { > std::vector 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..e8ce04ebf11 > --- /dev/null > +++ b/gdb/testsuite/gdb.cp/paramless.cc > @@ -0,0 +1,46 @@ > +/* Test case for template breakpoint test. > + > + Copyright 2022 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 . */ > + > +template > +struct outer > +{ > + template > + int fn (int x) > + { > + return x + 23; > + } > +}; > + > +template > +int toplev (int y) > +{ > + return y; > +} > + > +outer outer1; > +outer outer2; > + > +int main () > +{ > + int x1 = outer1.fn (0); > + int x2 = outer2.fn (-46); > + int x3 = toplev (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..fc90459e0e4 > --- /dev/null > +++ b/gdb/testsuite/gdb.cp/paramless.exp > @@ -0,0 +1,41 @@ > +# Copyright 2022 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 . > + > +# 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::fn" message > +delete_breakpoints > + > +gdb_breakpoint "outer::fn" 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" message > +delete_breakpoints > -- > 2.34.3 > -- Joel