* [RFC 1/3] [gdb/symtab] Fix data race in index_cache::enable
2023-07-28 8:56 [RFC 0/3] [gdb/symtab] Fix data-races in gdb.base/index-cache.exp Tom de Vries
@ 2023-07-28 8:56 ` Tom de Vries
2023-07-28 18:27 ` Simon Marchi
2023-07-28 8:56 ` [RFC 2/3] [gdb/symtab] Fix data race in bfd_open_file Tom de Vries
` (2 subsequent siblings)
3 siblings, 1 reply; 7+ messages in thread
From: Tom de Vries @ 2023-07-28 8:56 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 is between:
- the main thread setting index_cache::m_enabled
(due to command "set index-cache enabled on")
- 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")
Fix this by capturing the value of index_cache::m_enabled during the file
command.
Tested on x86_64-linux.
PR symtab/30392
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30392
---
gdb/dwarf2/cooked-index.c | 8 ++++++--
gdb/dwarf2/cooked-index.h | 3 +++
gdb/dwarf2/index-cache.c | 16 ++++++++++++++--
gdb/dwarf2/index-cache.h | 14 +++++++++++++-
4 files changed, 36 insertions(+), 5 deletions(-)
diff --git a/gdb/dwarf2/cooked-index.c b/gdb/dwarf2/cooked-index.c
index 25635d9b72e..b580e7e25a9 100644
--- a/gdb/dwarf2/cooked-index.c
+++ b/gdb/dwarf2/cooked-index.c
@@ -444,7 +444,7 @@ cooked_index_shard::wait (bool allow_quit) const
}
cooked_index::cooked_index (vec_type &&vec)
- : m_vector (std::move (vec))
+ : m_vector (std::move (vec)), m_index_cache_store_context (nullptr)
{
for (auto &idx : m_vector)
idx->finalize ();
@@ -460,6 +460,8 @@ cooked_index::cooked_index (vec_type &&vec)
void
cooked_index::start_writing_index (dwarf2_per_bfd *per_bfd)
{
+ m_index_cache_store_context = global_index_cache.get_store_context ();
+
/* This must be set after all the finalization tasks have been
started, because it may call 'wait'. */
m_write_future
@@ -635,7 +637,9 @@ cooked_index::maybe_write_index (dwarf2_per_bfd *per_bfd)
wait ();
/* (maybe) store an index in the cache. */
- global_index_cache.store (per_bfd);
+ global_index_cache.store (per_bfd, m_index_cache_store_context);
+ delete m_index_cache_store_context;
+ m_index_cache_store_context = nullptr;
}
/* 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..6f61cd45f67 100644
--- a/gdb/dwarf2/cooked-index.h
+++ b/gdb/dwarf2/cooked-index.h
@@ -443,6 +443,9 @@ class cooked_index : public dwarf_scanner_base
/* A future that tracks when the 'index_write' method is done. */
gdb::future<void> m_write_future;
+
+ /* The context to pass to global_index_cache.store. */
+ struct index_cache_store_context *m_index_cache_store_context;
};
#endif /* GDB_DWARF2_COOKED_INDEX_H */
diff --git a/gdb/dwarf2/index-cache.c b/gdb/dwarf2/index-cache.c
index 79ab706ee9d..3bce83ffcd0 100644
--- a/gdb/dwarf2/index-cache.c
+++ b/gdb/dwarf2/index-cache.c
@@ -88,10 +88,22 @@ index_cache::disable ()
/* See dwarf-index-cache.h. */
+struct index_cache_store_context *
+index_cache::get_store_context ()
+{
+ struct index_cache_store_context *res
+ = new struct index_cache_store_context;
+
+ res->enabled = enabled ();
+ return res;
+}
+
+/* See dwarf-index-cache.h. */
+
void
-index_cache::store (dwarf2_per_bfd *per_bfd)
+index_cache::store (dwarf2_per_bfd *per_bfd, struct index_cache_store_context *ctx)
{
- if (!enabled ())
+ if (!ctx->enabled)
return;
/* Get build id of objfile. */
diff --git a/gdb/dwarf2/index-cache.h b/gdb/dwarf2/index-cache.h
index 1efff17049f..217720b46d9 100644
--- a/gdb/dwarf2/index-cache.h
+++ b/gdb/dwarf2/index-cache.h
@@ -34,6 +34,15 @@ struct index_cache_resource
virtual ~index_cache_resource () = 0;
};
+/* Information to be captured in the main thread using get_store_context, and
+ to be used by worker threads during store (). */
+
+struct index_cache_store_context
+{
+ /* Captured value of enabled (). */
+ bool enabled;
+};
+
/* Class to manage the access to the DWARF index cache. */
class index_cache
@@ -54,8 +63,11 @@ class index_cache
/* Disable the cache. */
void disable ();
+ /* Get index_cache_store_context. */
+ struct index_cache_store_context *get_store_context ();
+
/* 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, struct 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] 7+ messages in thread* Re: [RFC 1/3] [gdb/symtab] Fix data race in index_cache::enable
2023-07-28 8:56 ` [RFC 1/3] [gdb/symtab] Fix data race in index_cache::enable Tom de Vries
@ 2023-07-28 18:27 ` Simon Marchi
2023-08-02 10:40 ` Tom de Vries
0 siblings, 1 reply; 7+ messages in thread
From: Simon Marchi @ 2023-07-28 18:27 UTC (permalink / raw)
To: Tom de Vries, gdb-patches
On 2023-07-28 04:56, Tom de Vries via Gdb-patches wrote:
> 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 is between:
> - the main thread setting index_cache::m_enabled
> (due to command "set index-cache enabled on")
> - 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")
>
> Fix this by capturing the value of index_cache::m_enabled during the file
> command.
For completeness: in 30392, I initially suggested making m_enabled an
std::atomic. To which you responded:
By making it a std::atomic<bool>, we make this non-determinism
defined behaviour, so tsan stops complaining, but we want
deterministic behaviour instead.
I agree with that. Capturing the value at the time of the command makes
sense.
One comment on the implementation. Do we really need to save an
index_cache_store_context object in cooked_index? Could we:
- make get_store_context return it by value
- std::move it to the thread (or just copy it, if it's not expected to
be large) into the worker thread's closure (in
cooked_index::start_writing_index)
- pass it down by const-ref to whatever needs it
Simon
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [RFC 1/3] [gdb/symtab] Fix data race in index_cache::enable
2023-07-28 18:27 ` Simon Marchi
@ 2023-08-02 10:40 ` Tom de Vries
0 siblings, 0 replies; 7+ messages in thread
From: Tom de Vries @ 2023-08-02 10:40 UTC (permalink / raw)
To: Simon Marchi, gdb-patches
On 7/28/23 20:27, Simon Marchi wrote:
Hi Simon,
thanks for the review.
>> The race is between:
>> - the main thread setting index_cache::m_enabled
>> (due to command "set index-cache enabled on")
>> - 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")
>>
>> Fix this by capturing the value of index_cache::m_enabled during the file
>> command.
> For completeness: in 30392, I initially suggested making m_enabled an
> std::atomic. To which you responded:
>
> By making it a std::atomic<bool>, we make this non-determinism
> defined behaviour, so tsan stops complaining, but we want
> deterministic behaviour instead.
>
> I agree with that. Capturing the value at the time of the command makes
> sense.
>
Ack, thanks for making that explicit.
> One comment on the implementation. Do we really need to save an
> index_cache_store_context object in cooked_index? Could we:
>
> - make get_store_context return it by value
> - std::move it to the thread (or just copy it, if it's not expected to
> be large) into the worker thread's closure (in
> cooked_index::start_writing_index)
> - pass it down by const-ref to whatever needs it
I've implemented this approach in v2 (
https://sourceware.org/pipermail/gdb-patches/2023-August/201273.html ).
I also realized that get_store_context is basically a constructor, so
I've remodeled things in this way as well.
Thanks,
- Tom
^ permalink raw reply [flat|nested] 7+ messages in thread
* [RFC 2/3] [gdb/symtab] Fix data race in bfd_open_file
2023-07-28 8:56 [RFC 0/3] [gdb/symtab] Fix data-races in gdb.base/index-cache.exp Tom de Vries
2023-07-28 8:56 ` [RFC 1/3] [gdb/symtab] Fix data race in index_cache::enable Tom de Vries
@ 2023-07-28 8:56 ` Tom de Vries
2023-07-28 8:56 ` [RFC 3/3] [gdb/symtab] Fix race on per_cu->queued Tom de Vries
2023-08-02 10:34 ` [RFC 0/3] [gdb/symtab] Fix data-races in gdb.base/index-cache.exp Tom de Vries
3 siblings, 0 replies; 7+ messages in thread
From: Tom de Vries @ 2023-07-28 8:56 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 during the "file $exec" command.
The race is between:
- the main thread calling symbol_file_add_main_1, and in the process setting
bfd::cacheable.
- a worker thread getting the build id while writing the index cache, and in
the process reading bfd::format.
The two bitfields bfd::cacheable and bfd::format share the same bitfield
container.
Fix this by caching the build id in struct index_cache_store_context.
Likewise for the dwz build id.
While we're at it, also move the creation of the cache directory to
get_store_context.
Tested on x86_64-linux.
PR symtab/30392
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30392
---
gdb/dwarf2/cooked-index.c | 2 +-
gdb/dwarf2/index-cache.c | 60 +++++++++++++++++++++++++--------------
gdb/dwarf2/index-cache.h | 8 +++++-
3 files changed, 46 insertions(+), 24 deletions(-)
diff --git a/gdb/dwarf2/cooked-index.c b/gdb/dwarf2/cooked-index.c
index b580e7e25a9..eca91435611 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)
{
- m_index_cache_store_context = global_index_cache.get_store_context ();
+ m_index_cache_store_context = global_index_cache.get_store_context (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 3bce83ffcd0..6f3984d8699 100644
--- a/gdb/dwarf2/index-cache.c
+++ b/gdb/dwarf2/index-cache.c
@@ -89,22 +89,14 @@ index_cache::disable ()
/* See dwarf-index-cache.h. */
struct index_cache_store_context *
-index_cache::get_store_context ()
+index_cache::get_store_context (dwarf2_per_bfd *per_bfd)
{
struct index_cache_store_context *res
= new struct index_cache_store_context;
res->enabled = enabled ();
- return res;
-}
-
-/* See dwarf-index-cache.h. */
-
-void
-index_cache::store (dwarf2_per_bfd *per_bfd, struct index_cache_store_context *ctx)
-{
- if (!ctx->enabled)
- return;
+ if (!res->enabled)
+ return res;
/* Get build id of objfile. */
const bfd_build_id *build_id = build_id_bfd_get (per_bfd->obfd);
@@ -112,15 +104,13 @@ index_cache::store (dwarf2_per_bfd *per_bfd, struct index_cache_store_context *c
{
index_cache_debug ("objfile %s has no build id",
bfd_get_filename (per_bfd->obfd));
- return;
+ res->enabled = false;
+ return res;
}
-
- std::string build_id_str = build_id_to_string (build_id);
+ res->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)
{
@@ -130,17 +120,18 @@ index_cache::store (dwarf2_per_bfd *per_bfd, struct index_cache_store_context *c
{
index_cache_debug ("dwz objfile %s has no build id",
dwz->filename ());
- return;
+ res->enabled = false;
+ return res;
}
- dwz_build_id_str = build_id_to_string (dwz_build_id);
- dwz_build_id_ptr = dwz_build_id_str->c_str ();
+ res->dwz_build_id_str = build_id_to_string (dwz_build_id);
}
if (m_dir.empty ())
{
warning (_("The index cache directory name is empty, skipping store."));
- return;
+ res->enabled = false;
+ return res;
}
try
@@ -150,16 +141,41 @@ index_cache::store (dwarf2_per_bfd *per_bfd, struct index_cache_store_context *c
{
warning (_("index cache: could not make cache directory: %s"),
safe_strerror (errno));
- return;
+ res->enabled = false;
+ return res;
}
+ }
+ 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 ());
+ res->enabled = false;
+ }
+
+ return res;
+}
+
+/* See dwarf-index-cache.h. */
+
+void
+index_cache::store (dwarf2_per_bfd *per_bfd, struct index_cache_store_context *ctx)
+{
+ if (!ctx->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 217720b46d9..38ef765a0a6 100644
--- a/gdb/dwarf2/index-cache.h
+++ b/gdb/dwarf2/index-cache.h
@@ -41,6 +41,12 @@ struct index_cache_store_context
{
/* Captured value of enabled (). */
bool 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. */
@@ -64,7 +70,7 @@ class index_cache
void disable ();
/* Get index_cache_store_context. */
- struct index_cache_store_context *get_store_context ();
+ struct index_cache_store_context *get_store_context (dwarf2_per_bfd *per_bfd);
/* Store an index for the specified object file in the cache. */
void store (dwarf2_per_bfd *per_bfd, struct index_cache_store_context *);
--
2.35.3
^ permalink raw reply [flat|nested] 7+ messages in thread* [RFC 3/3] [gdb/symtab] Fix race on per_cu->queued
2023-07-28 8:56 [RFC 0/3] [gdb/symtab] Fix data-races in gdb.base/index-cache.exp Tom de Vries
2023-07-28 8:56 ` [RFC 1/3] [gdb/symtab] Fix data race in index_cache::enable Tom de Vries
2023-07-28 8:56 ` [RFC 2/3] [gdb/symtab] Fix data race in bfd_open_file Tom de Vries
@ 2023-07-28 8:56 ` Tom de Vries
2023-08-02 10:34 ` [RFC 0/3] [gdb/symtab] Fix data-races in gdb.base/index-cache.exp Tom de Vries
3 siblings, 0 replies; 7+ messages in thread
From: Tom de Vries @ 2023-07-28 8:56 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 during the "file $exec" command.
The race is between:
- the main thread expanding the CU containing main, and in the process setting
dwarf2_per_cu_data::queued, and
- a worker thread writing the index cache, and in the process reading
dwarf2_per_cu_data::is_debug_type.
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 std::atomic<bool>.
Tested on x86_64-linux.
PR symtab/30392
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30392
---
gdb/dwarf2/read.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
index a99299cbc6d..28e4cb69a77 100644
--- a/gdb/dwarf2/read.h
+++ b/gdb/dwarf2/read.h
@@ -128,7 +128,7 @@ struct dwarf2_per_cu_data
public:
/* Flag indicating this compilation unit will be read in before
any of the current compilation units are processed. */
- unsigned int queued : 1;
+ std::atomic<bool> queued;
/* Non-zero if this CU is from .debug_types.
Struct dwarf2_per_cu_data is contained in struct signatured_type iff
--
2.35.3
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [RFC 0/3] [gdb/symtab] Fix data-races in gdb.base/index-cache.exp
2023-07-28 8:56 [RFC 0/3] [gdb/symtab] Fix data-races in gdb.base/index-cache.exp Tom de Vries
` (2 preceding siblings ...)
2023-07-28 8:56 ` [RFC 3/3] [gdb/symtab] Fix race on per_cu->queued Tom de Vries
@ 2023-08-02 10:34 ` Tom de Vries
3 siblings, 0 replies; 7+ messages in thread
From: Tom de Vries @ 2023-08-02 10:34 UTC (permalink / raw)
To: gdb-patches
On 7/28/23 10:56, Tom de Vries via Gdb-patches wrote:
> When building gdb with -fsanitize=thread, we run into a data race in
> gdb.base/index-cache.exp.
>
I've submitted a v2 here (
https://sourceware.org/pipermail/gdb-patches/2023-August/201270.html ).
> Fixing this leads us to another, and another, so there are three patches, each
> fixing a data race.
>
> 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 std::atomic
> approach:
> ...
> - unsigned int queued : 1;
> + std::atomic<bool> queued;
> ...
>
> The 3rd patch is the reason this is an RFC.
>
In v2 I'm no longer using std::atomic<bool>, instead I'm using
packed<bool, 1>.
> I spent some time convincing myself that this is not a benign data race. 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.
>
> As for fixing it, the std::atomic approach works. I checked that the struct
> size is not increased. It gives me an almost clean test-suite run (still
> running into PR30680), though I'm running into a lot of timeouts, so things
> still may be hidden there.
>
I now also ran on a fast server with lots of memory, and the timeouts do
not happen there.
> Alternatively, if I limit the fix to:
> ...
> - unsigned int queued : 1;
> + bool queued;
> ...
> I run into 19 more reported data races in the test-case. None of them look
> as evidence that this fix on queued is insufficient.
>
> Looking at the first one (see appendix for more detail), there's a race
> between:
> - the main thread calling find_main_name, because of calling main_language in
> set_initial_language, and
> - the worker thread calling find_main_name, because of calling main_name in
> write_cooked_index.
> At least 14 other of the 19 reported data races look like instances of the
> first one.
>
> This looks like a real problem to me, but with the proposed fix it no longer
> triggers, although I fail to see how it fixes it.
>
I managed to trigger data races by running with taskset -c 0. I ended
up fixing data races until the combination of make-check-all.sh and
taskset -c 0 no longer triggered any. Indeed one of the patches in v2
does address the find_main_name issue.
> Which leaves me with the questions:
> - is the std::atomic approach too blunt, and are we consequently hiding
> problems, and
> - should we address the find_main_name race in some form or another.
>
I no longer have these questions, so the v2 no longer is an RFC.
Thanks,
- Tom
> 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/
>
> Appendix:
> ...
> (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=1569)
> Read of size 1 at 0x7b4400097d08 by thread T12:
> #0 bfd_get_section_limit_octets bfd.h:2376 (gdb+0x149b613)
> #1 bfd_get_section_contents bfd/section.c:1571 (gdb+0x149cbe4)
> #2 gdb_bfd_scan_elf_dyntag(int, bfd*, unsigned long*, unsigned long*) gdb/solib.c:1601 (gdb+0xed8fc8)
> #3 elf_locate_base gdb/solib-svr4.c:705 (gdb+0xec29aa)
> #4 svr4_iterate_over_objfiles_in_search_order gdb/solib-svr4.c:3430 (gdb+0xeca65b)
> #5 gdbarch_iterate_over_objfiles_in_search_order(gdbarch*, gdb::function_view<bool (objfile*)>, objfile*) gdb/gdbarch.c:5041 (gdb+0x537cad)
> #6 find_main_name gdb/symtab.c:6270 (gdb+0xf744a3)
> #7 main_name() gdb/symtab.c:6299 (gdb+0xf74531)
> #8 write_cooked_index gdb/dwarf2/index-write.c:1131 (gdb+0x830af4)
> #9 write_gdbindex gdb/dwarf2/index-write.c:1256 (gdb+0x83134b)
> #10 write_dwarf_index(dwarf2_per_bfd*, char const*, char const*, char const*, dw_index_kind) gdb/dwarf2/index-write.c:1484 (gdb+0x83232f)
> #11 index_cache::store(dwarf2_per_bfd*, index_cache_store_context*) gdb/dwarf2/index-cache.c:177 (gdb+0x82d62b)
> #12 cooked_index::maybe_write_index(dwarf2_per_bfd*) gdb/dwarf2/cooked-index.c:640 (gdb+0x7f1bf7)
> #13 operator() gdb/dwarf2/cooked-index.c:470 (gdb+0x7f0f40)
> #14 _M_invoke /usr/include/c++/7/bits/std_function.h:316 (gdb+0x7f2909)
> #15 std::function<void ()>::operator()() const /usr/include/c++/7/bits/std_function.h:706 (gdb+0x700952)
> #16 void std::__invoke_impl<void, std::function<void ()>&>(std::__invoke_other, std::function<void ()>&) /usr/include/c++/7/bits/invoke.h:60 (gdb+0x7381a0)
> #17 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)
> #18 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)
> #19 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)
> #20 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)
> #21 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)
> #22 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)
> #23 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)
> #24 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)
> #25 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)
> #26 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)
> #27 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)
> #28 pthread_once <null> (libtsan.so.0+0x4457c)
> #29 __gthread_once /usr/include/c++/7/x86_64-suse-linux/bits/gthr-default.h:699 (gdb+0x72f5dd)
> #30 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)
> #31 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)
> #32 std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run() /usr/include/c++/7/future:1423 (gdb+0x737bef)
> #33 std::packaged_task<void ()>::operator()() /usr/include/c++/7/future:1556 (gdb+0x1daca1c)
> #34 gdb::thread_pool::thread_function() gdbsupport/thread-pool.cc:242 (gdb+0x1dac33e)
> #35 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+0x1dad3ed)
> #36 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+0x1dac81e)
> #37 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+0x1dafb50)
> #38 std::thread::_Invoker<std::tuple<void (gdb::thread_pool::*)(), gdb::thread_pool*> >::operator()() /usr/include/c++/7/thread:243 (gdb+0x1dafadb)
> #39 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+0x1dafa90)
> #40 <null> <null> (libstdc++.so.6+0xdcac2)
>
> Previous write of size 4 at 0x7b4400097d08 by main thread:
> #0 bfd_open_file bfd/cache.c:584 (gdb+0x148bc90)
> #1 bfd_cache_lookup_worker bfd/cache.c:261 (gdb+0x148b228)
> #2 cache_bseek bfd/cache.c:289 (gdb+0x148b422)
> #3 bfd_seek bfd/bfdio.c:459 (gdb+0x1489d2f)
> #4 _bfd_generic_get_section_contents bfd/libbfd.c:1069 (gdb+0x14978a2)
> #5 bfd_get_section_contents bfd/section.c:1606 (gdb+0x149cd7a)
> #6 gdb_bfd_scan_elf_dyntag(int, bfd*, unsigned long*, unsigned long*) gdb/solib.c:1601 (gdb+0xed8fc8)
> #7 elf_locate_base gdb/solib-svr4.c:705 (gdb+0xec29aa)
> #8 svr4_iterate_over_objfiles_in_search_order gdb/solib-svr4.c:3430 (gdb+0xeca65b)
> #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+0xf744a3)
> #11 main_language() gdb/symtab.c:6313 (gdb+0xf74597)
> #12 set_initial_language() gdb/symfile.c:1700 (gdb+0xf4295a)
> #13 symbol_file_add_main_1 gdb/symfile.c:1212 (gdb+0xf40f28)
> #14 symbol_file_command(char const*, int) gdb/symfile.c:1681 (gdb+0xf428cf)
> #15 file_command gdb/exec.c:554 (gdb+0x94f849)
> #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+0xff313a)
> #19 command_handler(char const*) gdb/event-top.c:552 (gdb+0x94aedc)
> #20 command_line_handler(std::unique_ptr<char, gdb::xfree_deleter<char> >&&) gdb/event-top.c:788 (gdb+0x94b599)
> #21 tui_command_line_handler gdb/tui/tui-interp.c:104 (gdb+0x103489a)
> #22 gdb_rl_callback_handler gdb/event-top.c:259 (gdb+0x94a481)
> #23 rl_callback_read_char readline/readline/callback.c:290 (gdb+0x11bdf5b)
> #24 gdb_rl_callback_read_char_wrapper_noexcept gdb/event-top.c:195 (gdb+0x94a280)
> #25 gdb_rl_callback_read_char_wrapper gdb/event-top.c:234 (gdb+0x94a341)
> #26 stdin_event_handler gdb/ui.c:155 (gdb+0x1074b3e)
> #27 handle_file_event gdbsupport/event-loop.cc:573 (gdb+0x1d9536e)
> #28 gdb_wait_for_event gdbsupport/event-loop.cc:694 (gdb+0x1d95aa6)
> #29 gdb_do_one_event(int) gdbsupport/event-loop.cc:264 (gdb+0x1d93e92)
> #30 start_event_loop gdb/main.c:412 (gdb+0xb5a472)
> #31 captured_command_loop gdb/main.c:476 (gdb+0xb5a661)
> #32 captured_main gdb/main.c:1320 (gdb+0xb5c7e1)
> #33 gdb_main(captured_main_args*) gdb/main.c:1339 (gdb+0xb5c890)
> #34 main gdb/gdb.c:32 (gdb+0x416776)
> ...
> SUMMARY: ThreadSanitizer: data race bfd.h:2376 in bfd_get_section_limit_octets
> ...
>
> Tom de Vries (3):
> [gdb/symtab] Fix data race in index_cache::enable
> [gdb/symtab] Fix data race in bfd_open_file
> [gdb/symtab] Fix race on per_cu->queued
>
> gdb/dwarf2/cooked-index.c | 8 ++++--
> gdb/dwarf2/cooked-index.h | 3 ++
> gdb/dwarf2/index-cache.c | 58 +++++++++++++++++++++++++++++----------
> gdb/dwarf2/index-cache.h | 20 +++++++++++++-
> gdb/dwarf2/read.h | 2 +-
> 5 files changed, 72 insertions(+), 19 deletions(-)
>
>
> base-commit: 29c108c9610640439daa5244a573348b7c47d994
^ permalink raw reply [flat|nested] 7+ messages in thread