From patchwork Sun Feb 12 23:12:39 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom Tromey X-Patchwork-Id: 64795 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 20F703858025 for ; Sun, 12 Feb 2023 23:13:34 +0000 (GMT) X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from qproxy3-pub.mail.unifiedlayer.com (qproxy3-pub.mail.unifiedlayer.com [67.222.38.20]) by sourceware.org (Postfix) with ESMTPS id 357FF3858D35 for ; Sun, 12 Feb 2023 23:13:14 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 357FF3858D35 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=tromey.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=tromey.com Received: from outbound-ss-761.bluehost.com (outbound-ss-761.bluehost.com [74.220.211.250]) by qproxy3.mail.unifiedlayer.com (Postfix) with ESMTP id 797E58027460 for ; Sun, 12 Feb 2023 23:13:13 +0000 (UTC) Received: from cmgw13.mail.unifiedlayer.com (unknown [10.0.90.128]) by progateway8.mail.pro1.eigbox.com (Postfix) with ESMTP id 6EE5110047D51 for ; Sun, 12 Feb 2023 23:12:43 +0000 (UTC) Received: from box5379.bluehost.com ([162.241.216.53]) by cmsmtp with ESMTP id RLWVpyl1QNX2aRLWVpjQoy; Sun, 12 Feb 2023 23:12:43 +0000 X-Authority-Reason: nr=8 X-Authority-Analysis: v=2.4 cv=NMAQR22g c=1 sm=1 tr=0 ts=63e9726b a=ApxJNpeYhEAb1aAlGBBbmA==:117 a=ApxJNpeYhEAb1aAlGBBbmA==:17 a=dLZJa+xiwSxG16/P+YVxDGlgEgI=:19 a=m04uMKEZRckA:10:nop_rcvd_month_year a=Qbun_eYptAEA:10:endurance_base64_authed_username_1 a=HumyfR2inJQtGsib2JYA:9 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=tromey.com; s=default; h=Content-Transfer-Encoding:MIME-Version:Message-Id:Date:Subject: Cc:To:From:Sender:Reply-To:Content-Type:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: In-Reply-To:References:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=9K722ykuTZ6FAiDS/MJc/IF8qTT1DzONmMqxi0wAJ5E=; b=D6I6lBJrcuwO1/7IB7OA6vl+kt EhrUnQN4CANtZylTR+h/6ps8vHXen67/yPI0YySSwdQSXEAoOnB9x1NTjOhovv6KbJmPm8qUgjAQ8 rXoBBwN0CRWZPEGznDCYfgNuW; Received: from [69.2.159.9] (port=46729 helo=prentzel.thompsonschools.org) by box5379.bluehost.com with esmtpsa (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1pRLWV-0024o2-62; Sun, 12 Feb 2023 16:12:43 -0700 From: Tom Tromey To: gdb-patches@sourceware.org Cc: Tom Tromey Subject: [PATCH] Clean up attribute reprocessing Date: Sun, 12 Feb 2023 16:12:39 -0700 Message-Id: <20230212231239.535298-1-tom@tromey.com> X-Mailer: git-send-email 2.39.1 MIME-Version: 1.0 X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - box5379.bluehost.com X-AntiAbuse: Original Domain - sourceware.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - tromey.com X-BWhitelist: no X-Source-IP: 69.2.159.9 X-Source-L: No X-Exim-ID: 1pRLWV-0024o2-62 X-Source: X-Source-Args: X-Source-Dir: X-Source-Sender: (prentzel.thompsonschools.org) [69.2.159.9]:46729 X-Source-Auth: tom+tromey.com X-Email-Count: 1 X-Source-Cap: ZWx5bnJvYmk7ZWx5bnJvYmk7Ym94NTM3OS5ibHVlaG9zdC5jb20= X-Local-Domain: yes X-Spam-Status: No, score=-3026.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, GIT_PATCH_0, JMQ_SPF_NEUTRAL, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, 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: , Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Sender: "Gdb-patches" I ran across the attribute reprocessing code recently and noticed that it unconditionally sets members of the CU when reading a DIE. Also, each spot reading attributes needs to be careful to "reprocess" them as a separate step. This seemed excessive to me, because while reprocessing applies to any DIE, setting the CU members is only necessary for the toplevel DIE in any given CU. This patch introduces a new read_toplevel_die function and changes a few spots to call it. This is easily done because reading the toplevel DIE is already special. I left the reprocessing flag and associated checks in attribute. It could be stripped out, but I am not sure it would provide much value (maybe some iota of performance). Regression tested on x86-64 Fedora 36. --- gdb/dwarf2/read.c | 205 +++++++++++++++++++++++----------------------- 1 file changed, 101 insertions(+), 104 deletions(-) diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index ce6c01ac771..6d2e70dfa0d 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -905,10 +905,15 @@ static unsigned int peek_abbrev_code (bfd *, const gdb_byte *); static const gdb_byte *read_attribute (const struct die_reader_specs *, struct attribute *, const struct attr_abbrev *, - const gdb_byte *); + const gdb_byte *, + bool allow_reprocess = true); +/* Note that the default for TAG is chosen because it only matters + when reading the top-level DIE, and that function is careful to + pass the correct tag. */ static void read_attribute_reprocess (const struct die_reader_specs *reader, - struct attribute *attr, dwarf_tag tag); + struct attribute *attr, + dwarf_tag tag = DW_TAG_padding); static CORE_ADDR read_addr_index (struct dwarf2_cu *cu, unsigned int addr_index); @@ -1105,10 +1110,12 @@ static struct die_info *read_die_and_siblings (const struct die_reader_specs *, static const gdb_byte *read_full_die_1 (const struct die_reader_specs *, struct die_info **, const gdb_byte *, - int); + int, bool); -static const gdb_byte *read_full_die (const struct die_reader_specs *, - struct die_info **, const gdb_byte *); +static const gdb_byte *read_toplevel_die (const struct die_reader_specs *, + struct die_info **, + const gdb_byte *, + gdb::array_view = {}); static void process_die (struct die_info *, struct dwarf2_cu *); @@ -5910,38 +5917,41 @@ read_cutu_die_from_dwo (dwarf2_cu *cu, struct objfile *objfile = per_objfile->objfile; bfd *abfd; const gdb_byte *begin_info_ptr, *info_ptr; - struct attribute *comp_dir, *stmt_list, *low_pc, *high_pc, *ranges; - int i,num_extra_attrs; struct dwarf2_section_info *dwo_abbrev_section; - struct die_info *comp_unit_die; /* At most one of these may be provided. */ gdb_assert ((stub_comp_unit_die != NULL) + (stub_comp_dir != NULL) <= 1); - /* These attributes aren't processed until later: - DW_AT_stmt_list, DW_AT_low_pc, DW_AT_high_pc, DW_AT_ranges. - DW_AT_comp_dir is used now, to find the DWO file, but it is also - referenced later. However, these attributes are found in the stub - which we won't have later. In order to not impose this complication - on the rest of the code, we read them here and copy them to the - DWO CU/TU die. */ + /* These attributes aren't processed until later: DW_AT_stmt_list, + DW_AT_low_pc, DW_AT_high_pc, DW_AT_ranges, DW_AT_comp_dir. + However, these attributes are found in the stub which we won't + have later. In order to not impose this complication on the rest + of the code, we read them here and copy them to the DWO CU/TU + die. */ - stmt_list = NULL; - low_pc = NULL; - high_pc = NULL; - ranges = NULL; - comp_dir = NULL; + /* We store them all in an array. */ + struct attribute *attributes[5] {}; + /* Next available element of the attributes array. */ + int next_attr_idx = 0; + + /* Push an element into ATTRIBUTES. */ + auto push_back = [&] (struct attribute *attr) + { + gdb_assert (next_attr_idx <= ARRAY_SIZE (attributes)); + if (attr != nullptr) + attributes[next_attr_idx++] = attr; + }; if (stub_comp_unit_die != NULL) { /* For TUs in DWO files, the DW_AT_stmt_list attribute lives in the DWO file. */ if (!per_cu->is_debug_types) - stmt_list = dwarf2_attr (stub_comp_unit_die, DW_AT_stmt_list, cu); - low_pc = dwarf2_attr (stub_comp_unit_die, DW_AT_low_pc, cu); - high_pc = dwarf2_attr (stub_comp_unit_die, DW_AT_high_pc, cu); - ranges = dwarf2_attr (stub_comp_unit_die, DW_AT_ranges, cu); - comp_dir = dwarf2_attr (stub_comp_unit_die, DW_AT_comp_dir, cu); + push_back (dwarf2_attr (stub_comp_unit_die, DW_AT_stmt_list, cu)); + push_back (dwarf2_attr (stub_comp_unit_die, DW_AT_low_pc, cu)); + push_back (dwarf2_attr (stub_comp_unit_die, DW_AT_high_pc, cu)); + push_back (dwarf2_attr (stub_comp_unit_die, DW_AT_ranges, cu)); + push_back (dwarf2_attr (stub_comp_unit_die, DW_AT_comp_dir, cu)); cu->addr_base = stub_comp_unit_die->addr_base (); @@ -5960,10 +5970,12 @@ read_cutu_die_from_dwo (dwarf2_cu *cu, else if (stub_comp_dir != NULL) { /* Reconstruct the comp_dir attribute to simplify the code below. */ - comp_dir = OBSTACK_ZALLOC (&cu->comp_unit_obstack, struct attribute); + struct attribute *comp_dir = OBSTACK_ZALLOC (&cu->comp_unit_obstack, + struct attribute); comp_dir->name = DW_AT_comp_dir; comp_dir->form = DW_FORM_string; comp_dir->set_string_noncanonical (stub_comp_dir); + push_back (comp_dir); } /* Set up for reading the DWO CU/TU. */ @@ -6020,42 +6032,13 @@ read_cutu_die_from_dwo (dwarf2_cu *cu, init_cu_die_reader (result_reader, cu, section, dwo_unit->dwo_file, result_dwo_abbrev_table->get ()); - /* Read in the die, but leave space to copy over the attributes - from the stub. This has the benefit of simplifying the rest of - the code - all the work to maintain the illusion of a single + /* Read in the die, filling in the attributes from the stub. This + has the benefit of simplifying the rest of the code - all the + work to maintain the illusion of a single DW_TAG_{compile,type}_unit DIE is done here. */ - num_extra_attrs = ((stmt_list != NULL) - + (low_pc != NULL) - + (high_pc != NULL) - + (ranges != NULL) - + (comp_dir != NULL)); - info_ptr = read_full_die_1 (result_reader, result_comp_unit_die, info_ptr, - num_extra_attrs); - - /* Copy over the attributes from the stub to the DIE we just read in. */ - comp_unit_die = *result_comp_unit_die; - i = comp_unit_die->num_attrs; - if (stmt_list != NULL) - comp_unit_die->attrs[i++] = *stmt_list; - if (low_pc != NULL) - comp_unit_die->attrs[i++] = *low_pc; - if (high_pc != NULL) - comp_unit_die->attrs[i++] = *high_pc; - if (ranges != NULL) - comp_unit_die->attrs[i++] = *ranges; - if (comp_dir != NULL) - comp_unit_die->attrs[i++] = *comp_dir; - comp_unit_die->num_attrs += num_extra_attrs; - - if (dwarf_die_debug) - { - gdb_printf (gdb_stdlog, - "Read die from %s@0x%x of %s:\n", - section->get_name (), - (unsigned) (begin_info_ptr - section->buffer), - bfd_get_filename (abfd)); - comp_unit_die->dump (dwarf_die_debug); - } + info_ptr = read_toplevel_die (result_reader, result_comp_unit_die, info_ptr, + gdb::make_array_view (attributes, + next_attr_idx)); /* Skip dummy compilation units. */ if (info_ptr >= begin_info_ptr + dwo_unit->length @@ -6329,7 +6312,7 @@ cutu_reader::cutu_reader (dwarf2_per_cu_data *this_cu, /* Read the top level CU/TU die. */ init_cu_die_reader (this, cu, section, NULL, abbrev_table); - info_ptr = read_full_die (this, &comp_unit_die, info_ptr); + info_ptr = read_toplevel_die (this, &comp_unit_die, info_ptr); if (skip_partial && comp_unit_die->tag == DW_TAG_partial_unit) { @@ -6471,7 +6454,7 @@ cutu_reader::cutu_reader (dwarf2_per_cu_data *this_cu, init_cu_die_reader (this, m_new_cu.get (), section, dwo_file, m_abbrev_table_holder.get ()); - info_ptr = read_full_die (this, &comp_unit_die, info_ptr); + info_ptr = read_toplevel_die (this, &comp_unit_die, info_ptr); } @@ -7405,7 +7388,10 @@ skip_one_die (const struct die_reader_specs *reader, const gdb_byte *info_ptr, /* The only abbrev we care about is DW_AT_sibling. */ if (do_skip_children && abbrev->attrs[i].name == DW_AT_sibling) { - read_attribute (reader, &attr, &abbrev->attrs[i], info_ptr); + /* Note there is no need for the extra work of + "reprocessing" here, so we pass false for that + argument. */ + read_attribute (reader, &attr, &abbrev->attrs[i], info_ptr, false); if (attr.form == DW_FORM_ref_addr) complaint (_("ignoring absolute DW_AT_sibling")); else @@ -17772,7 +17758,7 @@ read_die_and_children (const struct die_reader_specs *reader, struct die_info *die; const gdb_byte *cur_ptr; - cur_ptr = read_full_die_1 (reader, &die, info_ptr, 0); + cur_ptr = read_full_die_1 (reader, &die, info_ptr, 0, true); if (die == NULL) { *new_info_ptr = cur_ptr; @@ -17866,7 +17852,7 @@ read_die_and_siblings (const struct die_reader_specs *reader, static const gdb_byte * read_full_die_1 (const struct die_reader_specs *reader, struct die_info **diep, const gdb_byte *info_ptr, - int num_extra_attrs) + int num_extra_attrs, bool allow_reprocess) { unsigned int abbrev_number, bytes_read, i; const struct abbrev_info *abbrev; @@ -17901,54 +17887,57 @@ read_full_die_1 (const struct die_reader_specs *reader, attributes. */ die->num_attrs = abbrev->num_attrs; - bool any_need_reprocess = false; for (i = 0; i < abbrev->num_attrs; ++i) - { - info_ptr = read_attribute (reader, &die->attrs[i], &abbrev->attrs[i], - info_ptr); - if (die->attrs[i].requires_reprocessing_p ()) - any_need_reprocess = true; - } + info_ptr = read_attribute (reader, &die->attrs[i], &abbrev->attrs[i], + info_ptr, allow_reprocess); + + *diep = die; + return info_ptr; +} + +/* Read a die and all its attributes. + Set DIEP to point to a newly allocated die with its information, + except for its child, sibling, and parent fields. */ - struct attribute *attr = die->attr (DW_AT_str_offsets_base); +static const gdb_byte * +read_toplevel_die (const struct die_reader_specs *reader, + struct die_info **diep, const gdb_byte *info_ptr, + gdb::array_view extra_attrs) +{ + const gdb_byte *result; + struct dwarf2_cu *cu = reader->cu; + + result = read_full_die_1 (reader, diep, info_ptr, extra_attrs.size (), + false); + + /* Copy in the extra attributes, if any. */ + attribute *next = &(*diep)->attrs[(*diep)->num_attrs]; + for (attribute *extra : extra_attrs) + *next++ = *extra; + + struct attribute *attr = (*diep)->attr (DW_AT_str_offsets_base); if (attr != nullptr && attr->form_is_unsigned ()) cu->str_offsets_base = attr->as_unsigned (); - attr = die->attr (DW_AT_loclists_base); + attr = (*diep)->attr (DW_AT_loclists_base); if (attr != nullptr) cu->loclist_base = attr->as_unsigned (); - auto maybe_addr_base = die->addr_base (); + auto maybe_addr_base = (*diep)->addr_base (); if (maybe_addr_base.has_value ()) cu->addr_base = *maybe_addr_base; - attr = die->attr (DW_AT_rnglists_base); + attr = (*diep)->attr (DW_AT_rnglists_base); if (attr != nullptr) cu->rnglists_base = attr->as_unsigned (); - if (any_need_reprocess) + for (int i = 0; i < (*diep)->num_attrs; ++i) { - for (i = 0; i < abbrev->num_attrs; ++i) - { - if (die->attrs[i].requires_reprocessing_p ()) - read_attribute_reprocess (reader, &die->attrs[i], die->tag); - } + if ((*diep)->attrs[i].form_requires_reprocessing ()) + read_attribute_reprocess (reader, &(*diep)->attrs[i], (*diep)->tag); } - *diep = die; - return info_ptr; -} - -/* Read a die and all its attributes. - Set DIEP to point to a newly allocated die with its information, - except for its child, sibling, and parent fields. */ - -static const gdb_byte * -read_full_die (const struct die_reader_specs *reader, - struct die_info **diep, const gdb_byte *info_ptr) -{ - const gdb_byte *result; - result = read_full_die_1 (reader, diep, info_ptr, 0); + (*diep)->num_attrs += extra_attrs.size (); if (dwarf_die_debug) { @@ -18090,8 +18079,6 @@ cooked_indexer::scan_attributes (dwarf2_per_cu_data *scanning_per_cu, { attribute attr; info_ptr = read_attribute (reader, &attr, &abbrev->attrs[i], info_ptr); - if (attr.requires_reprocessing_p ()) - read_attribute_reprocess (reader, &attr, abbrev->tag); /* Store the data if it is of an attribute we want to keep in a partial symbol table. */ @@ -19052,7 +19039,8 @@ read_attribute_reprocess (const struct die_reader_specs *reader, static const gdb_byte * read_attribute_value (const struct die_reader_specs *reader, struct attribute *attr, unsigned form, - LONGEST implicit_const, const gdb_byte *info_ptr) + LONGEST implicit_const, const gdb_byte *info_ptr, + bool allow_reprocess) { struct dwarf2_cu *cu = reader->cu; dwarf2_per_objfile *per_objfile = cu->per_objfile; @@ -19133,6 +19121,8 @@ read_attribute_value (const struct die_reader_specs *reader, attr->set_unsigned_reprocess (read_unsigned_leb128 (abfd, info_ptr, &bytes_read)); info_ptr += bytes_read; + if (allow_reprocess) + read_attribute_reprocess (reader, attr); } break; case DW_FORM_string: @@ -19206,6 +19196,8 @@ read_attribute_value (const struct die_reader_specs *reader, attr->set_unsigned_reprocess (read_unsigned_leb128 (abfd, info_ptr, &bytes_read)); info_ptr += bytes_read; + if (allow_reprocess) + read_attribute_reprocess (reader, attr); } break; case DW_FORM_udata: @@ -19251,7 +19243,7 @@ read_attribute_value (const struct die_reader_specs *reader, info_ptr += bytes_read; } info_ptr = read_attribute_value (reader, attr, form, implicit_const, - info_ptr); + info_ptr, allow_reprocess); break; case DW_FORM_implicit_const: attr->set_signed (implicit_const); @@ -19261,6 +19253,8 @@ read_attribute_value (const struct die_reader_specs *reader, attr->set_unsigned_reprocess (read_unsigned_leb128 (abfd, info_ptr, &bytes_read)); info_ptr += bytes_read; + if (allow_reprocess) + read_attribute_reprocess (reader, attr); break; case DW_FORM_strx: case DW_FORM_strx1: @@ -19296,6 +19290,8 @@ read_attribute_value (const struct die_reader_specs *reader, info_ptr += bytes_read; } attr->set_unsigned_reprocess (str_index); + if (allow_reprocess) + read_attribute_reprocess (reader, attr); } break; default: @@ -19332,13 +19328,14 @@ read_attribute_value (const struct die_reader_specs *reader, static const gdb_byte * read_attribute (const struct die_reader_specs *reader, struct attribute *attr, const struct attr_abbrev *abbrev, - const gdb_byte *info_ptr) + const gdb_byte *info_ptr, + bool allow_reprocess) { attr->name = abbrev->name; attr->string_is_canonical = 0; - attr->requires_reprocessing = 0; return read_attribute_value (reader, attr, abbrev->form, - abbrev->implicit_const, info_ptr); + abbrev->implicit_const, info_ptr, + allow_reprocess); } /* Return pointer to string at .debug_str offset STR_OFFSET. */