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 9A70A3858D39 for ; Wed, 2 Aug 2023 09:53:25 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 9A70A3858D39 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 CC86B21B51 for ; Wed, 2 Aug 2023 09:53:24 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1690970004; 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=6NlfGBOpthyLZeumUbtODZn6xKjPvqedYFQ6BVpjOg8=; b=fcylk5FoSnKJSK9e3MIYmV8C/IN+87cYVsunBDPnuWUJQsgRN5XT1UXHn/Q72KuYuxgf9G loLQ1YrrQOclRLn4kuettdyJCeu12ZE8YT9BBaxpOmuBAXnE3jzoWjcTYdxbhBwg0/IGug 8prZsGu0nr/QrRQOwgMQ0mI6Y0MQeEI= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1690970004; 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=6NlfGBOpthyLZeumUbtODZn6xKjPvqedYFQ6BVpjOg8=; b=FYGpmDeL5Vw8T10mm/ti7bQZCPn79VsRoH7KEdA+hG7WYnNj2CmWX6c34pdjwrS3xDboZk 6HxtlXoben2tPZBw== 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 BA55413909 for ; Wed, 2 Aug 2023 09:53:24 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id aPVmLJQnymSscgAAMHmgww (envelope-from ) for ; Wed, 02 Aug 2023 09:53:24 +0000 From: Tom de Vries 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 Message-Id: <20230802095305.3668-3-tdevries@suse.de> X-Mailer: git-send-email 2.35.3 In-Reply-To: <20230802095305.3668-1-tdevries@suse.de> References: <20230802095305.3668-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 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 captur= ed 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=3D30392 --- 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); =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 39109626d65..5281020d18a 100644 --- a/gdb/dwarf2/index-cache.c +++ b/gdb/dwarf2/index-cache.c @@ -88,18 +88,11 @@ index_cache::disable () =20 /* See dwarf-index-cache.h. */ =20 -index_cache_store_context::index_cache_store_context (const index_cache &i= c) +index_cache_store_context::index_cache_store_context (const index_cache &i= c, + dwarf2_per_bfd *per_bfd) { m_enabled =3D 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; =20 /* 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 =3D false; return; } - - std::string build_id_str =3D build_id_to_string (build_id); + 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) { @@ -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 =3D false; return; } =20 dwz_build_id_str =3D build_id_to_string (dwz_build_id); - dwz_build_id_ptr =3D dwz_build_id_str->c_str (); } =20 - if (m_dir.empty ()) + if (ic.m_dir.empty ()) { warning (_("The index cache directory name is empty, skipping store.= ")); + m_enabled =3D false; return; } =20 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 =3D 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 =3D false; + } +} + +/* See dwarf-index-cache.h. */ =20 +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 =3D (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)); =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 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; =20 - index_cache_store_context(const index_cache &ic); + index_cache_store_context(const index_cache &ic, dwarf2_per_bfd *); =20 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 dwz_build_id_str; }; =20 /* Class to manage the access to the DWARF index cache. */ =20 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); --=20 2.35.3