From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 18257 invoked by alias); 11 Jun 2018 22:26:58 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 18244 invoked by uid 89); 11 Jun 2018 22:26:57 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-23.6 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_PASS autolearn=ham version=3.3.2 spammy=Small, well!, complaint, confess X-HELO: rock.gnat.com Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 11 Jun 2018 22:26:56 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 8AE2B560A9; Mon, 11 Jun 2018 18:26:54 -0400 (EDT) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id hp+fR4M7UXM8; Mon, 11 Jun 2018 18:26:54 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 55F755608E; Mon, 11 Jun 2018 18:26:54 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id 5B37B86EB5; Mon, 11 Jun 2018 15:26:52 -0700 (PDT) Date: Mon, 11 Jun 2018 22:26:00 -0000 From: Joel Brobecker To: Keith Seitz Cc: gdb-patches@sourceware.org Subject: Re: [RFC PATCH] Really fix gdb/23010: SYMBOL_LANGUAGE != DICT_LANGUAGE assertion Message-ID: <20180611222652.GA29595@adacore.com> References: <20180605180912.3256-1-keiths@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180605180912.3256-1-keiths@redhat.com> User-Agent: Mutt/1.9.4 (2018-02-28) X-SW-Source: 2018-06/txt/msg00295.txt.bz2 Hey Keith, First of all, thanks a lot again for all your work on this PR. Anyone, please feel free to comment as well! > gdb/ChangeLog: > > PR gdb/23010 > * dwarf2read.c (dw2_add_symbol_to_list): New function. > (fixup_go_packaging, new_symbol): Use dw2_add_symbol_to_list > instead of add_symbol_to_list. > (read_file_scope): Call prepare_one_comp_unit before reading > any other DIEs. > --- > gdb/dwarf2read.c | 38 ++++++++++++++++++++++++++++++++------ > 1 file changed, 32 insertions(+), 6 deletions(-) > > diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c > index 1cabfbb0d4..04f22114f8 100644 > --- a/gdb/dwarf2read.c > +++ b/gdb/dwarf2read.c > @@ -9701,6 +9701,33 @@ compute_delayed_physnames (struct dwarf2_cu *cu) > cu->method_list.clear (); > } > > +/* A wrapper for add_symbol_to_list to issue a complaint if a symbol > + with a different language is added to LISTHEAD. */ > + > +static inline void > +dw2_add_symbol_to_list (struct symbol *symbol, struct pending **listhead) > +{ > + /* Only complain if LISTHEAD already contains symbols of a different > + language. */ I confess I got confused a bit by this comment about thinking this was a comment about the function's behavior, as in "only complain once", and therefore belonging in the function's introductory comment. But thinking more clearly about this, you have to wait for at least one symbol to be in the list; otherwise, talking to myself what else would you compare it too, right? But I am wondering if we could do it a bit better. In particular, at the moment, it seems like all you have is the existing list of symbols; but more interesting would be the CU's language, right? What about passing the dwarf_cu? From what I can tell, we seem to have it each time we call this function. Or is that exactly the issue we're dealing with? > +#if USE_ASSERT > + gdb_assert ((*listhead) == NULL > + || (*listhead)->nsyms == 0 > + || (SYMBOL_LANGUAGE ((*listhead)->symbol[0]) > + == SYMBOL_LANGUAGE (symbol))); So, if I understand your preliminary explanation, this part will be removed, right? > +#else > + if ((*listhead) != NULL && (*listhead)->nsyms > 0 > + && SYMBOL_LANGUAGE ((*listhead)->symbol[0]) != SYMBOL_LANGUAGE (symbol)) > + { > + complaint (_("recording symbol \"%s\" with language %s " > + "into list of langauge %s"), SYMBOL_LINKAGE_NAME (symbol), ^^^^^^^^ Small typo in "langauge" Other than that, no other comment for branch "master". Do we want something for the gdb-8.1.1 release? I would have thought so. But I might suggest instead the shorter version, without the complaint (just because it gives us a bit more time on master to double-check that the overhead is indeed minimal -- not as obvious as I might have thought when I first thought about it). Thanks! -- Joel