[RFA/commit/dwarf] Create partial symbols for nested subprograms

[RFA/commit/dwarf] Create partial symbols for nested subprograms

[Joel Brobecker]

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

Hello,

I just realized that we are missing the nested subprograms in our
partial symbol list.  Normally, this should cause us to fail when
trying to break on a nested procedure until we have loaded the full
symbols for that unit.  But there is in fact a bit of code that
I couldn't understand in ada_lookup_symbol_list which works around
this issue: It searches the minimal symbols for a match, derives
the associated symbol address, then determines the associated psymtab,
converts the psymtab into a symtab; Contrary to the partial symtab,
the full symtab does contain a symbol for our nested procedure, and
thus the lookup succeeds.

I stumbled on this because I am about to remove the piece of code
mentioned above (in ada_lookup_symbol_list), as it introduces
a pretty significant performance penalty - I will post a separate
patch with more details later on.  But at the same time, I think
it is important by itself, as I want the partial symtab and full
symtab to be consistent.

2008-09-10  Joel Brobecker  <brobecker@adacore.com>

        * dwarf2read.c (add_partial_subprogram): New procedure.
        (scan_partial_symbols): Use it.
        (load_partial_dies): Read in children of subprogram and lexical
        blocks.

Tested on x86-linux. No regression. I would like to commit in a week
if there are no objections.

Thanks,
-- 
Joel

