From patchwork Thu Sep 22 13:00:22 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom de Vries X-Patchwork-Id: 57896 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id B7077385782D for ; Thu, 22 Sep 2022 13:00:50 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org B7077385782D DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1663851650; bh=pTzxYU8LCwazqLNpMJltSXDU1jNkIKJPLGTomlGiV+Y=; h=Date:To:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=fWspuXY3HRaITHk/DJWBjdCQXNNWNTOMTIUJ9J4XMJPzZHdjUBDrEyplosgjJxlmt coPDVw6HdQdAUjfblCv1UNnn+jgC4Zfma1QWi57C5AnYf5Y7UEJwhuZ6EssnJd6VVB 9xd6XRCjHzWEejoajoq3h67hj9dN9X/3LvvFwLBY= X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from smtp-out1.suse.de (smtp-out1.suse.de [IPv6:2001:67c:2178:6::1c]) by sourceware.org (Postfix) with ESMTPS id 096F13858422 for ; Thu, 22 Sep 2022 13:00:25 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 096F13858422 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-out1.suse.de (Postfix) with ESMTPS id 4560521A48 for ; Thu, 22 Sep 2022 13:00:24 +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 2DA841346B for ; Thu, 22 Sep 2022 13:00:24 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id Avd9CWhcLGMTaQAAMHmgww (envelope-from ) for ; Thu, 22 Sep 2022 13:00:24 +0000 Date: Thu, 22 Sep 2022 15:00:22 +0200 To: gdb-patches@sourceware.org Subject: [PATCH][gdb/symtab] Redo "Fix assert in set_length" Message-ID: <20220922130015.GA25067@delia.home> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.10.1 (2018-07-13) X-Spam-Status: No, score=-12.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org 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: , X-Patchwork-Original-From: Tom de Vries via Gdb-patches From: Tom de Vries Reply-To: Tom de Vries Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Sender: "Gdb-patches" Hi, This reverts commit 1c04f72368c ("[gdb/symtab] Fix assert in set_length"), due to a regression reported in PR29572, and implements a different fix for PR29453. The fix is to not use the CU table in a .debug_names section to construct all_comp_units, but instead use create_all_comp_units, and then verify the CU table from .debug_names. This also fixes PR25969, so remove the KFAIL. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29572 Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=25969 Any comments? Thanks, - Tom [gdb/symtab] Redo "Fix assert in set_length" --- gdb/dwarf2/read.c | 194 ++++++++++----------- gdb/dwarf2/read.h | 17 ++ gdb/testsuite/gdb.dwarf2/clang-debug-names.exp | 15 +- .../gdb.dwarf2/debug-names-duplicate-cu.exp | 2 +- .../gdb.dwarf2/debug-names-missing-cu.exp | 2 +- .../gdb.dwarf2/debug-names-non-ascending-cu.exp | 2 +- 6 files changed, 113 insertions(+), 119 deletions(-) diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index 35cce9396ae..ddf5f93b462 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -1201,7 +1201,8 @@ static void prepare_one_comp_unit (struct dwarf2_cu *cu, static struct type *set_die_type (struct die_info *, struct type *, struct dwarf2_cu *, bool = false); -static void create_all_units (dwarf2_per_objfile *per_objfile); +static void create_all_units (dwarf2_per_objfile *per_objfile, + bool pre_read_p = true); static void load_full_comp_unit (dwarf2_per_cu_data *per_cu, dwarf2_per_objfile *per_objfile, @@ -2216,52 +2217,46 @@ create_signatured_type_table_from_index per_bfd->signatured_types = std::move (sig_types_hash); } -/* Create the signatured type hash table from .debug_names. */ +/* Check the signatured type hash table from .debug_names. */ -static void -create_signatured_type_table_from_debug_names +static bool +check_signatured_type_table_from_debug_names (dwarf2_per_objfile *per_objfile, const mapped_debug_names &map, - struct dwarf2_section_info *section, - struct dwarf2_section_info *abbrev_section) + struct dwarf2_section_info *section) { struct objfile *objfile = per_objfile->objfile; + dwarf2_per_bfd *per_bfd = per_objfile->per_bfd; + int nr_cus = per_bfd->all_comp_units.size (); + int nr_cus_tus = per_bfd->all_units.size (); section->read (objfile); - abbrev_section->read (objfile); - - htab_up sig_types_hash = allocate_signatured_type_table (); + uint32_t j = nr_cus; for (uint32_t i = 0; i < map.tu_count; ++i) { - signatured_type_up sig_type; - void **slot; - sect_offset sect_off = (sect_offset) (extract_unsigned_integer (map.tu_table_reordered + i * map.offset_size, map.offset_size, map.dwarf5_byte_order)); - comp_unit_head cu_header; - read_and_check_comp_unit_head (per_objfile, &cu_header, section, - abbrev_section, - section->buffer + to_underlying (sect_off), - rcuh_kind::TYPE); - - sig_type = per_objfile->per_bfd->allocate_signatured_type - (cu_header.signature); - sig_type->type_offset_in_tu = cu_header.type_cu_offset_in_tu; - sig_type->section = section; - sig_type->sect_off = sect_off; - - slot = htab_find_slot (sig_types_hash.get (), sig_type.get (), INSERT); - *slot = sig_type.get (); - - per_objfile->per_bfd->all_units.emplace_back (sig_type.release ()); + bool found = false; + for (; j < nr_cus_tus; j++) + if (per_bfd->get_cu (j)->sect_off == sect_off) + { + found = true; + break; + } + if (!found) + { + warning (_("Section .debug_names has incorrect entry in TU table," + " ignoring .debug_names.")); + return false; + } + per_bfd->all_comp_units_index_tus.push_back (per_bfd->get_cu (j)); } - - per_objfile->per_bfd->signatured_types = std::move (sig_types_hash); + return true; } /* Read the address map data from the mapped index, and use it to @@ -4603,17 +4598,20 @@ read_debug_names_from_section (struct objfile *objfile, return true; } -/* A helper for create_cus_from_debug_names that handles the MAP's CU +/* A helper for check_cus_from_debug_names that handles the MAP's CU list. */ static bool -create_cus_from_debug_names_list (dwarf2_per_bfd *per_bfd, - const mapped_debug_names &map, - dwarf2_section_info §ion, - bool is_dwz) +check_cus_from_debug_names_list (dwarf2_per_bfd *per_bfd, + const mapped_debug_names &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 @@ -4621,56 +4619,44 @@ create_cus_from_debug_names_list (dwarf2_per_bfd *per_bfd, (map.cu_table_reordered + i * map.offset_size, map.offset_size, map.dwarf5_byte_order)); - /* We don't know the length of the CU, because the CU list in a - .debug_names index can be incomplete, so we can't use the start - of the next CU as end of this CU. We create the CUs here with - length 0, and in cutu_reader::cutu_reader we'll fill in the - actual length. */ - dwarf2_per_cu_data_up per_cu - = create_cu_from_index_list (per_bfd, §ion, is_dwz, - sect_off, 0); - per_bfd->all_units.push_back (std::move (per_cu)); + 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; } - sect_offset sect_off_prev; - for (uint32_t i = 0; i <= map.cu_count; ++i) + if (map.cu_count != nr_cus) { - sect_offset sect_off_next; - if (i < map.cu_count) - { - sect_off_next - = (sect_offset) (extract_unsigned_integer - (map.cu_table_reordered + i * map.offset_size, - map.offset_size, - map.dwarf5_byte_order)); - } - else - sect_off_next = (sect_offset) section.size; - if (i >= 1) + warning (_("Section .debug_names has incorrect number of CUs in CU table," + " ignoring .debug_names.")); + return false; + } + + 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)); + if (sect_off != per_bfd->get_cu (i)->sect_off) { - if (sect_off_next == sect_off_prev) - { - warning (_("Section .debug_names has duplicate entry in CU table," - " ignoring .debug_names.")); - return false; - } - if (sect_off_next < sect_off_prev) - { - warning (_("Section .debug_names has non-ascending CU table," - " ignoring .debug_names.")); - return false; - } - /* Note: we're not using length = sect_off_next - sect_off_prev, - to gracefully handle an incomplete CU list. */ - const ULONGEST length = 0; - dwarf2_per_cu_data_up per_cu - = create_cu_from_index_list (per_bfd, §ion, is_dwz, - sect_off_prev, length); - per_bfd->all_units.push_back (std::move (per_cu)); + warning (_("Section .debug_names has incorrect entry in CU table," + " ignoring .debug_names.")); + return false; } - sect_off_prev = sect_off_next; } return true; @@ -4680,23 +4666,20 @@ create_cus_from_debug_names_list (dwarf2_per_bfd *per_bfd, the CU objects for this dwarf2_per_objfile. */ static bool -create_cus_from_debug_names (dwarf2_per_bfd *per_bfd, - const mapped_debug_names &map, - const mapped_debug_names &dwz_map) +check_cus_from_debug_names (dwarf2_per_bfd *per_bfd, + const mapped_debug_names &map, + const mapped_debug_names &dwz_map) { - gdb_assert (per_bfd->all_units.empty ()); - per_bfd->all_units.reserve (map.cu_count + dwz_map.cu_count); - - if (!create_cus_from_debug_names_list (per_bfd, map, per_bfd->info, - false /* is_dwz */)) + if (!check_cus_from_debug_names_list (per_bfd, map, per_bfd->info, + false /* is_dwz */)) return false; if (dwz_map.cu_count == 0) return true; dwz_file *dwz = dwarf2_get_dwz_file (per_bfd); - return create_cus_from_debug_names_list (per_bfd, dwz_map, dwz->info, - true /* is_dwz */); + return check_cus_from_debug_names_list (per_bfd, dwz_map, dwz->info, + true /* is_dwz */); } /* Read .debug_names. If everything went ok, initialize the "quick" @@ -4733,7 +4716,8 @@ dwarf2_read_debug_names (dwarf2_per_objfile *per_objfile) } } - if (!create_cus_from_debug_names (per_bfd, *map, dwz_map)) + create_all_units (per_objfile, false); + if (!check_cus_from_debug_names (per_bfd, *map, dwz_map)) { per_bfd->all_units.clear (); return false; @@ -4754,8 +4738,12 @@ dwarf2_read_debug_names (dwarf2_per_objfile *per_objfile) ? &per_bfd->types[0] : &per_bfd->info); - create_signatured_type_table_from_debug_names - (per_objfile, *map, section, &per_bfd->abbrev); + if (!check_signatured_type_table_from_debug_names + (per_objfile, *map, section)) + { + per_bfd->all_units.clear (); + return false; + } } finalize_all_units (per_bfd); @@ -5032,7 +5020,7 @@ dw2_debug_names_iterator::next () continue; } } - per_cu = per_bfd->get_cu (ull); + per_cu = per_bfd->get_index_cu (ull); break; case DW_IDX_type_unit: /* Don't crash on bad data. */ @@ -5045,15 +5033,14 @@ dw2_debug_names_iterator::next () continue; } { - int nr_cus = per_bfd->all_comp_units.size (); - per_cu = per_bfd->get_cu (nr_cus + ull); + per_cu = per_bfd->get_index_tu (ull); } break; case DW_IDX_die_offset: /* 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_bfd->get_cu (0); + per_cu = per_bfd->get_index_cu (0); break; case DW_IDX_GNU_internal: if (!m_map.augmentation_is_gdb) @@ -7278,7 +7265,7 @@ finalize_all_units (dwarf2_per_bfd *per_bfd) This is only done for -readnow and building partial symtabs. */ static void -create_all_units (dwarf2_per_objfile *per_objfile) +create_all_units (dwarf2_per_objfile *per_objfile, bool pre_read_p) { htab_up types_htab; gdb_assert (per_objfile->per_bfd->all_units.empty ()); @@ -7294,12 +7281,15 @@ create_all_units (dwarf2_per_objfile *per_objfile) dwz_file *dwz = dwarf2_get_dwz_file (per_objfile->per_bfd); if (dwz != NULL) { - /* Pre-read the sections we'll need to construct an index. */ - struct objfile *objfile = per_objfile->objfile; - dwz->abbrev.read (objfile); - dwz->info.read (objfile); - dwz->str.read (objfile); - dwz->line.read (objfile); + if (pre_read_p) + { + /* Pre-read the sections we'll need to construct an index. */ + struct objfile *objfile = per_objfile->objfile; + dwz->abbrev.read (objfile); + dwz->info.read (objfile); + dwz->str.read (objfile); + dwz->line.read (objfile); + } read_comp_units_from_section (per_objfile, &dwz->info, &dwz->abbrev, 1, types_htab, rcuh_kind::COMPILE); } diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h index 5f01fbc1025..51e95a948e4 100644 --- a/gdb/dwarf2/read.h +++ b/gdb/dwarf2/read.h @@ -436,6 +436,20 @@ 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_data *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_data *get_index_tu (int index) const + { + return this->all_comp_units_index_tus[index]; + } + /* A convenience function to allocate a dwarf2_per_cu_data. The returned object has its "index" field set properly. The object is allocated on the dwarf2_per_bfd obstack. */ @@ -496,6 +510,9 @@ struct dwarf2_per_bfd gdb::array_view all_comp_units; gdb::array_view all_type_units; + 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; diff --git a/gdb/testsuite/gdb.dwarf2/clang-debug-names.exp b/gdb/testsuite/gdb.dwarf2/clang-debug-names.exp index 22fdd740c6f..0914f5939d5 100644 --- a/gdb/testsuite/gdb.dwarf2/clang-debug-names.exp +++ b/gdb/testsuite/gdb.dwarf2/clang-debug-names.exp @@ -34,20 +34,7 @@ if { [prepare_for_testing "failed to prepare" ${testfile} \ } set test "no file command warnings" -if { [regexp "warning: " $gdb_file_cmd_msg] } { - set kfail_re \ - [concat \ - "warning: Section .debug_aranges in \[^\r\n\]* entry" \ - "at offset 0 debug_info_offset 0 does not exists," \ - "ignoring \\.debug_aranges\\."] - if { [regexp $kfail_re $gdb_file_cmd_msg] } { - kfail symtab/25969 $test - } else { - fail $test - } -} else { - pass $test -} +gdb_assert { [regexp "warning: " $gdb_file_cmd_msg] == 0 } $test set cmd "ptype main" set pass_re \ diff --git a/gdb/testsuite/gdb.dwarf2/debug-names-duplicate-cu.exp b/gdb/testsuite/gdb.dwarf2/debug-names-duplicate-cu.exp index a79a60a6b5d..62134da2828 100644 --- a/gdb/testsuite/gdb.dwarf2/debug-names-duplicate-cu.exp +++ b/gdb/testsuite/gdb.dwarf2/debug-names-duplicate-cu.exp @@ -68,7 +68,7 @@ if [prepare_for_testing "failed to prepare" $testfile "${asm_file} ${srcfile}" \ set re \ [list \ "warning:" \ - "Section .debug_names has duplicate entry in CU table," \ + "Section .debug_names has incorrect number of CUs in CU table," \ "ignoring .debug_names."] set re [join $re] gdb_assert {[regexp $re $gdb_file_cmd_msg]} "warning" diff --git a/gdb/testsuite/gdb.dwarf2/debug-names-missing-cu.exp b/gdb/testsuite/gdb.dwarf2/debug-names-missing-cu.exp index f70debd35ad..9450728374d 100644 --- a/gdb/testsuite/gdb.dwarf2/debug-names-missing-cu.exp +++ b/gdb/testsuite/gdb.dwarf2/debug-names-missing-cu.exp @@ -69,7 +69,7 @@ if [prepare_for_testing "failed to prepare" $testfile "${asm_file} ${srcfile}" \ # Verify that .debug_names section is not ignored. set index [have_index $binfile] -gdb_assert { [string equal $index "debug_names"] } ".debug_names used" +gdb_assert { [string equal $index ""] } ".debug_names not used" # Verify that initially no symtab is expanded. gdb_test_no_output "maint info symtabs" diff --git a/gdb/testsuite/gdb.dwarf2/debug-names-non-ascending-cu.exp b/gdb/testsuite/gdb.dwarf2/debug-names-non-ascending-cu.exp index 53523eec46e..462ac4d5a7c 100644 --- a/gdb/testsuite/gdb.dwarf2/debug-names-non-ascending-cu.exp +++ b/gdb/testsuite/gdb.dwarf2/debug-names-non-ascending-cu.exp @@ -73,7 +73,7 @@ if [prepare_for_testing "failed to prepare" $testfile "${asm_file} ${srcfile}" \ set re \ [list \ "warning:" \ - "Section .debug_names has non-ascending CU table," \ + "Section .debug_names has incorrect entry in CU table," \ "ignoring .debug_names."] set re [join $re] gdb_assert {[regexp $re $gdb_file_cmd_msg]} "warning"