Bug 31715

Summary: race condition in cooked_index_shard::finalize
Product: gdb Reporter: Bernd Edlinger <bernd.edlinger>
Component: gdbAssignee: Not yet assigned to anyone <unassigned>
Status: RESOLVED FIXED    
Severity: normal CC: sam, tromey, vries
Priority: P2    
Version: HEAD   
Target Milestone: 16.1   
See Also: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110799
Host: Target:
Build: Last reconfirmed:
Bug Depends on:    
Bug Blocks: 31751    
Attachments: tsan output from gdb.base/align-c++.exp
tsan output from gdb.base/align-c.exp
tsan output from gdb.base/batch-preserve-term-settings.exp
tsan output from gdb.base/break.exp

Description Bernd Edlinger 2024-05-09 09:35:19 UTC 
There are a number of test cases where a thread sanitizer
built on host=x86_64-linux-gnu, using gcc 12.2.0 (Debian 12.2.0-14),
reports race conditons.
gdb.base/align-c++.exp
gdb.base/align-c.exp
gdb.base/batch-preserve-term-settings.exp
gdb.base/break.exp (several times)
gdb.base/cached-source-file.exp
gdb.base/chng-syms.exp
... etc. etc.
Comment 1 Bernd Edlinger 2024-05-09 09:36:23 UTC 
Created attachment 15503 [details]
tsan output from gdb.base/align-c++.exp
Comment 2 Bernd Edlinger 2024-05-09 09:37:03 UTC 
Created attachment 15504 [details]
tsan output from gdb.base/align-c.exp
Comment 3 Bernd Edlinger 2024-05-09 09:38:57 UTC 
Created attachment 15505 [details]
tsan output from gdb.base/batch-preserve-term-settings.exp
Comment 4 Bernd Edlinger 2024-05-09 09:43:00 UTC 
Created attachment 15506 [details]
tsan output from gdb.base/break.exp

These are some samples how the tsan reports look like
They are all very similar to those.
Comment 5 Tom Tromey 2024-05-17 15:21:12 UTC 
Can you try the dwarf synchronous change for this one?
https://sourceware.org/pipermail/gdb-patches/2024-May/209267.html
Comment 6 Bernd Edlinger 2024-05-18 14:17:27 UTC 
Confirmed.
The reported race conditions are no longer reproducible with your patch.
Comment 7 Tom Tromey 2024-09-30 19:57:50 UTC 
I suspect from the stack trace that this is happening
because cooked_index::full_name does:

  const char *local_name = for_main ? name : canonical;

but the finalizing code sets canonical:

		entry->canonical = entry->name;

However I think this seems like an erroneous report because
canonical shouldn't be usable from the "main" path, because
that code always passes for_main=true:

  return entry->full_name (obstack, true);
Comment 8 Bernd Edlinger 2024-10-03 13:42:44 UTC 
This is one sample of that trace with your latest patch applied:

