* [PATCH 0/4] Fix regression in new DWARF reader @ 2022-12-15 19:07 Tom Tromey 2022-12-15 19:07 ` [PATCH 1/4] Avoid submitting empty tasks in parallel_for_each Tom Tromey ` (3 more replies) 0 siblings, 4 replies; 17+ messages in thread From: Tom Tromey @ 2022-12-15 19:07 UTC (permalink / raw) To: gdb-patches This series started as a fix to PR c++/29896, but while investigating that, I found a few other minor cleanups to be done. Regression tested on x86-64 Fedora 34. Tom ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/4] Avoid submitting empty tasks in parallel_for_each 2022-12-15 19:07 [PATCH 0/4] Fix regression in new DWARF reader Tom Tromey @ 2022-12-15 19:07 ` Tom Tromey 2023-01-07 11:19 ` Joel Brobecker 2022-12-15 19:07 ` [PATCH 2/4] Don't erase empty indices in DWARF reader Tom Tromey ` (2 subsequent siblings) 3 siblings, 1 reply; 17+ messages in thread From: Tom Tromey @ 2022-12-15 19:07 UTC (permalink / raw) To: gdb-patches; +Cc: Tom Tromey I found that parallel_for_each would submit empty tasks to the thread pool. For example, this can happen if the number of tasks is smaller than the number of available threads. In the DWARF reader, this resulted in the cooked index containing empty sub-indices. This patch arranges to instead shrink the result vector and process the trailing entries in the calling thread. --- gdbsupport/parallel-for.h | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/gdbsupport/parallel-for.h b/gdbsupport/parallel-for.h index acd9137efbd..ed460f31f63 100644 --- a/gdbsupport/parallel-for.h +++ b/gdbsupport/parallel-for.h @@ -70,6 +70,12 @@ struct par_for_accumulator return result; } + /* Resize the results to N. */ + void resize (size_t n) + { + m_futures.resize (n); + } + private: /* A vector of futures coming from the tasks run in the @@ -108,6 +114,12 @@ struct par_for_accumulator<void> } } + /* Resize the results to N. */ + void resize (size_t n) + { + m_futures.resize (n); + } + private: std::vector<gdb::future<void>> m_futures; @@ -232,6 +244,17 @@ parallel_for_each (unsigned n, RandomIt first, RandomIt last, end = j; remaining_size -= chunk_size; } + + if (end == last) + { + /* We're about to dispatch the last batch of elements, which + we normally process in the main thread. So just truncate + the result list here. */ + count = i; + results.resize (count); + break; + } + if (parallel_for_each_debug) { debug_printf (_("Parallel for: elements on worker thread %i\t: %zu"), -- 2.34.3 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/4] Avoid submitting empty tasks in parallel_for_each 2022-12-15 19:07 ` [PATCH 1/4] Avoid submitting empty tasks in parallel_for_each Tom Tromey @ 2023-01-07 11:19 ` Joel Brobecker 2023-01-09 16:20 ` Tom Tromey 0 siblings, 1 reply; 17+ messages in thread From: Joel Brobecker @ 2023-01-07 11:19 UTC (permalink / raw) To: Tom Tromey via Gdb-patches; +Cc: Tom Tromey, Joel Brobecker Hi Tom, On Thu, Dec 15, 2022 at 12:07:56PM -0700, Tom Tromey via Gdb-patches wrote: > I found that parallel_for_each would submit empty tasks to the thread > pool. For example, this can happen if the number of tasks is smaller > than the number of available threads. In the DWARF reader, this > resulted in the cooked index containing empty sub-indices. This patch > arranges to instead shrink the result vector and process the trailing > entries in the calling thread. This patch looks good to me. If I'm allowed to nitpick... > --- > gdbsupport/parallel-for.h | 23 +++++++++++++++++++++++ > 1 file changed, 23 insertions(+) > > diff --git a/gdbsupport/parallel-for.h b/gdbsupport/parallel-for.h > index acd9137efbd..ed460f31f63 100644 > --- a/gdbsupport/parallel-for.h > +++ b/gdbsupport/parallel-for.h > @@ -70,6 +70,12 @@ struct par_for_accumulator > return result; > } > > + /* Resize the results to N. */ > + void resize (size_t n) > + { > + m_futures.resize (n); > + } > + > private: > > /* A vector of futures coming from the tasks run in the > @@ -108,6 +114,12 @@ struct par_for_accumulator<void> > } > } > > + /* Resize the results to N. */ > + void resize (size_t n) > + { > + m_futures.resize (n); > + } > + > private: > > std::vector<gdb::future<void>> m_futures; > @@ -232,6 +244,17 @@ parallel_for_each (unsigned n, RandomIt first, RandomIt last, > end = j; > remaining_size -= chunk_size; > } > + > + if (end == last) > + { > + /* We're about to dispatch the last batch of elements, which > + we normally process in the main thread. So just truncate > + the result list here. */ ... I'd expand the last sentence to explain that this is to avoid submitting empty tasks to the thread pool. But I'll leave it up to you do decide whether that's a useful suggestion or not ;-). Thanks for that patch! > + count = i; > + results.resize (count); > + break; > + } > + > if (parallel_for_each_debug) > { > debug_printf (_("Parallel for: elements on worker thread %i\t: %zu"), > -- > 2.34.3 > -- Joel ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/4] Avoid submitting empty tasks in parallel_for_each 2023-01-07 11:19 ` Joel Brobecker @ 2023-01-09 16:20 ` Tom Tromey 0 siblings, 0 replies; 17+ messages in thread From: Tom Tromey @ 2023-01-09 16:20 UTC (permalink / raw) To: Joel Brobecker; +Cc: Tom Tromey via Gdb-patches, Tom Tromey >>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes: >> + /* We're about to dispatch the last batch of elements, which >> + we normally process in the main thread. So just truncate >> + the result list here. */ Joel> ... I'd expand the last sentence to explain that this is to avoid Joel> submitting empty tasks to the thread pool. Joel> But I'll leave it up to you do decide whether that's a useful suggestion Joel> or not ;-). Makes sense to me, I did this. Tom ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/4] Don't erase empty indices in DWARF reader 2022-12-15 19:07 [PATCH 0/4] Fix regression in new DWARF reader Tom Tromey 2022-12-15 19:07 ` [PATCH 1/4] Avoid submitting empty tasks in parallel_for_each Tom Tromey @ 2022-12-15 19:07 ` Tom Tromey 2022-12-19 14:51 ` Andrew Burgess 2022-12-15 19:07 ` [PATCH 3/4] Move hash_entry and eq_entry into cooked_index::do_finalize Tom Tromey 2022-12-15 19:07 ` [PATCH 4/4] Fix parameter-less template regression in new DWARF reader Tom Tromey 3 siblings, 1 reply; 17+ messages in thread From: Tom Tromey @ 2022-12-15 19:07 UTC (permalink / raw) To: gdb-patches; +Cc: Tom Tromey The DWARF reader has some code to remove empty indices. However, I think this code has been obsolete since some earlier changes to parallel_for_each. This patch removes this code. --- gdb/dwarf2/read.c | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index 032e20af93a..a3f4ef351eb 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -7170,16 +7170,6 @@ dwarf2_build_psymtabs_hard (dwarf2_per_objfile *per_objfile) print_tu_stats (per_objfile); indexes.push_back (index_storage.release ()); - /* Remove any NULL entries. This might happen if parallel-for - decides to throttle the number of threads that were used. */ - indexes.erase - (std::remove_if (indexes.begin (), - indexes.end (), - [] (const std::unique_ptr<cooked_index> &entry) - { - return entry == nullptr; - }), - indexes.end ()); indexes.shrink_to_fit (); cooked_index_vector *vec = new cooked_index_vector (std::move (indexes)); -- 2.34.3 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] Don't erase empty indices in DWARF reader 2022-12-15 19:07 ` [PATCH 2/4] Don't erase empty indices in DWARF reader Tom Tromey @ 2022-12-19 14:51 ` Andrew Burgess 2022-12-19 17:09 ` Tom Tromey 0 siblings, 1 reply; 17+ messages in thread From: Andrew Burgess @ 2022-12-19 14:51 UTC (permalink / raw) To: Tom Tromey via Gdb-patches, gdb-patches; +Cc: Tom Tromey Tom Tromey via Gdb-patches <gdb-patches@sourceware.org> writes: > The DWARF reader has some code to remove empty indices. However, I > think this code has been obsolete since some earlier changes to > parallel_for_each. This patch removes this code. Would NULL entries cause problems later in GDB? Would it be worth replacing this code with an assert that there are no NULL entries? Or would an attempt to create a NULL entry trigger an assert/error elsewhere? Or maybe the cost of iterating over the list is what you want to remove here? In which case, could we guard an assert in '#ifdef DEVELOPER'? Thanks, Andrew > --- > gdb/dwarf2/read.c | 10 ---------- > 1 file changed, 10 deletions(-) > > diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c > index 032e20af93a..a3f4ef351eb 100644 > --- a/gdb/dwarf2/read.c > +++ b/gdb/dwarf2/read.c > @@ -7170,16 +7170,6 @@ dwarf2_build_psymtabs_hard (dwarf2_per_objfile *per_objfile) > print_tu_stats (per_objfile); > > indexes.push_back (index_storage.release ()); > - /* Remove any NULL entries. This might happen if parallel-for > - decides to throttle the number of threads that were used. */ > - indexes.erase > - (std::remove_if (indexes.begin (), > - indexes.end (), > - [] (const std::unique_ptr<cooked_index> &entry) > - { > - return entry == nullptr; > - }), > - indexes.end ()); > indexes.shrink_to_fit (); > > cooked_index_vector *vec = new cooked_index_vector (std::move (indexes)); > -- > 2.34.3 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] Don't erase empty indices in DWARF reader 2022-12-19 14:51 ` Andrew Burgess @ 2022-12-19 17:09 ` Tom Tromey 2023-01-07 11:24 ` Joel Brobecker 0 siblings, 1 reply; 17+ messages in thread From: Tom Tromey @ 2022-12-19 17:09 UTC (permalink / raw) To: Andrew Burgess; +Cc: Tom Tromey via Gdb-patches, Tom Tromey >>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes: >> The DWARF reader has some code to remove empty indices. However, I >> think this code has been obsolete since some earlier changes to >> parallel_for_each. This patch removes this code. Andrew> Would NULL entries cause problems later in GDB? Would it be worth Andrew> replacing this code with an assert that there are no NULL entries? Or Andrew> would an attempt to create a NULL entry trigger an assert/error Andrew> elsewhere? If it could happen, it would cause a crash when doing any kind of lookup, because cooked_index_vector iterates over the indices and calls methods on each one. Andrew> Or maybe the cost of iterating over the list is what you want to remove Andrew> here? In which case, could we guard an assert in '#ifdef DEVELOPER'? I don't think there's any performance issue, as the size of the array is normally just the number of cores on the user machine. It's more that this is a leftover and can't happen any more, due to the previous patch. It seems to me that the contract of parallel_for_each should be that, if it returns results (i.e., not void), then each entry in the result must be the result of actually calling the callback -- i.e., not some default. That said, it's also fine to drop this patch. Tom ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] Don't erase empty indices in DWARF reader 2022-12-19 17:09 ` Tom Tromey @ 2023-01-07 11:24 ` Joel Brobecker 2023-01-09 16:45 ` Tom Tromey 0 siblings, 1 reply; 17+ messages in thread From: Joel Brobecker @ 2023-01-07 11:24 UTC (permalink / raw) To: Tom Tromey via Gdb-patches; +Cc: Andrew Burgess, Tom Tromey, Joel Brobecker Hi Tom, On Mon, Dec 19, 2022 at 10:09:06AM -0700, Tom Tromey via Gdb-patches wrote: > >>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes: > > >> The DWARF reader has some code to remove empty indices. However, I > >> think this code has been obsolete since some earlier changes to > >> parallel_for_each. This patch removes this code. > > Andrew> Would NULL entries cause problems later in GDB? Would it be worth > Andrew> replacing this code with an assert that there are no NULL entries? Or > Andrew> would an attempt to create a NULL entry trigger an assert/error > Andrew> elsewhere? > > If it could happen, it would cause a crash when doing any kind of > lookup, because cooked_index_vector iterates over the indices and calls > methods on each one. > > Andrew> Or maybe the cost of iterating over the list is what you want to remove > Andrew> here? In which case, could we guard an assert in '#ifdef DEVELOPER'? > > I don't think there's any performance issue, as the size of the array is > normally just the number of cores on the user machine. > > It's more that this is a leftover and can't happen any more, due to the > previous patch. It seems to me that the contract of parallel_for_each > should be that, if it returns results (i.e., not void), then each entry > in the result must be the result of actually calling the callback -- > i.e., not some default. > > That said, it's also fine to drop this patch. If we believe the code is dead, let's drop it. Maybe Andrew's assert suggestion could be helpful, although perhaps a complaint might be a better compromise in this case, as I would say a violation of this expectation would still mean a usable debugging experience for the user, so let's not break things with an assert? Thank you! -- Joel ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] Don't erase empty indices in DWARF reader 2023-01-07 11:24 ` Joel Brobecker @ 2023-01-09 16:45 ` Tom Tromey 2023-01-09 18:18 ` Tom Tromey 0 siblings, 1 reply; 17+ messages in thread From: Tom Tromey @ 2023-01-09 16:45 UTC (permalink / raw) To: Joel Brobecker; +Cc: Tom Tromey via Gdb-patches, Andrew Burgess, Tom Tromey Joel> If we believe the code is dead, let's drop it. Joel> Maybe Andrew's assert suggestion could be helpful, although perhaps Joel> a complaint might be a better compromise in this case, as I would say Joel> a violation of this expectation would still mean a usable debugging Joel> experience for the user, so let's not break things with an assert? I think a complaint would be incorrect here. Normally complaints are reserved for bad debug information, but this is just an internal aspect of the code -- the contract of parallel-for. I think it may be best to just drop this patch and leave the current code in place. It doesn't do anything, but on the other hand, this seems to be non-obvious, so it may serve a useful assurance purpose. Tom ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] Don't erase empty indices in DWARF reader 2023-01-09 16:45 ` Tom Tromey @ 2023-01-09 18:18 ` Tom Tromey 2023-01-10 10:07 ` Andrew Burgess 0 siblings, 1 reply; 17+ messages in thread From: Tom Tromey @ 2023-01-09 18:18 UTC (permalink / raw) To: Tom Tromey; +Cc: Joel Brobecker, Tom Tromey via Gdb-patches, Andrew Burgess >>>>> "Tom" == Tom Tromey <tromey@adacore.com> writes: Tom> I think it may be best to just drop this patch and leave the current Tom> code in place. It doesn't do anything, but on the other hand, this Tom> seems to be non-obvious, so it may serve a useful assurance purpose. It occurred to me today that another approach might be to add a self-test for this -- that is, use parallel-for-each on (say) a single task but when multiple threads are available. Then, it could check that there are no NULL responses. Tom ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] Don't erase empty indices in DWARF reader 2023-01-09 18:18 ` Tom Tromey @ 2023-01-10 10:07 ` Andrew Burgess 2023-01-10 18:24 ` Tom Tromey 0 siblings, 1 reply; 17+ messages in thread From: Andrew Burgess @ 2023-01-10 10:07 UTC (permalink / raw) To: Tom Tromey, Tom Tromey; +Cc: Joel Brobecker, Tom Tromey via Gdb-patches Tom Tromey <tromey@adacore.com> writes: >>>>>> "Tom" == Tom Tromey <tromey@adacore.com> writes: > > Tom> I think it may be best to just drop this patch and leave the current > Tom> code in place. It doesn't do anything, but on the other hand, this > Tom> seems to be non-obvious, so it may serve a useful assurance purpose. > > It occurred to me today that another approach might be to add a > self-test for this -- that is, use parallel-for-each on (say) a single > task but when multiple threads are available. Then, it could check that > there are no NULL responses. Hi Tom, Sorry I missed your original reply, and thanks for the extra info. If the code is dead then I don't see any point keeping it around. If you don't believe there's any point adding an assert in its place then I'm fine with that. I'm always pleased to see new selftests being added, so if you feel that's worth doing then +1 from me. But I don't think you should drop this patch if you are happy with it, no point in holding on to dead code. Thanks, Andrew ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] Don't erase empty indices in DWARF reader 2023-01-10 10:07 ` Andrew Burgess @ 2023-01-10 18:24 ` Tom Tromey 0 siblings, 0 replies; 17+ messages in thread From: Tom Tromey @ 2023-01-10 18:24 UTC (permalink / raw) To: Andrew Burgess; +Cc: Tom Tromey, Joel Brobecker, Tom Tromey via Gdb-patches >>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes: Andrew> If you don't believe there's any point adding an assert in its place Andrew> then I'm fine with that. Andrew> I'm always pleased to see new selftests being added, so if you feel Andrew> that's worth doing then +1 from me. Andrew> But I don't think you should drop this patch if you are happy with it, Andrew> no point in holding on to dead code. Alright. v2 will keep this and add some unit tests to the first patch. Tom ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 3/4] Move hash_entry and eq_entry into cooked_index::do_finalize 2022-12-15 19:07 [PATCH 0/4] Fix regression in new DWARF reader Tom Tromey 2022-12-15 19:07 ` [PATCH 1/4] Avoid submitting empty tasks in parallel_for_each Tom Tromey 2022-12-15 19:07 ` [PATCH 2/4] Don't erase empty indices in DWARF reader Tom Tromey @ 2022-12-15 19:07 ` Tom Tromey 2023-01-07 11:26 ` Joel Brobecker 2022-12-15 19:07 ` [PATCH 4/4] Fix parameter-less template regression in new DWARF reader Tom Tromey 3 siblings, 1 reply; 17+ messages in thread From: Tom Tromey @ 2022-12-15 19:07 UTC (permalink / raw) To: gdb-patches; +Cc: Tom Tromey I was briefly confused by the hash_entry and eq_entry functions in the cooked index. They are only needed in a single method, and that method already has a couple of local lambdas for a different hash table. So, it seemed cleaner to move these there as well. --- gdb/dwarf2/cooked-index.c | 34 ++++++++++++++-------------------- 1 file changed, 14 insertions(+), 20 deletions(-) diff --git a/gdb/dwarf2/cooked-index.c b/gdb/dwarf2/cooked-index.c index 0aa026c7779..f3688a351c9 100644 --- a/gdb/dwarf2/cooked-index.c +++ b/gdb/dwarf2/cooked-index.c @@ -26,26 +26,6 @@ #include "split-name.h" #include <algorithm> -/* Hash function for cooked_index_entry. */ - -static hashval_t -hash_entry (const void *e) -{ - const cooked_index_entry *entry = (const cooked_index_entry *) e; - return dwarf5_djb_hash (entry->canonical); -} - -/* Equality function for cooked_index_entry. */ - -static int -eq_entry (const void *a, const void *b) -{ - const cooked_index_entry *ae = (const cooked_index_entry *) a; - const gdb::string_view *sv = (const gdb::string_view *) b; - return (strlen (ae->canonical) == sv->length () - && strncasecmp (ae->canonical, sv->data (), sv->length ()) == 0); -} - /* See cooked-index.h. */ const char * @@ -191,6 +171,20 @@ cooked_index::do_finalize () htab_up seen_names (htab_create_alloc (10, hash_name_ptr, eq_name_ptr, nullptr, xcalloc, xfree)); + auto hash_entry = [] (const void *e) + { + const cooked_index_entry *entry = (const cooked_index_entry *) e; + return dwarf5_djb_hash (entry->canonical); + }; + + auto eq_entry = [] (const void *a, const void *b) -> int + { + const cooked_index_entry *ae = (const cooked_index_entry *) a; + const gdb::string_view *sv = (const gdb::string_view *) b; + return (strlen (ae->canonical) == sv->length () + && strncasecmp (ae->canonical, sv->data (), sv->length ()) == 0); + }; + htab_up gnat_entries (htab_create_alloc (10, hash_entry, eq_entry, nullptr, xcalloc, xfree)); -- 2.34.3 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/4] Move hash_entry and eq_entry into cooked_index::do_finalize 2022-12-15 19:07 ` [PATCH 3/4] Move hash_entry and eq_entry into cooked_index::do_finalize Tom Tromey @ 2023-01-07 11:26 ` Joel Brobecker 0 siblings, 0 replies; 17+ messages in thread From: Joel Brobecker @ 2023-01-07 11:26 UTC (permalink / raw) To: Tom Tromey via Gdb-patches; +Cc: Tom Tromey, Joel Brobecker Hi Tom, On Thu, Dec 15, 2022 at 12:07:58PM -0700, Tom Tromey via Gdb-patches wrote: > I was briefly confused by the hash_entry and eq_entry functions in the > cooked index. They are only needed in a single method, and that > method already has a couple of local lambdas for a different hash > table. So, it seemed cleaner to move these there as well. Sounds good to me. I think the contextualization of those functions indeed helps understanding what those are for. Thanks for the patch! > --- > gdb/dwarf2/cooked-index.c | 34 ++++++++++++++-------------------- > 1 file changed, 14 insertions(+), 20 deletions(-) > > diff --git a/gdb/dwarf2/cooked-index.c b/gdb/dwarf2/cooked-index.c > index 0aa026c7779..f3688a351c9 100644 > --- a/gdb/dwarf2/cooked-index.c > +++ b/gdb/dwarf2/cooked-index.c > @@ -26,26 +26,6 @@ > #include "split-name.h" > #include <algorithm> > > -/* Hash function for cooked_index_entry. */ > - > -static hashval_t > -hash_entry (const void *e) > -{ > - const cooked_index_entry *entry = (const cooked_index_entry *) e; > - return dwarf5_djb_hash (entry->canonical); > -} > - > -/* Equality function for cooked_index_entry. */ > - > -static int > -eq_entry (const void *a, const void *b) > -{ > - const cooked_index_entry *ae = (const cooked_index_entry *) a; > - const gdb::string_view *sv = (const gdb::string_view *) b; > - return (strlen (ae->canonical) == sv->length () > - && strncasecmp (ae->canonical, sv->data (), sv->length ()) == 0); > -} > - > /* See cooked-index.h. */ > > const char * > @@ -191,6 +171,20 @@ cooked_index::do_finalize () > htab_up seen_names (htab_create_alloc (10, hash_name_ptr, eq_name_ptr, > nullptr, xcalloc, xfree)); > > + auto hash_entry = [] (const void *e) > + { > + const cooked_index_entry *entry = (const cooked_index_entry *) e; > + return dwarf5_djb_hash (entry->canonical); > + }; > + > + auto eq_entry = [] (const void *a, const void *b) -> int > + { > + const cooked_index_entry *ae = (const cooked_index_entry *) a; > + const gdb::string_view *sv = (const gdb::string_view *) b; > + return (strlen (ae->canonical) == sv->length () > + && strncasecmp (ae->canonical, sv->data (), sv->length ()) == 0); > + }; > + > htab_up gnat_entries (htab_create_alloc (10, hash_entry, eq_entry, > nullptr, xcalloc, xfree)); > > -- > 2.34.3 > -- Joel ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 4/4] Fix parameter-less template regression in new DWARF reader 2022-12-15 19:07 [PATCH 0/4] Fix regression in new DWARF reader Tom Tromey ` (2 preceding siblings ...) 2022-12-15 19:07 ` [PATCH 3/4] Move hash_entry and eq_entry into cooked_index::do_finalize Tom Tromey @ 2022-12-15 19:07 ` Tom Tromey 2023-01-07 11:37 ` Joel Brobecker 3 siblings, 1 reply; 17+ messages in thread From: Tom Tromey @ 2022-12-15 19:07 UTC (permalink / raw) To: gdb-patches; +Cc: Tom Tromey 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 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 <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 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<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 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<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..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 <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..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 <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.34.3 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/4] Fix parameter-less template regression in new DWARF reader 2022-12-15 19:07 ` [PATCH 4/4] Fix parameter-less template regression in new DWARF reader Tom Tromey @ 2023-01-07 11:37 ` Joel Brobecker 2023-01-07 11:45 ` Joel Brobecker 0 siblings, 1 reply; 17+ messages in thread From: Joel Brobecker @ 2023-01-07 11:37 UTC (permalink / raw) To: Tom Tromey via Gdb-patches; +Cc: Tom Tromey, Joel Brobecker 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 <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 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<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 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<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..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 <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..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 <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.34.3 > -- Joel ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/4] Fix parameter-less template regression in new DWARF reader 2023-01-07 11:37 ` Joel Brobecker @ 2023-01-07 11:45 ` Joel Brobecker 0 siblings, 0 replies; 17+ messages in thread From: Joel Brobecker @ 2023-01-07 11:45 UTC (permalink / raw) To: Tom Tromey via Gdb-patches; +Cc: Tom Tromey, Joel Brobecker > 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 ;-). I forgot to mention that it was absolutely awesome that you added some unit tests. Thanks for that! -- Joel ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2023-01-10 18:24 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-12-15 19:07 [PATCH 0/4] Fix regression in new DWARF reader Tom Tromey 2022-12-15 19:07 ` [PATCH 1/4] Avoid submitting empty tasks in parallel_for_each Tom Tromey 2023-01-07 11:19 ` Joel Brobecker 2023-01-09 16:20 ` Tom Tromey 2022-12-15 19:07 ` [PATCH 2/4] Don't erase empty indices in DWARF reader Tom Tromey 2022-12-19 14:51 ` Andrew Burgess 2022-12-19 17:09 ` Tom Tromey 2023-01-07 11:24 ` Joel Brobecker 2023-01-09 16:45 ` Tom Tromey 2023-01-09 18:18 ` Tom Tromey 2023-01-10 10:07 ` Andrew Burgess 2023-01-10 18:24 ` Tom Tromey 2022-12-15 19:07 ` [PATCH 3/4] Move hash_entry and eq_entry into cooked_index::do_finalize Tom Tromey 2023-01-07 11:26 ` Joel Brobecker 2022-12-15 19:07 ` [PATCH 4/4] Fix parameter-less template regression in new DWARF reader Tom Tromey 2023-01-07 11:37 ` Joel Brobecker 2023-01-07 11:45 ` Joel Brobecker
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).