From patchwork Wed Mar 20 20:34:40 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom Tromey X-Patchwork-Id: 31922 Received: (qmail 128153 invoked by alias); 20 Mar 2019 20:34:45 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 128136 invoked by uid 89); 20 Mar 2019 20:34:45 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-14.1 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_STOCKGEN, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS autolearn=ham version=3.3.1 spammy=patience, Joel, joel, uncomfortable X-HELO: gateway36.websitewelcome.com Received: from gateway36.websitewelcome.com (HELO gateway36.websitewelcome.com) (50.116.125.2) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 20 Mar 2019 20:34:43 +0000 Received: from cm13.websitewelcome.com (cm13.websitewelcome.com [100.42.49.6]) by gateway36.websitewelcome.com (Postfix) with ESMTP id E4F42400C88DE for ; Wed, 20 Mar 2019 14:50:59 -0500 (CDT) Received: from box5379.bluehost.com ([162.241.216.53]) by cmsmtp with SMTP id 6hv7hSMTqYTGM6hv7hf1ax; Wed, 20 Mar 2019 15:34:41 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=tromey.com; s=default; h=Content-Type:MIME-Version:Message-ID:In-Reply-To:Date: References:Subject:Cc:To:From:Sender:Reply-To:Content-Transfer-Encoding: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=1fFEjSaczAN7z9gnt6+mM7FnzRMhF6zt6G/QcQTB1rk=; b=n8rh9xabBUJi06hBwjj/Hh8mem 9AJJSNDpfoOtIVCSSz5NV8N5J2nmlONd0xk1sSlcIjbKBEtiVCYGA264EbEg1AGStAlQlWeyDXyvH 5StJXoM3T5OaG6/HC/H3y34+r; Received: from 174-29-37-56.hlrn.qwest.net ([174.29.37.56]:54708 helo=murgatroyd) by box5379.bluehost.com with esmtpsa (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.91) (envelope-from ) id 1h6hv7-001tUj-Ls; Wed, 20 Mar 2019 15:34:41 -0500 From: Tom Tromey To: Joel Brobecker Cc: gdb-patches@sourceware.org Subject: Re: [RFC] partial symbol name matching vs regexp References: <20181006001621.GA6358@adacore.com> Date: Wed, 20 Mar 2019 14:34:40 -0600 In-Reply-To: <20181006001621.GA6358@adacore.com> (Joel Brobecker's message of "Fri, 5 Oct 2018 17:16:21 -0700") Message-ID: <87k1gtp9nj.fsf@tromey.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 >>>>> "Joel" == Joel Brobecker writes: Joel> One of our users reported that info types was sometimes not working Joel> for Ada types. Consider for instance the following code: [...] I finally got to reading this message. Thanks for the thorough write-up; and for your patience waiting for a response. Joel> In our case, that symbol_matchers doesn't match alpha.beta, Joel> because SYMNAME is the symbol's search name, and in Ada, Joel> we search by encoded name. Thus, for the symbol we were Joel> hoping to match, it gets called with "alpha__beta". The crux of the problem. Joel> With that assumption in mind, I think the way to make it work Joel> is to change the symbol_matcher's signature to receive more Joel> information - so that each caller of expand_symtabs_matching can Joel> then decide which symbol name it needs to look at This makes sense to me. Joel> Following this, I thought the simplest would probably to have Joel> the symbol_matcher receive a general_symbol_info instead of Joel> just the name. I am not so sure about this part. See below. [ gdb index ] Joel> Trying to resolve that hitch with the gdb_index handling, and Joel> after having read the description of what's in the gdb_index Joel> section, I think there should be a way to link a symbol in Joel> the gdb_index back to the DWARF CU, and therefore get the symbol's Joel> language. This can be done, maybe with a small bit of difficulty. In the index, each symbol references its CU. However, to get the language, you'd have to instantiate a reader and read the outermost DIE -- not too difficult but does require some new code, I think. Joel> Thus, a second option would be to keep the name parameter Joel> in expand_symtabs_symbol_matcher_ftype, but then add the symbol Joel> language as a second parameter; we would then modify the symbol_matcher Joel> in search_symbols to check the language, and if the language is Ada, Joel> then decode the name before evaluating the regexp. ... Joel> But I'm not really satisfied with the second option (two parameters). What about passing a flag to expand_symtabs_matching that says whether the particular site wants the natural name or the search name? I've appended a symfile.h patch that shows the core change. Joel> What if a search routine wanted to expand based on the linkage name, Joel> one day, even for C++ symbols. I don't know if mangling a symbol back Joel> would be possible or not, but regardless of that, it would feel wrong Joel> to compute a mangled name when the caller had it. Maybe re-mangling a symbol could be done, but I don't think we'd want to attempt it. This whole area has been a bit broken for years now, because you still can't look up symbols by their mangled name, and the "demangle" setting basically doesn't impact C++. Oops on all that! At one point I had a complicated plan to fix this, but maybe something less complicated could be done... not sure. Another thing I wanted to mention here is that IMO we should start considering .gdb_index as deprecated, and should move to the DWARF 5 indices as much as possible. (I don't know the full state of this support in the toolchain though. So maybe I'm super mistaken about the viability of this.) Joel> For that reason, I tend to still think that changing Joel> expand_symtabs_symbol_matcher_ftype to receive a general_symbol_info Joel> is better. It's not perfect in the case of gdb_index handling, Joel> but I think that the consequences of that peculiarity would be Joel> contained within the gdb_index handling in dwarf2read.c. So, Joel> at least, it wouldn't be a caller in dwarf2read.c passing an Joel> unsuspecting symbol_matcher function defined elsewhere an incomplete Joel> general_symbol_info. This is the part of the patch that is a bit uncomfortable for me. I mean, on the one hand, maybe it isn't so bad: presumably if some callback mistreats the general_symbol_info, it will simply not work. But... I feel like we've had situations in the past with symbol tables where things didn't work properly and simply went undiagnosed for years (perhaps sometimes working via a minsym fallback). And, I think the peculiarity isn't actually isolated. dw2_expand_symtabs_matching_symbol calls symbol_matcher, which ultimately is a callback function passed in by the caller of expand_symtabs_matching. So, every caller of that will have to know to only look at certain fields of the symbol. Tom diff --git a/gdb/symfile.h b/gdb/symfile.h index 64d5a23f9b2..bb7a340132b 100644 --- a/gdb/symfile.h +++ b/gdb/symfile.h @@ -245,8 +245,9 @@ struct quick_symbol_functions Otherwise, if KIND does not match, this symbol is skipped. If even KIND matches, SYMBOL_MATCHER is called for each symbol - defined in the file. The symbol "search" name is passed to - SYMBOL_MATCHER. + defined in the file. If SEARCH_NAME is true, then the symbol + "search" name is passed to SYMBOL_MATCHER; otherwise the symbol + "natural" name is passed in. If SYMBOL_MATCHER returns false, then the symbol is skipped. @@ -257,7 +258,8 @@ struct quick_symbol_functions const lookup_name_info &lookup_name, gdb::function_view symbol_matcher, gdb::function_view expansion_notify, - enum search_domain kind); + enum search_domain kind, + bool search_name); /* Return the comp unit from OBJFILE that contains PC and SECTION. Return NULL if there is no such compunit. This @@ -526,7 +528,8 @@ void expand_symtabs_matching const lookup_name_info &lookup_name, gdb::function_view symbol_matcher, gdb::function_view expansion_notify, - enum search_domain kind); + enum search_domain kind, + bools search_name); void map_symbol_filenames (symbol_filename_ftype *fun, void *data, int need_fullname);