From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5650 invoked by alias); 18 Feb 2014 21:52:36 -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 5638 invoked by uid 89); 18 Feb 2014 21:52:35 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-4.3 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 18 Feb 2014 21:52:34 +0000 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s1ILqVHN027832 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 18 Feb 2014 16:52:31 -0500 Received: from [10.36.116.65] (ovpn-116-65.ams2.redhat.com [10.36.116.65]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id s1ILqUgd022819; Tue, 18 Feb 2014 16:52:31 -0500 Subject: Re: [RFA/DWARF] constant class of DW_AT_high_pc is offset for version >=4 only. From: Mark Wielaard To: Joel Brobecker Cc: gdb-patches@sourceware.org In-Reply-To: <20140218184906.GB15835@adacore.com> References: <1392478818-30320-1-git-send-email-brobecker@adacore.com> <20140218133000.GA15835@adacore.com> <1392739369.21975.145.camel@bordewijk.wildebeest.org> <20140218184906.GB15835@adacore.com> Content-Type: text/plain; charset="UTF-8" Date: Tue, 18 Feb 2014 21:52:00 -0000 Message-ID: <1392760350.21975.200.camel@bordewijk.wildebeest.org> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-SW-Source: 2014-02/txt/msg00577.txt.bz2 On Tue, 2014-02-18 at 19:49 +0100, Joel Brobecker wrote: > First of all, thanks for the comments! Just trying to make up for breaking your setup with my original patch. Although I might be too pedantic in my DWARF spec reading and I cannot actually approve the patch. So it might be of little help. Sorry about that. > On Tue, Feb 18, 2014 at 05:02:49PM +0100, Mark Wielaard wrote: > > On Tue, 2014-02-18 at 14:30 +0100, Joel Brobecker wrote: > > > 1. The introduction of attr_value_as_address, to be used > > > in place of DW_ADDR when dealing with address attributes. > > > I left a few uses of this macro in the situations where > > > we actually know that the form is an address form. > > > > This accepts any form as address/unsigned. I would at least check that > > it is either DW_FORM_data4 or DW_FORM_data8 (even better would be to > > check the CU address width too, although that would require to pass > > around cu too, which might not be practical). > > I don't mind adding that, but the real question is what would we do > if we found an unexpected size? We could emit a complaint, but > I wouldn't necessarily stop processing the unit's debugging info. > I think the guideline here is that we should try to do our best > within reasonable constraints. I think adding a complaint, here, > is of marginal value as presumably the address read in the odd > format might be correct. On the other hand, any dump of the debugging > info should quickly reveal the format used for those addresses. I think emitting a complaint is the right thing to do. IMHO this really is broken DWARF and accepting random DW_FORM_foo here might hide real issues (which as you showed might accidentally seem to work just because the union values overlap and produce something that is slightly but not completely wrong). > > Also I would add a comment > > that this is really to work around buggy producers. > > I will definitely add a comment saying something about some compiler > producing odd debugging info. It seems logical to me that address > attributes would naturally be encoded using an address form, but is > a constant form clearly forbidden by the (older) standard(s)? > I would therefore just label the format chosen as unusual. :-). Sadly DWARF doesn't seem to forbid anything. If it doesn't seem to make sense then a producer and consumer just have to come to an agreement about the meaning somehow. But even DWARF2 says that the only possible encoding of attribute values of class address is DW_FORM_addr. > > Note that this might break for DWARF5. See http://dwarfstd.org/ShowIssue.php?issue=120719.1 > > Interesting. I am curious why you would handle this attribute as > an offset even when the value is encoded in address form? As Doug said, it is a space saving (offsets are often small) and it saves a relocation (linkers have to resolve all DW_FORM_addr values and they add up). A more general form of this saving is the DW_FORM_GNU_addr_index extension which is also proposed as a DWARF5 update: http://dwarfstd.org/ShowIssue.php?issue=130313.2 > But regardless, I think my change doesn't break the current behavior; > and to support the DWARF5 standard, the behavior implemented by > the function is still correct (reading the data from the correct > union field). You will need to add some code right after the call, > regardless, which adds the base address if version >= 5. Yes, agreed. > > In general I would only use attr_value_as_address for attributes (low_pc > > and high_pc) which you know a buggy producer might encode with > > DW_FORM_data[48]. > > I think this suggestion goes against the spirit of trying to do our > best. Not using the function means reading the attribute value from > the wrong field of the attribute union, which means increasing our > chances of getting it wrong. On the other hand, using the function > might allow us to read the correct address and get things to actually > work. Be liberal in what you accept. Yeah. OK. I admit I am mostly worried because GDB is seen as the gold standard of DWARF consumers. When GDB accepts some DWARF then basically all other DWARF consumers have to adapt. And in this case I had some trouble recently with producers and consumers not agreeing on the meaning of DW_FORM_data[1248] (which I still have to report to the DWARF committee), so I am a little hyper-sensitive to accepting even more stuff encoded as DW_FORM_data... sorry about that. I do think your patch is basically fine. I am just pedantic about interpreting the DWARF standard. Because I do worry this will make things harder in the future. > Also, I don't have acces to said compiler, so I can't try to study > its output and list the attributes using this constant form. But > if the low/hi pc attributes use a constant form, why not the > entry_pc? OK, that makes things even harder. But could you ask whether they encode addresses always as DW_FORM_data[48] and whether an update will produce DW_FORM_addr? It would be good to know if this is just historical and will not be an issue in the future. Thanks, Mark