public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v3 0/5] [gdb/symtab] Fix data-races in gdb.base/index-cache.exp
@ 2023-08-04  0:03 Tom de Vries
  2023-08-04  0:03 ` [PATCH v3 1/5] [gdb/symtab] Fix data race on index_cache::m_enabled Tom de Vries
                   ` (5 more replies)
  0 siblings, 6 replies; 9+ messages in thread
From: Tom de Vries @ 2023-08-04  0:03 UTC (permalink / raw)
  To: gdb-patches

When building gdb with -fsanitize=thread, we run into a data race in
gdb.base/index-cache.exp.

Fixing this leads us to another, and so on, so each patch addresses one
particular data race, with the exception of the last patch, which extends
the test-case a bit.

The last patch, when applied without the series runs into a segfault with
target board native-extended-gdbserver, filed as PR symtab/30712, but that
seems to be fixed by a previous commit in this series.  This is the reason for
which the patch is part of this series.

The first two patches implement the approach mentioned in PR30392 comment 2:
...
The reader probably should capture the necessarily globals
on the main thread and stash them until the index has been
written.
...

The 3rd patch cannot be fixed with this approach, so it uses the packed<bool, 1>
approach:
...
-  unsigned int queued : 1;
+  packed<bool, 1> queued;
...

There's one more patch like that, I checked using pahole that the struct size
is not increased.

I spent some time convincing myself that the data races on disjoint bitfields
are not benign.  I started with reading [1], and got convinced by
"2.5 Disjoint bit manipulation" in [2].  Also [3] looked interesting, but
haven't read it in full.

Tested on x86_64-linux, with and without -fsanitize=thread.

PR symtab/30392
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30392

References:
[1] https://hacks.mozilla.org/2021/04/eliminating-data-races-in-firefox-a-technical-report/
[2] https://www.usenix.org/legacy/event/hotpar11/tech/final_files/Boehm.pdf
[3] https://bartoszmilewski.com/2020/08/11/benign-data-races-considered-harmful/

Tom de Vries (5):
  [gdb/symtab] Fix data race on index_cache::m_enabled
  [gdb/symtab] Fix data race on bfd::{cacheable,format}
  [gdb/symtab] Fix race on dwarf2_per_cu_data::{queued,is_debug_type}
  [gdb/symtab] Fix data race on
    dwarf2_per_cu_data::{m_header_read_in,is_debug_type}
  [gdb/testsuite] Extend gdb.base/index-cache.exp

 gdb/dwarf2/cooked-index.c              | 12 ++++---
 gdb/dwarf2/cooked-index.h              |  4 ++-
 gdb/dwarf2/index-cache.c               | 48 +++++++++++++++++++-------
 gdb/dwarf2/index-cache.h               | 25 +++++++++++++-
 gdb/dwarf2/read.h                      | 26 +++++++-------
 gdb/testsuite/gdb.base/index-cache-2.c | 24 +++++++++++++
 gdb/testsuite/gdb.base/index-cache.c   |  6 ++--
 gdb/testsuite/gdb.base/index-cache.exp | 22 ++++++++++--
 8 files changed, 132 insertions(+), 35 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/index-cache-2.c


base-commit: 8d76ceb7f1d2f0d25b76b0a16afd25dc0ba74dca
-- 
2.35.3


^ permalink raw reply	[flat|nested] 9+ messages in thread
* [PATCH v3 1/5] [gdb/symtab] Fix data race on index_cache::m_enabled
  2023-08-04  0:03 [PATCH v3 0/5] [gdb/symtab] Fix data-races in gdb.base/index-cache.exp Tom de Vries