(gdb) set width 0
(gdb) dir
Reinitialize source path to empty? (y or n) y
Source directories searched: $cdir:$cwd
(gdb) dir /home/ed/gnu/gdb-build-y/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.base
Source directories searched: /home/ed/gnu/gdb-build-y/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.base:$cdir:$cwd
(gdb) kill
The program is not being run.
(gdb) file /home/ed/gnu/gdb-build-y/gdb/testsuite/outputs/gdb.base/annota1/annota1
Reading symbols from /home/ed/gnu/gdb-build-y/gdb/testsuite/outputs/gdb.base/annota1/annota1...
(gdb) set height 0
==================
WARNING: ThreadSanitizer: data race (pid=29546)
  Read of size 8 at 0x7b8000020288 by main thread:
    #0 cooked_index_entry::full_name(obstack*, bool) const ../../binutils-gdb/gdb/dwarf2/cooked-index.c:220 (gdb+0x31c114)
    #1 cooked_index::get_main_name(obstack*, language*) const ../../binutils-gdb/gdb/dwarf2/cooked-index.c:735 (gdb+0x31d549)
    #2 cooked_index_worker::wait(cooked_state, bool) ../../binutils-gdb/gdb/dwarf2/cooked-index.c:559 (gdb+0x31d549)
    #3 cooked_index::wait(cooked_state, bool) ../../binutils-gdb/gdb/dwarf2/cooked-index.c:631 (gdb+0x31e085)
    #4 cooked_index_functions::wait(objfile*, bool) ../../binutils-gdb/gdb/dwarf2/cooked-index.h:729 (gdb+0x364e0c)
    #5 cooked_index_functions::compute_main_name(objfile*) ../../binutils-gdb/gdb/dwarf2/cooked-index.h:806 (gdb+0x364e0c)
    #6 objfile::compute_main_name() ../../binutils-gdb/gdb/symfile-debug.c:461 (gdb+0x69d904)
    #7 find_main_name ../../binutils-gdb/gdb/symtab.c:6503 (gdb+0x6b8b4c)
    #8 main_language() ../../binutils-gdb/gdb/symtab.c:6608 (gdb+0x6c71a3)
    #9 set_initial_language_callback ../../binutils-gdb/gdb/symfile.c:1634 (gdb+0x69ffdf)
    #10 get_current_language() ../../binutils-gdb/gdb/language.c:96 (gdb+0x4c9aaf)
    #11 parse_exp_in_context ../../binutils-gdb/gdb/parse.c:415 (gdb+0x5a7594)
    #12 parse_expression(char const*, innermost_block_tracker*, enum_flags<parser_flag>) ../../binutils-gdb/gdb/parse.c:478 (gdb+0x5a7dbe)
    #13 parse_and_eval_long(char const*) ../../binutils-gdb/gdb/eval.c:63 (gdb+0x3d61ca)
    #14 parse_cli_var_integer(var_types, literal_def const*, char const**, bool) ../../binutils-gdb/gdb/cli/cli-setshow.c:232 (gdb+0x2911d8)
    #15 do_set_command(char const*, int, cmd_list_element*) ../../binutils-gdb/gdb/cli/cli-setshow.c:421 (gdb+0x293144)
    #16 execute_command(char const*, int) ../../binutils-gdb/gdb/top.c:562 (gdb+0x732693)
    #17 command_handler(char const*) ../../binutils-gdb/gdb/event-top.c:580 (gdb+0x3e128d)
    #18 command_line_handler(std::unique_ptr<char, gdb::xfree_deleter<char> >&&) ../../binutils-gdb/gdb/event-top.c:816 (gdb+0x3e3304)
    #19 gdb_rl_callback_handler ../../binutils-gdb/gdb/event-top.c:272 (gdb+0x3e0b67)
    #20 rl_callback_read_char ../../../binutils-gdb/readline/readline/callback.c:290 (gdb+0x80f4e3)
    #21 gdb_rl_callback_read_char_wrapper_noexcept ../../binutils-gdb/gdb/event-top.c:197 (gdb+0x3e054d)
    #22 gdb_rl_callback_read_char_wrapper ../../binutils-gdb/gdb/event-top.c:236 (gdb+0x3e08ae)
    #23 stdin_event_handler ../../binutils-gdb/gdb/ui.c:154 (gdb+0x75a4ff)
    #24 handle_file_event ../../binutils-gdb/gdbsupport/event-loop.cc:551 (gdb+0x9ce523)
    #25 gdb_wait_for_event ../../binutils-gdb/gdbsupport/event-loop.cc:672 (gdb+0x9cec09)
    #26 gdb_do_one_event(int) ../../binutils-gdb/gdbsupport/event-loop.cc:263 (gdb+0x9cf5ed)
    #27 start_event_loop ../../binutils-gdb/gdb/main.c:401 (gdb+0x527329)
    #28 captured_command_loop ../../binutils-gdb/gdb/main.c:465 (gdb+0x527329)
    #29 gdb_wait_for_event ../../binutils-gdb/gdbsupport/event-loop.cc:672 (gdb+0x9cec09)
    #30 gdb_do_one_event(int) ../../binutils-gdb/gdbsupport/event-loop.cc:263 (gdb+0x9cf5ed)
    #31 start_event_loop ../../binutils-gdb/gdb/main.c:401 (gdb+0x527329)
    #32 captured_command_loop ../../binutils-gdb/gdb/main.c:465 (gdb+0x527329)
    #33 captured_main ../../binutils-gdb/gdb/main.c:1338 (gdb+0x52b074)
    #34 gdb_main(captured_main_args*) ../../binutils-gdb/gdb/main.c:1357 (gdb+0x52b074)
    #35 main ../../binutils-gdb/gdb/gdb.c:38 (gdb+0x1162e2)

  Previous write of size 8 at 0x7b8000020288 by thread T4:
    #0 cooked_index_shard::finalize(parent_map_map const*) ../../binutils-gdb/gdb/dwarf2/cooked-index.c:409 (gdb+0x31f6f3)
    #1 operator() ../../binutils-gdb/gdb/dwarf2/cooked-index.c:663 (gdb+0x31f832)
    #2 __invoke_impl<void, cooked_index::set_contents(vec_type&&, deferred_warnings*, const parent_map_map*)::<lambda()>&> /usr/include/c++/12/bits/invoke.h:61 (gdb+0x31f832)
    #3 __invoke_r<void, cooked_index::set_contents(vec_type&&, deferred_warnings*, const parent_map_map*)::<lambda()>&> /usr/include/c++/12/bits/invoke.h:111 (gdb+0x31f832)
    #4 _M_invoke /usr/include/c++/12/bits/std_function.h:290 (gdb+0x31f832)
    #5 std::function<void ()>::operator()() const /usr/include/c++/12/bits/std_function.h:591 (gdb+0x9d88f5)
    #6 operator() ../../binutils-gdb/gdbsupport/task-group.cc:68 (gdb+0x9d88f5)
    #7 __invoke_impl<void, gdb::task_group::impl::start()::<lambda()>&> /usr/include/c++/12/bits/invoke.h:61 (gdb+0x9d88f5)
    #8 __invoke_r<void, gdb::task_group::impl::start()::<lambda()>&> /usr/include/c++/12/bits/invoke.h:111 (gdb+0x9d88f5)
    #9 _M_invoke /usr/include/c++/12/bits/std_function.h:290 (gdb+0x9d88f5)
    #10 std::function<void ()>::operator()() const /usr/include/c++/12/bits/std_function.h:591 (gdb+0x321099)
    #11 void std::__invoke_impl<void, std::function<void ()>&>(std::__invoke_other, std::function<void ()>&) /usr/include/c++/12/bits/invoke.h:61 (gdb+0x321099)
    #12 std::enable_if<is_invocable_r_v<void, std::function<void ()>&>, void>::type std::__invoke_r<void, std::function<void ()>&>(std::function<void ()>&) /usr/include/c++/12/bits/invoke.h:111 (gdb+0x321099)
    #13 std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run()::{lambda()#1}::operator()() const /usr/include/c++/12/future:1469 (gdb+0x321099)
    #14 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++/12/future:1410 (gdb+0x321099)
    #15 std::unique_ptr<std::__future_base::_Result<void>, std::__future_base::_Result_base::_Deleter> std::__invoke_impl<std::unique_ptr<std::__future_base::_Result<void>, 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>&>(std::__invoke_other, 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>&) /usr/include/c++/12/bits/invoke.h:61 (gdb+0x321099)
    #16 std::enable_if<is_invocable_r_v<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>&>, std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> >::type std::__invoke_r<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>&>(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>&) /usr/include/c++/12/bits/invoke.h:116 (gdb+0x321099)
    #17 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++/12/bits/std_function.h:291 (gdb+0x321099)
    #18 std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>::operator()() const /usr/include/c++/12/bits/std_function.h:591 (gdb+0x3214c9)
    #19 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++/12/future:572 (gdb+0x3214c9)
    #20 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++/12/bits/invoke.h:74 (gdb+0x320e30)
    #21 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++/12/bits/invoke.h:96 (gdb+0x320e30)
    #22 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++/12/mutex:852 (gdb+0x320e30)
    #23 std::once_flag::_Prepare_execution::_Prepare_execution<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}>(void (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++/12/mutex:788 (gdb+0x320e30)
    #24 std::once_flag::_Prepare_execution::_Prepare_execution<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}>(void (std::__future_base::_State_baseV2::*&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*))::{lambda()#1}::_FUN() /usr/include/c++/12/mutex:788 (gdb+0x320e30)
    #25 pthread_once ../../../../src/libsanitizer/tsan/tsan_interceptors_posix.cpp:1517 (libtsan.so.2+0x46512)
    #26 __gthread_once /usr/include/x86_64-linux-gnu/c++/12/bits/gthr-default.h:700 (gdb+0x321f9d)
    #27 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++/12/mutex:859 (gdb+0x321f9d)
    #28 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++/12/future:412 (gdb+0x9dfab4)
    #29 std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run() /usr/include/c++/12/future:1472 (gdb+0x9dfab4)
    #30 std::packaged_task<void ()>::operator()() /usr/include/c++/12/future:1605 (gdb+0x9dfab4)
    #31 gdb::thread_pool::thread_function() ../../binutils-gdb/gdbsupport/thread-pool.cc:245 (gdb+0x9dfab4)
    #32 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++/12/bits/invoke.h:74 (gdb+0x9e0cc3)
    #33 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++/12/bits/invoke.h:96 (gdb+0x9e0cc3)
    #34 void std::thread::_Invoker<std::tuple<void (gdb::thread_pool::*)(), gdb::thread_pool*> >::_M_invoke<0ul, 1ul>(std::_Index_tuple<0ul, 1ul>) /usr/include/c++/12/bits/std_thread.h:252 (gdb+0x9e0cc3)
    #35 std::thread::_Invoker<std::tuple<void (gdb::thread_pool::*)(), gdb::thread_pool*> >::operator()() /usr/include/c++/12/bits/std_thread.h:259 (gdb+0x9e0cc3)
    #36 std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (gdb::thread_pool::*)(), gdb::thread_pool*> > >::_M_run() /usr/include/c++/12/bits/std_thread.h:210 (gdb+0x9e0cc3)
    #37 execute_native_thread_routine <null> (gdb+0xa90592)

  Location is heap block of size 4064 at 0x7b8000020000 allocated by thread T2:
    #0 malloc ../../../../src/libsanitizer/tsan/tsan_interceptors_posix.cpp:647 (libtsan.so.2+0x3ebb8)
    #1 xmalloc ../../binutils-gdb/gdb/alloc.c:52 (gdb+0x18f026)
    #2 call_chunkfun ../../binutils-gdb/libiberty/obstack.c:94 (gdb+0x9a5ae4)
    #3 _obstack_begin_worker ../../binutils-gdb/libiberty/obstack.c:141 (gdb+0x9a5ae4)
    #4 _obstack_begin ../../binutils-gdb/libiberty/obstack.c:164 (gdb+0x9a5b69)
    #5 auto_obstack::auto_obstack() ../../binutils-gdb/gdb/../gdbsupport/gdb_obstack.h:123 (gdb+0x380913)
    #6 cooked_index_shard::cooked_index_shard() ../../binutils-gdb/gdb/dwarf2/cooked-index.h:270 (gdb+0x380913)
    #7 cooked_index_storage::cooked_index_storage() ../../binutils-gdb/gdb/dwarf2/read.c:4411 (gdb+0x380913)
    #8 cooked_index_debug_info::process_cus(unsigned long, __gnu_cxx::__normal_iterator<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter>*, std::vector<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter>, std::allocator<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter> > > >, __gnu_cxx::__normal_iterator<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter>*, std::vector<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter>, std::allocator<std::unique_ptr<dwarf2_per_cu_data, dwarf2_per_cu_data_deleter> > > >) ../../binutils-gdb/gdb/dwarf2/read.c:4926 (gdb+0x39c672)
    #9 operator() ../../binutils-gdb/gdb/dwarf2/read.c:5035 (gdb+0x39dcb5)
    #10 __invoke_impl<void, cooked_index_debug_info::do_reading()::<lambda()>&> /usr/include/c++/12/bits/invoke.h:61 (gdb+0x39dcb5)
    #11 __invoke_r<void, cooked_index_debug_info::do_reading()::<lambda()>&> /usr/include/c++/12/bits/invoke.h:111 (gdb+0x39dcb5)
    #12 _M_invoke /usr/include/c++/12/bits/std_function.h:290 (gdb+0x39dcb5)
    #13 std::function<void ()>::operator()() const /usr/include/c++/12/bits/std_function.h:591 (gdb+0x9d88f5)
    #14 operator() ../../binutils-gdb/gdbsupport/task-group.cc:68 (gdb+0x9d88f5)
    #15 __invoke_impl<void, gdb::task_group::impl::start()::<lambda()>&> /usr/include/c++/12/bits/invoke.h:61 (gdb+0x9d88f5)
    #16 __invoke_r<void, gdb::task_group::impl::start()::<lambda()>&> /usr/include/c++/12/bits/invoke.h:111 (gdb+0x9d88f5)
    #17 _M_invoke /usr/include/c++/12/bits/std_function.h:290 (gdb+0x9d88f5)
    #18 std::function<void ()>::operator()() const /usr/include/c++/12/bits/std_function.h:591 (gdb+0x321099)
    #19 void std::__invoke_impl<void, std::function<void ()>&>(std::__invoke_other, std::function<void ()>&) /usr/include/c++/12/bits/invoke.h:61 (gdb+0x321099)
    #20 std::enable_if<is_invocable_r_v<void, std::function<void ()>&>, void>::type std::__invoke_r<void, std::function<void ()>&>(std::function<void ()>&) /usr/include/c++/12/bits/invoke.h:111 (gdb+0x321099)
    #21 std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run()::{lambda()#1}::operator()() const /usr/include/c++/12/future:1469 (gdb+0x321099)
    #22 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++/12/future:1410 (gdb+0x321099)
    #23 std::unique_ptr<std::__future_base::_Result<void>, std::__future_base::_Result_base::_Deleter> std::__invoke_impl<std::unique_ptr<std::__future_base::_Result<void>, 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>&>(std::__invoke_other, 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>&) /usr/include/c++/12/bits/invoke.h:61 (gdb+0x321099)
    #24 std::enable_if<is_invocable_r_v<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>&>, std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> >::type std::__invoke_r<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>&>(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>&) /usr/include/c++/12/bits/invoke.h:116 (gdb+0x321099)
    #25 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++/12/bits/std_function.h:291 (gdb+0x321099)
    #26 std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>::operator()() const /usr/include/c++/12/bits/std_function.h:591 (gdb+0x3214c9)
    #27 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++/12/future:572 (gdb+0x3214c9)
    #28 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++/12/bits/invoke.h:74 (gdb+0x320e30)
    #29 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++/12/bits/invoke.h:96 (gdb+0x320e30)
    #30 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++/12/mutex:852 (gdb+0x320e30)
    #31 std::once_flag::_Prepare_execution::_Prepare_execution<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}>(void (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++/12/mutex:788 (gdb+0x320e30)
    #32 std::once_flag::_Prepare_execution::_Prepare_execution<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}>(void (std::__future_base::_State_baseV2::*&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*))::{lambda()#1}::_FUN() /usr/include/c++/12/mutex:788 (gdb+0x320e30)
    #33 pthread_once ../../../../src/libsanitizer/tsan/tsan_interceptors_posix.cpp:1517 (libtsan.so.2+0x46512)
    #34 __gthread_once /usr/include/x86_64-linux-gnu/c++/12/bits/gthr-default.h:700 (gdb+0x321f9d)
    #35 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++/12/mutex:859 (gdb+0x321f9d)
    #36 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++/12/future:412 (gdb+0x9dfab4)
    #37 std::__future_base::_Task_state<std::function<void ()>, std::allocator<int>, void ()>::_M_run() /usr/include/c++/12/future:1472 (gdb+0x9dfab4)
    #38 std::packaged_task<void ()>::operator()() /usr/include/c++/12/future:1605 (gdb+0x9dfab4)
    #39 gdb::thread_pool::thread_function() ../../binutils-gdb/gdbsupport/thread-pool.cc:245 (gdb+0x9dfab4)
    #40 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++/12/bits/invoke.h:74 (gdb+0x9e0cc3)
    #41 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++/12/bits/invoke.h:96 (gdb+0x9e0cc3)
    #42 void std::thread::_Invoker<std::tuple<void (gdb::thread_pool::*)(), gdb::thread_pool*> >::_M_invoke<0ul, 1ul>(std::_Index_tuple<0ul, 1ul>) /usr/include/c++/12/bits/std_thread.h:252 (gdb+0x9e0cc3)
    #43 std::thread::_Invoker<std::tuple<void (gdb::thread_pool::*)(), gdb::thread_pool*> >::operator()() /usr/include/c++/12/bits/std_thread.h:259 (gdb+0x9e0cc3)
    #44 std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (gdb::thread_pool::*)(), gdb::thread_pool*> > >::_M_run() /usr/include/c++/12/bits/std_thread.h:210 (gdb+0x9e0cc3)
    #45 execute_native_thread_routine <null> (gdb+0xa90592)

  Thread T4 'gdb worker' (tid=29556, running) created by main thread at:
    #0 pthread_create ../../../../src/libsanitizer/tsan/tsan_interceptors_posix.cpp:1001 (libtsan.so.2+0x5e686)
    #1 std::thread::_M_start_thread(std::unique_ptr<std::thread::_State, std::default_delete<std::thread::_State> >, void (*)()) <null> (gdb+0xa90668)
    #2 update_thread_pool_size() ../../binutils-gdb/gdb/maint.c:865 (gdb+0x53356b)
    #3 captured_main_1 ../../binutils-gdb/gdb/main.c:1054 (gdb+0x529a13)
    #4 captured_main ../../binutils-gdb/gdb/main.c:1328 (gdb+0x52b06a)
    #5 gdb_main(captured_main_args*) ../../binutils-gdb/gdb/main.c:1357 (gdb+0x52b06a)
    #6 main ../../binutils-gdb/gdb/gdb.c:38 (gdb+0x1162e2)

  Thread T2 'gdb worker' (tid=29554, running) created by main thread at:
    #0 pthread_create ../../../../src/libsanitizer/tsan/tsan_interceptors_posix.cpp:1001 (libtsan.so.2+0x5e686)
    #1 std::thread::_M_start_thread(std::unique_ptr<std::thread::_State, std::default_delete<std::thread::_State> >, void (*)()) <null> (gdb+0xa90668)
    #2 update_thread_pool_size() ../../binutils-gdb/gdb/maint.c:865 (gdb+0x53356b)
    #3 captured_main_1 ../../binutils-gdb/gdb/main.c:1054 (gdb+0x529a13)
    #4 captured_main ../../binutils-gdb/gdb/main.c:1328 (gdb+0x52b06a)
    #5 gdb_main(captured_main_args*) ../../binutils-gdb/gdb/main.c:1357 (gdb+0x52b06a)
    #6 main ../../binutils-gdb/gdb/gdb.c:38 (gdb+0x1162e2)

