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 831FF3858D37 for ; Wed, 2 Aug 2023 09:53:25 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 831FF3858D37 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 B545A21B3B 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=2E10ePgb8RdxjN7LMUGKV0dAudnDZzbs9CszuUvANIo=; b=jwrkl/Gd4s/Njohjz6R9fY2W1VMd4uIoULSDLMtOOxPdqViEx4mD9UZHRm+isSNQ+D3ah9 zsNn3WuDNpW7fYhjKuyG4yD6GZ++mLbjqMncD6NslwMUSQ183fkvgB+JE5M8Zo6HkEi4BG 5rGbDCL09DiOuUdK3r5Ez7SFoKvEPmA= 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=2E10ePgb8RdxjN7LMUGKV0dAudnDZzbs9CszuUvANIo=; b=PQcV9pIglJTfBFanTcL3/6qDIyZU0UH8hPfCXuYbNxyc7wD92rqdhZfNAJ5QQGYyEcpYg6 BIPnxqk6RK5L4zBg== 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 A2E911391A 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 MHm8JpQnymSscgAAMHmgww (envelope-from ) for ; Wed, 02 Aug 2023 09:53:24 +0000 From: Tom de Vries To: gdb-patches@sourceware.org Subject: [PATCH v2 1/6] [gdb/symtab] Fix data race on index_cache::m_enabled Date: Wed, 2 Aug 2023 11:53:00 +0200 Message-Id: <20230802095305.3668-2-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... (gdb) show index-cache enabled The index cache is off. (gdb) PASS: gdb.base/index-cache.exp: test_basic_stuff: index-cache is disa= bled by default set index-cache enabled on =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D WARNING: ThreadSanitizer: data race (pid=3D32248) Write of size 1 at 0x00000321f540 by main thread: #0 index_cache::enable() gdb/dwarf2/index-cache.c:76 (gdb+0x82cfdd) #1 set_index_cache_enabled_command gdb/dwarf2/index-cache.c:270 (gdb+0x= 82d9af) #2 bool setting::set(bool const&) gdb/command.h:353 (gdb+0x6fe5f2) #3 do_set_command(char const*, int, cmd_list_element*) gdb/cli/cli-sets= how.c:414 (gdb+0x6fcd21) #4 execute_command(char const*, int) gdb/top.c:567 (gdb+0xff2e64) #5 command_handler(char const*) gdb/event-top.c:552 (gdb+0x94acc0) #6 command_line_handler(std::unique_ptr = >&&) gdb/event-top.c:788 (gdb+0x94b37d) #7 tui_command_line_handler gdb/tui/tui-interp.c:104 (gdb+0x103467e) #8 gdb_rl_callback_handler gdb/event-top.c:259 (gdb+0x94a265) #9 rl_callback_read_char readline/readline/callback.c:290 (gdb+0x11bdd3= f) #10 gdb_rl_callback_read_char_wrapper_noexcept gdb/event-top.c:195 (gdb= +0x94a064) #11 gdb_rl_callback_read_char_wrapper gdb/event-top.c:234 (gdb+0x94a125) #12 stdin_event_handler gdb/ui.c:155 (gdb+0x1074922) #13 handle_file_event gdbsupport/event-loop.cc:573 (gdb+0x1d94de4) #14 gdb_wait_for_event gdbsupport/event-loop.cc:694 (gdb+0x1d9551c) #15 gdb_do_one_event(int) gdbsupport/event-loop.cc:264 (gdb+0x1d93908) #16 start_event_loop gdb/main.c:412 (gdb+0xb5a256) #17 captured_command_loop gdb/main.c:476 (gdb+0xb5a445) #18 captured_main gdb/main.c:1320 (gdb+0xb5c5c5) #19 gdb_main(captured_main_args*) gdb/main.c:1339 (gdb+0xb5c674) #20 main gdb/gdb.c:32 (gdb+0x416776) Previous read of size 1 at 0x00000321f540 by thread T12: #0 index_cache::enabled() const gdb/dwarf2/index-cache.h:48 (gdb+0x82e1= a6) #1 index_cache::store(dwarf2_per_bfd*) gdb/dwarf2/index-cache.c:94 (gdb= +0x82d0bc) #2 cooked_index::maybe_write_index(dwarf2_per_bfd*) gdb/dwarf2/cooked-i= ndex.c:638 (gdb+0x7f1b97) #3 operator() gdb/dwarf2/cooked-index.c:468 (gdb+0x7f0f24) #4 _M_invoke /usr/include/c++/7/bits/std_function.h:316 (gdb+0x7f285b) #5 std::function::operator()() const /usr/include/c++/7/bits/s= td_function.h:706 (gdb+0x700952) #6 void std::__invoke_impl&>(std::__invoke= _other, std::function&) /usr/include/c++/7/bits/invoke.h:60 (gdb+0= x7381a0) #7 std::__invoke_result&>::type std::__invoke&>(std::function&) /usr/include/c++/7/bits/inv= oke.h:95 (gdb+0x737e91) #8 std::__future_base::_Task_state, std::allocat= or, void ()>::_M_run()::{lambda()#1}::operator()() const /usr/include/= c++/7/future:1421 (gdb+0x737b59) #9 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:1= 362 (gdb+0x738660) #10 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) #11 std::function ()>::operator()() const /usr/inclu= de/c++/7/bits/std_function.h:706 (gdb+0x733623) #12 std::__future_base::_State_baseV2::_M_do_set(std::function ()>*, bool*) /usr/include/c++/7/future:561 (gdb+0x732bdf) #13 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) #14 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) #15 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) #16 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) #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()#2}::_FUN() /usr/include/c++/7/mutex:677 (gdb+= 0x7330f2) #18 pthread_once (libtsan.so.0+0x4457c) #19 __gthread_once /usr/include/c++/7/x86_64-suse-linux/bits/gthr-defau= lt.h:699 (gdb+0x72f5dd) #20 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) #21 std::__future_base::_State_baseV2::_M_set_result(std::function ()>, bool) /usr/include/c++/7/future:401 (gdb+0x732852) #22 std::__future_base::_Task_state, std::alloca= tor, void ()>::_M_run() /usr/include/c++/7/future:1423 (gdb+0x737bef) #23 std::packaged_task::operator()() /usr/include/c++/7/future= :1556 (gdb+0x1dac492) #24 gdb::thread_pool::thread_function() gdbsupport/thread-pool.cc:242 (= gdb+0x1dabdb4) #25 void std::__invoke_impl(std::__invoke_memfun_deref, void (gdb::thread_pool::*&&)(), gdb= ::thread_pool*&&) /usr/include/c++/7/bits/invoke.h:73 (gdb+0x1dace63) #26 std::__invoke_result::type std::__invoke(voi= d (gdb::thread_pool::*&&)(), gdb::thread_pool*&&) /usr/include/c++/7/bits/i= nvoke.h:95 (gdb+0x1dac294) #27 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+0x1daf5c6) #28 std::thread::_Invoker >::operator()() /usr/include/c++/7/thread:243 (gdb+0x1daf551) #29 std::thread::_State_impl > >::_M_run() /usr/include/c++/7/th= read:186 (gdb+0x1daf506) #30 (libstdc++.so.6+0xdcac2) Location is global 'global_index_cache' of size 48 at 0x00000321f520 (gdb= +0x00000321f540) ... SUMMARY: ThreadSanitizer: data race gdb/dwarf2/index-cache.c:76 in index_ca= che::enable() ... The race happens when issuing a "file $exec" command followed by a "set index-cache enabled on" command. The race is between: - a worker thread reading index_cache::m_enabled to determine whether an index-cache entry for $exec needs to be written (due to command "file $exec"), and - the main thread setting index_cache::m_enabled (due to command "set index-cache enabled on"). Fix this by capturing the value of index_cache::m_enabled in the main threa= d, and using the captured value in the worker thread. Tested on x86_64-linux. PR symtab/30392 Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=3D30392 --- gdb/dwarf2/cooked-index.c | 12 ++++++++---- gdb/dwarf2/cooked-index.h | 3 ++- gdb/dwarf2/index-cache.c | 12 ++++++++++-- gdb/dwarf2/index-cache.h | 18 +++++++++++++++++- 4 files changed, 37 insertions(+), 8 deletions(-) diff --git a/gdb/dwarf2/cooked-index.c b/gdb/dwarf2/cooked-index.c index 25635d9b72e..af6d129218d 100644 --- a/gdb/dwarf2/cooked-index.c +++ b/gdb/dwarf2/cooked-index.c @@ -460,12 +460,15 @@ 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); + /* This must be set after all the finalization tasks have been started, because it may call 'wait'. */ m_write_future - =3D gdb::thread_pool::g_thread_pool->post_task ([this, per_bfd] () + =3D gdb::thread_pool::g_thread_pool->post_task ([this, per_bfd, + ctx =3D std::move (ctx)] () { - maybe_write_index (per_bfd); + maybe_write_index (per_bfd, ctx); }); } =20 @@ -629,13 +632,14 @@ cooked_index::dump (gdbarch *arch) const } =20 void -cooked_index::maybe_write_index (dwarf2_per_bfd *per_bfd) +cooked_index::maybe_write_index (dwarf2_per_bfd *per_bfd, + const struct index_cache_store_context &ctx) { /* Wait for finalization. */ wait (); =20 /* (maybe) store an index in the cache. */ - global_index_cache.store (per_bfd); + global_index_cache.store (per_bfd, ctx); } =20 /* Wait for all the index cache entries to be written before gdb diff --git a/gdb/dwarf2/cooked-index.h b/gdb/dwarf2/cooked-index.h index 0d6f3e5aa0e..d26d7ad4b1d 100644 --- a/gdb/dwarf2/cooked-index.h +++ b/gdb/dwarf2/cooked-index.h @@ -435,7 +435,8 @@ class cooked_index : public dwarf_scanner_base private: =20 /* Maybe write the index to the index cache. */ - void maybe_write_index (dwarf2_per_bfd *per_bfd); + void maybe_write_index (dwarf2_per_bfd *per_bfd, + const struct index_cache_store_context &); =20 /* The vector of cooked_index objects. This is stored because the entries are stored on the obstacks in those objects. */ diff --git a/gdb/dwarf2/index-cache.c b/gdb/dwarf2/index-cache.c index 79ab706ee9d..39109626d65 100644 --- a/gdb/dwarf2/index-cache.c +++ b/gdb/dwarf2/index-cache.c @@ -88,10 +88,18 @@ index_cache::disable () =20 /* See dwarf-index-cache.h. */ =20 +index_cache_store_context::index_cache_store_context (const index_cache &i= c) +{ + m_enabled =3D ic.enabled (); +} + +/* See dwarf-index-cache.h. */ + void -index_cache::store (dwarf2_per_bfd *per_bfd) +index_cache::store (dwarf2_per_bfd *per_bfd, + const struct index_cache_store_context &ctx) { - if (!enabled ()) + if (!ctx.m_enabled) return; =20 /* Get build id of objfile. */ diff --git a/gdb/dwarf2/index-cache.h b/gdb/dwarf2/index-cache.h index 1efff17049f..7f3c63198d5 100644 --- a/gdb/dwarf2/index-cache.h +++ b/gdb/dwarf2/index-cache.h @@ -25,6 +25,7 @@ #include "symfile.h" =20 class dwarf2_per_bfd; +class index_cache; =20 /* Base of the classes used to hold the resources of the indices loaded fr= om the cache (e.g. mmapped files). */ @@ -34,6 +35,20 @@ struct index_cache_resource virtual ~index_cache_resource () =3D 0; }; =20 +/* Information to be captured in the main thread, and to be used by worker + threads during store (). */ + +struct index_cache_store_context +{ + friend class index_cache; + + index_cache_store_context(const index_cache &ic); + +private: + /* Captured value of enabled (). */ + bool m_enabled; +}; + /* Class to manage the access to the DWARF index cache. */ =20 class index_cache @@ -55,7 +70,8 @@ class index_cache void disable (); =20 /* Store an index for the specified object file in the cache. */ - void store (dwarf2_per_bfd *per_bfd); + void store (dwarf2_per_bfd *per_bfd, + const struct index_cache_store_context &); =20 /* Look for an index file matching BUILD_ID. If found, return the conte= nts as an array_view and store the underlying resources (allocated memory, --=20 2.35.3