@ 2023-08-04  0:03 ` Tom de Vries
  2023-08-06 21:01   ` Tom Tromey
  2023-08-04  0:03 ` [PATCH v3 2/5] [gdb/symtab] Fix data race on bfd::{cacheable,format} Tom de Vries
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 9+ messages in thread
From: Tom de Vries @ 2023-08-04  0:03 UTC (permalink / raw)
  To: gdb-patches

With gdb build with -fsanitize=thread and test-case gdb.base/index-cache.exp I
run into:
...
(gdb) file build/gdb/testsuite/outputs/gdb.base/index-cache/index-cache
Reading symbols from build/gdb/testsuite/outputs/gdb.base/index-cache/index-cache...
(gdb) show index-cache enabled
The index cache is off.
(gdb) PASS: gdb.base/index-cache.exp: test_basic_stuff: index-cache is disabled by default
set index-cache enabled on
==================
WARNING: ThreadSanitizer: data race (pid=32248)
  Write of size 1 at 0x00000321f540 by main thread:
    #0 index_cache::enable() gdb/dwarf2/index-cache.c:76 (gdb+0x82cfdd)
    #1 set_index_cache_enabled_command gdb/dwarf2/index-cache.c:270 (gdb+0x82d9af)
    #2 bool setting::set<bool>(bool const&) gdb/command.h:353 (gdb+0x6fe5f2)
    #3 do_set_command(char const*, int, cmd_list_element*) gdb/cli/cli-setshow.c:414 (gdb+0x6fcd21)
    #4 execute_command(char const*, int) gdb/top.c:567 (gdb+0xff2e64)
    #5 command_handler(char const*) gdb/event-top.c:552 (gdb+0x94acc0)
    #6 command_line_handler(std::unique_ptr<char, gdb::xfree_deleter<char> >&&) gdb/event-top.c:788 (gdb+0x94b37d)
    #7 tui_command_line_handler gdb/tui/tui-interp.c:104 (gdb+0x103467e)
    #8 gdb_rl_callback_handler gdb/event-top.c:259 (gdb+0x94a265)
    #9 rl_callback_read_char readline/readline/callback.c:290 (gdb+0x11bdd3f)
    #10 gdb_rl_callback_read_char_wrapper_noexcept gdb/event-top.c:195 (gdb+0x94a064)
    #11 gdb_rl_callback_read_char_wrapper gdb/event-top.c:234 (gdb+0x94a125)
    #12 stdin_event_handler gdb/ui.c:155 (gdb+0x1074922)
    #13 handle_file_event gdbsupport/event-loop.cc:573 (gdb+0x1d94de4)
    #14 gdb_wait_for_event gdbsupport/event-loop.cc:694 (gdb+0x1d9551c)
    #15 gdb_do_one_event(int) gdbsupport/event-loop.cc:264 (gdb+0x1d93908)
    #16 start_event_loop gdb/main.c:412 (gdb+0xb5a256)
    #17 captured_command_loop gdb/main.c:476 (gdb+0xb5a445)
    #18 captured_main gdb/main.c:1320 (gdb+0xb5c5c5)
    #19 gdb_main(captured_main_args*) gdb/main.c:1339 (gdb+0xb5c674)
    #20 main gdb/gdb.c:32 (gdb+0x416776)

  Previous read of size 1 at 0x00000321f540 by thread T12:
    #0 index_cache::enabled() const gdb/dwarf2/index-cache.h:48 (gdb+0x82e1a6)
    #1 index_cache::store(dwarf2_per_bfd*) gdb/dwarf2/index-cache.c:94 (gdb+0x82d0bc)
    #2 cooked_index::maybe_write_index(dwarf2_per_bfd*) gdb/dwarf2/cooked-index.c:638 (gdb+0x7f1b97)
    #3 operator() gdb/dwarf2/cooked-index.c:468 (gdb+0x7f0f24)
    #4 _M_invoke /usr/include/c++/7/bits/std_function.h:316 (gdb+0x7f285b)
    #5 std::function<void ()>::operator()() const /usr/include/c++/7/bits/std_function.h:706 (gdb+0x700952)
    #6 void std::__invoke_impl<void, std::function<void ()>&>(std::__invoke_other, std::function<void ()>&) /usr/include/c++/7/bits/invoke.h:60 (gdb+0x7381a0)
    #7 std::__invoke_result<std::function<void ()>&>::type std::__invoke<std::function<void ()>&>(std::function<void ()>&) /usr/include/c++/7/bits/invoke.h:95 (gdb+0x737e91)
    #8 std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run()::{lambda()#1}::operator()() const /usr/include/c++/7/future:1421 (gdb+0x737b59)
    #9 std::__future_base::_Task_setter<std::unique_ptr<std::__future_base::_Result<void>, std::__future_base::_Result_base::_Deleter>, std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run()::{lambda()#1}, void>::operator()() const /usr/include/c++/7/future:1362 (gdb+0x738660)
    #10 std::_Function_handler<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> (), std::__future_base::_Task_setter<std::unique_ptr<std::__future_base::_Result<void>, std::__future_base::_Result_base::_Deleter>, std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run()::{lambda()#1}, void> >::_M_invoke(std::_Any_data const&) /usr/include/c++/7/bits/std_function.h:302 (gdb+0x73825c)
    #11 std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>::operator()() const /usr/include/c++/7/bits/std_function.h:706 (gdb+0x733623)
    #12 std::__future_base::_State_baseV2::_M_do_set(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*) /usr/include/c++/7/future:561 (gdb+0x732bdf)
    #13 void std::__invoke_impl<void, void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::__invoke_memfun_deref, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&) /usr/include/c++/7/bits/invoke.h:73 (gdb+0x734c4f)
    #14 std::__invoke_result<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>::type std::__invoke<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&) /usr/include/c++/7/bits/invoke.h:95 (gdb+0x733bc5)
    #15 std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&)::{lambda()#1}::operator()() const /usr/include/c++/7/mutex:672 (gdb+0x73300d)
    #16 std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&)::{lambda()#2}::operator()() const /usr/include/c++/7/mutex:677 (gdb+0x7330b2)
    #17 std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&)::{lambda()#2}::_FUN() /usr/include/c++/7/mutex:677 (gdb+0x7330f2)
    #18 pthread_once <null> (libtsan.so.0+0x4457c)
    #19 __gthread_once /usr/include/c++/7/x86_64-suse-linux/bits/gthr-default.h:699 (gdb+0x72f5dd)
    #20 void std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&) /usr/include/c++/7/mutex:684 (gdb+0x733224)
    #21 std::__future_base::_State_baseV2::_M_set_result(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>, bool) /usr/include/c++/7/future:401 (gdb+0x732852)
    #22 std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run() /usr/include/c++/7/future:1423 (gdb+0x737bef)
    #23 std::packaged_task<void ()>::operator()() /usr/include/c++/7/future:1556 (gdb+0x1dac492)
    #24 gdb::thread_pool::thread_function() gdbsupport/thread-pool.cc:242 (gdb+0x1dabdb4)
    #25 void std::__invoke_impl<void, void (gdb::thread_pool::*)(), gdb::thread_pool*>(std::__invoke_memfun_deref, void (gdb::thread_pool::*&&)(), gdb::thread_pool*&&) /usr/include/c++/7/bits/invoke.h:73 (gdb+0x1dace63)
    #26 std::__invoke_result<void (gdb::thread_pool::*)(), gdb::thread_pool*>::type std::__invoke<void (gdb::thread_pool::*)(), gdb::thread_pool*>(void (gdb::thread_pool::*&&)(), gdb::thread_pool*&&) /usr/include/c++/7/bits/invoke.h:95 (gdb+0x1dac294)
    #27 decltype (__invoke((_S_declval<0ul>)(), (_S_declval<1ul>)())) std::thread::_Invoker<std::tuple<void (gdb::thread_pool::*)(), gdb::thread_pool*> >::_M_invoke<0ul, 1ul>(std::_Index_tuple<0ul, 1ul>) /usr/include/c++/7/thread:234 (gdb+0x1daf5c6)
    #28 std::thread::_Invoker<std::tuple<void (gdb::thread_pool::*)(), gdb::thread_pool*> >::operator()() /usr/include/c++/7/thread:243 (gdb+0x1daf551)
    #29 std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (gdb::thread_pool::*)(), gdb::thread_pool*> > >::_M_run() /usr/include/c++/7/thread:186 (gdb+0x1daf506)
    #30 <null> <null> (libstdc++.so.6+0xdcac2)

  Location is global 'global_index_cache' of size 48 at 0x00000321f520 (gdb+0x00000321f540)
  ...
SUMMARY: ThreadSanitizer: data race gdb/dwarf2/index-cache.c:76 in index_cache::enable()
...

The race happens when issuing a "file $exec" command followed by a
"set index-cache enabled on" command.

The race is between:
- a worker thread reading index_cache::m_enabled to determine whether an
  index-cache entry for $exec needs to be written
  (due to command "file $exec"), and
- the main thread setting index_cache::m_enabled
  (due to command "set index-cache enabled on").

Fix this by capturing the value of index_cache::m_enabled in the main thread,
and using the captured value in the worker thread.

Tested on x86_64-linux.

PR symtab/30392
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30392
---
 gdb/dwarf2/cooked-index.c | 12 ++++++++----
 gdb/dwarf2/cooked-index.h |  4 +++-
 gdb/dwarf2/index-cache.c  | 12 ++++++++++--
 gdb/dwarf2/index-cache.h  | 18 +++++++++++++++++-
 4 files changed, 38 insertions(+), 8 deletions(-)

diff --git a/gdb/dwarf2/cooked-index.c b/gdb/dwarf2/cooked-index.c
index 25635d9b72e..7ddcdedf9d0 100644
--- a/gdb/dwarf2/cooked-index.c
+++ b/gdb/dwarf2/cooked-index.c
@@ -460,12 +460,15 @@ cooked_index::cooked_index (vec_type &&vec)
 void
 cooked_index::start_writing_index (dwarf2_per_bfd *per_bfd)
 {
+  index_cache_store_context ctx (global_index_cache);
+
   /* 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] ()
+    = gdb::thread_pool::g_thread_pool->post_task ([this, per_bfd,
+						   ctx = std::move (ctx)] ()
 	{
-	  maybe_write_index (per_bfd);
+	  maybe_write_index (per_bfd, ctx);
 	});
 }
 
@@ -629,13 +632,14 @@ cooked_index::dump (gdbarch *arch) const
 }
 
 void
-cooked_index::maybe_write_index (dwarf2_per_bfd *per_bfd)
+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);
+  global_index_cache.store (per_bfd, ctx);
 }
 
 /* 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 0d6f3e5aa0e..5aacb321c91 100644
--- a/gdb/dwarf2/cooked-index.h
+++ b/gdb/dwarf2/cooked-index.h
@@ -37,6 +37,7 @@
 
 struct dwarf2_per_cu_data;
 struct dwarf2_per_bfd;
+struct index_cache_store_context;
 
 /* Flags that describe an entry in the index.  */
 enum cooked_index_flag_enum : unsigned char
@@ -435,7 +436,8 @@ class cooked_index : public dwarf_scanner_base
 private:
 
   /* Maybe write the index to the index cache.  */
-  void maybe_write_index (dwarf2_per_bfd *per_bfd);
+  void maybe_write_index (dwarf2_per_bfd *per_bfd,
+			  const index_cache_store_context &);
 
   /* The vector of cooked_index objects.  This is stored because the
      entries are stored on the obstacks in those objects.  */
diff --git a/gdb/dwarf2/index-cache.c b/gdb/dwarf2/index-cache.c
index 79ab706ee9d..e2fb984799c 100644
--- a/gdb/dwarf2/index-cache.c
+++ b/gdb/dwarf2/index-cache.c
@@ -86,12 +86,20 @@ index_cache::disable ()
   m_enabled = false;
 }
 
+ /* See index-cache.h.  */
+
+index_cache_store_context::index_cache_store_context (const index_cache &ic)
+  :  m_enabled (ic.enabled ())
+{
+}
+
 /* See dwarf-index-cache.h.  */
 
 void
-index_cache::store (dwarf2_per_bfd *per_bfd)
+index_cache::store (dwarf2_per_bfd *per_bfd,
+		    const index_cache_store_context &ctx)
 {
-  if (!enabled ())
+  if (!ctx.m_enabled)
     return;
 
   /* Get build id of objfile.  */
diff --git a/gdb/dwarf2/index-cache.h b/gdb/dwarf2/index-cache.h
index 1efff17049f..7ea972d5c7f 100644
--- a/gdb/dwarf2/index-cache.h
+++ b/gdb/dwarf2/index-cache.h
@@ -25,6 +25,7 @@
 #include "symfile.h"
 
 class dwarf2_per_bfd;
+class index_cache;
 
 /* Base of the classes used to hold the resources of the indices loaded from
    the cache (e.g. mmapped files).  */
@@ -34,6 +35,20 @@ struct index_cache_resource
   virtual ~index_cache_resource () = 0;
 };
 
+/* Information to be captured in the main thread, and to be used by worker
+   threads during store ().  */
+
+struct index_cache_store_context
+{
+  friend class index_cache;
+
+  explicit index_cache_store_context (const index_cache &ic);
+
+private:
+  /* Captured value of enabled ().  */
+  bool m_enabled;
+};
+
 /* Class to manage the access to the DWARF index cache.  */
 
 class index_cache
@@ -55,7 +70,8 @@ class index_cache
   void disable ();
 
   /* Store an index for the specified object file in the cache.  */
-  void store (dwarf2_per_bfd *per_bfd);
+  void store (dwarf2_per_bfd *per_bfd,
+	      const index_cache_store_context &);
 
   /* Look for an index file matching BUILD_ID.  If found, return the contents
      as an array_view and store the underlying resources (allocated memory,
-- 
2.35.3


^ permalink raw reply	[flat|nested] 9+ messages in thread
* [PATCH v3 2/5] [gdb/symtab] Fix data race on bfd::{cacheable,format}
  2023-08-04  0:03 [PATCH v3 0/5] [gdb/symtab] Fix data-races in gdb.base/index-cache.exp Tom de Vries
  2023-08-04  0:03 ` [PATCH v3 1/5] [gdb/symtab] Fix data race on index_cache::m_enabled Tom de Vries
@ 2023-08-04  0:03 ` Tom de Vries
  2023-08-04  0:03 ` [PATCH v3 3/5] [gdb/symtab] Fix race on dwarf2_per_cu_data::{queued,is_debug_type} Tom de Vries
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Tom de Vries @ 2023-08-04  0:03 UTC (permalink / raw)
  To: gdb-patches

