Skip to content
This repository was archived by the owner on Aug 17, 2022. It is now read-only.

Commit 33fd0a3

Browse files
committed
[gdb/symtab] Fix data race on per_cu->dwarf_version
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.
1 parent 4722604 commit 33fd0a3

File tree

2 files changed

+23
-5
lines changed

2 files changed

+23
-5
lines changed

gdb/dwarf2/read.c

+7-3
Original file line numberDiff line numberDiff line change
@@ -6235,7 +6235,7 @@ cutu_reader::cutu_reader (dwarf2_per_cu_data *this_cu,
62356235
sig_type->type_offset_in_section =
62366236
this_cu->sect_off + to_underlying (sig_type->type_offset_in_tu);
62376237

6238-
this_cu->dwarf_version = cu->header.version;
6238+
this_cu->set_version (cu->header.version);
62396239
}
62406240
else
62416241
{
@@ -6249,7 +6249,7 @@ cutu_reader::cutu_reader (dwarf2_per_cu_data *this_cu,
62496249
this_cu->length = cu->header.get_length ();
62506250
else
62516251
gdb_assert (this_cu->length == cu->header.get_length ());
6252-
this_cu->dwarf_version = cu->header.version;
6252+
this_cu->set_version (cu->header.version);
62536253
}
62546254
}
62556255

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

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

11225-
if (cu->per_cu->dwarf_version < 5)
11229+
if (cu->per_cu->version () < 5)
1122611230
{
1122711231
create_debug_types_hash_table (per_objfile, dwo_file.get (),
1122811232
dwo_file->sections.types, dwo_file->tus);

gdb/dwarf2/read.h

+16-2
Original file line numberDiff line numberDiff line change
@@ -122,9 +122,11 @@ struct dwarf2_per_cu_data
122122
sect_offset sect_off {};
123123
unsigned int length = 0;
124124

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

129+
public:
128130
/* Flag indicating this compilation unit will be read in before
129131
any of the current compilation units are processed. */
130132
unsigned int queued : 1;
@@ -287,7 +289,19 @@ struct dwarf2_per_cu_data
287289
/* Return DWARF version number of this CU. */
288290
short version () const
289291
{
290-
return dwarf_version;
292+
/* Make sure it's set already. */
293+
gdb_assert (m_dwarf_version != 0);
294+
return m_dwarf_version;
295+
}
296+
297+
void set_version (short version)
298+
{
299+
if (m_dwarf_version == 0)
300+
/* Set if not set already. */
301+
m_dwarf_version = version;
302+
else
303+
/* If already set, verify that it's the same value. */
304+
gdb_assert (m_dwarf_version == version);
291305
}
292306

293307
/* Free any cached file names. */

0 commit comments

Comments
 (0)