Re: [RFA/DWARF] Set enum type "flag_enum" and "unsigned" flags at type creation.

[Mark Wielaard <mjw at redhat dot com>]

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 <tromey@redhat.com>
Date:   Wed Jul 28 20:05:03 2010 +0000

        * dwarf2read.c (dwarf2_const_value_data): Never sign extend.