With gdb build with -fsanitize=thread and test-case gdb.base/index-cache.exp I
run into:
...
(gdb) file build/gdb/testsuite/outputs/gdb.base/index-cache/index-cache
Reading symbols from build/gdb/testsuite/outputs/gdb.base/index-cache/index-cache...
==================
WARNING: ThreadSanitizer: data race (pid=12261)
  Write of size 4 at 0x7b4400097d08 by main thread:
    #0 bfd_open_file bfd/cache.c:584 (gdb+0x148bb92)
    #1 bfd_cache_lookup_worker bfd/cache.c:261 (gdb+0x148b12a)
    #2 cache_bseek bfd/cache.c:289 (gdb+0x148b324)
    #3 bfd_seek bfd/bfdio.c:459 (gdb+0x1489c31)
    #4 _bfd_generic_get_section_contents bfd/libbfd.c:1069 (gdb+0x14977a4)
    #5 bfd_get_section_contents bfd/section.c:1606 (gdb+0x149cc7c)
    #6 gdb_bfd_scan_elf_dyntag(int, bfd*, unsigned long*, unsigned long*) gdb/solib.c:1601 (gdb+0xed8eca)
    #7 elf_locate_base gdb/solib-svr4.c:705 (gdb+0xec28ac)
    #8 svr4_iterate_over_objfiles_in_search_order gdb/solib-svr4.c:3430 (gdb+0xeca55d)
    #9 gdbarch_iterate_over_objfiles_in_search_order(gdbarch*, gdb::function_view<bool (objfile*)>, objfile*) gdb/gdbarch.c:5041 (gdb+0x537cad)
    #10 find_main_name gdb/symtab.c:6270 (gdb+0xf743a5)
    #11 main_language() gdb/symtab.c:6313 (gdb+0xf74499)
    #12 set_initial_language() gdb/symfile.c:1700 (gdb+0xf4285c)
    #13 symbol_file_add_main_1 gdb/symfile.c:1212 (gdb+0xf40e2a)
    #14 symbol_file_command(char const*, int) gdb/symfile.c:1681 (gdb+0xf427d1)
    #15 file_command gdb/exec.c:554 (gdb+0x94f74b)
    #16 do_simple_func gdb/cli/cli-decode.c:95 (gdb+0x6d9528)
    #17 cmd_func(cmd_list_element*, char const*, int) gdb/cli/cli-decode.c:2735 (gdb+0x6e0f69)
    #18 execute_command(char const*, int) gdb/top.c:575 (gdb+0xff303c)
    #19 command_handler(char const*) gdb/event-top.c:552 (gdb+0x94adde)
    #20 command_line_handler(std::unique_ptr<char, gdb::xfree_deleter<char> >&&) gdb/event-top.c:788 (gdb+0x94b49b)
    #21 tui_command_line_handler gdb/tui/tui-interp.c:104 (gdb+0x103479c)
    #22 gdb_rl_callback_handler gdb/event-top.c:259 (gdb+0x94a383)
    #23 rl_callback_read_char readline/readline/callback.c:290 (gdb+0x11bde5d)
    #24 gdb_rl_callback_read_char_wrapper_noexcept gdb/event-top.c:195 (gdb+0x94a182)
    #25 gdb_rl_callback_read_char_wrapper gdb/event-top.c:234 (gdb+0x94a243)
    #26 stdin_event_handler gdb/ui.c:155 (gdb+0x1074a40)
    #27 handle_file_event gdbsupport/event-loop.cc:573 (gdb+0x1d94f02)
    #28 gdb_wait_for_event gdbsupport/event-loop.cc:694 (gdb+0x1d9563a)
    #29 gdb_do_one_event(int) gdbsupport/event-loop.cc:264 (gdb+0x1d93a26)
    #30 start_event_loop gdb/main.c:412 (gdb+0xb5a374)
    #31 captured_command_loop gdb/main.c:476 (gdb+0xb5a563)
    #32 captured_main gdb/main.c:1320 (gdb+0xb5c6e3)
    #33 gdb_main(captured_main_args*) gdb/main.c:1339 (gdb+0xb5c792)
    #34 main gdb/gdb.c:32 (gdb+0x416776)

  Previous read of size 1 at 0x7b4400097d08 by thread T12:
    #0 bfd_check_format_matches bfd/format.c:323 (gdb+0x1492db4)
    #1 bfd_check_format bfd/format.c:94 (gdb+0x1492104)
    #2 build_id_bfd_get(bfd*) gdb/build-id.c:42 (gdb+0x6648f7)
    #3 index_cache::store(dwarf2_per_bfd*, index_cache_store_context*) gdb/dwarf2/index-cache.c:110 (gdb+0x82d205)
    #4 cooked_index::maybe_write_index(dwarf2_per_bfd*) gdb/dwarf2/cooked-index.c:640 (gdb+0x7f1bf1)
    #5 operator() gdb/dwarf2/cooked-index.c:470 (gdb+0x7f0f40)
    #6 _M_invoke /usr/include/c++/7/bits/std_function.h:316 (gdb+0x7f28f7)
    #7 std::function<void ()>::operator()() const /usr/include/c++/7/bits/std_function.h:706 (gdb+0x700952)
    #8 void std::__invoke_impl<void, std::function<void ()>&>(std::__invoke_other, std::function<void ()>&) /usr/include/c++/7/bits/invoke.h:60 (gdb+0x7381a0)
    #9 std::__invoke_result<std::function<void ()>&>::type std::__invoke<std::function<void ()>&>(std::function<void ()>&) /usr/include/c++/7/bits/invoke.h:95 (gdb+0x737e91)
    #10 std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run()::{lambda()#1}::operator()() const /usr/include/c++/7/future:1421 (gdb+0x737b59)
    #11 std::__future_base::_Task_setter<std::unique_ptr<std::__future_base::_Result<void>, std::__future_base::_Result_base::_Deleter>, std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run()::{lambda()#1}, void>::operator()() const /usr/include/c++/7/future:1362 (gdb+0x738660)
    #12 std::_Function_handler<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> (), std::__future_base::_Task_setter<std::unique_ptr<std::__future_base::_Result<void>, std::__future_base::_Result_base::_Deleter>, std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run()::{lambda()#1}, void> >::_M_invoke(std::_Any_data const&) /usr/include/c++/7/bits/std_function.h:302 (gdb+0x73825c)
    #13 std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>::operator()() const /usr/include/c++/7/bits/std_function.h:706 (gdb+0x733623)
    #14 std::__future_base::_State_baseV2::_M_do_set(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*) /usr/include/c++/7/future:561 (gdb+0x732bdf)
    #15 void std::__invoke_impl<void, void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::__invoke_memfun_deref, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&) /usr/include/c++/7/bits/invoke.h:73 (gdb+0x734c4f)
    #16 std::__invoke_result<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>::type std::__invoke<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&) /usr/include/c++/7/bits/invoke.h:95 (gdb+0x733bc5)
    #17 std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&)::{lambda()#1}::operator()() const /usr/include/c++/7/mutex:672 (gdb+0x73300d)
    #18 std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&)::{lambda()#2}::operator()() const /usr/include/c++/7/mutex:677 (gdb+0x7330b2)
    #19 std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&)::{lambda()#2}::_FUN() /usr/include/c++/7/mutex:677 (gdb+0x7330f2)
    #20 pthread_once <null> (libtsan.so.0+0x4457c)
    #21 __gthread_once /usr/include/c++/7/x86_64-suse-linux/bits/gthr-default.h:699 (gdb+0x72f5dd)
    #22 void std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&) /usr/include/c++/7/mutex:684 (gdb+0x733224)
    #23 std::__future_base::_State_baseV2::_M_set_result(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>, bool) /usr/include/c++/7/future:401 (gdb+0x732852)
    #24 std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run() /usr/include/c++/7/future:1423 (gdb+0x737bef)
    #25 std::packaged_task<void ()>::operator()() /usr/include/c++/7/future:1556 (gdb+0x1dac5b0)
    #26 gdb::thread_pool::thread_function() gdbsupport/thread-pool.cc:242 (gdb+0x1dabed2)
    #27 void std::__invoke_impl<void, void (gdb::thread_pool::*)(), gdb::thread_pool*>(std::__invoke_memfun_deref, void (gdb::thread_pool::*&&)(), gdb::thread_pool*&&) /usr/include/c++/7/bits/invoke.h:73 (gdb+0x1dacf81)
    #28 std::__invoke_result<void (gdb::thread_pool::*)(), gdb::thread_pool*>::type std::__invoke<void (gdb::thread_pool::*)(), gdb::thread_pool*>(void (gdb::thread_pool::*&&)(), gdb::thread_pool*&&) /usr/include/c++/7/bits/invoke.h:95 (gdb+0x1dac3b2)
    #29 decltype (__invoke((_S_declval<0ul>)(), (_S_declval<1ul>)())) std::thread::_Invoker<std::tuple<void (gdb::thread_pool::*)(), gdb::thread_pool*> >::_M_invoke<0ul, 1ul>(std::_Index_tuple<0ul, 1ul>) /usr/include/c++/7/thread:234 (gdb+0x1daf6e4)
    #30 std::thread::_Invoker<std::tuple<void (gdb::thread_pool::*)(), gdb::thread_pool*> >::operator()() /usr/include/c++/7/thread:243 (gdb+0x1daf66f)
    #31 std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (gdb::thread_pool::*)(), gdb::thread_pool*> > >::_M_run() /usr/include/c++/7/thread:186 (gdb+0x1daf624)
    #32 <null> <null> (libstdc++.so.6+0xdcac2)
  ...
