Re: [PATCH] Handle DW_AT_encoding on DW_TAG_enumeration_type

[Andrew Burgess <aburgess@redhat.com>]

Tom Tromey <tromey@adacore.com> writes:

>>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:
>
>>> First, this is yet another instance of a general problem with DWARF.
>>> See https://sourceware.org/bugzilla/show_bug.cgi?id=32680 for the
>>> analysis.
>
> Andrew> Should there be a Bug: .... tag for this bug?
>
> Yeah.  At first I wasn't sure but see below.
>
> Andrew> I tried running your new test against GDB 17 and can confirm that it
> Andrew> fails.  But I also tried against GDB 16, and the test passes, and the
> Andrew> output looks fine.
>
> Andrew> You commit message seemed (to me) to indicate that you didn't think this
> Andrew> should have ever worked, but it looks like something changed 16 to 17
> Andrew> that broke this.  It would be nice (maybe?) to understand what changed.
>
> I looked into this and I introduced this difference in
>
>     commit 5363deffcfb1c1825db3a8446d377a0ad56eff70
>     Author: Tom Tromey <tromey@adacore.com>
>     Date:   Thu Mar 20 09:52:08 2025 -0600
>
>         Use correct sign extension for enumeration types
>
> ... without a test, which is disappointing.
>
> Anyway, though, I still think that patch is probably correct, at least
> as far as *this* test case is concerned.  Consider one of the
> enumeration literals, dumped by eu-readelf to show the form:
>
>  [  13b6]        enumerator           abbrev: 1
>                  name                 (strp) "value_129"
>                  const_value          (data1) 129
>
> "data1" is inherently ambiguous, from DWARF:
>
>     The data in DW_FORM_data1, DW_FORM_data2, DW_FORM_data4,
>     DW_FORM_data8 and DW_FORM_data16 can be anything. Depending on
>     context, it may be a signed integer, an unsigned integer, a
>     floating-point constant, or anything else. A consumer must use
>     context to know how to interpret the bits, which if they are target
>     machine data (such as an integer or floating-point constant) will be
>     in target machine byte order.
>
> ... which is the aforementioned bug.
>
> Now, we could change update_enumeration_type_from_children to also
> examine the attribute forms, with the idea that if we see DW_FORM_sdata
> then it is definitely signed.  While that wouldn't help this particular
> test, it's possible to construct one where it does.  In fact I happened
> recently to be investigating this for gnat-llvm, where there's a type:
>
>  [  10cf]      enumeration_type     abbrev: 21
>                name                 (strp) "tag_t"
>                byte_size            (data1) 1
>                decl_file            (data1) p.adb (1)
>                decl_line            (data1) 20
>                alignment            (udata) 1
> ...
>  [  10de]        enumerator           abbrev: 22
>                  name                 (strp) "object"
>                  const_value          (sdata) 18446744073709551615 (-1)
>
> Here the enumeration type doesn't specify a sign, but there's a constant
> that is clearly signed.  FWIW this occurs in array_of_variant.exp.
>
> I'll address this in a separate patch.

Thanks for digging into this.  I figured it was probably going to be
something like this, but it's nice to know (at least for me).  I agree
that this patch looks good to go in.

Thanks,
Andrew