Fix race in DWARF reader

The recent change to record the DWARF language in the per-CU data
yielded a race warning in my testing:

ThreadSanitizer: data race ../../binutils-gdb/gdb/dwarf2/read.c:21779 in prepare_one_comp_unit

This patch fixes the bug by applying the same style of fix that was
done for the ordinary (gdb) language.

I wonder if this code could be improved.  Requiring an atomic for the
language in particular seems unfortunate, as it is often consulted
during index finalization.  However, I haven't investigated this.

Regression tested on x86-64 Fedora 38.

Reviewed-by: Tom de Vries <tdevries@suse.de>
This commit is contained in:
Tom Tromey 2023-10-17 14:32:26 -06:00
parent 1b1b9bc05f
commit 379435351c
3 changed files with 41 additions and 25 deletions

View File

@ -1205,7 +1205,7 @@ write_shortcuts_table (cooked_index *table, data_buf &shortcuts,
if (main_info != nullptr)
{
dw_lang = main_info->per_cu->dw_lang;
dw_lang = main_info->per_cu->dw_lang ();
if (dw_lang != 0)
{

View File

@ -21615,6 +21615,26 @@ dwarf2_per_cu_data::ref_addr_size () const
return header->offset_size;
}
/* See read.h. */
void
dwarf2_per_cu_data::set_lang (enum language lang,
dwarf_source_language dw_lang)
{
if (unit_type () == DW_UT_partial)
return;
/* Set if not set already. */
packed<language, LANGUAGE_BYTES> new_value = lang;
packed<language, LANGUAGE_BYTES> old_value = m_lang.exchange (new_value);
/* If already set, verify that it's the same value. */
gdb_assert (old_value == language_unknown || old_value == lang);
packed<dwarf_source_language, 2> new_dw = dw_lang;
packed<dwarf_source_language, 2> old_dw = m_dw_lang.exchange (new_dw);
gdb_assert (old_dw == 0 || old_dw == dw_lang);
}
/* A helper function for dwarf2_find_containing_comp_unit that returns
the index of the result, and that searches a vector. It will
return a result even if the offset in question does not actually
@ -21776,7 +21796,6 @@ prepare_one_comp_unit (struct dwarf2_cu *cu, struct die_info *comp_unit_die,
else
lang = pretend_language;
cu->per_cu->dw_lang = dw_lang;
cu->language_defn = language_def (lang);
switch (comp_unit_die->tag)
@ -21796,7 +21815,7 @@ prepare_one_comp_unit (struct dwarf2_cu *cu, struct die_info *comp_unit_die,
sect_offset_str (cu->per_cu->sect_off));
}
cu->per_cu->set_lang (lang);
cu->per_cu->set_lang (lang, dw_lang);
}
/* See read.h. */

View File

@ -183,6 +183,15 @@ private:
/* The language of this CU. */
std::atomic<packed<language, LANGUAGE_BYTES>> m_lang {language_unknown};
/* The original DW_LANG_* value of the CU, as provided to us by
DW_AT_language. It is interesting to keep this value around in cases where
we can't use the values from the language enum, as the mapping to them is
lossy, and, while that is usually fine, things like the index have an
understandable bias towards not exposing internal GDB structures to the
outside world, and so prefer to use DWARF constants in their stead. */
std::atomic<packed<dwarf_source_language, 2>> m_dw_lang
{ (dwarf_source_language) 0 };
public:
/* True if this CU has been scanned by the indexer; false if
not. */
@ -245,14 +254,6 @@ public:
functions above. */
std::vector <dwarf2_per_cu_data *> *imported_symtabs = nullptr;
/* The original DW_LANG_* value of the CU, as provided to us by
DW_AT_language. It is interesting to keep this value around in cases where
we can't use the values from the language enum, as the mapping to them is
lossy, and, while that is usually fine, things like the index have an
understandable bias towards not exposing internal GDB structures to the
outside world, and so prefer to use DWARF constants in their stead. */
dwarf_source_language dw_lang;
/* Return true of IMPORTED_SYMTABS is empty or not yet allocated. */
bool imported_symtabs_empty () const
{
@ -365,20 +366,16 @@ public:
return l;
}
void set_lang (enum language lang)
{
if (unit_type () == DW_UT_partial)
return;
/* Set if not set already. */
packed<language, LANGUAGE_BYTES> nope = language_unknown;
if (m_lang.compare_exchange_strong (nope, lang))
return;
/* If already set, verify that it's the same value. */
nope = lang;
if (m_lang.compare_exchange_strong (nope, lang))
return;
gdb_assert_not_reached ();
}
/* Return the language of this CU, as a DWARF DW_LANG_* value. This
may be 0 in some situations. */
dwarf_source_language dw_lang () const
{ return m_dw_lang.load (); }
/* Set the language of this CU. LANG is the language in gdb terms,
and DW_LANG is the language as a DW_LANG_* value. These may
differ, as DW_LANG can be 0 for included units, whereas in this
situation LANG would be set by the importing CU. */
void set_lang (enum language lang, dwarf_source_language dw_lang);
/* Free any cached file names. */
void free_cached_file_names ();