SUMMARY: ThreadSanitizer: data race ../../binutils-gdb/gdb/dwarf2/cooked-index.c:220 in cooked_index_entry::full_name(obstack*, bool) const
==================
(gdb) FAIL: gdb.base/annota1.exp: set height 0
break annota1.c:15
Breakpoint 1 at 0x115b: file /home/ed/gnu/gdb-build-y/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.base/annota1.c, line 15.
(gdb) PASS: gdb.base/annota1.exp: breakpoint main
set annotate 2
Comment 9 Bernd Edlinger 2024-10-03 14:21:50 UTC 
Hmm...

  Read of size 8 at 0x7b8000020288 by main thread:
    #0 cooked_index_entry::full_name(obstack*, bool) const ../../binutils-gdb/gdb/dwarf2/cooked-index.c:220 (gdb+0x31c114)

When I disassemble the code in question it looks like a mis-compilation.
  31c0fc:       e8 3f 57 db ff          call   d1840 <__tsan_func_entry@plt>
  31c101:       48 89 df                mov    %rbx,%rdi
  31c104:       e8 37 5c db ff          call   d1d40 <__tsan_read8@plt>
  31c109:       48 8d 7b 08             lea    0x8(%rbx),%rdi
  31c10d:       4c 8b 23                mov    (%rbx),%r12
  31c110:       e8 2b 5c db ff          call   d1d40 <__tsan_read8@plt>
  31c115:       48 8d 7b 14             lea    0x14(%rbx),%rdi
  31c119:       45 84 ed                test   %r13b,%r13b
  31c11c:       4c 0f 44 63 08          cmove  0x8(%rbx),%r12
  31c121:       e8 aa 5e db ff          call   d1fd0 <__tsan_read1@plt>
  31c126:       0f b6 43 14             movzbl 0x14(%rbx),%eax
  31c12a:       a8 04                   test   $0x4,%al
  31c12c:       74 1a                   je     31c148 <_ZNK18cooked_index_entry9full_nameEP7obstackb+0x68>
  31c12e:       e8 fd 61 db ff          call   d2330 <__tsan_func_exit@plt>

