From: Tom de Vries via Gdb-patches <gdb-patches@sourceware.org>
To: gdb-patches@sourceware.org
Cc: Tom Tromey <tom@tromey.com>
Subject: [PATCH][gdb/symtab] Force usage of all_comp_units.size (CU/TU/CUTU)
Date: Thu, 14 Jul 2022 16:43:52 +0200 [thread overview]
Message-ID: <20220714144351.GA31826@delia.home> (raw)
Hi,
I noticed this code in dw2_debug_names_iterator::next:
...
case DW_IDX_compile_unit:
/* Don't crash on bad data. */
if (ull >= per_bfd->all_comp_units.size ())
{
complaint (_(".debug_names entry has bad CU index %s"
" [in module %s]"),
pulongest (ull),
objfile_name (objfile));
continue;
}
per_cu = per_bfd->get_cu (ull);
break;
...
This code used to DTRT, before we started keeping both CUs and TUs in
all_comp_units.
It's just too easy to read "all_comp_units.size ()" and only think about CUs
and forget about TUs.
Introduce "all_comp_units.size (CU/TU/CUTU)", to make it explicit which of the
three we mean at every use, and replace:
- uses of tu_stats.nr_tus with all_comp_units.size (TU)
(apart from the use in print_tu_stats),
- uses of (all_comp_units.size () - tu_stats.nr_tus)
with all_comp_units.size (CU), and
- all other uses of "all_comp_units.size ()" with "all_comp_units.size (CUTU)".
Fix the problem in dw2_debug_names_iterator::next by using
"all_comp_units.size (CU)".
It's hard to produce a test-case for this, but let's try at least to trigger
the complaint somehow. We start out by creating an exec with .debug_types and
.debug_names:
...
$ gcc -g ~/hello.c -fdebug-types-section
$ gdb-add-index -dwarf-5 a.out
...
and verify that we don't see any complaints:
...
$ gdb -q -batch -iex "set complaints 100" ./a.out
...
We look at the CU and TU table using readelf -w and conclude that we have
nr_cus == 6 and nr_tus == 1.
Now override ull in dw2_debug_names_iterator::next for the DW_IDX_compile_unit
case to 6, and we have:
...
$ gdb -q -batch -iex "set complaints 100" ./a.out
During symbol reading: .debug_names entry has bad CU index 6 [in module a.out]
...
[ After this, it still crashes because this code in
dw2_debug_names_iterator::next:
...
/* Skip if already read in. */
if (m_per_objfile->symtab_set_p (per_cu))
goto again;
...
is called with per_cu == nullptr, but I consider this a separate issue. ]
Now revert the fix (all_comp_units.size (CU) -> all_comp_units.size (CUTU))
and observe that the complaint disappears, so we've confirmed that the fix is
required.
There's possibly a similar issue for .gdb_index in dw2_symtab_iter_next, filed
as PR29367.
Tested on x86_64-linux, with native and target board cc-with-debug-names.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29336
Any comments?
Thanks,
- Tom
[gdb/symtab] Force usage of all_comp_units.size (CU/TU/CUTU)
---
gdb/dwarf2/index-write.c | 11 ++++----
gdb/dwarf2/read.c | 67 +++++++++++++++++++++++++++++++-----------------
gdb/dwarf2/read.h | 32 ++++++++++++++++++++++-
3 files changed, 80 insertions(+), 30 deletions(-)
diff --git a/gdb/dwarf2/index-write.c b/gdb/dwarf2/index-write.c
index efd154d41df..fceef3d0b1e 100644
--- a/gdb/dwarf2/index-write.c
+++ b/gdb/dwarf2/index-write.c
@@ -1176,7 +1176,7 @@ write_gdbindex (dwarf2_per_objfile *per_objfile,
in the index file). This will later be needed to write the address
table. */
cu_index_map cu_index_htab;
- cu_index_htab.reserve (per_objfile->per_bfd->all_comp_units.size ());
+ cu_index_htab.reserve (per_objfile->per_bfd->all_comp_units.size (CUTU));
/* Store out the .debug_type CUs, if any. */
data_buf types_cu_list;
@@ -1187,7 +1187,7 @@ write_gdbindex (dwarf2_per_objfile *per_objfile,
int counter = 0;
int types_counter = 0;
- for (int i = 0; i < per_objfile->per_bfd->all_comp_units.size (); ++i)
+ for (int i = 0; i < per_objfile->per_bfd->all_comp_units.size (CUTU); ++i)
{
dwarf2_per_cu_data *per_cu
= per_objfile->per_bfd->all_comp_units[i].get ();
@@ -1270,7 +1270,7 @@ write_debug_names (dwarf2_per_objfile *per_objfile,
debug_names nametable (per_objfile, dwarf5_is_dwarf64, dwarf5_byte_order);
int counter = 0;
int types_counter = 0;
- for (int i = 0; i < per_objfile->per_bfd->all_comp_units.size (); ++i)
+ for (int i = 0; i < per_objfile->per_bfd->all_comp_units.size (CUTU); ++i)
{
dwarf2_per_cu_data *per_cu
= per_objfile->per_bfd->all_comp_units[i].get ();
@@ -1286,9 +1286,8 @@ write_debug_names (dwarf2_per_objfile *per_objfile,
}
/* Verify that all units are represented. */
- gdb_assert (counter == (per_objfile->per_bfd->all_comp_units.size ()
- - per_objfile->per_bfd->tu_stats.nr_tus));
- gdb_assert (types_counter == per_objfile->per_bfd->tu_stats.nr_tus);
+ gdb_assert (counter == per_objfile->per_bfd->all_comp_units.size (CU));
+ gdb_assert (types_counter == per_objfile->per_bfd->all_comp_units.size (TU));
for (const cooked_index_entry *entry : table->all_entries ())
nametable.insert (entry);
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 5ca787cc095..e554bc4f642 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -1391,7 +1391,7 @@ class all_comp_units_iterator
all_comp_units_iterator (dwarf2_per_bfd *per_bfd, bool start)
: m_per_bfd (per_bfd),
- m_index (start ? 0 : per_bfd->all_comp_units.size ())
+ m_index (start ? 0 : per_bfd->all_comp_units.size (CUTU))
{
}
@@ -1448,11 +1448,33 @@ class all_comp_units_range
dwarf2_per_bfd *m_per_bfd;
};
+/* See read.h. */
+
+all_comp_units_type::size_type
+all_comp_units_type::size (all_comp_units_size_type what) const noexcept
+{
+ all_comp_units_type::size_type nr_cus_tus
+ = ((std::vector<dwarf2_per_cu_data_up> *)(this))->size ();
+
+ if (what == CUTU)
+ return nr_cus_tus;
+
+ gdb_assert (m_per_bfd != nullptr);
+ size_type nr_tus = m_per_bfd->tu_stats.nr_tus;
+ if (what == CU)
+ return nr_cus_tus - nr_tus;
+ else if (what == TU)
+ return nr_tus;
+ else
+ gdb_assert_not_reached ("");
+}
+
/* See declaration. */
dwarf2_per_bfd::dwarf2_per_bfd (bfd *obfd, const dwarf2_debug_sections *names,
bool can_copy_)
: obfd (obfd),
+ all_comp_units (this),
can_copy (can_copy_)
{
if (names == NULL)
@@ -2092,7 +2114,7 @@ dwarf2_per_bfd::allocate_per_cu ()
{
dwarf2_per_cu_data_up result (new dwarf2_per_cu_data);
result->per_bfd = this;
- result->index = all_comp_units.size ();
+ result->index = all_comp_units.size (CUTU);
return result;
}
@@ -2103,7 +2125,7 @@ dwarf2_per_bfd::allocate_signatured_type (ULONGEST signature)
{
signatured_type_up result (new signatured_type (signature));
result->per_bfd = this;
- result->index = all_comp_units.size ();
+ result->index = all_comp_units.size (CUTU);
result->is_debug_types = true;
tu_stats.nr_tus++;
return result;
@@ -2297,7 +2319,7 @@ create_addrmap_from_index (dwarf2_per_objfile *per_objfile,
continue;
}
- if (cu_index >= per_bfd->all_comp_units.size ())
+ if (cu_index >= per_bfd->all_comp_units.size (CUTU))
{
complaint (_(".gdb_index address table has invalid CU number %u"),
(unsigned) cu_index);
@@ -2708,7 +2730,7 @@ dwarf2_read_gdb_index
per_bfd->index_table = std::move (map);
per_bfd->quick_file_names_table =
- create_quick_file_names_table (per_bfd->all_comp_units.size ());
+ create_quick_file_names_table (per_bfd->all_comp_units.size (CUTU));
return 1;
}
@@ -2975,7 +2997,7 @@ dw2_symtab_iter_next (struct dw2_symtab_iterator *iter,
&& symbol_kind != GDB_INDEX_SYMBOL_KIND_NONE);
/* Don't crash on bad data. */
- if (cu_index >= per_objfile->per_bfd->all_comp_units.size ())
+ if (cu_index >= per_objfile->per_bfd->all_comp_units.size (CUTU))
{
complaint (_(".gdb_index entry has bad CU index"
" [in module %s]"), objfile_name (per_objfile->objfile));
@@ -3056,7 +3078,7 @@ dwarf2_base_index_functions::print_stats (struct objfile *objfile,
return;
dwarf2_per_objfile *per_objfile = get_dwarf2_per_objfile (objfile);
- int total = per_objfile->per_bfd->all_comp_units.size ();
+ int total = per_objfile->per_bfd->all_comp_units.size (CUTU);
int count = 0;
for (int i = 0; i < total; ++i)
@@ -3090,7 +3112,7 @@ void
dwarf2_base_index_functions::expand_all_symtabs (struct objfile *objfile)
{
dwarf2_per_objfile *per_objfile = get_dwarf2_per_objfile (objfile);
- int total_units = per_objfile->per_bfd->all_comp_units.size ();
+ int total_units = per_objfile->per_bfd->all_comp_units.size (CUTU);
for (int i = 0; i < total_units; ++i)
{
@@ -4053,7 +4075,7 @@ dw2_expand_marked_cus
}
/* Don't crash on bad data. */
- if (cu_index >= per_objfile->per_bfd->all_comp_units.size ())
+ if (cu_index >= per_objfile->per_bfd->all_comp_units.size (CUTU))
{
complaint (_(".gdb_index entry has bad CU index"
" [in module %s]"), objfile_name (per_objfile->objfile));
@@ -4713,7 +4735,7 @@ dwarf2_read_debug_names (dwarf2_per_objfile *per_objfile)
per_bfd->index_table = std::move (map);
per_bfd->quick_file_names_table =
- create_quick_file_names_table (per_bfd->all_comp_units.size ());
+ create_quick_file_names_table (per_bfd->all_comp_units.size (CUTU));
return true;
}
@@ -4971,7 +4993,7 @@ dw2_debug_names_iterator::next ()
{
case DW_IDX_compile_unit:
/* Don't crash on bad data. */
- if (ull >= per_bfd->all_comp_units.size ())
+ if (ull >= per_bfd->all_comp_units.size (CU))
{
complaint (_(".debug_names entry has bad CU index %s"
" [in module %s]"),
@@ -4983,7 +5005,7 @@ dw2_debug_names_iterator::next ()
break;
case DW_IDX_type_unit:
/* Don't crash on bad data. */
- if (ull >= per_bfd->tu_stats.nr_tus)
+ if (ull >= per_bfd->all_comp_units.size (TU))
{
complaint (_(".debug_names entry has bad TU index %s"
" [in module %s]"),
@@ -4992,8 +5014,7 @@ dw2_debug_names_iterator::next ()
continue;
}
{
- int nr_cus = (per_bfd->all_comp_units.size ()
- - per_bfd->tu_stats.nr_tus);
+ int nr_cus = per_bfd->all_comp_units.size (CU);
per_cu = per_bfd->get_cu (nr_cus + ull);
}
break;
@@ -5314,7 +5335,7 @@ dwarf2_initialize_objfile (struct objfile *objfile)
create_all_comp_units (per_objfile);
per_bfd->quick_file_names_table
- = create_quick_file_names_table (per_bfd->all_comp_units.size ());
+ = create_quick_file_names_table (per_bfd->all_comp_units.size (CUTU));
objfile->qf.emplace_front (new readnow_functions);
return;
@@ -5609,7 +5630,7 @@ create_debug_types_hash_table (dwarf2_per_objfile *per_objfile,
static struct signatured_type *
add_type_unit (dwarf2_per_objfile *per_objfile, ULONGEST sig, void **slot)
{
- if (per_objfile->per_bfd->all_comp_units.size ()
+ if (per_objfile->per_bfd->all_comp_units.size (CUTU)
== per_objfile->per_bfd->all_comp_units.capacity ())
++per_objfile->per_bfd->tu_stats.nr_all_type_units_reallocs;
@@ -6865,7 +6886,7 @@ build_type_psymtabs (dwarf2_per_objfile *per_objfile,
/* It's up to the caller to not call us multiple times. */
gdb_assert (per_objfile->per_bfd->type_unit_groups == NULL);
- if (per_objfile->per_bfd->tu_stats.nr_tus == 0)
+ if (per_objfile->per_bfd->all_comp_units.size (TU) == 0)
return;
/* TUs typically share abbrev tables, and there can be way more TUs than
@@ -6892,7 +6913,7 @@ build_type_psymtabs (dwarf2_per_objfile *per_objfile,
/* Sort in a separate table to maintain the order of all_comp_units
for .gdb_index: TU indices directly index all_type_units. */
std::vector<tu_abbrev_offset> sorted_by_abbrev;
- sorted_by_abbrev.reserve (per_objfile->per_bfd->tu_stats.nr_tus);
+ sorted_by_abbrev.reserve (per_objfile->per_bfd->all_comp_units.size (TU));
for (const auto &cu : per_objfile->per_bfd->all_comp_units)
{
@@ -7048,7 +7069,7 @@ dwarf2_build_psymtabs_hard (dwarf2_per_objfile *per_objfile)
std::vector<std::unique_ptr<cooked_index>> indexes;
per_bfd->quick_file_names_table
- = create_quick_file_names_table (per_bfd->all_comp_units.size ());
+ = create_quick_file_names_table (per_bfd->all_comp_units.size (CUTU));
if (!per_bfd->debug_aranges.empty ())
read_addrmap_from_aranges (per_objfile, &per_bfd->debug_aranges,
index_storage.get_addrmap ());
@@ -23424,12 +23445,12 @@ static int
dwarf2_find_containing_comp_unit
(sect_offset sect_off,
unsigned int offset_in_dwz,
- const std::vector<dwarf2_per_cu_data_up> &all_comp_units)
+ const all_comp_units_type &all_comp_units)
{
int low, high;
low = 0;
- high = all_comp_units.size () - 1;
+ high = all_comp_units.size (CUTU) - 1;
while (high > low)
{
struct dwarf2_per_cu_data *mid_cu;
@@ -23473,7 +23494,7 @@ dwarf2_find_containing_comp_unit (sect_offset sect_off,
}
else
{
- if (low == per_bfd->all_comp_units.size () - 1
+ if (low == per_bfd->all_comp_units.size (CUTU) - 1
&& 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 ());
@@ -23508,7 +23529,7 @@ run_test ()
four->set_length (7);
four->is_dwz = 1;
- std::vector<dwarf2_per_cu_data_up> units;
+ all_comp_units_type units (nullptr);
units.push_back (std::move (one));
units.push_back (std::move (two));
units.push_back (std::move (three));
diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
index c2f86a9d367..89e8ad29758 100644
--- a/gdb/dwarf2/read.h
+++ b/gdb/dwarf2/read.h
@@ -410,6 +410,36 @@ struct signatured_type : public dwarf2_per_cu_data
using signatured_type_up = std::unique_ptr<signatured_type>;
+/* To specify whether we want the number of CUs, TUs, or both CUs and TUs from
+ all_comp_units.size (). */
+
+enum all_comp_units_size_type
+{
+ CU,
+ TU,
+ CUTU
+};
+
+/* Variant of std::vector<dwarf2_per_cu_data_up> that deletes the size ()
+ member function and replaces it with size (all_comp_units_size_type).
+ This forces us to specify explicitly whether we want the number of CUs, TUs
+ or the total at every use. */
+
+class all_comp_units_type : public std::vector<dwarf2_per_cu_data_up>
+{
+public:
+ all_comp_units_type (struct dwarf2_per_bfd *per_bfd)
+ {
+ m_per_bfd = per_bfd;
+ }
+
+ size_type size () const noexcept = delete;
+ size_type size (all_comp_units_size_type what) const noexcept;
+
+private:
+ struct dwarf2_per_bfd *m_per_bfd = nullptr;
+};
+
/* Some DWARF data can be shared across objfiles who share the same BFD,
this data is stored in this object.
@@ -489,7 +519,7 @@ struct dwarf2_per_bfd
/* Table of all the compilation units. This is used to locate
the target compilation unit of a particular reference. */
- std::vector<dwarf2_per_cu_data_up> all_comp_units;
+ all_comp_units_type all_comp_units;
/* Table of struct type_unit_group objects.
The hash key is the DW_AT_stmt_list value. */
next reply other threads:[~2022-07-14 14:44 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-14 14:43 Tom de Vries via Gdb-patches [this message]
2022-07-21 11:10 ` [committed][gdb/symtab] Fix bad compile unit index complaint Tom de Vries via Gdb-patches
2022-07-21 11:20 ` [PATCH, v2][gdb/symtab] Force usage of all_comp_units.size (CU/TU/CUTU) Tom de Vries via Gdb-patches
2022-08-08 14:26 ` [PATCH][gdb/symtab] " Tom de Vries via Gdb-patches
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220714144351.GA31826@delia.home \
--to=gdb-patches@sourceware.org \
--cc=tdevries@suse.de \
--cc=tom@tromey.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox