* [PATCH v2 01/18] Don't use objfile::intern in DWO code
2023-11-12 20:25 [PATCH v2 00/18] Index DWARf in the background Tom Tromey
@ 2023-11-12 20:25 ` Tom Tromey
2023-11-12 20:25 ` [PATCH v2 02/18] Pre-read DWZ section data Tom Tromey
` (16 subsequent siblings)
17 siblings, 0 replies; 25+ messages in thread
From: Tom Tromey @ 2023-11-12 20:25 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 7a3b9c68279..f88e5023734 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;
@@ -7728,19 +7728,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;
@@ -7766,7 +7777,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)
@@ -7834,7 +7845,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;
}
@@ -8485,7 +8496,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;
@@ -8670,7 +8681,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,
@@ -8842,7 +8853,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,
@@ -17615,7 +17626,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.41.0
^ permalink raw reply [flat|nested] 25+ messages in thread* [PATCH v2 02/18] Pre-read DWZ section data
2023-11-12 20:25 [PATCH v2 00/18] Index DWARf in the background Tom Tromey
2023-11-12 20:25 ` [PATCH v2 01/18] Don't use objfile::intern in DWO code Tom Tromey
@ 2023-11-12 20:25 ` Tom Tromey
2023-11-12 20:25 ` [PATCH v2 03/18] Add a couple of bfd_cache_close calls Tom Tromey
` (15 subsequent siblings)
17 siblings, 0 replies; 25+ messages in thread
From: Tom Tromey @ 2023-11-12 20:25 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.c | 8 +----
4 files changed, 53 insertions(+), 60 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 78cce2d69eb..a354e894753 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.c b/gdb/dwarf2/read.c
index f88e5023734..d277c21622f 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -3217,7 +3217,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)
{
@@ -5134,12 +5134,6 @@ create_all_units (dwarf2_per_objfile *per_objfile)
dwz_file *dwz = dwarf2_get_dwz_file (per_objfile->per_bfd);
if (dwz != NULL)
{
- /* 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);
--
2.41.0
^ permalink raw reply [flat|nested] 25+ messages in thread* [PATCH v2 03/18] Add a couple of bfd_cache_close calls
2023-11-12 20:25 [PATCH v2 00/18] Index DWARf in the background Tom Tromey
2023-11-12 20:25 ` [PATCH v2 01/18] Don't use objfile::intern in DWO code Tom Tromey
2023-11-12 20:25 ` [PATCH v2 02/18] Pre-read DWZ section data Tom Tromey
@ 2023-11-12 20:25 ` Tom Tromey
2023-11-12 20:25 ` [PATCH v2 04/18] Add thread-safety to gdb's BFD wrappers Tom Tromey
` (14 subsequent siblings)
17 siblings, 0 replies; 25+ messages in thread
From: Tom Tromey @ 2023-11-12 20:25 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 d277c21622f..a83095f8c5d 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -9187,6 +9187,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 ();
}
@@ -9479,6 +9481,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.41.0
^ permalink raw reply [flat|nested] 25+ messages in thread* [PATCH v2 04/18] Add thread-safety to gdb's BFD wrappers
2023-11-12 20:25 [PATCH v2 00/18] Index DWARf in the background Tom Tromey
` (2 preceding siblings ...)
2023-11-12 20:25 ` [PATCH v2 03/18] Add a couple of bfd_cache_close calls Tom Tromey
@ 2023-11-12 20:25 ` Tom Tromey
2023-11-12 20:25 ` [PATCH v2 05/18] Refactor complaint thread-safety approach Tom Tromey
` (13 subsequent siblings)
17 siblings, 0 replies; 25+ messages in thread
From: Tom Tromey @ 2023-11-12 20:25 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 a83095f8c5d..86fd42c3579 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -3936,8 +3936,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 56a4c5ecc91..7152459de64 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 2da39f89a90..39eacd2208b 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -688,8 +688,7 @@ captured_main_1 (struct captured_main_args *context)
gdb_stdtargerr = gdb_stderr;
gdb_stdtargin = gdb_stdin;
- 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.41.0
^ permalink raw reply [flat|nested] 25+ messages in thread* [PATCH v2 05/18] Refactor complaint thread-safety approach
2023-11-12 20:25 [PATCH v2 00/18] Index DWARf in the background Tom Tromey
` (3 preceding siblings ...)
2023-11-12 20:25 ` [PATCH v2 04/18] Add thread-safety to gdb's BFD wrappers Tom Tromey
@ 2023-11-12 20:25 ` Tom Tromey
2023-11-12 20:25 ` [PATCH v2 06/18] Add quick_symbol_functions::compute_main_name Tom Tromey
` (12 subsequent siblings)
17 siblings, 0 replies; 25+ messages in thread
From: Tom Tromey @ 2023-11-12 20:25 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 b8612e1ac6d..a539faa00f0 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -545,7 +545,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 86fd42c3579..e1f1db4dcb3 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -4946,9 +4946,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)
@@ -4958,18 +4955,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)
@@ -4985,15 +4987,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 5028440b671..93c30c06ef3 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.41.0
^ permalink raw reply [flat|nested] 25+ messages in thread* [PATCH v2 06/18] Add quick_symbol_functions::compute_main_name
2023-11-12 20:25 [PATCH v2 00/18] Index DWARf in the background Tom Tromey
` (4 preceding siblings ...)
2023-11-12 20:25 ` [PATCH v2 05/18] Refactor complaint thread-safety approach Tom Tromey
@ 2023-11-12 20:25 ` Tom Tromey
2023-11-12 20:25 ` [PATCH v2 07/18] Add gdb::task_group Tom Tromey
` (11 subsequent siblings)
17 siblings, 0 replies; 25+ messages in thread
From: Tom Tromey @ 2023-11-12 20:25 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 4b8aa9bfcec..c0ca9de6874 100644
--- a/gdb/objfiles.h
+++ b/gdb/objfiles.h
@@ -601,6 +601,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 a7fea2ccb49..49505aef64a 100644
--- a/gdb/quick-symbol.h
+++ b/gdb/quick-symbol.h
@@ -212,6 +212,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 850da4147a3..4b27d9eb406 100644
--- a/gdb/symfile-debug.c
+++ b/gdb/symfile-debug.c
@@ -483,6 +483,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 5ec56f4f2af..ea003a4d4c9 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -6237,6 +6237,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.41.0
^ permalink raw reply [flat|nested] 25+ messages in thread* [PATCH v2 07/18] Add gdb::task_group
2023-11-12 20:25 [PATCH v2 00/18] Index DWARf in the background Tom Tromey
` (5 preceding siblings ...)
2023-11-12 20:25 ` [PATCH v2 06/18] Add quick_symbol_functions::compute_main_name Tom Tromey
@ 2023-11-12 20:25 ` Tom Tromey
2023-11-12 20:25 ` [PATCH v2 08/18] Move cooked_index_storage to cooked-index.h Tom Tromey
` (10 subsequent siblings)
17 siblings, 0 replies; 25+ messages in thread
From: Tom Tromey @ 2023-11-12 20:25 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 00524e9a566..1871b96f49b 100644
--- a/gdbsupport/Makefile.am
+++ b/gdbsupport/Makefile.am
@@ -74,6 +74,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 50a5a55e19d..bbbbbd57282 100644
--- a/gdbsupport/Makefile.in
+++ b/gdbsupport/Makefile.in
@@ -160,8 +160,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@)
@@ -401,6 +401,7 @@ libgdbsupport_a_SOURCES = \
search.cc \
signals.cc \
signals-state-save-restore.cc \
+ task-group.cc \
tdesc.cc \
thread-pool.cc \
xml-utils.cc \
@@ -511,6 +512,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.41.0
^ permalink raw reply [flat|nested] 25+ messages in thread* [PATCH v2 08/18] Move cooked_index_storage to cooked-index.h
2023-11-12 20:25 [PATCH v2 00/18] Index DWARf in the background Tom Tromey
` (6 preceding siblings ...)
2023-11-12 20:25 ` [PATCH v2 07/18] Add gdb::task_group Tom Tromey
@ 2023-11-12 20:25 ` Tom Tromey
2023-11-12 20:25 ` [PATCH v2 09/18] Add "maint set dwarf synchronous" Tom Tromey
` (9 subsequent siblings)
17 siblings, 0 replies; 25+ messages in thread
From: Tom Tromey @ 2023-11-12 20:25 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 5aacb321c91..0db6e8003b9 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 e1f1db4dcb3..58d13e880fd 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -747,7 +747,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);
@@ -4434,105 +4433,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.41.0
^ permalink raw reply [flat|nested] 25+ messages in thread* [PATCH v2 09/18] Add "maint set dwarf synchronous"
2023-11-12 20:25 [PATCH v2 00/18] Index DWARf in the background Tom Tromey
` (7 preceding siblings ...)
2023-11-12 20:25 ` [PATCH v2 08/18] Move cooked_index_storage to cooked-index.h Tom Tromey
@ 2023-11-12 20:25 ` Tom Tromey
2023-11-13 13:43 ` Eli Zaretskii
2023-11-12 20:25 ` [PATCH v2 10/18] Change how cooked index waits for threads Tom Tromey
` (8 subsequent siblings)
17 siblings, 1 reply; 25+ messages in thread
From: Tom Tromey @ 2023-11-12 20:25 UTC (permalink / raw)
To: gdb-patches
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.
---
gdb/NEWS | 3 +++
gdb/doc/gdb.texinfo | 14 ++++++++++++++
gdb/dwarf2/read.c | 23 +++++++++++++++++++++++
3 files changed, 40 insertions(+)
diff --git a/gdb/NEWS b/gdb/NEWS
index 3851114a9f7..ef87509c48b 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -9,6 +9,9 @@
* GDB index now contains information about the main function. This speeds up
startup when it is being used for some large binaries.
+* 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 2cd565ed5b4..36a83e94528 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -41700,6 +41700,20 @@ 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.
+
+By default, 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.
+
@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 58d13e880fd..7669af36b37 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -741,6 +741,16 @@ show_dwarf_max_cache_age (struct ui_file *file, int from_tty,
"DWARF compilation units is %s.\n"),
value);
}
+
+/* Wait for DWARF reading to be complete. */
+static bool dwarf_synchronous = false;
+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 */
@@ -21941,6 +21951,19 @@ 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 DWARF is read synchronously."), _("\
+By default, DWARF information is read in worker threads,\n\
+and gdb will not generally wait for this process to complete.\n\
+Enabling this setting will cause the DWARF reader to always wait\n\
+for completion 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.41.0
^ permalink raw reply [flat|nested] 25+ messages in thread* [PATCH v2 10/18] Change how cooked index waits for threads
2023-11-12 20:25 [PATCH v2 00/18] Index DWARf in the background Tom Tromey
` (8 preceding siblings ...)
2023-11-12 20:25 ` [PATCH v2 09/18] Add "maint set dwarf synchronous" Tom Tromey
@ 2023-11-12 20:25 ` Tom Tromey
2023-11-12 20:25 ` [PATCH v2 11/18] Do more DWARF reading in the background Tom Tromey
` (7 subsequent siblings)
17 siblings, 0 replies; 25+ messages in thread
From: Tom Tromey @ 2023-11-12 20:25 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 58ea541a5c9..a474691cfb4 100644
--- a/gdb/dwarf2/cooked-index.c
+++ b/gdb/dwarf2/cooked-index.c
@@ -405,8 +405,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);
@@ -529,6 +527,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 0db6e8003b9..14352c1e139 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.41.0
^ permalink raw reply [flat|nested] 25+ messages in thread* [PATCH v2 11/18] Do more DWARF reading in the background
2023-11-12 20:25 [PATCH v2 00/18] Index DWARf in the background Tom Tromey
` (9 preceding siblings ...)
2023-11-12 20:25 ` [PATCH v2 10/18] Change how cooked index waits for threads Tom Tromey
@ 2023-11-12 20:25 ` Tom Tromey
2023-11-13 7:15 ` Tom de Vries
2023-11-12 20:25 ` [PATCH v2 12/18] Simplify the public DWARF API Tom Tromey
` (6 subsequent siblings)
17 siblings, 1 reply; 25+ messages in thread
From: Tom Tromey @ 2023-11-12 20:25 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 | 205 ++++++----
gdb/dwarf2/cooked-index.h | 245 +++++++++---
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, 653 insertions(+), 308 deletions(-)
diff --git a/gdb/dwarf2/cooked-index.c b/gdb/dwarf2/cooked-index.c
index a474691cfb4..078d8d8c140 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)
@@ -302,7 +313,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)
{
@@ -426,70 +437,83 @@ 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 (gdb::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::wait (cooked_state desired_state, bool allow_quit)
+{
+ gdb_assert (desired_state != cooked_state::INITIAL);
+
+ /* 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::start_writing_index (dwarf2_per_bfd *per_bfd)
+cooked_index::set_contents (vec_type &&vec)
{
- index_cache_store_context ctx (global_index_cache, per_bfd);
+ gdb_assert (m_vector.empty ());
+ m_vector = std::move (vec);
- /* 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,
+ 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,
#if __cplusplus >= 201402L
- ctx = std::move (ctx)
+ ctx = std::move (ctx)
#else
- ctx
+ ctx
#endif
- ] ()
- {
- maybe_write_index (per_bfd, 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. */
@@ -502,6 +526,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);
@@ -514,8 +540,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);
@@ -525,9 +553,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)
@@ -537,38 +565,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");
@@ -641,11 +696,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 14352c1e139..df27f1214b8 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,158 @@ 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;
+
+#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. */
+ gdb::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 +584,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 +602,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,12 +622,15 @@ 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
{
return this;
@@ -488,20 +639,20 @@ class cooked_index : public dwarf_scanner_base
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 +664,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 7669af36b37..09563264ec8 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.
@@ -760,8 +760,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);
@@ -3210,7 +3208,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. */
@@ -3287,31 +3286,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
@@ -3945,7 +3924,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);
@@ -4877,12 +4856,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;
@@ -4890,109 +4868,248 @@ dwarf2_build_psymtabs_hard (dwarf2_per_objfile *per_objfile)
objfile_name (objfile));
per_bfd->map_info_sections (objfile);
+}
- cooked_index_storage index_storage;
- create_all_units (per_objfile);
- build_type_psymtabs (per_objfile, &index_storage);
+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));
+ }
+ }
+
+ 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 (per_objfile, &per_bfd->debug_aranges,
- index_storage.get_addrmap ());
+ read_addrmap_from_aranges (m_per_objfile, &per_bfd->debug_aranges,
+ m_index_storage.get_addrmap ());
+
+ /* 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 = total_size / n_worker_threads;
+
+ /* 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; )
+ {
+ 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 (m_reported)
+ return done;
+ m_reported = true;
- if (dwarf_read_debug > 0)
- print_tu_stats (per_objfile);
+ if (m_failed.has_value ())
+ {
+ /* start_reading failed -- report it. */
+ exception_print (gdb_stderr, *m_failed);
+ m_failed.reset ();
+ return done;
+ }
- indexes.push_back (index_storage.release ());
- indexes.shrink_to_fit ();
+ /* Only show a given exception a single time. */
+ std::unordered_set<gdb_exception> seen_exceptions;
+ for (auto &one_result : m_results)
+ {
+ 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);
+ }
- cooked_index *vec = new cooked_index (std::move (indexes));
- per_bfd->index_table.reset (vec);
+ if (dwarf_read_debug > 0)
+ print_tu_stats (m_per_objfile);
- /* Cannot start writing the index entry until after the
- 'index_table' member has been set. */
- vec->start_writing_index (per_bfd);
+ 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 ()));
- const cooked_index_entry *main_entry = vec->get_main ();
- if (main_entry != nullptr)
- {
- /* 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);
- }
+ 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
@@ -16518,26 +16635,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);
+ }
+
void expand_matching_symbols
(struct objfile *,
const lookup_name_info &lookup_name,
@@ -16555,55 +16706,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);
}
};
@@ -16614,8 +16738,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);
}
@@ -16627,11 +16749,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);
@@ -16650,11 +16768,7 @@ cooked_index_functions::expand_matching_symbols
symbol_compare_ftype *ordered_compare)
{
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;
+ cooked_index *table = wait (objfile, true);
const block_search_flags search_flags = (global
? SEARCH_GLOBAL_BLOCK
@@ -16692,13 +16806,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);
@@ -16818,15 +16926,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 da907729320..c679e47a91f 100644
--- a/gdb/dwarf2/read.h
+++ b/gdb/dwarf2/read.h
@@ -881,7 +881,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.41.0
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH v2 11/18] Do more DWARF reading in the background
2023-11-12 20:25 ` [PATCH v2 11/18] Do more DWARF reading in the background Tom Tromey
@ 2023-11-13 7:15 ` Tom de Vries
2023-11-13 17:56 ` John Baldwin
0 siblings, 1 reply; 25+ messages in thread
From: Tom de Vries @ 2023-11-13 7:15 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 11/12/23 21:25, Tom Tromey wrote:
> + /* How much effort should be put into each worker. */ + const size_t
> size_per_thread = total_size / n_worker_threads;
I've tested the patch series on x86_64-linux, and ran into trouble in
one test-case:
...
(gdb) file
/data/vries/gdb/leap-15-4/build/gdb/testsuite/outputs/gdb.dwarf2/dw2-dummy-cu/dw2-dummy-cu^M
Reading symbols from
/data/vries/gdb/leap-15-4/build/gdb/testsuite/outputs/gdb.dwarf2/dw2-dummy-cu/dw2-dummy-cu...^M
/data/vries/gdb/src/gdb/dwarf2/read.c:5000: internal-error: do_reading:
Assertion `iter != last' failed.^M
A problem internal to GDB has been detected,^M
further debugging may prove unreliable.^M
----- Backtrace -----^M
ERROR: Couldn't load dw2-dummy-cu into GDB (GDB internal error).
Resyncing due to internal error.
(gdb) maint expand-symtab^M
0x5ac476 gdb_internal_backtrace_1^M
/data/vries/gdb/src/gdb/bt-utils.c:122^M
0x5ac519 _Z22gdb_internal_backtracev^M
/data/vries/gdb/src/gdb/bt-utils.c:168^M
0xd11a89 internal_vproblem^M
/data/vries/gdb/src/gdb/utils.c:396^M
0xd11e58 _Z15internal_verrorPKciS0_P13__va_list_tag^M
/data/vries/gdb/src/gdb/utils.c:476^M
0x14ca6f0 _Z18internal_error_locPKciS0_z^M
/data/vries/gdb/src/gdbsupport/errors.cc:58^M
0x72b01b _ZN19cooked_index_worker10do_readingEv^M
/data/vries/gdb/src/gdb/dwarf2/read.c:5000^M
...
This fixes the test-case for me:
...
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 385160de0fd..191c2ad7f26 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -4974,7 +4974,8 @@ cooked_index_worker::do_reading ()
= 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 = total_size / n_worker_threads;
+ 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] ()
...
but I haven't done a full test-run.
Thanks,
- Tom
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH v2 11/18] Do more DWARF reading in the background
2023-11-13 7:15 ` Tom de Vries
@ 2023-11-13 17:56 ` John Baldwin
2023-11-13 19:39 ` Tom de Vries
0 siblings, 1 reply; 25+ messages in thread
From: John Baldwin @ 2023-11-13 17:56 UTC (permalink / raw)
To: Tom de Vries, Tom Tromey, gdb-patches
On 11/12/23 11:15 PM, Tom de Vries wrote:
> On 11/12/23 21:25, Tom Tromey wrote:
>> + /* How much effort should be put into each worker. */ + const size_t
>> size_per_thread = total_size / n_worker_threads;
>
> I've tested the patch series on x86_64-linux, and ran into trouble in
> one test-case:
> ...
> (gdb) file
> /data/vries/gdb/leap-15-4/build/gdb/testsuite/outputs/gdb.dwarf2/dw2-dummy-cu/dw2-dummy-cu^M
> Reading symbols from
> /data/vries/gdb/leap-15-4/build/gdb/testsuite/outputs/gdb.dwarf2/dw2-dummy-cu/dw2-dummy-cu...^M
> /data/vries/gdb/src/gdb/dwarf2/read.c:5000: internal-error: do_reading:
> Assertion `iter != last' failed.^M
> A problem internal to GDB has been detected,^M
> further debugging may prove unreliable.^M
> ----- Backtrace -----^M
> ERROR: Couldn't load dw2-dummy-cu into GDB (GDB internal error).
> Resyncing due to internal error.
> (gdb) maint expand-symtab^M
> 0x5ac476 gdb_internal_backtrace_1^M
> /data/vries/gdb/src/gdb/bt-utils.c:122^M
> 0x5ac519 _Z22gdb_internal_backtracev^M
> /data/vries/gdb/src/gdb/bt-utils.c:168^M
> 0xd11a89 internal_vproblem^M
> /data/vries/gdb/src/gdb/utils.c:396^M
> 0xd11e58 _Z15internal_verrorPKciS0_P13__va_list_tag^M
> /data/vries/gdb/src/gdb/utils.c:476^M
> 0x14ca6f0 _Z18internal_error_locPKciS0_z^M
> /data/vries/gdb/src/gdbsupport/errors.cc:58^M
> 0x72b01b _ZN19cooked_index_worker10do_readingEv^M
> /data/vries/gdb/src/gdb/dwarf2/read.c:5000^M
> ...
>
> This fixes the test-case for me:
> ...
> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
> index 385160de0fd..191c2ad7f26 100644
> --- a/gdb/dwarf2/read.c
> +++ b/gdb/dwarf2/read.c
> @@ -4974,7 +4974,8 @@ cooked_index_worker::do_reading ()
> = 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 = total_size / n_worker_threads;
> + const size_t size_per_thread
> + = std::max (total_size / n_worker_threads, (size_t)1);
Presumably total_size was zero in that case? Should there be a shortcut
to bother queueing a task in that case instead perhaps? Or alternatively
is the issue that total_size is < n_worker_threads? If the latter, then
a min size of 1 does seem sensible.
--
John Baldwin
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 11/18] Do more DWARF reading in the background
2023-11-13 17:56 ` John Baldwin
@ 2023-11-13 19:39 ` Tom de Vries
2023-11-14 17:31 ` John Baldwin
0 siblings, 1 reply; 25+ messages in thread
From: Tom de Vries @ 2023-11-13 19:39 UTC (permalink / raw)
To: John Baldwin, Tom Tromey, gdb-patches
On 11/13/23 18:56, John Baldwin wrote:
> On 11/12/23 11:15 PM, Tom de Vries wrote:
>> On 11/12/23 21:25, Tom Tromey wrote:
>>> + /* How much effort should be put into each worker. */ + const size_t
>>> size_per_thread = total_size / n_worker_threads;
>>
>> I've tested the patch series on x86_64-linux, and ran into trouble in
>> one test-case:
>> ...
>> (gdb) file
>> /data/vries/gdb/leap-15-4/build/gdb/testsuite/outputs/gdb.dwarf2/dw2-dummy-cu/dw2-dummy-cu^M
>> Reading symbols from
>> /data/vries/gdb/leap-15-4/build/gdb/testsuite/outputs/gdb.dwarf2/dw2-dummy-cu/dw2-dummy-cu...^M
>> /data/vries/gdb/src/gdb/dwarf2/read.c:5000: internal-error: do_reading:
>> Assertion `iter != last' failed.^M
>> A problem internal to GDB has been detected,^M
>> further debugging may prove unreliable.^M
>> ----- Backtrace -----^M
>> ERROR: Couldn't load dw2-dummy-cu into GDB (GDB internal error).
>> Resyncing due to internal error.
>> (gdb) maint expand-symtab^M
>> 0x5ac476 gdb_internal_backtrace_1^M
>> /data/vries/gdb/src/gdb/bt-utils.c:122^M
>> 0x5ac519 _Z22gdb_internal_backtracev^M
>> /data/vries/gdb/src/gdb/bt-utils.c:168^M
>> 0xd11a89 internal_vproblem^M
>> /data/vries/gdb/src/gdb/utils.c:396^M
>> 0xd11e58 _Z15internal_verrorPKciS0_P13__va_list_tag^M
>> /data/vries/gdb/src/gdb/utils.c:476^M
>> 0x14ca6f0 _Z18internal_error_locPKciS0_z^M
>> /data/vries/gdb/src/gdbsupport/errors.cc:58^M
>> 0x72b01b _ZN19cooked_index_worker10do_readingEv^M
>> /data/vries/gdb/src/gdb/dwarf2/read.c:5000^M
>> ...
>>
>> This fixes the test-case for me:
>> ...
>> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
>> index 385160de0fd..191c2ad7f26 100644
>> --- a/gdb/dwarf2/read.c
>> +++ b/gdb/dwarf2/read.c
>> @@ -4974,7 +4974,8 @@ cooked_index_worker::do_reading ()
>> = 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 = total_size / n_worker_threads;
>> + const size_t size_per_thread
>> + = std::max (total_size / n_worker_threads, (size_t)1);
>
> Presumably total_size was zero in that case?
No, it was 11.
> Should there be a shortcut
> to bother queueing a task in that case instead perhaps? Or alternatively
> is the issue that total_size is < n_worker_threads?
Yes, n_worker_threads was 12.
> If the latter, then
> a min size of 1 does seem sensible.
>
Thanks for the review.
- Tom
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 11/18] Do more DWARF reading in the background
2023-11-13 19:39 ` Tom de Vries
@ 2023-11-14 17:31 ` John Baldwin
2023-11-21 14:24 ` Tom Tromey
0 siblings, 1 reply; 25+ messages in thread
From: John Baldwin @ 2023-11-14 17:31 UTC (permalink / raw)
To: Tom de Vries, Tom Tromey, gdb-patches
On 11/13/23 11:39 AM, Tom de Vries wrote:
> On 11/13/23 18:56, John Baldwin wrote:
>> On 11/12/23 11:15 PM, Tom de Vries wrote:
>>> On 11/12/23 21:25, Tom Tromey wrote:
>>>> + /* How much effort should be put into each worker. */ + const size_t
>>>> size_per_thread = total_size / n_worker_threads;
>>>
>>> I've tested the patch series on x86_64-linux, and ran into trouble in
>>> one test-case:
>>> ...
>>> (gdb) file
>>> /data/vries/gdb/leap-15-4/build/gdb/testsuite/outputs/gdb.dwarf2/dw2-dummy-cu/dw2-dummy-cu^M
>>> Reading symbols from
>>> /data/vries/gdb/leap-15-4/build/gdb/testsuite/outputs/gdb.dwarf2/dw2-dummy-cu/dw2-dummy-cu...^M
>>> /data/vries/gdb/src/gdb/dwarf2/read.c:5000: internal-error: do_reading:
>>> Assertion `iter != last' failed.^M
>>> A problem internal to GDB has been detected,^M
>>> further debugging may prove unreliable.^M
>>> ----- Backtrace -----^M
>>> ERROR: Couldn't load dw2-dummy-cu into GDB (GDB internal error).
>>> Resyncing due to internal error.
>>> (gdb) maint expand-symtab^M
>>> 0x5ac476 gdb_internal_backtrace_1^M
>>> /data/vries/gdb/src/gdb/bt-utils.c:122^M
>>> 0x5ac519 _Z22gdb_internal_backtracev^M
>>> /data/vries/gdb/src/gdb/bt-utils.c:168^M
>>> 0xd11a89 internal_vproblem^M
>>> /data/vries/gdb/src/gdb/utils.c:396^M
>>> 0xd11e58 _Z15internal_verrorPKciS0_P13__va_list_tag^M
>>> /data/vries/gdb/src/gdb/utils.c:476^M
>>> 0x14ca6f0 _Z18internal_error_locPKciS0_z^M
>>> /data/vries/gdb/src/gdbsupport/errors.cc:58^M
>>> 0x72b01b _ZN19cooked_index_worker10do_readingEv^M
>>> /data/vries/gdb/src/gdb/dwarf2/read.c:5000^M
>>> ...
>>>
>>> This fixes the test-case for me:
>>> ...
>>> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
>>> index 385160de0fd..191c2ad7f26 100644
>>> --- a/gdb/dwarf2/read.c
>>> +++ b/gdb/dwarf2/read.c
>>> @@ -4974,7 +4974,8 @@ cooked_index_worker::do_reading ()
>>> = 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 = total_size / n_worker_threads;
>>> + const size_t size_per_thread
>>> + = std::max (total_size / n_worker_threads, (size_t)1);
>>
>> Presumably total_size was zero in that case?
>
> No, it was 11.
>
>> Should there be a shortcut
>> to bother queueing a task in that case instead perhaps? Or alternatively
>> is the issue that total_size is < n_worker_threads?
>
> Yes, n_worker_threads was 12.
>
>> If the latter, then
>> a min size of 1 does seem sensible.
>>
>
> Thanks for the review.
Is it possible that you want size_per_thread to always round up? That is,
if you had total_size == 23 with n_worker_threads == 12, is there a reason
to prefer size_per_thread being 2 in that case vs 1? (E.g. if the threads
each do "size_per_thread" items with the last one handling any remainder,
then you'd probably prefer 2 so that 11 threads do 2 and the last thread
does 1 rather than 11 threads doing 1 and the last thread doing 11.)
Looking at the patch, I do think it's the case I described, so perhaps you
want to always round up via:
const size_t size_per_thread
= (total_size + n_worker_threads - 1) / n_worker_threads;
--
John Baldwin
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 11/18] Do more DWARF reading in the background
2023-11-14 17:31 ` John Baldwin
@ 2023-11-21 14:24 ` Tom Tromey
0 siblings, 0 replies; 25+ messages in thread
From: Tom Tromey @ 2023-11-21 14:24 UTC (permalink / raw)
To: John Baldwin; +Cc: Tom de Vries, Tom Tromey, gdb-patches
>>>>> "John" == John Baldwin <jhb@FreeBSD.org> writes:
John> Is it possible that you want size_per_thread to always round up?
That would be better, except...
John> Looking at the patch, I do think it's the case I described, so perhaps you
John> want to always round up via:
John> const size_t size_per_thread
John> = (total_size + n_worker_threads - 1) / n_worker_threads;
... here 'total_size' is not a count, but instead the sum of the size of
all the CUs. (This is done to try to balance the number of DIEs handled
by each task.) So, adding n_worker_threads to this is like a units
mismatch.
Anyway I am going to use Tom's original proposal here:
const size_t size_per_thread
= std::max (total_size / n_worker_threads, (size_t) 1);
Tom
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v2 12/18] Simplify the public DWARF API
2023-11-12 20:25 [PATCH v2 00/18] Index DWARf in the background Tom Tromey
` (10 preceding siblings ...)
2023-11-12 20:25 ` [PATCH v2 11/18] Do more DWARF reading in the background Tom Tromey
@ 2023-11-12 20:25 ` Tom Tromey
2023-11-12 20:25 ` [PATCH v2 13/18] Remove two quick_symbol_functions methods Tom Tromey
` (5 subsequent siblings)
17 siblings, 0 replies; 25+ messages in thread
From: Tom Tromey @ 2023-11-12 20:25 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 | 8 +++-----
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(+), 52 deletions(-)
diff --git a/gdb/coffread.c b/gdb/coffread.c
index 4a6a83b15ad..b0c9c47f13d 100644
--- a/gdb/coffread.c
+++ b/gdb/coffread.c
@@ -718,11 +718,9 @@ coff_symfile_read (struct objfile *objfile, symfile_add_flags symfile_flags)
*info->stabsects,
info->stabstrsect->filepos, stabstrsize);
}
- if (dwarf2_has_info (objfile, NULL))
- {
- /* DWARF2 sections. */
- dwarf2_initialize_objfile (objfile);
- }
+
+ /* DWARF2 sections. */
+ dwarf2_initialize_objfile (objfile);
/* Try to add separate debug file if no symbols table found. */
if (!objfile->has_partial_symbols ())
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 09563264ec8..385160de0fd 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -1359,14 +1359,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)
@@ -3213,9 +3209,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;
@@ -3245,48 +3246,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 7900dfbc388..d12dbc25f7a 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -1200,8 +1200,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 38c252c2861..e701a7366c2 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 e6ecf2a6de5..23fd38c9aa2 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.41.0
^ permalink raw reply [flat|nested] 25+ messages in thread* [PATCH v2 13/18] Remove two quick_symbol_functions methods
2023-11-12 20:25 [PATCH v2 00/18] Index DWARf in the background Tom Tromey
` (11 preceding siblings ...)
2023-11-12 20:25 ` [PATCH v2 12/18] Simplify the public DWARF API Tom Tromey
@ 2023-11-12 20:25 ` Tom Tromey
2023-11-12 20:25 ` [PATCH v2 14/18] Change current_language to be a macro Tom Tromey
` (4 subsequent siblings)
17 siblings, 0 replies; 25+ messages in thread
From: Tom Tromey @ 2023-11-12 20:25 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 | 67 ++++++++++++++---------------------------------------
gdb/symfile.c | 4 ----
6 files changed, 17 insertions(+), 87 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 c0ca9de6874..13ee58d7454 100644
--- a/gdb/objfiles.h
+++ b/gdb/objfiles.h
@@ -612,9 +612,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
{
@@ -699,17 +696,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 41ecf31424b..0a470d68342 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 49505aef64a..f03e4cc1c1a 100644
--- a/gdb/quick-symbol.h
+++ b/gdb/quick-symbol.h
@@ -220,20 +220,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 4b27d9eb406..1ed80d0737a 100644
--- a/gdb/symfile-debug.c
+++ b/gdb/symfile-debug.c
@@ -85,11 +85,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;
}
@@ -110,7 +106,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))
{
@@ -135,7 +131,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)
@@ -168,7 +164,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);
}
@@ -215,7 +211,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,
@@ -280,7 +276,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,
@@ -311,7 +307,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);
}
@@ -337,7 +333,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,
@@ -356,7 +352,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);
}
@@ -374,7 +370,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,
@@ -399,7 +395,7 @@ objfile::expand_matching_symbols
domain_name (domain), global,
host_address_to_string (ordered_compare));
- for (const auto &iter : qf_require_partial_symbols ())
+ for (const auto &iter : qf)
iter->expand_matching_symbols (this, name, domain, global,
ordered_compare);
}
@@ -426,7 +422,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))
@@ -451,7 +447,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);
@@ -479,7 +475,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);
}
@@ -491,7 +487,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);
}
@@ -505,7 +501,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)
@@ -530,7 +526,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);
@@ -541,35 +537,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)));
- }
-}
-
\f
/* Debugging version of struct sym_probe_fns. */
diff --git a/gdb/symfile.c b/gdb/symfile.c
index eebc5ea44b9..a89876c6835 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. */
@@ -2621,8 +2619,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.41.0
^ permalink raw reply [flat|nested] 25+ messages in thread* [PATCH v2 14/18] Change current_language to be a macro
2023-11-12 20:25 [PATCH v2 00/18] Index DWARf in the background Tom Tromey
` (12 preceding siblings ...)
2023-11-12 20:25 ` [PATCH v2 13/18] Remove two quick_symbol_functions methods Tom Tromey
@ 2023-11-12 20:25 ` Tom Tromey
2023-11-12 20:25 ` [PATCH v2 15/18] Lazy language setting Tom Tromey
` (3 subsequent siblings)
17 siblings, 0 replies; 25+ messages in thread
From: Tom Tromey @ 2023-11-12 20:25 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 c768971be42..d38e2c04afb 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.41.0
^ permalink raw reply [flat|nested] 25+ messages in thread* [PATCH v2 15/18] Lazy language setting
2023-11-12 20:25 [PATCH v2 00/18] Index DWARf in the background Tom Tromey
` (13 preceding siblings ...)
2023-11-12 20:25 ` [PATCH v2 14/18] Change current_language to be a macro Tom Tromey
@ 2023-11-12 20:25 ` Tom Tromey
2023-11-12 20:25 ` [PATCH v2 16/18] Optimize lookup_minimal_symbol_text Tom Tromey
` (2 subsequent siblings)
17 siblings, 0 replies; 25+ messages in thread
From: Tom Tromey @ 2023-11-12 20:25 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 48f32bb3a0b..ed00cbcf32e 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -35,6 +35,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;
@@ -888,7 +889,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 d38e2c04afb..acb11a74486 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 a89876c6835..7b45343b5de 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -1683,13 +1683,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;
@@ -1714,6 +1712,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 0660589abbf..04aac1caf13 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -1405,13 +1405,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 ();
}
@@ -1419,8 +1419,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.41.0
^ permalink raw reply [flat|nested] 25+ messages in thread* [PATCH v2 16/18] Optimize lookup_minimal_symbol_text
2023-11-12 20:25 [PATCH v2 00/18] Index DWARf in the background Tom Tromey
` (14 preceding siblings ...)
2023-11-12 20:25 ` [PATCH v2 15/18] Lazy language setting Tom Tromey
@ 2023-11-12 20:25 ` Tom Tromey
2023-11-12 20:25 ` [PATCH v2 17/18] Avoid language-based lookups in startup path Tom Tromey
2023-11-12 20:25 ` [PATCH v2 18/18] Back out some parallel_for_each features Tom Tromey
17 siblings, 0 replies; 25+ messages in thread
From: Tom Tromey @ 2023-11-12 20:25 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 1d778822858..7e0522159b8 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.41.0
^ permalink raw reply [flat|nested] 25+ messages in thread* [PATCH v2 17/18] Avoid language-based lookups in startup path
2023-11-12 20:25 [PATCH v2 00/18] Index DWARf in the background Tom Tromey
` (15 preceding siblings ...)
2023-11-12 20:25 ` [PATCH v2 16/18] Optimize lookup_minimal_symbol_text Tom Tromey
@ 2023-11-12 20:25 ` Tom Tromey
2023-11-12 20:25 ` [PATCH v2 18/18] Back out some parallel_for_each features Tom Tromey
17 siblings, 0 replies; 25+ messages in thread
From: Tom Tromey @ 2023-11-12 20:25 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 fe09dbcbeee..0ff36108682 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 9e8325ab803..9e256173dd6 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.41.0
^ permalink raw reply [flat|nested] 25+ messages in thread* [PATCH v2 18/18] Back out some parallel_for_each features
2023-11-12 20:25 [PATCH v2 00/18] Index DWARf in the background Tom Tromey
` (16 preceding siblings ...)
2023-11-12 20:25 ` [PATCH v2 17/18] Avoid language-based lookups in startup path Tom Tromey
@ 2023-11-12 20:25 ` Tom Tromey
17 siblings, 0 replies; 25+ messages in thread
From: Tom Tromey @ 2023-11-12 20:25 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 1ad7eaa701c..b67b114e210 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 gdb::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 b57f7ea97e1..f9b2e49c701 100644
--- a/gdbsupport/parallel-for.h
+++ b/gdbsupport/parallel-for.h
@@ -29,104 +29,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.
@@ -137,23 +39,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 gdb::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 gdb::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;
@@ -163,87 +55,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
@@ -258,7 +100,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;
}
@@ -266,12 +107,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;
}
@@ -279,8 +120,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"));
}
@@ -289,14 +128,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
@@ -304,22 +141,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 gdb::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 gdb::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.41.0
^ permalink raw reply [flat|nested] 25+ messages in thread