From: Tom de Vries <tdevries@suse.de>
To: gdb-patches@sourceware.org
Subject: [PATCH v2 2/6] [gdb/symtab] Fix data race on bfd::{cacheable,format}
Date: Wed, 2 Aug 2023 11:53:01 +0200 [thread overview]
Message-ID: <20230802095305.3668-3-tdevries@suse.de> (raw)
In-Reply-To: <20230802095305.3668-1-tdevries@suse.de>
With gdb build with -fsanitize=thread and test-case gdb.base/index-cache.exp I
run into:
...
(gdb) file build/gdb/testsuite/outputs/gdb.base/index-cache/index-cache
Reading symbols from build/gdb/testsuite/outputs/gdb.base/index-cache/index-cache...
==================
WARNING: ThreadSanitizer: data race (pid=12261)
Write of size 4 at 0x7b4400097d08 by main thread:
#0 bfd_open_file bfd/cache.c:584 (gdb+0x148bb92)
#1 bfd_cache_lookup_worker bfd/cache.c:261 (gdb+0x148b12a)
#2 cache_bseek bfd/cache.c:289 (gdb+0x148b324)
#3 bfd_seek bfd/bfdio.c:459 (gdb+0x1489c31)
#4 _bfd_generic_get_section_contents bfd/libbfd.c:1069 (gdb+0x14977a4)
#5 bfd_get_section_contents bfd/section.c:1606 (gdb+0x149cc7c)
#6 gdb_bfd_scan_elf_dyntag(int, bfd*, unsigned long*, unsigned long*) gdb/solib.c:1601 (gdb+0xed8eca)
#7 elf_locate_base gdb/solib-svr4.c:705 (gdb+0xec28ac)
#8 svr4_iterate_over_objfiles_in_search_order gdb/solib-svr4.c:3430 (gdb+0xeca55d)
#9 gdbarch_iterate_over_objfiles_in_search_order(gdbarch*, gdb::function_view<bool (objfile*)>, objfile*) gdb/gdbarch.c:5041 (gdb+0x537cad)
#10 find_main_name gdb/symtab.c:6270 (gdb+0xf743a5)
#11 main_language() gdb/symtab.c:6313 (gdb+0xf74499)
#12 set_initial_language() gdb/symfile.c:1700 (gdb+0xf4285c)
#13 symbol_file_add_main_1 gdb/symfile.c:1212 (gdb+0xf40e2a)
#14 symbol_file_command(char const*, int) gdb/symfile.c:1681 (gdb+0xf427d1)
#15 file_command gdb/exec.c:554 (gdb+0x94f74b)
#16 do_simple_func gdb/cli/cli-decode.c:95 (gdb+0x6d9528)
#17 cmd_func(cmd_list_element*, char const*, int) gdb/cli/cli-decode.c:2735 (gdb+0x6e0f69)
#18 execute_command(char const*, int) gdb/top.c:575 (gdb+0xff303c)
#19 command_handler(char const*) gdb/event-top.c:552 (gdb+0x94adde)
#20 command_line_handler(std::unique_ptr<char, gdb::xfree_deleter<char> >&&) gdb/event-top.c:788 (gdb+0x94b49b)
#21 tui_command_line_handler gdb/tui/tui-interp.c:104 (gdb+0x103479c)
#22 gdb_rl_callback_handler gdb/event-top.c:259 (gdb+0x94a383)
#23 rl_callback_read_char readline/readline/callback.c:290 (gdb+0x11bde5d)
#24 gdb_rl_callback_read_char_wrapper_noexcept gdb/event-top.c:195 (gdb+0x94a182)
#25 gdb_rl_callback_read_char_wrapper gdb/event-top.c:234 (gdb+0x94a243)
#26 stdin_event_handler gdb/ui.c:155 (gdb+0x1074a40)
#27 handle_file_event gdbsupport/event-loop.cc:573 (gdb+0x1d94f02)
#28 gdb_wait_for_event gdbsupport/event-loop.cc:694 (gdb+0x1d9563a)
#29 gdb_do_one_event(int) gdbsupport/event-loop.cc:264 (gdb+0x1d93a26)
#30 start_event_loop gdb/main.c:412 (gdb+0xb5a374)
#31 captured_command_loop gdb/main.c:476 (gdb+0xb5a563)
#32 captured_main gdb/main.c:1320 (gdb+0xb5c6e3)
#33 gdb_main(captured_main_args*) gdb/main.c:1339 (gdb+0xb5c792)
#34 main gdb/gdb.c:32 (gdb+0x416776)
Previous read of size 1 at 0x7b4400097d08 by thread T12:
#0 bfd_check_format_matches bfd/format.c:323 (gdb+0x1492db4)
#1 bfd_check_format bfd/format.c:94 (gdb+0x1492104)
#2 build_id_bfd_get(bfd*) gdb/build-id.c:42 (gdb+0x6648f7)
#3 index_cache::store(dwarf2_per_bfd*, index_cache_store_context*) gdb/dwarf2/index-cache.c:110 (gdb+0x82d205)
#4 cooked_index::maybe_write_index(dwarf2_per_bfd*) gdb/dwarf2/cooked-index.c:640 (gdb+0x7f1bf1)
#5 operator() gdb/dwarf2/cooked-index.c:470 (gdb+0x7f0f40)
#6 _M_invoke /usr/include/c++/7/bits/std_function.h:316 (gdb+0x7f28f7)
#7 std::function<void ()>::operator()() const /usr/include/c++/7/bits/std_function.h:706 (gdb+0x700952)
#8 void std::__invoke_impl<void, std::function<void ()>&>(std::__invoke_other, std::function<void ()>&) /usr/include/c++/7/bits/invoke.h:60 (gdb+0x7381a0)
#9 std::__invoke_result<std::function<void ()>&>::type std::__invoke<std::function<void ()>&>(std::function<void ()>&) /usr/include/c++/7/bits/invoke.h:95 (gdb+0x737e91)
#10 std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run()::{lambda()#1}::operator()() const /usr/include/c++/7/future:1421 (gdb+0x737b59)
#11 std::__future_base::_Task_setter<std::unique_ptr<std::__future_base::_Result<void>, std::__future_base::_Result_base::_Deleter>, std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run()::{lambda()#1}, void>::operator()() const /usr/include/c++/7/future:1362 (gdb+0x738660)
#12 std::_Function_handler<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> (), std::__future_base::_Task_setter<std::unique_ptr<std::__future_base::_Result<void>, std::__future_base::_Result_base::_Deleter>, std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run()::{lambda()#1}, void> >::_M_invoke(std::_Any_data const&) /usr/include/c++/7/bits/std_function.h:302 (gdb+0x73825c)
#13 std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>::operator()() const /usr/include/c++/7/bits/std_function.h:706 (gdb+0x733623)
#14 std::__future_base::_State_baseV2::_M_do_set(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*) /usr/include/c++/7/future:561 (gdb+0x732bdf)
#15 void std::__invoke_impl<void, void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::__invoke_memfun_deref, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&) /usr/include/c++/7/bits/invoke.h:73 (gdb+0x734c4f)
#16 std::__invoke_result<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>::type std::__invoke<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&) /usr/include/c++/7/bits/invoke.h:95 (gdb+0x733bc5)
#17 std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&)::{lambda()#1}::operator()() const /usr/include/c++/7/mutex:672 (gdb+0x73300d)
#18 std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&)::{lambda()#2}::operator()() const /usr/include/c++/7/mutex:677 (gdb+0x7330b2)
#19 std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&)::{lambda()#2}::_FUN() /usr/include/c++/7/mutex:677 (gdb+0x7330f2)
#20 pthread_once <null> (libtsan.so.0+0x4457c)
#21 __gthread_once /usr/include/c++/7/x86_64-suse-linux/bits/gthr-default.h:699 (gdb+0x72f5dd)
#22 void std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&) /usr/include/c++/7/mutex:684 (gdb+0x733224)
#23 std::__future_base::_State_baseV2::_M_set_result(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>, bool) /usr/include/c++/7/future:401 (gdb+0x732852)
#24 std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run() /usr/include/c++/7/future:1423 (gdb+0x737bef)
#25 std::packaged_task<void ()>::operator()() /usr/include/c++/7/future:1556 (gdb+0x1dac5b0)
#26 gdb::thread_pool::thread_function() gdbsupport/thread-pool.cc:242 (gdb+0x1dabed2)
#27 void std::__invoke_impl<void, void (gdb::thread_pool::*)(), gdb::thread_pool*>(std::__invoke_memfun_deref, void (gdb::thread_pool::*&&)(), gdb::thread_pool*&&) /usr/include/c++/7/bits/invoke.h:73 (gdb+0x1dacf81)
#28 std::__invoke_result<void (gdb::thread_pool::*)(), gdb::thread_pool*>::type std::__invoke<void (gdb::thread_pool::*)(), gdb::thread_pool*>(void (gdb::thread_pool::*&&)(), gdb::thread_pool*&&) /usr/include/c++/7/bits/invoke.h:95 (gdb+0x1dac3b2)
#29 decltype (__invoke((_S_declval<0ul>)(), (_S_declval<1ul>)())) std::thread::_Invoker<std::tuple<void (gdb::thread_pool::*)(), gdb::thread_pool*> >::_M_invoke<0ul, 1ul>(std::_Index_tuple<0ul, 1ul>) /usr/include/c++/7/thread:234 (gdb+0x1daf6e4)
#30 std::thread::_Invoker<std::tuple<void (gdb::thread_pool::*)(), gdb::thread_pool*> >::operator()() /usr/include/c++/7/thread:243 (gdb+0x1daf66f)
#31 std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (gdb::thread_pool::*)(), gdb::thread_pool*> > >::_M_run() /usr/include/c++/7/thread:186 (gdb+0x1daf624)
#32 <null> <null> (libstdc++.so.6+0xdcac2)
...
SUMMARY: ThreadSanitizer: data race bfd/cache.c:584 in bfd_open_file
...
The race happens when issuing the "file $exec" command.
The race is between:
- a worker thread getting the build id while writing the index cache, and in
the process reading bfd::format, and
- the main thread calling find_main_name, and in the process setting
bfd::cacheable.
The two bitfields bfd::cacheable and bfd::format share the same bitfield
container.
Fix this by capturing the build id in the main thread, and using the captured
value in the worker thread.
Likewise for the dwz build id, which likely suffers from the same issue.
While we're at it, also move the creation of the cache directory to
the index_cache_store_context constructor, to:
- make sure there's no race between subsequent file commands, and
- issue any related warning or error messages during the file command.
Tested on x86_64-linux.
PR symtab/30392
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30392
---
gdb/dwarf2/cooked-index.c | 2 +-
gdb/dwarf2/index-cache.c | 52 +++++++++++++++++++++++++--------------
gdb/dwarf2/index-cache.h | 9 ++++++-
3 files changed, 43 insertions(+), 20 deletions(-)
diff --git a/gdb/dwarf2/cooked-index.c b/gdb/dwarf2/cooked-index.c
index af6d129218d..b5718ae5529 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)
{
- struct index_cache_store_context ctx (global_index_cache);
+ struct index_cache_store_context ctx (global_index_cache, per_bfd);
/* This must be set after all the finalization tasks have been
started, because it may call 'wait'. */
diff --git a/gdb/dwarf2/index-cache.c b/gdb/dwarf2/index-cache.c
index 39109626d65..5281020d18a 100644
--- a/gdb/dwarf2/index-cache.c
+++ b/gdb/dwarf2/index-cache.c
@@ -88,18 +88,11 @@ index_cache::disable ()
/* See dwarf-index-cache.h. */
-index_cache_store_context::index_cache_store_context (const index_cache &ic)
+index_cache_store_context::index_cache_store_context (const index_cache &ic,
+ dwarf2_per_bfd *per_bfd)
{
m_enabled = ic.enabled ();
-}
-
-/* See dwarf-index-cache.h. */
-
-void
-index_cache::store (dwarf2_per_bfd *per_bfd,
- const struct index_cache_store_context &ctx)
-{
- if (!ctx.m_enabled)
+ if (!m_enabled)
return;
/* Get build id of objfile. */
@@ -108,15 +101,13 @@ index_cache::store (dwarf2_per_bfd *per_bfd,
{
index_cache_debug ("objfile %s has no build id",
bfd_get_filename (per_bfd->obfd));
+ m_enabled = false;
return;
}
-
- std::string build_id_str = build_id_to_string (build_id);
+ build_id_str = build_id_to_string (build_id);
/* Get build id of dwz file, if present. */
- gdb::optional<std::string> dwz_build_id_str;
const dwz_file *dwz = dwarf2_get_dwz_file (per_bfd);
- const char *dwz_build_id_ptr = NULL;
if (dwz != nullptr)
{
@@ -126,36 +117,61 @@ index_cache::store (dwarf2_per_bfd *per_bfd,
{
index_cache_debug ("dwz objfile %s has no build id",
dwz->filename ());
+ m_enabled = false;
return;
}
dwz_build_id_str = build_id_to_string (dwz_build_id);
- dwz_build_id_ptr = dwz_build_id_str->c_str ();
}
- if (m_dir.empty ())
+ if (ic.m_dir.empty ())
{
warning (_("The index cache directory name is empty, skipping store."));
+ m_enabled = false;
return;
}
try
{
/* Try to create the containing directory. */
- if (!mkdir_recursive (m_dir.c_str ()))
+ if (!mkdir_recursive (ic.m_dir.c_str ()))
{
warning (_("index cache: could not make cache directory: %s"),
safe_strerror (errno));
+ m_enabled = false;
return;
}
+ }
+ catch (const gdb_exception_error &except)
+ {
+ index_cache_debug ("couldn't store index cache for objfile %s: %s",
+ bfd_get_filename (per_bfd->obfd), except.what ());
+ m_enabled = false;
+ }
+}
+
+/* See dwarf-index-cache.h. */
+void
+index_cache::store (dwarf2_per_bfd *per_bfd,
+ const struct index_cache_store_context &ctx)
+{
+ if (!ctx.m_enabled)
+ return;
+
+ const char *dwz_build_id_ptr = (ctx.dwz_build_id_str.has_value ()
+ ? ctx.dwz_build_id_str->c_str ()
+ : nullptr);
+
+ try
+ {
index_cache_debug ("writing index cache for objfile %s",
bfd_get_filename (per_bfd->obfd));
/* Write the index itself to the directory, using the build id as the
filename. */
write_dwarf_index (per_bfd, m_dir.c_str (),
- build_id_str.c_str (), dwz_build_id_ptr,
+ ctx.build_id_str.c_str (), dwz_build_id_ptr,
dw_index_kind::GDB_INDEX);
}
catch (const gdb_exception_error &except)
diff --git a/gdb/dwarf2/index-cache.h b/gdb/dwarf2/index-cache.h
index 7f3c63198d5..6e224f47f93 100644
--- a/gdb/dwarf2/index-cache.h
+++ b/gdb/dwarf2/index-cache.h
@@ -42,17 +42,24 @@ struct index_cache_store_context
{
friend class index_cache;
- index_cache_store_context(const index_cache &ic);
+ index_cache_store_context(const index_cache &ic, dwarf2_per_bfd *);
private:
/* Captured value of enabled (). */
bool m_enabled;
+
+ /* Captured value of build id. */
+ std::string build_id_str;
+
+ /* Captured value of dwz build id. */
+ gdb::optional<std::string> dwz_build_id_str;
};
/* Class to manage the access to the DWARF index cache. */
class index_cache
{
+ friend struct index_cache_store_context;
public:
/* Change the directory used to save/load index files. */
void set_directory (std::string dir);
--
2.35.3
next prev parent reply other threads:[~2023-08-02 9:53 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-02 9:52 [PATCH v2 0/6] [gdb/symtab] Fix data-races in gdb.base/index-cache.exp Tom de Vries
2023-08-02 9:53 ` [PATCH v2 1/6] [gdb/symtab] Fix data race on index_cache::m_enabled Tom de Vries
2023-08-02 19:29 ` Tom Tromey
2023-08-04 0:08 ` Tom de Vries
2023-08-02 9:53 ` Tom de Vries [this message]
2023-08-02 19:32 ` [PATCH v2 2/6] [gdb/symtab] Fix data race on bfd::{cacheable,format} Tom Tromey
2023-08-04 0:09 ` Tom de Vries
2023-08-04 15:57 ` Tom Tromey
2023-08-02 9:53 ` [PATCH v2 3/6] [gdb/symtab] Fix race on dwarf2_per_cu_data::{queued,is_debug_type} Tom de Vries
2023-08-02 19:34 ` [PATCH v2 3/6] [gdb/symtab] Fix race on dwarf2_per_cu_data::{queued, is_debug_type} Tom Tromey
2023-08-02 9:53 ` [PATCH v2 4/6] [gdb/symtab] Fix data race on bfd_last_cache Tom de Vries
2023-08-02 19:37 ` Tom Tromey
2023-08-03 14:04 ` Tom de Vries
2023-08-02 9:53 ` [PATCH v2 5/6] [gdb/symtab] Fix data race on dwarf2_per_cu_data::{m_header_read_in,is_debug_type} Tom de Vries
2023-08-02 19:39 ` [PATCH v2 5/6] [gdb/symtab] Fix data race on dwarf2_per_cu_data::{m_header_read_in, is_debug_type} Tom Tromey
2023-08-02 9:53 ` [PATCH v2 6/6] [gdb/testsuite] Extend gdb.base/index-cache.exp Tom de Vries
2023-08-02 19:41 ` Tom Tromey
2023-08-02 19:44 ` [PATCH v2 0/6] [gdb/symtab] Fix data-races in gdb.base/index-cache.exp Tom Tromey
2023-08-04 0:14 ` Tom de Vries
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230802095305.3668-3-tdevries@suse.de \
--to=tdevries@suse.de \
--cc=gdb-patches@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).