From 6f214d0f399847b13f979651c3b46befcbb42140 Mon Sep 17 00:00:00 2001 From: Tom Tromey Date: Fri, 24 Mar 2023 15:53:22 -0600 Subject: [PATCH] Fix race in background index-cache writing Tom de Vries pointed out a bug in the index-cache background writer -- sometimes it will fail. He also noted that it fails when the number of worker threads is set to zero. These turn out to be the same problem -- the cache can't be written to until the per-BFD's "index_table" member is set. This patch avoids the race by rearranging the code slightly, to ensure the cache cannot possibly be written before the member is set. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30261 --- gdb/dwarf2/cooked-index.c | 24 +++++++++++++++--------- gdb/dwarf2/cooked-index.h | 5 ++++- gdb/dwarf2/read.c | 6 +++++- 3 files changed, 24 insertions(+), 11 deletions(-) diff --git a/gdb/dwarf2/cooked-index.c b/gdb/dwarf2/cooked-index.c index 900f13c7de9..1b1a16b1ae2 100644 --- a/gdb/dwarf2/cooked-index.c +++ b/gdb/dwarf2/cooked-index.c @@ -443,26 +443,32 @@ cooked_index_shard::wait (bool allow_quit) const m_future.wait (); } -cooked_index::cooked_index (vec_type &&vec, dwarf2_per_bfd *per_bfd) +cooked_index::cooked_index (vec_type &&vec) : m_vector (std::move (vec)) { for (auto &idx : m_vector) idx->finalize (); - /* 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] () - { - maybe_write_index (per_bfd); - }); - /* 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_writing_index (dwarf2_per_bfd *per_bfd) +{ + /* 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] () + { + maybe_write_index (per_bfd); + }); +} + cooked_index::~cooked_index () { /* The 'finalize' method may be run in a different thread. If diff --git a/gdb/dwarf2/cooked-index.h b/gdb/dwarf2/cooked-index.h index b16b87bbb0e..f2664929e52 100644 --- a/gdb/dwarf2/cooked-index.h +++ b/gdb/dwarf2/cooked-index.h @@ -366,7 +366,7 @@ public: object. */ using vec_type = std::vector>; - cooked_index (vec_type &&vec, dwarf2_per_bfd *per_bfd); + explicit cooked_index (vec_type &&vec); ~cooked_index () override; DISABLE_COPY_AND_ASSIGN (cooked_index); @@ -429,6 +429,9 @@ public: 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); + private: /* Maybe write the index to the index cache. */ diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index b362dfddb0c..8f35b973f3e 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -5147,9 +5147,13 @@ dwarf2_build_psymtabs_hard (dwarf2_per_objfile *per_objfile) indexes.push_back (index_storage.release ()); indexes.shrink_to_fit (); - cooked_index *vec = new cooked_index (std::move (indexes), per_bfd); + cooked_index *vec = new cooked_index (std::move (indexes)); per_bfd->index_table.reset (vec); + /* Cannot start writing the index entry until after the + 'index_table' member has been set. */ + vec->start_writing_index (per_bfd); + const cooked_index_entry *main_entry = vec->get_main (); if (main_entry != nullptr) { -- 2.30.2