This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 1/3] Add gdb.ada/info_addr_mixed_case new testcase
On 01/04/2018 01:25 PM, Pedro Alves wrote:
> On 01/04/2018 08:35 AM, Joel Brobecker wrote:
>> This patch adds a new testcase to demonstrate a regression introduced by:
>>
>> commit b5ec771e60c1a0863e51eb491c85c674097e9e13
>> Date: Wed Nov 8 14:22:32 2017 +0000
>> Subject: Introduce lookup_name_info and generalize Ada's FULL/WILD name matching
>>
>> The purpose of the testcase is to verify that a user can use any
>> casing for an Ada symbol name passed to the "info address" command.
>> After the patch above was applied, GDB was no longer able to find
>> the symbol:
>>
>> (gdb) info address My_Table
>> No symbol "My_Table" in current context.
>
> The mixed-case aspect is actually a red herring here. Using
> lowercase doesn't work either:
>
> (gdb) info address my_table
> No symbol "my_table" in current context.
>
> I think the problem is instead that "info address" is doing a
> symbol_name_match_type::FULL match, but the symbol's full name
> is pck.my_table, which doesn't match.
>
> If you pass the fully-qualified name, then it work, regardless
> of casing:
>
> (gdb) info address pck.My_Table
> Symbol "pck.my_table" is static storage at address 0x6155e0.
>
> (gdb) info address pck.my_table
> Symbol "pck.my_table" is static storage at address 0x6155e0.
>
> (gdb) info address Pck.My_Table
> Symbol "pck.my_table" is static storage at address 0x6155e0.
>
> Ada mode wants symbol names in expressions to be looked up
> using wild matching, unlike other languages. To handle that
> I had added symbol_name_match_type::EXPRESSION:
>
> /* Expression matching. The same as FULL matching in most
> languages. The same as WILD matching in Ada. */
> EXPRESSION,
>
> IIRC, this is mainly used in the completion paths.
>
> I think we'll need to make "info address" use it too, and
> possibly other commands that accept an expression as argument.
Turns out that the Ada symbol lookup machinery is sufficiently decoupled
from "regular" lookup that the problem is elsewhere.
While we may still want to consider migrating that hack towards
symbol_name_match_type::EXPRESSION, things should still work in
principle without doing that, AFAICT.
Whether to do a full or wild match is decided based on the lookup name
string (see name_match_type_from_name). That was introduced basically
as a refactor of preexisting code, IIRC.
I traced the problem to the verbatim-wrapping hack in
ada_lookup_encoded_symbol.
See the attached patch. It fixes gdb.ada/info_addr_mixed_case,
and introduces no regressions for me. WDYT?
>From 170182ca1b40c9d9d421ffd2542271561e263650 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Thu, 4 Jan 2018 17:20:10 +0000
Subject: [PATCH] Fix gdb.ada/info_addr_mixed_case.exp (PR gdb/22670)
The comments about mixed case in the testcase are actually a red
herring. The problem here is that we'd get to
ada_lookup_encoded_symbol with "my_table", which wraps the looked up
name in "<>"s to force a verbatim match, and that in turn disables
wild matching.
Fix this by swapping around the internals of ada_lookup_encoded_symbol
and ada_lookup_symbol, thus avoiding the encoding and
verbatim-wrapping in the ada_lookup_symbol case, the case that starts
with a user-provided lookup name.
Ada encoding is still done of course, in the ada_lookup_name_info
ctor. This could be also seen as avoiding the double-encoding problem
in a different way.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
PR gdb/22670
* ada-lang.c (ada_lookup_encoded_symbol): Reimplement in terms of
ada_lookup_symbol.
(ada_lookup_symbol): Reimplement in terms of
ada_lookup_symbol_list, bits factored out from
ada_lookup_encoded_symbol.
gdb/testsuite/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
PR gdb/22670
* gdb.ada/info_addr_mixed_case.exp: Remove kfail. Extend test to
exercise lower case too, and to exercise both full matching and
wild matching.
---
gdb/ada-lang.c | 43 ++++++++++++--------------
gdb/testsuite/gdb.ada/info_addr_mixed_case.exp | 15 +++++----
2 files changed, 26 insertions(+), 32 deletions(-)
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 4ecf7b0051c..5f03014bb84 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -5911,10 +5911,6 @@ ada_lookup_encoded_symbol (const char *name, const struct block *block,
domain_enum domain,
struct block_symbol *info)
{
- struct block_symbol *candidates;
- int n_candidates;
- struct cleanup *old_chain;
-
/* Since we already have an encoded name, wrap it in '<>' to force a
verbatim match. Otherwise, if the name happens to not look like
an encoded name (because it doesn't include a "__"),
@@ -5924,22 +5920,7 @@ ada_lookup_encoded_symbol (const char *name, const struct block *block,
std::string verbatim = std::string ("<") + name + '>';
gdb_assert (info != NULL);
- memset (info, 0, sizeof (struct block_symbol));
-
- n_candidates = ada_lookup_symbol_list (verbatim.c_str (), block,
- domain, &candidates);
- old_chain = make_cleanup (xfree, candidates);
-
- if (n_candidates == 0)
- {
- do_cleanups (old_chain);
- return;
- }
-
- *info = candidates[0];
- info->symbol = fixup_symbol_section (info->symbol, NULL);
-
- do_cleanups (old_chain);
+ *info = ada_lookup_symbol (verbatim.c_str (), block, domain, NULL);
}
/* Return a symbol in DOMAIN matching NAME, in BLOCK0 and enclosing
@@ -5952,13 +5933,27 @@ struct block_symbol
ada_lookup_symbol (const char *name, const struct block *block0,
domain_enum domain, int *is_a_field_of_this)
{
- struct block_symbol info;
-
if (is_a_field_of_this != NULL)
*is_a_field_of_this = 0;
- ada_lookup_encoded_symbol (ada_encode (ada_fold_name (name)),
- block0, domain, &info);
+ struct block_symbol *candidates;
+ int n_candidates;
+ struct cleanup *old_chain;
+
+ n_candidates = ada_lookup_symbol_list (name, block0, domain, &candidates);
+ old_chain = make_cleanup (xfree, candidates);
+
+ if (n_candidates == 0)
+ {
+ do_cleanups (old_chain);
+ return {};
+ }
+
+ block_symbol info = candidates[0];
+ info.symbol = fixup_symbol_section (info.symbol, NULL);
+
+ do_cleanups (old_chain);
+
return info;
}
diff --git a/gdb/testsuite/gdb.ada/info_addr_mixed_case.exp b/gdb/testsuite/gdb.ada/info_addr_mixed_case.exp
index e9fce0d93c9..7840a434b37 100644
--- a/gdb/testsuite/gdb.ada/info_addr_mixed_case.exp
+++ b/gdb/testsuite/gdb.ada/info_addr_mixed_case.exp
@@ -31,12 +31,11 @@ if ![runto "foo.adb:$bp_location" ] then {
# The following test exercises the situation when uppercase letters
# are used in the name of the symbol passed to the "info address"
-# command. This should not make a difference, as the language is
-# Ada, and Ada is case-insensitive.
+# command. This should not make a difference, as the language is Ada,
+# and Ada is case-insensitive. Also, exercise both fully-qualified
+# name matching and wild matching.
-# commit b5ec771e60c1a0863e51eb491c85c674097e9e13 (Introduce
-# lookup_name_info and generalize Ada's FULL/WILD name matching)
-# caused the following test to fail. KFAIL it while investigating...
-setup_kfail gdb/22670 "*-*-*"
-gdb_test "info address My_Table" \
- "Symbol \"pck\\.my_table\" is static storage at address $hex\\."
+foreach sym {"my_table" "My_Table" "pck.my_table" "Pck.My_Table"} {
+ gdb_test "info address $sym" \
+ "Symbol \"pck\\.my_table\" is static storage at address $hex\\."
+}
--
2.14.3