[PATCH v2 4/6] [gdb/symtab] Fix data race on bfd_last_cache

[Tom de Vries <tdevries@suse.de>]

With gdb build with -fsanitize=thread and test-case gdb.base/index-cache.exp
run using "taskset -c 0", 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=5420)
  Write of size 8 at 0x0000032569e0 by main thread:
    #0 snip bfd/cache.c:155 (gdb+0x148b709)
    #1 bfd_cache_delete bfd/cache.c:176 (gdb+0x148b7a8)
    #2 bfd_cache_close bfd/cache.c:537 (gdb+0x148c406)
    #3 bfd_cache_close_all bfd/cache.c:561 (gdb+0x148c44a)
    #4 symbol_file_add_with_addrs gdb/symfile.c:1132 (gdb+0xf412ad)
    #5 symbol_file_add_from_bfd(gdb::ref_ptr<bfd, gdb_bfd_ref_policy> const&, char const*, enum_flags<symfile_add_flag>, std::vector<other_sections, std::allocator<other_sections> >*, enum_flags<objfile_flag>, objfile*) gdb/symfile.c:1167 (gdb+0xf4141d)
    #6 symbol_file_add(char const*, enum_flags<symfile_add_flag>, std::vector<other_sections, std::allocator<other_sections> >*, enum_flags<objfile_flag>) gdb/symfile.c:1180 (gdb+0xf4149d)
    #7 symbol_file_add_main_1 gdb/symfile.c:1203 (gdb+0xf415a9)
    #8 symbol_file_command(char const*, int) gdb/symfile.c:1681 (gdb+0xf42f97)
    #9 file_command gdb/exec.c:554 (gdb+0x94ff11)
    #10 do_simple_func gdb/cli/cli-decode.c:95 (gdb+0x6d9528)
    #11 cmd_func(cmd_list_element*, char const*, int) gdb/cli/cli-decode.c:2735 (gdb+0x6e0f69)
    #12 execute_command(char const*, int) gdb/top.c:575 (gdb+0xff3784)
    #13 command_handler(char const*) gdb/event-top.c:552 (gdb+0x94b5a4)
    #14 command_line_handler(std::unique_ptr<char, gdb::xfree_deleter<char> >&&) gdb/event-top.c:788 (gdb+0x94bc61)
    #15 tui_command_line_handler gdb/tui/tui-interp.c:104 (gdb+0x1034ee4)
    #16 gdb_rl_callback_handler gdb/event-top.c:259 (gdb+0x94ab49)
    #17 rl_callback_read_char readline/readline/callback.c:290 (gdb+0x11be4d7)
    #18 gdb_rl_callback_read_char_wrapper_noexcept gdb/event-top.c:195 (gdb+0x94a948)
    #19 gdb_rl_callback_read_char_wrapper gdb/event-top.c:234 (gdb+0x94aa09)
    #20 stdin_event_handler gdb/ui.c:155 (gdb+0x1075188)
    #21 handle_file_event gdbsupport/event-loop.cc:573 (gdb+0x1d95b94)
    #22 gdb_wait_for_event gdbsupport/event-loop.cc:694 (gdb+0x1d962cc)
    #23 gdb_do_one_event(int) gdbsupport/event-loop.cc:217 (gdb+0x1d94573)
    #24 start_event_loop gdb/main.c:412 (gdb+0xb5ab3a)
    #25 captured_command_loop gdb/main.c:476 (gdb+0xb5ad29)
    #26 captured_main gdb/main.c:1320 (gdb+0xb5cea9)
    #27 gdb_main(captured_main_args*) gdb/main.c:1339 (gdb+0xb5cf58)
    #28 main gdb/gdb.c:32 (gdb+0x416776)

  Previous read of size 8 at 0x0000032569e0 by thread T12:
    #0 cache_bseek bfd/cache.c:289 (gdb+0x148bbf7)
    #1 bfd_seek bfd/bfdio.c:459 (gdb+0x148a554)
    #2 _bfd_generic_get_section_contents bfd/libbfd.c:1069 (gdb+0x14980c7)
    #3 bfd_get_section_contents bfd/section.c:1606 (gdb+0x149d59f)
    #4 section_table_xfer_memory_partial(unsigned char*, unsigned char const*, unsigned long, unsigned long, unsigned long*, std::vector<target_section, std::allocator<target_section> > const&, gdb::function_view<bool (target_section const*)>) gdb/exec.c:840 (gdb+0x951171)
    #5 exec_target::xfer_partial(target_object, char const*, unsigned char*, unsigned char const*, unsigned long, unsigned long, unsigned long*) gdb/exec.c:894 (gdb+0x951372)
    #6 raw_memory_xfer_partial(target_ops*, unsigned char*, unsigned char const*, unsigned long, long, unsigned long*) gdb/target.c:1476 (gdb+0xfade7a)
    #7 memory_xfer_partial_1 gdb/target.c:1607 (gdb+0xfae39a)
    #8 memory_xfer_partial gdb/target.c:1636 (gdb+0xfae46c)
    #9 target_xfer_partial(target_ops*, target_object, char const*, unsigned char*, unsigned char const*, unsigned long, unsigned long, unsigned long*) gdb/target.c:1693 (gdb+0xfae7f4)
    #10 target_read_partial gdb/target.c:1947 (gdb+0xfaf3e4)
    #11 target_read(target_ops*, target_object, char const*, unsigned char*, unsigned long, long) gdb/target.c:1986 (gdb+0xfaf54a)
    #12 target_read_memory(unsigned long, unsigned char*, long) gdb/target.c:1782 (gdb+0xfaecc6)
    #13 gdb_bfd_scan_elf_dyntag(int, bfd*, unsigned long*, unsigned long*) gdb/solib.c:1638 (gdb+0xed98cf)
    #14 elf_locate_base gdb/solib-svr4.c:743 (gdb+0xec32e5)
    #15 svr4_iterate_over_objfiles_in_search_order gdb/solib-svr4.c:3430 (gdb+0xecad23)
    #16 gdbarch_iterate_over_objfiles_in_search_order(gdbarch*, gdb::function_view<bool (objfile*)>, objfile*) gdb/gdbarch.c:5041 (gdb+0x537cad)
    #17 find_main_name gdb/symtab.c:6270 (gdb+0xf74b6b)
    #18 main_name() gdb/symtab.c:6299 (gdb+0xf74bf9)
    #19 write_cooked_index gdb/dwarf2/index-write.c:1131 (gdb+0x831044)
    #20 write_gdbindex gdb/dwarf2/index-write.c:1256 (gdb+0x83189b)
    #21 write_dwarf_index(dwarf2_per_bfd*, char const*, char const*, char const*, dw_index_kind) gdb/dwarf2/index-write.c:1484 (gdb+0x83287f)
    #22 index_cache::store(dwarf2_per_bfd*, index_cache_store_context const&) gdb/dwarf2/index-cache.c:173 (gdb+0x82db35)
    #23 cooked_index::maybe_write_index(dwarf2_per_bfd*, index_cache_store_context const&) gdb/dwarf2/cooked-index.c:642 (gdb+0x7f1d31)
    #24 operator() gdb/dwarf2/cooked-index.c:471 (gdb+0x7f0f31)
    #25 _M_invoke /usr/include/c++/7/bits/std_function.h:316 (gdb+0x7f29fb)
    #26 std::function<void ()>::operator()() const /usr/include/c++/7/bits/std_function.h:706 (gdb+0x700952)
    #27 void std::__invoke_impl<void, std::function<void ()>&>(std::__invoke_other, std::function<void ()>&) /usr/include/c++/7/bits/invoke.h:60 (gdb+0x7381a0)
    #28 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)
    #29 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)
    #30 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)
    #31 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)
    #32 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)
    #33 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)
    #34 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)
    #35 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)
    #36 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)
    #37 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)
    #38 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)
    #39 pthread_once <null> (libtsan.so.0+0x4457c)
    #40 __gthread_once /usr/include/c++/7/x86_64-suse-linux/bits/gthr-default.h:699 (gdb+0x72f5dd)
    #41 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)
    #42 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)
    #43 std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run() /usr/include/c++/7/future:1423 (gdb+0x737bef)
    #44 std::packaged_task<void ()>::operator()() /usr/include/c++/7/future:1556 (gdb+0x1dad242)
    #45 gdb::thread_pool::thread_function() gdbsupport/thread-pool.cc:242 (gdb+0x1dacb64)
    #46 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+0x1dadc13)
    #47 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+0x1dad044)
    #48 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+0x1db0376)
    #49 std::thread::_Invoker<std::tuple<void (gdb::thread_pool::*)(), gdb::thread_pool*> >::operator()() /usr/include/c++/7/thread:243 (gdb+0x1db0301)
    #50 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+0x1db02b6)
    #51 <null> <null> (libstdc++.so.6+0xdcac2)

  Location is global 'bfd_last_cache' of size 8 at 0x0000032569e0 (gdb+0x0000032569e0)
  ...
