From: Keith Seitz <keiths@redhat.com>
To: Simon Marchi <simark@simark.ca>, gdb-patches@sourceware.org
Subject: Re: [PATCH v2] Really fix gdb/23010: SYMBOL_LANGUAGE != DICT_LANGUAGE assertion
Date: Thu, 12 Jul 2018 15:47:00 -0000 [thread overview]
Message-ID: <97d83efa-56b9-0e1a-a6d5-1ee2115c8a99@redhat.com> (raw)
In-Reply-To: <33b99113-c705-a029-47df-3e6c0fc49139@simark.ca>
On 07/10/2018 06:17 PM, Simon Marchi wrote:
> I wouldn't mind if you included the information here.
I will certainly include a redux of that analysis in the commit log.
> 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.
IMO the former. The code is currently structured so that a dictionary can *only* have symbols of one language inside it. While it may be possible to trigger this with malicious DWARF hacking, I cannot really say for certain that it isn't possible in valid DWARF. [Sorry, that's more of a politician's answer to your question.]
>> 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).
Ha! Yes, that's true. I didn't even look. I just erred on the defensive side. I'll remove that.
> 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.
The assertion holds for any debug reader that uses hashed dictionaries, and that list isn't a straightforward list to compose. That's probably why I originally isolated to dwarf2read.c. Or I just plain didn't think about (interfering with) other debuginfo readers.
Any symbol reader using add_symbol_to_list could create hashed dictionaries, but AFAICT there is no requirement that it does.
Out of curiosity, I moved (and adjusted) the assertion to add_symbol_to_list and ran some tests. As expected, DWARF tests showed no difference. STABS testing, though, triggers the assertion. add_symbol_to_list is often called before any dictionary is created.
Thank you for taking a look,
Keith
next prev parent reply other threads:[~2018-07-12 15:47 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-06-27 19:03 Keith Seitz
2018-07-11 1:17 ` Simon Marchi
2018-07-12 15:47 ` Keith Seitz [this message]
2018-07-12 17:12 ` Simon Marchi
2018-07-24 20:13 ` Simon Marchi
2018-07-24 20:25 ` Keith Seitz
2018-07-12 17:12 ` Tom Tromey
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=97d83efa-56b9-0e1a-a6d5-1ee2115c8a99@redhat.com \
--to=keiths@redhat.com \
--cc=gdb-patches@sourceware.org \
--cc=simark@simark.ca \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).