* [PATCH v4 01/19] Don't use objfile::intern in DWO code
2023-12-10 21:41 [PATCH v4 00/19] Index DWARF in the background Tom Tromey
@ 2023-12-10 21:41 ` Tom Tromey
2024-01-09 18:44 ` Simon Marchi
2023-12-10 21:41 ` [PATCH v4 02/19] Pre-read DWZ section data Tom Tromey
` (18 subsequent siblings)
19 siblings, 1 reply; 25+ messages in thread
From: Tom Tromey @ 2023-12-10 21:41 UTC (permalink / raw)
To: gdb-patches
The DWO code in the DWARF reader currently uses objfile::intern. This
accesses a shared data structure and so would be racy when used from
multiple threads. I don't believe this can happen right now, but
background reading could provoke this, and in any case it's better to
avoid this, just to be sure.
This patch changes this code to just use a std::string. A new type is
introduced to do hash table lookups, to avoid unnecessary copies.
---
gdb/dwarf2/read.c | 31 +++++++++++++++++++++----------
1 file changed, 21 insertions(+), 10 deletions(-)
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 37cabe52ecc..9a5f52d0f7e 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -353,7 +353,7 @@ struct dwo_file
For virtual DWO files the name is constructed from the section offsets
of abbrev,line,loc,str_offsets so that we combine virtual DWO files
from related CU+TUs. */
- const char *dwo_name = nullptr;
+ std::string dwo_name;
/* The DW_AT_comp_dir attribute. */
const char *comp_dir = nullptr;
@@ -7714,19 +7714,30 @@ hash_dwo_file (const void *item)
const struct dwo_file *dwo_file = (const struct dwo_file *) item;
hashval_t hash;
- hash = htab_hash_string (dwo_file->dwo_name);
+ hash = htab_hash_string (dwo_file->dwo_name.c_str ());
if (dwo_file->comp_dir != NULL)
hash += htab_hash_string (dwo_file->comp_dir);
return hash;
}
+/* This is used when looking up entries in the DWO hash table. */
+
+struct dwo_file_search
+{
+ /* Name of the DWO to look for. */
+ const char *dwo_name;
+ /* Compilation directory to look for. */
+ const char *comp_dir;
+};
+
static int
eq_dwo_file (const void *item_lhs, const void *item_rhs)
{
const struct dwo_file *lhs = (const struct dwo_file *) item_lhs;
- const struct dwo_file *rhs = (const struct dwo_file *) item_rhs;
+ const struct dwo_file_search *rhs
+ = (const struct dwo_file_search *) item_rhs;
- if (strcmp (lhs->dwo_name, rhs->dwo_name) != 0)
+ if (lhs->dwo_name != rhs->dwo_name)
return 0;
if (lhs->comp_dir == NULL || rhs->comp_dir == NULL)
return lhs->comp_dir == rhs->comp_dir;
@@ -7752,7 +7763,7 @@ lookup_dwo_file_slot (dwarf2_per_objfile *per_objfile,
const char *dwo_name,
const char *comp_dir)
{
- struct dwo_file find_entry;
+ struct dwo_file_search find_entry;
void **slot;
if (per_objfile->per_bfd->dwo_files == NULL)
@@ -7820,7 +7831,7 @@ create_dwo_cu_reader (const struct die_reader_specs *reader,
{
complaint (_("Dwarf Error: debug entry at offset %s is missing"
" its dwo_id [in module %s]"),
- sect_offset_str (sect_off), dwo_file->dwo_name);
+ sect_offset_str (sect_off), dwo_file->dwo_name.c_str ());
return;
}
@@ -8471,7 +8482,7 @@ create_dwo_unit_in_dwp_v1 (dwarf2_per_objfile *per_objfile,
virtual_dwo_name.c_str ());
dwo_file = new struct dwo_file;
- dwo_file->dwo_name = per_objfile->objfile->intern (virtual_dwo_name);
+ dwo_file->dwo_name = std::move (virtual_dwo_name);
dwo_file->comp_dir = comp_dir;
dwo_file->sections.abbrev = sections.abbrev;
dwo_file->sections.line = sections.line;
@@ -8656,7 +8667,7 @@ create_dwo_unit_in_dwp_v2 (dwarf2_per_objfile *per_objfile,
virtual_dwo_name.c_str ());
dwo_file = new struct dwo_file;
- dwo_file->dwo_name = per_objfile->objfile->intern (virtual_dwo_name);
+ dwo_file->dwo_name = std::move (virtual_dwo_name);
dwo_file->comp_dir = comp_dir;
dwo_file->sections.abbrev =
create_dwp_v2_or_v5_section (per_objfile, &dwp_file->sections.abbrev,
@@ -8828,7 +8839,7 @@ create_dwo_unit_in_dwp_v5 (dwarf2_per_objfile *per_objfile,
virtual_dwo_name.c_str ());
dwo_file = new struct dwo_file;
- dwo_file->dwo_name = per_objfile->objfile->intern (virtual_dwo_name);
+ dwo_file->dwo_name = std::move (virtual_dwo_name);
dwo_file->comp_dir = comp_dir;
dwo_file->sections.abbrev =
create_dwp_v2_or_v5_section (per_objfile,
@@ -17507,7 +17518,7 @@ read_dwo_str_index (const struct die_reader_specs *reader, ULONGEST str_index)
least has a limit. */
complaint (_("Section .debug_str_offsets in %s has unsupported"
" version %d, use empty string."),
- reader->dwo_file->dwo_name, version);
+ reader->dwo_file->dwo_name.c_str (), version);
return "";
}
--
2.43.0
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH v4 01/19] Don't use objfile::intern in DWO code
2023-12-10 21:41 ` [PATCH v4 01/19] Don't use objfile::intern in DWO code Tom Tromey
@ 2024-01-09 18:44 ` Simon Marchi
2024-01-09 19:12 ` Tom Tromey
0 siblings, 1 reply; 25+ messages in thread
From: Simon Marchi @ 2024-01-09 18:44 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 12/10/23 16:41, Tom Tromey wrote:
> The DWO code in the DWARF reader currently uses objfile::intern. This
> accesses a shared data structure and so would be racy when used from
> multiple threads. I don't believe this can happen right now, but
> background reading could provoke this, and in any case it's better to
> avoid this, just to be sure.
>
> This patch changes this code to just use a std::string. A new type is
> introduced to do hash table lookups, to avoid unnecessary copies.
Hi Tom,
I am getting some ASan-related failures on my CI, starting with this
patch. I had trouble reproducing on my machine, but I got it to
reproduce when using the exact source and build paths that my CI job
used (since the patch is related to how strings (including paths) are
handled, it's possible that the bug requires some specific strings in
order to show up). That is:
Source directory (the git repo): /home/jenkins/workspace/binutils-gdb_master_linuxbuild/platform/deb12-amd64/target_board/unix/src/binutils-gdb
Build directory: /home/jenkins/workspace/binutils-gdb_master_linuxbuild/platform/deb12-amd64/target_board/unix/tmp/tmp.Rud1Y0ryoD
My full configure line is:
$ /home/jenkins/workspace/binutils-gdb_master_linuxbuild/platform/deb12-amd64/target_board/unix/src/binutils-gdb/configure --prefix=/build --disable-binutils --disable-ld --disable-gold --disable-gas --disable-sim --disable-gprof --disable-gprofng --with-system-readline --with-system-zlib --with-expat=yes --with-python=python3 --with-guile --enable-libctf --enable-build-warnings --enable-gdb-build-warnings --enable-unit-tests --enable-ubsan CXXFLAGS="-O2 -g -fsanitize=address -D_GLIBCXX_DEBUG=1" CFLAGS="-O2 -g -fsanitize=address" LDFLAGS="-fsanitize=address -fuse-ld=mold"
If that matters, this is on Debian 12, using the default compiler (gcc
12.2.0).
An example of failing test is gdb.dwarf2/fission-absolute-dwo.exp.
The failure looks like:
$ ./gdb -nx -q --data-directory=data-directory testsuite/outputs/gdb.dwarf2/fission-absolute-dwo/fission-absolute-dwo
=================================================================
==3166157==ERROR: AddressSanitizer: heap-use-after-free on address 0x61500000fff8 at pc 0x55f870746885 bp 0x7fabf4f3f720 sp 0x7fabf4f3f718
READ of size 1 at 0x61500000fff8 thread T5
#0 0x55f870746884 in htab_hash_string /home/jenkins/workspace/binutils-gdb_master_linuxbuild/platform/deb12-amd64/target_board/unix/src/binutils-gdb/libiberty/hashtab.c:838
#1 0x55f86e2ad7f5 in hash_dwo_file /home/jenkins/workspace/binutils-gdb_master_linuxbuild/platform/deb12-amd64/target_board/unix/src/binutils-gdb/gdb/dwarf2/read.c:7722
#2 0x55f8707463b5 in htab_find_slot /home/jenkins/workspace/binutils-gdb_master_linuxbuild/platform/deb12-amd64/target_board/unix/src/binutils-gdb/libiberty/hashtab.c:703
#3 0x55f86e2acd54 in lookup_dwo_file_slot /home/jenkins/workspace/binutils-gdb_master_linuxbuild/platform/deb12-amd64/target_board/unix/src/binutils-gdb/gdb/dwarf2/read.c:7777
#4 0x55f86e35a6f3 in lookup_dwo_cutu /home/jenkins/workspace/binutils-gdb_master_linuxbuild/platform/deb12-amd64/target_board/unix/src/binutils-gdb/gdb/dwarf2/read.c:9551
#5 0x55f86e36041c in lookup_dwo_comp_unit /home/jenkins/workspace/binutils-gdb_master_linuxbuild/platform/deb12-amd64/target_board/unix/src/binutils-gdb/gdb/dwarf2/read.c:9630
#6 0x55f86e36041c in lookup_dwo_unit /home/jenkins/workspace/binutils-gdb_master_linuxbuild/platform/deb12-amd64/target_board/unix/src/binutils-gdb/gdb/dwarf2/read.c:3951
#7 0x55f86e36041c in cutu_reader::cutu_reader(dwarf2_per_cu_data*, dwarf2_per_objfile*, abbrev_table*, dwarf2_cu*, bool, abbrev_cache*) /home/jenkins/workspace/binutils-gdb_master_linuxbuild/platform/deb12-amd64/target_board/unix/src/binutils-gdb/gdb/dwarf2/read.c:4192
#8 0x55f86e4ea4f0 in process_psymtab_comp_unit /home/jenkins/workspace/binutils-gdb_master_linuxbuild/platform/deb12-amd64/target_board/unix/src/binutils-gdb/gdb/dwarf2/read.c:4652
#9 0x55f86e4ea4f0 in operator() /home/jenkins/workspace/binutils-gdb_master_linuxbuild/platform/deb12-amd64/target_board/unix/src/binutils-gdb/gdb/dwarf2/read.c:4965
#10 0x55f86e4eda44 in operator() /home/jenkins/workspace/binutils-gdb_master_linuxbuild/platform/deb12-amd64/target_board/unix/src/binutils-gdb/gdb/../gdbsupport/parallel-for.h:273
#11 0x55f86e4eda44 in __invoke_impl<std::pair<std::unique_ptr<cooked_index_shard>, std::__debug::vector<gdb_exception> >, gdb::parallel_for_each<__gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter>*, std::__cxx1998::vector<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter>, std::allocator<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter> > > >, std::__debug::vector<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter> >, std::random_access_iterator_tag>, dwarf2_build_psymtabs_hard(dwarf2_per_objfile*)::<lambda(iter_type, iter_type)> >(unsigned int, __gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter>*, std::__cxx1998::vector<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter>, std::allocator<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter> > > >, std::__debug::vector<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter> >, std::random_access_iterator_tag>, __gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter>*, std::__cxx1998::vector<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter>, std::allocator<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter> > > >, std::__debug::vector<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter> >, std::random_access_iterator_tag>, dwarf2_build_psymtabs_hard(dwarf2_per_objfile*)::<lambda(iter_type, iter_type)>, function_view<long unsigned int(__gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter>*, std::__cxx1998::vector<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter>, std::allocator<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter> > > >, std::__debug::vector<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter> >, std::random_access_iterator_tag>)>)::<lambda()>&> /usr/include/c++/12/bits/invoke.h:61
#12 0x55f86e4eda44 in __invoke_r<std::pair<std::unique_ptr<cooked_index_shard>, std::__debug::vector<gdb_exception> >, gdb::parallel_for_each<__gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter>*, std::__cxx1998::vector<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter>, std::allocator<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter> > > >, std::__debug::vector<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter> >, std::random_access_iterator_tag>, dwarf2_build_psymtabs_hard(dwarf2_per_objfile*)::<lambda(iter_type, iter_type)> >(unsigned int, __gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter>*, std::__cxx1998::vector<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter>, std::allocator<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter> > > >, std::__debug::vector<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter> >, std::random_access_iterator_tag>, __gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter>*, std::__cxx1998::vector<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter>, std::allocator<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter> > > >, std::__debug::vector<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter> >, std::random_access_iterator_tag>, dwarf2_build_psymtabs_hard(dwarf2_per_objfile*)::<lambda(iter_type, iter_type)>, function_view<long unsigned int(__gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter>*, std::__cxx1998::vector<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter>, std::allocator<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter> > > >, std::__debug::vector<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter> >, std::random_access_iterator_tag>)>)::<lambda()>&> /usr/include/c++/12/bits/invoke.h:116
#13 0x55f86e4eda44 in _M_invoke /usr/include/c++/12/bits/std_function.h:291
#14 0x55f86e414f80 in std::function<std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > ()>::operator()() const /usr/include/c++/12/bits/std_function.h:591
#15 0x55f86e414f80 in std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > std::__invoke_impl<std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > >, std::function<std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > ()>&>(std::__invoke_other, std::function<std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > ()>&) /usr/include/c++/12/bits/invoke.h:61
#16 0x55f86e414f80 in std::enable_if<is_invocable_r_v<std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > >, std::function<std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > ()>&>, std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > >::type std::__invoke_r<std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > >, std::function<std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > ()>&>(std::function<std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > ()>&) /usr/include/c++/12/bits/invoke.h:116
#17 0x55f86e414f80 in std::__future_base::_Task_state<std::function<std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > ()>, std::allocator<int>, std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > ()>::_M_run()::{lambda()#1}::operator()() const /usr/include/c++/12/future:1470
#18 0x55f86e414f80 in std::__future_base::_Task_setter<std::unique_ptr<std::__future_base::_Result<std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > >, std::__future_base::_Result_base::_Deleter>, std::__future_base::_Task_state<std::function<std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > ()>, std::allocator<int>, std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > ()>::_M_run()::{lambda()#1}, std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > >::operator()() const /usr/include/c++/12/future:1387
#19 0x55f86e414f80 in std::unique_ptr<std::__future_base::_Result<std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > >, std::__future_base::_Result_base::_Deleter> std::__invoke_impl<std::unique_ptr<std::__future_base::_Result<std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > >, std::__future_base::_Result_base::_Deleter>, std::__future_base::_Task_setter<std::unique_ptr<std::__future_base::_Result<std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > >, std::__future_base::_Result_base::_Deleter>, std::__future_base::_Task_state<std::function<std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > ()>, std::allocator<int>, std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > ()>::_M_run()::{lambda()#1}, std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > >&>(std::__invoke_other, std::__future_base::_Task_setter<std::unique_ptr<std::__future_base::_Result<std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > >, std::__future_base::_Result_base::_Deleter>, std::__future_base::_Task_state<std::function<std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > ()>, std::allocator<int>, std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > ()>::_M_run()::{lambda()#1}, std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > >&) /usr/include/c++/12/bits/invoke.h:61
#20 0x55f86e414f80 in std::enable_if<is_invocable_r_v<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter>, std::__future_base::_Task_setter<std::unique_ptr<std::__future_base::_Result<std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > >, std::__future_base::_Result_base::_Deleter>, std::__future_base::_Task_state<std::function<std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > ()>, std::allocator<int>, std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > ()>::_M_run()::{lambda()#1}, std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > >&>, std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> >::type std::__invoke_r<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter>, std::__future_base::_Task_setter<std::unique_ptr<std::__future_base::_Result<std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > >, std::__future_base::_Result_base::_Deleter>, std::__future_base::_Task_state<std::function<std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > ()>, std::allocator<int>, std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > ()>::_M_run()::{lambda()#1}, std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > >&>(std::__future_base::_Task_setter<std::unique_ptr<std::__future_base::_Result<std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > >, std::__future_base::_Result_base::_Deleter>, std::__future_base::_Task_state<std::function<std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > ()>, std::allocator<int>, std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > ()>::_M_run()::{lambda()#1}, std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > >&) /usr/include/c++/12/bits/invoke.h:116
#21 0x55f86e414f80 in std::_Function_handler<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> (), std::__future_base::_Task_setter<std::unique_ptr<std::__future_base::_Result<std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > >, std::__future_base::_Result_base::_Deleter>, std::__future_base::_Task_state<std::function<std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > ()>, std::allocator<int>, std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > ()>::_M_run()::{lambda()#1}, std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > > >::_M_invoke(std::_Any_data const&) /usr/include/c++/12/bits/std_function.h:291
#22 0x55f86e058b96 in std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>::operator()() const /usr/include/c++/12/bits/std_function.h:591
#23 0x55f86e058b96 in std::__future_base::_State_baseV2::_M_do_set(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*) /usr/include/c++/12/future:572
#24 0x7fabfa699ea6 in __pthread_once_slow nptl/pthread_once.c:116
#25 0x55f86e05b1a3 in __gthread_once /usr/include/x86_64-linux-gnu/c++/12/bits/gthr-default.h:700
#26 0x55f86e05b1a3 in void std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&) /usr/include/c++/12/mutex:859
#27 0x55f86e41ab0d in std::__future_base::_State_baseV2::_M_set_result(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>, bool) /usr/include/c++/12/future:412
#28 0x55f86e41ab0d in std::__future_base::_Task_state<std::function<std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > ()>, std::allocator<int>, std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > ()>::_M_run() /usr/include/c++/12/future:1472
#29 0x55f86e41ab0d in std::packaged_task<std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > ()>::operator()() /usr/include/c++/12/future:1605
#30 0x55f86e41ab0d in void std::__invoke_impl<void, std::packaged_task<std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > ()>&>(std::__invoke_other, std::packaged_task<std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > ()>&) /usr/include/c++/12/bits/invoke.h:61
#31 0x55f86e41ab0d in std::enable_if<is_invocable_r_v<void, std::packaged_task<std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > ()>&>, void>::type std::__invoke_r<void, std::packaged_task<std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > ()>&>(std::packaged_task<std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > ()>&) /usr/include/c++/12/bits/invoke.h:111
#32 0x55f86e41ab0d in std::__future_base::_Task_state<std::packaged_task<std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > ()>, std::allocator<int>, void ()>::_M_run()::{lambda()#1}::operator()() const /usr/include/c++/12/future:1469
#33 0x55f86e41ab0d in std::__future_base::_Task_setter<std::unique_ptr<std::__future_base::_Result<void>, std::__future_base::_Result_base::_Deleter>, std::__future_base::_Task_state<std::packaged_task<std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > ()>, std::allocator<int>, void ()>::_M_run()::{lambda()#1}, void>::operator()() const /usr/include/c++/12/future:1410
#34 0x55f86e41ab0d in std::unique_ptr<std::__future_base::_Result<void>, std::__future_base::_Result_base::_Deleter> std::__invoke_impl<std::unique_ptr<std::__future_base::_Result<void>, std::__future_base::_Result_base::_Deleter>, std::__future_base::_Task_setter<std::unique_ptr<std::__future_base::_Result<void>, std::__future_base::_Result_base::_Deleter>, std::__future_base::_Task_state<std::packaged_task<std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > ()>, std::allocator<int>, void ()>::_M_run()::{lambda()#1}, void>&>(std::__invoke_other, std::__future_base::_Task_setter<std::unique_ptr<std::__future_base::_Result<void>, std::__future_base::_Result_base::_Deleter>, std::__future_base::_Task_state<std::packaged_task<std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > ()>, std::allocator<int>, void ()>::_M_run()::{lambda()#1}, void>&) /usr/include/c++/12/bits/invoke.h:61
#35 0x55f86e41ab0d in std::enable_if<is_invocable_r_v<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter>, std::__future_base::_Task_setter<std::unique_ptr<std::__future_base::_Result<void>, std::__future_base::_Result_base::_Deleter>, std::__future_base::_Task_state<std::packaged_task<std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > ()>, std::allocator<int>, void ()>::_M_run()::{lambda()#1}, void>&>, std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> >::type std::__invoke_r<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter>, std::__future_base::_Task_setter<std::unique_ptr<std::__future_base::_Result<void>, std::__future_base::_Result_base::_Deleter>, std::__future_base::_Task_state<std::packaged_task<std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > ()>, std::allocator<int>, void ()>::_M_run()::{lambda()#1}, void>&>(std::__future_base::_Task_setter<std::unique_ptr<std::__future_base::_Result<void>, std::__future_base::_Result_base::_Deleter>, std::__future_base::_Task_state<std::packaged_task<std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > ()>, std::allocator<int>, void ()>::_M_run()::{lambda()#1}, void>&) /usr/include/c++/12/bits/invoke.h:116
#36 0x55f86e41ab0d in std::_Function_handler<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> (), std::__future_base::_Task_setter<std::unique_ptr<std::__future_base::_Result<void>, std::__future_base::_Result_base::_Deleter>, std::__future_base::_Task_state<std::packaged_task<std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > ()>, std::allocator<int>, void ()>::_M_run()::{lambda()#1}, void> >::_M_invoke(std::_Any_data const&) /usr/include/c++/12/bits/std_function.h:291
#37 0x55f86e058b96 in std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>::operator()() const /usr/include/c++/12/bits/std_function.h:591
#38 0x55f86e058b96 in std::__future_base::_State_baseV2::_M_do_set(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*) /usr/include/c++/12/future:572
#39 0x7fabfa699ea6 in __pthread_once_slow nptl/pthread_once.c:116
#40 0x55f86e05b1a3 in __gthread_once /usr/include/x86_64-linux-gnu/c++/12/bits/gthr-default.h:700
#41 0x55f86e05b1a3 in void std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&) /usr/include/c++/12/mutex:859
#42 0x55f86e419ba7 in std::__future_base::_State_baseV2::_M_set_result(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>, bool) /usr/include/c++/12/future:412
#43 0x55f86e419ba7 in std::__future_base::_Task_state<std::packaged_task<std::pair<std::unique_ptr<cooked_index_shard, std::default_delete<cooked_index_shard> >, std::__debug::vector<gdb_exception, std::allocator<gdb_exception> > > ()>, std::allocator<int>, void ()>::_M_run() /usr/include/c++/12/future:1472
#44 0x55f8707e3c6c in std::packaged_task<void ()>::operator()() /usr/include/c++/12/future:1605
#45 0x55f8707e3c6c in gdb::thread_pool::thread_function() /home/jenkins/workspace/binutils-gdb_master_linuxbuild/platform/deb12-amd64/target_board/unix/src/binutils-gdb/gdbsupport/thread-pool.cc:246
#46 0x7fabfaff44a2 (/lib/x86_64-linux-gnu/libstdc++.so.6+0xd44a2)
#47 0x7fabfa695043 in start_thread nptl/pthread_create.c:442
#48 0x7fabfa71561b in clone3 ../sysdeps/unix/sysv/linux/x86_64/clone3.S:81
0x61500000fff8 is located 120 bytes inside of 456-byte region [0x61500000ff80,0x615000010148)
freed by thread T5 here:
#0 0x7fabfbeef3c8 in operator delete(void*, unsigned long) ../../../../src/libsanitizer/asan/asan_new_delete.cpp:164
#1 0x55f86e410fe3 in std::default_delete<dwarf2_cu>::operator()(dwarf2_cu*) const /usr/include/c++/12/bits/unique_ptr.h:95
#2 0x55f86e410fe3 in std::default_delete<dwarf2_cu>::operator()(dwarf2_cu*) const /usr/include/c++/12/bits/unique_ptr.h:89
#3 0x55f86e410fe3 in std::unique_ptr<dwarf2_cu, std::default_delete<dwarf2_cu> >::~unique_ptr() /usr/include/c++/12/bits/unique_ptr.h:396
#4 0x55f86e410fe3 in cutu_reader::~cutu_reader() /home/jenkins/workspace/binutils-gdb_master_linuxbuild/platform/deb12-amd64/target_board/unix/src/binutils-gdb/gdb/dwarf2/read.c:581
previously allocated by thread T5 here:
#0 0x7fabfbeee4c8 in operator new(unsigned long) ../../../../src/libsanitizer/asan/asan_new_delete.cpp:95
#1 0x55f86e35ff94 in cutu_reader::cutu_reader(dwarf2_per_cu_data*, dwarf2_per_objfile*, abbrev_table*, dwarf2_cu*, bool, abbrev_cache*) /home/jenkins/workspace/binutils-gdb_master_linuxbuild/platform/deb12-amd64/target_board/unix/src/binutils-gdb/gdb/dwarf2/read.c:4085
#2 0x55f86e4ea4f0 in process_psymtab_comp_unit /home/jenkins/workspace/binutils-gdb_master_linuxbuild/platform/deb12-amd64/target_board/unix/src/binutils-gdb/gdb/dwarf2/read.c:4652
#3 0x55f86e4ea4f0 in operator() /home/jenkins/workspace/binutils-gdb_master_linuxbuild/platform/deb12-amd64/target_board/unix/src/binutils-gdb/gdb/dwarf2/read.c:4965
#4 0x55f86e4eda44 in operator() /home/jenkins/workspace/binutils-gdb_master_linuxbuild/platform/deb12-amd64/target_board/unix/src/binutils-gdb/gdb/../gdbsupport/parallel-for.h:273
#5 0x55f86e4eda44 in __invoke_impl<std::pair<std::unique_ptr<cooked_index_shard>, std::__debug::vector<gdb_exception> >, gdb::parallel_for_each<__gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter>*, std::__cxx1998::vector<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter>, std::allocator<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter> > > >, std::__debug::vector<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter> >, std::random_access_iterator_tag>, dwarf2_build_psymtabs_hard(dwarf2_per_objfile*)::<lambda(iter_type, iter_type)> >(unsigned int, __gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter>*, std::__cxx1998::vector<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter>, std::allocator<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter> > > >, std::__debug::vector<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter> >, std::random_access_iterator_tag>, __gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter>*, std::__cxx1998::vector<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter>, std::allocator<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter> > > >, std::__debug::vector<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter> >, std::random_access_iterator_tag>, dwarf2_build_psymtabs_hard(dwarf2_per_objfile*)::<lambda(iter_type, iter_type)>, function_view<long unsigned int(__gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter>*, std::__cxx1998::vector<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter>, std::allocator<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter> > > >, std::__debug::vector<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter> >, std::random_access_iterator_tag>)>)::<lambda()>&> /usr/include/c++/12/bits/invoke.h:61
#6 0x55f86e4eda44 in __invoke_r<std::pair<std::unique_ptr<cooked_index_shard>, std::__debug::vector<gdb_exception> >, gdb::parallel_for_each<__gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter>*, std::__cxx1998::vector<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter>, std::allocator<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter> > > >, std::__debug::vector<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter> >, std::random_access_iterator_tag>, dwarf2_build_psymtabs_hard(dwarf2_per_objfile*)::<lambda(iter_type, iter_type)> >(unsigned int, __gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter>*, std::__cxx1998::vector<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter>, std::allocator<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter> > > >, std::__debug::vector<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter> >, std::random_access_iterator_tag>, __gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter>*, std::__cxx1998::vector<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter>, std::allocator<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter> > > >, std::__debug::vector<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter> >, std::random_access_iterator_tag>, dwarf2_build_psymtabs_hard(dwarf2_per_objfile*)::<lambda(iter_type, iter_type)>, function_view<long unsigned int(__gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter>*, std::__cxx1998::vector<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter>, std::allocator<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter> > > >, std::__debug::vector<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter> >, std::random_access_iterator_tag>)>)::<lambda()>&> /usr/include/c++/12/bits/invoke.h:116
#7 0x55f86e4eda44 in _M_invoke /usr/include/c++/12/bits/std_function.h:291
#8 0x55f86d388cbf (/home/jenkins/workspace/binutils-gdb_master_linuxbuild/platform/deb12-amd64/target_board/unix/tmp/tmp.Rud1Y0ryoD/gdb/gdb+0x5896cbf)
Thread T5 created by T0 here:
#0 0x7fabfbe7e726 in __interceptor_pthread_create ../../../../src/libsanitizer/asan/asan_interceptors.cpp:207
#1 0x7fabfaff4578 in std::thread::_M_start_thread(std::unique_ptr<std::thread::_State, std::default_delete<std::thread::_State> >, void (*)()) (/lib/x86_64-linux-gnu/libstdc++.so.6+0xd4578)
SUMMARY: AddressSanitizer: heap-use-after-free /home/jenkins/workspace/binutils-gdb_master_linuxbuild/platform/deb12-amd64/target_board/unix/src/binutils-gdb/libiberty/hashtab.c:838 in htab_hash_string
Shadow bytes around the buggy address:
0x0c2a7fff9fa0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c2a7fff9fb0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c2a7fff9fc0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c2a7fff9fd0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c2a7fff9fe0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c2a7fff9ff0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd[fd]
0x0c2a7fffa000: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x0c2a7fffa010: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x0c2a7fffa020: fd fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa
0x0c2a7fffa030: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c2a7fffa040: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Partially addressable: 01 02 03 04 05 06 07
Heap left redzone: fa
Freed heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack after return: f5
Stack use after scope: f8
Global redzone: f9
Global init order: f6
Poisoned by user: f7
Container overflow: fc
Array cookie: ac
Intra object redzone: bb
ASan internal: fe
Left alloca redzone: ca
Right alloca redzone: cb
==3166157==ABORTING
Simon
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH v4 01/19] Don't use objfile::intern in DWO code
2024-01-09 18:44 ` Simon Marchi
@ 2024-01-09 19:12 ` Tom Tromey
2024-01-09 19:51 ` Tom Tromey
0 siblings, 1 reply; 25+ messages in thread
From: Tom Tromey @ 2024-01-09 19:12 UTC (permalink / raw)
To: Simon Marchi; +Cc: Tom Tromey, gdb-patches
>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:
Simon> On 12/10/23 16:41, Tom Tromey wrote:
>> The DWO code in the DWARF reader currently uses objfile::intern. This
>> accesses a shared data structure and so would be racy when used from
>> multiple threads. I don't believe this can happen right now, but
>> background reading could provoke this, and in any case it's better to
>> avoid this, just to be sure.
>>
>> This patch changes this code to just use a std::string. A new type is
>> introduced to do hash table lookups, to avoid unnecessary copies.
Simon> Hi Tom,
Simon> I am getting some ASan-related failures on my CI, starting with this
Simon> patch. I had trouble reproducing on my machine, but I got it to
Simon> reproduce when using the exact source and build paths that my CI job
Simon> used (since the patch is related to how strings (including paths) are
Simon> handled, it's possible that the bug requires some specific strings in
Simon> order to show up).
Sorry about that. I will try to investigate.
It's probably this though:
https://bholley.net/blog/2015/must-be-this-tall-to-write-multi-threaded-code.html
Tom
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v4 02/19] Pre-read DWZ section data
2023-12-10 21:41 [PATCH v4 00/19] Index DWARF in the background Tom Tromey
2023-12-10 21:41 ` [PATCH v4 01/19] Don't use objfile::intern in DWO code Tom Tromey
@ 2023-12-10 21:41 ` Tom Tromey
2023-12-10 21:41 ` [PATCH v4 03/19] Add a couple of bfd_cache_close calls Tom Tromey
` (17 subsequent siblings)
19 siblings, 0 replies; 25+ messages in thread
From: Tom Tromey @ 2023-12-10 21:41 UTC (permalink / raw)
To: gdb-patches
This changes the DWZ code to pre-read the section data and somewhat
simplify the DWZ API. This makes it easier to add the bfd_cache_close
call to the new dwarf2_read_dwz_file function -- after this is done,
there shouldn't be a reason to keep the BFD's file descriptor open.
---
gdb/dwarf2/dwz.c | 90 ++++++++++++++++++++-----------------------
gdb/dwarf2/dwz.h | 13 +++++--
gdb/dwarf2/macro.c | 2 -
gdb/dwarf2/read-debug-names.c | 2 +-
gdb/dwarf2/read.c | 13 +------
gdb/dwarf2/read.h | 3 +-
6 files changed, 56 insertions(+), 67 deletions(-)
diff --git a/gdb/dwarf2/dwz.c b/gdb/dwarf2/dwz.c
index edd5b56d778..13f95801ebb 100644
--- a/gdb/dwarf2/dwz.c
+++ b/gdb/dwarf2/dwz.c
@@ -53,49 +53,35 @@ dwz_file::read_string (struct objfile *objfile, LONGEST str_offset)
/* A helper function to find the sections for a .dwz file. */
static void
-locate_dwz_sections (bfd *abfd, asection *sectp, dwz_file *dwz_file)
+locate_dwz_sections (struct objfile *objfile, bfd *abfd, asection *sectp,
+ dwz_file *dwz_file)
{
+ dwarf2_section_info *sect = nullptr;
+
/* Note that we only support the standard ELF names, because .dwz
is ELF-only (at the time of writing). */
if (dwarf2_elf_names.abbrev.matches (sectp->name))
- {
- dwz_file->abbrev.s.section = sectp;
- dwz_file->abbrev.size = bfd_section_size (sectp);
- }
+ sect = &dwz_file->abbrev;
else if (dwarf2_elf_names.info.matches (sectp->name))
- {
- dwz_file->info.s.section = sectp;
- dwz_file->info.size = bfd_section_size (sectp);
- }
+ sect = &dwz_file->info;
else if (dwarf2_elf_names.str.matches (sectp->name))
- {
- dwz_file->str.s.section = sectp;
- dwz_file->str.size = bfd_section_size (sectp);
- }
+ sect = &dwz_file->str;
else if (dwarf2_elf_names.line.matches (sectp->name))
- {
- dwz_file->line.s.section = sectp;
- dwz_file->line.size = bfd_section_size (sectp);
- }
+ sect = &dwz_file->line;
else if (dwarf2_elf_names.macro.matches (sectp->name))
- {
- dwz_file->macro.s.section = sectp;
- dwz_file->macro.size = bfd_section_size (sectp);
- }
+ sect = &dwz_file->macro;
else if (dwarf2_elf_names.gdb_index.matches (sectp->name))
- {
- dwz_file->gdb_index.s.section = sectp;
- dwz_file->gdb_index.size = bfd_section_size (sectp);
- }
+ sect = &dwz_file->gdb_index;
else if (dwarf2_elf_names.debug_names.matches (sectp->name))
- {
- dwz_file->debug_names.s.section = sectp;
- dwz_file->debug_names.size = bfd_section_size (sectp);
- }
+ sect = &dwz_file->debug_names;
else if (dwarf2_elf_names.types.matches (sectp->name))
+ sect = &dwz_file->types;
+
+ if (sect != nullptr)
{
- dwz_file->types.s.section = sectp;
- dwz_file->types.size = bfd_section_size (sectp);
+ sect->s.section = sectp;
+ sect->size = bfd_section_size (sectp);
+ sect->read (objfile);
}
}
@@ -190,20 +176,13 @@ dwz_search_other_debugdirs (std::string &filename, bfd_byte *buildid,
/* See dwz.h. */
-struct dwz_file *
-dwarf2_get_dwz_file (dwarf2_per_bfd *per_bfd, bool require)
+void
+dwarf2_read_dwz_file (dwarf2_per_objfile *per_objfile)
{
bfd_size_type buildid_len_arg;
size_t buildid_len;
bfd_byte *buildid;
-
- if (per_bfd->dwz_file.has_value ())
- {
- dwz_file *result = per_bfd->dwz_file->get ();
- if (require && result == nullptr)
- error (_("could not read '.gnu_debugaltlink' section"));
- return result;
- }
+ dwarf2_per_bfd *per_bfd = per_objfile->per_bfd;
/* This may query the user via the debuginfod support, so it may
only be run in the main thread. */
@@ -219,11 +198,7 @@ dwarf2_get_dwz_file (dwarf2_per_bfd *per_bfd, bool require)
if (data == NULL)
{
if (bfd_get_error () == bfd_error_no_error)
- {
- if (!require)
- return nullptr;
- error (_("could not read '.gnu_debugaltlink' section"));
- }
+ return;
error (_("could not read '.gnu_debugaltlink' section: %s"),
bfd_errmsg (bfd_get_error ()));
}
@@ -292,9 +267,28 @@ dwarf2_get_dwz_file (dwarf2_per_bfd *per_bfd, bool require)
(new struct dwz_file (std::move (dwz_bfd)));
for (asection *sec : gdb_bfd_sections (result->dwz_bfd))
- locate_dwz_sections (result->dwz_bfd.get (), sec, result.get ());
+ locate_dwz_sections (per_objfile->objfile, result->dwz_bfd.get (),
+ sec, result.get ());
gdb_bfd_record_inclusion (per_bfd->obfd, result->dwz_bfd.get ());
+ bfd_cache_close (result->dwz_bfd.get ());
+
per_bfd->dwz_file = std::move (result);
- return per_bfd->dwz_file->get ();
+}
+
+/* See dwz.h. */
+
+struct dwz_file *
+dwarf2_get_dwz_file (dwarf2_per_bfd *per_bfd, bool require)
+{
+ gdb_assert (!require || per_bfd->dwz_file.has_value ());
+
+ dwz_file *result = nullptr;
+ if (per_bfd->dwz_file.has_value ())
+ {
+ result = per_bfd->dwz_file->get ();
+ if (require && result == nullptr)
+ error (_("could not read '.gnu_debugaltlink' section"));
+ }
+ return result;
}
diff --git a/gdb/dwarf2/dwz.h b/gdb/dwarf2/dwz.h
index 5f61b5e1216..a427cda902d 100644
--- a/gdb/dwarf2/dwz.h
+++ b/gdb/dwarf2/dwz.h
@@ -65,13 +65,20 @@ struct dwz_file
const char *read_string (struct objfile *objfile, LONGEST str_offset);
};
-/* Open the separate '.dwz' debug file, if needed. If there is no
+/* Return the separate '.dwz' debug file. If there is no
.gnu_debugaltlink section in the file, then the result depends on
REQUIRE: if REQUIRE is true, then error; if REQUIRE is false,
- return NULL. Always error if there is such a section but the file
- cannot be found. */
+ return NULL. */
extern dwz_file *dwarf2_get_dwz_file (dwarf2_per_bfd *per_bfd,
bool require = false);
+/* Open the separate '.dwz' debug file, if needed. This just sets the
+ appropriate field in the per-BFD structure. If the DWZ file
+ exists, the relevant sections are read in as well. Throws an error
+ if the .gnu_debugaltlink section exists but the file cannot be
+ found. */
+
+extern void dwarf2_read_dwz_file (dwarf2_per_objfile *per_objfile);
+
#endif /* GDB_DWARF2_DWZ_H */
diff --git a/gdb/dwarf2/macro.c b/gdb/dwarf2/macro.c
index 7d86d16d0e3..2ee9f8c1009 100644
--- a/gdb/dwarf2/macro.c
+++ b/gdb/dwarf2/macro.c
@@ -735,8 +735,6 @@ dwarf_decode_macro_bytes (dwarf2_per_objfile *per_objfile,
dwz_file *dwz = dwarf2_get_dwz_file (per_objfile->per_bfd,
true);
- dwz->macro.read (objfile);
-
include_section = &dwz->macro;
include_bfd = include_section->get_bfd_owner ();
include_mac_end = dwz->macro.buffer + dwz->macro.size;
diff --git a/gdb/dwarf2/read-debug-names.c b/gdb/dwarf2/read-debug-names.c
index 59c49da5553..e912c2a1114 100644
--- a/gdb/dwarf2/read-debug-names.c
+++ b/gdb/dwarf2/read-debug-names.c
@@ -467,7 +467,7 @@ dwarf2_read_debug_names (dwarf2_per_objfile *per_objfile)
}
}
- create_all_units (per_objfile, false);
+ create_all_units (per_objfile);
if (!check_cus_from_debug_names (per_bfd, *map, dwz_map))
{
per_bfd->all_units.clear ();
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 9a5f52d0f7e..9fd61eb0777 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -3201,7 +3201,7 @@ dwarf2_initialize_objfile (struct objfile *objfile)
main thread. */
try
{
- dwarf2_get_dwz_file (per_bfd);
+ dwarf2_read_dwz_file (per_objfile);
}
catch (const gdb_exception_error &err)
{
@@ -5101,7 +5101,7 @@ finalize_all_units (dwarf2_per_bfd *per_bfd)
/* See read.h. */
void
-create_all_units (dwarf2_per_objfile *per_objfile, bool pre_read_p)
+create_all_units (dwarf2_per_objfile *per_objfile)
{
htab_up types_htab;
gdb_assert (per_objfile->per_bfd->all_units.empty ());
@@ -5117,15 +5117,6 @@ create_all_units (dwarf2_per_objfile *per_objfile, bool pre_read_p)
dwz_file *dwz = dwarf2_get_dwz_file (per_objfile->per_bfd);
if (dwz != NULL)
{
- if (pre_read_p)
- {
- /* Pre-read the sections we'll need to construct an index. */
- struct objfile *objfile = per_objfile->objfile;
- dwz->abbrev.read (objfile);
- dwz->info.read (objfile);
- dwz->str.read (objfile);
- dwz->line.read (objfile);
- }
read_comp_units_from_section (per_objfile, &dwz->info, &dwz->abbrev, 1,
types_htab, rcuh_kind::COMPILE);
diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
index 6f97f641c8e..4ab869cdd29 100644
--- a/gdb/dwarf2/read.h
+++ b/gdb/dwarf2/read.h
@@ -966,8 +966,7 @@ extern void finalize_all_units (dwarf2_per_bfd *per_bfd);
/* Create a list of all compilation units in OBJFILE. */
-extern void create_all_units (dwarf2_per_objfile *per_objfile,
- bool pre_read_p = true);
+extern void create_all_units (dwarf2_per_objfile *per_objfile);
/* Create a quick_file_names hash table. */
--
2.43.0
^ permalink raw reply [flat|nested] 25+ messages in thread* [PATCH v4 03/19] Add a couple of bfd_cache_close calls
2023-12-10 21:41 [PATCH v4 00/19] Index DWARF in the background Tom Tromey
2023-12-10 21:41 ` [PATCH v4 01/19] Don't use objfile::intern in DWO code Tom Tromey
2023-12-10 21:41 ` [PATCH v4 02/19] Pre-read DWZ section data Tom Tromey
@ 2023-12-10 21:41 ` Tom Tromey
2023-12-10 21:41 ` [PATCH v4 04/19] Add thread-safety to gdb's BFD wrappers Tom Tromey
` (16 subsequent siblings)
19 siblings, 0 replies; 25+ messages in thread
From: Tom Tromey @ 2023-12-10 21:41 UTC (permalink / raw)
To: gdb-patches
This adds a couple of calls to bfd_cache_close at points where a BFD
isn't actively needed by gdb. Normally at these points, all the
needed section data is already mapped, so we can simply close the file
descriptor. This is harmless at worst, because if this is needed
after all, the BFD file descriptor cache will reopen it.
---
gdb/dwarf2/read.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 9fd61eb0777..ba58dae12bb 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -9170,6 +9170,8 @@ open_and_init_dwo_file (dwarf2_cu *cu, const char *dwo_name,
dwarf_read_debug_printf ("DWO file found: %s", dwo_name);
+ bfd_cache_close (dwo_file->dbfd.get ());
+
return dwo_file.release ();
}
@@ -9462,6 +9464,8 @@ open_and_init_dwp_file (dwarf2_per_objfile *per_objfile)
pulongest (dwp_file->cus ? dwp_file->cus->nr_units : 0),
pulongest (dwp_file->tus ? dwp_file->tus->nr_units : 0));
+ bfd_cache_close (dwp_file->dbfd.get ());
+
return dwp_file;
}
--
2.43.0
^ permalink raw reply [flat|nested] 25+ messages in thread* [PATCH v4 04/19] Add thread-safety to gdb's BFD wrappers
2023-12-10 21:41 [PATCH v4 00/19] Index DWARF in the background Tom Tromey
` (2 preceding siblings ...)
2023-12-10 21:41 ` [PATCH v4 03/19] Add a couple of bfd_cache_close calls Tom Tromey
@ 2023-12-10 21:41 ` Tom Tromey
2023-12-10 21:41 ` [PATCH v4 05/19] Refactor complaint thread-safety approach Tom Tromey
` (15 subsequent siblings)
19 siblings, 0 replies; 25+ messages in thread
From: Tom Tromey @ 2023-12-10 21:41 UTC (permalink / raw)
To: gdb-patches
This changes gdb to ensure that gdb's BFD cache is guarded by a lock.
This avoids any races when multiple threads might open a BFD (and thus
use the BFD cache) at the same time.
Currently, this change is not needed because the the main thread waits
for some DWARF scanning to be completed before returning. The only
locking that's required is when opening DWO files, and there's a local
lock to this end in dwarf2/read.c.
However, in the coming patches, the DWARF reader will begin its work
earlier, in the background. This means there is the potential for the
DWARF reader and other code on the main thread to both attempt to open
BFDs at the same time.
---
gdb/dwarf2/read.c | 3 +--
gdb/gdb_bfd.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
gdb/gdb_bfd.h | 5 +++++
gdb/main.c | 3 +--
4 files changed, 65 insertions(+), 4 deletions(-)
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index ba58dae12bb..78acbd39ef1 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -3920,8 +3920,7 @@ static struct dwo_unit *
lookup_dwo_unit (dwarf2_cu *cu, die_info *comp_unit_die, const char *dwo_name)
{
#if CXX_STD_THREAD
- /* We need a lock here both to handle the DWO hash table, and BFD,
- which is not thread-safe. */
+ /* We need a lock here both to handle the DWO hash table. */
static std::mutex dwo_lock;
std::lock_guard<std::mutex> guard (dwo_lock);
diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c
index 1c496c498ac..9f7d0f626c0 100644
--- a/gdb/gdb_bfd.c
+++ b/gdb/gdb_bfd.c
@@ -35,6 +35,36 @@
#include "cli/cli-style.h"
#include <unordered_map>
+#if CXX_STD_THREAD
+
+#include <mutex>
+
+/* Lock held when doing BFD operations. A recursive mutex is used
+ because we use this mutex internally and also for BFD, just to make
+ life a bit simpler, and we may sometimes hold it while calling into
+ BFD. */
+static std::recursive_mutex gdb_bfd_mutex;
+
+/* BFD locking function. */
+
+static bool
+gdb_bfd_lock (void *ignore)
+{
+ gdb_bfd_mutex.lock ();
+ return true;
+}
+
+/* BFD unlocking function. */
+
+static bool
+gdb_bfd_unlock (void *ignore)
+{
+ gdb_bfd_mutex.unlock ();
+ return true;
+}
+
+#endif /* CXX_STD_THREAD */
+
/* An object of this type is stored in the section's user data when
mapping a section. */
@@ -498,6 +528,10 @@ gdb_bfd_open (const char *name, const char *target, int fd,
name += strlen (TARGET_SYSROOT_PREFIX);
}
+#if CXX_STD_THREAD
+ std::lock_guard<std::recursive_mutex> guard (gdb_bfd_mutex);
+#endif
+
if (gdb_bfd_cache == NULL)
gdb_bfd_cache = htab_create_alloc (1, hash_bfd, eq_bfd, NULL,
xcalloc, xfree);
@@ -625,6 +659,10 @@ gdb_bfd_ref (struct bfd *abfd)
if (abfd == NULL)
return;
+#if CXX_STD_THREAD
+ std::lock_guard<std::recursive_mutex> guard (gdb_bfd_mutex);
+#endif
+
gdata = (struct gdb_bfd_data *) bfd_usrdata (abfd);
bfd_cache_debug_printf ("Increase reference count on bfd %s (%s)",
@@ -654,6 +692,10 @@ gdb_bfd_unref (struct bfd *abfd)
if (abfd == NULL)
return;
+#if CXX_STD_THREAD
+ std::lock_guard<std::recursive_mutex> guard (gdb_bfd_mutex);
+#endif
+
gdata = (struct gdb_bfd_data *) bfd_usrdata (abfd);
gdb_assert (gdata->refc >= 1);
@@ -1171,6 +1213,22 @@ gdb_bfd_error_handler (const char *fmt, va_list ap)
(*default_bfd_error_handler) (fmt, ap);
}
+/* See gdb_bfd.h. */
+
+void
+gdb_bfd_init ()
+{
+ if (bfd_init () == BFD_INIT_MAGIC)
+ {
+#if CXX_STD_THREAD
+ if (bfd_thread_init (gdb_bfd_lock, gdb_bfd_unlock, nullptr))
+#endif
+ return;
+ }
+
+ error (_("fatal error: libbfd ABI mismatch"));
+}
+
void _initialize_gdb_bfd ();
void
_initialize_gdb_bfd ()
diff --git a/gdb/gdb_bfd.h b/gdb/gdb_bfd.h
index 604365b61b1..669c3a06c98 100644
--- a/gdb/gdb_bfd.h
+++ b/gdb/gdb_bfd.h
@@ -250,4 +250,9 @@ gdb_bfd_sections (const gdb_bfd_ref_ptr &abfd)
extern std::string gdb_bfd_errmsg (bfd_error_type error_tag, char **matching);
+/* A wrapper for bfd_init that also handles setting up for
+ multi-threading. */
+
+extern void gdb_bfd_init ();
+
#endif /* GDB_BFD_H */
diff --git a/gdb/main.c b/gdb/main.c
index 688db7655a9..67c7a52f54f 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -699,8 +699,7 @@ captured_main_1 (struct captured_main_args *context)
auto temp_uiout = std::make_unique<cli_ui_out> (gdb_stdout);
current_uiout = temp_uiout.get ();
- if (bfd_init () != BFD_INIT_MAGIC)
- error (_("fatal error: libbfd ABI mismatch"));
+ gdb_bfd_init ();
#ifdef __MINGW32__
/* On Windows, argv[0] is not necessarily set to absolute form when
--
2.43.0
^ permalink raw reply [flat|nested] 25+ messages in thread* [PATCH v4 05/19] Refactor complaint thread-safety approach
2023-12-10 21:41 [PATCH v4 00/19] Index DWARF in the background Tom Tromey
` (3 preceding siblings ...)
2023-12-10 21:41 ` [PATCH v4 04/19] Add thread-safety to gdb's BFD wrappers Tom Tromey
@ 2023-12-10 21:41 ` Tom Tromey
2023-12-10 21:41 ` [PATCH v4 06/19] Add deferred_warnings parameter to read_addrmap_from_aranges Tom Tromey
` (14 subsequent siblings)
19 siblings, 0 replies; 25+ messages in thread
From: Tom Tromey @ 2023-12-10 21:41 UTC (permalink / raw)
To: gdb-patches
This patch changes the way complaint works in a background thread.
The new approach requires installing a complaint interceptor in each
worker, and then the resulting complaints are treated as one of the
results of the computation. This change is needed for a subsequent
patch, where installing a complaint interceptor around a parallel-for
is no longer a viable approach.
---
gdb/complaints.c | 24 +++++++++++-------------
gdb/complaints.h | 37 +++++++++++++++++++++++++++++--------
gdb/defs.h | 2 +-
gdb/dwarf2/read.c | 31 ++++++++++++++++++-------------
gdb/top.c | 2 +-
5 files changed, 60 insertions(+), 36 deletions(-)
diff --git a/gdb/complaints.c b/gdb/complaints.c
index 302f107b519..eb648c655ed 100644
--- a/gdb/complaints.c
+++ b/gdb/complaints.c
@@ -21,6 +21,7 @@
#include "complaints.h"
#include "command.h"
#include "gdbcmd.h"
+#include "run-on-main-thread.h"
#include "gdbsupport/selftest.h"
#include <unordered_map>
#include <mutex>
@@ -78,17 +79,14 @@ clear_complaints ()
/* See complaints.h. */
-complaint_interceptor *complaint_interceptor::g_complaint_interceptor;
+thread_local complaint_interceptor *complaint_interceptor::g_complaint_interceptor;
/* See complaints.h. */
complaint_interceptor::complaint_interceptor ()
- : m_saved_warning_hook (deprecated_warning_hook)
+ : m_saved_warning_hook (&deprecated_warning_hook, issue_complaint),
+ m_saved_complaint_interceptor (&g_complaint_interceptor, this)
{
- /* These cannot be stacked. */
- gdb_assert (g_complaint_interceptor == nullptr);
- g_complaint_interceptor = this;
- deprecated_warning_hook = issue_complaint;
}
/* A helper that wraps a warning hook. */
@@ -104,19 +102,19 @@ wrap_warning_hook (void (*hook) (const char *, va_list), ...)
/* See complaints.h. */
-complaint_interceptor::~complaint_interceptor ()
+void
+re_emit_complaints (const complaint_collection &complaints)
{
- for (const std::string &str : m_complaints)
+ gdb_assert (is_main_thread ());
+
+ for (const std::string &str : complaints)
{
- if (m_saved_warning_hook)
- wrap_warning_hook (m_saved_warning_hook, str.c_str ());
+ if (deprecated_warning_hook)
+ wrap_warning_hook (deprecated_warning_hook, str.c_str ());
else
gdb_printf (gdb_stderr, _("During symbol reading: %s\n"),
str.c_str ());
}
-
- g_complaint_interceptor = nullptr;
- deprecated_warning_hook = m_saved_warning_hook;
}
/* See complaints.h. */
diff --git a/gdb/complaints.h b/gdb/complaints.h
index e9e846684ac..1626f200685 100644
--- a/gdb/complaints.h
+++ b/gdb/complaints.h
@@ -57,29 +57,45 @@ have_complaint ()
extern void clear_complaints ();
+/* Type of collected complaints. */
+
+typedef std::unordered_set<std::string> complaint_collection;
+
/* A class that can handle calls to complaint from multiple threads.
When this is instantiated, it hooks into the complaint mechanism,
- so the 'complaint' macro can continue to be used. When it is
- destroyed, it issues all the complaints that have been stored. It
- should only be instantiated in the main thread. */
+ so the 'complaint' macro can continue to be used. It collects all
+ the complaints (and warnings) emitted while it is installed, and
+ these can be retrieved before the object is destroyed. This is
+ intended for use from worker threads (though it will also work on
+ the main thread). Messages can be re-emitted on the main thread
+ using re_emit_complaints, see below. */
class complaint_interceptor
{
public:
complaint_interceptor ();
- ~complaint_interceptor ();
+ ~complaint_interceptor () = default;
DISABLE_COPY_AND_ASSIGN (complaint_interceptor);
+ complaint_collection &&release ()
+ {
+ return std::move (m_complaints);
+ }
+
private:
/* The issued complaints. */
- std::unordered_set<std::string> m_complaints;
+ complaint_collection m_complaints;
+
+ typedef void (*saved_warning_hook_ftype) (const char *, va_list);
/* The saved value of deprecated_warning_hook. */
- void (*m_saved_warning_hook) (const char *, va_list)
- ATTRIBUTE_FPTR_PRINTF (1,0);
+ scoped_restore_tmpl<saved_warning_hook_ftype> m_saved_warning_hook;
+
+ /* The saved value of g_complaint_interceptor. */
+ scoped_restore_tmpl<complaint_interceptor *> m_saved_complaint_interceptor;
/* A helper function that is used by the 'complaint' implementation
to issue a complaint. */
@@ -87,7 +103,12 @@ class complaint_interceptor
ATTRIBUTE_PRINTF (1, 0);
/* This object. Used by the static callback function. */
- static complaint_interceptor *g_complaint_interceptor;
+ static thread_local complaint_interceptor *g_complaint_interceptor;
};
+/* Re-emit complaints that were collected by complaint_interceptor.
+ This can only be called on the main thread. */
+
+extern void re_emit_complaints (const complaint_collection &);
+
#endif /* !defined (COMPLAINTS_H) */
diff --git a/gdb/defs.h b/gdb/defs.h
index bcce4f4c3e4..e73a82402c9 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -539,7 +539,7 @@ extern void (*deprecated_print_frame_info_listing_hook) (struct symtab * s,
int noerror);
extern int (*deprecated_query_hook) (const char *, va_list)
ATTRIBUTE_FPTR_PRINTF(1,0);
-extern void (*deprecated_warning_hook) (const char *, va_list)
+extern thread_local void (*deprecated_warning_hook) (const char *, va_list)
ATTRIBUTE_FPTR_PRINTF(1,0);
extern void (*deprecated_readline_begin_hook) (const char *, ...)
ATTRIBUTE_FPTR_PRINTF_1;
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 78acbd39ef1..bd45c2c9170 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -4930,9 +4930,6 @@ dwarf2_build_psymtabs_hard (dwarf2_per_objfile *per_objfile)
index_storage.get_addrmap ());
{
- /* Ensure that complaints are handled correctly. */
- complaint_interceptor complaint_handler;
-
using iter_type = decltype (per_bfd->all_units.begin ());
auto task_size_ = [] (iter_type iter)
@@ -4942,18 +4939,23 @@ dwarf2_build_psymtabs_hard (dwarf2_per_objfile *per_objfile)
};
auto task_size = gdb::make_function_view (task_size_);
- /* Each thread returns a pair holding a cooked index, and a vector
- of errors that should be printed. The latter is done because
- GDB's I/O system is not thread-safe. run_on_main_thread could be
- used, but that would mean the messages are printed after the
- prompt, which looks weird. */
- using result_type = std::pair<std::unique_ptr<cooked_index_shard>,
- std::vector<gdb_exception>>;
+ /* Each thread returns a tuple holding a cooked index, any
+ collected complaints, and a vector of errors that should be
+ printed. The latter is done because GDB's I/O system is not
+ thread-safe. run_on_main_thread could be used, but that would
+ mean the messages are printed after the prompt, which looks
+ weird. */
+ using result_type = std::tuple<std::unique_ptr<cooked_index_shard>,
+ complaint_collection,
+ std::vector<gdb_exception>>;
std::vector<result_type> results
= gdb::parallel_for_each (1, per_bfd->all_units.begin (),
per_bfd->all_units.end (),
[=] (iter_type iter, iter_type end)
{
+ /* Ensure that complaints are handled correctly. */
+ complaint_interceptor complaint_handler;
+
std::vector<gdb_exception> errors;
cooked_index_storage thread_storage;
for (; iter != end; ++iter)
@@ -4969,15 +4971,18 @@ dwarf2_build_psymtabs_hard (dwarf2_per_objfile *per_objfile)
errors.push_back (std::move (except));
}
}
- return result_type (thread_storage.release (), std::move (errors));
+ return result_type (thread_storage.release (),
+ complaint_handler.release (),
+ std::move (errors));
}, task_size);
/* Only show a given exception a single time. */
std::unordered_set<gdb_exception> seen_exceptions;
for (auto &one_result : results)
{
- indexes.push_back (std::move (one_result.first));
- for (auto &one_exc : one_result.second)
+ indexes.push_back (std::move (std::get<0> (one_result)));
+ re_emit_complaints (std::get<1> (one_result));
+ for (auto &one_exc : std::get<2> (one_result))
if (seen_exceptions.insert (one_exc).second)
exception_print (gdb_stderr, one_exc);
}
diff --git a/gdb/top.c b/gdb/top.c
index d211f1b08be..009bf2b0c1c 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -221,7 +221,7 @@ int (*deprecated_query_hook) (const char *, va_list);
/* Replaces most of warning. */
-void (*deprecated_warning_hook) (const char *, va_list);
+thread_local void (*deprecated_warning_hook) (const char *, va_list);
/* These three functions support getting lines of text from the user.
They are used in sequence. First deprecated_readline_begin_hook is
--
2.43.0
^ permalink raw reply [flat|nested] 25+ messages in thread* [PATCH v4 06/19] Add deferred_warnings parameter to read_addrmap_from_aranges
2023-12-10 21:41 [PATCH v4 00/19] Index DWARF in the background Tom Tromey
` (4 preceding siblings ...)
2023-12-10 21:41 ` [PATCH v4 05/19] Refactor complaint thread-safety approach Tom Tromey
@ 2023-12-10 21:41 ` Tom Tromey
2023-12-10 21:41 ` [PATCH v4 07/19] Add quick_symbol_functions::compute_main_name Tom Tromey
` (13 subsequent siblings)
19 siblings, 0 replies; 25+ messages in thread
From: Tom Tromey @ 2023-12-10 21:41 UTC (permalink / raw)
To: gdb-patches
When DWARF reading is done in the background,
read_addrmap_from_aranges will be called from a worker thread.
Because warnings can't be emitted from these threads, this patch adds
a new deferred_warnings parameter to the function, letting the caller
control exactly how the warnings are emitted.
---
gdb/dwarf2/aranges.c | 77 ++++++++++++++++++++++---------------------
gdb/dwarf2/aranges.h | 3 +-
gdb/dwarf2/read-debug-names.c | 6 +++-
gdb/dwarf2/read.c | 8 +++--
gdb/utils.h | 16 ++++++---
5 files changed, 65 insertions(+), 45 deletions(-)
diff --git a/gdb/dwarf2/aranges.c b/gdb/dwarf2/aranges.c
index 06f49a494ba..b7d050b73f6 100644
--- a/gdb/dwarf2/aranges.c
+++ b/gdb/dwarf2/aranges.c
@@ -26,7 +26,8 @@
bool
read_addrmap_from_aranges (dwarf2_per_objfile *per_objfile,
dwarf2_section_info *section,
- addrmap *mutable_map)
+ addrmap *mutable_map,
+ deferred_warnings *warn)
{
/* Caller must ensure that the section has already been read. */
gdb_assert (section->readin);
@@ -76,13 +77,13 @@ read_addrmap_from_aranges (dwarf2_per_objfile *per_objfile,
const uint8_t offset_size = dwarf5_is_dwarf64 ? 8 : 4;
if (addr + entry_length > section->buffer + section->size)
{
- warning (_("Section .debug_aranges in %s entry at offset %s "
- "length %s exceeds section length %s, "
- "ignoring .debug_aranges."),
- objfile_name (objfile),
- plongest (entry_addr - section->buffer),
- plongest (bytes_read + entry_length),
- pulongest (section->size));
+ warn->warn (_("Section .debug_aranges in %s entry at offset %s "
+ "length %s exceeds section length %s, "
+ "ignoring .debug_aranges."),
+ objfile_name (objfile),
+ plongest (entry_addr - section->buffer),
+ plongest (bytes_read + entry_length),
+ pulongest (section->size));
return false;
}
@@ -91,10 +92,11 @@ read_addrmap_from_aranges (dwarf2_per_objfile *per_objfile,
addr += 2;
if (version != 2)
{
- warning (_("Section .debug_aranges in %s entry at offset %s "
- "has unsupported version %d, ignoring .debug_aranges."),
- objfile_name (objfile),
- plongest (entry_addr - section->buffer), version);
+ warn->warn
+ (_("Section .debug_aranges in %s entry at offset %s "
+ "has unsupported version %d, ignoring .debug_aranges."),
+ objfile_name (objfile),
+ plongest (entry_addr - section->buffer), version);
return false;
}
@@ -105,22 +107,22 @@ read_addrmap_from_aranges (dwarf2_per_objfile *per_objfile,
= debug_info_offset_to_per_cu.find (sect_offset (debug_info_offset));
if (per_cu_it == debug_info_offset_to_per_cu.cend ())
{
- warning (_("Section .debug_aranges in %s entry at offset %s "
- "debug_info_offset %s does not exists, "
- "ignoring .debug_aranges."),
- objfile_name (objfile),
- plongest (entry_addr - section->buffer),
- pulongest (debug_info_offset));
+ warn->warn (_("Section .debug_aranges in %s entry at offset %s "
+ "debug_info_offset %s does not exists, "
+ "ignoring .debug_aranges."),
+ objfile_name (objfile),
+ plongest (entry_addr - section->buffer),
+ pulongest (debug_info_offset));
return false;
}
const auto insertpair
= debug_info_offset_seen.insert (sect_offset (debug_info_offset));
if (!insertpair.second)
{
- warning (_("Section .debug_aranges in %s has duplicate "
- "debug_info_offset %s, ignoring .debug_aranges."),
- objfile_name (objfile),
- sect_offset_str (sect_offset (debug_info_offset)));
+ warn->warn (_("Section .debug_aranges in %s has duplicate "
+ "debug_info_offset %s, ignoring .debug_aranges."),
+ objfile_name (objfile),
+ sect_offset_str (sect_offset (debug_info_offset)));
return false;
}
dwarf2_per_cu_data *const per_cu = per_cu_it->second;
@@ -128,22 +130,23 @@ read_addrmap_from_aranges (dwarf2_per_objfile *per_objfile,
const uint8_t address_size = *addr++;
if (address_size < 1 || address_size > 8)
{
- warning (_("Section .debug_aranges in %s entry at offset %s "
- "address_size %u is invalid, ignoring .debug_aranges."),
- objfile_name (objfile),
- plongest (entry_addr - section->buffer), address_size);
+ warn->warn
+ (_("Section .debug_aranges in %s entry at offset %s "
+ "address_size %u is invalid, ignoring .debug_aranges."),
+ objfile_name (objfile),
+ plongest (entry_addr - section->buffer), address_size);
return false;
}
const uint8_t segment_selector_size = *addr++;
if (segment_selector_size != 0)
{
- warning (_("Section .debug_aranges in %s entry at offset %s "
- "segment_selector_size %u is not supported, "
- "ignoring .debug_aranges."),
- objfile_name (objfile),
- plongest (entry_addr - section->buffer),
- segment_selector_size);
+ warn->warn (_("Section .debug_aranges in %s entry at offset %s "
+ "segment_selector_size %u is not supported, "
+ "ignoring .debug_aranges."),
+ objfile_name (objfile),
+ plongest (entry_addr - section->buffer),
+ segment_selector_size);
return false;
}
@@ -160,11 +163,11 @@ read_addrmap_from_aranges (dwarf2_per_objfile *per_objfile,
{
if (addr + 2 * address_size > entry_end)
{
- warning (_("Section .debug_aranges in %s entry at offset %s "
- "address list is not properly terminated, "
- "ignoring .debug_aranges."),
- objfile_name (objfile),
- plongest (entry_addr - section->buffer));
+ warn->warn (_("Section .debug_aranges in %s entry at offset %s "
+ "address list is not properly terminated, "
+ "ignoring .debug_aranges."),
+ objfile_name (objfile),
+ plongest (entry_addr - section->buffer));
return false;
}
ULONGEST start = extract_unsigned_integer (addr, address_size,
diff --git a/gdb/dwarf2/aranges.h b/gdb/dwarf2/aranges.h
index 43e1cbd0930..e3e9cde8b5b 100644
--- a/gdb/dwarf2/aranges.h
+++ b/gdb/dwarf2/aranges.h
@@ -30,6 +30,7 @@ class addrmap;
extern bool read_addrmap_from_aranges (dwarf2_per_objfile *per_objfile,
dwarf2_section_info *section,
- addrmap *mutable_map);
+ addrmap *mutable_map,
+ deferred_warnings *warn);
#endif /* GDB_DWARF2_ARANGES_H */
diff --git a/gdb/dwarf2/read-debug-names.c b/gdb/dwarf2/read-debug-names.c
index e912c2a1114..0877bf3e778 100644
--- a/gdb/dwarf2/read-debug-names.c
+++ b/gdb/dwarf2/read-debug-names.c
@@ -153,12 +153,16 @@ create_addrmap_from_aranges (dwarf2_per_objfile *per_objfile,
dwarf2_per_bfd *per_bfd = per_objfile->per_bfd;
addrmap_mutable mutable_map;
+ deferred_warnings warnings;
section->read (per_objfile->objfile);
- if (read_addrmap_from_aranges (per_objfile, section, &mutable_map))
+ if (read_addrmap_from_aranges (per_objfile, section, &mutable_map,
+ &warnings))
per_bfd->index_addrmap
= new (&per_bfd->obstack) addrmap_fixed (&per_bfd->obstack,
&mutable_map);
+
+ warnings.emit ();
}
/* DWARF-5 debug_names reader. */
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index bd45c2c9170..5189a7e0c45 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -4926,8 +4926,12 @@ dwarf2_build_psymtabs_hard (dwarf2_per_objfile *per_objfile)
per_bfd->quick_file_names_table
= create_quick_file_names_table (per_bfd->all_units.size ());
if (!per_bfd->debug_aranges.empty ())
- read_addrmap_from_aranges (per_objfile, &per_bfd->debug_aranges,
- index_storage.get_addrmap ());
+ {
+ deferred_warnings warn;
+ read_addrmap_from_aranges (per_objfile, &per_bfd->debug_aranges,
+ index_storage.get_addrmap (), &warn);
+ warn.emit ();
+ }
{
using iter_type = decltype (per_bfd->all_units.begin ());
diff --git a/gdb/utils.h b/gdb/utils.h
index f646b300530..0acf11cf65f 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -395,13 +395,16 @@ assign_return_if_changed (T &lval, const T &val)
struct deferred_warnings
{
+ deferred_warnings ()
+ : m_can_style (gdb_stderr->can_emit_style_escape ())
+ {
+ }
+
/* Add a warning to the list of deferred warnings. */
void warn (const char *format, ...) ATTRIBUTE_PRINTF(2,3)
{
- /* Generate the warning text into a string_file. We allow the text to
- be styled only if gdb_stderr allows styling -- warnings are sent to
- gdb_stderr. */
- string_file msg (gdb_stderr->can_emit_style_escape ());
+ /* Generate the warning text into a string_file. */
+ string_file msg (m_can_style);
va_list args;
va_start (args, format);
@@ -421,6 +424,11 @@ struct deferred_warnings
private:
+ /* True if gdb_stderr supports styling at the moment this object is
+ constructed. This is done just once so that objects of this type
+ can be used off the main thread. */
+ bool m_can_style;
+
/* The list of all deferred warnings. */
std::vector<string_file> m_warnings;
};
--
2.43.0
^ permalink raw reply [flat|nested] 25+ messages in thread* [PATCH v4 07/19] Add quick_symbol_functions::compute_main_name
2023-12-10 21:41 [PATCH v4 00/19] Index DWARF in the background Tom Tromey
` (5 preceding siblings ...)
2023-12-10 21:41 ` [PATCH v4 06/19] Add deferred_warnings parameter to read_addrmap_from_aranges Tom Tromey
@ 2023-12-10 21:41 ` Tom Tromey
2023-12-10 21:41 ` [PATCH v4 08/19] Add gdb::task_group Tom Tromey
` (12 subsequent siblings)
19 siblings, 0 replies; 25+ messages in thread
From: Tom Tromey @ 2023-12-10 21:41 UTC (permalink / raw)
To: gdb-patches
This adds a new compute_main_name method to quick_symbol_functions.
Currently there are no implementations of this, but a subsequent patch
will add one.
---
gdb/objfiles.h | 3 +++
gdb/quick-symbol.h | 9 +++++++++
gdb/symfile-debug.c | 12 ++++++++++++
gdb/symtab.c | 2 ++
4 files changed, 26 insertions(+)
diff --git a/gdb/objfiles.h b/gdb/objfiles.h
index 0cad5961659..a7b5a71485e 100644
--- a/gdb/objfiles.h
+++ b/gdb/objfiles.h
@@ -605,6 +605,9 @@ struct objfile
void map_symbol_filenames (gdb::function_view<symbol_filename_ftype> fun,
bool need_fullname);
+ /* See quick_symbol_functions. */
+ void compute_main_name ();
+
/* See quick_symbol_functions. */
struct compunit_symtab *find_compunit_symtab_by_address (CORE_ADDR address);
diff --git a/gdb/quick-symbol.h b/gdb/quick-symbol.h
index e48eeeda972..56714ba4d6b 100644
--- a/gdb/quick-symbol.h
+++ b/gdb/quick-symbol.h
@@ -185,6 +185,15 @@ struct quick_symbol_functions
gdb::function_view<symbol_filename_ftype> fun,
bool need_fullname) = 0;
+ /* Compute the name and language of the main function for the given
+ objfile. Normally this is done during symbol reading, but this
+ method exists in case this work is done in a worker thread and
+ must be waited for. The implementation can call
+ set_objfile_main_name if results are found. */
+ virtual void compute_main_name (struct objfile *objfile)
+ {
+ }
+
/* Return true if this class can lazily read the symbols. This may
only return true if there are in fact symbols to be read, because
this is used in the implementation of 'has_partial_symbols'. */
diff --git a/gdb/symfile-debug.c b/gdb/symfile-debug.c
index d31ff3353ee..7b73b42b70d 100644
--- a/gdb/symfile-debug.c
+++ b/gdb/symfile-debug.c
@@ -467,6 +467,18 @@ objfile::map_symbol_filenames (gdb::function_view<symbol_filename_ftype> fun,
iter->map_symbol_filenames (this, fun, need_fullname);
}
+void
+objfile::compute_main_name ()
+{
+ if (debug_symfile)
+ gdb_printf (gdb_stdlog,
+ "qf->compute_main_name (%s)\n",
+ objfile_debug_name (this));
+
+ for (const auto &iter : qf_require_partial_symbols ())
+ iter->compute_main_name (this);
+}
+
struct compunit_symtab *
objfile::find_compunit_symtab_by_address (CORE_ADDR address)
{
diff --git a/gdb/symtab.c b/gdb/symtab.c
index d5d229f9562..0e648f26bff 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -6243,6 +6243,8 @@ find_main_name (void)
accurate. */
for (objfile *objfile : current_program_space->objfiles ())
{
+ objfile->compute_main_name ();
+
if (objfile->per_bfd->name_of_main != NULL)
{
set_main_name (pspace,
--
2.43.0
^ permalink raw reply [flat|nested] 25+ messages in thread* [PATCH v4 08/19] Add gdb::task_group
2023-12-10 21:41 [PATCH v4 00/19] Index DWARF in the background Tom Tromey
` (6 preceding siblings ...)
2023-12-10 21:41 ` [PATCH v4 07/19] Add quick_symbol_functions::compute_main_name Tom Tromey
@ 2023-12-10 21:41 ` Tom Tromey
2023-12-18 14:55 ` Tom Tromey
2023-12-10 21:41 ` [PATCH v4 09/19] Move cooked_index_storage to cooked-index.h Tom Tromey
` (11 subsequent siblings)
19 siblings, 1 reply; 25+ messages in thread
From: Tom Tromey @ 2023-12-10 21:41 UTC (permalink / raw)
To: gdb-patches
This adds gdb::task_group, a convenient way to group background tasks
and then call a function when all the tasks have completed.
---
gdbsupport/Makefile.am | 1 +
gdbsupport/Makefile.in | 6 ++-
gdbsupport/task-group.cc | 97 ++++++++++++++++++++++++++++++++++++++++++++++++
gdbsupport/task-group.h | 64 ++++++++++++++++++++++++++++++++
4 files changed, 166 insertions(+), 2 deletions(-)
diff --git a/gdbsupport/Makefile.am b/gdbsupport/Makefile.am
index f1a641308fe..fdca034ce5e 100644
--- a/gdbsupport/Makefile.am
+++ b/gdbsupport/Makefile.am
@@ -79,6 +79,7 @@ libgdbsupport_a_SOURCES = \
search.cc \
signals.cc \
signals-state-save-restore.cc \
+ task-group.cc \
tdesc.cc \
thread-pool.cc \
xml-utils.cc \
diff --git a/gdbsupport/Makefile.in b/gdbsupport/Makefile.in
index 9fdc23c39a9..070e36a6aff 100644
--- a/gdbsupport/Makefile.in
+++ b/gdbsupport/Makefile.in
@@ -166,8 +166,8 @@ am_libgdbsupport_a_OBJECTS = agent.$(OBJEXT) btrace-common.$(OBJEXT) \
ptid.$(OBJEXT) rsp-low.$(OBJEXT) run-time-clock.$(OBJEXT) \
safe-strerror.$(OBJEXT) scoped_mmap.$(OBJEXT) search.$(OBJEXT) \
signals.$(OBJEXT) signals-state-save-restore.$(OBJEXT) \
- tdesc.$(OBJEXT) thread-pool.$(OBJEXT) xml-utils.$(OBJEXT) \
- $(am__objects_1) $(am__objects_2)
+ task-group.$(OBJEXT) tdesc.$(OBJEXT) thread-pool.$(OBJEXT) \
+ xml-utils.$(OBJEXT) $(am__objects_1) $(am__objects_2)
libgdbsupport_a_OBJECTS = $(am_libgdbsupport_a_OBJECTS)
AM_V_P = $(am__v_P_@AM_V@)
am__v_P_ = $(am__v_P_@AM_DEFAULT_V@)
@@ -436,6 +436,7 @@ libgdbsupport_a_SOURCES = \
search.cc \
signals.cc \
signals-state-save-restore.cc \
+ task-group.cc \
tdesc.cc \
thread-pool.cc \
xml-utils.cc \
@@ -546,6 +547,7 @@ distclean-compile:
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/selftest.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/signals-state-save-restore.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/signals.Po@am__quote@
+@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/task-group.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/tdesc.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/thread-pool.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/xml-utils.Po@am__quote@
diff --git a/gdbsupport/task-group.cc b/gdbsupport/task-group.cc
new file mode 100644
index 00000000000..e6c8a6f2e31
--- /dev/null
+++ b/gdbsupport/task-group.cc
@@ -0,0 +1,97 @@
+/* Task group
+
+ Copyright (C) 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/>. */
+
+#include "common-defs.h"
+#include "task-group.h"
+#include "thread-pool.h"
+
+namespace gdb
+{
+
+class task_group::impl : public std::enable_shared_from_this<task_group::impl>
+{
+public:
+
+ explicit impl (std::function<void ()> &&done)
+ : m_completed (0),
+ m_done (std::move (done))
+ { }
+ DISABLE_COPY_AND_ASSIGN (impl);
+
+ ~impl ()
+ {
+ if (m_started)
+ m_done ();
+ }
+
+ /* Add a task to the task group. */
+ void add_task (std::function<void ()> &&task)
+ {
+ m_tasks.push_back (std::move (task));
+ };
+
+ /* Start this task group. */
+ void start ();
+
+ /* True if started. */
+ bool m_started = false;
+ /* The tasks. */
+ std::vector<std::function<void ()>> m_tasks;
+ /* The number of tasks that have completed. */
+ std::atomic<int> m_completed;
+ /* The 'done' function. */
+ std::function<void ()> m_done;
+};
+
+void
+task_group::impl::start ()
+{
+ std::shared_ptr<impl> shared_this = shared_from_this ();
+ m_started = true;
+ for (size_t i = 0; i < m_tasks.size (); ++i)
+ {
+ gdb::thread_pool::g_thread_pool->post_task ([=] ()
+ {
+ /* Be sure to capture a shared reference here. */
+ shared_this->m_tasks[i] ();
+ });
+ }
+}
+
+task_group::task_group (std::function<void ()> &&done)
+ : m_task (new impl (std::move (done)))
+{
+}
+
+void
+task_group::add_task (std::function<void ()> &&task)
+{
+ gdb_assert (m_task != nullptr);
+ m_task->add_task (std::move (task));
+}
+
+void
+task_group::start ()
+{
+ gdb_assert (m_task != nullptr);
+ m_task->start ();
+ m_task.reset ();
+}
+
+} /* namespace gdb */
diff --git a/gdbsupport/task-group.h b/gdbsupport/task-group.h
new file mode 100644
index 00000000000..c11b5ccf158
--- /dev/null
+++ b/gdbsupport/task-group.h
@@ -0,0 +1,64 @@
+/* Task group
+
+ Copyright (C) 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/>. */
+
+#ifndef GDBSUPPORT_TASK_GROUP_H
+#define GDBSUPPORT_TASK_GROUP_H
+
+#include <memory>
+
+namespace gdb
+{
+
+/* A task group is a collection of tasks. Each task in the group is
+ submitted to the thread pool. When all the tasks in the group have
+ finished, a final action is run. */
+
+class task_group
+{
+public:
+
+ explicit task_group (std::function<void ()> &&done);
+ DISABLE_COPY_AND_ASSIGN (task_group);
+
+ /* Add a task to the task group. All tasks must be added before the
+ group is started. Note that a task may not throw an
+ exception. */
+ void add_task (std::function<void ()> &&task);
+
+ /* Start this task group. A task group may only be started once.
+ This will submit all the tasks to the global thread pool. */
+ void start ();
+
+private:
+
+ class impl;
+
+ /* A task group is just a facade around an impl. This is done
+ because the impl object must live as long as its longest-lived
+ task, so it is heap-allocated and destroyed when the last task
+ completes. Before 'start' is called, the impl is owned by this
+ task_group, but after 'start' it is owned jointly by the running
+ tasks, and m_task is cleared. */
+ // FIXME
+ std::shared_ptr<impl> m_task;
+};
+
+} /* namespace gdb */
+
+#endif /* GDBSUPPORT_TASK_GROUP_H */
--
2.43.0
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH v4 08/19] Add gdb::task_group
2023-12-10 21:41 ` [PATCH v4 08/19] Add gdb::task_group Tom Tromey
@ 2023-12-18 14:55 ` Tom Tromey
0 siblings, 0 replies; 25+ messages in thread
From: Tom Tromey @ 2023-12-18 14:55 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:
Tom> This adds gdb::task_group, a convenient way to group background tasks
Tom> and then call a function when all the tasks have completed.
Tom> + /* A task group is just a facade around an impl. This is done
Tom> + because the impl object must live as long as its longest-lived
Tom> + task, so it is heap-allocated and destroyed when the last task
Tom> + completes. Before 'start' is called, the impl is owned by this
Tom> + task_group, but after 'start' it is owned jointly by the running
Tom> + tasks, and m_task is cleared. */
Tom> + // FIXME
Tom> + std::shared_ptr<impl> m_task;
I found this stray FIXME comment in here today. I think it's a reminder
to rewrite the comment, since the comment talks about ownership but with
shared_ptr this isn't needed.
I'm going to fix this up.
Tom
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v4 09/19] Move cooked_index_storage to cooked-index.h
2023-12-10 21:41 [PATCH v4 00/19] Index DWARF in the background Tom Tromey
` (7 preceding siblings ...)
2023-12-10 21:41 ` [PATCH v4 08/19] Add gdb::task_group Tom Tromey
@ 2023-12-10 21:41 ` Tom Tromey
2023-12-10 21:41 ` [PATCH v4 10/19] Add "maint set dwarf synchronous" Tom Tromey
` (10 subsequent siblings)
19 siblings, 0 replies; 25+ messages in thread
From: Tom Tromey @ 2023-12-10 21:41 UTC (permalink / raw)
To: gdb-patches
This moves cooked_index_storage to cooked-index.h. This is needed by
a subsequent patch.
---
gdb/dwarf2/cooked-index.h | 70 ++++++++++++++++++++++++
gdb/dwarf2/read.c | 135 ++++++++++++++--------------------------------
2 files changed, 111 insertions(+), 94 deletions(-)
diff --git a/gdb/dwarf2/cooked-index.h b/gdb/dwarf2/cooked-index.h
index 5675ea68bb8..36b734f0fb6 100644
--- a/gdb/dwarf2/cooked-index.h
+++ b/gdb/dwarf2/cooked-index.h
@@ -33,6 +33,7 @@
#include "gdbsupport/thread-pool.h"
#include "dwarf2/mapped-index.h"
#include "dwarf2/tag.h"
+#include "dwarf2/abbrev-cache.h"
#include "gdbsupport/range-chain.h"
struct dwarf2_per_cu_data;
@@ -354,6 +355,75 @@ class cooked_index_shard
gdb::future<void> m_future;
};
+class cutu_reader;
+
+/* An instance of this is created when scanning DWARF to create a
+ cooked index. */
+
+class cooked_index_storage
+{
+public:
+
+ cooked_index_storage ();
+ DISABLE_COPY_AND_ASSIGN (cooked_index_storage);
+
+ /* Return the current abbrev cache. */
+ abbrev_cache *get_abbrev_cache ()
+ {
+ return &m_abbrev_cache;
+ }
+
+ /* Return the DIE reader corresponding to PER_CU. If no such reader
+ has been registered, return NULL. */
+ cutu_reader *get_reader (dwarf2_per_cu_data *per_cu);
+
+ /* Preserve READER by storing it in the local hash table. */
+ cutu_reader *preserve (std::unique_ptr<cutu_reader> reader);
+
+ /* Add an entry to the index. The arguments describe the entry; see
+ cooked-index.h. The new entry is returned. */
+ const cooked_index_entry *add (sect_offset die_offset, enum dwarf_tag tag,
+ cooked_index_flag flags,
+ const char *name,
+ const cooked_index_entry *parent_entry,
+ dwarf2_per_cu_data *per_cu)
+ {
+ return m_index->add (die_offset, tag, flags, name, parent_entry, per_cu);
+ }
+
+ /* Install the current addrmap into the shard being constructed,
+ then transfer ownership of the index to the caller. */
+ std::unique_ptr<cooked_index_shard> release ()
+ {
+ m_index->install_addrmap (&m_addrmap);
+ return std::move (m_index);
+ }
+
+ /* Return the mutable addrmap that is currently being created. */
+ addrmap_mutable *get_addrmap ()
+ {
+ return &m_addrmap;
+ }
+
+private:
+
+ /* Hash function for a cutu_reader. */
+ static hashval_t hash_cutu_reader (const void *a);
+
+ /* Equality function for cutu_reader. */
+ static int eq_cutu_reader (const void *a, const void *b);
+
+ /* The abbrev cache used by this indexer. */
+ abbrev_cache m_abbrev_cache;
+ /* A hash table of cutu_reader objects. */
+ htab_up m_reader_hash;
+ /* The index shard that is being constructed. */
+ std::unique_ptr<cooked_index_shard> m_index;
+
+ /* A writeable addrmap being constructed by this scanner. */
+ addrmap_mutable m_addrmap;
+};
+
/* The main index of DIEs. The parallel DIE indexers create
cooked_index_shard objects. Then, these are all handled to a
cooked_index for storage and final indexing. The index is
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 5189a7e0c45..afadb05d9d0 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -742,7 +742,6 @@ show_dwarf_max_cache_age (struct ui_file *file, int from_tty,
static void dwarf2_find_base_address (struct die_info *die,
struct dwarf2_cu *cu);
-class cooked_index_storage;
static void build_type_psymtabs_reader (cutu_reader *reader,
cooked_index_storage *storage);
@@ -4418,105 +4417,53 @@ get_type_unit_group (struct dwarf2_cu *cu, const struct attribute *stmt_list)
}
\f
-/* An instance of this is created when scanning DWARF to create a
- cooked index. */
-
-class cooked_index_storage
+cooked_index_storage::cooked_index_storage ()
+ : m_reader_hash (htab_create_alloc (10, hash_cutu_reader,
+ eq_cutu_reader,
+ htab_delete_entry<cutu_reader>,
+ xcalloc, xfree)),
+ m_index (new cooked_index_shard)
{
-public:
-
- cooked_index_storage ()
- : m_reader_hash (htab_create_alloc (10, hash_cutu_reader,
- eq_cutu_reader,
- htab_delete_entry<cutu_reader>,
- xcalloc, xfree)),
- m_index (new cooked_index_shard)
- {
- }
-
- DISABLE_COPY_AND_ASSIGN (cooked_index_storage);
-
- /* Return the current abbrev cache. */
- abbrev_cache *get_abbrev_cache ()
- {
- return &m_abbrev_cache;
- }
-
- /* Return the DIE reader corresponding to PER_CU. If no such reader
- has been registered, return NULL. */
- cutu_reader *get_reader (dwarf2_per_cu_data *per_cu)
- {
- int index = per_cu->index;
- return (cutu_reader *) htab_find_with_hash (m_reader_hash.get (),
- &index, index);
- }
-
- /* Preserve READER by storing it in the local hash table. */
- cutu_reader *preserve (std::unique_ptr<cutu_reader> reader)
- {
- m_abbrev_cache.add (reader->release_abbrev_table ());
-
- int index = reader->cu->per_cu->index;
- void **slot = htab_find_slot_with_hash (m_reader_hash.get (), &index,
- index, INSERT);
- gdb_assert (*slot == nullptr);
- cutu_reader *result = reader.get ();
- *slot = reader.release ();
- return result;
- }
-
- /* Add an entry to the index. The arguments describe the entry; see
- cooked-index.h. The new entry is returned. */
- const cooked_index_entry *add (sect_offset die_offset, enum dwarf_tag tag,
- cooked_index_flag flags,
- const char *name,
- const cooked_index_entry *parent_entry,
- dwarf2_per_cu_data *per_cu)
- {
- return m_index->add (die_offset, tag, flags, name, parent_entry, per_cu);
- }
-
- /* Install the current addrmap into the index shard being constructed,
- then transfer ownership of the index to the caller. */
- std::unique_ptr<cooked_index_shard> release ()
- {
- m_index->install_addrmap (&m_addrmap);
- return std::move (m_index);
- }
-
- /* Return the mutable addrmap that is currently being created. */
- addrmap_mutable *get_addrmap ()
- {
- return &m_addrmap;
- }
+}
-private:
+cutu_reader *
+cooked_index_storage::get_reader (dwarf2_per_cu_data *per_cu)
+{
+ int index = per_cu->index;
+ return (cutu_reader *) htab_find_with_hash (m_reader_hash.get (),
+ &index, index);
+}
- /* Hash function for a cutu_reader. */
- static hashval_t hash_cutu_reader (const void *a)
- {
- const cutu_reader *reader = (const cutu_reader *) a;
- return reader->cu->per_cu->index;
- }
+cutu_reader *
+cooked_index_storage::preserve (std::unique_ptr<cutu_reader> reader)
+{
+ m_abbrev_cache.add (reader->release_abbrev_table ());
- /* Equality function for cutu_reader. */
- static int eq_cutu_reader (const void *a, const void *b)
- {
- const cutu_reader *ra = (const cutu_reader *) a;
- const int *rb = (const int *) b;
- return ra->cu->per_cu->index == *rb;
- }
+ int index = reader->cu->per_cu->index;
+ void **slot = htab_find_slot_with_hash (m_reader_hash.get (), &index,
+ index, INSERT);
+ gdb_assert (*slot == nullptr);
+ cutu_reader *result = reader.get ();
+ *slot = reader.release ();
+ return result;
+}
- /* The abbrev cache used by this indexer. */
- abbrev_cache m_abbrev_cache;
- /* A hash table of cutu_reader objects. */
- htab_up m_reader_hash;
- /* The index shard that is being constructed. */
- std::unique_ptr<cooked_index_shard> m_index;
+/* Hash function for a cutu_reader. */
+hashval_t
+cooked_index_storage::hash_cutu_reader (const void *a)
+{
+ const cutu_reader *reader = (const cutu_reader *) a;
+ return reader->cu->per_cu->index;
+}
- /* A writeable addrmap being constructed by this scanner. */
- addrmap_mutable m_addrmap;
-};
+/* Equality function for cutu_reader. */
+int
+cooked_index_storage::eq_cutu_reader (const void *a, const void *b)
+{
+ const cutu_reader *ra = (const cutu_reader *) a;
+ const int *rb = (const int *) b;
+ return ra->cu->per_cu->index == *rb;
+}
/* An instance of this is created to index a CU. */
--
2.43.0
^ permalink raw reply [flat|nested] 25+ messages in thread* [PATCH v4 10/19] Add "maint set dwarf synchronous"
2023-12-10 21:41 [PATCH v4 00/19] Index DWARF in the background Tom Tromey
` (8 preceding siblings ...)
2023-12-10 21:41 ` [PATCH v4 09/19] Move cooked_index_storage to cooked-index.h Tom Tromey
@ 2023-12-10 21:41 ` Tom Tromey
2023-12-10 21:41 ` [PATCH v4 11/19] Change how cooked index waits for threads Tom Tromey
` (9 subsequent siblings)
19 siblings, 0 replies; 25+ messages in thread
From: Tom Tromey @ 2023-12-10 21:41 UTC (permalink / raw)
To: gdb-patches; +Cc: Eli Zaretskii
For testing, it's sometimes convenient to be able to request that
DWARF reading be done synchronously. This patch adds a new "maint"
setting for this purpose.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
---
gdb/NEWS | 4 ++++
gdb/doc/gdb.texinfo | 18 ++++++++++++++++++
gdb/dwarf2/read.c | 27 +++++++++++++++++++++++++++
3 files changed, 49 insertions(+)
diff --git a/gdb/NEWS b/gdb/NEWS
index c8166f08218..810fe617f52 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -9,6 +9,10 @@
* GDB index now contains information about the main function. This speeds up
startup when it is being used for some large binaries.
+* On hosts where threading is available, DWARF reading is now done in
+ the background, resulting in faster startup. This can be controlled
+ using "maint set dwarf synchronous".
+
* Changed commands
disassemble
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 58685a77fd4..139484460c8 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -41764,6 +41764,24 @@ compilation units will be stored in memory longer, and more total
memory will be used. Setting it to zero disables caching, which will
slow down @value{GDBN} startup, but reduce memory consumption.
+@kindex maint set dwarf synchronous
+@kindex maint show dwarf synchronous
+@item maint set dwarf synchronous
+@itemx maint show dwarf synchronous
+Control whether DWARF is read asynchronously.
+
+On hosts where threading is available, the DWARF reader is mostly
+asynchronous with respect to the rest of @value{GDBN}. That is, the
+bulk of the reading is done in the background, and @value{GDBN} will
+only pause for completion of this task when absolutely necessary.
+
+When this setting is enabled, @value{GDBN} will instead wait for DWARF
+processing to complete before continuing.
+
+On hosts without threading, or where worker threads have been disabled
+at runtime, this setting has no effect, as DWARF reading is always
+done on the main thread, and is therefore always synchronous.
+
@kindex maint set dwarf unwinders
@kindex maint show dwarf unwinders
@item maint set dwarf unwinders
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index afadb05d9d0..b3f1c6b5dae 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -736,6 +736,18 @@ show_dwarf_max_cache_age (struct ui_file *file, int from_tty,
"DWARF compilation units is %s.\n"),
value);
}
+
+/* When true, wait for DWARF reading to be complete. */
+static bool dwarf_synchronous = false;
+
+/* "Show" callback for "maint set dwarf synchronous". */
+static void
+show_dwarf_synchronous (struct ui_file *file, int from_tty,
+ struct cmd_list_element *c, const char *value)
+{
+ gdb_printf (file, _("Whether DWARF reading is synchronous is %s.\n"),
+ value);
+}
\f
/* local function prototypes */
@@ -21832,6 +21844,21 @@ caching, which can slow down startup."),
&set_dwarf_cmdlist,
&show_dwarf_cmdlist);
+ add_setshow_boolean_cmd ("synchronous", class_obscure,
+ &dwarf_synchronous, _("\
+Set whether DWARF is read synchronously."), _("\
+Show whether DWARF is read synchronously."), _("\
+By default, DWARF information is read in worker threads,\n\
+and gdb will not generally wait for the reading to complete\n\
+before continuing with other work, for example presenting a\n\
+prompt to the user.\n\
+Enabling this setting will cause the DWARF reader to always wait\n\
+for debug info processing to be finished before gdb can proceed."),
+ nullptr,
+ show_dwarf_synchronous,
+ &set_dwarf_cmdlist,
+ &show_dwarf_cmdlist);
+
add_setshow_zuinteger_cmd ("dwarf-read", no_class, &dwarf_read_debug, _("\
Set debugging of the DWARF reader."), _("\
Show debugging of the DWARF reader."), _("\
--
2.43.0
^ permalink raw reply [flat|nested] 25+ messages in thread* [PATCH v4 11/19] Change how cooked index waits for threads
2023-12-10 21:41 [PATCH v4 00/19] Index DWARF in the background Tom Tromey
` (9 preceding siblings ...)
2023-12-10 21:41 ` [PATCH v4 10/19] Add "maint set dwarf synchronous" Tom Tromey
@ 2023-12-10 21:41 ` Tom Tromey
2023-12-10 21:41 ` [PATCH v4 12/19] Do more DWARF reading in the background Tom Tromey
` (8 subsequent siblings)
19 siblings, 0 replies; 25+ messages in thread
From: Tom Tromey @ 2023-12-10 21:41 UTC (permalink / raw)
To: gdb-patches
This changes the cooked index code to wait for threads in its
public-facing API. That is, the waits are done in cooked_index now,
and never in the cooked_index_shard. Centralizing this decision makes
it easier to wait for other events here as well.
---
gdb/dwarf2/cooked-index.c | 3 +--
gdb/dwarf2/cooked-index.h | 2 +-
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/gdb/dwarf2/cooked-index.c b/gdb/dwarf2/cooked-index.c
index ba77f9cb373..7588b5c2896 100644
--- a/gdb/dwarf2/cooked-index.c
+++ b/gdb/dwarf2/cooked-index.c
@@ -409,8 +409,6 @@ cooked_index_shard::do_finalize ()
cooked_index_shard::range
cooked_index_shard::find (const std::string &name, bool completing) const
{
- wait ();
-
cooked_index_entry::comparison_mode mode = (completing
? cooked_index_entry::COMPLETE
: cooked_index_entry::MATCH);
@@ -528,6 +526,7 @@ cooked_index::get_addrmaps () const
cooked_index::range
cooked_index::find (const std::string &name, bool completing) const
{
+ wait ();
std::vector<cooked_index_shard::range> result_range;
result_range.reserve (m_vector.size ());
for (auto &entry : m_vector)
diff --git a/gdb/dwarf2/cooked-index.h b/gdb/dwarf2/cooked-index.h
index 36b734f0fb6..97f09668c7b 100644
--- a/gdb/dwarf2/cooked-index.h
+++ b/gdb/dwarf2/cooked-index.h
@@ -287,7 +287,6 @@ class cooked_index_shard
/* Return a range of all the entries. */
range all_entries () const
{
- wait ();
return { m_entries.cbegin (), m_entries.cend () };
}
@@ -460,6 +459,7 @@ class cooked_index : public dwarf_scanner_base
/* Return a range of all the entries. */
range all_entries () const
{
+ wait ();
std::vector<cooked_index_shard::range> result_range;
result_range.reserve (m_vector.size ());
for (auto &entry : m_vector)
--
2.43.0
^ permalink raw reply [flat|nested] 25+ messages in thread* [PATCH v4 12/19] Do more DWARF reading in the background
2023-12-10 21:41 [PATCH v4 00/19] Index DWARF in the background Tom Tromey
` (10 preceding siblings ...)
2023-12-10 21:41 ` [PATCH v4 11/19] Change how cooked index waits for threads Tom Tromey
@ 2023-12-10 21:41 ` Tom Tromey
2023-12-10 21:41 ` [PATCH v4 13/19] Simplify the public DWARF API Tom Tromey
` (7 subsequent siblings)
19 siblings, 0 replies; 25+ messages in thread
From: Tom Tromey @ 2023-12-10 21:41 UTC (permalink / raw)
To: gdb-patches
This patch rearranges the DWARF reader so that more work is done in
the background. This is PR symtab/29942.
The idea here is that there is only a small amount of work that must
be done on the main thread when scanning DWARF -- before the main
scan, the only part is mapping the section data.
Currently, the DWARF reader uses the quick_symbol_functions "lazy"
functionality to defer even starting to read. This patch instead
changes the reader to start reading immediately, but doing more in
worker tasks.
Before this patch, "file" on my machine:
(gdb) file /tmp/gdb
2023-10-23 12:29:56.885 - command started
Reading symbols from /tmp/gdb...
2023-10-23 12:29:58.047 - command finished
Command execution time: 5.867228 (cpu), 1.162444 (wall)
After the patch, more work is done in the background and so this takes
a bit less time:
(gdb) file /tmp/gdb
2023-10-23 13:25:51.391 - command started
Reading symbols from /tmp/gdb...
2023-10-23 13:25:51.712 - command finished
Command execution time: 1.894500 (cpu), 0.320306 (wall)
I think this could be further sped up by using the shared library load
map to avoid objfile loops like the one in expand_symtab_containing_pc
-- it seems like the correct objfile could be chosen more directly.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29942
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30174
---
gdb/dwarf2/cooked-index.c | 200 ++++++----
gdb/dwarf2/cooked-index.h | 252 +++++++++---
gdb/dwarf2/read.c | 482 ++++++++++++++---------
gdb/dwarf2/read.h | 2 +-
gdb/testsuite/gdb.dwarf2/dw2-error.exp | 1 +
gdb/testsuite/gdb.dwarf2/dw2-missing-cu-tag.exp | 2 +
gdb/testsuite/gdb.dwarf2/dw2-stack-boundary.exp | 2 +
gdb/testsuite/gdb.dwarf2/dw2-using-debug-str.exp | 2 +
gdb/testsuite/gdb.dwarf2/dw2-zero-range.exp | 4 +-
gdb/testsuite/gdb.dwarf2/fission-reread.exp | 5 +-
gdb/testsuite/gdb.dwarf2/no-gnu-debuglink.exp | 1 +
gdb/testsuite/gdb.dwarf2/struct-with-sig-2.exp | 10 +-
12 files changed, 660 insertions(+), 303 deletions(-)
diff --git a/gdb/dwarf2/cooked-index.c b/gdb/dwarf2/cooked-index.c
index 7588b5c2896..fabffc75d92 100644
--- a/gdb/dwarf2/cooked-index.c
+++ b/gdb/dwarf2/cooked-index.c
@@ -66,6 +66,23 @@ language_requires_canonicalization (enum language lang)
|| lang == language_cplus);
}
+/* Return true if a plain "main" could be the main program for this
+ language. Languages that are known to use some other mechanism are
+ excluded here. */
+
+static bool
+language_may_use_plain_main (enum language lang)
+{
+ /* No need to handle "unknown" here. */
+ return (lang == language_c
+ || lang == language_objc
+ || lang == language_cplus
+ || lang == language_m2
+ || lang == language_asm
+ || lang == language_opencl
+ || lang == language_minimal);
+}
+
/* See cooked-index.h. */
int
@@ -242,23 +259,17 @@ cooked_index_shard::add (sect_offset die_offset, enum dwarf_tag tag,
implicit "main" discovery. */
if ((flags & IS_MAIN) != 0)
m_main = result;
+ else if (parent_entry == nullptr
+ && m_main == nullptr
+ && language_may_use_plain_main (per_cu->lang ())
+ && strcmp (name, "main") == 0)
+ m_main = result;
return result;
}
/* See cooked-index.h. */
-void
-cooked_index_shard::finalize ()
-{
- m_future = gdb::thread_pool::g_thread_pool->post_task ([this] ()
- {
- do_finalize ();
- });
-}
-
-/* See cooked-index.h. */
-
gdb::unique_xmalloc_ptr<char>
cooked_index_shard::handle_gnat_encoded_entry (cooked_index_entry *entry,
htab_t gnat_entries)
@@ -306,7 +317,7 @@ cooked_index_shard::handle_gnat_encoded_entry (cooked_index_entry *entry,
/* See cooked-index.h. */
void
-cooked_index_shard::do_finalize ()
+cooked_index_shard::finalize ()
{
auto hash_name_ptr = [] (const void *p)
{
@@ -430,65 +441,77 @@ cooked_index_shard::find (const std::string &name, bool completing) const
return range (lower, upper);
}
-/* See cooked-index.h. */
-
-void
-cooked_index_shard::wait (bool allow_quit) const
-{
- if (allow_quit)
- {
- std::chrono::milliseconds duration { 15 };
- while (m_future.wait_for (duration) == gdb::future_status::timeout)
- QUIT;
- }
- else
- m_future.wait ();
-}
-cooked_index::cooked_index (vec_type &&vec)
- : m_vector (std::move (vec))
+cooked_index::cooked_index (dwarf2_per_objfile *per_objfile)
+ : m_state (std::make_unique<cooked_index_worker> (per_objfile)),
+ m_per_bfd (per_objfile->per_bfd)
{
- for (auto &idx : m_vector)
- idx->finalize ();
-
/* ACTIVE_VECTORS is not locked, and this assert ensures that this
will be caught if ever moved to the background. */
gdb_assert (is_main_thread ());
active_vectors.insert (this);
}
-/* See cooked-index.h. */
+void
+cooked_index::start_reading ()
+{
+ m_state->start ();
+}
void
-cooked_index::start_writing_index (dwarf2_per_bfd *per_bfd)
+cooked_index::wait (cooked_state desired_state, bool allow_quit)
{
- index_cache_store_context ctx (global_index_cache, per_bfd);
+ gdb_assert (desired_state != cooked_state::INITIAL);
- /* This must be set after all the finalization tasks have been
- started, because it may call 'wait'. */
- m_write_future
- = gdb::thread_pool::g_thread_pool->post_task ([this, per_bfd,
- ctx = std::move (ctx)] ()
- {
- maybe_write_index (per_bfd, ctx);
- });
+ /* If the state object has been deleted, then that means waiting is
+ completely done. */
+ if (m_state == nullptr)
+ return;
+
+ if (m_state->wait (desired_state, allow_quit))
+ {
+ /* Only the main thread can modify this. */
+ gdb_assert (is_main_thread ());
+ m_state.reset (nullptr);
+ }
+}
+
+void
+cooked_index::set_contents (vec_type &&vec)
+{
+ gdb_assert (m_vector.empty ());
+ m_vector = std::move (vec);
+
+ m_state->set (cooked_state::MAIN_AVAILABLE);
+
+ index_cache_store_context ctx (global_index_cache, m_per_bfd);
+
+ /* This is run after finalization is done -- but not before. If
+ this task were submitted earlier, it would have to wait for
+ finalization. However, that would take a slot in the global
+ thread pool, and if enough such tasks were submitted at once, it
+ would cause a livelock. */
+ gdb::task_group finalizers ([this, ctx = std::move (ctx)] ()
+ {
+ m_state->set (cooked_state::FINALIZED);
+ maybe_write_index (m_per_bfd, ctx);
+ });
+
+ for (auto &idx : m_vector)
+ {
+ auto this_index = idx.get ();
+ finalizers.add_task ([=] () { this_index->finalize (); });
+ }
+
+ finalizers.start ();
}
cooked_index::~cooked_index ()
{
- /* The 'finalize' method may be run in a different thread. If
- this object is destroyed before this completes, then the method
- will end up writing to freed memory. Waiting for this to
- complete avoids this problem; and the cost seems ignorable
- because creating and immediately destroying the debug info is a
- relatively rare thing to do. */
- for (auto &item : m_vector)
- item->wait (false);
-
- /* Likewise for the index-creating future, though this one must also
+ /* Wait for index-creation to be done, though this one must also
waited for by the per-BFD object to ensure the required data
remains live. */
- wait_completely ();
+ wait (cooked_state::CACHE_DONE);
/* Remove our entry from the global list. See the assert in the
constructor to understand this. */
@@ -501,6 +524,8 @@ cooked_index::~cooked_index ()
dwarf2_per_cu_data *
cooked_index::lookup (CORE_ADDR addr)
{
+ /* Ensure that the address maps are ready. */
+ wait (cooked_state::MAIN_AVAILABLE, true);
for (const auto &index : m_vector)
{
dwarf2_per_cu_data *result = index->lookup (addr);
@@ -513,8 +538,10 @@ cooked_index::lookup (CORE_ADDR addr)
/* See cooked-index.h. */
std::vector<const addrmap *>
-cooked_index::get_addrmaps () const
+cooked_index::get_addrmaps ()
{
+ /* Ensure that the address maps are ready. */
+ wait (cooked_state::MAIN_AVAILABLE, true);
std::vector<const addrmap *> result;
for (const auto &index : m_vector)
result.push_back (index->m_addrmap);
@@ -524,9 +551,9 @@ cooked_index::get_addrmaps () const
/* See cooked-index.h. */
cooked_index::range
-cooked_index::find (const std::string &name, bool completing) const
+cooked_index::find (const std::string &name, bool completing)
{
- wait ();
+ wait (cooked_state::FINALIZED, true);
std::vector<cooked_index_shard::range> result_range;
result_range.reserve (m_vector.size ());
for (auto &entry : m_vector)
@@ -536,38 +563,65 @@ cooked_index::find (const std::string &name, bool completing) const
/* See cooked-index.h. */
+const char *
+cooked_index::get_main_name (struct obstack *obstack, enum language *lang)
+ const
+{
+ const cooked_index_entry *entry = get_main ();
+ if (entry == nullptr)
+ return nullptr;
+
+ *lang = entry->per_cu->lang ();
+ return entry->full_name (obstack, true);
+}
+
+/* See cooked_index.h. */
+
const cooked_index_entry *
cooked_index::get_main () const
{
- const cooked_index_entry *result = nullptr;
-
+ const cooked_index_entry *best_entry = nullptr;
for (const auto &index : m_vector)
{
const cooked_index_entry *entry = index->get_main ();
- /* Choose the first "main" we see. The choice among several is
- arbitrary. See the comment by the sole caller to understand
- the rationale for filtering by language. */
- if (entry != nullptr
- && !language_requires_canonicalization (entry->per_cu->lang ()))
+ /* Choose the first "main" we see. We only do this for names
+ not requiring canonicalization. At this point in the process
+ names might not have been canonicalized. However, currently,
+ languages that require this step also do not use
+ DW_AT_main_subprogram. An assert is appropriate here because
+ this filtering is done in get_main. */
+ if (entry != nullptr)
{
- result = entry;
- break;
+ if ((entry->flags & IS_MAIN) != 0)
+ {
+ if (!language_requires_canonicalization (entry->per_cu->lang ()))
+ {
+ /* There won't be one better than this. */
+ return entry;
+ }
+ }
+ else
+ {
+ /* This is one that is named "main". Here we don't care
+ if the language requires canonicalization, due to how
+ the entry is detected. Entries like this have worse
+ priority than IS_MAIN entries. */
+ if (best_entry == nullptr)
+ best_entry = entry;
+ }
}
}
- return result;
+ return best_entry;
}
/* See cooked-index.h. */
void
-cooked_index::dump (gdbarch *arch) const
+cooked_index::dump (gdbarch *arch)
{
auto_obstack temp_storage;
- /* Ensure the index is done building. */
- this->wait ();
-
gdb_printf (" entries:\n");
gdb_printf ("\n");
@@ -640,11 +694,9 @@ void
cooked_index::maybe_write_index (dwarf2_per_bfd *per_bfd,
const index_cache_store_context &ctx)
{
- /* Wait for finalization. */
- wait ();
-
/* (maybe) store an index in the cache. */
- global_index_cache.store (per_bfd, ctx);
+ global_index_cache.store (m_per_bfd, ctx);
+ m_state->set (cooked_state::CACHE_DONE);
}
/* Wait for all the index cache entries to be written before gdb
diff --git a/gdb/dwarf2/cooked-index.h b/gdb/dwarf2/cooked-index.h
index 97f09668c7b..700e63f76b4 100644
--- a/gdb/dwarf2/cooked-index.h
+++ b/gdb/dwarf2/cooked-index.h
@@ -32,9 +32,18 @@
#include "gdbsupport/iterator-range.h"
#include "gdbsupport/thread-pool.h"
#include "dwarf2/mapped-index.h"
+#include "dwarf2/read.h"
#include "dwarf2/tag.h"
#include "dwarf2/abbrev-cache.h"
#include "gdbsupport/range-chain.h"
+#include "gdbsupport/task-group.h"
+#include "complaints.h"
+#include "run-on-main-thread.h"
+
+#if CXX_STD_THREAD
+#include <mutex>
+#include <condition_variable>
+#endif /* CXX_STD_THREAD */
struct dwarf2_per_cu_data;
struct dwarf2_per_bfd;
@@ -64,7 +73,7 @@ std::string to_string (cooked_index_flag flags);
/* Return true if LANG requires canonicalization. This is used
primarily to work around an issue computing the name of "main".
This function must be kept in sync with
- cooked_index_shard::do_finalize. */
+ cooked_index_shard::finalize. */
extern bool language_requires_canonicalization (enum language lang);
@@ -270,14 +279,6 @@ class cooked_index_shard
m_addrmap = new (&m_storage) addrmap_fixed (&m_storage, map);
}
- /* Finalize the index. This should be called a single time, when
- the index has been fully populated. It enters all the entries
- into the internal table. */
- void finalize ();
-
- /* Wait for this index's finalization to be complete. */
- void wait (bool allow_quit = true) const;
-
friend class cooked_index;
/* A simple range over part of m_entries. */
@@ -334,8 +335,11 @@ class cooked_index_shard
gdb::unique_xmalloc_ptr<char> handle_gnat_encoded_entry
(cooked_index_entry *entry, htab_t gnat_entries);
- /* A helper method that does the work of 'finalize'. */
- void do_finalize ();
+ /* Finalize the index. This should be called a single time, when
+ the index has been fully populated. It enters all the entries
+ into the internal table. This may be invoked in a worker
+ thread. */
+ void finalize ();
/* Storage for the entries. */
auto_obstack m_storage;
@@ -348,10 +352,6 @@ class cooked_index_shard
addrmap *m_addrmap = nullptr;
/* Storage for canonical names. */
std::vector<gdb::unique_xmalloc_ptr<char>> m_names;
- /* A future that tracks when the 'finalize' method is done. Note
- that the 'get' method is never called on this future, only
- 'wait'. */
- gdb::future<void> m_future;
};
class cutu_reader;
@@ -423,10 +423,164 @@ class cooked_index_storage
addrmap_mutable m_addrmap;
};
-/* The main index of DIEs. The parallel DIE indexers create
- cooked_index_shard objects. Then, these are all handled to a
- cooked_index for storage and final indexing. The index is
- made by iterating over the entries previously created. */
+/* The possible states of the index. See the explanatory comment
+ before cooked_index for more details. */
+enum class cooked_state
+{
+ /* The default state. This is not a valid argument to 'wait'. */
+ INITIAL,
+ /* The initial scan has completed. The name of "main" is now
+ available (if known). The addrmaps are usable now.
+ Finalization has started but is not complete. */
+ MAIN_AVAILABLE,
+ /* Finalization has completed. This means the index is fully
+ available for queries. */
+ FINALIZED,
+ /* Writing to the index cache has finished. */
+ CACHE_DONE,
+};
+
+/* An object of this type controls the scanning of the DWARF. It
+ schedules the worker tasks and tracks the current state. Once
+ scanning is done, this object is discarded. */
+
+class cooked_index_worker
+{
+public:
+
+ explicit cooked_index_worker (dwarf2_per_objfile *per_objfile);
+ DISABLE_COPY_AND_ASSIGN (cooked_index_worker);
+
+ /* Start reading. */
+ void start ();
+
+ /* Wait for a particular state to be achieved. If ALLOW_QUIT is
+ true, then the loop will check the QUIT flag. Normally this
+ method may only be called from the main thread; however, it can
+ be called from a worker thread provided that the desired state
+ has already been attained. (This oddity is used by the index
+ cache writer.) */
+ bool wait (cooked_state desired_state, bool allow_quit);
+
+private:
+
+ /* Let cooked_index call the 'set' method. */
+ friend class cooked_index;
+ void set (cooked_state desired_state);
+
+ /* Start reading DWARF. This can be run in a worker thread without
+ problems. */
+ void start_reading ();
+
+ /* Helper function that does most of the work for start_reading. */
+ void do_reading ();
+
+ /* After the last DWARF-reading task has finished, this function
+ does the remaining work to finish the scan. */
+ void done_reading ();
+
+ /* An iterator for the comp units. */
+ typedef std::vector<dwarf2_per_cu_data_up>::iterator unit_iterator;
+
+ /* Process a batch of CUs. This may be called multiple times in
+ separate threads. TASK_NUMBER indicates which task this is --
+ the result is stored in that slot of M_RESULTS. */
+ void process_cus (size_t task_number, unit_iterator first,
+ unit_iterator end);
+
+ /* Each thread returns a tuple holding a cooked index, any collected
+ complaints, and a vector of errors that should be printed. The
+ latter is done because GDB's I/O system is not thread-safe.
+ run_on_main_thread could be used, but that would mean the
+ messages are printed after the prompt, which looks weird. */
+ using result_type = std::tuple<std::unique_ptr<cooked_index_shard>,
+ complaint_collection,
+ std::vector<gdb_exception>>;
+
+ /* The per-objfile object. */
+ dwarf2_per_objfile *m_per_objfile;
+ /* A storage object for "leftovers" -- see the 'start' method, but
+ essentially things not parsed during the normal CU parsing
+ passes. */
+ cooked_index_storage m_index_storage;
+ /* Result of each worker task. */
+ std::vector<result_type> m_results;
+ /* Any warnings emitted. This is not in 'result_type' because (for
+ the time being at least), it's only needed in do_reading, not in
+ every worker. Note that deferred_warnings uses gdb_stderr in its
+ constructor, and this should only be done from the main thread.
+ This is enforced in the cooked_index_worker constructor. */
+ deferred_warnings m_warnings;
+
+#if CXX_STD_THREAD
+ /* Current state of this object. */
+ cooked_state m_state = cooked_state::INITIAL;
+ /* This flag indicates whether any complaints or exceptions that
+ arose during scanning have been reported by 'wait'. This may
+ only be modified on the main thread. */
+ bool m_reported = false;
+ /* Mutex and condition variable used to synchronize. */
+ std::mutex m_mutex;
+ std::condition_variable m_cond;
+ /* If set, an exception occurred during start_reading; in this case
+ the scanning is stopped and this exception will later be reported
+ by the 'wait' method. */
+ std::optional<gdb_exception> m_failed;
+#endif /* CXX_STD_THREAD */
+};
+
+/* The main index of DIEs.
+
+ The index is created by multiple threads. The overall process is
+ somewhat complicated, so here's a diagram to help sort it out.
+
+ The basic idea behind this design is (1) to do as much work as
+ possible in worker threads, and (2) to start the work as early as
+ possible. This combination should help hide the effort from the
+ user to the maximum possible degree.
+
+ . Main Thread | Worker Threads
+ ============================================================
+ . dwarf2_initialize_objfile
+ . |
+ . v
+ . cooked index ------------> cooked_index_worker::start
+ . | / | \
+ . v / | \
+ . install / | \
+ . cooked_index_functions scan CUs in workers
+ . | create cooked_index_shard objects
+ . | \ | /
+ . v \|/
+ . return to caller v
+ . initial scan is done
+ . state = MAIN_AVAILABLE
+ . "main" name now available
+ . |
+ . |
+ . if main thread calls... v
+ . compute_main_name cooked_index::set_contents
+ . | / | \
+ . v / | \
+ . wait (MAIN_AVAILABLE) finalization
+ . | \ | /
+ . v \ | /
+ . done state = FINALIZED
+ . |
+ . v
+ . maybe write to index cache
+ . state = CACHE_DONE
+ .
+ .
+ . if main thread calls...
+ . any other "quick" API
+ . |
+ . v
+ . wait (FINALIZED)
+ . |
+ . v
+ . use the index
+*/
class cooked_index : public dwarf_scanner_base
{
@@ -436,17 +590,17 @@ class cooked_index : public dwarf_scanner_base
object. */
using vec_type = std::vector<std::unique_ptr<cooked_index_shard>>;
- explicit cooked_index (vec_type &&vec);
+ explicit cooked_index (dwarf2_per_objfile *per_objfile);
~cooked_index () override;
+
DISABLE_COPY_AND_ASSIGN (cooked_index);
- /* Wait until the finalization of the entire cooked_index is
- done. */
- void wait () const
- {
- for (auto &item : m_vector)
- item->wait ();
- }
+ /* Start reading the DWARF. */
+ void start_reading ();
+
+ /* Called by cooked_index_worker to set the contents of this index
+ and transition to the MAIN_AVAILABLE state. */
+ void set_contents (vec_type &&vec);
/* A range over a vector of subranges. */
using range = range_chain<cooked_index_shard::range>;
@@ -454,12 +608,12 @@ class cooked_index : 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 (const std::string &name, bool completing) const;
+ range find (const std::string &name, bool completing);
/* Return a range of all the entries. */
- range all_entries () const
+ range all_entries ()
{
- wait ();
+ wait (cooked_state::FINALIZED, true);
std::vector<cooked_index_shard::range> result_range;
result_range.reserve (m_vector.size ());
for (auto &entry : m_vector)
@@ -474,34 +628,38 @@ class cooked_index : public dwarf_scanner_base
/* Return a new vector of all the addrmaps used by all the indexes
held by this object. */
- std::vector<const addrmap *> get_addrmaps () const;
+ std::vector<const addrmap *> get_addrmaps ();
/* Return the entry that is believed to represent the program's
"main". This will return NULL if no such entry is available. */
const cooked_index_entry *get_main () const;
+ const char *get_main_name (struct obstack *obstack, enum language *lang)
+ const;
+
cooked_index *index_for_writing () override
{
+ wait (cooked_state::FINALIZED, true);
return this;
}
quick_symbol_functions_up make_quick_functions () const override;
/* Dump a human-readable form of the contents of the index. */
- void dump (gdbarch *arch) const;
+ void dump (gdbarch *arch);
+
+ /* Wait until this object reaches the desired state. Note that
+ DESIRED_STATE may not be INITIAL -- it does not make sense to
+ wait for this. If ALLOW_QUIT is true, timed waits will be done
+ and the quit flag will be checked in a loop. This may normally
+ only be called from the main thread; however, it is ok to call
+ from a worker as long as the desired state has already been
+ attained. (This property is needed by the index cache
+ writer.) */
+ void wait (cooked_state desired_state, bool allow_quit = false);
- /* Wait for the index to be completely finished. For ordinary uses,
- the index code ensures this itself -- e.g., 'all_entries' will
- wait on the 'finalize' future. However, on destruction, if an
- index is being written, it's also necessary to wait for that to
- complete. */
void wait_completely () override
- {
- m_write_future.wait ();
- }
-
- /* Start writing to the index cache, if the user asked for this. */
- void start_writing_index (dwarf2_per_bfd *per_bfd);
+ { wait (cooked_state::CACHE_DONE); }
private:
@@ -513,8 +671,12 @@ class cooked_index : public dwarf_scanner_base
entries are stored on the obstacks in those objects. */
vec_type m_vector;
- /* A future that tracks when the 'index_write' method is done. */
- gdb::future<void> m_write_future;
+ /* This tracks the current state. When this is nullptr, it means
+ that the state is CACHE_DONE -- it's important to note that only
+ the main thread may change the value of this pointer. */
+ std::unique_ptr<cooked_index_worker> m_state;
+
+ dwarf2_per_bfd *m_per_bfd;
};
#endif /* GDB_DWARF2_COOKED_INDEX_H */
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index b3f1c6b5dae..8a60cf91612 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -94,8 +94,8 @@
#include "dwarf2/abbrev-cache.h"
#include "cooked-index.h"
#include "split-name.h"
-#include "gdbsupport/parallel-for.h"
#include "gdbsupport/thread-pool.h"
+#include "run-on-main-thread.h"
/* When == 1, print basic high level tracing messages.
When > 1, be more verbose.
@@ -757,8 +757,6 @@ static void dwarf2_find_base_address (struct die_info *die,
static void build_type_psymtabs_reader (cutu_reader *reader,
cooked_index_storage *storage);
-static void dwarf2_build_psymtabs_hard (dwarf2_per_objfile *per_objfile);
-
static void var_decode_location (struct attribute *attr,
struct symbol *sym,
struct dwarf2_cu *cu);
@@ -3196,7 +3194,8 @@ get_gdb_index_contents_from_cache_dwz (objfile *obj, dwz_file *dwz)
return global_index_cache.lookup_gdb_index (build_id, &dwz->index_cache_res);
}
-static quick_symbol_functions_up make_cooked_index_funcs ();
+static quick_symbol_functions_up make_cooked_index_funcs
+ (dwarf2_per_objfile *);
/* See dwarf2/public.h. */
@@ -3273,31 +3272,11 @@ dwarf2_initialize_objfile (struct objfile *objfile)
}
global_index_cache.miss ();
- objfile->qf.push_front (make_cooked_index_funcs ());
+ objfile->qf.push_front (make_cooked_index_funcs (per_objfile));
}
\f
-/* Build a partial symbol table. */
-
-static void
-dwarf2_build_psymtabs (struct objfile *objfile)
-{
- dwarf2_per_objfile *per_objfile = get_dwarf2_per_objfile (objfile);
-
- if (per_objfile->per_bfd->index_table != nullptr)
- return;
-
- try
- {
- dwarf2_build_psymtabs_hard (per_objfile);
- }
- catch (const gdb_exception_error &except)
- {
- exception_print (gdb_stderr, except);
- }
-}
-
/* Find the base address of the compilation unit for range lists and
location lists. It will normally be specified by DW_AT_low_pc.
In DWARF-3 draft 4, the base address could be overridden by
@@ -3931,7 +3910,7 @@ static struct dwo_unit *
lookup_dwo_unit (dwarf2_cu *cu, die_info *comp_unit_die, const char *dwo_name)
{
#if CXX_STD_THREAD
- /* We need a lock here both to handle the DWO hash table. */
+ /* We need a lock here to handle the DWO hash table. */
static std::mutex dwo_lock;
std::lock_guard<std::mutex> guard (dwo_lock);
@@ -4863,12 +4842,11 @@ process_skeletonless_type_units (dwarf2_per_objfile *per_objfile,
}
}
-/* Build the partial symbol table by doing a quick pass through the
- .debug_info and .debug_abbrev sections. */
-
-static void
-dwarf2_build_psymtabs_hard (dwarf2_per_objfile *per_objfile)
+cooked_index_worker::cooked_index_worker (dwarf2_per_objfile *per_objfile)
+ : m_per_objfile (per_objfile)
{
+ gdb_assert (is_main_thread ());
+
struct objfile *objfile = per_objfile->objfile;
dwarf2_per_bfd *per_bfd = per_objfile->per_bfd;
@@ -4876,113 +4854,254 @@ dwarf2_build_psymtabs_hard (dwarf2_per_objfile *per_objfile)
objfile_name (objfile));
per_bfd->map_info_sections (objfile);
+}
+
+void
+cooked_index_worker::start ()
+{
+ gdb::thread_pool::g_thread_pool->post_task ([=] ()
+ {
+ this->start_reading ();
+ });
+}
+
+void
+cooked_index_worker::process_cus (size_t task_number, unit_iterator first,
+ unit_iterator end)
+{
+ SCOPE_EXIT { bfd_thread_cleanup (); };
+
+ /* Ensure that complaints are handled correctly. */
+ complaint_interceptor complaint_handler;
+
+ std::vector<gdb_exception> errors;
+ cooked_index_storage thread_storage;
+ for (auto inner = first; inner != end; ++inner)
+ {
+ dwarf2_per_cu_data *per_cu = inner->get ();
+ try
+ {
+ process_psymtab_comp_unit (per_cu, m_per_objfile, &thread_storage);
+ }
+ catch (gdb_exception &except)
+ {
+ errors.push_back (std::move (except));
+ }
+ }
- cooked_index_storage index_storage;
- create_all_units (per_objfile);
- build_type_psymtabs (per_objfile, &index_storage);
+ m_results[task_number] = result_type (thread_storage.release (),
+ complaint_handler.release (),
+ std::move (errors));
+}
+
+void
+cooked_index_worker::done_reading ()
+{
+ /* Only handle the scanning results here. Complaints and exceptions
+ can only be dealt with on the main thread. */
std::vector<std::unique_ptr<cooked_index_shard>> indexes;
+ for (auto &one_result : m_results)
+ indexes.push_back (std::move (std::get<0> (one_result)));
+
+ /* This has to wait until we read the CUs, we need the list of DWOs. */
+ process_skeletonless_type_units (m_per_objfile, &m_index_storage);
+
+ indexes.push_back (m_index_storage.release ());
+ indexes.shrink_to_fit ();
+
+ dwarf2_per_bfd *per_bfd = m_per_objfile->per_bfd;
+ cooked_index *table
+ = (gdb::checked_static_cast<cooked_index *>
+ (per_bfd->index_table.get ()));
+ table->set_contents (std::move (indexes));
+}
+
+void
+cooked_index_worker::start_reading ()
+{
+ SCOPE_EXIT { bfd_thread_cleanup (); };
+
+ try
+ {
+ do_reading ();
+ }
+ catch (const gdb_exception &exc)
+ {
+ m_failed = exc;
+ set (cooked_state::CACHE_DONE);
+ }
+}
+
+void
+cooked_index_worker::do_reading ()
+{
+ dwarf2_per_bfd *per_bfd = m_per_objfile->per_bfd;
+
+ create_all_units (m_per_objfile);
+ build_type_psymtabs (m_per_objfile, &m_index_storage);
per_bfd->quick_file_names_table
= create_quick_file_names_table (per_bfd->all_units.size ());
if (!per_bfd->debug_aranges.empty ())
+ read_addrmap_from_aranges (m_per_objfile, &per_bfd->debug_aranges,
+ m_index_storage.get_addrmap (),
+ &m_warnings);
+
+ /* We want to balance the load between the worker threads. This is
+ done by using the size of each CU as a rough estimate of how
+ difficult it will be to operate on. This isn't ideal -- for
+ example if dwz is used, the early CUs will all tend to be
+ "included" and won't be parsed independently. However, this
+ heuristic works well for typical compiler output. */
+
+ size_t total_size = 0;
+ for (const auto &per_cu : per_bfd->all_units)
+ total_size += per_cu->length ();
+
+ /* How many worker threads we plan to use. We may not actually use
+ this many. We use 1 as the minimum to avoid division by zero,
+ and anyway in the N==0 case the work will be done
+ synchronously. */
+ const size_t n_worker_threads
+ = std::max (gdb::thread_pool::g_thread_pool->thread_count (), (size_t) 1);
+
+ /* How much effort should be put into each worker. */
+ const size_t size_per_thread
+ = std::max (total_size / n_worker_threads, (size_t) 1);
+
+ /* Work is done in a task group. */
+ gdb::task_group workers ([this] ()
+ {
+ this->done_reading ();
+ });
+
+ auto end = per_bfd->all_units.end ();
+ size_t task_count = 0;
+ for (auto iter = per_bfd->all_units.begin (); iter != end; )
{
- deferred_warnings warn;
- read_addrmap_from_aranges (per_objfile, &per_bfd->debug_aranges,
- index_storage.get_addrmap (), &warn);
- warn.emit ();
+ auto last = iter;
+ /* Put all remaining CUs into the last task. */
+ if (task_count == n_worker_threads - 1)
+ last = end;
+ else
+ {
+ size_t chunk_size = 0;
+ for (; last != end && chunk_size < size_per_thread; ++last)
+ chunk_size += (*last)->length ();
+ }
+
+ gdb_assert (iter != last);
+ workers.add_task ([=] ()
+ {
+ process_cus (task_count, iter, last);
+ });
+
+ ++task_count;
+ iter = last;
}
+ m_results.resize (task_count);
+ workers.start ();
+}
+
+bool
+cooked_index_worker::wait (cooked_state desired_state, bool allow_quit)
+{
+ bool done;
+#if CXX_STD_THREAD
{
- using iter_type = decltype (per_bfd->all_units.begin ());
+ std::unique_lock<std::mutex> lock (m_mutex);
- auto task_size_ = [] (iter_type iter)
- {
- dwarf2_per_cu_data *per_cu = iter->get ();
- return (size_t)per_cu->length ();
- };
- auto task_size = gdb::make_function_view (task_size_);
-
- /* Each thread returns a tuple holding a cooked index, any
- collected complaints, and a vector of errors that should be
- printed. The latter is done because GDB's I/O system is not
- thread-safe. run_on_main_thread could be used, but that would
- mean the messages are printed after the prompt, which looks
- weird. */
- using result_type = std::tuple<std::unique_ptr<cooked_index_shard>,
- complaint_collection,
- std::vector<gdb_exception>>;
- std::vector<result_type> results
- = gdb::parallel_for_each (1, per_bfd->all_units.begin (),
- per_bfd->all_units.end (),
- [=] (iter_type iter, iter_type end)
- {
- /* Ensure that complaints are handled correctly. */
- complaint_interceptor complaint_handler;
+ /* This may be called from a non-main thread -- this functionality
+ is needed for the index cache -- but in this case we require
+ that the desired state already have been attained. */
+ gdb_assert (is_main_thread () || desired_state <= m_state);
- std::vector<gdb_exception> errors;
- cooked_index_storage thread_storage;
- for (; iter != end; ++iter)
+ while (desired_state > m_state)
+ {
+ if (allow_quit)
{
- dwarf2_per_cu_data *per_cu = iter->get ();
- try
- {
- process_psymtab_comp_unit (per_cu, per_objfile,
- &thread_storage);
- }
- catch (gdb_exception &except)
- {
- errors.push_back (std::move (except));
- }
+ std::chrono::milliseconds duration { 15 };
+ if (m_cond.wait_for (lock, duration) == std::cv_status::timeout)
+ QUIT;
}
- return result_type (thread_storage.release (),
- complaint_handler.release (),
- std::move (errors));
- }, task_size);
-
- /* Only show a given exception a single time. */
- std::unordered_set<gdb_exception> seen_exceptions;
- for (auto &one_result : results)
- {
- indexes.push_back (std::move (std::get<0> (one_result)));
- re_emit_complaints (std::get<1> (one_result));
- for (auto &one_exc : std::get<2> (one_result))
- if (seen_exceptions.insert (one_exc).second)
- exception_print (gdb_stderr, one_exc);
+ else
+ m_cond.wait (lock);
}
+ done = m_state == cooked_state::CACHE_DONE;
}
+#else
+ /* Without threads, all the work is done immediately on the main
+ thread, and there is never anything to wait for. */
+ done = true;
+#endif /* CXX_STD_THREAD */
+
+ /* Only the main thread is allowed to report complaints and the
+ like. */
+ if (!is_main_thread ())
+ return false;
- /* This has to wait until we read the CUs, we need the list of DWOs. */
- process_skeletonless_type_units (per_objfile, &index_storage);
-
- if (dwarf_read_debug > 0)
- print_tu_stats (per_objfile);
-
- indexes.push_back (index_storage.release ());
- indexes.shrink_to_fit ();
+ if (m_reported)
+ return done;
+ m_reported = true;
- cooked_index *vec = new cooked_index (std::move (indexes));
- per_bfd->index_table.reset (vec);
+ /* Emit warnings first, maybe they were emitted before an exception
+ (if any) was thrown. */
+ m_warnings.emit ();
- /* Cannot start writing the index entry until after the
- 'index_table' member has been set. */
- vec->start_writing_index (per_bfd);
+ if (m_failed.has_value ())
+ {
+ /* start_reading failed -- report it. */
+ exception_print (gdb_stderr, *m_failed);
+ m_failed.reset ();
+ return done;
+ }
- const cooked_index_entry *main_entry = vec->get_main ();
- if (main_entry != nullptr)
+ /* Only show a given exception a single time. */
+ std::unordered_set<gdb_exception> seen_exceptions;
+ for (auto &one_result : m_results)
{
- /* We only do this for names not requiring canonicalization. At
- this point in the process names have not been canonicalized.
- However, currently, languages that require this step also do
- not use DW_AT_main_subprogram. An assert is appropriate here
- because this filtering is done in get_main. */
- enum language lang = main_entry->per_cu->lang ();
- gdb_assert (!language_requires_canonicalization (lang));
- const char *full_name = main_entry->full_name (&per_bfd->obstack, true);
- set_objfile_main_name (objfile, full_name, lang);
+ re_emit_complaints (std::get<1> (one_result));
+ for (auto &one_exc : std::get<2> (one_result))
+ if (seen_exceptions.insert (one_exc).second)
+ exception_print (gdb_stderr, one_exc);
}
+ if (dwarf_read_debug > 0)
+ print_tu_stats (m_per_objfile);
+
+ struct objfile *objfile = m_per_objfile->objfile;
+ dwarf2_per_bfd *per_bfd = m_per_objfile->per_bfd;
+ cooked_index *table
+ = (gdb::checked_static_cast<cooked_index *>
+ (per_bfd->index_table.get ()));
+
+ auto_obstack temp_storage;
+ enum language lang = language_unknown;
+ const char *main_name = table->get_main_name (&temp_storage, &lang);
+ if (main_name != nullptr)
+ set_objfile_main_name (objfile, main_name, lang);
+
dwarf_read_debug_printf ("Done building psymtabs of %s",
objfile_name (objfile));
+
+ return done;
+}
+
+void
+cooked_index_worker::set (cooked_state desired_state)
+{
+ gdb_assert (desired_state != cooked_state::INITIAL);
+
+#if CXX_STD_THREAD
+ std::lock_guard<std::mutex> guard (m_mutex);
+ gdb_assert (desired_state > m_state);
+ m_state = desired_state;
+ m_cond.notify_one ();
+#else
+ /* Without threads, all the work is done immediately on the main
+ thread, and there is never anything to do. */
+#endif /* CXX_STD_THREAD */
}
static void
@@ -16458,26 +16577,60 @@ cooked_indexer::make_index (cutu_reader *reader)
struct cooked_index_functions : public dwarf2_base_index_functions
{
+ cooked_index *wait (struct objfile *objfile, bool allow_quit)
+ {
+ dwarf2_per_objfile *per_objfile = get_dwarf2_per_objfile (objfile);
+ cooked_index *table
+ = (gdb::checked_static_cast<cooked_index *>
+ (per_objfile->per_bfd->index_table.get ()));
+ table->wait (cooked_state::MAIN_AVAILABLE, allow_quit);
+ return table;
+ }
+
dwarf2_per_cu_data *find_per_cu (dwarf2_per_bfd *per_bfd,
CORE_ADDR adjusted_pc) override;
struct compunit_symtab *find_compunit_symtab_by_address
(struct objfile *objfile, CORE_ADDR address) override;
- void dump (struct objfile *objfile) override
+ bool has_unexpanded_symtabs (struct objfile *objfile) override
{
- dwarf2_per_objfile *per_objfile = get_dwarf2_per_objfile (objfile);
- cooked_index *index
- = (gdb::checked_static_cast<cooked_index *>
- (per_objfile->per_bfd->index_table.get ()));
- if (index == nullptr)
- return;
+ wait (objfile, true);
+ return dwarf2_base_index_functions::has_unexpanded_symtabs (objfile);
+ }
+ struct symtab *find_last_source_symtab (struct objfile *objfile) override
+ {
+ wait (objfile, true);
+ return dwarf2_base_index_functions::find_last_source_symtab (objfile);
+ }
+
+ void forget_cached_source_info (struct objfile *objfile) override
+ {
+ wait (objfile, true);
+ dwarf2_base_index_functions::forget_cached_source_info (objfile);
+ }
+
+ void print_stats (struct objfile *objfile, bool print_bcache) override
+ {
+ wait (objfile, true);
+ dwarf2_base_index_functions::print_stats (objfile, print_bcache);
+ }
+
+ void dump (struct objfile *objfile) override
+ {
+ cooked_index *index = wait (objfile, true);
gdb_printf ("Cooked index in use:\n");
gdb_printf ("\n");
index->dump (objfile->arch ());
}
+ void expand_all_symtabs (struct objfile *objfile) override
+ {
+ wait (objfile, true);
+ dwarf2_base_index_functions::expand_all_symtabs (objfile);
+ }
+
bool expand_symtabs_matching
(struct objfile *objfile,
gdb::function_view<expand_symtabs_file_matcher_ftype> file_matcher,
@@ -16488,55 +16641,28 @@ struct cooked_index_functions : public dwarf2_base_index_functions
domain_enum domain,
enum search_domain kind) override;
- bool can_lazily_read_symbols () override
+ struct compunit_symtab *find_pc_sect_compunit_symtab
+ (struct objfile *objfile, struct bound_minimal_symbol msymbol,
+ CORE_ADDR pc, struct obj_section *section, int warn_if_readin) override
{
- return true;
+ wait (objfile, true);
+ return (dwarf2_base_index_functions::find_pc_sect_compunit_symtab
+ (objfile, msymbol, pc, section, warn_if_readin));
}
- void read_partial_symbols (struct objfile *objfile) override
+ void map_symbol_filenames
+ (struct objfile *objfile,
+ gdb::function_view<symbol_filename_ftype> fun,
+ bool need_fullname) override
{
- if (dwarf2_has_info (objfile, nullptr))
- dwarf2_build_psymtabs (objfile);
+ wait (objfile, true);
+ return (dwarf2_base_index_functions::map_symbol_filenames
+ (objfile, fun, need_fullname));
}
- enum language lookup_global_symbol_language (struct objfile *objfile,
- const char *name,
- domain_enum domain,
- bool *symbol_found_p) override
+ void compute_main_name (struct objfile *objfile) override
{
- *symbol_found_p = false;
-
- if (!(domain == VAR_DOMAIN && streq (name, "main")))
- return language_unknown;
-
- dwarf2_per_objfile *per_objfile = get_dwarf2_per_objfile (objfile);
- struct dwarf2_per_bfd *per_bfd = per_objfile->per_bfd;
- if (per_bfd->index_table == nullptr)
- return language_unknown;
-
- /* Expansion of large CUs can be slow. By returning the language of main
- here for C and C++, we avoid CU expansion during set_initial_language.
- But by doing a symbol lookup in the cooked index, we are forced to wait
- for finalization to complete. See PR symtab/30174 for ideas how to
- bypass that as well. */
- cooked_index *table
- = (gdb::checked_static_cast<cooked_index *>
- (per_bfd->index_table.get ()));
-
- for (const cooked_index_entry *entry : table->find (name, false))
- {
- if (entry->tag != DW_TAG_subprogram)
- continue;
-
- enum language lang = entry->per_cu->lang ();
- if (!(lang == language_c || lang == language_cplus))
- continue;
-
- *symbol_found_p = true;
- return lang;
- }
-
- return language_unknown;
+ wait (objfile, false);
}
};
@@ -16547,8 +16673,6 @@ cooked_index_functions::find_per_cu (dwarf2_per_bfd *per_bfd,
cooked_index *table
= (gdb::checked_static_cast<cooked_index *>
(per_bfd->index_table.get ()));
- if (table == nullptr)
- return nullptr;
return table->lookup (adjusted_pc);
}
@@ -16560,11 +16684,7 @@ cooked_index_functions::find_compunit_symtab_by_address
return nullptr;
dwarf2_per_objfile *per_objfile = get_dwarf2_per_objfile (objfile);
- cooked_index *table
- = (gdb::checked_static_cast<cooked_index *>
- (per_objfile->per_bfd->index_table.get ()));
- if (table == nullptr)
- return nullptr;
+ cooked_index *table = wait (objfile, true);
CORE_ADDR baseaddr = objfile->data_section_offset ();
dwarf2_per_cu_data *per_cu = table->lookup (address - baseaddr);
@@ -16587,13 +16707,7 @@ cooked_index_functions::expand_symtabs_matching
{
dwarf2_per_objfile *per_objfile = get_dwarf2_per_objfile (objfile);
- cooked_index *table
- = (gdb::checked_static_cast<cooked_index *>
- (per_objfile->per_bfd->index_table.get ()));
- if (table == nullptr)
- return true;
-
- table->wait ();
+ cooked_index *table = wait (objfile, true);
dw_expand_symtabs_matching_file_matcher (per_objfile, file_matcher);
@@ -16713,15 +16827,27 @@ cooked_index_functions::expand_symtabs_matching
/* Return a new cooked_index_functions object. */
static quick_symbol_functions_up
-make_cooked_index_funcs ()
+make_cooked_index_funcs (dwarf2_per_objfile *per_objfile)
{
+ /* Set the index table early so that sharing works even while
+ scanning; and then start the scanning. */
+ dwarf2_per_bfd *per_bfd = per_objfile->per_bfd;
+ cooked_index *idx = new cooked_index (per_objfile);
+ per_bfd->index_table.reset (idx);
+ /* Don't start reading until after 'index_table' is set. This
+ avoids races. */
+ idx->start_reading ();
+
+ if (dwarf_synchronous)
+ idx->wait_completely ();
+
return quick_symbol_functions_up (new cooked_index_functions);
}
quick_symbol_functions_up
cooked_index::make_quick_functions () const
{
- return make_cooked_index_funcs ();
+ return quick_symbol_functions_up (new cooked_index_functions);
}
\f
diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
index 4ab869cdd29..493d64acf68 100644
--- a/gdb/dwarf2/read.h
+++ b/gdb/dwarf2/read.h
@@ -898,7 +898,7 @@ struct dwarf2_base_index_functions : public quick_symbol_functions
struct compunit_symtab *find_pc_sect_compunit_symtab
(struct objfile *objfile, struct bound_minimal_symbol msymbol,
CORE_ADDR pc, struct obj_section *section, int warn_if_readin)
- override final;
+ override;
struct compunit_symtab *find_compunit_symtab_by_address
(struct objfile *objfile, CORE_ADDR address) override
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-error.exp b/gdb/testsuite/gdb.dwarf2/dw2-error.exp
index 76886d5c1b6..f8dc08d47aa 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-error.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-error.exp
@@ -31,6 +31,7 @@ if {[build_executable $testfile.exp $testfile $srcfile {nodebug quiet}]} {
clean_restart
gdb_test_no_output "set breakpoint pending off"
+gdb_test_no_output "maint set dwarf synchronous on"
set host_binfile [gdb_remote_download host $binfile]
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-missing-cu-tag.exp b/gdb/testsuite/gdb.dwarf2/dw2-missing-cu-tag.exp
index f57e8086a7c..f24c7938568 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-missing-cu-tag.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-missing-cu-tag.exp
@@ -49,6 +49,8 @@ set host_binfile [gdb_remote_download host $binfile]
# Restart with no executable.
clean_restart
+gdb_test_no_output "maint set dwarf synchronous on"
+
# This pattern is hit when GDB does not use -readnow (i.e. the default
# behaviour).
set pattern1 \
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-stack-boundary.exp b/gdb/testsuite/gdb.dwarf2/dw2-stack-boundary.exp
index a72564c075c..65b91b3a504 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-stack-boundary.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-stack-boundary.exp
@@ -25,6 +25,8 @@ if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" object {}] != ""
clean_restart
+gdb_test_no_output "maint set dwarf synchronous on"
+
set host_binfile [gdb_remote_download host $binfile]
gdb_test_no_output "set complaints 100"
set w1 0
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-using-debug-str.exp b/gdb/testsuite/gdb.dwarf2/dw2-using-debug-str.exp
index 7974cb7f20b..ec5d3183f60 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-using-debug-str.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-using-debug-str.exp
@@ -128,6 +128,8 @@ if {[run_on_host "objcopy" [gdb_find_objcopy] "$args"]} {
# executable we're going to get an error, which we check for below.
clean_restart
+gdb_test_no_output "maint set dwarf synchronous on"
+
set line1 "Reading symbols from \[^\r\n\]+"
set dwarf_error "Dwarf Error: DW_FORM_strp used without required section"
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-zero-range.exp b/gdb/testsuite/gdb.dwarf2/dw2-zero-range.exp
index 7bef05684f3..38c85a47b80 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-zero-range.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-zero-range.exp
@@ -124,7 +124,7 @@ foreach_with_prefix ranges_sect {ranges rnglists} {
with_complaints 1 {
set test "Zero address complaint - relocated - psymtab"
set have_complaint 0
- gdb_test_multiple "sharedlibrary [file tail $lib1]" $test {
+ gdb_test_multiple "maint with dwarf synchronous on -- sharedlibrary [file tail $lib1]" $test {
-re -wrap $re {
set have_complaint 1
}
@@ -144,12 +144,14 @@ foreach_with_prefix ranges_sect {ranges rnglists} {
clean_restart
# Test for presence of complaint, with lib1 unrelocated.
+ gdb_test_no_output "maint set dwarf synchronous on"
with_complaints 1 {
gdb_load $lib1
set test "Zero address complaint - unrelocated - psymtab"
set have_complaint [regexp $re.* $gdb_file_cmd_msg]
gdb_assert { $have_complaint } $test
}
+ gdb_test_no_output "maint set dwarf synchronous off"
if { ! $readnow_p } {
with_complaints 1 {
diff --git a/gdb/testsuite/gdb.dwarf2/fission-reread.exp b/gdb/testsuite/gdb.dwarf2/fission-reread.exp
index 01e9eada575..884a8359fa5 100644
--- a/gdb/testsuite/gdb.dwarf2/fission-reread.exp
+++ b/gdb/testsuite/gdb.dwarf2/fission-reread.exp
@@ -60,7 +60,10 @@ pass "$testfile - unload"
# Test-case for PR24620: Delete the .dwo file and verify that
# save gdb-index doesn't crash.
remote_file target delete $dwo
-clean_restart $binfile
+save_vars { GDBFLAGS } {
+ append GDBFLAGS " -iex \"maint set dwarf synchronous on\""
+ clean_restart $binfile
+}
set output_dir [standard_output_file ""]
set cmd "save gdb-index"
gdb_test_multiple "$cmd $output_dir" $cmd {
diff --git a/gdb/testsuite/gdb.dwarf2/no-gnu-debuglink.exp b/gdb/testsuite/gdb.dwarf2/no-gnu-debuglink.exp
index a76136fdfd8..ff9c4dd142d 100644
--- a/gdb/testsuite/gdb.dwarf2/no-gnu-debuglink.exp
+++ b/gdb/testsuite/gdb.dwarf2/no-gnu-debuglink.exp
@@ -36,6 +36,7 @@ if { [build_executable $testfile.exp $testfile [list $srcfile $asm_file]] } {
}
clean_restart
+gdb_test_no_output "maint set dwarf synchronous on"
set msg "\r\nwarning: could not find '\.gnu_debugaltlink' file for \[^\r\n\]*"
gdb_test "file $binfile" "$msg" "file command"
diff --git a/gdb/testsuite/gdb.dwarf2/struct-with-sig-2.exp b/gdb/testsuite/gdb.dwarf2/struct-with-sig-2.exp
index f6877fc6a17..8bf200e8902 100644
--- a/gdb/testsuite/gdb.dwarf2/struct-with-sig-2.exp
+++ b/gdb/testsuite/gdb.dwarf2/struct-with-sig-2.exp
@@ -119,9 +119,13 @@ Dwarf::assemble $asm_file {
}
}
-if { [prepare_for_testing "failed to prepare" ${testfile} \
- [list $srcfile $asm_file] {nodebug}] } {
- return -1
+save_vars { GDBFLAGS } {
+ append GDBFLAGS " -iex \"maint set dwarf synchronous on\""
+
+ if { [prepare_for_testing "failed to prepare" ${testfile} \
+ [list $srcfile $asm_file] {nodebug}] } {
+ return -1
+ }
}
set re "Dwarf Error: .debug_types section not supported in dwz file"
--
2.43.0
^ permalink raw reply [flat|nested] 25+ messages in thread* [PATCH v4 13/19] Simplify the public DWARF API
2023-12-10 21:41 [PATCH v4 00/19] Index DWARF in the background Tom Tromey
` (11 preceding siblings ...)
2023-12-10 21:41 ` [PATCH v4 12/19] Do more DWARF reading in the background Tom Tromey
@ 2023-12-10 21:41 ` Tom Tromey
2023-12-10 21:41 ` [PATCH v4 14/19] Remove two quick_symbol_functions methods Tom Tromey
` (6 subsequent siblings)
19 siblings, 0 replies; 25+ messages in thread
From: Tom Tromey @ 2023-12-10 21:41 UTC (permalink / raw)
To: gdb-patches
dwarf2_has_info and dwarf2_initialize_objfile are only separate
because the DWARF reader implemented lazy psymtab reading. However,
now that this is gone, we can simplify the public DWARF API again.
---
gdb/coffread.c | 6 +++---
gdb/dwarf2/public.h | 18 +++++++++++-------
gdb/dwarf2/read.c | 49 ++++++++++++++++++++++---------------------------
gdb/elfread.c | 6 ++++--
gdb/machoread.c | 11 ++---------
gdb/xcoffread.c | 3 +--
6 files changed, 43 insertions(+), 50 deletions(-)
diff --git a/gdb/coffread.c b/gdb/coffread.c
index 37363631915..3a64325a822 100644
--- a/gdb/coffread.c
+++ b/gdb/coffread.c
@@ -716,10 +716,10 @@ coff_symfile_read (struct objfile *objfile, symfile_add_flags symfile_flags)
*info->stabsects,
info->stabstrsect->filepos, stabstrsize);
}
- if (dwarf2_has_info (objfile, NULL))
+
+ if (dwarf2_initialize_objfile (objfile))
{
- /* DWARF2 sections. */
- dwarf2_initialize_objfile (objfile);
+ /* Nothing. */
}
/* Try to add separate debug file if no symbols table found. */
diff --git a/gdb/dwarf2/public.h b/gdb/dwarf2/public.h
index 0e74857eb1a..efb754b5fe8 100644
--- a/gdb/dwarf2/public.h
+++ b/gdb/dwarf2/public.h
@@ -20,10 +20,6 @@
#ifndef DWARF2_PUBLIC_H
#define DWARF2_PUBLIC_H
-extern bool dwarf2_has_info (struct objfile *,
- const struct dwarf2_debug_sections *,
- bool = false);
-
/* A DWARF names index variant. */
enum class dw_index_kind
{
@@ -34,9 +30,17 @@ enum class dw_index_kind
DEBUG_NAMES,
};
-/* Initialize for reading DWARF for OBJFILE, and push the appropriate
- entry on the objfile's "qf" list. */
-extern void dwarf2_initialize_objfile (struct objfile *objfile);
+/* Try to locate the sections we need for DWARF 2 debugging
+ information. If these are found, begin reading the DWARF and
+ return true. Otherwise, return false. NAMES points to the dwarf2
+ section names, or is NULL if the standard ELF names are used.
+ CAN_COPY is true for formats where symbol interposition is possible
+ and so symbol values must follow copy relocation rules. */
+
+extern bool dwarf2_initialize_objfile
+ (struct objfile *,
+ const struct dwarf2_debug_sections * = nullptr,
+ bool = false);
extern void dwarf2_build_frame_info (struct objfile *);
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 8a60cf91612..8778bffc485 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -1354,14 +1354,10 @@ dwarf2_per_objfile::set_symtab (const dwarf2_per_cu_data *per_cu,
this->m_symtabs[per_cu->index] = symtab;
}
-/* Try to locate the sections we need for DWARF 2 debugging
- information and return true if we have enough to do something.
- NAMES points to the dwarf2 section names, or is NULL if the standard
- ELF names are used. CAN_COPY is true for formats where symbol
- interposition is possible and so symbol values must follow copy
- relocation rules. */
+/* Helper function for dwarf2_initialize_objfile that creates the
+ per-BFD object. */
-bool
+static bool
dwarf2_has_info (struct objfile *objfile,
const struct dwarf2_debug_sections *names,
bool can_copy)
@@ -3199,9 +3195,14 @@ static quick_symbol_functions_up make_cooked_index_funcs
/* See dwarf2/public.h. */
-void
-dwarf2_initialize_objfile (struct objfile *objfile)
+bool
+dwarf2_initialize_objfile (struct objfile *objfile,
+ const struct dwarf2_debug_sections *names,
+ bool can_copy)
{
+ if (!dwarf2_has_info (objfile, names, can_copy))
+ return false;
+
dwarf2_per_objfile *per_objfile = get_dwarf2_per_objfile (objfile);
dwarf2_per_bfd *per_bfd = per_objfile->per_bfd;
@@ -3231,48 +3232,42 @@ dwarf2_initialize_objfile (struct objfile *objfile)
= create_quick_file_names_table (per_bfd->all_units.size ());
objfile->qf.emplace_front (new readnow_functions);
- return;
}
-
/* Was a GDB index already read when we processed an objfile sharing
PER_BFD? */
- if (per_bfd->index_table != nullptr)
+ else if (per_bfd->index_table != nullptr)
{
dwarf_read_debug_printf ("re-using symbols");
objfile->qf.push_front (per_bfd->index_table->make_quick_functions ());
- return;
}
-
- if (dwarf2_read_debug_names (per_objfile))
+ else if (dwarf2_read_debug_names (per_objfile))
{
dwarf_read_debug_printf ("found debug names");
objfile->qf.push_front
(per_bfd->index_table->make_quick_functions ());
- return;
}
-
- if (dwarf2_read_gdb_index (per_objfile,
- get_gdb_index_contents_from_section<struct dwarf2_per_bfd>,
- get_gdb_index_contents_from_section<dwz_file>))
+ else if (dwarf2_read_gdb_index (per_objfile,
+ get_gdb_index_contents_from_section<struct dwarf2_per_bfd>,
+ get_gdb_index_contents_from_section<dwz_file>))
{
dwarf_read_debug_printf ("found gdb index from file");
objfile->qf.push_front (per_bfd->index_table->make_quick_functions ());
- return;
}
-
/* ... otherwise, try to find the index in the index cache. */
- if (dwarf2_read_gdb_index (per_objfile,
+ else if (dwarf2_read_gdb_index (per_objfile,
get_gdb_index_contents_from_cache,
get_gdb_index_contents_from_cache_dwz))
{
dwarf_read_debug_printf ("found gdb index from cache");
global_index_cache.hit ();
objfile->qf.push_front (per_bfd->index_table->make_quick_functions ());
- return;
}
-
- global_index_cache.miss ();
- objfile->qf.push_front (make_cooked_index_funcs (per_objfile));
+ else
+ {
+ global_index_cache.miss ();
+ objfile->qf.push_front (make_cooked_index_funcs (per_objfile));
+ }
+ return true;
}
\f
diff --git a/gdb/elfread.c b/gdb/elfread.c
index 5dc8f21d7c7..c5ba8837217 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -1198,8 +1198,10 @@ elf_symfile_read_dwarf2 (struct objfile *objfile,
{
bool has_dwarf2 = true;
- if (dwarf2_has_info (objfile, NULL, true))
- dwarf2_initialize_objfile (objfile);
+ if (dwarf2_initialize_objfile (objfile, nullptr, true))
+ {
+ /* Nothing. */
+ }
/* If the file has its own symbol tables it has no separate debug
info. `.dynsym'/`.symtab' go to MSYMBOLS, `.debug_info' goes to
SYMTABS/PSYMTABS. `.gnu_debuglink' may no longer be present with
diff --git a/gdb/machoread.c b/gdb/machoread.c
index 87d30ec8587..f27ce63c24c 100644
--- a/gdb/machoread.c
+++ b/gdb/machoread.c
@@ -785,6 +785,8 @@ macho_symfile_read (struct objfile *objfile, symfile_add_flags symfile_flags)
macho_symfile_read_all_oso at the end of this function. */
gdb::def_vector<asymbol *> symbol_table;
+ dwarf2_initialize_objfile (objfile);
+
/* Get symbols from the symbol table only if the file is an executable.
The symbol table of object files is not relocated and is expected to
be in the executable. */
@@ -822,9 +824,6 @@ macho_symfile_read (struct objfile *objfile, symfile_add_flags symfile_flags)
}
/* Try to read .eh_frame / .debug_frame. */
- /* First, locate these sections. We ignore the result status
- as it only checks for debug info. */
- dwarf2_has_info (objfile, NULL);
dwarf2_build_frame_info (objfile);
/* Check for DSYM file. */
@@ -854,12 +853,6 @@ macho_symfile_read (struct objfile *objfile, symfile_add_flags symfile_flags)
}
}
- if (dwarf2_has_info (objfile, NULL))
- {
- /* DWARF 2 sections */
- dwarf2_initialize_objfile (objfile);
- }
-
/* Then the oso. */
if (!oso_vector.empty ())
macho_symfile_read_all_oso (&oso_vector, objfile, symfile_flags);
diff --git a/gdb/xcoffread.c b/gdb/xcoffread.c
index 856fa134ed7..13ee3105baf 100644
--- a/gdb/xcoffread.c
+++ b/gdb/xcoffread.c
@@ -2871,8 +2871,7 @@ xcoff_initial_scan (struct objfile *objfile, symfile_add_flags symfile_flags)
/* DWARF2 sections. */
- if (dwarf2_has_info (objfile, &dwarf2_xcoff_names))
- dwarf2_initialize_objfile (objfile);
+ dwarf2_initialize_objfile (objfile, &dwarf2_xcoff_names);
}
\f
static void
--
2.43.0
^ permalink raw reply [flat|nested] 25+ messages in thread* [PATCH v4 14/19] Remove two quick_symbol_functions methods
2023-12-10 21:41 [PATCH v4 00/19] Index DWARF in the background Tom Tromey
` (12 preceding siblings ...)
2023-12-10 21:41 ` [PATCH v4 13/19] Simplify the public DWARF API Tom Tromey
@ 2023-12-10 21:41 ` Tom Tromey
2023-12-10 21:41 ` [PATCH v4 15/19] Change current_language to be a macro Tom Tromey
` (5 subsequent siblings)
19 siblings, 0 replies; 25+ messages in thread
From: Tom Tromey @ 2023-12-10 21:41 UTC (permalink / raw)
To: gdb-patches
quick_symbol_functions::read_partial_symbols is no longer implemented,
so both it and quick_symbol_functions::can_lazily_read_symbols can be
removed. This allows for other functions to be removed as well.
Note that SYMFILE_NO_READ is now pretty much dead. I haven't removed
it here -- but could if that's desirable. I tend to think that this
functionality would be better implemented in the core; but whenever I
dive into the non-DWARF readers it is pretty depressing.
---
gdb/objfile-flags.h | 4 ----
gdb/objfiles.h | 14 ------------
gdb/psymtab.c | 1 -
gdb/quick-symbol.h | 14 ------------
gdb/symfile-debug.c | 65 +++++++++++++----------------------------------------
gdb/symfile.c | 4 ----
6 files changed, 16 insertions(+), 86 deletions(-)
diff --git a/gdb/objfile-flags.h b/gdb/objfile-flags.h
index 9dee2ee51a0..74aea1a88d3 100644
--- a/gdb/objfile-flags.h
+++ b/gdb/objfile-flags.h
@@ -44,10 +44,6 @@ enum objfile_flag : unsigned
add-symbol-file command. */
OBJF_USERLOADED = 1 << 2, /* User loaded */
- /* Set if we have tried to read partial symtabs for this objfile.
- This is used to allow lazy reading of partial symtabs. */
- OBJF_PSYMTABS_READ = 1 << 3,
-
/* Set if this is the main symbol file (as opposed to symbol file
for dynamically loaded code). */
OBJF_MAINLINE = 1 << 4,
diff --git a/gdb/objfiles.h b/gdb/objfiles.h
index a7b5a71485e..621342d22b7 100644
--- a/gdb/objfiles.h
+++ b/gdb/objfiles.h
@@ -616,9 +616,6 @@ struct objfile
domain_enum domain,
bool *symbol_found_p);
- /* See quick_symbol_functions. */
- void require_partial_symbols (bool verbose);
-
/* Return the relocation offset applied to SECTION. */
CORE_ADDR section_offset (bfd_section *section) const
{
@@ -703,17 +700,6 @@ struct objfile
section_iterator (sections_end, sections_end)));
}
-private:
-
- /* Ensure that partial symbols have been read and return the "quick" (aka
- partial) symbol functions for this symbol reader. */
- const std::forward_list<quick_symbol_functions_up> &
- qf_require_partial_symbols ()
- {
- this->require_partial_symbols (true);
- return qf;
- }
-
public:
/* The object file's original name as specified by the user,
diff --git a/gdb/psymtab.c b/gdb/psymtab.c
index ca1bcf18c4d..33959073003 100644
--- a/gdb/psymtab.c
+++ b/gdb/psymtab.c
@@ -81,7 +81,6 @@ psymtab_storage::install_psymtab (partial_symtab *pst)
psymtab_storage::partial_symtab_range
psymbol_functions::partial_symbols (struct objfile *objfile)
{
- gdb_assert ((objfile->flags & OBJF_PSYMTABS_READ) != 0);
return m_partial_symtabs->range ();
}
diff --git a/gdb/quick-symbol.h b/gdb/quick-symbol.h
index 56714ba4d6b..4cd234162ae 100644
--- a/gdb/quick-symbol.h
+++ b/gdb/quick-symbol.h
@@ -193,20 +193,6 @@ struct quick_symbol_functions
virtual void compute_main_name (struct objfile *objfile)
{
}
-
- /* Return true if this class can lazily read the symbols. This may
- only return true if there are in fact symbols to be read, because
- this is used in the implementation of 'has_partial_symbols'. */
- virtual bool can_lazily_read_symbols ()
- {
- return false;
- }
-
- /* Read the partial symbols for OBJFILE. This will only ever be
- called if can_lazily_read_symbols returns true. */
- virtual void read_partial_symbols (struct objfile *objfile)
- {
- }
};
typedef std::unique_ptr<quick_symbol_functions> quick_symbol_functions_up;
diff --git a/gdb/symfile-debug.c b/gdb/symfile-debug.c
index 7b73b42b70d..32c3ed9d349 100644
--- a/gdb/symfile-debug.c
+++ b/gdb/symfile-debug.c
@@ -87,11 +87,7 @@ objfile::has_partial_symbols ()
not be present in this objfile. */
for (const auto &iter : qf)
{
- if ((flags & OBJF_PSYMTABS_READ) == 0
- && iter->can_lazily_read_symbols ())
- retval = true;
- else
- retval = iter->has_symbols (this);
+ retval = iter->has_symbols (this);
if (retval)
break;
}
@@ -112,7 +108,7 @@ objfile::has_unexpanded_symtabs ()
objfile_debug_name (this));
bool result = false;
- for (const auto &iter : qf_require_partial_symbols ())
+ for (const auto &iter : qf)
{
if (iter->has_unexpanded_symtabs (this))
{
@@ -137,7 +133,7 @@ objfile::find_last_source_symtab ()
gdb_printf (gdb_stdlog, "qf->find_last_source_symtab (%s)\n",
objfile_debug_name (this));
- for (const auto &iter : qf_require_partial_symbols ())
+ for (const auto &iter : qf)
{
retval = iter->find_last_source_symtab (this);
if (retval != nullptr)
@@ -170,7 +166,7 @@ objfile::forget_cached_source_info ()
}
}
- for (const auto &iter : qf_require_partial_symbols ())
+ for (const auto &iter : qf)
iter->forget_cached_source_info (this);
}
@@ -217,7 +213,7 @@ objfile::map_symtabs_matching_filename
return result;
};
- for (const auto &iter : qf_require_partial_symbols ())
+ for (const auto &iter : qf)
{
if (!iter->expand_symtabs_matching (this,
match_one_filename,
@@ -282,7 +278,7 @@ objfile::lookup_symbol (block_enum kind, const char *name, domain_enum domain)
return true;
};
- for (const auto &iter : qf_require_partial_symbols ())
+ for (const auto &iter : qf)
{
if (!iter->expand_symtabs_matching (this,
nullptr,
@@ -313,7 +309,7 @@ objfile::print_stats (bool print_bcache)
gdb_printf (gdb_stdlog, "qf->print_stats (%s, %d)\n",
objfile_debug_name (this), print_bcache);
- for (const auto &iter : qf_require_partial_symbols ())
+ for (const auto &iter : qf)
iter->print_stats (this, print_bcache);
}
@@ -339,7 +335,7 @@ objfile::expand_symtabs_for_function (const char *func_name)
lookup_name_info base_lookup (func_name, symbol_name_match_type::FULL);
lookup_name_info lookup_name = base_lookup.make_ignore_params ();
- for (const auto &iter : qf_require_partial_symbols ())
+ for (const auto &iter : qf)
iter->expand_symtabs_matching (this,
nullptr,
&lookup_name,
@@ -358,7 +354,7 @@ objfile::expand_all_symtabs ()
gdb_printf (gdb_stdlog, "qf->expand_all_symtabs (%s)\n",
objfile_debug_name (this));
- for (const auto &iter : qf_require_partial_symbols ())
+ for (const auto &iter : qf)
iter->expand_all_symtabs (this);
}
@@ -376,7 +372,7 @@ objfile::expand_symtabs_with_fullname (const char *fullname)
return filename_cmp (basenames ? basename : fullname, filename) == 0;
};
- for (const auto &iter : qf_require_partial_symbols ())
+ for (const auto &iter : qf)
iter->expand_symtabs_matching (this,
file_matcher,
nullptr,
@@ -410,7 +406,7 @@ objfile::expand_symtabs_matching
host_address_to_string (&expansion_notify),
search_domain_name (kind));
- for (const auto &iter : qf_require_partial_symbols ())
+ for (const auto &iter : qf)
if (!iter->expand_symtabs_matching (this, file_matcher, lookup_name,
symbol_matcher, expansion_notify,
search_flags, domain, kind))
@@ -435,7 +431,7 @@ objfile::find_pc_sect_compunit_symtab (struct bound_minimal_symbol msymbol,
host_address_to_string (section),
warn_if_readin);
- for (const auto &iter : qf_require_partial_symbols ())
+ for (const auto &iter : qf)
{
retval = iter->find_pc_sect_compunit_symtab (this, msymbol, pc, section,
warn_if_readin);
@@ -463,7 +459,7 @@ objfile::map_symbol_filenames (gdb::function_view<symbol_filename_ftype> fun,
objfile_debug_name (this),
need_fullname);
- for (const auto &iter : qf_require_partial_symbols ())
+ for (const auto &iter : qf)
iter->map_symbol_filenames (this, fun, need_fullname);
}
@@ -475,7 +471,7 @@ objfile::compute_main_name ()
"qf->compute_main_name (%s)\n",
objfile_debug_name (this));
- for (const auto &iter : qf_require_partial_symbols ())
+ for (const auto &iter : qf)
iter->compute_main_name (this);
}
@@ -489,7 +485,7 @@ objfile::find_compunit_symtab_by_address (CORE_ADDR address)
hex_string (address));
struct compunit_symtab *result = NULL;
- for (const auto &iter : qf_require_partial_symbols ())
+ for (const auto &iter : qf)
{
result = iter->find_compunit_symtab_by_address (this, address);
if (result != nullptr)
@@ -514,7 +510,7 @@ objfile::lookup_global_symbol_language (const char *name,
enum language result = language_unknown;
*symbol_found_p = false;
- for (const auto &iter : qf_require_partial_symbols ())
+ for (const auto &iter : qf)
{
result = iter->lookup_global_symbol_language (this, name, domain,
symbol_found_p);
@@ -525,35 +521,6 @@ objfile::lookup_global_symbol_language (const char *name,
return result;
}
-void
-objfile::require_partial_symbols (bool verbose)
-{
- if ((flags & OBJF_PSYMTABS_READ) == 0)
- {
- flags |= OBJF_PSYMTABS_READ;
-
- bool printed = false;
- for (const auto &iter : qf)
- {
- if (iter->can_lazily_read_symbols ())
- {
- if (verbose && !printed)
- {
- gdb_printf (_("Reading symbols from %ps...\n"),
- styled_string (file_name_style.style (),
- objfile_name (this)));
- printed = true;
- }
- iter->read_partial_symbols (this);
- }
- }
- if (printed && !objfile_has_symbols (this))
- gdb_printf (_("(No debugging symbols found in %ps)\n"),
- styled_string (file_name_style.style (),
- objfile_name (this)));
- }
-}
-
/* Call LOOKUP_FUNC to find the filename of a file containing the separate
debug information matching OBJFILE. If LOOKUP_FUNC does return a
filename then open this file and return a std::pair containing the
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 09aa70be1d5..633d0bd3268 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -790,8 +790,6 @@ read_symbols (struct objfile *objfile, symfile_add_flags add_flags)
add_flags | SYMFILE_NOT_FILENAME, objfile);
}
}
- if ((add_flags & SYMFILE_NO_READ) == 0)
- objfile->require_partial_symbols (false);
}
/* Initialize entry point information for this objfile. */
@@ -2620,8 +2618,6 @@ reread_symbols (int from_tty)
(*objfile->sf->sym_init) (objfile);
clear_complaints ();
- objfile->flags &= ~OBJF_PSYMTABS_READ;
-
/* We are about to read new symbols and potentially also
DWARF information. Some targets may want to pass addresses
read from DWARF DIE's through an adjustment function before
--
2.43.0
^ permalink raw reply [flat|nested] 25+ messages in thread* [PATCH v4 15/19] Change current_language to be a macro
2023-12-10 21:41 [PATCH v4 00/19] Index DWARF in the background Tom Tromey
` (13 preceding siblings ...)
2023-12-10 21:41 ` [PATCH v4 14/19] Remove two quick_symbol_functions methods Tom Tromey
@ 2023-12-10 21:41 ` Tom Tromey
2023-12-10 21:41 ` [PATCH v4 16/19] Lazy language setting Tom Tromey
` (4 subsequent siblings)
19 siblings, 0 replies; 25+ messages in thread
From: Tom Tromey @ 2023-12-10 21:41 UTC (permalink / raw)
To: gdb-patches
This changes the 'current_language' global to be a macro that wraps a
function call. This change will let a subsequent patch introduce lazy
language setting.
---
gdb/language.c | 16 ++++++++++++----
gdb/language.h | 7 ++++++-
2 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/gdb/language.c b/gdb/language.c
index cfd4a621e6b..12a953eaa96 100644
--- a/gdb/language.c
+++ b/gdb/language.c
@@ -79,9 +79,17 @@ enum case_sensitivity case_sensitivity = case_sensitive_on;
/* The current language and language_mode (see language.h). */
-const struct language_defn *current_language = nullptr;
+static const struct language_defn *global_current_language;
enum language_mode language_mode = language_mode_auto;
+/* See language.h. */
+
+const struct language_defn *
+get_current_language ()
+{
+ return global_current_language;
+}
+
/* The language that the user expects to be typing in (the language
of main(), or the last language we notified them about, or C). */
@@ -177,9 +185,9 @@ set_language (const char *language)
/* Found it! Go into manual mode, and use this language. */
language_mode = language_mode_manual;
- current_language = lang;
+ global_current_language = lang;
set_range_case ();
- expected_language = current_language;
+ expected_language = lang;
return;
}
@@ -364,7 +372,7 @@ set_range_case (void)
void
set_language (enum language lang)
{
- current_language = language_def (lang);
+ global_current_language = language_def (lang);
set_range_case ();
}
\f
diff --git a/gdb/language.h b/gdb/language.h
index 6ee8f6160e1..6ff6f5eb95f 100644
--- a/gdb/language.h
+++ b/gdb/language.h
@@ -680,6 +680,11 @@ struct language_defn
(const lookup_name_info &lookup_name) const;
};
+/* Return the current language. Normally code just uses the
+ 'current_language' macro. */
+
+extern const struct language_defn *get_current_language ();
+
/* Pointer to the language_defn for our current language. This pointer
always points to *some* valid struct; it can be used without checking
it for validity.
@@ -696,7 +701,7 @@ struct language_defn
the language of symbol files (e.g. detecting when ".c" files are
C++), it should be a separate setting from the current_language. */
-extern const struct language_defn *current_language;
+#define current_language (get_current_language ())
/* Pointer to the language_defn expected by the user, e.g. the language
of main(), or the language we last mentioned in a message, or C. */
--
2.43.0
^ permalink raw reply [flat|nested] 25+ messages in thread* [PATCH v4 16/19] Lazy language setting
2023-12-10 21:41 [PATCH v4 00/19] Index DWARF in the background Tom Tromey
` (14 preceding siblings ...)
2023-12-10 21:41 ` [PATCH v4 15/19] Change current_language to be a macro Tom Tromey
@ 2023-12-10 21:41 ` Tom Tromey
2023-12-10 21:41 ` [PATCH v4 17/19] Optimize lookup_minimal_symbol_text Tom Tromey
` (3 subsequent siblings)
19 siblings, 0 replies; 25+ messages in thread
From: Tom Tromey @ 2023-12-10 21:41 UTC (permalink / raw)
To: gdb-patches
When gdb starts up with a symbol file, it uses the program's "main" to
decide the "static context" and the initial language. With background
DWARF reading, this means that gdb has to wait for a significant
amount of DWARF to be read synchronously.
This patch introduces lazy language setting. The idea here is that in
many cases, the prompt can show up early, making gdb feel more
responsive.
---
gdb/gdbthread.h | 3 ++-
gdb/language.c | 35 +++++++++++++++++++++++++++++++++++
gdb/language.h | 27 ++++++++++++++++++++-------
gdb/symfile.c | 18 +++++++++++++-----
gdb/thread.c | 8 +++-----
5 files changed, 73 insertions(+), 18 deletions(-)
diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index b2c6baf4432..a8ad11b3468 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -36,6 +36,7 @@ struct symtab;
#include "displaced-stepping.h"
#include "gdbsupport/intrusive_list.h"
#include "thread-fsm.h"
+#include "language.h"
struct inferior;
struct process_stratum_target;
@@ -904,7 +905,7 @@ class scoped_restore_current_thread
/* Save/restore the language as well, because selecting a frame
changes the current language to the frame's language if "set
language auto". */
- enum language m_lang;
+ scoped_restore_current_language m_lang;
};
/* Returns a pointer into the thread_info corresponding to
diff --git a/gdb/language.c b/gdb/language.c
index 12a953eaa96..3fbd735a697 100644
--- a/gdb/language.c
+++ b/gdb/language.c
@@ -80,6 +80,7 @@ enum case_sensitivity case_sensitivity = case_sensitive_on;
/* The current language and language_mode (see language.h). */
static const struct language_defn *global_current_language;
+static lazily_set_language_ftype *lazy_language_setter;
enum language_mode language_mode = language_mode_auto;
/* See language.h. */
@@ -87,9 +88,41 @@ enum language_mode language_mode = language_mode_auto;
const struct language_defn *
get_current_language ()
{
+ if (lazy_language_setter != nullptr)
+ {
+ /* Avoid recursive calls -- set_language refers to
+ current_language. */
+ lazily_set_language_ftype *call = lazy_language_setter;
+ lazy_language_setter = nullptr;
+ call ();
+ }
return global_current_language;
}
+void
+lazily_set_language (lazily_set_language_ftype *fun)
+{
+ lazy_language_setter = fun;
+}
+
+scoped_restore_current_language::scoped_restore_current_language ()
+ : m_lang (global_current_language),
+ m_fun (lazy_language_setter)
+{
+}
+
+scoped_restore_current_language::~scoped_restore_current_language ()
+{
+ /* If both are NULL, then that means dont_restore was called. */
+ if (m_lang != nullptr || m_fun != nullptr)
+ {
+ global_current_language = m_lang;
+ lazy_language_setter = m_fun;
+ if (lazy_language_setter == nullptr)
+ set_range_case ();
+ }
+}
+
/* The language that the user expects to be typing in (the language
of main(), or the last language we notified them about, or C). */
@@ -185,6 +218,7 @@ set_language (const char *language)
/* Found it! Go into manual mode, and use this language. */
language_mode = language_mode_manual;
+ lazy_language_setter = nullptr;
global_current_language = lang;
set_range_case ();
expected_language = lang;
@@ -372,6 +406,7 @@ set_range_case (void)
void
set_language (enum language lang)
{
+ lazy_language_setter = nullptr;
global_current_language = language_def (lang);
set_range_case ();
}
diff --git a/gdb/language.h b/gdb/language.h
index 6ff6f5eb95f..6cbcd119dac 100644
--- a/gdb/language.h
+++ b/gdb/language.h
@@ -787,6 +787,10 @@ extern void language_info ();
extern void set_language (enum language lang);
+typedef void lazily_set_language_ftype ();
+extern void lazily_set_language (lazily_set_language_ftype *fun);
+\f
+
/* Test a character to decide whether it can be printed in literal form
or needs to be printed in another representation. For example,
in C the literal form of the character with octal value 141 is 'a'
@@ -837,14 +841,14 @@ class scoped_restore_current_language
{
public:
- explicit scoped_restore_current_language ()
- : m_lang (current_language->la_language)
- {
- }
+ scoped_restore_current_language ();
+ ~scoped_restore_current_language ();
- ~scoped_restore_current_language ()
+ scoped_restore_current_language (scoped_restore_current_language &&other)
{
- set_language (m_lang);
+ m_lang = other.m_lang;
+ m_fun = other.m_fun;
+ other.dont_restore ();
}
scoped_restore_current_language (const scoped_restore_current_language &)
@@ -852,9 +856,18 @@ class scoped_restore_current_language
scoped_restore_current_language &operator=
(const scoped_restore_current_language &) = delete;
+ /* Cancel restoring on scope exit. */
+ void dont_restore ()
+ {
+ /* This is implemented using a sentinel value. */
+ m_lang = nullptr;
+ m_fun = nullptr;
+ }
+
private:
- enum language m_lang;
+ const language_defn *m_lang;
+ lazily_set_language_ftype *m_fun;
};
/* If language_mode is language_mode_auto,
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 633d0bd3268..f84ec34ffde 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -1682,13 +1682,11 @@ symbol_file_command (const char *args, int from_tty)
}
}
-/* Set the initial language. */
+/* Lazily set the initial language. */
-void
-set_initial_language (void)
+static void
+set_initial_language_callback ()
{
- if (language_mode == language_mode_manual)
- return;
enum language lang = main_language ();
/* Make C the default language. */
enum language default_lang = language_c;
@@ -1713,6 +1711,16 @@ set_initial_language (void)
expected_language = current_language; /* Don't warn the user. */
}
+/* Set the initial language. */
+
+void
+set_initial_language (void)
+{
+ if (language_mode == language_mode_manual)
+ return;
+ lazily_set_language (set_initial_language_callback);
+}
+
/* Open the file specified by NAME and hand it off to BFD for
preliminary analysis. Return a newly initialized bfd *, which
includes a newly malloc'd` copy of NAME (tilde-expanded and made
diff --git a/gdb/thread.c b/gdb/thread.c
index c0ed64e5f8b..b27d8d721b2 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -1428,13 +1428,13 @@ scoped_restore_current_thread::restore ()
&& target_has_stack ()
&& target_has_memory ())
restore_selected_frame (m_selected_frame_id, m_selected_frame_level);
-
- set_language (m_lang);
}
scoped_restore_current_thread::~scoped_restore_current_thread ()
{
- if (!m_dont_restore)
+ if (m_dont_restore)
+ m_lang.dont_restore ();
+ else
restore ();
}
@@ -1442,8 +1442,6 @@ scoped_restore_current_thread::scoped_restore_current_thread ()
{
m_inf = inferior_ref::new_reference (current_inferior ());
- m_lang = current_language->la_language;
-
if (inferior_ptid != null_ptid)
{
m_thread = thread_info_ref::new_reference (inferior_thread ());
--
2.43.0
^ permalink raw reply [flat|nested] 25+ messages in thread* [PATCH v4 17/19] Optimize lookup_minimal_symbol_text
2023-12-10 21:41 [PATCH v4 00/19] Index DWARF in the background Tom Tromey
` (15 preceding siblings ...)
2023-12-10 21:41 ` [PATCH v4 16/19] Lazy language setting Tom Tromey
@ 2023-12-10 21:41 ` Tom Tromey
2023-12-10 21:41 ` [PATCH v4 18/19] Avoid language-based lookups in startup path Tom Tromey
` (2 subsequent siblings)
19 siblings, 0 replies; 25+ messages in thread
From: Tom Tromey @ 2023-12-10 21:41 UTC (permalink / raw)
To: gdb-patches
lookup_minimal_symbol_text always loops over all objfiles, even when
an objfile is passed in as an argument. This patch changes the
function to loop over the minimal number of objfiles in the latter
situation.
---
gdb/minsyms.c | 69 +++++++++++++++++++++++++++++++++++------------------------
1 file changed, 41 insertions(+), 28 deletions(-)
diff --git a/gdb/minsyms.c b/gdb/minsyms.c
index 71e22ce1a90..0e24da918d9 100644
--- a/gdb/minsyms.c
+++ b/gdb/minsyms.c
@@ -623,38 +623,51 @@ lookup_minimal_symbol_text (const char *name, struct objfile *objf)
unsigned int hash = msymbol_hash (name) % MINIMAL_SYMBOL_HASH_SIZE;
- for (objfile *objfile : current_program_space->objfiles ())
- {
- if (found_symbol.minsym != NULL)
- break;
+ auto search = [&] (struct objfile *objfile)
+ {
+ for (msymbol = objfile->per_bfd->msymbol_hash[hash];
+ msymbol != NULL && found_symbol.minsym == NULL;
+ msymbol = msymbol->hash_next)
+ {
+ if (strcmp (msymbol->linkage_name (), name) == 0 &&
+ (msymbol->type () == mst_text
+ || msymbol->type () == mst_text_gnu_ifunc
+ || msymbol->type () == mst_file_text))
+ {
+ switch (msymbol->type ())
+ {
+ case mst_file_text:
+ found_file_symbol.minsym = msymbol;
+ found_file_symbol.objfile = objfile;
+ break;
+ default:
+ found_symbol.minsym = msymbol;
+ found_symbol.objfile = objfile;
+ break;
+ }
+ }
+ }
+ };
- if (objf == NULL || objf == objfile
- || objf == objfile->separate_debug_objfile_backlink)
+ if (objf == nullptr)
+ {
+ for (objfile *objfile : current_program_space->objfiles ())
{
- for (msymbol = objfile->per_bfd->msymbol_hash[hash];
- msymbol != NULL && found_symbol.minsym == NULL;
- msymbol = msymbol->hash_next)
- {
- if (strcmp (msymbol->linkage_name (), name) == 0 &&
- (msymbol->type () == mst_text
- || msymbol->type () == mst_text_gnu_ifunc
- || msymbol->type () == mst_file_text))
- {
- switch (msymbol->type ())
- {
- case mst_file_text:
- found_file_symbol.minsym = msymbol;
- found_file_symbol.objfile = objfile;
- break;
- default:
- found_symbol.minsym = msymbol;
- found_symbol.objfile = objfile;
- break;
- }
- }
- }
+ if (found_symbol.minsym != NULL)
+ break;
+ search (objfile);
}
}
+ else
+ {
+ for (objfile *objfile : objf->separate_debug_objfiles ())
+ {
+ if (found_symbol.minsym != NULL)
+ break;
+ search (objfile);
+ }
+ }
+
/* External symbols are best. */
if (found_symbol.minsym)
return found_symbol;
--
2.43.0
^ permalink raw reply [flat|nested] 25+ messages in thread* [PATCH v4 18/19] Avoid language-based lookups in startup path
2023-12-10 21:41 [PATCH v4 00/19] Index DWARF in the background Tom Tromey
` (16 preceding siblings ...)
2023-12-10 21:41 ` [PATCH v4 17/19] Optimize lookup_minimal_symbol_text Tom Tromey
@ 2023-12-10 21:41 ` Tom Tromey
2023-12-10 21:41 ` [PATCH v4 19/19] Back out some parallel_for_each features Tom Tromey
2024-01-09 1:08 ` [PATCH v4 00/19] Index DWARF in the background Tom Tromey
19 siblings, 0 replies; 25+ messages in thread
From: Tom Tromey @ 2023-12-10 21:41 UTC (permalink / raw)
To: gdb-patches
The previous patches are nearly enough to enable background DWARF
reading. However, this hack in language_defn::get_symbol_name_matcher
causes an early computation of current_language:
/* If currently in Ada mode, and the lookup name is wrapped in
'<...>', hijack all symbol name comparisons using the Ada
matcher, which handles the verbatim matching. */
if (current_language->la_language == language_ada
&& lookup_name.ada ().verbatim_p ())
return current_language->get_symbol_name_matcher_inner (lookup_name);
I considered various options here -- reversing the order of the
checks, or promoting the verbatim mode to not be a purely Ada feature
-- but in the end found that the few calls to this during startup
could be handled more directly.
In the JIT code, and in create_exception_master_breakpoint_hook, gdb
is really looking for a certain kind of symbol (text or data) using a
linkage name. Changing the lookup here is clearer and probably more
efficient as well.
In create_std_terminate_master_breakpoint, the lookup can't really be
done by linkage name (it would require relying on a certain mangling
scheme, and also may trip over versioned symbols) -- but we know that
this spot is C++-specific, and so the language ought to be temporarily
set to C++ here.
After this patch, the "file" case is much faster:
(gdb) file /tmp/gdb
2023-10-23 13:16:54.456 - command started
Reading symbols from /tmp/gdb...
2023-10-23 13:16:54.520 - command finished
Command execution time: 0.225906 (cpu), 0.064313 (wall)
---
gdb/breakpoint.c | 4 +++-
gdb/jit.c | 4 ++--
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 699919e32b3..94aafea15a3 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -3733,6 +3733,8 @@ create_std_terminate_master_breakpoint (void)
const char *const func_name = "std::terminate()";
scoped_restore_current_program_space restore_pspace;
+ scoped_restore_current_language save_language;
+ set_language (language_cplus);
for (struct program_space *pspace : program_spaces)
{
@@ -3845,7 +3847,7 @@ create_exception_master_breakpoint_hook (objfile *objfile)
{
struct bound_minimal_symbol debug_hook;
- debug_hook = lookup_minimal_symbol (func_name, NULL, objfile);
+ debug_hook = lookup_minimal_symbol_text (func_name, objfile);
if (debug_hook.minsym == NULL)
{
bp_objfile_data->exception_msym.minsym = &msym_not_found;
diff --git a/gdb/jit.c b/gdb/jit.c
index 0e3a4285bdd..85a10be3055 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -880,7 +880,7 @@ jit_breakpoint_re_set_internal (struct gdbarch *gdbarch, program_space *pspace)
/* Lookup the registration symbol. If it is missing, then we
assume we are not attached to a JIT. */
bound_minimal_symbol reg_symbol
- = lookup_minimal_symbol (jit_break_name, nullptr, the_objfile);
+ = lookup_minimal_symbol_text (jit_break_name, the_objfile);
if (reg_symbol.minsym == NULL
|| reg_symbol.value_address () == 0)
{
@@ -890,7 +890,7 @@ jit_breakpoint_re_set_internal (struct gdbarch *gdbarch, program_space *pspace)
}
bound_minimal_symbol desc_symbol
- = lookup_minimal_symbol (jit_descriptor_name, NULL, the_objfile);
+ = lookup_minimal_symbol_linkage (jit_descriptor_name, the_objfile);
if (desc_symbol.minsym == NULL
|| desc_symbol.value_address () == 0)
{
--
2.43.0
^ permalink raw reply [flat|nested] 25+ messages in thread* [PATCH v4 19/19] Back out some parallel_for_each features
2023-12-10 21:41 [PATCH v4 00/19] Index DWARF in the background Tom Tromey
` (17 preceding siblings ...)
2023-12-10 21:41 ` [PATCH v4 18/19] Avoid language-based lookups in startup path Tom Tromey
@ 2023-12-10 21:41 ` Tom Tromey
2024-01-09 1:08 ` [PATCH v4 00/19] Index DWARF in the background Tom Tromey
19 siblings, 0 replies; 25+ messages in thread
From: Tom Tromey @ 2023-12-10 21:41 UTC (permalink / raw)
To: gdb-patches
Now that the DWARF reader does not use parallel_for_each, we can
remove some of the features that were added just for it: return values
and task sizing.
The thread_pool typed tasks feature could also be removed, but I
haven't done so here. This one seemed less intrusive and perhaps more
likely to be needed at some point.
---
gdb/unittests/parallel-for-selftests.c | 47 -------
gdbsupport/parallel-for.h | 234 +++++----------------------------
2 files changed, 30 insertions(+), 251 deletions(-)
diff --git a/gdb/unittests/parallel-for-selftests.c b/gdb/unittests/parallel-for-selftests.c
index 63e9512ea18..a957b2daa3e 100644
--- a/gdb/unittests/parallel-for-selftests.c
+++ b/gdb/unittests/parallel-for-selftests.c
@@ -120,34 +120,6 @@ TEST (int n_threads)
});
SELF_CHECK (counter == 0);
- auto task_size_max_ = [] (int iter)
- {
- return (size_t)SIZE_MAX;
- };
- auto task_size_max = gdb::make_function_view (task_size_max_);
-
- counter = 0;
- FOR_EACH (1, 0, NUMBER,
- [&] (int start, int end)
- {
- counter += end - start;
- }, task_size_max);
- SELF_CHECK (counter == NUMBER);
-
- auto task_size_one_ = [] (int iter)
- {
- return (size_t)1;
- };
- auto task_size_one = gdb::make_function_view (task_size_one_);
-
- counter = 0;
- FOR_EACH (1, 0, NUMBER,
- [&] (int start, int end)
- {
- counter += end - start;
- }, task_size_one);
- SELF_CHECK (counter == NUMBER);
-
#undef NUMBER
/* Check that if there are fewer tasks than threads, then we won't
@@ -169,25 +141,6 @@ TEST (int n_threads)
{
return entry != nullptr;
}));
-
- /* The same but using the task size parameter. */
- intresults.clear ();
- any_empty_tasks = false;
- FOR_EACH (1, 0, 1,
- [&] (int start, int end)
- {
- if (start == end)
- any_empty_tasks = true;
- return std::make_unique<int> (end - start);
- },
- task_size_one);
- SELF_CHECK (!any_empty_tasks);
- SELF_CHECK (std::all_of (intresults.begin (),
- intresults.end (),
- [] (const std::unique_ptr<int> &entry)
- {
- return entry != nullptr;
- }));
}
#endif /* FOR_EACH */
diff --git a/gdbsupport/parallel-for.h b/gdbsupport/parallel-for.h
index ee7bfd948e2..1825dbc69b1 100644
--- a/gdbsupport/parallel-for.h
+++ b/gdbsupport/parallel-for.h
@@ -28,104 +28,6 @@
namespace gdb
{
-namespace detail
-{
-
-/* This is a helper class that is used to accumulate results for
- parallel_for. There is a specialization for 'void', below. */
-template<typename T>
-struct par_for_accumulator
-{
-public:
-
- explicit par_for_accumulator (size_t n_threads)
- : m_futures (n_threads)
- {
- }
-
- /* The result type that is accumulated. */
- typedef std::vector<T> result_type;
-
- /* Post the Ith task to a background thread, and store a future for
- later. */
- void post (size_t i, std::function<T ()> task)
- {
- m_futures[i]
- = gdb::thread_pool::g_thread_pool->post_task (std::move (task));
- }
-
- /* Invoke TASK in the current thread, then compute all the results
- from all background tasks and put them into a result vector,
- which is returned. */
- result_type finish (gdb::function_view<T ()> task)
- {
- result_type result (m_futures.size () + 1);
-
- result.back () = task ();
-
- for (size_t i = 0; i < m_futures.size (); ++i)
- result[i] = m_futures[i].get ();
-
- 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
- background. */
- std::vector<gdb::future<T>> m_futures;
-};
-
-/* See the generic template. */
-template<>
-struct par_for_accumulator<void>
-{
-public:
-
- explicit par_for_accumulator (size_t n_threads)
- : m_futures (n_threads)
- {
- }
-
- /* This specialization does not compute results. */
- typedef void result_type;
-
- void post (size_t i, std::function<void ()> task)
- {
- m_futures[i]
- = gdb::thread_pool::g_thread_pool->post_task (std::move (task));
- }
-
- result_type finish (gdb::function_view<void ()> task)
- {
- task ();
-
- for (auto &future : m_futures)
- {
- /* Use 'get' and not 'wait', to propagate any exception. */
- future.get ();
- }
- }
-
- /* Resize the results to N. */
- void resize (size_t n)
- {
- m_futures.resize (n);
- }
-
-private:
-
- std::vector<gdb::future<void>> m_futures;
-};
-
-}
-
/* A very simple "parallel for". This splits the range of iterators
into subranges, and then passes each subrange to the callback. The
work may or may not be done in separate threads.
@@ -136,23 +38,13 @@ struct par_for_accumulator<void>
The parameter N says how batching ought to be done -- there will be
at least N elements processed per thread. Setting N to 0 is not
- allowed.
-
- If the function returns a non-void type, then a vector of the
- results is returned. The size of the resulting vector depends on
- the number of threads that were used. */
+ allowed. */
template<class RandomIt, class RangeFunction>
-typename gdb::detail::par_for_accumulator<
- typename std::invoke_result<RangeFunction, RandomIt, RandomIt>::type
- >::result_type
+void
parallel_for_each (unsigned n, RandomIt first, RandomIt last,
- RangeFunction callback,
- gdb::function_view<size_t(RandomIt)> task_size = nullptr)
+ RangeFunction callback)
{
- using result_type
- = typename std::invoke_result<RangeFunction, RandomIt, RandomIt>::type;
-
/* If enabled, print debug info about how the work is distributed across
the threads. */
const bool parallel_for_each_debug = false;
@@ -162,87 +54,37 @@ parallel_for_each (unsigned n, RandomIt first, RandomIt last,
size_t n_elements = last - first;
size_t elts_per_thread = 0;
size_t elts_left_over = 0;
- size_t total_size = 0;
- size_t size_per_thread = 0;
- size_t max_element_size = n_elements == 0 ? 1 : SIZE_MAX / n_elements;
if (n_threads > 1)
{
- if (task_size != nullptr)
- {
- gdb_assert (n == 1);
- for (RandomIt i = first; i != last; ++i)
- {
- size_t element_size = task_size (i);
- gdb_assert (element_size > 0);
- if (element_size > max_element_size)
- /* We could start scaling here, but that doesn't seem to be
- worth the effort. */
- element_size = max_element_size;
- size_t prev_total_size = total_size;
- total_size += element_size;
- /* Check for overflow. */
- gdb_assert (prev_total_size < total_size);
- }
- size_per_thread = total_size / n_threads;
- }
- else
- {
- /* Require that there should be at least N elements in a
- thread. */
- gdb_assert (n > 0);
- if (n_elements / n_threads < n)
- n_threads = std::max (n_elements / n, (size_t) 1);
- elts_per_thread = n_elements / n_threads;
- elts_left_over = n_elements % n_threads;
- /* n_elements == n_threads * elts_per_thread + elts_left_over. */
- }
+ /* Require that there should be at least N elements in a
+ thread. */
+ gdb_assert (n > 0);
+ if (n_elements / n_threads < n)
+ n_threads = std::max (n_elements / n, (size_t) 1);
+ elts_per_thread = n_elements / n_threads;
+ elts_left_over = n_elements % n_threads;
+ /* n_elements == n_threads * elts_per_thread + elts_left_over. */
}
size_t count = n_threads == 0 ? 0 : n_threads - 1;
- gdb::detail::par_for_accumulator<result_type> results (count);
+ std::vector<gdb::future<void>> results;
if (parallel_for_each_debug)
{
debug_printf (_("Parallel for: n_elements: %zu\n"), n_elements);
- if (task_size != nullptr)
- {
- debug_printf (_("Parallel for: total_size: %zu\n"), total_size);
- debug_printf (_("Parallel for: size_per_thread: %zu\n"), size_per_thread);
- }
- else
- {
- debug_printf (_("Parallel for: minimum elements per thread: %u\n"), n);
- debug_printf (_("Parallel for: elts_per_thread: %zu\n"), elts_per_thread);
- }
+ debug_printf (_("Parallel for: minimum elements per thread: %u\n"), n);
+ debug_printf (_("Parallel for: elts_per_thread: %zu\n"), elts_per_thread);
}
- size_t remaining_size = total_size;
for (int i = 0; i < count; ++i)
{
RandomIt end;
- size_t chunk_size = 0;
- if (task_size == nullptr)
- {
- end = first + elts_per_thread;
- if (i < elts_left_over)
- /* Distribute the leftovers over the worker threads, to avoid having
- to handle all of them in a single thread. */
- end++;
- }
- else
- {
- RandomIt j;
- for (j = first; j < last && chunk_size < size_per_thread; ++j)
- {
- size_t element_size = task_size (j);
- if (element_size > max_element_size)
- element_size = max_element_size;
- chunk_size += element_size;
- }
- end = j;
- remaining_size -= chunk_size;
- }
+ end = first + elts_per_thread;
+ if (i < elts_left_over)
+ /* Distribute the leftovers over the worker threads, to avoid having
+ to handle all of them in a single thread. */
+ end++;
/* This case means we don't have enough elements to really
distribute them. Rather than ever submit a task that does
@@ -257,7 +99,6 @@ parallel_for_each (unsigned n, RandomIt first, RandomIt last,
the result list here. This avoids submitting empty tasks
to the thread pool. */
count = i;
- results.resize (count);
break;
}
@@ -265,12 +106,12 @@ parallel_for_each (unsigned n, RandomIt first, RandomIt last,
{
debug_printf (_("Parallel for: elements on worker thread %i\t: %zu"),
i, (size_t)(end - first));
- if (task_size != nullptr)
- debug_printf (_("\t(size: %zu)"), chunk_size);
debug_printf (_("\n"));
}
- results.post (i, [=] ()
- { return callback (first, end); });
+ results.push_back (gdb::thread_pool::g_thread_pool->post_task ([=] ()
+ {
+ return callback (first, end);
+ }));
first = end;
}
@@ -278,8 +119,6 @@ parallel_for_each (unsigned n, RandomIt first, RandomIt last,
if (parallel_for_each_debug)
{
debug_printf (_("Parallel for: elements on worker thread %i\t: 0"), i);
- if (task_size != nullptr)
- debug_printf (_("\t(size: 0)"));
debug_printf (_("\n"));
}
@@ -288,14 +127,12 @@ parallel_for_each (unsigned n, RandomIt first, RandomIt last,
{
debug_printf (_("Parallel for: elements on main thread\t\t: %zu"),
(size_t)(last - first));
- if (task_size != nullptr)
- debug_printf (_("\t(size: %zu)"), remaining_size);
debug_printf (_("\n"));
}
- return results.finish ([=] ()
- {
- return callback (first, last);
- });
+ callback (first, last);
+
+ for (auto &fut : results)
+ fut.get ();
}
/* A sequential drop-in replacement of parallel_for_each. This can be useful
@@ -303,22 +140,11 @@ parallel_for_each (unsigned n, RandomIt first, RandomIt last,
multi-threading in a fine-grained way. */
template<class RandomIt, class RangeFunction>
-typename gdb::detail::par_for_accumulator<
- typename std::invoke_result<RangeFunction, RandomIt, RandomIt>::type
- >::result_type
+void
sequential_for_each (unsigned n, RandomIt first, RandomIt last,
- RangeFunction callback,
- gdb::function_view<size_t(RandomIt)> task_size = nullptr)
+ RangeFunction callback)
{
- using result_type = typename std::invoke_result<RangeFunction, RandomIt, RandomIt>::type;
-
- gdb::detail::par_for_accumulator<result_type> results (0);
-
- /* Process all the remaining elements in the main thread. */
- return results.finish ([=] ()
- {
- return callback (first, last);
- });
+ callback (first, last);
}
}
--
2.43.0
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH v4 00/19] Index DWARF in the background
2023-12-10 21:41 [PATCH v4 00/19] Index DWARF in the background Tom Tromey
` (18 preceding siblings ...)
2023-12-10 21:41 ` [PATCH v4 19/19] Back out some parallel_for_each features Tom Tromey
@ 2024-01-09 1:08 ` Tom Tromey
19 siblings, 0 replies; 25+ messages in thread
From: Tom Tromey @ 2024-01-09 1:08 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:
Tom> This series changes gdb to do its initial DWARF indexing in the
Tom> background.
I'm checking this in now. As always, if it causes any problems, let me
know and I will fix them as soon as possible.
The .debug_names rewrite series depends on this one. I plan to push
that one a little later, say next week or so.
thanks,
Tom
^ permalink raw reply [flat|nested] 25+ messages in thread