From: Tom de Vries <tdevries@suse.de>
To: gdb-patches@sourceware.org
Subject: [RFC 2/3] [gdb/symtab] Fix data race in bfd_open_file
Date: Fri, 28 Jul 2023 10:56:44 +0200 [thread overview]
Message-ID: <20230728085645.3746-3-tdevries@suse.de> (raw)
In-Reply-To: <20230728085645.3746-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 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
next prev parent reply other threads:[~2023-07-28 8:57 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
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 ` Tom de Vries [this message]
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
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=20230728085645.3746-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).