diff options
author | Tom de Vries <tdevries@suse.de> | 2023-08-04 15:02:43 +0200 |
---|---|---|
committer | Tom de Vries <tdevries@suse.de> | 2023-08-04 15:02:43 +0200 |
commit | 8adc5522280c06c1c750185af0dc5a8461d9a13f (patch) | |
tree | 71610b575143ef759c1387380da110d4ff48e23e /gdb | |
parent | 066c738ec5d803a080e3da3b0936484a10c8f31b (diff) | |
download | gdb-8adc5522280c06c1c750185af0dc5a8461d9a13f.zip gdb-8adc5522280c06c1c750185af0dc5a8461d9a13f.tar.gz gdb-8adc5522280c06c1c750185af0dc5a8461d9a13f.tar.bz2 |
[gdb/symtab] Fix data race on index_cache::m_enabled
With gdb build with -fsanitize=thread and test-case gdb.base/index-cache.exp I
run into:
...
(gdb) file build/gdb/testsuite/outputs/gdb.base/index-cache/index-cache
Reading symbols from build/gdb/testsuite/outputs/gdb.base/index-cache/index-cache...
(gdb) show index-cache enabled
The index cache is off.
(gdb) PASS: gdb.base/index-cache.exp: test_basic_stuff: index-cache is disabled by default
set index-cache enabled on
==================
WARNING: ThreadSanitizer: data race (pid=32248)
Write of size 1 at 0x00000321f540 by main thread:
#0 index_cache::enable() gdb/dwarf2/index-cache.c:76 (gdb+0x82cfdd)
#1 set_index_cache_enabled_command gdb/dwarf2/index-cache.c:270 (gdb+0x82d9af)
#2 bool setting::set<bool>(bool const&) gdb/command.h:353 (gdb+0x6fe5f2)
#3 do_set_command(char const*, int, cmd_list_element*) gdb/cli/cli-setshow.c:414 (gdb+0x6fcd21)
#4 execute_command(char const*, int) gdb/top.c:567 (gdb+0xff2e64)
#5 command_handler(char const*) gdb/event-top.c:552 (gdb+0x94acc0)
#6 command_line_handler(std::unique_ptr<char, gdb::xfree_deleter<char> >&&) gdb/event-top.c:788 (gdb+0x94b37d)
#7 tui_command_line_handler gdb/tui/tui-interp.c:104 (gdb+0x103467e)
#8 gdb_rl_callback_handler gdb/event-top.c:259 (gdb+0x94a265)
#9 rl_callback_read_char readline/readline/callback.c:290 (gdb+0x11bdd3f)
#10 gdb_rl_callback_read_char_wrapper_noexcept gdb/event-top.c:195 (gdb+0x94a064)
#11 gdb_rl_callback_read_char_wrapper gdb/event-top.c:234 (gdb+0x94a125)
#12 stdin_event_handler gdb/ui.c:155 (gdb+0x1074922)
#13 handle_file_event gdbsupport/event-loop.cc:573 (gdb+0x1d94de4)
#14 gdb_wait_for_event gdbsupport/event-loop.cc:694 (gdb+0x1d9551c)
#15 gdb_do_one_event(int) gdbsupport/event-loop.cc:264 (gdb+0x1d93908)
#16 start_event_loop gdb/main.c:412 (gdb+0xb5a256)
#17 captured_command_loop gdb/main.c:476 (gdb+0xb5a445)
#18 captured_main gdb/main.c:1320 (gdb+0xb5c5c5)
#19 gdb_main(captured_main_args*) gdb/main.c:1339 (gdb+0xb5c674)
#20 main gdb/gdb.c:32 (gdb+0x416776)
Previous read of size 1 at 0x00000321f540 by thread T12:
#0 index_cache::enabled() const gdb/dwarf2/index-cache.h:48 (gdb+0x82e1a6)
#1 index_cache::store(dwarf2_per_bfd*) gdb/dwarf2/index-cache.c:94 (gdb+0x82d0bc)
#2 cooked_index::maybe_write_index(dwarf2_per_bfd*) gdb/dwarf2/cooked-index.c:638 (gdb+0x7f1b97)
#3 operator() gdb/dwarf2/cooked-index.c:468 (gdb+0x7f0f24)
#4 _M_invoke /usr/include/c++/7/bits/std_function.h:316 (gdb+0x7f285b)
#5 std::function<void ()>::operator()() const /usr/include/c++/7/bits/std_function.h:706 (gdb+0x700952)
#6 void std::__invoke_impl<void, std::function<void ()>&>(std::__invoke_other, std::function<void ()>&) /usr/include/c++/7/bits/invoke.h:60 (gdb+0x7381a0)
#7 std::__invoke_result<std::function<void ()>&>::type std::__invoke<std::function<void ()>&>(std::function<void ()>&) /usr/include/c++/7/bits/invoke.h:95 (gdb+0x737e91)
#8 std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run()::{lambda()#1}::operator()() const /usr/include/c++/7/future:1421 (gdb+0x737b59)
#9 std::__future_base::_Task_setter<std::unique_ptr<std::__future_base::_Result<void>, std::__future_base::_Result_base::_Deleter>, std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run()::{lambda()#1}, void>::operator()() const /usr/include/c++/7/future:1362 (gdb+0x738660)
#10 std::_Function_handler<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> (), std::__future_base::_Task_setter<std::unique_ptr<std::__future_base::_Result<void>, std::__future_base::_Result_base::_Deleter>, std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run()::{lambda()#1}, void> >::_M_invoke(std::_Any_data const&) /usr/include/c++/7/bits/std_function.h:302 (gdb+0x73825c)
#11 std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>::operator()() const /usr/include/c++/7/bits/std_function.h:706 (gdb+0x733623)
#12 std::__future_base::_State_baseV2::_M_do_set(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*) /usr/include/c++/7/future:561 (gdb+0x732bdf)
#13 void std::__invoke_impl<void, void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::__invoke_memfun_deref, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&) /usr/include/c++/7/bits/invoke.h:73 (gdb+0x734c4f)
#14 std::__invoke_result<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>::type std::__invoke<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&) /usr/include/c++/7/bits/invoke.h:95 (gdb+0x733bc5)
#15 std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&)::{lambda()#1}::operator()() const /usr/include/c++/7/mutex:672 (gdb+0x73300d)
#16 std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&)::{lambda()#2}::operator()() const /usr/include/c++/7/mutex:677 (gdb+0x7330b2)
#17 std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&)::{lambda()#2}::_FUN() /usr/include/c++/7/mutex:677 (gdb+0x7330f2)
#18 pthread_once <null> (libtsan.so.0+0x4457c)
#19 __gthread_once /usr/include/c++/7/x86_64-suse-linux/bits/gthr-default.h:699 (gdb+0x72f5dd)
#20 void std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&) /usr/include/c++/7/mutex:684 (gdb+0x733224)
#21 std::__future_base::_State_baseV2::_M_set_result(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>, bool) /usr/include/c++/7/future:401 (gdb+0x732852)
#22 std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run() /usr/include/c++/7/future:1423 (gdb+0x737bef)
#23 std::packaged_task<void ()>::operator()() /usr/include/c++/7/future:1556 (gdb+0x1dac492)
#24 gdb::thread_pool::thread_function() gdbsupport/thread-pool.cc:242 (gdb+0x1dabdb4)
#25 void std::__invoke_impl<void, void (gdb::thread_pool::*)(), gdb::thread_pool*>(std::__invoke_memfun_deref, void (gdb::thread_pool::*&&)(), gdb::thread_pool*&&) /usr/include/c++/7/bits/invoke.h:73 (gdb+0x1dace63)
#26 std::__invoke_result<void (gdb::thread_pool::*)(), gdb::thread_pool*>::type std::__invoke<void (gdb::thread_pool::*)(), gdb::thread_pool*>(void (gdb::thread_pool::*&&)(), gdb::thread_pool*&&) /usr/include/c++/7/bits/invoke.h:95 (gdb+0x1dac294)
#27 decltype (__invoke((_S_declval<0ul>)(), (_S_declval<1ul>)())) std::thread::_Invoker<std::tuple<void (gdb::thread_pool::*)(), gdb::thread_pool*> >::_M_invoke<0ul, 1ul>(std::_Index_tuple<0ul, 1ul>) /usr/include/c++/7/thread:234 (gdb+0x1daf5c6)
#28 std::thread::_Invoker<std::tuple<void (gdb::thread_pool::*)(), gdb::thread_pool*> >::operator()() /usr/include/c++/7/thread:243 (gdb+0x1daf551)
#29 std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (gdb::thread_pool::*)(), gdb::thread_pool*> > >::_M_run() /usr/include/c++/7/thread:186 (gdb+0x1daf506)
#30 <null> <null> (libstdc++.so.6+0xdcac2)
Location is global 'global_index_cache' of size 48 at 0x00000321f520 (gdb+0x00000321f540)
...
SUMMARY: ThreadSanitizer: data race gdb/dwarf2/index-cache.c:76 in index_cache::enable()
...
The race happens when issuing a "file $exec" command followed by a
"set index-cache enabled on" command.
The race is between:
- a worker thread reading index_cache::m_enabled to determine whether an
index-cache entry for $exec needs to be written
(due to command "file $exec"), and
- the main thread setting index_cache::m_enabled
(due to command "set index-cache enabled on").
Fix this by capturing the value of index_cache::m_enabled in the main thread,
and using the captured value in the worker thread.
Tested on x86_64-linux.
PR symtab/30392
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30392
Diffstat (limited to 'gdb')
-rw-r--r-- | gdb/dwarf2/cooked-index.c | 12 | ||||
-rw-r--r-- | gdb/dwarf2/cooked-index.h | 4 | ||||
-rw-r--r-- | gdb/dwarf2/index-cache.c | 12 | ||||
-rw-r--r-- | gdb/dwarf2/index-cache.h | 18 |
4 files changed, 38 insertions, 8 deletions
diff --git a/gdb/dwarf2/cooked-index.c b/gdb/dwarf2/cooked-index.c index 25635d9..7ddcded 100644 --- a/gdb/dwarf2/cooked-index.c +++ b/gdb/dwarf2/cooked-index.c @@ -460,12 +460,15 @@ cooked_index::cooked_index (vec_type &&vec) void cooked_index::start_writing_index (dwarf2_per_bfd *per_bfd) { + index_cache_store_context ctx (global_index_cache); + /* This must be set after all the finalization tasks have been started, because it may call 'wait'. */ m_write_future - = gdb::thread_pool::g_thread_pool->post_task ([this, per_bfd] () + = gdb::thread_pool::g_thread_pool->post_task ([this, per_bfd, + ctx = std::move (ctx)] () { - maybe_write_index (per_bfd); + maybe_write_index (per_bfd, ctx); }); } @@ -629,13 +632,14 @@ cooked_index::dump (gdbarch *arch) const } void -cooked_index::maybe_write_index (dwarf2_per_bfd *per_bfd) +cooked_index::maybe_write_index (dwarf2_per_bfd *per_bfd, + const index_cache_store_context &ctx) { /* Wait for finalization. */ wait (); /* (maybe) store an index in the cache. */ - global_index_cache.store (per_bfd); + global_index_cache.store (per_bfd, ctx); } /* Wait for all the index cache entries to be written before gdb diff --git a/gdb/dwarf2/cooked-index.h b/gdb/dwarf2/cooked-index.h index 0d6f3e5..5aacb32 100644 --- a/gdb/dwarf2/cooked-index.h +++ b/gdb/dwarf2/cooked-index.h @@ -37,6 +37,7 @@ struct dwarf2_per_cu_data; struct dwarf2_per_bfd; +struct index_cache_store_context; /* Flags that describe an entry in the index. */ enum cooked_index_flag_enum : unsigned char @@ -435,7 +436,8 @@ public: private: /* Maybe write the index to the index cache. */ - void maybe_write_index (dwarf2_per_bfd *per_bfd); + void maybe_write_index (dwarf2_per_bfd *per_bfd, + const index_cache_store_context &); /* The vector of cooked_index objects. This is stored because the entries are stored on the obstacks in those objects. */ diff --git a/gdb/dwarf2/index-cache.c b/gdb/dwarf2/index-cache.c index 79ab706..e2fb984 100644 --- a/gdb/dwarf2/index-cache.c +++ b/gdb/dwarf2/index-cache.c @@ -86,12 +86,20 @@ index_cache::disable () m_enabled = false; } + /* See index-cache.h. */ + +index_cache_store_context::index_cache_store_context (const index_cache &ic) + : m_enabled (ic.enabled ()) +{ +} + /* See dwarf-index-cache.h. */ void -index_cache::store (dwarf2_per_bfd *per_bfd) +index_cache::store (dwarf2_per_bfd *per_bfd, + const index_cache_store_context &ctx) { - if (!enabled ()) + if (!ctx.m_enabled) return; /* Get build id of objfile. */ diff --git a/gdb/dwarf2/index-cache.h b/gdb/dwarf2/index-cache.h index 1efff17..7ea972d 100644 --- a/gdb/dwarf2/index-cache.h +++ b/gdb/dwarf2/index-cache.h @@ -25,6 +25,7 @@ #include "symfile.h" class dwarf2_per_bfd; +class index_cache; /* Base of the classes used to hold the resources of the indices loaded from the cache (e.g. mmapped files). */ @@ -34,6 +35,20 @@ struct index_cache_resource virtual ~index_cache_resource () = 0; }; +/* Information to be captured in the main thread, and to be used by worker + threads during store (). */ + +struct index_cache_store_context +{ + friend class index_cache; + + explicit index_cache_store_context (const index_cache &ic); + +private: + /* Captured value of enabled (). */ + bool m_enabled; +}; + /* Class to manage the access to the DWARF index cache. */ class index_cache @@ -55,7 +70,8 @@ public: void disable (); /* Store an index for the specified object file in the cache. */ - void store (dwarf2_per_bfd *per_bfd); + void store (dwarf2_per_bfd *per_bfd, + const index_cache_store_context &); /* Look for an index file matching BUILD_ID. If found, return the contents as an array_view and store the underlying resources (allocated memory, |