From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11183 invoked by alias); 25 Feb 2014 14:32:27 -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 11170 invoked by uid 89); 25 Feb 2014 14:32:26 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-4.0 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, 25 Feb 2014 14:32:25 +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 s1PEWK3u031648 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 25 Feb 2014 09:32:20 -0500 Received: from [10.36.116.104] (ovpn-116-104.ams2.redhat.com [10.36.116.104]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id s1PEUfCI030608; Tue, 25 Feb 2014 09:30:42 -0500 Subject: Re: [RFA/DWARF] Set enum type "flag_enum" and "unsigned" flags at type creation. From: Mark Wielaard To: Joel Brobecker Cc: gdb-patches@sourceware.org In-Reply-To: <20140221092123.GA4720@adacore.com> References: <1390796357-3739-1-git-send-email-brobecker@adacore.com> <1392820455.21975.235.camel@bordewijk.wildebeest.org> <1392823115.21975.238.camel@bordewijk.wildebeest.org> <20140221092123.GA4720@adacore.com> Content-Type: text/plain; charset="UTF-8" Date: Tue, 25 Feb 2014 14:32:00 -0000 Message-ID: <1393338641.8933.35.camel@bordewijk.wildebeest.org> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-SW-Source: 2014-02/txt/msg00750.txt.bz2 On Fri, 2014-02-21 at 10:21 +0100, Joel Brobecker wrote: > Interestingly, I had thought at the time that the only way to express > signedness of the underlying value was by using a base type as a subtype > (IIRC) which, of course, if a lot of data for one small attribute. > The use of the form seems like a much more efficient way to achieve > that goal. But the above explains why I interpreted the current code > as a way to work around the fact that compilers might not be emitting > the required subtype for enums with negative values. Not sure how > correct or not that interpretation was... I think DWARF producers will normally use DW_FORM_sdata to indicate a signed (negative) value. In theory there can be confusion if the producer uses DW_FORM_data[1248] because you then don't know the "encoding" of the value. That is also why the DWARF spec discourages the use of those forms. And it explicitly doesn't define how to interpret the value of DW_FORM_data[1248]. Second guessing the producer if it did explicitly use DW_FORM_sdata or DW_FORM_udata seems wrong to me. If you do want to do that, then only do it for DW_FORM_data[1248]. But I think you will get it wrong because in practice when a producer uses those forms it expects you to zero-extend the value if it is encoded in less bits than the type size [*]. This is something that could use clarification or a hint from the DWARF spec, because I think the current wording isn't very helpful in practice. > I will verify your fix against my testcase as well, but I like the idea > of removing code that should normally not be there. I hope it works, because the current code looks a bit fragile to me. > But to answer your question above (do I also need my patch?), I think > the need might become less obvious once your patch is in. But I also > think it would probably be cleaner to have a complete type right from > the get-go, especially since I don't think the patch actually > complexifies the code (maybe even the opposite). That being said, > I'm not strongly attached to it, as long as GDB does TRT :-). I haven't looked too deeply into how GDB represents types. But the problem you are trying to solve here is trying to determine whether the type represented by a DW_TAG_enumeration_type is signed or unsigned. You try to determine that by iterating through all DW_TAG_enumerator children values and if you see one encoded as a negative value, you then assume the enumeration_type as a whole is signed, otherwise you assume unsigned. I am not really convinced that is the proper way to determine whether the enumeration_type is signed or not (the language might not care about that and the result might be random depending on DW_FORM used for the enumerator constants). I am intrigued by the idea of setting TYPE_FLAG_ENUM. The heuristic to determine that is very cute. It looks like it is only used for printing a value of an enum type. Does it work reliably? Are there languages that let you declare enumeration values that way? (If so, would it make sense to propose a DWARF extension attribute to mark them?) Or is this mainly for detecting people defining enums by hand as flag values? Cheers, Mark [*] This is something fixed in gdb some years ago: commit 053315c2134b7832b351c599fa3fa11abf6ca4e7 Author: Tom Tromey Date: Wed Jul 28 20:05:03 2010 +0000 * dwarf2read.c (dwarf2_const_value_data): Never sign extend.