From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23779 invoked by alias); 11 Jul 2018 01:17:24 -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 23729 invoked by uid 89); 11 Jul 2018 01:17:22 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.4 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_STOCKGEN,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 spammy=20180627, 2018-06-27, variation, 23010 X-HELO: simark.ca Received: from simark.ca (HELO simark.ca) (158.69.221.121) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 11 Jul 2018 01:17:21 +0000 Received: from [10.0.0.11] (unknown [192.222.164.54]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 5724B1E059; Tue, 10 Jul 2018 21:17:19 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=simark.ca; s=mail; t=1531271839; bh=IojDNEv/v6fD66JP97ShmU1ApFJUMvB8NUm6Slk3Pyo=; h=Subject:To:References:From:Date:In-Reply-To:From; b=bGnnn/gioQOD1KcQCUaKO6Y1WvZdK5Qb4zUsOf1CsBclQZYx4ZzIz/KaWAYq2VYKy 26pHn5ifZ2fmN0G0TAQfIx1o3ax/+zMYQ3AtGCHzgNhiZpyV4E3C8l7RxnCKNsipXJ f2D/z7ECTedThkAIKzxCEPsS7aXPwfLbD/TF0Hz0= Subject: Re: [PATCH v2] Really fix gdb/23010: SYMBOL_LANGUAGE != DICT_LANGUAGE assertion To: Keith Seitz , gdb-patches@sourceware.org References: <20180627190341.24442-1-keiths@redhat.com> From: Simon Marchi Message-ID: <33b99113-c705-a029-47df-3e6c0fc49139@simark.ca> Date: Wed, 11 Jul 2018 01:17:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 MIME-Version: 1.0 In-Reply-To: <20180627190341.24442-1-keiths@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2018-07/txt/msg00290.txt.bz2 Hi Keith, On 2018-06-27 03:03 PM, Keith Seitz wrote: > This patch is another attempt at really fixing the multitude of assertions > being seen where symbols of one language are being added to symbol lists of > another language. > > In short, this is happening because read_file_scope was reading DIEs without > having its language set. As a result, the default language, > language_minimal, was being passed to imported DIE trees and, in certain > specific situations, causing the SYMBOL_LANGUAGE != DICT_LANGUAGE assertion > being widely reported. > > For a more thorough explanation, see the following mailing list > post: > > https://sourceware.org/ml/gdb-patches/2018-05/msg00703.html I wouldn't mind if you included the information here. > Since a test reproducer for this has been so elusive, this patch also adds a > wrapper function around add_symbol_to_list which asserts when adding a > symbol of one language to a list containing symbols of a different language. I have a question about the assertion vs complaint. Is it logically impossible (putting aside other GDB bugs) for this assertion to fail? Or is it possible now to feed bad debug info to GDB that will trigger this new assert? If it's the former, then no problem, if it's the later then an assertion is not appropriate. > Differences from v1: > - Removed complaint variation > - Updated comments > - Fixed typo > > 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 | 29 +++++++++++++++++++++++------ > 1 file changed, 23 insertions(+), 6 deletions(-) > > diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c > index 4ad0527406..ba70315957 100644 > --- a/gdb/dwarf2read.c > +++ b/gdb/dwarf2read.c > @@ -9701,6 +9701,24 @@ compute_delayed_physnames (struct dwarf2_cu *cu) > cu->method_list.clear (); > } > > +/* A wrapper for add_symbol_to_list to ensure that SYMBOL's language is > + the same as all other symbols in LISTHEAD. If a new symbol is added > + with a different language, this function asserts. */ > + > +static inline void > +dw2_add_symbol_to_list (struct symbol *symbol, struct pending **listhead) > +{ > + /* Only assert if LISTHEAD already contains symbols of a different > + language (dict_create_hashed/insert_symbol_hashed requires that all > + symbols in this list are of the same language). */ > + gdb_assert ((*listhead) == NULL > + || (*listhead)->nsyms == 0 > + || (SYMBOL_LANGUAGE ((*listhead)->symbol[0]) > + == SYMBOL_LANGUAGE (symbol))); I wonder if it's actually possible to have (*listhead)->nsyms == 0, since as soon as a "struct pending" is allocated, there is at least one sym put into it (in add_symbol_to_list). It's probably a bit hard to tell for older debug formats, but is it only for DWARF that this condition must hold? Did you consider putting this assertion directly in add_symbol_to_list? I'm not saying it's the right thing to do, I'm just asking. Thanks, Simon