Bug 30837 - [gdb/symtab, index-cache] data race on current_inferior_
Summary: [gdb/symtab, index-cache] data race on current_inferior_
Status: RESOLVED FIXED
Alias: None
Product: gdb
Classification: Unclassified
Component: symtab (show other bugs)
Version: HEAD
: P2 normal
Target Milestone: 15.1
Assignee: Tom Tromey
URL:
Keywords:
Depends on:
Blocks: 29366 31751
  Show dependency treegraph
 
Reported: 2023-09-08 14:27 UTC by Tom de Vries
Modified: 2024-05-17 14:17 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments
gdb.log (2.88 KB, text/x-log)
2023-09-08 14:27 UTC, Tom de Vries
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Tom de Vries 2023-09-08 14:27:01 UTC
Created attachment 15106 [details]
gdb.log

When building gdb with -fsanitize=threads and using soon to be submitted target board cc-with-index-cache, I run into a data race in test-case gdb.ada/non-ascii-utf-8.exp
...
==================^M
^[[1m^[[31mWARNING: ThreadSanitizer: data race (pid=25220)^M
^[[1m^[[0m^[[1m^[[34m  Read of size 8 at 0x000003342928 by thread T12:^M
^[[1m^[[0m    #0 gdb::ref_ptr<inferior, refcounted_object_ref_policy>::get() const /data/vries/gdb/src/gdb/../gdbsupport/gdb_ref_ptr.h:132 (gdb+0xa690b8)^M
    #1 current_inferior() /data/vries/gdb/src/gdb/inferior.c:57 (gdb+0xa63b74)^M
    #2 target_supports_terminal_ours() /data/vries/gdb/src/gdb/target.c:1121 (gdb+0xfaefd4)^M
    #3 vwarning(char const*, __va_list_tag*) /data/vries/gdb/src/gdb/utils.c:147 (gdb+0x10e54c5)^M
    #4 warning(char const*, ...) /data/vries/gdb/src/gdbsupport/errors.cc:34 (gdb+0x1dc04e4)^M
    #5 ada_encode_1 /data/vries/gdb/src/gdb/ada-lang.c:1000 (gdb+0x47d348)^M
    #6 ada_encode[abi:cxx11](char const*, bool) /data/vries/gdb/src/gdb/ada-lang.c:1159 (gdb+0x47dd12)^M
    #7 write_cooked_index /data/vries/gdb/src/gdb/dwarf2/index-write.c:1142 (gdb+0x832e42)^M
    #8 write_gdbindex /data/vries/gdb/src/gdb/dwarf2/index-write.c:1236 (gdb+0x83348b)^M
  ...
^M
^[[1m^[[34m  Previous write of size 8 at 0x000003342928 by main thread:^M
^[[1m^[[0m    #0 gdb::ref_ptr<inferior, refcounted_object_ref_policy>::reset(inferior*) /data/vries/gdb/src/gdb/../gdbsupport/gdb_ref_ptr.h:125 (gdb+0xa6a2bf)^M
    #1 gdb::ref_ptr<inferior, refcounted_object_ref_policy>::operator=(gdb::ref_ptr<inferior, refcounted_object_ref_policy>&&) /data/vries/gdb/src/gdb/../gdbsupport/gdb_ref_ptr.h:113 (gdb+0xa691cc)^M
    #2 set_current_inferior(inferior*) /data/vries/gdb/src/gdb/inferior.c:66 (gdb+0xa63bf6)^M
    #3 switch_to_inferior_no_thread(inferior*) /data/vries/gdb/src/gdb/inferior.c:714 (gdb+0xa66df9)^M
    #4 scoped_restore_current_thread::restore() /data/vries/gdb/src/gdb/thread.c:1397 (gdb+0xfe1f9b)^M
    #5 scoped_restore_current_thread::~scoped_restore_current_thread() /data/vries/gdb/src/gdb/thread.c:1415 (gdb+0xfe20c9)^M
    #6 scoped_restore_current_pspace_and_thread::~scoped_restore_current_pspace_and_thread() /data/vries/gdb/src/gdb/progspace-and-thread.h:29 (gdb+0x628ef7)^M
    #7 bp_loc_is_permanent /data/vries/gdb/src/gdb/breakpoint.c:8532 (gdb+0x609bed)^M
    #8 code_breakpoint::add_location(symtab_and_line const&) /data/vries/gdb/src/gdb/breakpoint.c:8509 (gdb+0x609acc)^M
    #9 internal_breakpoint::internal_breakpoint(gdbarch*, bptype, unsigned long) /data/vries/gdb/src/gdb/breakpoint.c:350 (gdb+0x6282a3)^M
    #10 create_internal_breakpoint /data/vries/gdb/src/gdb/breakpoint.c:3454 (gdb+0x5f71c0)^M
    #11 create_internal_breakpoint /data/vries/gdb/src/gdb/breakpoint.c:3485 (gdb+0x5f7346)^M
    #12 create_exception_master_breakpoint_hook /data/vries/gdb/src/gdb/breakpoint.c:3856 (gdb+0x5f86dc)^M
    #13 create_exception_master_breakpoint /data/vries/gdb/src/gdb/breakpoint.c:3882 (gdb+0x5f8899)^M
    #14 breakpoint_re_set() /data/vries/gdb/src/gdb/breakpoint.c:13262 (gdb+0x61c3f1)^M
    #15 symbol_file_command(char const*, int) /data/vries/gdb/src/gdb/symfile.c:1686 (gdb+0xf44c8d)^M
    #16 file_command /data/vries/gdb/src/gdb/exec.c:554 (gdb+0x94f99f)^M
...
Comment 1 Sourceware Commits 2023-11-27 20:15:33 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=f1b8ee6f2b4381bc46a0ad4c233b6eddc1e135b5

commit f1b8ee6f2b4381bc46a0ad4c233b6eddc1e135b5
Author: Tom de Vries <tdevries@suse.de>
Date:   Mon Nov 27 21:15:26 2023 +0100

    [gdb/testsuite] Add boards/cc-with-index-cache.exp
    
    We have a target board cc-with-gdb-index that uses the gdb-add-index script to
    add a .gdb_index index to an exec.
    
    There is however an alternative way of adding a .gdb_index: the index-cache.
    
    Add a new target board cc-with-index-cache.
    
    This is not superfluous for two reasons:
    - there is functionality that gdb-add-index doesn't support, but the
      index-cache does: the index-cache can add an index to an exec with a
      .gnu_debugaltlink (note that when using the cc-with-gdb-index board this
      case is quietly ignored), and
    - using the index-cache is excercised in only a few test-cases, and having
      this target board extends the test coverage to the entire test suite.  This
      is for instance relevant because the index-cache is written by a worker
      thread in the background, so we can check more thoroughly for data races
      (see PR symtab/30837).
    
    Tested on x86_64-linux.
    
    Shell script changes checked with shellcheck.
    
    Approved-By: Tom Tromey <tom@tromey.com>
Comment 2 Tom Tromey 2023-11-27 23:28:36 UTC
Probably need to defer warnings in this code.
Comment 3 Tom Tromey 2023-12-02 00:23:10 UTC
Also worth noting that if/when the .debug_names writer is fixed,
this warning will be impossible, because that will just be
writing out the DW_AT_name values verbatim.
Comment 4 Tom Tromey 2023-12-04 15:54:11 UTC
Actually the debug-names writer shouldn't be invoked from the
background so this must be something else.
Comment 5 Tom Tromey 2024-01-19 16:35:30 UTC
(In reply to Tom Tromey from comment #4)
> Actually the debug-names writer shouldn't be invoked from the
> background so this must be something else.

It's the call to ada_encode in index-write.c:write_cooked_index.
Comment 6 Tom Tromey 2024-02-13 21:04:36 UTC
I have a patch for this that I'll send tomorrow.

It's tempting to unify the ways to intercept warnings
and not have a separate "deferred warning" class, but
in the end I chose not to do this for now.
Comment 8 Tom Tromey 2024-03-12 14:41:34 UTC
v2 (just a rebase): https://sourceware.org/pipermail/gdb-patches/2024-March/207215.html
Comment 9 Sourceware Commits 2024-03-26 15:55:46 UTC
The master branch has been updated by Tom Tromey <tromey@sourceware.org>:

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

commit 818ef5f4137aaff3afdb52f8bbd3a4c3a9ffa28b
Author: Tom Tromey <tromey@adacore.com>
Date:   Tue Feb 13 13:55:34 2024 -0700

    Capture warnings when writing to the index cache
    
    PR symtab/30837 points out a race that can occur when writing to the
    index cache: a call to ada_encode can cause a warning, which is
    forbidden on a worker thread.
    
    This patch fixes the problem by arranging to capture any such
    warnings.
    
    This is v2 of the patch.  It is rebased on top of some other changes
    in the same area.  v1 was here:
    
        https://sourceware.org/pipermail/gdb-patches/2024-February/206595.html
    
    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30837
Comment 10 Tom Tromey 2024-03-26 15:56:14 UTC
Fixed.