From: Tom de Vries via Gdb-patches <gdb-patches@sourceware.org>
To: gdb-patches@sourceware.org
Cc: Tom Tromey <tom@tromey.com>
Subject: [PATCH, v2][gdb/symtab] Force usage of all_comp_units.size (CU/TU/CUTU)
Date: Thu, 21 Jul 2022 13:20:52 +0200 [thread overview]
Message-ID: <da8c7473-f2c5-2957-3e62-b6a088042cb8@suse.de> (raw)
In-Reply-To: <3b04b910-f020-7dba-54cd-46e23eb9657c@suse.de>
[-- Attachment #1: Type: text/plain, Size: 1320 bytes --]
[ was: Re: [committed][gdb/symtab] Fix bad compile unit index complaint ]
On 7/21/22 13:10, Tom de Vries wrote:
> [ was: Re: [PATCH][gdb/symtab] Force usage of all_comp_units.size
> (CU/TU/CUTU) ]
>
> On 7/14/22 16:43, Tom de Vries wrote:
>> 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.
>>
>
> I've dropped the all_comp_units.size (CU/TU/CUTU) part, since that
> somewhat violates the One-Patch-Per-Independent=Change rule.
And here is it, independent of the dw2_debug_names_iterator::next issue.
Thanks,
- Tom
[-- Attachment #2: 0001-gdb-symtab-Force-usage-of-all_comp_units.size-CU-TU-CUTU.patch --]
[-- Type: text/x-patch, Size: 13233 bytes --]
[gdb/symtab] Force usage of all_comp_units.size (CU/TU/CUTU)
It's 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)".
Tested on x86_64-linux.
---
gdb/dwarf2/index-write.c | 11 +++---
gdb/dwarf2/read.c | 87 +++++++++++++++++++++++++++++-------------------
gdb/dwarf2/read.h | 32 +++++++++++++++++-
3 files changed, 88 insertions(+), 42 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 42230607fe0..3dbf306ba86 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;
}
@@ -4970,24 +4992,20 @@ dw2_debug_names_iterator::next ()
switch (attr.dw_idx)
{
case DW_IDX_compile_unit:
- {
- /* Don't crash on bad data. */
- int nr_cus = (per_bfd->all_comp_units.size ()
- - per_bfd->tu_stats.nr_tus);
- if (ull >= nr_cus)
- {
- complaint (_(".debug_names entry has bad CU index %s"
- " [in module %s]"),
- pulongest (ull),
- objfile_name (objfile));
- continue;
- }
- }
+ /* Don't crash on bad data. */
+ if (ull >= per_bfd->all_comp_units.size (CU))
+ {
+ 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;
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]"),
@@ -4996,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;
@@ -5322,7 +5339,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;
@@ -5617,7 +5634,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;
@@ -6873,7 +6890,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
@@ -6900,7 +6917,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)
{
@@ -7056,7 +7073,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 ());
@@ -23434,12 +23451,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;
@@ -23483,7 +23500,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 ());
@@ -23518,7 +23535,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 prev parent reply other threads:[~2022-07-21 11:21 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-14 14:43 [PATCH][gdb/symtab] " Tom de Vries via Gdb-patches
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 ` Tom de Vries via Gdb-patches [this message]
2022-08-08 14:26 ` [PATCH][gdb/symtab] Force usage of all_comp_units.size (CU/TU/CUTU) 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=da8c7473-f2c5-2957-3e62-b6a088042cb8@suse.de \
--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