[RFA] fix Ada SYMBOL_PRINT_NAME problem

[Joel Brobecker <brobecker@adacore.com>]

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

Hello,

The debugger sometimes prints weird names for certain Ada entities.
This shows up inside multiple-choice menus printed when an expression
is ambiguous. For instance, given the following two variables declared
with the same name but in a different (global) scope:

   package First is
      I : Integer := 1;
   end First;

   package Second is
      I : Integer := 2;
   end Second;

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
    > 

What happened in our case is that symtab.c:symbol_natural_name
checks for gsymbol->language_specific.cplus_specific.demangled_name,
and if not null, returns it. For Ada symbols, we expect this field
to be not null (as a side question, I wonder why we even check this
field when dealing with Ada symbols at all, but that's a separate
question - I will ask Paul).  The following comment that I added
with the patch explains why:

     Unfortunately, there is a case that we have to watch out for:
     During the symbol table processing, when creating the minimal
     symbols, we don't know the language associated to each entry.
     As a result, symbol_find_demangled_name tries various non-Ada
     demanglers, and sometimes, a demangler accepts the symbol and
     returns an unexpected demangled form of our symbol name.  This
     has consequences during the debugging info processing: This time,
     we do know the language, but since we have already found an entry
     in the demangled_name_hash using the linkage name, we use the
     demangled name stored in the hash instead of trying to recompute it.

So we had to add an explicit guard for Ada languages inside
symtab.c:symbol_set_names to make sure that Ada symbol do not
unexpectedly get the demangled_name field set.

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

        * symtab.c (symbol_set_names): Do not set the demangled_name
        for Ada symbols.

I also wrote a small testcase.

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

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

Tested on x86-linux, no regression.
OK to commit?

Thanks,
-- 
Joel

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

Index: symtab.c
===================================================================
--- symtab.c	(revision 94)
+++ symtab.c	(revision 97)
@@ -577,8 +577,27 @@ symbol_set_names (struct general_symbol_
 	(*slot)[lookup_len + 1] = '\0';
     }
 
+  /* Set the name and demangled_name.
+     
+     For Ada symbols, the demangled_name does not apply and should
+     be NULL (see how symbol_find_demangled_name always return NULL
+     for Ada symbols).
+
+     Unfortunately, there is a case that we have to watch out for:
+     During the symbol table processing, when creating the minimal
+     symbols, we don't know the language associated to each entry.
+     As a result, symbol_find_demangled_name tries various non-Ada
+     demanglers, and sometimes, a demangler accepts the symbol and
+     returns an unexpected demangled form of our symbol name.  This
+     has consequences during the debugging info processing: This time,
+     we do know the language, but since we have already found an entry
+     in the demangled_name_hash using the linkage name, we use the
+     demangled name stored in the hash instead of trying to recompute it.
+
+     To guard against this case, we add an explicit check against
+     Ada symbols before setting the symbol demangled_name.  */
   gsymbol->name = *slot + lookup_len - len;
-  if ((*slot)[lookup_len + 1] != '\0')
+  if (gsymbol->language != language_ada && (*slot)[lookup_len + 1] != '\0')
     gsymbol->language_specific.cplus_specific.demangled_name
       = &(*slot)[lookup_len + 1];
   else

[-- 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;