From: Joel Brobecker <brobecker@adacore.com>
To: Pedro Alves <palves@redhat.com>
Cc: gdb-patches@sourceware.org
Subject: Re: Fix gdb.ada/bp_c_mixed_case.exp (PR gdb/22670) (Re: [PATCH 3/3] Add new gdb.ada/bp_c_mixed_case testcase for PR gdb/22670)
Date: Tue, 09 Jan 2018 09:46:00 -0000 [thread overview]
Message-ID: <20180109094625.7udsip4v23i4d5no@adacore.com> (raw)
In-Reply-To: <f2581935-af98-ea32-6c1e-82a85364485a@redhat.com>
> > I am wondering why minimal symbols are involved in this case,
> > considering that the C file was build with debugging information.
> > Shouldn't we be getting the function's address from the partial/full
> > symtabs instead?
>
> AFAIK, GDB always worked this way for linespecs, even before my C++
> wildmatching patches -- we collect symbols from both debug info and
> minsyms, and coalesce them by address to avoid duplicates
> (linespec.c:add_matching_symbols_to_info).
That's true.
What surprises me is that, before your patch, we were finding
no symbol at all. So we were failing the lookup both with minimal
symbols, and within the partial/full symtab.
Your patch, IIUC, handles the lookup at the minimal symbol level,
which is indeed a good thing. But shouldn't we also be finding
that same symbol through the partial/full symtab search? I have
a feeling that your minimal symbol patch might be hiding a bug
in the search for the symbol, at least from the linespec module.
I did a bit of debugging this morning, first with the following
snapshot, which is shortly before the wild-matching patch series:
commit b346cb961f729e2955391513a5b05eaf02b308ea
Author: GDB Administrator <gdbadmin@sourceware.org>
Date: Wed Nov 8 00:00:20 2017 +0000
The function iterate_over_all_matching_symtabs finds the function
in the bar.c's partial symtab because the matching function is...
[&] (const char *symbol_name)
{
return symbol_name_cmp (symbol_name, name) == 0;
},
... where name, in this case is "MixedCaseFunc" -- The "<>" has been
stripped. They got stripped by linespec.c::find_linespec_symbols
when it took that name and converted it to a lookup name via:
if (state->language->la_language == language_ada)
{
/* In Ada, the symbol lookups are performed using the encoded
name rather than the demangled name. */
ada_lookup_storage = ada_name_for_lookup (name);
lookup_name = ada_lookup_storage.c_str ();
}
else
{
lookup_name = demangle_for_lookup (name,
state->language->la_language,
demangle_storage);
}
In the newer version, find_linespec_symbols gets passed the lookup_name
directly, and that lookup_name is now "<MixedCaseFunc>". Those extra
"<...>" are what eventually gets in the way when we compare this
lookup_name against the partial's symbols name (in
default_symbol_name_matcher, which does an strncmp_iw_with_mode
comparison, IIUC).
The call to find_linespec_symbols comes from linespace_parse_basic,
which has:
/* Try looking it up as a function/method. */
find_linespec_symbols (PARSER_STATE (parser),
PARSER_RESULT (parser)->file_symtabs, name,
PARSER_EXPLICIT (parser)->func_name_match_type,
&symbols, &minimal_symbols);
I really hate to be stopping the investigation at this point, as
I feel I am onto something, but I am running out of time for today.
The part where I am not sure yet is whether we should be transforming
"name" into a "lookup_name" before calling find_linespec_symbols, or
whether we should be handling the angle brackets during the symbol
comparison... Or something else entirely! This is still all fairly
new to me...
Note that I was thinkg we would need to be stripping the executable
for us to demonstrate an error, but in fact, this is what happens
if I use "print" instead of "break":
(gdb) p <MixedCaseFunc>
$1 = {<text variable, no debug info>} 0x4024dc <MixedCaseFunc>
With the snapshot prior to the patch series, GDB knows that
MixedCaseFunc is a function without parameters, and the expression
above means calling it. As I was debugging without having started
the inferior, I got the following (expected) error:
(gdb) print <MixedCaseFunc>
You can't do that without a process to debug.
in the bp_c_mixed_case.exp, we should see GDB telling us that
we stopped on our MixedCaseFunc breakpoint while evaluating
a function call...
Does this make some kind of sense to you? I can get back to this
for more digging again tomorrow.
Thanks!
--
Joel
next prev parent reply other threads:[~2018-01-09 9:46 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-01-04 8:36 FYI/pushed: Additional tests showing regression post C++ wild matching Joel Brobecker
2018-01-04 8:36 ` [PATCH 1/3] Add gdb.ada/info_addr_mixed_case new testcase Joel Brobecker
2018-01-04 13:25 ` Pedro Alves
2018-01-04 18:33 ` Pedro Alves
2018-01-05 3:22 ` Joel Brobecker
2018-01-05 16:06 ` Pedro Alves
2018-01-04 8:36 ` [PATCH 2/3] Add "complete break ada" test to gdb.ada/complete.exp Joel Brobecker
2018-01-05 16:37 ` [PATCH] Fix gdb.ada/complete.exp's "complete break ada" test (PR, gdb/22670) (Re: [PATCH 2/3] Add "complete break ada" test to gdb.ada/complete.exp) Pedro Alves
2018-01-08 4:05 ` Joel Brobecker
2018-01-04 8:36 ` [PATCH 3/3] Add new gdb.ada/bp_c_mixed_case testcase for PR gdb/22670 Joel Brobecker
2018-01-05 16:34 ` Fix gdb.ada/bp_c_mixed_case.exp (PR gdb/22670) (Re: [PATCH 3/3] Add new gdb.ada/bp_c_mixed_case testcase for PR gdb/22670) Pedro Alves
2018-01-08 3:57 ` Joel Brobecker
2018-01-08 15:00 ` Pedro Alves
2018-01-09 9:46 ` Joel Brobecker [this message]
2018-01-09 14:59 ` Pedro Alves
2018-01-09 16:45 ` Pedro Alves
2018-01-09 17:22 ` Pedro Alves
2018-01-10 3:36 ` Joel Brobecker
2018-01-10 23:41 ` Pedro Alves
2018-01-11 4:00 ` Joel Brobecker
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=20180109094625.7udsip4v23i4d5no@adacore.com \
--to=brobecker@adacore.com \
--cc=gdb-patches@sourceware.org \
--cc=palves@redhat.com \
/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).