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

[Joel Brobecker <brobecker@adacore.com>]

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