SUMMARY: ThreadSanitizer: data race bfd/cache.c:155 in snip
...

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

The race is between:
- a worker thread calling main_name (), and in the process reading
  bfd_last_cache, and
- the main thread writing to bfd_last_cache.

Fix this by calling main_name () from the main thread.

Tested on x86_64-linux.

PR symtab/30392
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30392
---
 gdb/dwarf2/cooked-index.c | 7 +++++++
 gdb/dwarf2/read.c         | 8 ++++----
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/gdb/dwarf2/cooked-index.c b/gdb/dwarf2/cooked-index.c
index b5718ae5529..9f647cfd4a2 100644
--- a/gdb/dwarf2/cooked-index.c
+++ b/gdb/dwarf2/cooked-index.c
@@ -462,6 +462,13 @@ cooked_index::start_writing_index (dwarf2_per_bfd *per_bfd)
 {
   struct index_cache_store_context ctx (global_index_cache, per_bfd);
 
+  if (global_index_cache.enabled ())
+    {
+      /* Make sure find_main_name is called in the main thread rather than a
+	 worker thread, to avoid data races on bfd.  */
+      main_name ();
+    }
+
   /* This must be set after all the finalization tasks have been
      started, because it may call 'wait'.  */
   m_write_future
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 61730f6481c..c37bacad3b1 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -5175,10 +5175,6 @@ dwarf2_build_psymtabs_hard (dwarf2_per_objfile *per_objfile)
   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)
     {
@@ -5193,6 +5189,10 @@ dwarf2_build_psymtabs_hard (dwarf2_per_objfile *per_objfile)
       set_objfile_main_name (objfile, full_name, lang);
     }
 
+  /* Cannot start writing the index entry until after the
+     'index_table' member has been set.  */
+  vec->start_writing_index (per_bfd);
+
   dwarf_read_debug_printf ("Done building psymtabs of %s",
 			   objfile_name (objfile));
 }
-- 
2.35.3