From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6839 invoked by alias); 14 Apr 2004 17:47:34 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 6821 invoked from network); 14 Apr 2004 17:47:33 -0000 Received: from unknown (HELO nevyn.them.org) (66.93.172.17) by sources.redhat.com with SMTP; 14 Apr 2004 17:47:33 -0000 Received: from drow by nevyn.them.org with local (Exim 4.31 #1 (Debian)) id 1BDoTx-0000Dc-LI; Wed, 14 Apr 2004 13:47:29 -0400 Date: Wed, 14 Apr 2004 17:47:00 -0000 From: Daniel Jacobowitz To: Jim Blandy Cc: Joel Brobecker , Elena Zannoni , gdb-patches@sources.redhat.com Subject: Re: [RFA/dwarf-2] Fix for the null record problem Message-ID: <20040414174729.GA612@nevyn.them.org> Mail-Followup-To: Jim Blandy , Joel Brobecker , Elena Zannoni , gdb-patches@sources.redhat.com References: <20040219140145.GB804@gnat.com> <16437.11835.435941.553479@localhost.redhat.com> <20040401011813.GE888@gnat.com> <20040413052655.GB1173@gnat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.5.1+cvs20040105i X-SW-Source: 2004-04/txt/msg00288.txt.bz2 On Wed, Apr 14, 2004 at 12:21:44PM -0500, Jim Blandy wrote: > > Joel Brobecker writes: > > The patch is quite tiny, seems almost obvious, and Elena seemed happy > > with it. But it didn't get in because she asked that a testcase be added > > first (on Feb 19): > > << > > the patch looks sensible, but I would like to see the testcase go in > > at the same time, or we'll forget. > > >> > > > > The new testcase has been checked in now, so I was wondering if somebody > > had a moment to have a look at it, and confirm Elena's first review? > > I think it looks fine; I've just one question before you commit. > > As it stands, the code sets TYPE_FLAG_STUB if the type die has no > children, or if die_is_declaration (die, cu) is true. Your patch > correctly ditches the first criterion; no problems there. > > But it also modifies the second criterion as well, without comment. > In particular, die_is_declaration checks for both DW_AT_declaration > and DW_AT_specification, but your patch only tests for > DW_AT_declaration. I think this is correct: in section 5.6.1, the > Dwarf 2 spec says that the definition of the type has a > DW_AT_specification attribute pointing to the declaration. Since it's > the definition of the type that actually lists the fields, > DW_AT_specification should not cause GDB to mark the type as a stub. > Just the opposite: the referent of that attribute is the stub. > > The following C++ code produces Dwarf 2 info where the definition of > struct s::t has a DW_AT_specification attribute, but GDB doesn't skip > it, and I don't really understand why: Did you misread die_is_declaration? return (dwarf2_attr (die, DW_AT_declaration, cu) && ! dwarf2_attr (die, DW_AT_specification, cu)); I don't even know what that DW_AT_specification test is doing there - the idea of a declaration with a specification is pretty peculiar. But the important bit is that it's not going to report that something is a declartion, if it has a DW_AT_specification. If the check is important, then Joel should probably use !die_is_declaration. This was added by Jason Merrill in 2000, without much of an explanation; here it is: http://sources.redhat.com/ml/gdb-patches/2000-q1/msg00015.html It's not clear why the specification check is there; the important bit was presumably: > ! if (die->has_children) > ! if (die->has_children && ! die_is_declaration (die)) i.e. the point of the patch was to add the DW_AT_declaration check. -- Daniel Jacobowitz MontaVista Software Debian GNU/Linux Developer