From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from lndn.lancelotsix.com (lndn.lancelotsix.com [51.195.220.111]) by sourceware.org (Postfix) with ESMTPS id D811C3858439 for ; Mon, 6 Sep 2021 22:31:29 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org D811C3858439 Received: from ubuntu.lan (unknown [IPv6:2a02:390:9086::635]) by lndn.lancelotsix.com (Postfix) with ESMTPSA id 2BF8F83FBC; Mon, 6 Sep 2021 22:31:28 +0000 (UTC) Date: Mon, 6 Sep 2021 22:31:23 +0000 From: Lancelot SIX To: Tom Tromey Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 13/30] Statically examine abbrev properties Message-ID: <20210906223123.tyvpxqevmtlpurcg@ubuntu.lan> References: <20210826021937.1490292-1-tom@tromey.com> <20210826021937.1490292-14-tom@tromey.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210826021937.1490292-14-tom@tromey.com> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.5.11 (lndn.lancelotsix.com [0.0.0.0]); Mon, 06 Sep 2021 22:31:28 +0000 (UTC) X-Spam-Status: No, score=-11.4 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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-List-Received-Date: Mon, 06 Sep 2021 22:31:33 -0000 On Wed, Aug 25, 2021 at 08:19:20PM -0600, Tom Tromey wrote: > The new DIE scanner works more or less along the lines indicated by > the text for the .debug_names section, disregarding the bugs in the > specification. > > While working on this, I noticed that whether a DIE is interesting is > a static property of the DIE's abbrev. It also turns out that many > abbrevs imply a static size for the DIE data, and additionally that > for many abbrevs, the sibling offset is stored at a constant offset > from the start of the DIE. > > This patch changes the abbrev reader to analyze each abbrev and stash > the results on the abbrev. These combine to speed up the new indexer. > If the "interesting" flag is false, GDB knows to skip the DIE > immediately. If the sibling offset is statically known, skipping can > be done without reading any attributes; and in some other cases, the > DIE can be skipped using simple arithmetic. > --- > gdb/dwarf2/abbrev.c | 158 ++++++++++++++++++++++++++++++++++++++++++++ > gdb/dwarf2/abbrev.h | 7 +- > 2 files changed, 163 insertions(+), 2 deletions(-) > > diff --git a/gdb/dwarf2/abbrev.c b/gdb/dwarf2/abbrev.c > index c84f21256fd..658868aceba 100644 > --- a/gdb/dwarf2/abbrev.c > +++ b/gdb/dwarf2/abbrev.c > @@ -76,6 +76,43 @@ abbrev_table::add_abbrev (struct abbrev_info *abbrev) > *slot = abbrev; > } > > +/* Helper function that returns true if a DIE with the given tag might > + plausibly be indexed. */ > + > +static bool > +tag_interesting_for_index (dwarf_tag tag) > +{ > + switch (tag) > + { > + case DW_TAG_array_type: > + case DW_TAG_base_type: > + case DW_TAG_class_type: > + case DW_TAG_constant: > + case DW_TAG_enumeration_type: > + case DW_TAG_enumerator: > + case DW_TAG_imported_declaration: > + case DW_TAG_imported_unit: > + case DW_TAG_inlined_subroutine: > + case DW_TAG_interface_type: > + case DW_TAG_module: > + case DW_TAG_namespace: > + case DW_TAG_ptr_to_member_type: > + case DW_TAG_set_type: > + case DW_TAG_string_type: > + case DW_TAG_structure_type: > + case DW_TAG_subprogram: > + case DW_TAG_subrange_type: > + case DW_TAG_subroutine_type: > + case DW_TAG_typedef: > + case DW_TAG_union_type: > + case DW_TAG_unspecified_type: > + case DW_TAG_variable: > + return true; > + } > + > + return false; > +} > + > /* Read in an abbrev table. */ > > abbrev_table_up > @@ -116,6 +153,17 @@ abbrev_table::read (struct dwarf2_section_info *section, > cur_abbrev->has_children = read_1_byte (abfd, abbrev_ptr); > abbrev_ptr += 1; > > + unsigned int size = 0; > + unsigned int sibling_offset = -1; > + bool is_csize = true; > + > + bool has_hardcoded_declaration = false; > + bool has_specification_or_origin = false; > + bool has_name = false; > + bool has_linkage_name = false; > + bool has_location = false; > + bool has_external = false; > + > /* Now read in declarations. */ > int num_attrs = 0; > for (;;) > @@ -142,12 +190,122 @@ abbrev_table::read (struct dwarf2_section_info *section, > if (cur_attr.name == 0) > break; > > + switch (cur_attr.name) > + { > + case DW_AT_declaration: > + if (cur_attr.form == DW_FORM_flag_present) > + has_hardcoded_declaration = true; > + break; > + > + case DW_AT_external: > + has_external = true; > + break; > + > + case DW_AT_specification: > + case DW_AT_abstract_origin: > + case DW_AT_extension: > + has_specification_or_origin = true; > + break; > + > + case DW_AT_name: > + has_name = true; > + break; > + > + case DW_AT_MIPS_linkage_name: > + case DW_AT_linkage_name: > + has_linkage_name = true; > + break; > + > + case DW_AT_const_value: > + case DW_AT_location: > + has_location = true; > + break; > + > + case DW_AT_sibling: > + if (is_csize && cur_attr.form == DW_FORM_ref4) > + sibling_offset = size; > + break; > + } > + > + switch (cur_attr.form) > + { > + case DW_FORM_data1: > + case DW_FORM_ref1: > + case DW_FORM_flag: > + case DW_FORM_strx1: > + size += 1; > + break; > + case DW_FORM_flag_present: > + case DW_FORM_implicit_const: > + break; > + case DW_FORM_data2: > + case DW_FORM_ref2: > + case DW_FORM_strx2: > + size += 2; > + break; > + case DW_FORM_strx3: > + size += 3; > + break; > + case DW_FORM_data4: > + case DW_FORM_ref4: > + case DW_FORM_strx4: > + size += 4; > + break; > + case DW_FORM_data8: > + case DW_FORM_ref8: > + case DW_FORM_ref_sig8: > + size += 8; > + break; > + case DW_FORM_data16: > + size += 16; > + break; > + > + default: > + is_csize = false; > + break; > + case DW_FORM_ref_addr: > + case DW_FORM_GNU_ref_alt: > + case DW_FORM_addr: > + is_csize = false; > + break; > + } Hi, Were the three last case meant to be replaced by the 'default' entry but forgotten at some point? It looks like they could be removed. Best, Lancelot. > + > ++num_attrs; > obstack_grow (obstack, &cur_attr, sizeof (cur_attr)); > } > > cur_abbrev = (struct abbrev_info *) obstack_finish (obstack); > cur_abbrev->num_attrs = num_attrs; > + > + if (!has_name && !has_linkage_name && !has_specification_or_origin) > + { > + /* Some anonymous DIEs are worth examining. */ > + cur_abbrev->interesting > + = (cur_abbrev->tag == DW_TAG_namespace > + || cur_abbrev->tag == DW_TAG_enumeration_type); > + } > + else if (has_hardcoded_declaration > + && (cur_abbrev->tag != DW_TAG_variable || !has_external)) > + cur_abbrev->interesting = false; > + else if (!tag_interesting_for_index (cur_abbrev->tag)) > + cur_abbrev->interesting = false; > + else if (!has_location && !has_specification_or_origin && !has_external > + && cur_abbrev->tag == DW_TAG_variable) > + cur_abbrev->interesting = false; > + else > + cur_abbrev->interesting = true; > + > + /* If there are no children, and the abbrev has a constant size, > + then we don't care about the sibling offset, because it's > + simple to just skip the entire DIE without reading a sibling > + offset. */ > + if ((!cur_abbrev->has_children && is_csize) > + /* Overflow. */ > + || sibling_offset != (unsigned short) sibling_offset) > + sibling_offset = -1; > + cur_abbrev->size_if_constant = is_csize ? size : 0; > + cur_abbrev->sibling_offset = sibling_offset; > + > abbrev_table->add_abbrev (cur_abbrev); > } > > diff --git a/gdb/dwarf2/abbrev.h b/gdb/dwarf2/abbrev.h > index 0a1ca4d39b4..ae02f76885d 100644 > --- a/gdb/dwarf2/abbrev.h > +++ b/gdb/dwarf2/abbrev.h > @@ -44,9 +44,12 @@ struct abbrev_info > /* Number identifying abbrev. */ > unsigned int number; > /* DWARF tag. */ > - enum dwarf_tag tag; > + ENUM_BITFIELD (dwarf_tag) tag : 16; > /* True if the DIE has children. */ > - unsigned short has_children; > + bool has_children; > + bool interesting; > + unsigned short size_if_constant; > + unsigned short sibling_offset; > /* Number of attributes. */ > unsigned short num_attrs; > /* An array of attribute descriptions, allocated using the struct > -- > 2.31.1 >