From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out1.suse.de (smtp-out1.suse.de [IPv6:2001:67c:2178:6::1c]) by sourceware.org (Postfix) with ESMTPS id 394AE385841B for ; Fri, 28 Jul 2023 08:57:04 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 394AE385841B Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.de Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 4A516219E1 for ; Fri, 28 Jul 2023 08:57:02 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1690534622; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=0Vg6YIvZ+rkvbCxyerGmApOFCCupmSdGOKDwdm7CpOI=; b=cXWJ5lUMEcJ8mblSaegj0YV329BHgS1rkpGiBIetAu+OueuiFvAxuOumuFi7AfRL6Tc50j dtNWHvtnZBGcqQLvx0MCXlWgejsQO/QFSin0xeOd6WZkCNEJqHoyuNlmZnWihMjz4X9MKD 7SBx54NMj+xsAkSr4GmPO4YRe8xU7Zs= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1690534622; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=0Vg6YIvZ+rkvbCxyerGmApOFCCupmSdGOKDwdm7CpOI=; b=6PF1oxkhl8lQTTcaP/0Y2n085riBaGRd1lH3DjiIbkPJxytE/pNpVymZgD3Q3Ftl81ZCGX IwgRrsImIPOQrrBA== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 38094133F7 for ; Fri, 28 Jul 2023 08:57:02 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id UImSDN6Cw2QreAAAMHmgww (envelope-from ) for ; Fri, 28 Jul 2023 08:57:02 +0000 From: Tom de Vries 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 Message-Id: <20230728085645.3746-3-tdevries@suse.de> X-Mailer: git-send-email 2.35.3 In-Reply-To: <20230728085645.3746-1-tdevries@suse.de> References: <20230728085645.3746-1-tdevries@suse.de> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-11.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE,WEIRD_PORT autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: With gdb build with -fsanitize=3Dthread 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... =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D WARNING: ThreadSanitizer: data race (pid=3D12261) 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*) g= db/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 (gd= b+0xeca55d) #9 gdbarch_iterate_over_objfiles_in_search_order(gdbarch*, gdb::functio= n_view, 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+0xf42= 7d1) #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= >&&) 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+0x11bde= 5d) #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-i= ndex.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::operator()() const /usr/include/c++/7/bits/s= td_function.h:706 (gdb+0x700952) #8 void std::__invoke_impl&>(std::__invoke= _other, std::function&) /usr/include/c++/7/bits/invoke.h:60 (gdb+0= x7381a0) #9 std::__invoke_result&>::type std::__invoke&>(std::function&) /usr/include/c++/7/bits/inv= oke.h:95 (gdb+0x737e91) #10 std::__future_base::_Task_state, std::alloca= tor, void ()>::_M_run()::{lambda()#1}::operator()() const /usr/include= /c++/7/future:1421 (gdb+0x737b59) #11 std::__future_base::_Task_setter, std::__future_base::_Result_base::_Deleter>, std::__future= _base::_Task_state, std::allocator, void ()>::_= M_run()::{lambda()#1}, void>::operator()() const /usr/include/c++/7/future:= 1362 (gdb+0x738660) #12 std::_Function_handler (), std::__future_base::_= Task_setter, std::__futur= e_base::_Result_base::_Deleter>, std::__future_base::_Task_state, std::allocator, 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 ()>::operator()() const /usr/inclu= de/c++/7/bits/std_function.h:706 (gdb+0x733623) #14 std::__future_base::_State_baseV2::_M_do_set(std::function ()>*, bool*) /usr/include/c++/7/future:561 (gdb+0x732bdf) #15 void std::__invoke_impl ()>*, bool*), std::__future_base::_= State_baseV2*, std::function ()>*, bool*>(std::__invoke_= memfun_deref, void (std::__future_base::_State_baseV2::*&&)(std::function ()>*, bool*), std::__future_base::_State_baseV2*&&, std::= function ()>*&&, bool*&&) /usr/include/c++/7/bits/invoke= .h:73 (gdb+0x734c4f) #16 std::__invoke_result ()>*, bool*), std::__future_base::_State_bas= eV2*, std::function ()>*, bool*>::type std::__invoke (= )>*, bool*), std::__future_base::_State_baseV2*, std::function ()>*, bool*>(void (std::__future_base::_State_baseV2::*&&)(std::func= tion ()>*, bool*), std::__future_base::_State_baseV2*&&,= std::function ()>*&&, bool*&&) /usr/include/c++/7/bits/= invoke.h:95 (gdb+0x733bc5) #17 std::call_once ()>*, bool*), std::__future_base::_State_baseV2*, = std::function ()>*, bool*>(std::once_flag&, void (std::_= _future_base::_State_baseV2::*&&)(std::function ()>*, bo= ol*), std::__future_base::_State_baseV2*&&, std::function ()>*&&, bool*&&)::{lambda()#1}::operator()() const /usr/include/c++/7/mut= ex:672 (gdb+0x73300d) #18 std::call_once ()>*, bool*), std::__future_base::_State_baseV2*, = std::function ()>*, bool*>(std::once_flag&, void (std::_= _future_base::_State_baseV2::*&&)(std::function ()>*, bo= ol*), std::__future_base::_State_baseV2*&&, std::function ()>*&&, bool*&&)::{lambda()#2}::operator()() const /usr/include/c++/7/mut= ex:677 (gdb+0x7330b2) #19 std::call_once ()>*, bool*), std::__future_base::_State_baseV2*, = std::function ()>*, bool*>(std::once_flag&, void (std::_= _future_base::_State_baseV2::*&&)(std::function ()>*, bo= ol*), std::__future_base::_State_baseV2*&&, std::function ()>*&&, bool*&&)::{lambda()#2}::_FUN() /usr/include/c++/7/mutex:677 (gdb+= 0x7330f2) #20 pthread_once (libtsan.so.0+0x4457c) #21 __gthread_once /usr/include/c++/7/x86_64-suse-linux/bits/gthr-defau= lt.h:699 (gdb+0x72f5dd) #22 void std::call_once ()>*, bool*), std::__future_base::_State_base= V2*, std::function ()>*, bool*>(std::once_flag&, void (s= td::__future_base::_State_baseV2::*&&)(std::function ()>= *, bool*), std::__future_base::_State_baseV2*&&, std::function ()>*&&, bool*&&) /usr/include/c++/7/mutex:684 (gdb+0x733224) #23 std::__future_base::_State_baseV2::_M_set_result(std::function ()>, bool) /usr/include/c++/7/future:401 (gdb+0x732852) #24 std::__future_base::_Task_state, std::alloca= tor, void ()>::_M_run() /usr/include/c++/7/future:1423 (gdb+0x737bef) #25 std::packaged_task::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(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::type std::__invoke(voi= d (gdb::thread_pool::*&&)(), gdb::thread_pool*&&) /usr/include/c++/7/bits/i= nvoke.h:95 (gdb+0x1dac3b2) #29 decltype (__invoke((_S_declval<0ul>)(), (_S_declval<1ul>)())) std::= thread::_Invoker >::_M_invoke<0ul, 1ul>(std::_Index_tuple<0ul, 1ul>) /usr/include/c++/7/th= read:234 (gdb+0x1daf6e4) #30 std::thread::_Invoker >::operator()() /usr/include/c++/7/thread:243 (gdb+0x1daf66f) #31 std::thread::_State_impl > >::_M_run() /usr/include/c++/7/th= read:186 (gdb+0x1daf624) #32 (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=3D30392 --- 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 =3D global_index_cache.get_store_context (); + m_index_cache_store_context =3D global_index_cache.get_store_context (pe= r_bfd); =20 /* 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. */ =20 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 =3D new struct index_cache_store_context; =20 res->enabled =3D enabled (); - return res; -} - -/* See dwarf-index-cache.h. */ - -void -index_cache::store (dwarf2_per_bfd *per_bfd, struct index_cache_store_cont= ext *ctx) -{ - if (!ctx->enabled) - return; + if (!res->enabled) + return res; =20 /* Get build id of objfile. */ const bfd_build_id *build_id =3D build_id_bfd_get (per_bfd->obfd); @@ -112,15 +104,13 @@ index_cache::store (dwarf2_per_bfd *per_bfd, struct i= ndex_cache_store_context *c { index_cache_debug ("objfile %s has no build id", bfd_get_filename (per_bfd->obfd)); - return; + res->enabled =3D false; + return res; } - - std::string build_id_str =3D build_id_to_string (build_id); + res->build_id_str =3D build_id_to_string (build_id); =20 /* Get build id of dwz file, if present. */ - gdb::optional dwz_build_id_str; const dwz_file *dwz =3D dwarf2_get_dwz_file (per_bfd); - const char *dwz_build_id_ptr =3D NULL; =20 if (dwz !=3D nullptr) { @@ -130,17 +120,18 @@ index_cache::store (dwarf2_per_bfd *per_bfd, struct i= ndex_cache_store_context *c { index_cache_debug ("dwz objfile %s has no build id", dwz->filename ()); - return; + res->enabled =3D false; + return res; } =20 - dwz_build_id_str =3D build_id_to_string (dwz_build_id); - dwz_build_id_ptr =3D dwz_build_id_str->c_str (); + res->dwz_build_id_str =3D build_id_to_string (dwz_build_id); } =20 if (m_dir.empty ()) { warning (_("The index cache directory name is empty, skipping store.= ")); - return; + res->enabled =3D false; + return res; } =20 try @@ -150,16 +141,41 @@ index_cache::store (dwarf2_per_bfd *per_bfd, struct i= ndex_cache_store_context *c { warning (_("index cache: could not make cache directory: %s"), safe_strerror (errno)); - return; + res->enabled =3D 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 =3D false; + } + + return res; +} + +/* See dwarf-index-cache.h. */ + +void +index_cache::store (dwarf2_per_bfd *per_bfd, struct index_cache_store_cont= ext *ctx) +{ + if (!ctx->enabled) + return; + + const char *dwz_build_id_ptr =3D (ctx->dwz_build_id_str.has_value () + ? ctx->dwz_build_id_str->c_str () + : nullptr); =20 + try + { index_cache_debug ("writing index cache for objfile %s", bfd_get_filename (per_bfd->obfd)); =20 /* 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 dwz_build_id_str; }; =20 /* Class to manage the access to the DWARF index cache. */ @@ -64,7 +70,7 @@ class index_cache void disable (); =20 /* 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); =20 /* Store an index for the specified object file in the cache. */ void store (dwarf2_per_bfd *per_bfd, struct index_cache_store_context *); --=20 2.35.3