public inbox for gdb-cvs@sourceware.org
help / color / mirror / Atom feed
* [binutils-gdb] [gdb/symtab] Fix data race in per_cu->length
@ 2022-07-11 9:37 Tom de Vries
0 siblings, 0 replies; only message in thread
From: Tom de Vries @ 2022-07-11 9:37 UTC (permalink / raw)
To: gdb-cvs
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=53a7a7e17c5d21b7b182ddf6bd8bfc092af196f5
commit 53a7a7e17c5d21b7b182ddf6bd8bfc092af196f5
Author: Tom de Vries <tdevries@suse.de>
Date: Mon Jul 11 11:36:54 2022 +0200
[gdb/symtab] Fix data race in per_cu->length
With gdb build with -fsanitize=thread and test-case
gdb.dwarf2/dw4-sig-types.exp and target board cc-with-dwz-m we run into a data
race between:
...
Write of size 4 at 0x7b2800002268 by thread T4:^M
#0 cutu_reader::cutu_reader(dwarf2_per_cu_data*, dwarf2_per_objfile*, \
abbrev_table*, dwarf2_cu*, bool, abbrev_cache*) gdb/dwarf2/read.c:6236 \
(gdb+0x82f525)^M
...
and this read:
...
Previous read of size 4 at 0x7b2800002268 by thread T1:^M
#0 dwarf2_find_containing_comp_unit gdb/dwarf2/read.c:23444 \
(gdb+0x86e22e)^M
...
In other words, between this write:
...
this_cu->length = cu->header.get_length ();
...
and this read:
...
&& mid_cu->sect_off + mid_cu->length > sect_off))
...
of per_cu->length.
Fix this similar to the per_cu->dwarf_version case, by only setting it if
needed, and otherwise verifying that the same value is used. [ Note that the
same code is already present in the other cutu_reader constructor. ]
Move this logic into into a member function set_length 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_length.
This exposes (running test-case gdb.dwarf2/fission-reread.exp) that in
fill_in_sig_entry_from_dwo_entry, the m_length field is overwritten.
For now, allow for that exception.
While we're at it, make sure that the length is set before read.
Tested on x86_64-linux.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29344
Diff:
---
gdb/dwarf2/index-write.c | 2 +-
gdb/dwarf2/read.c | 43 ++++++++++++++++++++-----------------------
gdb/dwarf2/read.h | 21 ++++++++++++++++++++-
3 files changed, 41 insertions(+), 25 deletions(-)
diff --git a/gdb/dwarf2/index-write.c b/gdb/dwarf2/index-write.c
index c3aad8dd999..efd154d41df 100644
--- a/gdb/dwarf2/index-write.c
+++ b/gdb/dwarf2/index-write.c
@@ -1214,7 +1214,7 @@ write_gdbindex (dwarf2_per_objfile *per_objfile,
sig_type->signature);
}
else
- cu_list.append_uint (8, BFD_ENDIAN_LITTLE, per_cu->length);
+ cu_list.append_uint (8, BFD_ENDIAN_LITTLE, per_cu->length ());
++this_counter;
}
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 40a18796f8d..31000877f42 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -2120,7 +2120,7 @@ create_cu_from_index_list (dwarf2_per_bfd *per_bfd,
{
dwarf2_per_cu_data_up the_cu = per_bfd->allocate_per_cu ();
the_cu->sect_off = sect_off;
- the_cu->length = length;
+ the_cu->set_length (length);
the_cu->section = section;
the_cu->is_dwz = is_dwz;
return the_cu;
@@ -5655,7 +5655,7 @@ fill_in_sig_entry_from_dwo_entry (dwarf2_per_objfile *per_objfile,
sig_entry->section = dwo_entry->section;
sig_entry->sect_off = dwo_entry->sect_off;
- sig_entry->length = dwo_entry->length;
+ sig_entry->set_length (dwo_entry->length, false);
sig_entry->reading_dwo_directly = 1;
sig_entry->per_bfd = per_bfd;
sig_entry->type_offset_in_tu = dwo_entry->type_offset_in_tu;
@@ -6233,7 +6233,7 @@ cutu_reader::cutu_reader (dwarf2_per_cu_data *this_cu,
/* LENGTH has not been set yet for type units if we're
using .gdb_index. */
- this_cu->length = cu->header.get_length ();
+ this_cu->set_length (cu->header.get_length ());
/* Establish the type offset that can be used to lookup the type. */
sig_type->type_offset_in_section =
@@ -6249,16 +6249,13 @@ cutu_reader::cutu_reader (dwarf2_per_cu_data *this_cu,
rcuh_kind::COMPILE);
gdb_assert (this_cu->sect_off == cu->header.sect_off);
- if (this_cu->length == 0)
- this_cu->length = cu->header.get_length ();
- else
- gdb_assert (this_cu->length == cu->header.get_length ());
+ this_cu->set_length (cu->header.get_length ());
this_cu->set_version (cu->header.version);
}
}
/* Skip dummy compilation units. */
- if (info_ptr >= begin_info_ptr + this_cu->length
+ if (info_ptr >= begin_info_ptr + this_cu->length ()
|| peek_abbrev_code (abfd, info_ptr) == 0)
{
dummy_p = true;
@@ -6412,10 +6409,10 @@ cutu_reader::cutu_reader (dwarf2_per_cu_data *this_cu,
m_new_cu->str_offsets_base = parent_cu->str_offsets_base;
m_new_cu->addr_base = parent_cu->addr_base;
}
- this_cu->length = m_new_cu->header.get_length ();
+ this_cu->set_length (m_new_cu->header.get_length ());
/* Skip dummy compilation units. */
- if (info_ptr >= begin_info_ptr + this_cu->length
+ if (info_ptr >= begin_info_ptr + this_cu->length ()
|| peek_abbrev_code (abfd, info_ptr) == 0)
{
dummy_p = true;
@@ -7207,7 +7204,7 @@ read_comp_units_from_section (dwarf2_per_objfile *per_objfile,
*slot = sig_ptr;
}
this_cu->sect_off = sect_off;
- this_cu->length = cu_header.get_length ();
+ this_cu->set_length (cu_header.get_length ());
this_cu->is_dwz = is_dwz;
this_cu->section = section;
/* Init this asap, to avoid a data race in the set_version in
@@ -7215,7 +7212,7 @@ read_comp_units_from_section (dwarf2_per_objfile *per_objfile,
index case). */
this_cu->set_version (cu_header.version);
- info_ptr = info_ptr + this_cu->length;
+ info_ptr = info_ptr + this_cu->length ();
per_objfile->per_bfd->all_comp_units.push_back (std::move (this_cu));
}
}
@@ -9904,7 +9901,7 @@ create_dwo_cu_reader (const struct die_reader_specs *reader,
dwo_unit->signature = *signature;
dwo_unit->section = section;
dwo_unit->sect_off = sect_off;
- dwo_unit->length = cu->per_cu->length;
+ dwo_unit->length = cu->per_cu->length ();
dwarf_read_debug_printf (" offset %s, dwo_id %s",
sect_offset_str (sect_off),
@@ -9951,7 +9948,7 @@ create_cus_hash_table (dwarf2_per_objfile *per_objfile,
if (!reader.dummy_p)
create_dwo_cu_reader (&reader, reader.info_ptr, reader.comp_unit_die,
&dwo_file, &read_unit);
- info_ptr += per_cu.length;
+ info_ptr += per_cu.length ();
// If the unit could not be parsed, skip it.
if (read_unit.dwo_file == NULL)
@@ -23459,7 +23456,7 @@ dwarf2_find_containing_comp_unit
mid_cu = all_comp_units[mid].get ();
if (mid_cu->is_dwz > offset_in_dwz
|| (mid_cu->is_dwz == offset_in_dwz
- && mid_cu->sect_off + mid_cu->length > sect_off))
+ && mid_cu->sect_off + mid_cu->length () > sect_off))
high = mid;
else
low = mid + 1;
@@ -23495,9 +23492,9 @@ dwarf2_find_containing_comp_unit (sect_offset sect_off,
else
{
if (low == per_bfd->all_comp_units.size () - 1
- && sect_off >= this_cu->sect_off + this_cu->length)
+ && sect_off >= this_cu->sect_off + this_cu->length ())
error (_("invalid dwarf2 offset %s"), sect_offset_str (sect_off));
- gdb_assert (sect_off < this_cu->sect_off + this_cu->length);
+ gdb_assert (sect_off < this_cu->sect_off + this_cu->length ());
return this_cu;
}
}
@@ -23519,14 +23516,14 @@ run_test ()
dwarf2_per_cu_data_up four (new dwarf2_per_cu_data);
dwarf2_per_cu_data *four_ptr = four.get ();
- one->length = 5;
- two->sect_off = sect_offset (one->length);
- two->length = 7;
+ one->set_length (5);
+ two->sect_off = sect_offset (one->length ());
+ two->set_length (7);
- three->length = 5;
+ three->set_length (5);
three->is_dwz = 1;
- four->sect_off = sect_offset (three->length);
- four->length = 7;
+ four->sect_off = sect_offset (three->length ());
+ four->set_length (7);
four->is_dwz = 1;
std::vector<dwarf2_per_cu_data_up> units;
diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
index c9afa074ae9..5e0ce5e91a2 100644
--- a/gdb/dwarf2/read.h
+++ b/gdb/dwarf2/read.h
@@ -120,9 +120,10 @@ struct dwarf2_per_cu_data
If the DIE refers to a DWO file, this is always of the original die,
not the DWO file. */
sect_offset sect_off {};
- unsigned int length = 0;
private:
+ unsigned int m_length = 0;
+
/* DWARF standard version this data has been read from (such as 4 or 5). */
unsigned char m_dwarf_version = 0;
@@ -288,6 +289,24 @@ public:
header for this CU. */
int ref_addr_size () const;
+ /* Return length of this CU. */
+ unsigned int length () const
+ {
+ /* Make sure it's set already. */
+ gdb_assert (m_length != 0);
+ return m_length;
+ }
+
+ void set_length (unsigned int length, bool strict_p = true)
+ {
+ if (m_length == 0)
+ /* Set if not set already. */
+ m_length = length;
+ else if (strict_p)
+ /* If already set, verify that it's the same value. */
+ gdb_assert (m_length == length);
+ }
+
/* Return DWARF version number of this CU. */
short version () const
{
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2022-07-11 9:37 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-11 9:37 [binutils-gdb] [gdb/symtab] Fix data race in per_cu->length Tom de Vries
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).