[-- Attachment #2: 03-nested_psym.diff --]
[-- Type: text/plain, Size: 2747 bytes --]

diff -r d71d6b13aae1 -r ad68e435fc43 gdb/dwarf2read.c
--- a/gdb/dwarf2read.c	Wed Sep 10 12:23:56 2008 -0700
+++ b/gdb/dwarf2read.c	Wed Sep 10 12:24:48 2008 -0700
@@ -766,6 +766,10 @@
 
 static void add_partial_enumeration (struct partial_die_info *enum_pdi,
 				     struct dwarf2_cu *cu);
+
+static void add_partial_subprogram (struct partial_die_info *pdi,
+				    CORE_ADDR *lowpc, CORE_ADDR *highpc,
+				    struct dwarf2_cu *cu);
 
 static gdb_byte *locate_pdi_sibling (struct partial_die_info *orig_pdi,
                                      gdb_byte *info_ptr,
@@ -1783,21 +1787,7 @@
 	  switch (pdi->tag)
 	    {
 	    case DW_TAG_subprogram:
-	      if (pdi->has_pc_info)
-		{
-		  if (pdi->lowpc < *lowpc)
-		    {
-		      *lowpc = pdi->lowpc;
-		    }
-		  if (pdi->highpc > *highpc)
-		    {
-		      *highpc = pdi->highpc;
-		    }
-		  if (!pdi->is_declaration)
-		    {
-		      add_partial_symbol (pdi, cu);
-		    }
-		}
+	      add_partial_subprogram (pdi, lowpc, highpc, cu);
 	      break;
 	    case DW_TAG_variable:
 	    case DW_TAG_typedef:
@@ -2145,6 +2135,45 @@
 
   if (pdi->has_children)
     scan_partial_symbols (pdi->die_child, lowpc, highpc, cu);
+}
+
+/* Read a partial die corresponding to a subprogram and create a partial
+   symbol for that subprogram.  Also define a partial symbol for each
+   nested subprogram defined inside it.
+   
+   DIE my also be a lexical block, in which case we simply search
+   recursively for suprograms defined inside that lexical block.  */
+
+static void
+add_partial_subprogram (struct partial_die_info *pdi,
+			CORE_ADDR *lowpc, CORE_ADDR *highpc,
+			struct dwarf2_cu *cu)
+{
+  if (pdi->tag == DW_TAG_subprogram)
+    {
+      if (pdi->has_pc_info)
+        {
+          if (pdi->lowpc < *lowpc)
+            *lowpc = pdi->lowpc;
+          if (pdi->highpc > *highpc)
+            *highpc = pdi->highpc;
+          if (!pdi->is_declaration)
+            add_partial_symbol (pdi, cu);
+        }
+    }
+  
+  if (! pdi->has_children)
+    return;
+
+  pdi = pdi->die_child;
+  while (pdi != NULL)
+    {
+      fixup_partial_die (pdi, cu);
+      if (pdi->tag == DW_TAG_subprogram
+          || pdi->tag == DW_TAG_lexical_block)
+        add_partial_subprogram (pdi, lowpc, highpc, cu);
+      pdi = pdi->die_sibling;
+    }
 }
 
 /* See if we can figure out if the class lives in a namespace.  We do
@@ -5696,6 +5725,8 @@
 	  && (load_all
 	      || last_die->tag == DW_TAG_namespace
 	      || last_die->tag == DW_TAG_enumeration_type
+	      || last_die->tag == DW_TAG_subprogram
+	      || last_die->tag == DW_TAG_lexical_block
 	      || (cu->language != language_c
 		  && (last_die->tag == DW_TAG_class_type
 		      || last_die->tag == DW_TAG_interface_type

Re: [RFA/commit/dwarf] Create partial symbols for nested subprograms

[Daniel Jacobowitz]

On Wed, Sep 10, 2008 at 01:19:59PM -0700, Joel Brobecker wrote:
> I stumbled on this because I am about to remove the piece of code
> mentioned above (in ada_lookup_symbol_list), as it introduces
> a pretty significant performance penalty - I will post a separate
> patch with more details later on.  But at the same time, I think
> it is important by itself, as I want the partial symtab and full
> symtab to be consistent.
> 
> 2008-09-10  Joel Brobecker  <brobecker@adacore.com>
> 
>         * dwarf2read.c (add_partial_subprogram): New procedure.
>         (scan_partial_symbols): Use it.
>         (load_partial_dies): Read in children of subprogram and lexical
>         blocks.

:-(

Do you have any performance data for GDB startup with this change?
Not that I see any useful way around it, if subprograms really can be
children of lexical blocks.  But it's going to cause us to read in a
ton more DIEs, including all scopes and local variables.

The lexical block check won't work as is; we'd walk children of
lexical blocks, except we'll never read one in the first place.
See the skip_one_die call earlier in load_partial_dies.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA/commit/dwarf] Create partial symbols for nested subprograms

[Tom Tromey]

>>>>> "Daniel" == Daniel Jacobowitz <drow@false.org> writes:

Daniel> Not that I see any useful way around it, if subprograms really
Daniel> can be children of lexical blocks.

Can we do it only for languages where we know this can occur?
IOW, Ada?

Tom

Re: [RFA/commit/dwarf] Create partial symbols for nested subprograms

[Joel Brobecker]

> Daniel> Not that I see any useful way around it, if subprograms really
> Daniel> can be children of lexical blocks.
> 
> Can we do it only for languages where we know this can occur?
> IOW, Ada?

That's what I was going to propose, but I wanted to do some measurement
too (tomorrow). Nested subprograms are possible in C, and aren't they
also available in C++? I don't mind supporting this only in Ada, though.

-- 
Joel

Re: [RFA/commit/dwarf] Create partial symbols for nested subprograms

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Daniel> Not that I see any useful way around it, if subprograms really
Daniel> can be children of lexical blocks.

Tom> Can we do it only for languages where we know this can occur?
Tom> IOW, Ada?

Joel> That's what I was going to propose, but I wanted to do some measurement
Joel> too (tomorrow). Nested subprograms are possible in C, and aren't they
Joel> also available in C++? I don't mind supporting this only in Ada, though.

GNU C has nested functions.  Standard C does not.  I wrote a simple
test case and compiled it with GCC.  The resulting DWARF (I appended a
snippet) does not nest functions here .. but perhaps this is a bug in
GCC, I couldn't say.

I don't know the situation with C++.

Tom


 <0><b>: Abbrev Number: 1 (DW_TAG_compile_unit)
  < c>     DW_AT_stmt_list   : 0	
  <10>     DW_AT_high_pc     : 0x17	
  <14>     DW_AT_low_pc      : 0	
  <18>     DW_AT_producer    : GNU C 4.1.2 20070925 (Red Hat 4.1.2-33)	
  <40>     DW_AT_language    : 1	(ANSI C)
  <41>     DW_AT_name        : q.c	
  <45>     DW_AT_comp_dir    : /tmp	
 <1><4a>: Abbrev Number: 2 (DW_TAG_subprogram)
  <4b>     DW_AT_sibling     : <84>	
  <4f>     DW_AT_external    : 1	
  <50>     DW_AT_name        : outer	
  <56>     DW_AT_decl_file   : 1	
  <57>     DW_AT_decl_line   : 2	
  <58>     DW_AT_prototyped  : 1	
  <59>     DW_AT_type        : <84>	
  <5d>     DW_AT_low_pc      : 0	
  <61>     DW_AT_high_pc     : 0xd	
  <65>     DW_AT_frame_base  : 0	(location list)
 <2><69>: Abbrev Number: 3 (DW_TAG_subprogram)
  <6a>     DW_AT_name        : inner	
  <70>     DW_AT_decl_file   : 1	
  <71>     DW_AT_decl_line   : 3	
  <72>     DW_AT_prototyped  : 1	
  <73>     DW_AT_type        : <84>	
  <77>     DW_AT_low_pc      : 0xd	
  <7b>     DW_AT_high_pc     : 0x17	
  <7f>     DW_AT_frame_base  : 0x2c	(location list)

Re: [RFA/commit/dwarf] Create partial symbols for nested subprograms

[Daniel Jacobowitz]

On Thu, Sep 11, 2008 at 04:13:24PM -0600, Tom Tromey wrote:
> GNU C has nested functions.  Standard C does not.  I wrote a simple
> test case and compiled it with GCC.  The resulting DWARF (I appended a
> snippet) does not nest functions here .. but perhaps this is a bug in
> GCC, I couldn't say.

Doesn't it?

>  <1><4a>: Abbrev Number: 2 (DW_TAG_subprogram)
>  <2><69>: Abbrev Number: 3 (DW_TAG_subprogram)

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA/commit/dwarf] Create partial symbols for nested subprograms

[Tom Tromey]

>>>>> "Daniel" == Daniel Jacobowitz <drow@false.org> writes:

Daniel> Doesn't it?

>> <1><4a>: Abbrev Number: 2 (DW_TAG_subprogram)
>> <2><69>: Abbrev Number: 3 (DW_TAG_subprogram)

Yeah, duh.  Whoops.

Tom

Re: [RFA/commit/dwarf] Create partial symbols for nested subprograms

[Joel Brobecker]

> > GNU C has nested functions.  Standard C does not.  I wrote a simple
> > test case and compiled it with GCC.  The resulting DWARF (I appended a
> > snippet) does not nest functions here .. but perhaps this is a bug in
> > GCC, I couldn't say.
> 
> Doesn't it?
> 
> >  <1><4a>: Abbrev Number: 2 (DW_TAG_subprogram)
> >  <2><69>: Abbrev Number: 3 (DW_TAG_subprogram)

Yes, I confirm it does. The readelf output can be confusing the first
time. That nesting level on the left is hard to notice...

-- 
Joel

Re: [RFA/commit/dwarf] Create partial symbols for nested subprograms

[Joel Brobecker]

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

Hi Daniel,

> > 2008-09-10  Joel Brobecker  <brobecker@adacore.com>
> > 
> >         * dwarf2read.c (add_partial_subprogram): New procedure.
> >         (scan_partial_symbols): Use it.
> >         (load_partial_dies): Read in children of subprogram and lexical
> >         blocks.
> 
> :-(

Sorry! :-(.

> Not that I see any useful way around it, if subprograms really can be
> children of lexical blocks.  But it's going to cause us to read in a
> ton more DIEs, including all scopes and local variables.

Unfortunately, it is possible to define a subprogram inside a lexical
block in Ada, so we have to search lexical blocks as well...
For instance:

  procedure Foo is
  begin
     declare
        procedure Bar is [...]
     begin
        Bar;
     end;
  end Foo;

> The lexical block check won't work as is; we'd walk children of
> lexical blocks, except we'll never read one in the first place.
> See the skip_one_die call earlier in load_partial_dies.

Arg, you are absolutely right. That's another part of the change
that we forgot to contribute (we now have setup strict rules where
all our changes should be contributed very soon after we make them,
precisely to avoid this sort of mistake). While I was at it, I also
noticed that we have a comment in our code that we also forgot to
contribute and which I find informative. Added as well. Let me know
if you don't think it bring anything.

> Do you have any performance data for GDB startup with this change?

I am not sure I am doing the right kind of performance test, so if you
have some better suggestions, please let me know. For now, here is what
I did:

  . I used one of the large Ada applications we have:
    It is 290MB in size - reduced to 160MB when I strip the debugging
    info => the debugging info size is roughly 130MB.
    The number of real lines of code is 700K spread in ~3500 files,
    and it has a total of 39,000 subprograms declared in the debugging
    info.

  . To measure the startup time, I used the --statistics command-line
    switch.

Here are the times reported before I apply the patch:

        Startup time: 12.076754
        Startup time: 12.096756
        Startup time: 12.200762
        Startup time: 11.904744
        (average = 12.069754)

        Startup size: data size 132330172

Here are the same times reported after:

        Startup time: 12.600787
        Startup time: 12.640790
        Startup time: 12.432777
        Startup time: 12.628789
        (average = 12.575786)
        
        Startup size: data size 133001788 (+700KB).

It looks like roughly a 4% increase in startup time. Not sure whether
that's considered a large increase or not - I just think that it's not
noticeable. None of our users have reported issues with startup time.

But, to make sure that only languages that have this feature get hit,
I adjusted the patch to only search nested procedure when the language
allows them.

2008-09-11  Joel Brobecker  <brobecker@adacore.com>

        * dwarf2read.c (add_partial_subprogram): New procedure.
        (scan_partial_symbols): Use it.
        (load_partial_dies): Read in children of subprogram and lexical
        blocks.

Tested on x86-linux. No regression.
OK to commit?

Thanks,
-- 
Joel

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

diff -r d71d6b13aae1 -r a7f96d0b3354 gdb/dwarf2read.c
--- a/gdb/dwarf2read.c	Wed Sep 10 12:23:56 2008 -0700
+++ b/gdb/dwarf2read.c	Thu Sep 11 10:32:58 2008 -0700
@@ -766,6 +766,10 @@
 
 static void add_partial_enumeration (struct partial_die_info *enum_pdi,
 				     struct dwarf2_cu *cu);
+
+static void add_partial_subprogram (struct partial_die_info *pdi,
+				    CORE_ADDR *lowpc, CORE_ADDR *highpc,
+				    struct dwarf2_cu *cu);
 
 static gdb_byte *locate_pdi_sibling (struct partial_die_info *orig_pdi,
                                      gdb_byte *info_ptr,
@@ -1783,21 +1787,7 @@
 	  switch (pdi->tag)
 	    {
 	    case DW_TAG_subprogram:
-	      if (pdi->has_pc_info)
-		{
-		  if (pdi->lowpc < *lowpc)
-		    {
-		      *lowpc = pdi->lowpc;
-		    }
-		  if (pdi->highpc > *highpc)
-		    {
-		      *highpc = pdi->highpc;
-		    }
-		  if (!pdi->is_declaration)
-		    {
-		      add_partial_symbol (pdi, cu);
-		    }
-		}
+	      add_partial_subprogram (pdi, lowpc, highpc, cu);
 	      break;
 	    case DW_TAG_variable:
 	    case DW_TAG_typedef:
@@ -2145,6 +2135,51 @@
 
   if (pdi->has_children)
     scan_partial_symbols (pdi->die_child, lowpc, highpc, cu);
+}
+
+/* Read a partial die corresponding to a subprogram and create a partial
+   symbol for that subprogram.  When the CU language allows it, this
+   routine also defines a partial symbol for each nested subprogram
+   that this subprogram contains.
+   
+   DIE my also be a lexical block, in which case we simply search
+   recursively for suprograms defined inside that lexical block.
+   Again, this is only performed when the CU language allows this
+   type of definitions.  */
+
+static void
+add_partial_subprogram (struct partial_die_info *pdi,
+			CORE_ADDR *lowpc, CORE_ADDR *highpc,
+			struct dwarf2_cu *cu)
+{
+  if (pdi->tag == DW_TAG_subprogram)
+    {
+      if (pdi->has_pc_info)
+        {
+          if (pdi->lowpc < *lowpc)
+            *lowpc = pdi->lowpc;
+          if (pdi->highpc > *highpc)
+            *highpc = pdi->highpc;
+          if (!pdi->is_declaration)
+            add_partial_symbol (pdi, cu);
+        }
+    }
+  
+  if (! pdi->has_children)
+    return;
+
+  if (cu->language == language_ada)
+    {
+      pdi = pdi->die_child;
+      while (pdi != NULL)
+	{
+	  fixup_partial_die (pdi, cu);
+	  if (pdi->tag == DW_TAG_subprogram
+	      || pdi->tag == DW_TAG_lexical_block)
+	    add_partial_subprogram (pdi, lowpc, highpc, cu);
+	  pdi = pdi->die_sibling;
+	}
+    }
 }
 
 /* See if we can figure out if the class lives in a namespace.  We do
@@ -5567,6 +5602,7 @@
 	  && !is_type_tag_for_partial (abbrev->tag)
 	  && abbrev->tag != DW_TAG_enumerator
 	  && abbrev->tag != DW_TAG_subprogram
+	  && abbrev->tag != DW_TAG_lexical_block
 	  && abbrev->tag != DW_TAG_variable
 	  && abbrev->tag != DW_TAG_namespace
 	  && abbrev->tag != DW_TAG_member)
@@ -5689,13 +5725,19 @@
 				sizeof (struct partial_die_info));
 
       /* For some DIEs we want to follow their children (if any).  For C
-         we have no reason to follow the children of structures; for other
+	 we have no reason to follow the children of structures; for other
 	 languages we have to, both so that we can get at method physnames
-	 to infer fully qualified class names, and for DW_AT_specification.  */
+	 to infer fully qualified class names, and for DW_AT_specification.
+	 We need to scan the children of procedures and lexical blocks
+	 because certain languages such as Ada allow the definition of
+	 nested entities that could be interesting for the debugger, such
+	 as nested procedures for instance.  */
       if (last_die->has_children
 	  && (load_all
 	      || last_die->tag == DW_TAG_namespace
 	      || last_die->tag == DW_TAG_enumeration_type
+	      || last_die->tag == DW_TAG_subprogram
+	      || last_die->tag == DW_TAG_lexical_block
 	      || (cu->language != language_c
 		  && (last_die->tag == DW_TAG_class_type
 		      || last_die->tag == DW_TAG_interface_type

Re: [RFA/commit/dwarf] Create partial symbols for nested subprograms

[Daniel Jacobowitz]

On Thu, Sep 11, 2008 at 10:54:22AM -0700, Joel Brobecker wrote:
> > Do you have any performance data for GDB startup with this change?
> 
> I am not sure I am doing the right kind of performance test, so if you
> have some better suggestions, please let me know. For now, here is what
> I did:
> 
>   . I used one of the large Ada applications we have:
>     It is 290MB in size - reduced to 160MB when I strip the debugging
>     info => the debugging info size is roughly 130MB.
>     The number of real lines of code is 700K spread in ~3500 files,
>     and it has a total of 39,000 subprograms declared in the debugging
>     info.
> 
>   . To measure the startup time, I used the --statistics command-line
>     switch.

I usually use 'gdb -batch foo.exe' to time reading partial symbols,
and 'gdb -readnow -batch foo.exe' to time full symbols (only partial
symbols are at issue here).

> It looks like roughly a 4% increase in startup time. Not sure whether
> that's considered a large increase or not - I just think that it's not
> noticeable. None of our users have reported issues with startup time.

Your users must be more patient than mine or Tom's :-)  I consider
startup time to be pretty important, and I've been working on bringing
it down... Tom's been working on an even more drastic version.

> But, to make sure that only languages that have this feature get hit,
> I adjusted the patch to only search nested procedure when the language
> allows them.

Does it work / help to do the check here too:

>  	  && (load_all
>  	      || last_die->tag == DW_TAG_namespace
>  	      || last_die->tag == DW_TAG_enumeration_type
> +	      || last_die->tag == DW_TAG_subprogram
> +	      || last_die->tag == DW_TAG_lexical_block
>  	      || (cu->language != language_c
>  		  && (last_die->tag == DW_TAG_class_type
>  		      || last_die->tag == DW_TAG_interface_type

That'll bring the memory usage back down for non-Ada.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA/commit/dwarf] Create partial symbols for nested subprograms

[Joel Brobecker]

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

> I usually use 'gdb -batch foo.exe' to time reading partial symbols,
> and 'gdb -readnow -batch foo.exe' to time full symbols (only partial
> symbols are at issue here).

And use the  "time" command to measure the time elapsed? Somehow,
I got reported user times and total times that fluctuated a little
more than when I used the --statistics.

> Your users must be more patient than mine or Tom's :-)  I consider
> startup time to be pretty important, and I've been working on bringing
> it down... Tom's been working on an even more drastic version.

They must be :). I wish I had time to try to make this time shorter,
but the truth is we some other areas where I think improving performance
is more critical. For instance, we could still do better with symbol
lookups, but we are limited by the fact that we do our lookups by
linkage name. We need to change that, but the initial reason for doing
it that way is that because we had customers whose program was so big
that we exceeded the maximum amount of memory we could allocate. I have
some ideas on how to fix this and hopefully bring Ada back closer to
what C++ does, but that'll have to wait for a moment when things are
less hectic for me.

> Does it work / help to do the check here too:
> 
> >  	  && (load_all
> >  	      || last_die->tag == DW_TAG_namespace
> >  	      || last_die->tag == DW_TAG_enumeration_type
> > +	      || last_die->tag == DW_TAG_subprogram
> > +	      || last_die->tag == DW_TAG_lexical_block
> >  	      || (cu->language != language_c
> >  		  && (last_die->tag == DW_TAG_class_type
> >  		      || last_die->tag == DW_TAG_interface_type

Sure! Is that what you had in mind?

2008-09-11  Joel Brobecker  <brobecker@adacore.com>

        * dwarf2read.c (add_partial_subprogram): New procedure.
        (scan_partial_symbols): Use it.
        (load_partial_dies): Read in children of subprogram and lexical
        blocks for Ada compilation units.

Tested on x86-linux, no regression.

-- 
Joel

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

diff -r d71d6b13aae1 -r 23f6a0ad1bee gdb/dwarf2read.c
--- a/gdb/dwarf2read.c	Wed Sep 10 12:23:56 2008 -0700
+++ b/gdb/dwarf2read.c	Thu Sep 11 14:41:11 2008 -0700
@@ -766,6 +766,10 @@ static void add_partial_namespace (struc
 
 static void add_partial_enumeration (struct partial_die_info *enum_pdi,
 				     struct dwarf2_cu *cu);
+
+static void add_partial_subprogram (struct partial_die_info *pdi,
+				    CORE_ADDR *lowpc, CORE_ADDR *highpc,
+				    struct dwarf2_cu *cu);
 
 static gdb_byte *locate_pdi_sibling (struct partial_die_info *orig_pdi,
                                      gdb_byte *info_ptr,
@@ -1783,21 +1787,7 @@ scan_partial_symbols (struct partial_die
 	  switch (pdi->tag)
 	    {
 	    case DW_TAG_subprogram:
-	      if (pdi->has_pc_info)
-		{
-		  if (pdi->lowpc < *lowpc)
-		    {
-		      *lowpc = pdi->lowpc;
-		    }
-		  if (pdi->highpc > *highpc)
-		    {
-		      *highpc = pdi->highpc;
-		    }
-		  if (!pdi->is_declaration)
-		    {
-		      add_partial_symbol (pdi, cu);
-		    }
-		}
+	      add_partial_subprogram (pdi, lowpc, highpc, cu);
 	      break;
 	    case DW_TAG_variable:
 	    case DW_TAG_typedef:
@@ -2145,6 +2135,51 @@ add_partial_namespace (struct partial_di
 
   if (pdi->has_children)
     scan_partial_symbols (pdi->die_child, lowpc, highpc, cu);
+}
+
+/* Read a partial die corresponding to a subprogram and create a partial
+   symbol for that subprogram.  When the CU language allows it, this
+   routine also defines a partial symbol for each nested subprogram
+   that this subprogram contains.
+   
+   DIE my also be a lexical block, in which case we simply search
+   recursively for suprograms defined inside that lexical block.
+   Again, this is only performed when the CU language allows this
+   type of definitions.  */
+
+static void
+add_partial_subprogram (struct partial_die_info *pdi,
+			CORE_ADDR *lowpc, CORE_ADDR *highpc,
+			struct dwarf2_cu *cu)
+{
+  if (pdi->tag == DW_TAG_subprogram)
+    {
+      if (pdi->has_pc_info)
+        {
+          if (pdi->lowpc < *lowpc)
+            *lowpc = pdi->lowpc;
+          if (pdi->highpc > *highpc)
+            *highpc = pdi->highpc;
+          if (!pdi->is_declaration)
+            add_partial_symbol (pdi, cu);
+        }
+    }
+  
+  if (! pdi->has_children)
+    return;
+
+  if (cu->language == language_ada)
+    {
+      pdi = pdi->die_child;
+      while (pdi != NULL)
+	{
+	  fixup_partial_die (pdi, cu);
+	  if (pdi->tag == DW_TAG_subprogram
+	      || pdi->tag == DW_TAG_lexical_block)
+	    add_partial_subprogram (pdi, lowpc, highpc, cu);
+	  pdi = pdi->die_sibling;
+	}
+    }
 }
 
 /* See if we can figure out if the class lives in a namespace.  We do
@@ -5567,6 +5602,7 @@ load_partial_dies (bfd *abfd, gdb_byte *
 	  && !is_type_tag_for_partial (abbrev->tag)
 	  && abbrev->tag != DW_TAG_enumerator
 	  && abbrev->tag != DW_TAG_subprogram
+	  && abbrev->tag != DW_TAG_lexical_block
 	  && abbrev->tag != DW_TAG_variable
 	  && abbrev->tag != DW_TAG_namespace
 	  && abbrev->tag != DW_TAG_member)
@@ -5689,9 +5725,14 @@ load_partial_dies (bfd *abfd, gdb_byte *
 				sizeof (struct partial_die_info));
 
       /* For some DIEs we want to follow their children (if any).  For C
-         we have no reason to follow the children of structures; for other
+	 we have no reason to follow the children of structures; for other
 	 languages we have to, both so that we can get at method physnames
-	 to infer fully qualified class names, and for DW_AT_specification.  */
+	 to infer fully qualified class names, and for DW_AT_specification.
+
+	 For Ada, we need to scan the children of subprograms and lexical
+	 blocks as well because Ada allows the definition of nested
+	 entities that could be interesting for the debugger, such as
+	 nested subprograms for instance.  */
       if (last_die->has_children
 	  && (load_all
 	      || last_die->tag == DW_TAG_namespace
@@ -5700,7 +5741,10 @@ load_partial_dies (bfd *abfd, gdb_byte *
 		  && (last_die->tag == DW_TAG_class_type
 		      || last_die->tag == DW_TAG_interface_type
 		      || last_die->tag == DW_TAG_structure_type
-		      || last_die->tag == DW_TAG_union_type))))
+		      || last_die->tag == DW_TAG_union_type))
+	      || (cu->language == language_ada
+		  && (last_die->tag == DW_TAG_subprogram
+		      || last_die->tag == DW_TAG_lexical_block))))
 	{
 	  nesting_level++;
 	  parent_die = last_die;

Re: [RFA/commit/dwarf] Create partial symbols for nested subprograms

[Daniel Jacobowitz]

On Thu, Sep 11, 2008 at 02:52:24PM -0700, Joel Brobecker wrote:
> > I usually use 'gdb -batch foo.exe' to time reading partial symbols,
> > and 'gdb -readnow -batch foo.exe' to time full symbols (only partial
> > symbols are at issue here).
> 
> And use the  "time" command to measure the time elapsed? Somehow,
> I got reported user times and total times that fluctuated a little
> more than when I used the --statistics.

Yes, I have a script to time things repeatedly and give me
average/stddev.

> Sure! Is that what you had in mind?
> 
> 2008-09-11  Joel Brobecker  <brobecker@adacore.com>
> 
>         * dwarf2read.c (add_partial_subprogram): New procedure.
>         (scan_partial_symbols): Use it.
>         (load_partial_dies): Read in children of subprogram and lexical
>         blocks for Ada compilation units.
> 
> Tested on x86-linux, no regression.

Yes, this is OK.  I agree it's the right compromise, at least for the
moment.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA/commit/dwarf] Create partial symbols for nested subprograms

[Joel Brobecker]

> > 2008-09-11  Joel Brobecker  <brobecker@adacore.com>
> > 
> >         * dwarf2read.c (add_partial_subprogram): New procedure.
> >         (scan_partial_symbols): Use it.
> >         (load_partial_dies): Read in children of subprogram and lexical
> >         blocks for Ada compilation units.
> > 
> > Tested on x86-linux, no regression.
> 
> Yes, this is OK.  I agree it's the right compromise, at least for the
> moment.

Thanks! Now checked in.

-- 
Joel

Re: [RFA/commit/dwarf] Create partial symbols for nested subprograms

[Tom Tromey]

>>>>> "Daniel" == Daniel Jacobowitz <drow@false.org> writes:

Joel> . To measure the startup time, I used the --statistics command-line
Joel> switch.

Daniel> I usually use 'gdb -batch foo.exe' to time reading partial
Daniel> symbols

FWIW -- me too.

Joel> It looks like roughly a 4% increase in startup time. Not sure whether
Joel> that's considered a large increase or not - I just think that it's not
Joel> noticeable. None of our users have reported issues with startup time.

How much memory do you have on your machine?  Are you using a 32- or
64-bit machine?  And how many objfiles are made with this test
program?

I am just curious to know what differs between my tests and yours.  In
my case, I start the system OpenOffice writer and then attach to it.
I have all the debuginfo packages installed.  I'm using x86, and on
this machine with that test, gdb takes 256569344 bytes (measured by
maint space 1) and it takes about 1:00 elapsed time to start up.  I
think there are 262 objfiles created, half of them for separate
debuginfo (this is from memory, I could be off by a little).

Daniel> Your users must be more patient than mine or Tom's :-)  I consider
Daniel> startup time to be pretty important, and I've been working on bringing
Daniel> it down... Tom's been working on an even more drastic version.

FWIW I don't think Joel's patch will negatively affect my current
approach.

Tom

Re: [RFA/commit/dwarf] Create partial symbols for nested subprograms

[Joel Brobecker]

> Joel> It looks like roughly a 4% increase in startup time. Not sure whether
> Joel> that's considered a large increase or not - I just think that it's not
> Joel> noticeable. None of our users have reported issues with startup time.
> 
> How much memory do you have on your machine?  Are you using a 32- or
> 64-bit machine?  And how many objfiles are made with this test
> program?

Phew, all these technical questions :). I'm using a 32bit machine
(Intel(R) Core(TM)2 CPU T7400 @ 2.16GHz) with 2GB of memory.
I'm running a 2.6.19.3 Linux kernel.

Because I cannot run the test program (all these large programs that
we got from our customers require a very precise environement to start),
I can only time "gdb EXE". As a result, I think we only end up creating
one object file for that executable.

> I am just curious to know what differs between my tests and yours.  In
> my case, I start the system OpenOffice writer and then attach to it.

Yes, that's a little different, because the shared libraries have been
mapped, and so debugging info for these are loaded in as well.

> FWIW I don't think Joel's patch will negatively affect my current
> approach.

Just curious, I think I either forgot a previous discussion or missed
it entirely. What approach are you going to take?

-- 
Joel

Re: [RFA/commit/dwarf] Create partial symbols for nested subprograms

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Tom> FWIW I don't think Joel's patch will negatively affect my current
Tom> approach.

Joel> Just curious, I think I either forgot a previous discussion or missed
Joel> it entirely.

It is on the archer list.

Joel> What approach are you going to take?

My patch defers reading partial symbol tables until they are needed.
This is done on a per-objfile basis, so some user requests can be
satisfied without reading all the psymbols for the inferior.

FWIW, initial results are pretty good.  Attach time drops from 1
minute to 20 seconds, and memory use after attach drops 35% (I
expected more -- I'm curious to know where all the memory goes).
Operations like "bt" still seem responsive.

There are still a couple FIXMEs to sort out.  And, I am not completely
sure that I've found all the spots where a call to
require_partial_symbols is needed.

Tom

Re: [RFA/commit/dwarf] Create partial symbols for nested subprograms

[Joel Brobecker]

> Joel> What approach are you going to take?
> 
> My patch defers reading partial symbol tables until they are needed.
> This is done on a per-objfile basis, so some user requests can be
> satisfied without reading all the psymbols for the inferior.

Oh, wow. How do you detect when partial symbols are needed or not?
(it's ok if you don't have time to answer my question now - I will
find out anyways when you do submit the patch here, so...)

-- 
Joel

Re: [RFA/commit/dwarf] Create partial symbols for nested subprograms

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Tom> My patch defers reading partial symbol tables until they are needed.
Tom> This is done on a per-objfile basis, so some user requests can be
Tom> satisfied without reading all the psymbols for the inferior.

Joel> Oh, wow. How do you detect when partial symbols are needed or not?

To find the places needing a call to require_partial_symbols, first I
read a bunch of code, then I ran the test suite, and finally I did
some interactive tests while debugging the modified gdb.

Essentially, the drawback of this approach is that, before writing
code that uses psymbols, you must decide whether they should be loaded
or not.

In some cases, other sorts of changes are appropriate.  E.g., I
changed have_partial_symbols() to have two loops -- one that first
checks whether any partial symbols exist, and then a second to try to
demand-load partial symbols.

Joel> (it's ok if you don't have time to answer my question now - I will
Joel> find out anyways when you do submit the patch here, so...)

If you want to see it sooner, I posted it to the archer list.
Also it is in the archer git repository.

Tom

Re: [RFA/commit/dwarf] Create partial symbols for nested subprograms

[Joel Brobecker]

> Essentially, the drawback of this approach is that, before writing
> code that uses psymbols, you must decide whether they should be loaded
> or not.

I quickly looked at the patch, and it doesn't seem as bad as it sounds.
I just joined the archer list, so we can discuss the details there
when you have found what the issue is with number of symtabs, but
the one thing that I am wondering is how users will respond to an
unexpectedly long delay when entering a command that is usually fast
and yet takes a noticeably long time that one time. For intance, imagine
the following scenario:
  
  % gdb my_300MB_exe
  (gdb) break main
  [12 secs later...]
  Breakpoint 1, ...

-- 
Joel

Re: [RFA/commit/dwarf] Create partial symbols for nested subprograms

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> but the one thing that I am wondering is how users will respond
Joel> to an unexpectedly long delay when entering a command that is
Joel> usually fast and yet takes a noticeably long time that one
Joel> time. For intance, imagine the following scenario:

Joel>   % gdb my_300MB_exe
Joel>   (gdb) break main
Joel>   [12 secs later...]
Joel>   Breakpoint 1, ...

Yeah, I wonder that too.

One thing to note is that, on average, we should expect the delay here
to be half of the startup delay that users experience today.  My
reasoning is that, on average, we should only have to load half the
psymtabs before finding a symbol.  (Of course, the worst case does
happen regularly, so this is not entirely satisfactory.)

I will do some more tests and fixing of this patch sometime soon.  If
this is a big problem perhaps I'll reopen the pubnames investigation.
Or maybe we can use the minimal symbols as a guide somehow, in some
common cases (or maybe we do already, I did not look at this in
depth).  (FWIW I'm also curious about lazily reading minimal symbols.)

Tom

Re: [RFA/commit/dwarf] Create partial symbols for nested subprograms

[Frank Ch. Eigler]

Joel Brobecker <brobecker@adacore.com> writes:

>>[...]
> the one thing that I am wondering is how users will respond to an
> unexpectedly long delay when entering a command that is usually fast
> and yet takes a noticeably long time that one time. [...]

Could gdb use something like alarm(2) to issue progress notifications?

- FChE

Re: [RFA/commit/dwarf] Create partial symbols for nested subprograms

[Tom Tromey]

Joel> the one thing that I am wondering is how users will respond to an
Joel> unexpectedly long delay when entering a command that is usually fast
Joel> and yet takes a noticeably long time that one time. [...]

Frank> Could gdb use something like alarm(2) to issue progress notifications?

One open question with this patch is what to do about notifications.
There are two things to consider.

One problem is that gdb prints "(no debugging symbols found)" when
reading partial symbols.  With lazy loading, this is always printed,
even though it is not correct.  In my patch I removed this code.  This
is not really satisfying, at least if we assume that people want and
pay attention to these messages (I certainly have on occasion).

The other problem is whether to print something when reading partial
symbols.  This is a problem because it might print any time,
potentially messing up other output.

I'm not sure how this affects MI.

Tom