Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Andrew Burgess <aburgess@redhat.com>
To: Tom Tromey <tromey@adacore.com>, gdb-patches@sourceware.org
Cc: Tom Tromey <tromey@adacore.com>
Subject: Re: [PATCH] Handle DW_AT_encoding on DW_TAG_enumeration_type
Date: Fri, 15 May 2026 20:06:09 +0100	[thread overview]
Message-ID: <87se7scx26.fsf@redhat.com> (raw)
In-Reply-To: <20260513142722.3135998-1-tromey@adacore.com>

Tom Tromey <tromey@adacore.com> writes:

> A user pointed out a problem when printing certain values from an Ada
> enumeration type.  Investigation showed that the problem was that some
> enumeration constants were emitted using DW_FORM_data1, and were
> incorrectly sign-extended by gdb.
>
> 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.

Should there be a Bug: .... tag for this bug?

>
> Meanwhile, it turns out that GCC implements an extension to handle
> this scenario.  In particular, in non-strict mode, it will emit
> DW_AT_encoding using either DW_ATE_signed or DW_ATE_unsigned.  This
> was done back in 2017 by Pierre-Marie, in support of Ada -- but then
> somehow nothing was ever implemented on the gdb side.  For this see
> GCC commit f76f096e ("DWARF: add DW_AT_encoding attributes for
> DW_TAG_enumeration_type DIEs").
>
> This patch adds the missing code to gdb.  The included test case shows
> the bug that was originally reported.  I've also included the snippet
> from Pierre-Marie's commit message for good measure.

I tried running your new test against GDB 17 and can confirm that it
fails.  But I also tried against GDB 16, and the test passes, and the
output looks fine.

You commit message seemed (to me) to indicate that you didn't think this
should have ever worked, but it looks like something changed 16 to 17
that broke this.  It would be nice (maybe?) to understand what changed.

Anyway, the change itself looks great.  But I'd really like to know why
GDB 16 worked.

Approved-By: Andrew Burgess <aburgess@redhat.com>

Thanks,
Andrew



> ---
>  gdb/dwarf2/read.c                        |  16 ++
>  gdb/testsuite/gdb.ada/enum-sign.exp      |  40 +++
>  gdb/testsuite/gdb.ada/enum-sign/prog.adb | 331 +++++++++++++++++++++++
>  3 files changed, 387 insertions(+)
>  create mode 100644 gdb/testsuite/gdb.ada/enum-sign.exp
>  create mode 100644 gdb/testsuite/gdb.ada/enum-sign/prog.adb
>
> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
> index efe96f6510d..de5690202b2 100644
> --- a/gdb/dwarf2/read.c
> +++ b/gdb/dwarf2/read.c
> @@ -11171,6 +11171,22 @@ read_enumeration_type (struct die_info *die, struct dwarf2_cu *cu)
>  	set_type_align (type, TYPE_RAW_ALIGN (underlying_type));
>      }
>  
> +  /* DW_AT_encoding on an enumeration type is a GCC extension.  GCC
> +     will only emit DW_ATE_signed or DW_ATE_unsigned here.  If this is
> +     seen, it provides a way to find the signed-ness without having to
> +     guess based on the constants, which is somewhat fraught anyway
> +     (see PR symtab/32680).  */
> +  if (!is_unsigned.has_value ())
> +    {
> +      attribute *encoding_attr = dwarf2_attr (die, DW_AT_encoding, cu);
> +      if (encoding_attr != nullptr)
> +	{
> +	  std::optional<ULONGEST> val = encoding_attr->unsigned_constant ();
> +	  if (val.has_value ())
> +	    is_unsigned = *val == DW_ATE_unsigned;
> +	}
> +    }
> +
>    type->set_is_declared_class (dwarf2_flag_true_p (die, DW_AT_enum_class, cu));
>  
>    type->set_endianity_is_not_default (die_byte_order (die, cu, nullptr));
> diff --git a/gdb/testsuite/gdb.ada/enum-sign.exp b/gdb/testsuite/gdb.ada/enum-sign.exp
> new file mode 100644
> index 00000000000..84098027849
> --- /dev/null
> +++ b/gdb/testsuite/gdb.ada/enum-sign.exp
> @@ -0,0 +1,40 @@
> +# Copyright 2026 Free Software Foundation, Inc.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +load_lib "ada.exp"
> +
> +require allow_ada_tests
> +
> +standard_ada_testfile prog
> +
> +if {[gdb_compile_ada "${srcfile}" "${binfile}" executable debug] != ""} {
> +    return
> +}
> +
> +clean_restart ${testfile}
> +
> +set bp_location [gdb_get_line_number "STOP" ${testdir}/prog.adb]
> +runto "prog.adb:$bp_location"
> +
> +for {set num 0} {$num <= 300} {incr num} {
> +    gdb_test "print Value_$num" " = value_$num"
> +    gdb_test "print Unsigned_Enumeration'Val($num)" " = value_$num"
> +    gdb_test "print/d Unsigned_Enumeration'Val($num)" " = $num"
> +}
> +
> +foreach {name dec} {se_a -1 se_b 0 se_c 1 se_d 2} {
> +    gdb_test "print $name" " = $name"
> +    gdb_test "print/d $name" " = $dec"
> +}
> diff --git a/gdb/testsuite/gdb.ada/enum-sign/prog.adb b/gdb/testsuite/gdb.ada/enum-sign/prog.adb
> new file mode 100644
> index 00000000000..6ef3c956917
> --- /dev/null
> +++ b/gdb/testsuite/gdb.ada/enum-sign/prog.adb
> @@ -0,0 +1,331 @@
> +--  Copyright 2026 Free Software Foundation, Inc.
> +--
> +--  This program is free software; you can redistribute it and/or modify
> +--  it under the terms of the GNU General Public License as published by
> +--  the Free Software Foundation; either version 3 of the License, or
> +--  (at your option) any later version.
> +--
> +--  This program is distributed in the hope that it will be useful,
> +--  but WITHOUT ANY WARRANTY; without even the implied warranty of
> +--  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +--  GNU General Public License for more details.
> +--
> +--  You should have received a copy of the GNU General Public License
> +--  along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +procedure Prog is
> +   type Unsigned_Enumeration is
> +     (
> +      Value_0,
> +      Value_1,
> +      Value_2,
> +      Value_3,
> +      Value_4,
> +      Value_5,
> +      Value_6,
> +      Value_7,
> +      Value_8,
> +      Value_9,
> +      Value_10,
> +      Value_11,
> +      Value_12,
> +      Value_13,
> +      Value_14,
> +      Value_15,
> +      Value_16,
> +      Value_17,
> +      Value_18,
> +      Value_19,
> +      Value_20,
> +      Value_21,
> +      Value_22,
> +      Value_23,
> +      Value_24,
> +      Value_25,
> +      Value_26,
> +      Value_27,
> +      Value_28,
> +      Value_29,
> +      Value_30,
> +      Value_31,
> +      Value_32,
> +      Value_33,
> +      Value_34,
> +      Value_35,
> +      Value_36,
> +      Value_37,
> +      Value_38,
> +      Value_39,
> +      Value_40,
> +      Value_41,
> +      Value_42,
> +      Value_43,
> +      Value_44,
> +      Value_45,
> +      Value_46,
> +      Value_47,
> +      Value_48,
> +      Value_49,
> +      Value_50,
> +      Value_51,
> +      Value_52,
> +      Value_53,
> +      Value_54,
> +      Value_55,
> +      Value_56,
> +      Value_57,
> +      Value_58,
> +      Value_59,
> +      Value_60,
> +      Value_61,
> +      Value_62,
> +      Value_63,
> +      Value_64,
> +      Value_65,
> +      Value_66,
> +      Value_67,
> +      Value_68,
> +      Value_69,
> +      Value_70,
> +      Value_71,
> +      Value_72,
> +      Value_73,
> +      Value_74,
> +      Value_75,
> +      Value_76,
> +      Value_77,
> +      Value_78,
> +      Value_79,
> +      Value_80,
> +      Value_81,
> +      Value_82,
> +      Value_83,
> +      Value_84,
> +      Value_85,
> +      Value_86,
> +      Value_87,
> +      Value_88,
> +      Value_89,
> +      Value_90,
> +      Value_91,
> +      Value_92,
> +      Value_93,
> +      Value_94,
> +      Value_95,
> +      Value_96,
> +      Value_97,
> +      Value_98,
> +      Value_99,
> +      Value_100,
> +      Value_101,
> +      Value_102,
> +      Value_103,
> +      Value_104,
> +      Value_105,
> +      Value_106,
> +      Value_107,
> +      Value_108,
> +      Value_109,
> +      Value_110,
> +      Value_111,
> +      Value_112,
> +      Value_113,
> +      Value_114,
> +      Value_115,
> +      Value_116,
> +      Value_117,
> +      Value_118,
> +      Value_119,
> +      Value_120,
> +      Value_121,
> +      Value_122,
> +      Value_123,
> +      Value_124,
> +      Value_125,
> +      Value_126,
> +      Value_127,
> +      Value_128,
> +      Value_129,
> +      Value_130,
> +      Value_131,
> +      Value_132,
> +      Value_133,
> +      Value_134,
> +      Value_135,
> +      Value_136,
> +      Value_137,
> +      Value_138,
> +      Value_139,
> +      Value_140,
> +      Value_141,
> +      Value_142,
> +      Value_143,
> +      Value_144,
> +      Value_145,
> +      Value_146,
> +      Value_147,
> +      Value_148,
> +      Value_149,
> +      Value_150,
> +      Value_151,
> +      Value_152,
> +      Value_153,
> +      Value_154,
> +      Value_155,
> +      Value_156,
> +      Value_157,
> +      Value_158,
> +      Value_159,
> +      Value_160,
> +      Value_161,
> +      Value_162,
> +      Value_163,
> +      Value_164,
> +      Value_165,
> +      Value_166,
> +      Value_167,
> +      Value_168,
> +      Value_169,
> +      Value_170,
> +      Value_171,
> +      Value_172,
> +      Value_173,
> +      Value_174,
> +      Value_175,
> +      Value_176,
> +      Value_177,
> +      Value_178,
> +      Value_179,
> +      Value_180,
> +      Value_181,
> +      Value_182,
> +      Value_183,
> +      Value_184,
> +      Value_185,
> +      Value_186,
> +      Value_187,
> +      Value_188,
> +      Value_189,
> +      Value_190,
> +      Value_191,
> +      Value_192,
> +      Value_193,
> +      Value_194,
> +      Value_195,
> +      Value_196,
> +      Value_197,
> +      Value_198,
> +      Value_199,
> +      Value_200,
> +      Value_201,
> +      Value_202,
> +      Value_203,
> +      Value_204,
> +      Value_205,
> +      Value_206,
> +      Value_207,
> +      Value_208,
> +      Value_209,
> +      Value_210,
> +      Value_211,
> +      Value_212,
> +      Value_213,
> +      Value_214,
> +      Value_215,
> +      Value_216,
> +      Value_217,
> +      Value_218,
> +      Value_219,
> +      Value_220,
> +      Value_221,
> +      Value_222,
> +      Value_223,
> +      Value_224,
> +      Value_225,
> +      Value_226,
> +      Value_227,
> +      Value_228,
> +      Value_229,
> +      Value_230,
> +      Value_231,
> +      Value_232,
> +      Value_233,
> +      Value_234,
> +      Value_235,
> +      Value_236,
> +      Value_237,
> +      Value_238,
> +      Value_239,
> +      Value_240,
> +      Value_241,
> +      Value_242,
> +      Value_243,
> +      Value_244,
> +      Value_245,
> +      Value_246,
> +      Value_247,
> +      Value_248,
> +      Value_249,
> +      Value_250,
> +      Value_251,
> +      Value_252,
> +      Value_253,
> +      Value_254,
> +      Value_255,
> +      Value_256,
> +      Value_257,
> +      Value_258,
> +      Value_259,
> +      Value_260,
> +      Value_261,
> +      Value_262,
> +      Value_263,
> +      Value_264,
> +      Value_265,
> +      Value_266,
> +      Value_267,
> +      Value_268,
> +      Value_269,
> +      Value_270,
> +      Value_271,
> +      Value_272,
> +      Value_273,
> +      Value_274,
> +      Value_275,
> +      Value_276,
> +      Value_277,
> +      Value_278,
> +      Value_279,
> +      Value_280,
> +      Value_281,
> +      Value_282,
> +      Value_283,
> +      Value_284,
> +      Value_285,
> +      Value_286,
> +      Value_287,
> +      Value_288,
> +      Value_289,
> +      Value_290,
> +      Value_291,
> +      Value_292,
> +      Value_293,
> +      Value_294,
> +      Value_295,
> +      Value_296,
> +      Value_297,
> +      Value_298,
> +      Value_299,
> +      Value_300
> +     );
> +
> +   X : Unsigned_Enumeration := Value_23;
> +
> +   type Signed_Enumeration is ( SE_A, SE_B, SE_C, SE_D);
> +   for Signed_Enumeration use (-1, 0, 1, 2);
> +
> +   Y : Signed_Enumeration := SE_D;
> +
> +begin
> +   null;                        --  STOP
> +end;
>
> base-commit: 2d7f2dbbd4adadf7c388fa0d8b9ce95d9dfde641
> -- 
> 2.54.0


  reply	other threads:[~2026-05-15 19:06 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-13 14:27 Tom Tromey
2026-05-15 19:06 ` Andrew Burgess [this message]
2026-05-15 20:33   ` Tom Tromey
2026-05-16 11:13     ` Andrew Burgess
2026-05-18 15:29     ` Tom Tromey

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87se7scx26.fsf@redhat.com \
    --to=aburgess@redhat.com \
    --cc=gdb-patches@sourceware.org \
    --cc=tromey@adacore.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox