From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out2.suse.de (smtp-out2.suse.de [IPv6:2001:67c:2178:6::1d]) by sourceware.org (Postfix) with ESMTPS id 478833858C50 for ; Tue, 18 Oct 2022 13:41:54 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 478833858C50 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 848E620724; Tue, 18 Oct 2022 13:41:53 +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 6BAFD139D2; Tue, 18 Oct 2022 13:41:53 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id 6CuvGCGtTmP5IwAAMHmgww (envelope-from ); Tue, 18 Oct 2022 13:41:53 +0000 Message-ID: <47302e23-30af-d8ca-b1fb-28977110a72a@suse.de> Date: Tue, 18 Oct 2022 15:41:53 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.2.2 Subject: Re: [PATCH] Fix incorrect .gdb_index with new DWARF scanner Content-Language: en-US To: Tom Tromey , gdb-patches@sourceware.org References: <20221017194118.2129436-1-tromey@adacore.com> From: Tom de Vries In-Reply-To: <20221017194118.2129436-1-tromey@adacore.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-13.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, NICE_REPLY_A, RCVD_IN_DNSWL_LOW, 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-List-Received-Date: Tue, 18 Oct 2022 13:41:56 -0000 On 10/17/22 21:41, Tom Tromey via Gdb-patches wrote: > PR symtab/29694 points out a regression caused by the new DWARF > scanner when the cc-with-gdb-index target board is used. > > What happens here is that an older version of gdb will make an index > describing the "A" type as: > > [737] A: 1 [global, type] > > whereas the new gdb says: > > [1008] A: 0 [global, type] > > Here the old one is correct because the A in CU 0 is just a > declaration without a size: > > <1><45>: Abbrev Number: 10 (DW_TAG_structure_type) > <46> DW_AT_name : A > <48> DW_AT_declaration : 1 > <48> DW_AT_sibling : <0x6d> > > This patch fixes the problem by introducing the idea of a "type > declaration". I think gdb still needs to recurse into these types, > searching for methods, but by marking the type itself as a > declaration, gdb can skip this type during lookups and when writing > the index. > > Regression tested on x86-64 using the cc-with-gdb-index board. > Regression tested on openSUSE Tumbleweed x86_64 with native and cc-with-gdb-index board, results looks good. Patch LGTM. Thanks, - Tom > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29694 > --- > gdb/dwarf2/cooked-index.h | 15 +++++++++++++++ > gdb/dwarf2/index-write.c | 5 +++++ > gdb/dwarf2/read.c | 22 ++++++++++++++-------- > 3 files changed, 34 insertions(+), 8 deletions(-) > > diff --git a/gdb/dwarf2/cooked-index.h b/gdb/dwarf2/cooked-index.h > index f3c26480a81..2ea32781be5 100644 > --- a/gdb/dwarf2/cooked-index.h > +++ b/gdb/dwarf2/cooked-index.h > @@ -48,6 +48,9 @@ enum cooked_index_flag_enum : unsigned char > IS_ENUM_CLASS = 4, > /* True if this entry uses the linkage name. */ > IS_LINKAGE = 8, > + /* True if this entry is just for the declaration of a type, not the > + definition. */ > + IS_TYPE_DECLARATION = 16, > }; > DEF_ENUM_FLAGS_TYPE (enum cooked_index_flag_enum, cooked_index_flag); > > @@ -76,6 +79,10 @@ struct cooked_index_entry : public allocate_on_obstack > /* Return true if this entry matches SEARCH_FLAGS. */ > bool matches (block_search_flags search_flags) const > { > + /* Just reject type declarations. */ > + if ((flags & IS_TYPE_DECLARATION) != 0) > + return false; > + > if ((search_flags & SEARCH_STATIC_BLOCK) != 0 > && (flags & IS_STATIC) != 0) > return true; > @@ -88,6 +95,10 @@ struct cooked_index_entry : public allocate_on_obstack > /* Return true if this entry matches DOMAIN. */ > bool matches (domain_enum domain) const > { > + /* Just reject type declarations. */ > + if ((flags & IS_TYPE_DECLARATION) != 0) > + return false; > + > switch (domain) > { > case LABEL_DOMAIN: > @@ -106,6 +117,10 @@ struct cooked_index_entry : public allocate_on_obstack > /* Return true if this entry matches KIND. */ > bool matches (enum search_domain kind) const > { > + /* Just reject type declarations. */ > + if ((flags & IS_TYPE_DECLARATION) != 0) > + return false; > + > switch (kind) > { > case VARIABLES_DOMAIN: > diff --git a/gdb/dwarf2/index-write.c b/gdb/dwarf2/index-write.c > index f592734addc..3d215a6307b 100644 > --- a/gdb/dwarf2/index-write.c > +++ b/gdb/dwarf2/index-write.c > @@ -1167,6 +1167,11 @@ write_cooked_index (cooked_index_vector *table, > be redundant are rare and not worth supporting. */ > continue; > } > + else if ((entry->flags & IS_TYPE_DECLARATION) != 0) > + { > + /* Don't add type declarations to the index. */ > + continue; > + } > > gdb_index_symbol_kind kind; > if (entry->tag == DW_TAG_subprogram) > diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c > index b5efcb3cc09..e849f62576d 100644 > --- a/gdb/dwarf2/read.c > +++ b/gdb/dwarf2/read.c > @@ -18192,14 +18192,20 @@ cooked_indexer::scan_attributes (dwarf2_per_cu_data *scanning_per_cu, > that is ok. Similarly, we allow an external variable without a > location; those are resolved via minimal symbols. */ > if (is_declaration && !for_specification > - && !(abbrev->tag == DW_TAG_variable && (*flags & IS_STATIC) == 0) > - && !((abbrev->tag == DW_TAG_class_type > - || abbrev->tag == DW_TAG_structure_type > - || abbrev->tag == DW_TAG_union_type) > - && abbrev->has_children)) > - { > - *linkage_name = nullptr; > - *name = nullptr; > + && !(abbrev->tag == DW_TAG_variable && (*flags & IS_STATIC) == 0)) > + { > + /* We always want to recurse into some types, but we may not > + want to treat them as definitions. */ > + if ((abbrev->tag == DW_TAG_class_type > + || abbrev->tag == DW_TAG_structure_type > + || abbrev->tag == DW_TAG_union_type) > + && abbrev->has_children) > + *flags |= IS_TYPE_DECLARATION; > + else > + { > + *linkage_name = nullptr; > + *name = nullptr; > + } > } > else if ((*name == nullptr > || (*linkage_name == nullptr