So this->name is read unconditinally, and the read value
is replaced with this->canonical conditionally with a cmove,
that is actually not executed, but from the tsan instrumentation
it looks as if the read is done in any case.

instead of the used gcc version 12.2.0 (Debian 12.2.0-14)
I can also bootstrap a brand new gcc and see what happens there,
although I would still suggest to rewrite the code in question
like this:

  const char **local_name = for_main ? &name : &canonical;

  if ((flags & IS_LINKAGE) != 0 || get_parent () == nullptr) 
    return *local_name;
Comment 10 Tom de Vries 2024-10-03 15:05:58 UTC 
(In reply to Bernd Edlinger from comment #9)
> So this->name is read unconditinally, and the read value
> is replaced with this->canonical conditionally with a cmove,
> that is actually not executed, but from the tsan instrumentation
> it looks as if the read is done in any case.

Reminds me of https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110799
Comment 11 Bernd Edlinger 2024-10-04 04:38:24 UTC 
(In reply to Tom de Vries from comment #10)
> (In reply to Bernd Edlinger from comment #9)
> > So this->name is read unconditinally, and the read value
> > is replaced with this->canonical conditionally with a cmove,
> > that is actually not executed, but from the tsan instrumentation
> > it looks as if the read is done in any case.
> 
> Reminds me of https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110799

Yeah that explains really a lot.

Now with this little tweak the number of warnings drops to zero:

diff --git a/gdb/dwarf2/cooked-index.c b/gdb/dwarf2/cooked-index.c
index 4073c924890..10f2b697a4c 100644
--- a/gdb/dwarf2/cooked-index.c
+++ b/gdb/dwarf2/cooked-index.c
@@ -217,7 +217,7 @@ cooked_index_entry::matches (domain_search_flags kind) const
 const char *
 cooked_index_entry::full_name (struct obstack *storage, bool for_main) const
 {
-  const char *local_name = for_main ? name : canonical;
+  const char *const &local_name = for_main ? name : canonical;
 
   if ((flags & IS_LINKAGE) != 0 || get_parent () == nullptr)
     return local_name;


Would you like to include this into your patch?

Thanks
Bernd.
Comment 12 Tom de Vries 2024-10-07 11:19:46 UTC 
(In reply to Bernd Edlinger from comment #11)
> Now with this little tweak the number of warnings drops to zero:
> 
> diff --git a/gdb/dwarf2/cooked-index.c b/gdb/dwarf2/cooked-index.c
> index 4073c924890..10f2b697a4c 100644
> --- a/gdb/dwarf2/cooked-index.c
> +++ b/gdb/dwarf2/cooked-index.c
> @@ -217,7 +217,7 @@ cooked_index_entry::matches (domain_search_flags kind)
> const
>  const char *
>  cooked_index_entry::full_name (struct obstack *storage, bool for_main) const
>  {
> -  const char *local_name = for_main ? name : canonical;
> +  const char *const &local_name = for_main ? name : canonical;
>  
>    if ((flags & IS_LINKAGE) != 0 || get_parent () == nullptr)
>      return local_name;
> 
> 

I think I prefer a workaround like this:
...
+#if defined (__SANITIZE_THREAD__) && defined (__GNUC__) && !defined (__clang__)

+/* Work around PR gcc/110799.  */
+__attribute__((optimize("-fno-hoist-adjacent-loads")))
+#endif
...
which doesn't require any changes in the function body.
Comment 13 Bernd Edlinger 2024-10-08 15:50:15 UTC 
(In reply to Tom de Vries from comment #12)
> (In reply to Bernd Edlinger from comment #11)
> > Now with this little tweak the number of warnings drops to zero:
> > 
> > diff --git a/gdb/dwarf2/cooked-index.c b/gdb/dwarf2/cooked-index.c
> > index 4073c924890..10f2b697a4c 100644
> > --- a/gdb/dwarf2/cooked-index.c
> > +++ b/gdb/dwarf2/cooked-index.c
> > @@ -217,7 +217,7 @@ cooked_index_entry::matches (domain_search_flags kind)
> > const
> >  const char *
> >  cooked_index_entry::full_name (struct obstack *storage, bool for_main) const
> >  {
> > -  const char *local_name = for_main ? name : canonical;
> > +  const char *const &local_name = for_main ? name : canonical;
> >  
> >    if ((flags & IS_LINKAGE) != 0 || get_parent () == nullptr)
> >      return local_name;
> > 
> > 
> 
> I think I prefer a workaround like this:
> ...
> +#if defined (__SANITIZE_THREAD__) && defined (__GNUC__) && !defined
> (__clang__)
> 
> +/* Work around PR gcc/110799.  */
> +__attribute__((optimize("-fno-hoist-adjacent-loads")))
> +#endif
> ...
> which doesn't require any changes in the function body.

That's also okay for me, if you prefer that one.
Maybe you can just add "&& __GNUC__ < 14" to the if-condition
since the mis-compilation is fixed with gcc-14, while
the all gcc-13 versions seem to be affected.  I've just tried that out.
Comment 14 Tom Tromey 2024-10-08 23:05:00 UTC 
(In reply to Tom de Vries from comment #12)

> I think I prefer a workaround like this:
> ...
> +#if defined (__SANITIZE_THREAD__) && defined (__GNUC__) && !defined
> (__clang__)
> 
> +/* Work around PR gcc/110799.  */
> +__attribute__((optimize("-fno-hoist-adjacent-loads")))
> +#endif
> ...
> which doesn't require any changes in the function body.

Should we just require TSAN builds to always use this option?
Or would that obscure real bugs sometimes?
Comment 15 Tom de Vries 2024-10-09 07:23:20 UTC 
(In reply to Tom Tromey from comment #14)
> (In reply to Tom de Vries from comment #12)
> 
> > I think I prefer a workaround like this:
> > ...
> > +#if defined (__SANITIZE_THREAD__) && defined (__GNUC__) && !defined
> > (__clang__)
> > 
> > +/* Work around PR gcc/110799.  */
> > +__attribute__((optimize("-fno-hoist-adjacent-loads")))
> > +#endif
> > ...
> > which doesn't require any changes in the function body.
> 
> Should we just require TSAN builds to always use this option?

That was my initial idea: in gdb/Makefile.in, do something like:
...
ifneq ($(filter -fsanitizer=thread,$(CFLAGS) $(CXXFLAGS)),)
CFLAGS = $(CFLAGS) -fno-hoist-adjacent-loads
CXXFLAGS = $(CXXFLAGS) -fno-hoist-adjacent-loads
endif
...
but clang doesn't support this option.

So, we could export gcc-ness of the compiler to the makefile level.

Or, use -O0 instead of -fno-hoist-adjacent-loads which I guess should be accepted by all compilers using -fsanitizer=thread.  But that wouldn't work if sufficient amount of -f flags would be provided to enable options, so it's not watertight.

Either way, there's the question of scope.  We'd do this also in gdbserver and gdbsupport, but what about say bfd, readline, libopcodes?  Should this be at the top-level?  In that case, it needs to be proposed for the gcc repository.

I figured a local fix using __SANITIZE_THREAD__ doesn't require any infrastructure changes, so propose that first.  If that's not acceptable, we can try to export gcc-ness to the makefile.

> Or would that obscure real bugs sometimes?

I don't think so.
Comment 16 Tom Tromey 2024-10-10 20:27:05 UTC 
I was actually thinking of something a lot less automated: just
a note somewhere saying "if you want TSAN you also need this option".

However something more robust is probably better I guess.
Can we just add your code snippet to common-defs.h?
Comment 17 Tom de Vries 2024-10-10 22:06:42 UTC 
(In reply to Tom Tromey from comment #16)
> I was actually thinking of something a lot less automated: just
> a note somewhere saying "if you want TSAN you also need this option".
> 
> However something more robust is probably better I guess.
> Can we just add your code snippet to common-defs.h?

Aha, so like this:
...
diff --git a/gdbsupport/common-defs.h b/gdbsupport/common-defs.h
index 6120719480b..60574dcd369 100644
--- a/gdbsupport/common-defs.h
+++ b/gdbsupport/common-defs.h
@@ -20,6 +20,13 @@
 #ifndef COMMON_COMMON_DEFS_H
 #define COMMON_COMMON_DEFS_H
 
+#if defined (__SANITIZE_THREAD__) && defined (__GNUC__) \
+  && !defined (__clang__) && __GNUC__ <= 13
+
+/* Work around PR gcc/110799.  */
+#pragma gcc optimize("-fno-hoist-adjacent-loads")
+#endif
+
 #include <gdbsupport/config.h>
 
 #undef PACKAGE_NAME
...

That would work.
Comment 18 Tom de Vries 2024-10-14 18:34:24 UTC 
(In reply to Tom de Vries from comment #17)
> (In reply to Tom Tromey from comment #16)
> > I was actually thinking of something a lot less automated: just
> > a note somewhere saying "if you want TSAN you also need this option".
> > 
> > However something more robust is probably better I guess.
> > Can we just add your code snippet to common-defs.h?
> 
> Aha, so like this:
> ...
> diff --git a/gdbsupport/common-defs.h b/gdbsupport/common-defs.h
> index 6120719480b..60574dcd369 100644
> --- a/gdbsupport/common-defs.h
> +++ b/gdbsupport/common-defs.h
> @@ -20,6 +20,13 @@
>  #ifndef COMMON_COMMON_DEFS_H
>  #define COMMON_COMMON_DEFS_H
>  
> +#if defined (__SANITIZE_THREAD__) && defined (__GNUC__) \
> +  && !defined (__clang__) && __GNUC__ <= 13
> +
> +/* Work around PR gcc/110799.  */
> +#pragma gcc optimize("-fno-hoist-adjacent-loads")
> +#endif
> +
>  #include <gdbsupport/config.h>
>  
>  #undef PACKAGE_NAME
> ...
> 
> That would work.

Bernd, can you test if this fixes your problem?  I'm having trouble reproducing.
Comment 19 Tom de Vries 2024-10-15 12:34:27 UTC 
(In reply to Tom de Vries from comment #18)
> (In reply to Tom de Vries from comment #17)
> > (In reply to Tom Tromey from comment #16)
> > > I was actually thinking of something a lot less automated: just
> > > a note somewhere saying "if you want TSAN you also need this option".
> > > 
> > > However something more robust is probably better I guess.
> > > Can we just add your code snippet to common-defs.h?
> > 
> > Aha, so like this:
> > ...
> > diff --git a/gdbsupport/common-defs.h b/gdbsupport/common-defs.h
> > index 6120719480b..60574dcd369 100644
> > --- a/gdbsupport/common-defs.h
> > +++ b/gdbsupport/common-defs.h
> > @@ -20,6 +20,13 @@
> >  #ifndef COMMON_COMMON_DEFS_H
> >  #define COMMON_COMMON_DEFS_H
> >  
> > +#if defined (__SANITIZE_THREAD__) && defined (__GNUC__) \
> > +  && !defined (__clang__) && __GNUC__ <= 13
> > +
> > +/* Work around PR gcc/110799.  */
> > +#pragma gcc optimize("-fno-hoist-adjacent-loads")
> > +#endif
> > +
> >  #include <gdbsupport/config.h>
> >  
> >  #undef PACKAGE_NAME
> > ...
> > 
> > That would work.
> 
> Bernd, can you test if this fixes your problem?  I'm having trouble
> reproducing.

I've managed to reproduce it now.

Currently testing.  FWIW, the pragma line should be:
...
-#pragma gcc optimize("-fno-hoist-adjacent-loads")
+#pragma GCC optimize("-fno-hoist-adjacent-loads")
...
Comment 20 Bernd Edlinger 2024-10-15 17:58:06 UTC 
Hi Tom,

really good work, my testing is also OK.
Comment 22 Sourceware Commits 2024-10-17 22:18:10 UTC 
The master branch has been updated by Tom de Vries <vries@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=9a56fae565b641d6e7557e8c59adf9018d304041

commit 9a56fae565b641d6e7557e8c59adf9018d304041
Author: Tom de Vries <tdevries@suse.de>
Date:   Fri Oct 18 00:18:24 2024 +0200

    [gdb/build] Use -fno-hoist-adjacent-loads for gcc <= 13
    
    When building gdb with gcc 12 and -fsanitize=threads while renabling
    background dwarf reading by setting dwarf_synchronous to false, I run into:
    ...
    (gdb) file amd64-watchpoint-downgrade
    Reading symbols from amd64-watchpoint-downgrade...
    (gdb) watch global_var
    ==================
    WARNING: ThreadSanitizer: data race (pid=20124)
      Read of size 8 at 0x7b80000500d8 by main thread:
        #0 cooked_index_entry::full_name(obstack*, bool) const cooked-index.c:220
        #1 cooked_index::get_main_name(obstack*, language*) const cooked-index.c:735
        #2 cooked_index_worker::wait(cooked_state, bool) cooked-index.c:559
        #3 cooked_index::wait(cooked_state, bool) cooked-index.c:631
        #4 cooked_index_functions::wait(objfile*, bool) cooked-index.h:729
        #5 cooked_index_functions::compute_main_name(objfile*) cooked-index.h:806
        #6 objfile::compute_main_name() symfile-debug.c:461
        #7 find_main_name symtab.c:6503
        #8 main_language() symtab.c:6608
        #9 set_initial_language_callback symfile.c:1634
        #10 get_current_language() language.c:96
        ...
    
      Previous write of size 8 at 0x7b80000500d8 by thread T1:
        #0 cooked_index_shard::finalize(parent_map_map const*) \
             dwarf2/cooked-index.c:409
        #1 operator() cooked-index.c:663
        ...
    
      ...
    
    SUMMARY: ThreadSanitizer: data race cooked-index.c:220 in \
      cooked_index_entry::full_name(obstack*, bool) const
    ==================
    Hardware watchpoint 1: global_var
    (gdb) PASS: gdb.arch/amd64-watchpoint-downgrade.exp: watch global_var
    ...
    
    This was also reported in PR31715.
    
    This is due do gcc PR110799 [1], generating wrong code with
    -fhoist-adjacent-loads, and causing a false positive for
    -fsanitize=threads.
    
    Work around the gcc PR by forcing -fno-hoist-adjacent-loads for gcc <= 13
    and -fsanitize=threads.
    
    Tested in that same configuration on x86_64-linux.  Remaining ThreadSanitizer
    problems are the ones reported in PR31626 (gdb.rust/dwindex.exp) and
    PR32247 (gdb.trace/basic-libipa.exp).
    
    PR gdb/31715
    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31715
    
    Tested-By: Bernd Edlinger <bernd.edlinger@hotmail.de>
    Approved-By: Tom Tromey <tom@tromey.com>
    
    [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110799
Comment 23 Tom de Vries 2024-10-17 22:20:18 UTC 
Committed patch with workaround, marking the PR resolved-fixed.