From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id QLYdMsMr0GKswxIAWB0awg (envelope-from ) for ; Thu, 14 Jul 2022 10:44:19 -0400 Received: by simark.ca (Postfix, from userid 112) id CAAC01E5EA; Thu, 14 Jul 2022 10:44:19 -0400 (EDT) Authentication-Results: simark.ca; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.a=rsa-sha256 header.s=default header.b=Q1ds81Pk; dkim-atps=neutral X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-3.0 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id A3B781E222 for ; Thu, 14 Jul 2022 10:44:18 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 852E03856245 for ; Thu, 14 Jul 2022 14:44:16 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 852E03856245 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1657809856; bh=4YQJOu5NuaV3lmiJX3Is8fZhxVF+IUU22Vkmy3M4R7E=; h=Date:To:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=Q1ds81PkLyQv+4sIyH4SJXVxiB6XiOfjQbDbKTU4X7Xe049+OODjLw7UXrc8tvtlz ZuLfAaPPoHCySiHnv/GOiSL6Y4yDBjXjBEbpxqTAjk9d7qcAvcuexlO02Vxo7T8HGh hJJHSqi8pw8NG7A4PWX/u75DR2bPp5SWQ8zMIBZ4= Received: from smtp-out2.suse.de (smtp-out2.suse.de [IPv6:2001:67c:2178:6::1d]) by sourceware.org (Postfix) with ESMTPS id 21C1B385740F for ; Thu, 14 Jul 2022 14:43:55 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 21C1B385740F Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id 5C05D1F889; Thu, 14 Jul 2022 14:43:54 +0000 (UTC) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 42777133A6; Thu, 14 Jul 2022 14:43:54 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id EkYxD6or0GJYOwAAMHmgww (envelope-from ); Thu, 14 Jul 2022 14:43:54 +0000 Date: Thu, 14 Jul 2022 16:43:52 +0200 To: gdb-patches@sourceware.org Subject: [PATCH][gdb/symtab] Force usage of all_comp_units.size (CU/TU/CUTU) Message-ID: <20220714144351.GA31826@delia.home> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.10.1 (2018-07-13) X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Tom de Vries via Gdb-patches Reply-To: Tom de Vries Cc: Tom Tromey Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Sender: "Gdb-patches" 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 *)(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 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> 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 &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 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; +/* 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 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 +{ +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 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. */