Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Pedro Alves <palves@redhat.com>
To: Joel Brobecker <brobecker@adacore.com>, gdb-patches@sourceware.org
Subject: 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: Fri, 05 Jan 2018 16:34:00 -0000	[thread overview]
Message-ID: <f6c80127-ef1e-7381-d7e5-3f2f0dc3a02c@redhat.com> (raw)
In-Reply-To: <1515054953-81012-4-git-send-email-brobecker@adacore.com>

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 insert
> a breakpoint on a C function while debugging Ada, even if the name
> of the function includes uppercase letters, requiring us to use
> Ada's "<...>" notation to tell the GDB that the symbol name should
> be looked up verbatim.
> 
> As of the commit above, GDB is no longer finding the function:
> 
>     (gdb) break <MixedCaseFunc>
>     Function "<MixedCaseFunc>" not defined.
>     Make breakpoint pending on future shared library load? (y or [n])
> 
> Before the patch, the breakpoint was inserted without problem.
> 

Below's a fix for this one.

I've force-pushed this one along with the following on to the
users/palves/literal-matching branch, rebased on master to pick up the
other fixes that went in meanwhile.

From 439f8c51ff8f6cd9fb3bbc330a40492a15992add Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Fri, 5 Jan 2018 00:17:19 +0000
Subject: [PATCH 1/2] Fix gdb.ada/bp_c_mixed_case.exp (PR gdb/22670)

The problem here is that we were using the user-provided lookup name
literally for linkage name comparisons.  I.e., "<MixedCase>" with the
"<>"s included.  That obviously can't work since the "<>" are not
really part of the linkage name.  The original idea was that we'd use
the symbol's language to select the right symbol name matching
algorithm, but that doesn't work for Ada because it's not really
possible to unambiguously tell from the linkage name alone whether
we're dealing with Ada symbols, so Ada minsyms end up with no language
set, or sometimes C++ set.  So fix this by treating Ada mode specially
when determining the linkage name to match against.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	PR gdb/22670
	* minsyms.c (linkage_name_str): New function.
	(iterate_over_minimal_symbols): Use it.

gdb/testsuite/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	PR gdb/22670
	* gdb.ada/bp_c_mixed_case.exp: Remove setup_kfail calls.
---
 gdb/minsyms.c                             | 21 ++++++++++++++++++++-
 gdb/testsuite/gdb.ada/bp_c_mixed_case.exp |  4 +---
 2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/gdb/minsyms.c b/gdb/minsyms.c
index 26c91ec8819..fded0d65e93 100644
--- a/gdb/minsyms.c
+++ b/gdb/minsyms.c
@@ -447,6 +447,25 @@ find_minimal_symbol_address (const char *name, CORE_ADDR *addr,
   return sym.minsym == NULL;
 }
 
+/* Get the lookup name form best suitable for linkage name
+   matching.  */
+
+static const char *
+linkage_name_str (const lookup_name_info &lookup_name)
+{
+  /* Unlike most languages (including C++), Ada uses the
+     encoded/linkage name as the search name recorded in symbols.  So
+     if debugging in Ada mode, prefer the Ada-encoded name.  This also
+     makes Ada's verbatim match syntax ("<...>") work, because
+     "lookup_name.name()" includes the "<>"s, while
+     "lookup_name.ada().lookup_name()" is the encoded name with "<>"s
+     stripped.  */
+  if (current_language->la_language == language_ada)
+    return lookup_name.ada ().lookup_name ().c_str ();
+
+  return lookup_name.name ().c_str ();
+}
+
 /* See minsyms.h.  */
 
 void
@@ -459,7 +478,7 @@ iterate_over_minimal_symbols (struct objfile *objf,
 
   /* The first pass is over the ordinary hash table.  */
     {
-      const char *name = lookup_name.name ().c_str ();
+      const char *name = linkage_name_str (lookup_name);
       unsigned int hash = msymbol_hash (name) % MINIMAL_SYMBOL_HASH_SIZE;
       auto *mangled_cmp
 	= (case_sensitivity == case_sensitive_on
diff --git a/gdb/testsuite/gdb.ada/bp_c_mixed_case.exp b/gdb/testsuite/gdb.ada/bp_c_mixed_case.exp
index 54c61e3a8e8..7787646c67f 100644
--- a/gdb/testsuite/gdb.ada/bp_c_mixed_case.exp
+++ b/gdb/testsuite/gdb.ada/bp_c_mixed_case.exp
@@ -40,13 +40,11 @@ gdb_test "show lang" \
 # Try inserting a breakpoint inside a C function. Because the function's
 # name has some uppercase letters, we need to use the "<...>" notation.
 # The purpose of this testcase is to verify that we can in fact do so
-# and that it inserts the breakpoint at the expected location.
-setup_kfail gdb/22670 "*-*-*"
+# and that it inserts the breakpoint at the expected location.  See gdb/22670.
 gdb_test "break <MixedCaseFunc>" \
          "Breakpoint $decimal at $hex: file .*bar.c, line $decimal\\."
 
 # Resume the program's execution, verifying that it lands at the expected
 # location.
-setup_kfail gdb/22670 "*-*-*"
 gdb_test "continue" \
          "Breakpoint $decimal, MixedCaseFunc \\(\\) at .*bar\\.c:$decimal.*"
-- 
2.14.3


  reply	other threads:[~2018-01-05 16:34 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 3/3] Add new gdb.ada/bp_c_mixed_case testcase for PR gdb/22670 Joel Brobecker
2018-01-05 16:34   ` Pedro Alves [this message]
2018-01-08  3:57     ` 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) Joel Brobecker
2018-01-08 15:00       ` Pedro Alves
2018-01-09  9:46         ` Joel Brobecker
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
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 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

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=f6c80127-ef1e-7381-d7e5-3f2f0dc3a02c@redhat.com \
    --to=palves@redhat.com \
    --cc=brobecker@adacore.com \
    --cc=gdb-patches@sourceware.org \
    /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