From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7415 invoked by alias); 11 Sep 2008 17:55:04 -0000 Received: (qmail 7393 invoked by uid 22791); 11 Sep 2008 17:55:02 -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; Thu, 11 Sep 2008 17:54:27 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 356BC2A96DA for ; Thu, 11 Sep 2008 13:54:25 -0400 (EDT) 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 AYjBUOQyIb0B for ; Thu, 11 Sep 2008 13:54:25 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id A24442A96D9 for ; Thu, 11 Sep 2008 13:54:24 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id 86FE7E7ACD; Thu, 11 Sep 2008 19:54:22 +0200 (CEST) Date: Thu, 11 Sep 2008 17:55:00 -0000 From: Joel Brobecker To: gdb-patches@sourceware.org Subject: Re: [RFA/commit/dwarf] Create partial symbols for nested subprograms Message-ID: <20080911175422.GS12222@adacore.com> References: <20080910201959.GC10133@adacore.com> <20080910203437.GA26162@caradoc.them.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="dgjlcl3Tl+kb3YDk" Content-Disposition: inline In-Reply-To: <20080910203437.GA26162@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-09/txt/msg00237.txt.bz2 --dgjlcl3Tl+kb3YDk Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-length: 3233 Hi Daniel, > > 2008-09-10 Joel Brobecker > > > > * 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 * 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 --dgjlcl3Tl+kb3YDk Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="nested.diff" Content-length: 3994 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 --dgjlcl3Tl+kb3YDk--