* [PATCH 00/15] Index DWARF in the background
@ 2023-10-29 17:35 Tom Tromey
2023-10-29 17:35 ` [PATCH 01/15] Add thread-safety to gdb's BFD wrappers Tom Tromey
` (14 more replies)
0 siblings, 15 replies; 18+ messages in thread
From: Tom Tromey @ 2023-10-29 17:35 UTC (permalink / raw)
To: gdb-patches
This series changes gdb to do its initial DWARF indexing in the
background.
This process is mostly asynchronous with respect to the rest of gdb.
That is, rather than pre-emptively waiting for scanning to complete,
now gdb's main thread will only wait when some result of the scan is
required.
This drastically improves gdb's apparent startup time in the "normal"
case where a user does "gdb some-executable" -- e.g., for starting gdb
on itself, the time until the prompt returns goes from ~1.2 seconds to
~0.06 seconds on my machine.
This approach works by hiding most of the work from the user. Waiting
can still be needed; for example if one starts gdb and immediately
sets a breakpoint -- however, because the indexer is reasonably fast,
and human reaction times are slow, this series still manages to be
fairly successful.
My current belief is that doing any better than this will probably
require a new debug format that isn't quite so cursed to read.
This series requires the BFD threading series I posted to the binutils
list, and the DWZ refactoring patch I sent recently.
I regression tested this on x86-64 Fedora 38. I've also built it with
TSAN and tested that, though TSAN seems to introduce random timeouts
into the testsuite when I use "make -j8 check".
Tom
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 01/15] Add thread-safety to gdb's BFD wrappers
2023-10-29 17:35 [PATCH 00/15] Index DWARF in the background Tom Tromey
@ 2023-10-29 17:35 ` Tom Tromey
2023-10-29 17:35 ` [PATCH 02/15] Refactor complaint thread-safety approach Tom Tromey
` (13 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Tom Tromey @ 2023-10-29 17:35 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
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 | 53 +++++++++++++++++++++++++++++++++++++++++++++++
gdb/gdb_bfd.h | 5 +++++
gdb/main.c | 3 +--
4 files changed, 60 insertions(+), 4 deletions(-)
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 3b3daf28965..088f01d807d 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..5f100a50fa5 100644
--- a/gdb/gdb_bfd.c
+++ b/gdb/gdb_bfd.c
@@ -35,6 +35,34 @@
#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 void
+gdb_bfd_lock ()
+{
+ gdb_bfd_mutex.lock ();
+}
+
+/* BFD unlocking function. */
+
+static void
+gdb_bfd_unlock ()
+{
+ gdb_bfd_mutex.unlock ();
+}
+
+#endif /* CXX_STD_THREAD */
+
/* An object of this type is stored in the section's user data when
mapping a section. */
@@ -498,6 +526,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 +657,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 +690,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 +1211,19 @@ 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)
+ error (_("fatal error: libbfd ABI mismatch"));
+
+#if CXX_STD_THREAD
+ bfd_thread_init (gdb_bfd_lock, gdb_bfd_unlock);
+#endif
+}
+
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] 18+ messages in thread
* [PATCH 02/15] Refactor complaint thread-safety approach
2023-10-29 17:35 [PATCH 00/15] Index DWARF in the background Tom Tromey
2023-10-29 17:35 ` [PATCH 01/15] Add thread-safety to gdb's BFD wrappers Tom Tromey
@ 2023-10-29 17:35 ` Tom Tromey
2023-10-29 17:35 ` [PATCH 03/15] Add quick_symbol_functions::compute_main_name Tom Tromey
` (12 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Tom Tromey @ 2023-10-29 17:35 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
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 f5af3e617c4..c2e68cee45a 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 088f01d807d..024b66cc7ad 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] 18+ messages in thread
* [PATCH 03/15] Add quick_symbol_functions::compute_main_name
2023-10-29 17:35 [PATCH 00/15] Index DWARF in the background Tom Tromey
2023-10-29 17:35 ` [PATCH 01/15] Add thread-safety to gdb's BFD wrappers Tom Tromey
2023-10-29 17:35 ` [PATCH 02/15] Refactor complaint thread-safety approach Tom Tromey
@ 2023-10-29 17:35 ` Tom Tromey
2023-10-29 17:35 ` [PATCH 04/15] Add gdb::task_group Tom Tromey
` (11 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Tom Tromey @ 2023-10-29 17:35 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
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] 18+ messages in thread
* [PATCH 04/15] Add gdb::task_group
2023-10-29 17:35 [PATCH 00/15] Index DWARF in the background Tom Tromey
` (2 preceding siblings ...)
2023-10-29 17:35 ` [PATCH 03/15] Add quick_symbol_functions::compute_main_name Tom Tromey
@ 2023-10-29 17:35 ` Tom Tromey
2023-10-29 17:35 ` [PATCH 05/15] Move cooked_index_storage to cooked-index.h Tom Tromey
` (10 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Tom Tromey @ 2023-10-29 17:35 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
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(-)
create mode 100644 gdbsupport/task-group.cc
create mode 100644 gdbsupport/task-group.h
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] 18+ messages in thread
* [PATCH 05/15] Move cooked_index_storage to cooked-index.h
2023-10-29 17:35 [PATCH 00/15] Index DWARF in the background Tom Tromey
` (3 preceding siblings ...)
2023-10-29 17:35 ` [PATCH 04/15] Add gdb::task_group Tom Tromey
@ 2023-10-29 17:35 ` Tom Tromey
2023-10-29 17:35 ` [PATCH 06/15] Add "maint set dwarf synchronous" Tom Tromey
` (9 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Tom Tromey @ 2023-10-29 17:35 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
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 024b66cc7ad..e8cd4496395 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -745,7 +745,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] 18+ messages in thread
* [PATCH 06/15] Add "maint set dwarf synchronous"
2023-10-29 17:35 [PATCH 00/15] Index DWARF in the background Tom Tromey
` (4 preceding siblings ...)
2023-10-29 17:35 ` [PATCH 05/15] Move cooked_index_storage to cooked-index.h Tom Tromey
@ 2023-10-29 17:35 ` Tom Tromey
2023-10-29 17:50 ` Eli Zaretskii
2023-10-29 17:35 ` [PATCH 07/15] Change how cooked index waits for threads Tom Tromey
` (8 subsequent siblings)
14 siblings, 1 reply; 18+ messages in thread
From: Tom Tromey @ 2023-10-29 17:35 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
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 95663433f1c..088651a359b 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".
+
* Python API
** New function gdb.notify_mi(NAME, DATA), that emits custom
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index db1a82ec838..78fe5a27ff5 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -41692,6 +41692,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 e8cd4496395..8ee6c6d4762 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -739,6 +739,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 */
@@ -21899,6 +21909,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] 18+ messages in thread
* [PATCH 07/15] Change how cooked index waits for threads
2023-10-29 17:35 [PATCH 00/15] Index DWARF in the background Tom Tromey
` (5 preceding siblings ...)
2023-10-29 17:35 ` [PATCH 06/15] Add "maint set dwarf synchronous" Tom Tromey
@ 2023-10-29 17:35 ` Tom Tromey
2023-10-29 17:35 ` [PATCH 08/15] Do more DWARF reading in the background Tom Tromey
` (7 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Tom Tromey @ 2023-10-29 17:35 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
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] 18+ messages in thread
* [PATCH 08/15] Do more DWARF reading in the background
2023-10-29 17:35 [PATCH 00/15] Index DWARF in the background Tom Tromey
` (6 preceding siblings ...)
2023-10-29 17:35 ` [PATCH 07/15] Change how cooked index waits for threads Tom Tromey
@ 2023-10-29 17:35 ` Tom Tromey
2023-10-29 17:35 ` [PATCH 09/15] Simplify the public DWARF API Tom Tromey
` (6 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Tom Tromey @ 2023-10-29 17:35 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
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.dwarf2/dw2-missing-cu-tag.exp | 2 +
.../gdb.dwarf2/dw2-stack-boundary.exp | 2 +
.../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.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 8ee6c6d4762..3dbe65432d2 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.
@@ -758,8 +758,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);
@@ -3208,7 +3206,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
@@ -16476,26 +16593,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,
@@ -16513,55 +16664,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);
}
};
@@ -16572,8 +16696,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);
}
@@ -16585,11 +16707,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);
@@ -16608,11 +16726,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
@@ -16650,13 +16764,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);
@@ -16776,15 +16884,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 0c4e808e553..c300e9b0637 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\ncould 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] 18+ messages in thread
* [PATCH 09/15] Simplify the public DWARF API
2023-10-29 17:35 [PATCH 00/15] Index DWARF in the background Tom Tromey
` (7 preceding siblings ...)
2023-10-29 17:35 ` [PATCH 08/15] Do more DWARF reading in the background Tom Tromey
@ 2023-10-29 17:35 ` Tom Tromey
2023-10-29 17:35 ` [PATCH 10/15] Remove two quick_symbol_functions methods Tom Tromey
` (5 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Tom Tromey @ 2023-10-29 17:35 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
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 3dbe65432d2..5a32710233d 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -1357,14 +1357,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)
@@ -3211,9 +3207,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] 18+ messages in thread
* [PATCH 10/15] Remove two quick_symbol_functions methods
2023-10-29 17:35 [PATCH 00/15] Index DWARF in the background Tom Tromey
` (8 preceding siblings ...)
2023-10-29 17:35 ` [PATCH 09/15] Simplify the public DWARF API Tom Tromey
@ 2023-10-29 17:35 ` Tom Tromey
2023-10-29 17:35 ` [PATCH 11/15] Change current_language to be a macro Tom Tromey
` (4 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Tom Tromey @ 2023-10-29 17:35 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
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] 18+ messages in thread
* [PATCH 11/15] Change current_language to be a macro
2023-10-29 17:35 [PATCH 00/15] Index DWARF in the background Tom Tromey
` (9 preceding siblings ...)
2023-10-29 17:35 ` [PATCH 10/15] Remove two quick_symbol_functions methods Tom Tromey
@ 2023-10-29 17:35 ` Tom Tromey
2023-10-29 17:35 ` [PATCH 12/15] Lazy language setting Tom Tromey
` (3 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Tom Tromey @ 2023-10-29 17:35 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
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] 18+ messages in thread
* [PATCH 12/15] Lazy language setting
2023-10-29 17:35 [PATCH 00/15] Index DWARF in the background Tom Tromey
` (10 preceding siblings ...)
2023-10-29 17:35 ` [PATCH 11/15] Change current_language to be a macro Tom Tromey
@ 2023-10-29 17:35 ` Tom Tromey
2023-10-29 17:35 ` [PATCH 13/15] Optimize lookup_minimal_symbol_text Tom Tromey
` (2 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Tom Tromey @ 2023-10-29 17:35 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
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 c8145da59bc..94f2050c459 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] 18+ messages in thread
* [PATCH 13/15] Optimize lookup_minimal_symbol_text
2023-10-29 17:35 [PATCH 00/15] Index DWARF in the background Tom Tromey
` (11 preceding siblings ...)
2023-10-29 17:35 ` [PATCH 12/15] Lazy language setting Tom Tromey
@ 2023-10-29 17:35 ` Tom Tromey
2023-10-29 17:35 ` [PATCH 14/15] Avoid language-based lookups in startup path Tom Tromey
2023-10-29 17:35 ` [PATCH 15/15] Back out some parallel_for_each features Tom Tromey
14 siblings, 0 replies; 18+ messages in thread
From: Tom Tromey @ 2023-10-29 17:35 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
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] 18+ messages in thread
* [PATCH 14/15] Avoid language-based lookups in startup path
2023-10-29 17:35 [PATCH 00/15] Index DWARF in the background Tom Tromey
` (12 preceding siblings ...)
2023-10-29 17:35 ` [PATCH 13/15] Optimize lookup_minimal_symbol_text Tom Tromey
@ 2023-10-29 17:35 ` Tom Tromey
2023-10-29 17:35 ` [PATCH 15/15] Back out some parallel_for_each features Tom Tromey
14 siblings, 0 replies; 18+ messages in thread
From: Tom Tromey @ 2023-10-29 17:35 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
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 828c99cabc0..c57741514b3 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] 18+ messages in thread
* [PATCH 15/15] Back out some parallel_for_each features
2023-10-29 17:35 [PATCH 00/15] Index DWARF in the background Tom Tromey
` (13 preceding siblings ...)
2023-10-29 17:35 ` [PATCH 14/15] Avoid language-based lookups in startup path Tom Tromey
@ 2023-10-29 17:35 ` Tom Tromey
14 siblings, 0 replies; 18+ messages in thread
From: Tom Tromey @ 2023-10-29 17:35 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
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] 18+ messages in thread
* Re: [PATCH 06/15] Add "maint set dwarf synchronous"
2023-10-29 17:35 ` [PATCH 06/15] Add "maint set dwarf synchronous" Tom Tromey
@ 2023-10-29 17:50 ` Eli Zaretskii
[not found] ` <87v89tfz40.fsf@tromey.com>
0 siblings, 1 reply; 18+ messages in thread
From: Eli Zaretskii @ 2023-10-29 17:50 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
> From: Tom Tromey <tom@tromey.com>
> Cc: Tom Tromey <tom@tromey.com>
> Date: Sun, 29 Oct 2023 11:35:25 -0600
>
> 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(+)
Thanks.
> --- 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".
I'm guessing this isn't supported in all build configurations, and if
so, this text should say so, and should give some indication which
configurations don't support this feature.
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -41692,6 +41692,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.
Same here. I'm guessing on platforms where this doesn't work, this
setting doesn't have effect, and the default value is "synchronous"?
> +/* 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);
> +}
The comment seems to describe a different function?
> + add_setshow_boolean_cmd ("synchronous", class_obscure,
> + &dwarf_synchronous, _("\
> +Set whether DWARF is read synchronously."), _("\
> +Show DWARF is read synchronously."), _("\
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
"Show whether DWARF is read synchronously."
> +By default, DWARF information is read in worker threads,\n\
> +and gdb will not generally wait for this process to complete.\n\
> +Enabling this setting will cause the DWARF reader to always wait\n\
> +for completion before gdb can proceed."),
This could use some clarifications. The "wait for this process to
complete" and "wait for completion before gdb can proceed" parts are
somewhat mysterious: what exactly does "proceed" mean, and when it is
important to wait for this to complete?
IOW, this doc string leaves unsaid what processing needs DWARF reading
to complete.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 06/15] Add "maint set dwarf synchronous"
[not found] ` <87v89tfz40.fsf@tromey.com>
@ 2023-11-23 6:08 ` Eli Zaretskii
0 siblings, 0 replies; 18+ messages in thread
From: Eli Zaretskii @ 2023-11-23 6:08 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
> From: Tom Tromey <tom@tromey.com>
> Cc: Tom Tromey <tom@tromey.com>, gdb-patches@sourceware.org
> Date: Wed, 22 Nov 2023 10:59:27 -0700
>
> >>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:
>
> Eli> I'm guessing this isn't supported in all build configurations, and if
> Eli> so, this text should say so, and should give some indication which
> Eli> configurations don't support this feature.
>
> It's somewhat hard to know. Anybody can disable it at configure time,
> and of course there's the mingw situation -- but even that is
> complicated because I know working setups exist, just not universally.
OK, but at least let's mention that some builds don't support
threading in GDB, and those will always behave synchronously.
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2023-11-23 6:08 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-29 17:35 [PATCH 00/15] Index DWARF in the background Tom Tromey
2023-10-29 17:35 ` [PATCH 01/15] Add thread-safety to gdb's BFD wrappers Tom Tromey
2023-10-29 17:35 ` [PATCH 02/15] Refactor complaint thread-safety approach Tom Tromey
2023-10-29 17:35 ` [PATCH 03/15] Add quick_symbol_functions::compute_main_name Tom Tromey
2023-10-29 17:35 ` [PATCH 04/15] Add gdb::task_group Tom Tromey
2023-10-29 17:35 ` [PATCH 05/15] Move cooked_index_storage to cooked-index.h Tom Tromey
2023-10-29 17:35 ` [PATCH 06/15] Add "maint set dwarf synchronous" Tom Tromey
2023-10-29 17:50 ` Eli Zaretskii
[not found] ` <87v89tfz40.fsf@tromey.com>
2023-11-23 6:08 ` Eli Zaretskii
2023-10-29 17:35 ` [PATCH 07/15] Change how cooked index waits for threads Tom Tromey
2023-10-29 17:35 ` [PATCH 08/15] Do more DWARF reading in the background Tom Tromey
2023-10-29 17:35 ` [PATCH 09/15] Simplify the public DWARF API Tom Tromey
2023-10-29 17:35 ` [PATCH 10/15] Remove two quick_symbol_functions methods Tom Tromey
2023-10-29 17:35 ` [PATCH 11/15] Change current_language to be a macro Tom Tromey
2023-10-29 17:35 ` [PATCH 12/15] Lazy language setting Tom Tromey
2023-10-29 17:35 ` [PATCH 13/15] Optimize lookup_minimal_symbol_text Tom Tromey
2023-10-29 17:35 ` [PATCH 14/15] Avoid language-based lookups in startup path Tom Tromey
2023-10-29 17:35 ` [PATCH 15/15] Back out some parallel_for_each features Tom Tromey
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).