* [RFC 0/3] [gdb/symtab] Fix data-races in gdb.base/index-cache.exp
@ 2023-07-28 8:56 Tom de Vries
2023-07-28 8:56 ` [RFC 1/3] [gdb/symtab] Fix data race in index_cache::enable Tom de Vries
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: Tom de Vries @ 2023-07-28 8:56 UTC (permalink / raw)
To: gdb-patches
When building gdb with -fsanitize=thread, we run into a data race in
gdb.base/index-cache.exp.
Fixing this leads us to another, and 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.
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.
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.
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.
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
--
2.35.3
^ permalink raw reply [flat|nested] 7+ messages in thread
* [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
* [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 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 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
* 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
end of thread, other threads:[~2023-08-02 10:40 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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 18:27 ` Simon Marchi
2023-08-02 10:40 ` 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 ` [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
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).