Re: [RFA] fix Ada SYMBOL_PRINT_NAME problem

[Joel Brobecker <brobecker@adacore.com>]

[-- Attachment #1: Type: text/plain, Size: 1625 bytes --]

> > Trying to print "i" will result in the following menu:
> > 
> >     (gdb) print i
> >     Multiple matches for i
> >     [0] cancel
> >     [1] pck__first(int) at pck.adb:3
> >     [2] pck__second(int) at pck.adb:7
> >     >
> > 
> > The expected output (particularly for choices 1 and 2) is:
> > 
> >     (gdb) p i
> >     Multiple matches for i
> >     [0] cancel
> >     [1] pck.first.i at pck.adb:3
> >     [2] pck.second.i at pck.adb:7
[...]
> The clever demangling lookup table was clever for C++, but less clever
> for other languages; you'll see that Java already has some
> special-case code, and the comments there (e.g. about the msymbol
> having the wrong demangling) also apply to Ada.
> 
> I've no objection to this patch in principle, but you're doing it in
> the wrong place.  If you already know the symbol is an Ada symbol, why
> go through all the rigamarole of putting it in the hash table?

Duh! I feel ashamed I didn't think of that, and even more so that
it took me a long while to understand what you meant. Thanks for
the suggestion. I propose the following patch:

2008-01-30  Joel Brobecker  <brobecker@adacore.com>
                        
        * symtab.c (symbol_set_names): Do not add an entry in the demangling
        hash table for Ada symbols. Just store the linkage name as is,
        and leave the demangled_name as NULL.

Testcase unchanged:

2008-01-08  Joel Brobecker  <brobecker@adacore.com>

        * gdb.ada/sym_print_name: New test program.
        * gdb.ada/sym_print_name.exp: New testcase.

Does it look better? All tested on x86-linux, no regression.

Thanks,
-- 
Joel

[-- Attachment #2: demangle.diff --]
[-- Type: text/plain, Size: 1240 bytes --]

Index: symtab.c
===================================================================
--- symtab.c	(revision 135)
+++ symtab.c	(revision 136)
@@ -541,6 +541,24 @@ symbol_set_names (struct general_symbol_
   if (objfile->demangled_names_hash == NULL)
     create_demangled_names_hash (objfile);
 
+  if (gsymbol->language == language_ada)
+    {
+      /* In Ada, we do the symbol lookups using the mangled name, so
+         we can save some space by not storing the demangled name.
+
+         As a side note, we have also observed some overlap between
+         the C++ mangling and Ada mangling, similarly to what has
+         been observed with Java.  Because we don't store the demangled
+         name with the symbol, we don't need to use the same trick
+         as Java.  */
+      gsymbol->name = obstack_alloc (&objfile->objfile_obstack, len + 1);
+      memcpy (gsymbol->name, linkage_name, len);
+      gsymbol->name[len] = '\0';
+      gsymbol->language_specific.cplus_specific.demangled_name = NULL;
+
+      return;
+    }
+
   /* The stabs reader generally provides names that are not
      NUL-terminated; most of the other readers don't do this, so we
      can just use the given copy, unless we're in the Java case.  */

[-- Attachment #3: demangle-tc.diff --]
[-- Type: text/plain, Size: 5797 bytes --]

Index: gdb.ada/sym_print_name.exp
===================================================================
--- gdb.ada/sym_print_name.exp	(revision 0)
+++ gdb.ada/sym_print_name.exp	(revision 96)
@@ -0,0 +1,73 @@
+# Copyright 2008 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+if $tracelevel then {
+    strace $tracelevel
+}
+
+load_lib "ada.exp"
+
+set testdir "sym_print_name"
+set testfile "${testdir}/foo"
+set srcfile ${srcdir}/${subdir}/${testfile}.adb
+set binfile ${objdir}/${subdir}/${testfile}
+
+file mkdir ${objdir}/${subdir}/${testdir}
+if {[gdb_compile_ada "${srcfile}" "${binfile}" executable [list debug]] != "" } {
+  return -1
+}
+
+# A convenience function that joins all the arguments together,
+# with a regexp that matches zero-or-more end of lines in between
+# each argument.  This function is ideal to write the expected output
+# of a GDB command that generates more than a couple of lines, as
+# this allows us to write each line as a separate string, which is
+# easier to read by a human being.
+
+proc multi_line { args } {
+    return [join $args "\[\r\n\]*"]
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+set bp_location [gdb_get_line_number "STOP" ${testdir}/foo.adb]
+runto "foo.adb:$bp_location"
+
+set menu [multi_line "Multiple matches for i" \
+                     "\\\[0\\\] cancel" \
+                     "\\\[1\\\] pck\\.first\\.i.*" \
+                     "\\\[2\\\] pck\\.second\\.i.*" \
+                     "> $" ]
+
+set test_name "multiple matches for symbol i"
+gdb_test_multiple "print i" "$test_name" \
+{
+      -re "$menu" {
+          pass "$test_name"
+      }
+
+      default {
+          fail "$test_name"
+      }
+}
+
+# Select the first choice from the multiple-choice menu above.
+gdb_test "1" \
+         "48" \
+         "select first choice from multiple-choice menu"
+
Index: gdb.ada/sym_print_name/pck.adb
===================================================================
--- gdb.ada/sym_print_name/pck.adb	(revision 0)
+++ gdb.ada/sym_print_name/pck.adb	(revision 96)
@@ -0,0 +1,21 @@
+--  Copyright 2008 Free Software Foundation, Inc.
+--
+--  This program is free software; you can redistribute it and/or modify
+--  it under the terms of the GNU General Public License as published by
+--  the Free Software Foundation; either version 3 of the License, or
+--  (at your option) any later version.
+--
+--  This program is distributed in the hope that it will be useful,
+--  but WITHOUT ANY WARRANTY; without even the implied warranty of
+--  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+--  GNU General Public License for more details.
+--
+--  You should have received a copy of the GNU General Public License
+--  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+package body Pck is
+   procedure Do_Nothing (Val : in out Integer) is
+   begin
+      null;
+   end Do_Nothing;
+end Pck;
Index: gdb.ada/sym_print_name/pck.ads
===================================================================
--- gdb.ada/sym_print_name/pck.ads	(revision 0)
+++ gdb.ada/sym_print_name/pck.ads	(revision 96)
@@ -0,0 +1,26 @@
+--  Copyright 2008 Free Software Foundation, Inc.
+--
+--  This program is free software; you can redistribute it and/or modify
+--  it under the terms of the GNU General Public License as published by
+--  the Free Software Foundation; either version 3 of the License, or
+--  (at your option) any later version.
+--
+--  This program is distributed in the hope that it will be useful,
+--  but WITHOUT ANY WARRANTY; without even the implied warranty of
+--  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+--  GNU General Public License for more details.
+--
+--  You should have received a copy of the GNU General Public License
+--  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+package Pck is
+   package First is
+      I : Integer := 48;
+   end First;
+
+   package Second is
+      I : Integer := 74;
+   end Second;
+
+   procedure Do_Nothing (Val : in out Integer);
+end Pck;
Index: gdb.ada/sym_print_name/foo.adb
===================================================================
--- gdb.ada/sym_print_name/foo.adb	(revision 0)
+++ gdb.ada/sym_print_name/foo.adb	(revision 96)
@@ -0,0 +1,22 @@
+--  Copyright 2008 Free Software Foundation, Inc.
+--
+--  This program is free software; you can redistribute it and/or modify
+--  it under the terms of the GNU General Public License as published by
+--  the Free Software Foundation; either version 3 of the License, or
+--  (at your option) any later version.
+--
+--  This program is distributed in the hope that it will be useful,
+--  but WITHOUT ANY WARRANTY; without even the implied warranty of
+--  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+--  GNU General Public License for more details.
+--
+--  You should have received a copy of the GNU General Public License
+--  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+with Pck; use Pck;
+
+procedure Foo is
+begin
+   Do_Nothing (First.I);  -- STOP
+   Do_Nothing (Second.I);
+end Foo;