SUMMARY: ThreadSanitizer: data race bfd/cache.c:584 in bfd_open_file
...

The race happens when issuing the "file $exec" command.

The race is between:
- a worker thread getting the build id while writing the index cache, and in
  the process reading bfd::format, and
- the main thread calling find_main_name, and in the process setting
  bfd::cacheable.

The two bitfields bfd::cacheable and bfd::format share the same bitfield
container.

Fix this by capturing the build id in the main thread, and using the captured
value in the worker thread.

Likewise for the dwz build id, which likely suffers from the same issue.

While we're at it, also move the creation of the cache directory to
the index_cache_store_context constructor, to:
- make sure there's no race between subsequent file commands, and
- issue any related warning or error messages during the file command.

Tested on x86_64-linux.

Approved-By: Tom Tromey <tom@tromey.com>

PR symtab/30392
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30392
---
 gdb/dwarf2/cooked-index.c |  2 +-
 gdb/dwarf2/index-cache.c  | 52 +++++++++++++++++++++++++--------------
 gdb/dwarf2/index-cache.h  |  9 ++++++-
 3 files changed, 43 insertions(+), 20 deletions(-)

diff --git a/gdb/dwarf2/cooked-index.c b/gdb/dwarf2/cooked-index.c
index 7ddcdedf9d0..df82e86b690 100644
--- a/gdb/dwarf2/cooked-index.c
+++ b/gdb/dwarf2/cooked-index.c
@@ -460,7 +460,7 @@ cooked_index::cooked_index (vec_type &&vec)
 void
 cooked_index::start_writing_index (dwarf2_per_bfd *per_bfd)
 {
-  index_cache_store_context ctx (global_index_cache);
+  index_cache_store_context ctx (global_index_cache, per_bfd);
 
   /* This must be set after all the finalization tasks have been
      started, because it may call 'wait'.  */
diff --git a/gdb/dwarf2/index-cache.c b/gdb/dwarf2/index-cache.c
index e2fb984799c..69f70642dc6 100644
--- a/gdb/dwarf2/index-cache.c
+++ b/gdb/dwarf2/index-cache.c
@@ -88,18 +88,11 @@ index_cache::disable ()
 
  /* See index-cache.h.  */
 
-index_cache_store_context::index_cache_store_context (const index_cache &ic)
+index_cache_store_context::index_cache_store_context (const index_cache &ic,
+						      dwarf2_per_bfd *per_bfd)
   :  m_enabled (ic.enabled ())
 {
-}
-
-/* See dwarf-index-cache.h.  */
-
-void
-index_cache::store (dwarf2_per_bfd *per_bfd,
-		    const index_cache_store_context &ctx)
-{
-  if (!ctx.m_enabled)
+  if (!m_enabled)
     return;
 
   /* Get build id of objfile.  */
@@ -108,15 +101,13 @@ index_cache::store (dwarf2_per_bfd *per_bfd,
     {
       index_cache_debug ("objfile %s has no build id",
 			 bfd_get_filename (per_bfd->obfd));
+      m_enabled = false;
       return;
     }
-
-  std::string build_id_str = build_id_to_string (build_id);
+  build_id_str = build_id_to_string (build_id);
 
   /* Get build id of dwz file, if present.  */
-  gdb::optional<std::string> dwz_build_id_str;
   const dwz_file *dwz = dwarf2_get_dwz_file (per_bfd);
-  const char *dwz_build_id_ptr = NULL;
 
   if (dwz != nullptr)
     {
@@ -126,36 +117,61 @@ index_cache::store (dwarf2_per_bfd *per_bfd,
 	{
 	  index_cache_debug ("dwz objfile %s has no build id",
 			     dwz->filename ());
+	  m_enabled = false;
 	  return;
 	}
 
       dwz_build_id_str = build_id_to_string (dwz_build_id);
-      dwz_build_id_ptr = dwz_build_id_str->c_str ();
     }
 
-  if (m_dir.empty ())
+  if (ic.m_dir.empty ())
     {
       warning (_("The index cache directory name is empty, skipping store."));
+      m_enabled = false;
       return;
     }
 
   try
     {
       /* Try to create the containing directory.  */
-      if (!mkdir_recursive (m_dir.c_str ()))
+      if (!mkdir_recursive (ic.m_dir.c_str ()))
 	{
 	  warning (_("index cache: could not make cache directory: %s"),
 		   safe_strerror (errno));
+	  m_enabled = false;
 	  return;
 	}
+    }
+  catch (const gdb_exception_error &except)
+    {
+      index_cache_debug ("couldn't store index cache for objfile %s: %s",
+			 bfd_get_filename (per_bfd->obfd), except.what ());
+      m_enabled = false;
+    }
+}
+
+/* See dwarf-index-cache.h.  */
 
+void
+index_cache::store (dwarf2_per_bfd *per_bfd,
+		    const index_cache_store_context &ctx)
+{
+  if (!ctx.m_enabled)
+    return;
+
+  const char *dwz_build_id_ptr = (ctx.dwz_build_id_str.has_value ()
+				  ? ctx.dwz_build_id_str->c_str ()
+				  : nullptr);
+
+  try
+    {
       index_cache_debug ("writing index cache for objfile %s",
 			 bfd_get_filename (per_bfd->obfd));
 
       /* Write the index itself to the directory, using the build id as the
 	 filename.  */
       write_dwarf_index (per_bfd, m_dir.c_str (),
-			 build_id_str.c_str (), dwz_build_id_ptr,
+			 ctx.build_id_str.c_str (), dwz_build_id_ptr,
 			 dw_index_kind::GDB_INDEX);
     }
   catch (const gdb_exception_error &except)
diff --git a/gdb/dwarf2/index-cache.h b/gdb/dwarf2/index-cache.h
index 7ea972d5c7f..cfa45435fbd 100644
--- a/gdb/dwarf2/index-cache.h
+++ b/gdb/dwarf2/index-cache.h
@@ -42,17 +42,24 @@ struct index_cache_store_context
 {
   friend class index_cache;
 
-  explicit index_cache_store_context (const index_cache &ic);
+  index_cache_store_context (const index_cache &ic, dwarf2_per_bfd *per_bfd);
 
 private:
   /* Captured value of enabled ().  */
   bool m_enabled;
+
+  /* Captured value of build id.  */
+  std::string build_id_str;
+
+  /* Captured value of dwz build id.  */
+  gdb::optional<std::string> dwz_build_id_str;
 };
 
 /* Class to manage the access to the DWARF index cache.  */
 
 class index_cache
 {
+  friend struct index_cache_store_context;
 public:
   /* Change the directory used to save/load index files.  */
   void set_directory (std::string dir);
-- 
2.35.3


^ permalink raw reply	[flat|nested] 9+ messages in thread
* [PATCH v3 3/5] [gdb/symtab] Fix race on dwarf2_per_cu_data::{queued,is_debug_type}
  2023-08-04  0:03 [PATCH v3 0/5] [gdb/symtab] Fix data-races in gdb.base/index-cache.exp Tom de Vries
  2023-08-04  0:03 ` [PATCH v3 1/5] [gdb/symtab] Fix data race on index_cache::m_enabled Tom de Vries
  2023-08-04  0:03 ` [PATCH v3 2/5] [gdb/symtab] Fix data race on bfd::{cacheable,format} Tom de Vries
@ 2023-08-04  0:03 ` Tom de Vries
  2023-08-04  0:03 ` [PATCH v3 4/5] [gdb/symtab] Fix data race on dwarf2_per_cu_data::{m_header_read_in,is_debug_type} Tom de Vries
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Tom de Vries @ 2023-08-04  0:03 UTC (permalink / raw)
  To: gdb-patches

With gdb build with -fsanitize=thread and test-case gdb.base/index-cache.exp I
run into:
...
(gdb) file build/gdb/testsuite/outputs/gdb.base/index-cache/index-cache
Reading symbols from build/gdb/testsuite/outputs/gdb.base/index-cache/index-cache...
==================
WARNING: ThreadSanitizer: data race (pid=24296)
  Write of size 1 at 0x7b200000420d by main thread:
    #0 queue_comp_unit gdb/dwarf2/read.c:5564 (gdb+0x8939ce)
    #1 dw2_do_instantiate_symtab gdb/dwarf2/read.c:1754 (gdb+0x885b96)
    #2 dw2_instantiate_symtab gdb/dwarf2/read.c:1792 (gdb+0x885d86)
    #3 dw2_expand_symtabs_matching_one(dwarf2_per_cu_data*, dwarf2_per_objfile*, gdb::function_view<bool (char const*, bool)>, gdb::function_view<bool (compunit_symtab*)>) gdb/dwarf2/read.c:3042 (gdb+0x88ac77)
    #4 cooked_index_functions::expand_symtabs_matching(objfile*, gdb::function_view<bool (char const*, bool)>, lookup_name_info const*, gdb::function_view<bool (char const*)>, gdb::function_view<bool (compunit_symtab*)>, enum_flags<block_search_flag_values>, domain_enum, search_domain) gdb/dwarf2/read.c:16915 (gdb+0x8c1c8a)
    #5 objfile::lookup_symbol(block_enum, char const*, domain_enum) gdb/symfile-debug.c:288 (gdb+0xf389a1)
    #6 lookup_symbol_via_quick_fns gdb/symtab.c:2385 (gdb+0xf66403)
    #7 lookup_symbol_in_objfile gdb/symtab.c:2516 (gdb+0xf66a67)
    #8 operator() gdb/symtab.c:2562 (gdb+0xf66bbe)
    #9 operator() gdb/../gdbsupport/function-view.h:305 (gdb+0xf76ffd)
    #10 _FUN gdb/../gdbsupport/function-view.h:299 (gdb+0xf77054)
    #11 gdb::function_view<bool (objfile*)>::operator()(objfile*) const gdb/../gdbsupport/function-view.h:289 (gdb+0xc3f5e3)
    #12 svr4_iterate_over_objfiles_in_search_order gdb/solib-svr4.c:3455 (gdb+0xeca793)
    #13 gdbarch_iterate_over_objfiles_in_search_order(gdbarch*, gdb::function_view<bool (objfile*)>, objfile*) gdb/gdbarch.c:5041 (gdb+0x537cad)
    #14 lookup_global_or_static_symbol gdb/symtab.c:2559 (gdb+0xf66e47)
    #15 lookup_global_symbol(char const*, block const*, domain_enum) gdb/symtab.c:2615 (gdb+0xf670cc)
    #16 language_defn::lookup_symbol_nonlocal(char const*, block const*, domain_enum) const gdb/symtab.c:2447 (gdb+0xf666ba)
    #17 lookup_symbol_aux gdb/symtab.c:2123 (gdb+0xf655ff)
    #18 lookup_symbol_in_language(char const*, block const*, domain_enum, language, field_of_this_result*) gdb/symtab.c:1931 (gdb+0xf646f7)
    #19 set_initial_language() gdb/symfile.c:1708 (gdb+0xf429c0)
    #20 symbol_file_add_main_1 gdb/symfile.c:1212 (gdb+0xf40f54)
    #21 symbol_file_command(char const*, int) gdb/symfile.c:1681 (gdb+0xf428fb)
    #22 file_command gdb/exec.c:554 (gdb+0x94f875)
    #23 do_simple_func gdb/cli/cli-decode.c:95 (gdb+0x6d9528)
    #24 cmd_func(cmd_list_element*, char const*, int) gdb/cli/cli-decode.c:2735 (gdb+0x6e0f69)
    #25 execute_command(char const*, int) gdb/top.c:575 (gdb+0xff3166)
    #26 command_handler(char const*) gdb/event-top.c:552 (gdb+0x94af08)
    #27 command_line_handler(std::unique_ptr<char, gdb::xfree_deleter<char> >&&) gdb/event-top.c:788 (gdb+0x94b5c5)
    #28 tui_command_line_handler gdb/tui/tui-interp.c:104 (gdb+0x10348c6)
    #29 gdb_rl_callback_handler gdb/event-top.c:259 (gdb+0x94a4ad)
    #30 rl_callback_read_char readline/readline/callback.c:290 (gdb+0x11bdf87)
    #31 gdb_rl_callback_read_char_wrapper_noexcept gdb/event-top.c:195 (gdb+0x94a2ac)
    #32 gdb_rl_callback_read_char_wrapper gdb/event-top.c:234 (gdb+0x94a36d)
    #33 stdin_event_handler gdb/ui.c:155 (gdb+0x1074b6a)
    #34 handle_file_event gdbsupport/event-loop.cc:573 (gdb+0x1d9502c)
    #35 gdb_wait_for_event gdbsupport/event-loop.cc:694 (gdb+0x1d95764)
    #36 gdb_do_one_event(int) gdbsupport/event-loop.cc:264 (gdb+0x1d93b50)
    #37 start_event_loop gdb/main.c:412 (gdb+0xb5a49e)
    #38 captured_command_loop gdb/main.c:476 (gdb+0xb5a68d)
    #39 captured_main gdb/main.c:1320 (gdb+0xb5c80d)
    #40 gdb_main(captured_main_args*) gdb/main.c:1339 (gdb+0xb5c8bc)
    #41 main gdb/gdb.c:32 (gdb+0x416776)

  Previous read of size 1 at 0x7b200000420d by thread T12:
    #0 write_gdbindex gdb/dwarf2/index-write.c:1229 (gdb+0x8310c8)
    #1 write_dwarf_index(dwarf2_per_bfd*, char const*, char const*, char const*, dw_index_kind) gdb/dwarf2/index-write.c:1484 (gdb+0x83232f)
    #2 index_cache::store(dwarf2_per_bfd*, index_cache_store_context*) gdb/dwarf2/index-cache.c:177 (gdb+0x82d62b)
    #3 cooked_index::maybe_write_index(dwarf2_per_bfd*) gdb/dwarf2/cooked-index.c:640 (gdb+0x7f1bf7)
    #4 operator() gdb/dwarf2/cooked-index.c:470 (gdb+0x7f0f40)
    #5 _M_invoke /usr/include/c++/7/bits/std_function.h:316 (gdb+0x7f2909)
    #6 std::function<void ()>::operator()() const /usr/include/c++/7/bits/std_function.h:706 (gdb+0x700952)
    #7 void std::__invoke_impl<void, std::function<void ()>&>(std::__invoke_other, std::function<void ()>&) /usr/include/c++/7/bits/invoke.h:60 (gdb+0x7381a0)
    #8 std::__invoke_result<std::function<void ()>&>::type std::__invoke<std::function<void ()>&>(std::function<void ()>&) /usr/include/c++/7/bits/invoke.h:95 (gdb+0x737e91)
    #9 std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run()::{lambda()#1}::operator()() const /usr/include/c++/7/future:1421 (gdb+0x737b59)
    #10 std::__future_base::_Task_setter<std::unique_ptr<std::__future_base::_Result<void>, std::__future_base::_Result_base::_Deleter>, std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run()::{lambda()#1}, void>::operator()() const /usr/include/c++/7/future:1362 (gdb+0x738660)
    #11 std::_Function_handler<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> (), std::__future_base::_Task_setter<std::unique_ptr<std::__future_base::_Result<void>, std::__future_base::_Result_base::_Deleter>, std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run()::{lambda()#1}, void> >::_M_invoke(std::_Any_data const&) /usr/include/c++/7/bits/std_function.h:302 (gdb+0x73825c)
    #12 std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>::operator()() const /usr/include/c++/7/bits/std_function.h:706 (gdb+0x733623)
    #13 std::__future_base::_State_baseV2::_M_do_set(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*) /usr/include/c++/7/future:561 (gdb+0x732bdf)
    #14 void std::__invoke_impl<void, void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::__invoke_memfun_deref, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&) /usr/include/c++/7/bits/invoke.h:73 (gdb+0x734c4f)
    #15 std::__invoke_result<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>::type std::__invoke<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&) /usr/include/c++/7/bits/invoke.h:95 (gdb+0x733bc5)
    #16 std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&)::{lambda()#1}::operator()() const /usr/include/c++/7/mutex:672 (gdb+0x73300d)
    #17 std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&)::{lambda()#2}::operator()() const /usr/include/c++/7/mutex:677 (gdb+0x7330b2)
    #18 std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&)::{lambda()#2}::_FUN() /usr/include/c++/7/mutex:677 (gdb+0x7330f2)
    #19 pthread_once <null> (libtsan.so.0+0x4457c)
    #20 __gthread_once /usr/include/c++/7/x86_64-suse-linux/bits/gthr-default.h:699 (gdb+0x72f5dd)
    #21 void std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&) /usr/include/c++/7/mutex:684 (gdb+0x733224)
    #22 std::__future_base::_State_baseV2::_M_set_result(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>, bool) /usr/include/c++/7/future:401 (gdb+0x732852)
    #23 std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run() /usr/include/c++/7/future:1423 (gdb+0x737bef)
    #24 std::packaged_task<void ()>::operator()() /usr/include/c++/7/future:1556 (gdb+0x1dac6da)
    #25 gdb::thread_pool::thread_function() gdbsupport/thread-pool.cc:242 (gdb+0x1dabffc)
    #26 void std::__invoke_impl<void, void (gdb::thread_pool::*)(), gdb::thread_pool*>(std::__invoke_memfun_deref, void (gdb::thread_pool::*&&)(), gdb::thread_pool*&&) /usr/include/c++/7/bits/invoke.h:73 (gdb+0x1dad0ab)
    #27 std::__invoke_result<void (gdb::thread_pool::*)(), gdb::thread_pool*>::type std::__invoke<void (gdb::thread_pool::*)(), gdb::thread_pool*>(void (gdb::thread_pool::*&&)(), gdb::thread_pool*&&) /usr/include/c++/7/bits/invoke.h:95 (gdb+0x1dac4dc)
    #28 decltype (__invoke((_S_declval<0ul>)(), (_S_declval<1ul>)())) std::thread::_Invoker<std::tuple<void (gdb::thread_pool::*)(), gdb::thread_pool*> >::_M_invoke<0ul, 1ul>(std::_Index_tuple<0ul, 1ul>) /usr/include/c++/7/thread:234 (gdb+0x1daf80e)
    #29 std::thread::_Invoker<std::tuple<void (gdb::thread_pool::*)(), gdb::thread_pool*> >::operator()() /usr/include/c++/7/thread:243 (gdb+0x1daf799)
    #30 std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (gdb::thread_pool::*)(), gdb::thread_pool*> > >::_M_run() /usr/include/c++/7/thread:186 (gdb+0x1daf74e)
    #31 <null> <null> (libstdc++.so.6+0xdcac2)
 ...
SUMMARY: ThreadSanitizer: data race gdb/dwarf2/read.c:5564 in queue_comp_unit
...

The race happens when issuing the "file $exec" command.

The race is between:
- a worker thread writing the index cache, and in the process reading
  dwarf2_per_cu_data::is_debug_type, and
- the main thread expanding the CU containing main, and in the process setting
  dwarf2_per_cu_data::queued.

The two bitfields dwarf2_per_cu_data::queue and
dwarf2_per_cu_data::is_debug_type share the same bitfield container.

Fix this by making dwarf2_per_cu_data::queued a packed<bool, 1>.

Tested on x86_64-linux.

Approved-By: Tom Tromey <tom@tromey.com>

PR symtab/30392
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30392
---
 gdb/dwarf2/read.h | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
index a99299cbc6d..5fa276a7c1b 100644
--- a/gdb/dwarf2/read.h
+++ b/gdb/dwarf2/read.h
@@ -100,14 +100,14 @@ typedef std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter>
 struct dwarf2_per_cu_data
 {
   dwarf2_per_cu_data ()
-    : queued (false),
-      is_debug_types (false),
+    : is_debug_types (false),
       is_dwz (false),
       reading_dwo_directly (false),
       tu_read (false),
       m_header_read_in (false),
       mark (false),
       files_read (false),
+      queued (false),
       scanned (false)
   {
   }
@@ -126,10 +126,6 @@ struct dwarf2_per_cu_data
   unsigned char m_dwarf_version = 0;
 
 public:
-  /* Flag indicating this compilation unit will be read in before
-     any of the current compilation units are processed.  */
-  unsigned int queued : 1;
-
   /* Non-zero if this CU is from .debug_types.
      Struct dwarf2_per_cu_data is contained in struct signatured_type iff
      this is non-zero.  */
@@ -176,6 +172,10 @@ struct dwarf2_per_cu_data
      .debug_aranges), then this flag is set.  */
   packed<bool, 1> addresses_seen = false;
 
+  /* Flag indicating this compilation unit will be read in before
+     any of the current compilation units are processed.  */
+  packed<bool, 1> queued;
+
 private:
   /* The unit type of this CU.  */
   std::atomic<packed<dwarf_unit_type, 1>> m_unit_type {(dwarf_unit_type)0};
-- 
2.35.3


^ permalink raw reply	[flat|nested] 9+ messages in thread
* [PATCH v3 4/5] [gdb/symtab] Fix data race on dwarf2_per_cu_data::{m_header_read_in,is_debug_type}
  2023-08-04  0:03 [PATCH v3 0/5] [gdb/symtab] Fix data-races in gdb.base/index-cache.exp Tom de Vries
                   ` (2 preceding siblings ...)
  2023-08-04  0:03 ` [PATCH v3 3/5] [gdb/symtab] Fix race on dwarf2_per_cu_data::{queued,is_debug_type} Tom de Vries
@ 2023-08-04  0:03 ` Tom de Vries
  2023-08-04  0:03 ` [PATCH v3 5/5] [gdb/testsuite] Extend gdb.base/index-cache.exp Tom de Vries
  2023-08-04 17:03 ` [PATCH v3 0/5] [gdb/symtab] Fix data-races in gdb.base/index-cache.exp Tom Tromey
  5 siblings, 0 replies; 9+ messages in thread
From: Tom de Vries @ 2023-08-04  0:03 UTC (permalink / raw)
  To: gdb-patches

With gdb build with -fsanitize=thread and test-case gdb.base/index-cache.exp
and target board debug-types, I run into:
...
(gdb) file build/gdb/testsuite/outputs/gdb.base/index-cache/index-cache
Reading symbols from build/gdb/testsuite/outputs/gdb.base/index-cache/index-cache...
==================
WARNING: ThreadSanitizer: data race (pid=9654)
  Write of size 1 at 0x7b200000420d by main thread:
    #0 dwarf2_per_cu_data::get_header() const gdb/dwarf2/read.c:21513 (gdb+0x8d1eee)
    #1 dwarf2_per_cu_data::addr_size() const gdb/dwarf2/read.c:21524 (gdb+0x8d1f4e)
    #2 dwarf2_cu::addr_type() const gdb/dwarf2/cu.c:112 (gdb+0x806327)
    #3 set_die_type gdb/dwarf2/read.c:21932 (gdb+0x8d3870)
    #4 read_base_type gdb/dwarf2/read.c:15448 (gdb+0x8bcacb)
    #5 read_type_die_1 gdb/dwarf2/read.c:19832 (gdb+0x8cc0a5)
    #6 read_type_die gdb/dwarf2/read.c:19767 (gdb+0x8cbe6d)
    #7 lookup_die_type gdb/dwarf2/read.c:19739 (gdb+0x8cbdc7)
    #8 die_type gdb/dwarf2/read.c:19593 (gdb+0x8cb68a)
    #9 read_subroutine_type gdb/dwarf2/read.c:14648 (gdb+0x8b998e)
    #10 read_type_die_1 gdb/dwarf2/read.c:19792 (gdb+0x8cbf2f)
    #11 read_type_die gdb/dwarf2/read.c:19767 (gdb+0x8cbe6d)
    #12 read_func_scope gdb/dwarf2/read.c:10154 (gdb+0x8a4f36)
    #13 process_die gdb/dwarf2/read.c:6667 (gdb+0x898daa)
    #14 read_file_scope gdb/dwarf2/read.c:7682 (gdb+0x89bad8)
    #15 process_die gdb/dwarf2/read.c:6654 (gdb+0x898ced)
    #16 process_full_comp_unit gdb/dwarf2/read.c:6418 (gdb+0x8981de)
    #17 process_queue gdb/dwarf2/read.c:5690 (gdb+0x894433)
    #18 dw2_do_instantiate_symtab gdb/dwarf2/read.c:1770 (gdb+0x88623a)
    #19 dw2_instantiate_symtab gdb/dwarf2/read.c:1792 (gdb+0x886300)
    #20 dw2_expand_symtabs_matching_one(dwarf2_per_cu_data*, dwarf2_per_objfile*, gdb::function_view<bool (char const*, bool)>, gdb::function_view<bool (compunit_symtab*)>) gdb/dwarf2/read.c:3042 (gdb+0x88b1f1)
    #21 cooked_index_functions::expand_symtabs_matching(objfile*, gdb::function_view<bool (char const*, bool)>, lookup_name_info const*, gdb::function_view<bool (char const*)>, gdb::function_view<bool (compunit_symtab*)>, enum_flags<block_search_flag_values>, domain_enum, search_domain) gdb/dwarf2/read.c:16917 (gdb+0x8c228e)
    #22 objfile::lookup_symbol(block_enum, char const*, domain_enum) gdb/symfile-debug.c:288 (gdb+0xf39055)
    #23 lookup_symbol_via_quick_fns gdb/symtab.c:2385 (gdb+0xf66ab7)
    #24 lookup_symbol_in_objfile gdb/symtab.c:2516 (gdb+0xf6711b)
    #25 operator() gdb/symtab.c:2562 (gdb+0xf67272)
    #26 operator() gdb/../gdbsupport/function-view.h:305 (gdb+0xf776b1)
    #27 _FUN gdb/../gdbsupport/function-view.h:299 (gdb+0xf77708)
    #28 gdb::function_view<bool (objfile*)>::operator()(objfile*) const gdb/../gdbsupport/function-view.h:289 (gdb+0xc3fc97)
    #29 svr4_iterate_over_objfiles_in_search_order gdb/solib-svr4.c:3455 (gdb+0xecae47)
    #30 gdbarch_iterate_over_objfiles_in_search_order(gdbarch*, gdb::function_view<bool (objfile*)>, objfile*) gdb/gdbarch.c:5041 (gdb+0x537cad)
    #31 lookup_global_or_static_symbol gdb/symtab.c:2559 (gdb+0xf674fb)
    #32 lookup_global_symbol(char const*, block const*, domain_enum) gdb/symtab.c:2615 (gdb+0xf67780)
    #33 language_defn::lookup_symbol_nonlocal(char const*, block const*, domain_enum) const gdb/symtab.c:2447 (gdb+0xf66d6e)
    #34 lookup_symbol_aux gdb/symtab.c:2123 (gdb+0xf65cb3)
    #35 lookup_symbol_in_language(char const*, block const*, domain_enum, language, field_of_this_result*) gdb/symtab.c:1931 (gdb+0xf64dab)
    #36 set_initial_language() gdb/symfile.c:1708 (gdb+0xf43074)
    #37 symbol_file_add_main_1 gdb/symfile.c:1212 (gdb+0xf41608)
    #38 symbol_file_command(char const*, int) gdb/symfile.c:1681 (gdb+0xf42faf)
    #39 file_command gdb/exec.c:554 (gdb+0x94ff29)
    #40 do_simple_func gdb/cli/cli-decode.c:95 (gdb+0x6d9528)
    #41 cmd_func(cmd_list_element*, char const*, int) gdb/cli/cli-decode.c:2735 (gdb+0x6e0f69)
    #42 execute_command(char const*, int) gdb/top.c:575 (gdb+0xff379c)
    #43 command_handler(char const*) gdb/event-top.c:552 (gdb+0x94b5bc)
    #44 command_line_handler(std::unique_ptr<char, gdb::xfree_deleter<char> >&&) gdb/event-top.c:788 (gdb+0x94bc79)
    #45 tui_command_line_handler gdb/tui/tui-interp.c:104 (gdb+0x1034efc)
    #46 gdb_rl_callback_handler gdb/event-top.c:259 (gdb+0x94ab61)
    #47 rl_callback_read_char readline/readline/callback.c:290 (gdb+0x11be4ef)
    #48 gdb_rl_callback_read_char_wrapper_noexcept gdb/event-top.c:195 (gdb+0x94a960)
    #49 gdb_rl_callback_read_char_wrapper gdb/event-top.c:234 (gdb+0x94aa21)
    #50 stdin_event_handler gdb/ui.c:155 (gdb+0x10751a0)
    #51 handle_file_event gdbsupport/event-loop.cc:573 (gdb+0x1d95bac)
    #52 gdb_wait_for_event gdbsupport/event-loop.cc:694 (gdb+0x1d962e4)
    #53 gdb_do_one_event(int) gdbsupport/event-loop.cc:264 (gdb+0x1d946d0)
    #54 start_event_loop gdb/main.c:412 (gdb+0xb5ab52)
    #55 captured_command_loop gdb/main.c:476 (gdb+0xb5ad41)
    #56 captured_main gdb/main.c:1320 (gdb+0xb5cec1)
    #57 gdb_main(captured_main_args*) gdb/main.c:1339 (gdb+0xb5cf70)
    #58 main gdb/gdb.c:32 (gdb+0x416776)

  Previous read of size 1 at 0x7b200000420d by thread T11:
    #0 write_gdbindex gdb/dwarf2/index-write.c:1229 (gdb+0x831630)
    #1 write_dwarf_index(dwarf2_per_bfd*, char const*, char const*, char const*, dw_index_kind) gdb/dwarf2/index-write.c:1484 (gdb+0x832897)
    #2 index_cache::store(dwarf2_per_bfd*, index_cache_store_context const&) gdb/dwarf2/index-cache.c:173 (gdb+0x82db8d)
    #3 cooked_index::maybe_write_index(dwarf2_per_bfd*, index_cache_store_context const&) gdb/dwarf2/cooked-index.c:645 (gdb+0x7f1d49)
    #4 operator() gdb/dwarf2/cooked-index.c:474 (gdb+0x7f0f31)
    #5 _M_invoke /usr/include/c++/7/bits/std_function.h:316 (gdb+0x7f2a13)
    #6 std::function<void ()>::operator()() const /usr/include/c++/7/bits/std_function.h:706 (gdb+0x700952)
    #7 void std::__invoke_impl<void, std::function<void ()>&>(std::__invoke_other, std::function<void ()>&) /usr/include/c++/7/bits/invoke.h:60 (gdb+0x7381a0)
    #8 std::__invoke_result<std::function<void ()>&>::type std::__invoke<std::function<void ()>&>(std::function<void ()>&) /usr/include/c++/7/bits/invoke.h:95 (gdb+0x737e91)
    #9 std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run()::{lambda()#1}::operator()() const /usr/include/c++/7/future:1421 (gdb+0x737b59)
    #10 std::__future_base::_Task_setter<std::unique_ptr<std::__future_base::_Result<void>, std::__future_base::_Result_base::_Deleter>, std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run()::{lambda()#1}, void>::operator()() const /usr/include/c++/7/future:1362 (gdb+0x738660)
    #11 std::_Function_handler<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> (), std::__future_base::_Task_setter<std::unique_ptr<std::__future_base::_Result<void>, std::__future_base::_Result_base::_Deleter>, std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run()::{lambda()#1}, void> >::_M_invoke(std::_Any_data const&) /usr/include/c++/7/bits/std_function.h:302 (gdb+0x73825c)
    #12 std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>::operator()() const /usr/include/c++/7/bits/std_function.h:706 (gdb+0x733623)
    #13 std::__future_base::_State_baseV2::_M_do_set(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*) /usr/include/c++/7/future:561 (gdb+0x732bdf)
    #14 void std::__invoke_impl<void, void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::__invoke_memfun_deref, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&) /usr/include/c++/7/bits/invoke.h:73 (gdb+0x734c4f)
    #15 std::__invoke_result<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>::type std::__invoke<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&) /usr/include/c++/7/bits/invoke.h:95 (gdb+0x733bc5)
    #16 std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&)::{lambda()#1}::operator()() const /usr/include/c++/7/mutex:672 (gdb+0x73300d)
    #17 std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&)::{lambda()#2}::operator()() const /usr/include/c++/7/mutex:677 (gdb+0x7330b2)
    #18 std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&)::{lambda()#2}::_FUN() /usr/include/c++/7/mutex:677 (gdb+0x7330f2)
    #19 pthread_once <null> (libtsan.so.0+0x4457c)
    #20 __gthread_once /usr/include/c++/7/x86_64-suse-linux/bits/gthr-default.h:699 (gdb+0x72f5dd)
    #21 void std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&) /usr/include/c++/7/mutex:684 (gdb+0x733224)
    #22 std::__future_base::_State_baseV2::_M_set_result(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>, bool) /usr/include/c++/7/future:401 (gdb+0x732852)
    #23 std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run() /usr/include/c++/7/future:1423 (gdb+0x737bef)
    #24 std::packaged_task<void ()>::operator()() /usr/include/c++/7/future:1556 (gdb+0x1dad25a)
    #25 gdb::thread_pool::thread_function() gdbsupport/thread-pool.cc:242 (gdb+0x1dacb7c)
    #26 void std::__invoke_impl<void, void (gdb::thread_pool::*)(), gdb::thread_pool*>(std::__invoke_memfun_deref, void (gdb::thread_pool::*&&)(), gdb::thread_pool*&&) /usr/include/c++/7/bits/invoke.h:73 (gdb+0x1dadc2b)
    #27 std::__invoke_result<void (gdb::thread_pool::*)(), gdb::thread_pool*>::type std::__invoke<void (gdb::thread_pool::*)(), gdb::thread_pool*>(void (gdb::thread_pool::*&&)(), gdb::thread_pool*&&) /usr/include/c++/7/bits/invoke.h:95 (gdb+0x1dad05c)
    #28 decltype (__invoke((_S_declval<0ul>)(), (_S_declval<1ul>)())) std::thread::_Invoker<std::tuple<void (gdb::thread_pool::*)(), gdb::thread_pool*> >::_M_invoke<0ul, 1ul>(std::_Index_tuple<0ul, 1ul>) /usr/include/c++/7/thread:234 (gdb+0x1db038e)
    #29 std::thread::_Invoker<std::tuple<void (gdb::thread_pool::*)(), gdb::thread_pool*> >::operator()() /usr/include/c++/7/thread:243 (gdb+0x1db0319)
    #30 std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (gdb::thread_pool::*)(), gdb::thread_pool*> > >::_M_run() /usr/include/c++/7/thread:186 (gdb+0x1db02ce)
    #31 <null> <null> (libstdc++.so.6+0xdcac2)
  ...
SUMMARY: ThreadSanitizer: data race gdb/dwarf2/read.c:21513 in dwarf2_per_cu_data::get_header() const
...

The race happens when issuing the "file $exec" command.

The race is between:
- a worker thread writing the index cache, and in the process reading
   dwarf2_per_cu_data::is_debug_type, and
- the main thread writing to dwarf2_per_cu_data::m_header_read_in.

The two bitfields dwarf2_per_cu_data::m_header_read_in and
dwarf2_per_cu_data::is_debug_type share the same bitfield container.

Fix this by making dwarf2_per_cu_data::m_header_read_in a packed<bool, 1>.

Tested on x86_64-linux.

Approved-By: Tom Tromey <tom@tromey.com>

PR symtab/30392
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30392
---
 gdb/dwarf2/read.h | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
index 5fa276a7c1b..5f7d2dcd895 100644
--- a/gdb/dwarf2/read.h
+++ b/gdb/dwarf2/read.h
@@ -104,10 +104,10 @@ struct dwarf2_per_cu_data
       is_dwz (false),
       reading_dwo_directly (false),
       tu_read (false),
-      m_header_read_in (false),
       mark (false),
       files_read (false),
       queued (false),
+      m_header_read_in (false),
       scanned (false)
   {
   }
@@ -150,12 +150,6 @@ struct dwarf2_per_cu_data
      This flag is only valid if is_debug_types is true.  */
   unsigned int tu_read : 1;
 
-  /* True if HEADER has been read in.
-
-     Don't access this field directly.  It should be private, but we can't make
-     it private at the moment.  */
-  mutable bool m_header_read_in : 1;
-
   /* A temporary mark bit used when iterating over all CUs in
      expand_symtabs_matching.  */
   unsigned int mark : 1;
@@ -176,6 +170,12 @@ struct dwarf2_per_cu_data
      any of the current compilation units are processed.  */
   packed<bool, 1> queued;
 
+  /* True if HEADER has been read in.
+
+     Don't access this field directly.  It should be private, but we can't make
+     it private at the moment.  */
+  mutable packed<bool, 1> m_header_read_in;
+
 private:
   /* The unit type of this CU.  */
   std::atomic<packed<dwarf_unit_type, 1>> m_unit_type {(dwarf_unit_type)0};
-- 
2.35.3


^ permalink raw reply	[flat|nested] 9+ messages in thread
* [PATCH v3 5/5] [gdb/testsuite] Extend gdb.base/index-cache.exp
  2023-08-04  0:03 [PATCH v3 0/5] [gdb/symtab] Fix data-races in gdb.base/index-cache.exp Tom de Vries
                   ` (3 preceding siblings ...)
  2023-08-04  0:03 ` [PATCH v3 4/5] [gdb/symtab] Fix data race on dwarf2_per_cu_data::{m_header_read_in,is_debug_type} Tom de Vries
@ 2023-08-04  0:03 ` Tom de Vries
  2023-08-04 17:03 ` [PATCH v3 0/5] [gdb/symtab] Fix data-races in gdb.base/index-cache.exp Tom Tromey
  5 siblings, 0 replies; 9+ messages in thread
From: Tom de Vries @ 2023-08-04  0:03 UTC (permalink / raw)
  To: gdb-patches

The test-case gdb.base/index-cache.exp uses only one source file, which
contains main.

While doing "file $exec", in set_initial_language a symbol lookup of "main" is
done, causing the symtab containing main to be expanded.

Handling of main is special, and a future optimization may skip the lookup and
expansion.

Reliably exercise:
- the lookup of main, expanding the symtab containing main, by doing
  "ptype main", and
- the lookup of another symbol, expanding a symtab not containing main, by:
  - adding another source file containing function foo, and
  - doing "ptype foo".

This triggered a segfault with target board native-extended-gdbserver, filed
as PR symtab/30712, but that seems to be fixed by a previous commit in this
series.

Tested on x86_64-linux.

Approved-By: Tom Tromey <tom@tromey.com>

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30712
---
 gdb/testsuite/gdb.base/index-cache-2.c | 24 ++++++++++++++++++++++++
 gdb/testsuite/gdb.base/index-cache.c   |  6 ++++--
 gdb/testsuite/gdb.base/index-cache.exp | 22 ++++++++++++++++++++--
 3 files changed, 48 insertions(+), 4 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/index-cache-2.c

diff --git a/gdb/testsuite/gdb.base/index-cache-2.c b/gdb/testsuite/gdb.base/index-cache-2.c
new file mode 100644
index 00000000000..d0c10780499
--- /dev/null
+++ b/gdb/testsuite/gdb.base/index-cache-2.c
@@ -0,0 +1,24 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2023 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+extern int foo (void);
+
+int
+foo (void)
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/index-cache.c b/gdb/testsuite/gdb.base/index-cache.c
index 16521df96f5..fecba981b91 100644
--- a/gdb/testsuite/gdb.base/index-cache.c
+++ b/gdb/testsuite/gdb.base/index-cache.c
@@ -15,9 +15,11 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
+extern int foo (void);
+
 int
-main ()
+main (void)
 {
-  return 0;
+  return foo ();
 }
 
diff --git a/gdb/testsuite/gdb.base/index-cache.exp b/gdb/testsuite/gdb.base/index-cache.exp
index 9f2f53053cb..c26c4f94e65 100644
--- a/gdb/testsuite/gdb.base/index-cache.exp
+++ b/gdb/testsuite/gdb.base/index-cache.exp
@@ -16,9 +16,9 @@
 # This test checks that the index-cache feature generates the expected files at
 # the expected location.
 
-standard_testfile
+standard_testfile .c -2.c
 
-if { [build_executable "failed to prepare" $testfile $srcfile \
+if { [build_executable "failed to prepare" $testfile [list $srcfile $srcfile2] \
 	  {debug ldflags=-Wl,--build-id}] } {
     return
 }
@@ -147,6 +147,12 @@ proc_with_prefix test_cache_disabled { cache_dir test_prefix } {
 	    gdb_assert "$nfiles_created == 0" "no files were created"
 
 	    check_cache_stats 0 0
+
+	    # Trigger expansion of symtab containing main, if not already done.
+	    gdb_test "ptype main" "^type = int \\(void\\)"
+
+	    # Trigger expansion of symtab not containing main.
+	    gdb_test "ptype foo" "^type = int \\(void\\)"
 	}
     }
 }
@@ -192,6 +198,12 @@ proc_with_prefix test_cache_enabled_miss { cache_dir } {
 	} else {
 	    check_cache_stats 0 0
 	}
+
+	# Trigger expansion of symtab containing main, if not already done.
+	gdb_test "ptype main" "^type = int \\(void\\)"
+
+	# Trigger expansion of symtab not containing main.
+	gdb_test "ptype foo" "^type = int \\(void\\)"
     }
 }
 
@@ -221,6 +233,12 @@ proc_with_prefix test_cache_enabled_hit { cache_dir } {
 	} else {
 	    check_cache_stats 0 0
 	}
+
+	# Trigger expansion of symtab containing main, if not already done.
+	gdb_test "ptype main" "^type = int \\(void\\)"
+
+	# Trigger expansion of symtab not containing main.
+	gdb_test "ptype foo" "^type = int \\(void\\)"
     }
 }
 
-- 
2.35.3


^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH v3 0/5] [gdb/symtab] Fix data-races in gdb.base/index-cache.exp
  2023-08-04  0:03 [PATCH v3 0/5] [gdb/symtab] Fix data-races in gdb.base/index-cache.exp Tom de Vries
                   ` (4 preceding siblings ...)
  2023-08-04  0:03 ` [PATCH v3 5/5] [gdb/testsuite] Extend gdb.base/index-cache.exp Tom de Vries
@ 2023-08-04 17:03 ` Tom Tromey
  5 siblings, 0 replies; 9+ messages in thread
From: Tom Tromey @ 2023-08-04 17:03 UTC (permalink / raw)
  To: Tom de Vries via Gdb-patches; +Cc: Tom de Vries

>>>>> "Tom" == Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> writes:
Tom> Tested on x86_64-linux, with and without -fsanitize=thread.

Tom> PR symtab/30392
Tom> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30392

Thank you.  This looks good to me.
Approved-By: Tom Tromey <tom@tromey.com>

Tom

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/5] [gdb/symtab] Fix data race on index_cache::m_enabled
  2023-08-04  0:03 ` [PATCH v3 1/5] [gdb/symtab] Fix data race on index_cache::m_enabled Tom de Vries
@ 2023-08-06 21:01   ` Tom Tromey
  2023-08-07  0:01     ` Tom de Vries
  0 siblings, 1 reply; 9+ messages in thread
From: Tom Tromey @ 2023-08-06 21:01 UTC (permalink / raw)
  To: Tom de Vries via Gdb-patches; +Cc: Tom de Vries

>>>>> "Tom" == Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> writes:

Tom> -    = gdb::thread_pool::g_thread_pool->post_task ([this, per_bfd] ()
Tom> +    = gdb::thread_pool::g_thread_pool->post_task ([this, per_bfd,
Tom> +						   ctx = std::move (ctx)] ()

I just noticed this.  Move captures are part of C++14, not C++11.

It doesn't look too expensive to just copy the index_cache_store_context
instead.

Tom

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/5] [gdb/symtab] Fix data race on index_cache::m_enabled
  2023-08-06 21:01   ` Tom Tromey
@ 2023-08-07  0:01     ` Tom de Vries
  0 siblings, 0 replies; 9+ messages in thread
From: Tom de Vries @ 2023-08-07  0:01 UTC (permalink / raw)
  To: Tom Tromey, Tom de Vries via Gdb-patches

On 8/6/23 23:01, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Tom> -    = gdb::thread_pool::g_thread_pool->post_task ([this, per_bfd] ()
> Tom> +    = gdb::thread_pool::g_thread_pool->post_task ([this, per_bfd,
> Tom> +						   ctx = std::move (ctx)] ()
> 
> I just noticed this.  Move captures are part of C++14, not C++11.
> 
> It doesn't look too expensive to just copy the index_cache_store_context
> instead.

Thanks for letting me know, I"ve submitted this patch ( 
https://sourceware.org/pipermail/gdb-patches/2023-August/201386.html ).

Thanks,
- Tom


^ permalink raw reply	[flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-08-07  0:02 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-04  0:03 [PATCH v3 0/5] [gdb/symtab] Fix data-races in gdb.base/index-cache.exp Tom de Vries
2023-08-04  0:03 ` [PATCH v3 1/5] [gdb/symtab] Fix data race on index_cache::m_enabled Tom de Vries
2023-08-06 21:01   ` Tom Tromey
2023-08-07  0:01     ` Tom de Vries
2023-08-04  0:03 ` [PATCH v3 2/5] [gdb/symtab] Fix data race on bfd::{cacheable,format} Tom de Vries
2023-08-04  0:03 ` [PATCH v3 3/5] [gdb/symtab] Fix race on dwarf2_per_cu_data::{queued,is_debug_type} Tom de Vries
2023-08-04  0:03 ` [PATCH v3 4/5] [gdb/symtab] Fix data race on dwarf2_per_cu_data::{m_header_read_in,is_debug_type} Tom de Vries
2023-08-04  0:03 ` [PATCH v3 5/5] [gdb/testsuite] Extend gdb.base/index-cache.exp Tom de Vries
2023-08-04 17:03 ` [PATCH v3 0/5] [gdb/symtab] Fix data-races in gdb.base/index-cache.exp 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).