[RFA/dwarf] save nested Ada subprograms as global symbol

[Joel Brobecker <brobecker@adacore.com>]

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

Hello,

This is part of what I hope will be a string of patches that should
bring the GDB close to the AdaCore version of GDB in terms of Ada support.
My hope is to find the time this year to reach that level, which
would then make it easier for us to keep Ada support between our tree
and the FSF tree synchronized.

The Ada language provides support for nested subprograms. Consider
the following simple program:

        procedure Hello is
        
           procedure First is
           begin
              null;
           end First;
        
           procedure Second is
           begin
              First;
           end Second;
        
           procedure Third is
           begin
              Second;
           end Third;
        
        begin
           Third;
        end Hello;

This is a trivial program, where the main procedure (Hello) contains
three nested procedures (Third, Second, and First). To compile it,
do the following:

        % gnatmake -g hello

The problem is trying to break on First:

        % gdb hello
        (gdb) break first
        Function "first" not defined.
        Make breakpoint pending on future shared library load? (y or [n]) n

What we'd like to be able to do is:

        (gdb) b first
        Breakpoint 1 at 0x804954a: file hello.adb, line 6.
        (gdb) run
        Starting program: /home/no-backup/brobecke/ada-fsf/nested/hello 
        
        Breakpoint 1, hello.first () at hello.adb:6
        6          end First;

To achieve this, we modified dwarf2read to store all Ada subprograms
in the global scope, even the ones that are not "external". Another
approach that was considered was to modify the Ada lookup routines
to extend the search to non-global/static scopes, but I'm concerned
about performance.

In practice, even though these routines are indeed local to our
procedure in the Ada program, we want to be flexible with the user
in the debugger, and treat these procedures as global, so that the
user can break inside them without specifying the scope or having
to be in the scope where the function is defined.

This is what the attached patch does:

2007-12-27  Joel Brobecker  <brobecker@adacore.com>

        * dwarf2read.c (add_partial_symbol): Always store all Ada subprograms
        in the global scope.
        (new_symbol): Likewise.

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

I am also attaching a testcase that fails without this patch:

2007-12-27  Joel Brobecker  <brobecker@adacore.com>

        * gdb.ada/nested/hello.adb: New file.
        * gdb.ada/nested.exp: New testcase.
        * gdb.ada/Makefile.in (EXECUTABLES): Update list.

Tested on x86-linux as well.

Thanks,
-- 
Joel

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

Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.243
diff -u -p -r1.243 dwarf2read.c
--- dwarf2read.c	22 Dec 2007 20:58:30 -0000	1.243
+++ dwarf2read.c	27 Dec 2007 05:46:35 -0000
@@ -1898,8 +1898,12 @@ add_partial_symbol (struct partial_die_i
   switch (pdi->tag)
     {
     case DW_TAG_subprogram:
-      if (pdi->is_external)
+      if (pdi->is_external || cu->language == language_ada)
 	{
+          /* brobecker/2007-12-26: Normally, only "external" DIEs are part
+             of the global scope.  But in Ada, we want to be able to access
+             nested procedures globally.  So all Ada subprograms are stored
+             in the global scope.  */
 	  /*prim_record_minimal_symbol (actual_name, pdi->lowpc + baseaddr,
 	     mst_text, objfile); */
 	  psym = add_psymbol_to_list (actual_name, strlen (actual_name),
@@ -7288,8 +7292,15 @@ new_symbol (struct die_info *die, struct
 	     finish_block.  */
 	  SYMBOL_CLASS (sym) = LOC_BLOCK;
 	  attr2 = dwarf2_attr (die, DW_AT_external, cu);
-	  if (attr2 && (DW_UNSND (attr2) != 0))
+	  if ((attr2 && (DW_UNSND (attr2) != 0))
+              || cu->language == language_ada)
 	    {
+              /* Subprograms marked external are stored as a global symbol.
+                 Ada subprograms, whether marked external or not, are always
+                 stored as a global symbol, because we want to be able to
+                 access them globally.  For instance, we want to be able
+                 to break on a nested subprogram without having to
+                 specify the context.  */
 	      add_symbol_to_list (sym, &global_symbols);
 	    }
 	  else

[-- Attachment #3: hello.adb --]
[-- Type: text/plain, Size: 937 bytes --]

--  Copyright 2007 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/>.

procedure Hello is

   procedure First is
   begin
      null;
   end First;

   procedure Second is
   begin
      First;
   end Second;

   procedure Third is
   begin
      Second;
   end Third;

begin
   Third;
end Hello;


[-- Attachment #4: nested.exp --]
[-- Type: text/plain, Size: 1355 bytes --]

# Copyright 2007 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 "nested"
set testfile "${testdir}/hello"
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
}

gdb_exit
gdb_start
gdb_reinitialize_dir $srcdir/$subdir
gdb_load ${binfile}

set any_nb "\[0-9\]+"
set any_addr "0x\[0-9a-zA-Z\]+"

# Try breaking on a nested function.

gdb_test "break first" \
         "Breakpoint $any_nb at $any_addr: file .*hello.adb, line $any_nb." \
         "break on nested function First"


[-- Attachment #5: gdb.ada-makefile.in.diff --]
[-- Type: text/plain, Size: 512 bytes --]

Index: gdb.ada/Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.ada/Makefile.in,v
retrieving revision 1.3
diff -u -p -r1.3 Makefile.in
--- gdb.ada/Makefile.in	26 Dec 2007 14:21:53 -0000	1.3
+++ gdb.ada/Makefile.in	27 Dec 2007 07:37:36 -0000
@@ -11,6 +11,7 @@ EXECUTABLES = \
   exec_changed/second \
   fixed_points/fixed_points \
   frame_args/foo \
+  nested/hello \
   null_record/null_record \
   packed_array/pa \
   print_chars/foo \