* [PATCH][gdb/symtab] Force usage of all_comp_units.size (CU/TU/CUTU)
@ 2022-07-14 14:43 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-08-08 14:26 ` [PATCH][gdb/symtab] " Tom de Vries via Gdb-patches
0 siblings, 2 replies; 4+ messages in thread
From: Tom de Vries via Gdb-patches @ 2022-07-14 14:43 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
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. */
^ permalink raw reply [flat|nested] 4+ messages in thread
* [committed][gdb/symtab] Fix bad compile unit index complaint
2022-07-14 14:43 [PATCH][gdb/symtab] Force usage of all_comp_units.size (CU/TU/CUTU) Tom de Vries via Gdb-patches
@ 2022-07-21 11:10 ` 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
1 sibling, 1 reply; 4+ messages in thread
From: Tom de Vries via Gdb-patches @ 2022-07-21 11:10 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
[-- Attachment #1: Type: text/plain, Size: 985 bytes --]
[ 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.
That also makes the patch trivial, so committed as attached.
Thanks,
- Tom
[-- Attachment #2: 0001-gdb-symtab-Fix-bad-compile-unit-index-complaint.patch --]
[-- Type: text/x-patch, Size: 3338 bytes --]
[gdb/symtab] Fix bad compile unit index complaint
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.
Fix by using "per_bfd->all_comp_units.size () - per_bfd->tu_stats.nr_tus"
instead.
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.
Fix this by skipping the entry if per_cu == nullptr.
Now revert the fix and observe that the complaint disappears, so we've
confirmed that the fix is required.
A somewhat similar issue for .gdb_index in dw2_symtab_iter_next has been 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
---
gdb/dwarf2/read.c | 26 +++++++++++++++++---------
1 file changed, 17 insertions(+), 9 deletions(-)
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index bcd01107377..42230607fe0 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -4970,15 +4970,19 @@ dw2_debug_names_iterator::next ()
switch (attr.dw_idx)
{
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;
- }
+ {
+ /* 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;
+ }
+ }
per_cu = per_bfd->get_cu (ull);
break;
case DW_IDX_type_unit:
@@ -5016,6 +5020,10 @@ dw2_debug_names_iterator::next ()
}
}
+ /* Skip if we couldn't find a valid CU/TU index. */
+ if (per_cu == nullptr)
+ goto again;
+
/* Skip if already read in. */
if (m_per_objfile->symtab_set_p (per_cu))
goto again;
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH, v2][gdb/symtab] Force usage of all_comp_units.size (CU/TU/CUTU)
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
0 siblings, 0 replies; 4+ messages in thread
From: Tom de Vries via Gdb-patches @ 2022-07-21 11:20 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
[-- 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. */
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH][gdb/symtab] Force usage of all_comp_units.size (CU/TU/CUTU)
2022-07-14 14:43 [PATCH][gdb/symtab] Force usage of all_comp_units.size (CU/TU/CUTU) 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-08-08 14:26 ` Tom de Vries via Gdb-patches
1 sibling, 0 replies; 4+ messages in thread
From: Tom de Vries via Gdb-patches @ 2022-08-08 14:26 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
On 7/14/22 16:43, Tom de Vries wrote:
> 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.
I've submitted a test-case for this here (
https://sourceware.org/pipermail/gdb-patches/2022-August/191274.html ).
Thanks,
- Tom
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-08-08 14:27 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-14 14:43 [PATCH][gdb/symtab] Force usage of all_comp_units.size (CU/TU/CUTU) 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 ` [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
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox