From patchwork Thu Jan 4 18:32:57 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 25213 Received: (qmail 30050 invoked by alias); 4 Jan 2018 18:33:02 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 30019 invoked by uid 89); 4 Jan 2018 18:33:01 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, SPF_HELO_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=Expression, 3111 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 04 Jan 2018 18:33:00 +0000 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id E2BAAC051785; Thu, 4 Jan 2018 18:32:58 +0000 (UTC) Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id 204466017B; Thu, 4 Jan 2018 18:32:57 +0000 (UTC) Subject: Re: [PATCH 1/3] Add gdb.ada/info_addr_mixed_case new testcase To: Joel Brobecker , gdb-patches@sourceware.org References: <1515054953-81012-1-git-send-email-brobecker@adacore.com> <1515054953-81012-2-git-send-email-brobecker@adacore.com> <5c1398c0-0b8a-f480-551c-d1c2c65daed8@redhat.com> From: Pedro Alves Message-ID: Date: Thu, 4 Jan 2018 18:32:57 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: <5c1398c0-0b8a-f480-551c-d1c2c65daed8@redhat.com> 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 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 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 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