From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 54585 invoked by alias); 21 Dec 2017 11:03:37 -0000 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 Received: (qmail 54574 invoked by uid 89); 21 Dec 2017 11:03:37 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.0 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=inc.c, UD:inc.c, incc X-HELO: rock.gnat.com Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 21 Dec 2017 11:03:34 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id AC390116F15; Thu, 21 Dec 2017 06:03:32 -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 lJc27VIZaL6z; Thu, 21 Dec 2017 06:03:32 -0500 (EST) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 36433116E74; Thu, 21 Dec 2017 06:03:32 -0500 (EST) Received: by joel.gnat.com (Postfix, from userid 1000) id C4A0C809B4; Thu, 21 Dec 2017 15:03:27 +0400 (+04) Date: Thu, 21 Dec 2017 11:03:00 -0000 From: Joel Brobecker To: Simon Marchi Cc: Xavier Roirand , gdb-patches@sourceware.org Subject: Re: [RFA/DWARF v2] Fix breakpoint add on inlined function using function name. Message-ID: <20171221110327.zjsnnghq7qvnvyme@adacore.com> References: <1513776096-19313-1-git-send-email-roirand@adacore.com> <848029766ceee39c449712a53db88482@polymtl.ca> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <848029766ceee39c449712a53db88482@polymtl.ca> User-Agent: NeoMutt/20170113 (1.7.2) X-SW-Source: 2017-12/txt/msg00467.txt.bz2 Hi Simon, On Wed, Dec 20, 2017 at 09:35:34PM -0500, Simon Marchi wrote: > On 2017-12-20 08:21, Xavier Roirand wrote: > > So far only approved by Joel B. > > > > Using this Ada example: > > > > package B is > > procedure Read_Small with Inline_Always; > > end B; > > > > package body B is > > Total : Natural := 0; > > procedure Read_Small is > > begin > > Total := Total + 1; > > end Read_Small; > > end B; > > > > and > > > > with B; > > > > procedure M is > > begin > > B.Read_Small; > > end M; > > > > % gnatmake -g -O0 -m m.adb -cargs -gnatn > > % gdb m > > > > Inserting a breakpoint on Read_Small inlined function does not work: > > > > (gdb) b read_small > > Breakpoint 1 at 0x40250e: file b.adb, line 5. > > (gdb) info b > > Num Type Disp Enb Address What > > 1 breakpoint keep y 0x000000000040250e in b.doit at b.adb:5 > > (gdb) > > > > In this exemple we should have two breakpoints set, one in package B and > > "example" > > > the other one in the inlined instance inside procedure M), like below: > > > > (gdb) b read_small > > Breakpoint 1 at 0x40250e: b.adb:5. (2 locations) > > (gdb) info b > > Num Type Disp Enb Address What > > 1 breakpoint keep y > > 1.1 y 0x000000000040250e in b.doit at > > b.adb:5 > > 1.2 y 0x0000000000402540 in m at b.adb:5 > > (gdb) > > > > Looking at the DWARF info for inlined instance of Read_Small: > > > > <1><1526>: Abbrev Number: 2 (DW_TAG_subprogram) > > <1527> DW_AT_name : ([...], offset: 0x1e82): b__read_small > > <152b> DW_AT_decl_file : 2 > > <152c> DW_AT_decl_line : 3 > > <152d> DW_AT_inline : 3 (declared as inline and inlined) > > [...] > > <2><1547>: Abbrev Number: 4 (DW_TAG_inlined_subroutine) > > <1548> DW_AT_abstract_origin: <0x1526> > > <154c> DW_AT_low_pc : 0x402552 > > <1554> DW_AT_high_pc : 0x2b > > <155c> DW_AT_call_file : 1 > > <155d> DW_AT_call_line : 5 > > <2><155e>: Abbrev Number: 0 > > > > During the parsing of DWARF info in order to produce partial DIE linked > > list, the DW_TAG_inlined_subroutine were skipped thus not present in the > > final partial dies. > > Taking DW_TAG_inlined_subroutine in account during the parsing process > > fixes the problem. > > Do you have some insights as to why we don't see the same problem in other > languages such as C? I tried to make a test case in C similar to yours. I > made a shared library that contained a function, with both the standalone > instance and an inlined instance. In the beginning, we have only loaded > partial symbols for the library. I tried placing a breakpoint on the > function, and got the expected two locations. What seems to happen is that > placing the breakpoint triggers the read of the full symbols, which > eventually finds the inline instance. Do you have any idea what's different > about Ada? I think we'll need to look more deeply at your example to figure out what is going on. First, just to make sure we understand the lookup process: When trying to search for a symbol, we first search through all the partial symtabs to see if they contain partial symbols that match our search. From there, we expand those partial symtabs into full symtabs, and only then do we search the full symtabs for symbols that match, and that's the list we return. In our case, here is the reasoning: Imagine we have the following C pseudo-code: /* inc.h */ inline int increment (int i); /* inc.c */ int increment (int i) { return i + 1; } /* foo.c */ #include "inc.h" int main (void) { return increment (0); } For function increment, we would assume that the compiler would first generate a DW_TAG_subprogram DIE, and that this DIE would be a child of inc.c's compilation unit. Looking at the debugging information for foo.c's unit, we would see the DW_TAG_subprogram DIE for the function "main". And if the compiler were to elect to inline increment, we would see one of the children of main's DW_TAG_subprogram be a DW_TAG_inline_subroutine DIE. Now, let's take a look at what happens when the dwarf2read module processes the DWARF, and transforms a DW_TAG_subprogram DIE into a partial symbol: First, it processes the DIE's attributes. So far so good, but then you see that it processes the DIE's children only in the case of Ada. See dwarf2read.c::add_partial_subprogram: 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, set_addrmap, cu); pdi = pdi->die_sibling; } } The reason why we do it for Ada is that nested subprograms are common-place in Ada, and users expect to be able to be able to break on those subprograms without having to be inside the function itself to do so. In other words, we don't treat the symbol as being local to the enclosing subprogram. With that in mind, what happens in the C case while dwarf2read processes the main's DW_TAG_subprogram: It adds "main"'s partial symbol, but stops there instead of looking at its children. The net result is that the inline instance of our "increment" function never makes it to the partial symtabs/symbols. This impacts our partial symtab scan, as we now only have one partial symbol for increment in inc.c's partial symtab which matches. So, that's the only partial symtab selected for expansion. This then potentially affects the next step, which is the full symtab scan, and if "foo.c"'s partial symtab hasn't be expanded for some reasons unrelated to our lookup, we'll end up only finding one match, the non-inlined one. This is of course predicated on the assumption that the compiler generates the DW_TAG_inline_subroutine DIE as a child of the DW_TAG_subprogram DIE. Xavier tells me he tried it, and verified that this was the case. In your scenario, a number of things might be different, and might explain why it works. For instance, you might have expanded the partial symtab where your inlined instance lives as a side-effect of some earlier command, for instance. Or maybe the inlined instances are described differently with your compiler... > I don't see anything wrong with the change, but I'm far from an expert > in that area :) I used to know this fairly well, and the patch does seem relatively logical, but it's been a while, hence the caution and the delay before pushing ;-). -- Joel