public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Tom de Vries <tdevries@suse.de>
To: gdb-patches@sourceware.org
Cc: Tom Tromey <tom@tromey.com>
Subject: [PATCH] [gdb/symtab] Remove superfluous handling of Ada main in write_cooked_index
Date: Thu,  3 Aug 2023 15:34:14 +0200	[thread overview]
Message-ID: <20230803133414.22219-1-tdevries@suse.de> (raw)

I filed PR29179 about the following FAIL in test-case
gdb.ada/O2_float_param.exp with target board cc-with-gdb-index:
...
(gdb) break increment^M
Function "increment" not defined.^M
Make breakpoint pending on future shared library load? (y or [n]) n^M
(gdb) FAIL: gdb.ada/O2_float_param.exp: scenario=all: gdb_breakpoint: \
  set breakpoint at increment
...

The FAIL was a regression since commit 2cf349be0e3 ("Do not put linkage names
into .gdb_index").

Before that commit we had:
...
$ readelf -w foo > READELF
$ grep callee.*increment READELF
[1568] callee__increment: 5 [global, function]
[3115] callee.increment: 5 [global, function]
...
but after only:
...
$ grep callee.*increment READELF
[3115] callee.increment: 5 [global, function]
...

The regression was fixed by commit 67e83a0deef ("Fix regression in
c-linkage-name.exp with gdb index"), which got us again:
...
$ grep callee.*increment READELF
[1568] callee__increment: 5 [global, function]
[3115] callee.increment: 5 [global, function]
...

The commit however did not claim that particular PR.  A subsequent commit,
commit 5fea9794325 ("Improve Ada support in .gdb_index") did claim to fix it,
together with commit dd05fc7071a ("Change .gdb_index de-duplication
implementation").

The commit 5fea9794325 contained the following addition in write_cooked_index:
...
+      if (entry->per_cu->lang () == language_ada)
+	{
+	  /* We want to ensure that the Ada main function's name
+	     appears verbatim in the index.  However, this name will
+	     be of the form "_ada_mumble", and will be rewritten by
+	     ada_decode.  So, recognize it specially here and add it
+	     to the index by hand.  */
+	  if (entry->tag == DW_TAG_subprogram
+	      && strcmp (main_for_ada, name) == 0)
+	    {
+	      /* Leave it alone.  */
+	    }
+	  else
+	    {
+	      /* In order for the index to work when read back into
+		 gdb, it has to use the encoded name, with any
+		 suffixes stripped.  */
+	      std::string encoded = ada_encode (name, false);
+	      name = obstack_strdup (&symtab->m_string_obstack,
+				     encoded.c_str ());
+	    }
+	}
...

The code contains some special handling related to the Ada main function, so
let's look at that one: foo.  Before commit 67e83a0deef we have:
...
$ grep foo.*function READELF
[3733] foo: 7 [global, function]
...
and after:
...
$ grep foo.*function READELF
[2738] _ada_foo: 7 [global, function]
[3733] foo: 7 [global, function]
...
so that looks identical to the callee.increment case.

At commit 5fea9794325, we have slightly different index numbers:
...
$ grep foo.*function READELF
[1658] foo: 7 [global, function]
[2738] _ada_foo: 7 [global, function]
...
but otherwise the same result.

If we disable the special handling of the Ada main function like so:
...
-	  if (entry->tag == DW_TAG_subprogram
+	  if (false && entry->tag == DW_TAG_subprogram
...
we still have the exact same result because:
...
(gdb) p main_for_ada
$1 = 0x352e6a0 "_ada_foo"
...
and ada_encode ("_ada_foo", false) == "_ada_foo".

The comment seems to be copied from debug_names::insert, which does indeed use
ada_decode, while the code in write_cooked_index uses ada_encode instead.

Remove the superfluous special handling of Ada main in write_cooked_index.

Tested on x86_64-linux, with target boards unix and cc-with-gdb-index.
---
 gdb/dwarf2/index-write.c | 27 ++++++---------------------
 1 file changed, 6 insertions(+), 21 deletions(-)

diff --git a/gdb/dwarf2/index-write.c b/gdb/dwarf2/index-write.c
index 62c2cc6ac7d..66c5378d677 100644
--- a/gdb/dwarf2/index-write.c
+++ b/gdb/dwarf2/index-write.c
@@ -1128,8 +1128,6 @@ write_cooked_index (cooked_index *table,
 		    const cu_index_map &cu_index_htab,
 		    struct mapped_symtab *symtab)
 {
-  const char *main_for_ada = main_name ();
-
   for (const cooked_index_entry *entry : table->all_entries ())
     {
       const auto it = cu_index_htab.find (entry->per_cu);
@@ -1139,25 +1137,12 @@ write_cooked_index (cooked_index *table,
 
       if (entry->per_cu->lang () == language_ada)
 	{
-	  /* We want to ensure that the Ada main function's name
-	     appears verbatim in the index.  However, this name will
-	     be of the form "_ada_mumble", and will be rewritten by
-	     ada_decode.  So, recognize it specially here and add it
-	     to the index by hand.  */
-	  if (entry->tag == DW_TAG_subprogram
-	      && strcmp (main_for_ada, name) == 0)
-	    {
-	      /* Leave it alone.  */
-	    }
-	  else
-	    {
-	      /* In order for the index to work when read back into
-		 gdb, it has to use the encoded name, with any
-		 suffixes stripped.  */
-	      std::string encoded = ada_encode (name, false);
-	      name = obstack_strdup (&symtab->m_string_obstack,
-				     encoded.c_str ());
-	    }
+	  /* In order for the index to work when read back into
+	     gdb, it has to use the encoded name, with any
+	     suffixes stripped.  */
+	  std::string encoded = ada_encode (name, false);
+	  name = obstack_strdup (&symtab->m_string_obstack,
+				 encoded.c_str ());
 	}
       else if (entry->per_cu->lang () == language_cplus
 	       && (entry->flags & IS_LINKAGE) != 0)

base-commit: 1720b64f735ff2798ab50ea9e2a40ab42af6cc6e
-- 
2.35.3


             reply	other threads:[~2023-08-03 13:34 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-03 13:34 Tom de Vries [this message]
2023-08-03 15:11 ` 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=20230803133414.22219-1-tdevries@suse.de \
    --to=tdevries@suse.de \
    --cc=gdb-patches@sourceware.org \
    --cc=tom@tromey.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).