From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 820 invoked by alias); 30 Jan 2008 21:39:07 -0000 Received: (qmail 809 invoked by uid 22791); 30 Jan 2008 21:39:06 -0000 X-Spam-Check-By: sourceware.org Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.31) with ESMTP; Wed, 30 Jan 2008 21:38:37 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id DA4B52A9803 for ; Wed, 30 Jan 2008 16:38:35 -0500 (EST) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id g7BXnUAz3pbX for ; Wed, 30 Jan 2008 16:38:35 -0500 (EST) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 4BE8F2A97FC for ; Wed, 30 Jan 2008 16:38:35 -0500 (EST) Received: by joel.gnat.com (Postfix, from userid 1000) id 1A3E0E7ACB; Wed, 30 Jan 2008 13:38:33 -0800 (PST) Date: Wed, 30 Jan 2008 21:55:00 -0000 From: Joel Brobecker To: gdb-patches@sourceware.org Subject: Re: [RFA] fix Ada SYMBOL_PRINT_NAME problem Message-ID: <20080130213833.GF12387@adacore.com> References: <20080108114358.GE24614@adacore.com> <20080129175458.GG3773@caradoc.them.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="azLHFNyN32YCQGCU" Content-Disposition: inline In-Reply-To: <20080129175458.GG3773@caradoc.them.org> User-Agent: Mutt/1.4.2.2i Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2008-01/txt/msg00832.txt.bz2 --azLHFNyN32YCQGCU Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-length: 1625 > > 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 * 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 * 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 --azLHFNyN32YCQGCU Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="demangle.diff" Content-length: 1240 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. */ --azLHFNyN32YCQGCU Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="demangle-tc.diff" Content-length: 5797 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 . + +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 . + +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 . + +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 . + +with Pck; use Pck; + +procedure Foo is +begin + Do_Nothing (First.I); -- STOP + Do_Nothing (Second.I); +end Foo; --azLHFNyN32YCQGCU--