[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.

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index d5088395fb1f32f11d38349bbe8a272ea657646f..410cc909c2601dd59a6921b111d6a628325b1abb 100644 (file)
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -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
        {
@@ -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);
        }
     }
 
@@ -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));
@@ -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);

diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
index 51e02dfc4572761ce80d814fe11c4d0e0a28ac6f..b7a03933aa52ea99c68be0753f46d579587ea4e8 100644 (file)
--- a/gdb/dwarf2/read.h
+++ b/gdb/dwarf2/read.h
@@ -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;
@@ -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.  */