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 360B13858CDA for ; Fri, 28 Jul 2023 08:57:04 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 360B13858CDA 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 1B97321992 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; bh=ZraSSAT3dcjQes2s7U+OscKyNV1swtGYIQ2uDfL6bx4=; b=D0MOehyZ2BsT72myAVx/4mkCzCTSFW6NM9NQYOY2jFSX2bUPItF9JPEPaKveMUrvoERAsS TVF1/AAr45MUYOBcoA25C9GI262krU8r//yECkKkw4hRtXNCnz12owJ6v8yNvGeXXJVUTa mWeSa5apO6sRpS3Ru1PkzU/3ezXx2nQ= 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; bh=ZraSSAT3dcjQes2s7U+OscKyNV1swtGYIQ2uDfL6bx4=; b=RvC0Ux1R1Xt7yco4ZaVKsic7iuWpMJ/rUsnu0uKiyaBJM2UajdIFJEIe5iOE+89F/atgDn lEcMhx8HWoyA94DQ== 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 07B5A133F7 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 4PXFAN6Cw2QreAAAMHmgww (envelope-from ) for ; Fri, 28 Jul 2023 08:57:02 +0000 From: Tom de Vries To: gdb-patches@sourceware.org Subject: [RFC 0/3] [gdb/symtab] Fix data-races in gdb.base/index-cache.exp Date: Fri, 28 Jul 2023 10:56:42 +0200 Message-Id: <20230728085645.3746-1-tdevries@suse.de> X-Mailer: git-send-email 2.35.3 MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-5.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,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: When building gdb with -fsanitize=3Dthread, we run into a data race in gdb.base/index-cache.exp. Fixing this leads us to another, and another, so there are three patches, e= ach fixing a data race. The first two patches implement the approach mentioned in PR30392 comment 2: ... The reader probably should capture the necessarily globals on the main thread and stash them until the index has been written. ... The 3rd patch cannot be fixed with this approach, so it uses the std::atomic approach: ... - unsigned int queued : 1; + std::atomic queued; ... The 3rd patch is the reason this is an RFC. I spent some time convincing myself that this is not a benign data race. I started with reading [1], and got convinced by "2.5 Disjoint bit manipulati= on" in [2]. Also [3] looked interesting, but haven't read it in full. As for fixing it, the std::atomic approach works. I checked that the struct size is not increased. It gives me an almost clean test-suite run (still running into PR30680), though I'm running into a lot of timeouts, so things still may be hidden there. Alternatively, if I limit the fix to: ... - unsigned int queued : 1; + bool queued; ... I run into 19 more reported data races in the test-case. None of them look as evidence that this fix on queued is insufficient. Looking at the first one (see appendix for more detail), there's a race between: - the main thread calling find_main_name, because of calling main_language = in set_initial_language, and - the worker thread calling find_main_name, because of calling main_name in write_cooked_index. At least 14 other of the 19 reported data races look like instances of the first one. This looks like a real problem to me, but with the proposed fix it no longer triggers, although I fail to see how it fixes it. Which leaves me with the questions: - is the std::atomic approach too blunt, and are we consequently hiding problems, and - should we address the find_main_name race in some form or another. Tested on x86_64-linux, with and without -fsanitize=3Dthread. PR symtab/30392 Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=3D30392 References: [1] https://hacks.mozilla.org/2021/04/eliminating-data-races-in-firefox-a-t= echnical-report/ [2] https://www.usenix.org/legacy/event/hotpar11/tech/final_files/Boehm.pdf [3] https://bartoszmilewski.com/2020/08/11/benign-data-races-considered-har= mful/ Appendix: ... (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=3D1569) Read of size 1 at 0x7b4400097d08 by thread T12: #0 bfd_get_section_limit_octets bfd.h:2376 (gdb+0x149b613) #1 bfd_get_section_contents bfd/section.c:1571 (gdb+0x149cbe4) #2 gdb_bfd_scan_elf_dyntag(int, bfd*, unsigned long*, unsigned long*) g= db/solib.c:1601 (gdb+0xed8fc8) #3 elf_locate_base gdb/solib-svr4.c:705 (gdb+0xec29aa) #4 svr4_iterate_over_objfiles_in_search_order gdb/solib-svr4.c:3430 (gd= b+0xeca65b) #5 gdbarch_iterate_over_objfiles_in_search_order(gdbarch*, gdb::functio= n_view, objfile*) gdb/gdbarch.c:5041 (gdb+0x537cad) #6 find_main_name gdb/symtab.c:6270 (gdb+0xf744a3) #7 main_name() gdb/symtab.c:6299 (gdb+0xf74531) #8 write_cooked_index gdb/dwarf2/index-write.c:1131 (gdb+0x830af4) #9 write_gdbindex gdb/dwarf2/index-write.c:1256 (gdb+0x83134b) #10 write_dwarf_index(dwarf2_per_bfd*, char const*, char const*, char c= onst*, dw_index_kind) gdb/dwarf2/index-write.c:1484 (gdb+0x83232f) #11 index_cache::store(dwarf2_per_bfd*, index_cache_store_context*) gdb= /dwarf2/index-cache.c:177 (gdb+0x82d62b) #12 cooked_index::maybe_write_index(dwarf2_per_bfd*) gdb/dwarf2/cooked-= index.c:640 (gdb+0x7f1bf7) #13 operator() gdb/dwarf2/cooked-index.c:470 (gdb+0x7f0f40) #14 _M_invoke /usr/include/c++/7/bits/std_function.h:316 (gdb+0x7f2909) #15 std::function::operator()() const /usr/include/c++/7/bits/= std_function.h:706 (gdb+0x700952) #16 void std::__invoke_impl&>(std::__invok= e_other, std::function&) /usr/include/c++/7/bits/invoke.h:60 (gdb+= 0x7381a0) #17 std::__invoke_result&>::type std::__invoke&>(std::function&) /usr/include/c++/7/bits/in= voke.h:95 (gdb+0x737e91) #18 std::__future_base::_Task_state, std::alloca= tor, void ()>::_M_run()::{lambda()#1}::operator()() const /usr/include= /c++/7/future:1421 (gdb+0x737b59) #19 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) #20 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) #21 std::function ()>::operator()() const /usr/inclu= de/c++/7/bits/std_function.h:706 (gdb+0x733623) #22 std::__future_base::_State_baseV2::_M_do_set(std::function ()>*, bool*) /usr/include/c++/7/future:561 (gdb+0x732bdf) #23 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) #24 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) #25 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) #26 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) #27 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) #28 pthread_once (libtsan.so.0+0x4457c) #29 __gthread_once /usr/include/c++/7/x86_64-suse-linux/bits/gthr-defau= lt.h:699 (gdb+0x72f5dd) #30 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) #31 std::__future_base::_State_baseV2::_M_set_result(std::function ()>, bool) /usr/include/c++/7/future:401 (gdb+0x732852) #32 std::__future_base::_Task_state, std::alloca= tor, void ()>::_M_run() /usr/include/c++/7/future:1423 (gdb+0x737bef) #33 std::packaged_task::operator()() /usr/include/c++/7/future= :1556 (gdb+0x1daca1c) #34 gdb::thread_pool::thread_function() gdbsupport/thread-pool.cc:242 (= gdb+0x1dac33e) #35 void std::__invoke_impl(std::__invoke_memfun_deref, void (gdb::thread_pool::*&&)(), gdb= ::thread_pool*&&) /usr/include/c++/7/bits/invoke.h:73 (gdb+0x1dad3ed) #36 std::__invoke_result::type std::__invoke(voi= d (gdb::thread_pool::*&&)(), gdb::thread_pool*&&) /usr/include/c++/7/bits/i= nvoke.h:95 (gdb+0x1dac81e) #37 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+0x1dafb50) #38 std::thread::_Invoker >::operator()() /usr/include/c++/7/thread:243 (gdb+0x1dafadb) #39 std::thread::_State_impl > >::_M_run() /usr/include/c++/7/th= read:186 (gdb+0x1dafa90) #40 (libstdc++.so.6+0xdcac2) Previous write of size 4 at 0x7b4400097d08 by main thread: #0 bfd_open_file bfd/cache.c:584 (gdb+0x148bc90) #1 bfd_cache_lookup_worker bfd/cache.c:261 (gdb+0x148b228) #2 cache_bseek bfd/cache.c:289 (gdb+0x148b422) #3 bfd_seek bfd/bfdio.c:459 (gdb+0x1489d2f) #4 _bfd_generic_get_section_contents bfd/libbfd.c:1069 (gdb+0x14978a2) #5 bfd_get_section_contents bfd/section.c:1606 (gdb+0x149cd7a) #6 gdb_bfd_scan_elf_dyntag(int, bfd*, unsigned long*, unsigned long*) g= db/solib.c:1601 (gdb+0xed8fc8) #7 elf_locate_base gdb/solib-svr4.c:705 (gdb+0xec29aa) #8 svr4_iterate_over_objfiles_in_search_order gdb/solib-svr4.c:3430 (gd= b+0xeca65b) #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+0xf744a3) #11 main_language() gdb/symtab.c:6313 (gdb+0xf74597) #12 set_initial_language() gdb/symfile.c:1700 (gdb+0xf4295a) #13 symbol_file_add_main_1 gdb/symfile.c:1212 (gdb+0xf40f28) #14 symbol_file_command(char const*, int) gdb/symfile.c:1681 (gdb+0xf42= 8cf) #15 file_command gdb/exec.c:554 (gdb+0x94f849) #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+0xff313a) #19 command_handler(char const*) gdb/event-top.c:552 (gdb+0x94aedc) #20 command_line_handler(std::unique_ptr= >&&) gdb/event-top.c:788 (gdb+0x94b599) #21 tui_command_line_handler gdb/tui/tui-interp.c:104 (gdb+0x103489a) #22 gdb_rl_callback_handler gdb/event-top.c:259 (gdb+0x94a481) #23 rl_callback_read_char readline/readline/callback.c:290 (gdb+0x11bdf= 5b) #24 gdb_rl_callback_read_char_wrapper_noexcept gdb/event-top.c:195 (gdb= +0x94a280) #25 gdb_rl_callback_read_char_wrapper gdb/event-top.c:234 (gdb+0x94a341) #26 stdin_event_handler gdb/ui.c:155 (gdb+0x1074b3e) #27 handle_file_event gdbsupport/event-loop.cc:573 (gdb+0x1d9536e) #28 gdb_wait_for_event gdbsupport/event-loop.cc:694 (gdb+0x1d95aa6) #29 gdb_do_one_event(int) gdbsupport/event-loop.cc:264 (gdb+0x1d93e92) #30 start_event_loop gdb/main.c:412 (gdb+0xb5a472) #31 captured_command_loop gdb/main.c:476 (gdb+0xb5a661) #32 captured_main gdb/main.c:1320 (gdb+0xb5c7e1) #33 gdb_main(captured_main_args*) gdb/main.c:1339 (gdb+0xb5c890) #34 main gdb/gdb.c:32 (gdb+0x416776) ... SUMMARY: ThreadSanitizer: data race bfd.h:2376 in bfd_get_section_limit_oct= ets ... Tom de Vries (3): [gdb/symtab] Fix data race in index_cache::enable [gdb/symtab] Fix data race in bfd_open_file [gdb/symtab] Fix race on per_cu->queued gdb/dwarf2/cooked-index.c | 8 ++++-- gdb/dwarf2/cooked-index.h | 3 ++ gdb/dwarf2/index-cache.c | 58 +++++++++++++++++++++++++++++---------- gdb/dwarf2/index-cache.h | 20 +++++++++++++- gdb/dwarf2/read.h | 2 +- 5 files changed, 72 insertions(+), 19 deletions(-) base-commit: 29c108c9610640439daa5244a573348b7c47d994 --=20 2.35.3