From b3fa38aef59d97a8ce57395b1f3902f6361d9bd4 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Mon, 10 Mar 2025 12:47:21 -0400 Subject: [PATCH] gdb/dwarf: move index unit vectors to debug names reader and use them Since these vectors contain the CU and TU lists as found in the .debug_names header, it seems like they are meant to be used by the .debug_names reader when handling a DW_IDX_compile_unit or DW_IDX_type_unit attribute. The value of the attribute would translate directly into an index into one of these vectors. However there's something fishy: it looks like these vectors aren't actually used in practice. They are used in the dwarf2_per_bfd::get_index_{c,t}u methods, which in turn aren't used anywhere. The handlers of DW_IDX_compile_unit and DW_IDX_type_unit use the dwarf2_per_bfd::get_cu method, assuming that all compile units are placed before type units in the dwarf2_per_bfd::all_units vector. I see several problems with that: 1. I found out [1] that the dwarf2_per_bfd::all_units didn't always have the CUs before the TUs. So indexing dwarf2_per_bfd::all_units with that assumption will not work. 2. The dwarf2_find_containing_comp_unit function assumes an ordering of units by section offset (among other criteria) in order to do a binary search. Even though it's probably commonly the case, nothing guarantees that the order of CUs and TUs in the .debug_names header (which defines the indices used to refer to them) will be sorted by section offset. It's not possible to make dwarf2_find_containing_comp_unit (assuming it wants to do a binary search by section offset) and the DW_IDX_compile_unit / DW_IDX_type_unit handlers use the same vector. 3. I have not tested this, but in the presence of a dwz supplementary file, the .debug_names reader should probably not put the units from the main and dwz files in the same vectors to look them up by index. Presumably, if both the main and dwz files have a .debug_names index, they have distinct CU / TU lists. So, an CU index of 1 in an index entry in the main file would refer to a different CU than an index of 1 in an index entry in the dwz file. The current code doesn't seem to account for that, it just indexes dwarf2_per_bfd::all_units. Since those vectors are kind of specific to the .debug_names reader, move them there, in the mapped_debug_names_reader struct. Then, update the handlers of DW_IDX_compile_unit and DW_IDX_type_unit to use them. [1] https://inbox.sourceware.org/gdb-patches/87a5ab5i5m.fsf@tromey.com/T/#mbdcfe35f94db33e59500eb0d3d225661cab016a4 Change-Id: I3958d70bb3875268143471da745aa09336ab2500 --- gdb/dwarf2/read-debug-names.c | 142 +++++++++++++++++----------------- gdb/dwarf2/read.h | 17 ---- 2 files changed, 72 insertions(+), 87 deletions(-) diff --git a/gdb/dwarf2/read-debug-names.c b/gdb/dwarf2/read-debug-names.c index 6e9ea667c3a..1d32b378936 100644 --- a/gdb/dwarf2/read-debug-names.c +++ b/gdb/dwarf2/read-debug-names.c @@ -114,6 +114,14 @@ struct mapped_debug_names_reader gdb::unordered_map abbrev_map; + /* List of CUs in the same order as found in the index header (DWARF 5 section + 6.1.1.4.2). */ + std::vector comp_units; + + /* List of local TUs in the same order as found in the index (DWARF 5 section + 6.1.1.4.3). */ + std::vector type_units; + /* Even though the scanning of .debug_names and creation of the cooked index entries is done serially, we create multiple shards so that the finalization step can be parallelized. The shards are filled in a round @@ -231,7 +239,7 @@ mapped_debug_names_reader::scan_one_entry (const char *name, case DW_IDX_compile_unit: { /* Don't crash on bad data. */ - if (ull >= per_objfile->per_bfd->all_comp_units.size ()) + if (ull >= this->comp_units.size ()) { complaint (_(".debug_names entry has bad CU index %s" " [in module %s]"), @@ -239,30 +247,31 @@ mapped_debug_names_reader::scan_one_entry (const char *name, bfd_get_filename (abfd)); continue; } + + per_cu = this->comp_units[ull]; + break; } - per_cu = per_objfile->per_bfd->get_cu (ull); - break; case DW_IDX_type_unit: - /* Don't crash on bad data. */ - if (ull >= per_objfile->per_bfd->all_type_units.size ()) - { - complaint (_(".debug_names entry has bad TU index %s" - " [in module %s]"), - pulongest (ull), - bfd_get_filename (abfd)); - continue; - } { - int nr_cus = per_objfile->per_bfd->all_comp_units.size (); - per_cu = per_objfile->per_bfd->get_cu (nr_cus + ull); + /* Don't crash on bad data. */ + if (ull >= this->type_units.size ()) + { + complaint (_(".debug_names entry has bad TU index %s" + " [in module %s]"), + pulongest (ull), + bfd_get_filename (abfd)); + continue; + } + + per_cu = this->type_units[ull]; + break; } - break; case DW_IDX_die_offset: die_offset = sect_offset (ull); /* In a per-CU index (as opposed to a per-module index), index entries without CU attribute implicitly refer to the single CU. */ - if (per_cu == NULL) - per_cu = per_objfile->per_bfd->get_cu (0); + if (per_cu == nullptr) + per_cu = this->comp_units[0]; break; case DW_IDX_parent: parent = ull; @@ -442,13 +451,16 @@ cooked_index_worker_debug_names::do_reading () bfd_thread_cleanup (); } -/* Check the signatured type hash table from .debug_names. */ +/* Build the list of TUs (mapped_debug_names_reader::type_units) from the index + header and verify that it matches the list of TUs read from the DIEs in + `.debug_info`. + + Return true if they match, false otherwise. */ static bool -check_signatured_type_table_from_debug_names - (dwarf2_per_objfile *per_objfile, - const mapped_debug_names_reader &map, - struct dwarf2_section_info *section) +build_and_check_tu_list_from_debug_names (dwarf2_per_objfile *per_objfile, + mapped_debug_names_reader &map, + dwarf2_section_info *section) { struct objfile *objfile = per_objfile->objfile; dwarf2_per_bfd *per_bfd = per_objfile->per_bfd; @@ -479,7 +491,8 @@ check_signatured_type_table_from_debug_names " ignoring .debug_names.")); return false; } - per_bfd->all_comp_units_index_tus.push_back (per_bfd->get_cu (j)); + + map.type_units.emplace_back (per_bfd->get_cu (j)); } return true; } @@ -702,42 +715,13 @@ read_debug_names_from_section (dwarf2_per_objfile *per_objfile, list. */ static bool -check_cus_from_debug_names_list (dwarf2_per_bfd *per_bfd, - const mapped_debug_names_reader &map, - dwarf2_section_info §ion, - bool is_dwz) +build_and_check_cu_list_from_debug_names (dwarf2_per_bfd *per_bfd, + mapped_debug_names_reader &map, + dwarf2_section_info §ion, + bool is_dwz) { - int nr_cus = per_bfd->all_comp_units.size (); - if (!map.augmentation_is_gdb) - { - uint32_t j = 0; - for (uint32_t i = 0; i < map.cu_count; ++i) - { - sect_offset sect_off - = (sect_offset) (extract_unsigned_integer - (map.cu_table_reordered + i * map.offset_size, - map.offset_size, - map.dwarf5_byte_order)); - bool found = false; - for (; j < nr_cus; j++) - if (per_bfd->get_cu (j)->sect_off == sect_off) - { - found = true; - break; - } - if (!found) - { - warning (_("Section .debug_names has incorrect entry in CU table," - " ignoring .debug_names.")); - return false; - } - per_bfd->all_comp_units_index_cus.push_back (per_bfd->get_cu (j)); - } - return true; - } - - if (map.cu_count != nr_cus) + if (map.cu_count != per_bfd->num_comp_units) { warning (_("Section .debug_names has incorrect number of CUs in CU table," " ignoring .debug_names.")); @@ -746,40 +730,58 @@ check_cus_from_debug_names_list (dwarf2_per_bfd *per_bfd, for (uint32_t i = 0; i < map.cu_count; ++i) { + /* Read one entry from the CU list. */ sect_offset sect_off = (sect_offset) (extract_unsigned_integer (map.cu_table_reordered + i * map.offset_size, map.offset_size, map.dwarf5_byte_order)); - if (sect_off != per_bfd->get_cu (i)->sect_off) + dwarf2_per_cu *found = nullptr; + + /* Find the matching dwarf2_per_cu. */ + for (auto &unit : per_bfd->all_units) + if (unit->sect_off == sect_off && !unit->is_debug_types + && unit->is_dwz == is_dwz) + { + found = unit.get (); + break; + } + + if (found == nullptr) { warning (_("Section .debug_names has incorrect entry in CU table," " ignoring .debug_names.")); return false; } + + map.comp_units.emplace_back (found); } return true; } -/* Read the CU list from the mapped index, and use it to create all - the CU objects for this dwarf2_per_objfile. */ +/* Build the list of CUs (mapped_debug_names_reader::compile_units) from the + index header and verify that it matches the list of CUs read from the DIEs in + `.debug_info`. + + Return true if they match, false otherwise. */ static bool -check_cus_from_debug_names (dwarf2_per_bfd *per_bfd, - const mapped_debug_names_reader &map, - const mapped_debug_names_reader &dwz_map) +build_and_check_cu_lists_from_debug_names (dwarf2_per_bfd *per_bfd, + mapped_debug_names_reader &map, + mapped_debug_names_reader &dwz_map) { - if (!check_cus_from_debug_names_list (per_bfd, map, per_bfd->infos[0], - false /* is_dwz */)) + if (!build_and_check_cu_list_from_debug_names (per_bfd, map, + per_bfd->infos[0], + false /* is_dwz */)) return false; if (dwz_map.cu_count == 0) return true; dwz_file *dwz = per_bfd->get_dwz_file (); - return check_cus_from_debug_names_list (per_bfd, dwz_map, dwz->info, - true /* is_dwz */); + return build_and_check_cu_list_from_debug_names (per_bfd, dwz_map, dwz->info, + true /* is_dwz */); } /* This does all the work for dwarf2_read_debug_names, but putting it @@ -817,7 +819,7 @@ do_dwarf2_read_debug_names (dwarf2_per_objfile *per_objfile) } create_all_units (per_objfile); - if (!check_cus_from_debug_names (per_bfd, map, dwz_map)) + if (!build_and_check_cu_lists_from_debug_names (per_bfd, map, dwz_map)) return false; if (map.tu_count != 0) @@ -833,8 +835,8 @@ do_dwarf2_read_debug_names (dwarf2_per_objfile *per_objfile) ? &per_bfd->types[0] : &per_bfd->infos[0]); - if (!check_signatured_type_table_from_debug_names (per_objfile, - map, section)) + if (!build_and_check_tu_list_from_debug_names (per_objfile, map, + section)) return false; } diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h index b7780cef650..45296633f9c 100644 --- a/gdb/dwarf2/read.h +++ b/gdb/dwarf2/read.h @@ -500,20 +500,6 @@ struct dwarf2_per_bfd return this->all_units[index].get (); } - /* Return the CU given its index in the CU table in the index. */ - dwarf2_per_cu *get_index_cu (int index) const - { - if (this->all_comp_units_index_cus.empty ()) - return get_cu (index); - - return this->all_comp_units_index_cus[index]; - } - - dwarf2_per_cu *get_index_tu (int index) const - { - return this->all_comp_units_index_tus[index]; - } - /* Return the separate '.dwz' debug file. If there is no .gnu_debugaltlink section in the file, then the result depends on REQUIRE: if REQUIRE is true, error out; if REQUIRE is false, @@ -606,9 +592,6 @@ public: unsigned int num_comp_units = 0; unsigned int num_type_units = 0; - std::vector all_comp_units_index_cus; - std::vector all_comp_units_index_tus; - /* Table of struct type_unit_group objects. The hash key is the DW_AT_stmt_list value. */ htab_up type_unit_groups; -- 2.39.5