Skip to content

Commit

Permalink
[gdb/symtab] Fix data race on per_cu->dwarf_version
Browse files Browse the repository at this point in the history
When building gdb with -fsanitize=thread and gcc 12, and running test-case
gdb.dwarf2/dwz.exp, we run into a data race between thread T2 and the main
thread in the same write:
...
Write of size 1 at 0x7b200000300c:^M
    #0 cutu_reader::cutu_reader(dwarf2_per_cu_data*, dwarf2_per_objfile*, \
    abbrev_table*, dwarf2_cu*, bool, abbrev_cache*) gdb/dwarf2/read.c:6252 \
    (gdb+0x82f3b3)^M
...
which is here:
...
         this_cu->dwarf_version = cu->header.version;
...

Both writes are called from the parallel for in dwarf2_build_psymtabs_hard,
this one directly:
...
    #1 process_psymtab_comp_unit gdb/dwarf2/read.c:6774 (gdb+0x8304d7)^M
    #2 operator() gdb/dwarf2/read.c:7098 (gdb+0x8317be)^M
    #3 operator() gdbsupport/parallel-for.h:163 (gdb+0x872380)^M
...
and this via the PU import:
...
    #1 cooked_indexer::ensure_cu_exists(cutu_reader*, dwarf2_per_objfile*, \
    sect_offset, bool,  bool) gdb/dwarf2/read.c:17964 (gdb+0x85c43b)^M
    #2 cooked_indexer::index_imported_unit(cutu_reader*, unsigned char const*, \
    abbrev_info const*) gdb/dwarf2/read.c:18248 (gdb+0x85d8ff)^M
    #3 cooked_indexer::index_dies(cutu_reader*, unsigned char const*, \
    cooked_index_entry const*, bool) gdb/dwarf2/read.c:18302 (gdb+0x85dcdb)^M
    #4 cooked_indexer::make_index(cutu_reader*) gdb/dwarf2/read.c:18443 \
    (gdb+0x85e68a)^M
    #5 process_psymtab_comp_unit gdb/dwarf2/read.c:6812 (gdb+0x830879)^M
    #6 operator() gdb/dwarf2/read.c:7098 (gdb+0x8317be)^M
    #7 operator() gdbsupport/parallel-for.h:171 (gdb+0x8723e2)^M
...

Fix this by setting the field earlier, in read_comp_units_from_section.

The write in cutu_reader::cutu_reader() is still needed, in case
read_comp_units_from_section is not used (run the test-case with say, target
board cc-with-gdb-index).

Make the write conditional, such that it doesn't trigger if the field is
already set by read_comp_units_from_section.  Instead, verify that the
field already has the value that we're trying to set it to.

Move this logic into into a member function set_version (in analogy to the
already present member function version) to make sure it's used consistenly,
and make the field private in order to enforce access through the member
functions, and rename it to m_dwarf_version.

While we're at it, make sure that the version is set before read, to avoid
say returning true for "per_cu.version () < 5" if "per_cu.version () == 0".

Tested on x86_64-linux.
  • Loading branch information
vries committed Jul 2, 2022
1 parent 4722604 commit 33fd0a3
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 5 deletions.
10 changes: 7 additions & 3 deletions gdb/dwarf2/read.c
Original file line number Diff line number Diff line change
Expand Up @@ -6235,7 +6235,7 @@ cutu_reader::cutu_reader (dwarf2_per_cu_data *this_cu,
sig_type->type_offset_in_section =
this_cu->sect_off + to_underlying (sig_type->type_offset_in_tu);

this_cu->dwarf_version = cu->header.version;
this_cu->set_version (cu->header.version);
}
else
{
Expand All @@ -6249,7 +6249,7 @@ cutu_reader::cutu_reader (dwarf2_per_cu_data *this_cu,
this_cu->length = cu->header.get_length ();
else
gdb_assert (this_cu->length == cu->header.get_length ());
this_cu->dwarf_version = cu->header.version;
this_cu->set_version (cu->header.version);
}
}

Expand Down Expand Up @@ -7206,6 +7206,10 @@ read_comp_units_from_section (dwarf2_per_objfile *per_objfile,
this_cu->length = cu_header.length + cu_header.initial_length_size;
this_cu->is_dwz = is_dwz;
this_cu->section = section;
/* Init this asap, to avoid a data race in the set_version in
cutu_reader::cutu_reader (which may be run in parallel for the cooked
index case). */
this_cu->set_version (cu_header.version);

info_ptr = info_ptr + this_cu->length;
per_objfile->per_bfd->all_comp_units.push_back (std::move (this_cu));
Expand Down Expand Up @@ -11222,7 +11226,7 @@ open_and_init_dwo_file (dwarf2_cu *cu, const char *dwo_name,
create_cus_hash_table (per_objfile, cu, *dwo_file, dwo_file->sections.info,
dwo_file->cus);

if (cu->per_cu->dwarf_version < 5)
if (cu->per_cu->version () < 5)
{
create_debug_types_hash_table (per_objfile, dwo_file.get (),
dwo_file->sections.types, dwo_file->tus);
Expand Down
18 changes: 16 additions & 2 deletions gdb/dwarf2/read.h
Original file line number Diff line number Diff line change
Expand Up @@ -122,9 +122,11 @@ struct dwarf2_per_cu_data
sect_offset sect_off {};
unsigned int length = 0;

private:
/* DWARF standard version this data has been read from (such as 4 or 5). */
unsigned char dwarf_version = 0;
unsigned char m_dwarf_version = 0;

public:
/* Flag indicating this compilation unit will be read in before
any of the current compilation units are processed. */
unsigned int queued : 1;
Expand Down Expand Up @@ -287,7 +289,19 @@ struct dwarf2_per_cu_data
/* Return DWARF version number of this CU. */
short version () const
{
return dwarf_version;
/* Make sure it's set already. */
gdb_assert (m_dwarf_version != 0);
return m_dwarf_version;
}

void set_version (short version)
{
if (m_dwarf_version == 0)
/* Set if not set already. */
m_dwarf_version = version;
else
/* If already set, verify that it's the same value. */
gdb_assert (m_dwarf_version == version);
}

/* Free any cached file names. */
Expand Down

0 comments on commit 33fd0a3

Please sign in to comment.