Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH] gdb/dwarf2: don't sign-extend DW_FORM_data<n> enumerator constants
@ 2026-09-10  2:25 simon.marchi
  2026-09-11 14:58 ` Tom Tromey
  0 siblings, 1 reply; 3+ messages in thread
From: simon.marchi @ 2026-09-10  2:25 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

From: Simon Marchi <simon.marchi@polymtl.ca>

Commit 5363deffcfb1 ("Use correct sign extension for enumeration types")
made update_enumeration_type_from_children read enumerator constants
with attribute::signed_constant when the enumeration type is not
explicitly known to be unsigned (through the underlying type).  That
sign-extends values of forms such as DW_FORM_data1, which is wrong for
the constants that GCC actually emits.

Given this enumeration:

    enum class Test {
        A = 126,
        B = 127,
        C = 128,
        D = 129,
        G = 155,
        E = 255,
        F = 256
    };

... we have this DWARF (for the first 3 enumerators):

    0x00000684:   DW_TAG_enumeration_type
                    DW_AT_name [DW_FORM_strp]       ("Test")
                    DW_AT_enum_class [DW_FORM_flag_present] (true)
                    DW_AT_encoding [DW_FORM_data1]  (DW_ATE_signed)
                    DW_AT_byte_size [DW_FORM_data1] (0x04)
                    DW_AT_type [DW_FORM_ref4]       (0x0000006f "int")
                    DW_AT_decl_file [DW_FORM_data1] ("/tmp/test.c")
                    DW_AT_decl_line [DW_FORM_data1] (3)
                    DW_AT_decl_column [DW_FORM_data1]       (12)
                    DW_AT_sibling [DW_FORM_ref4]    (0x000006b4)

    0x00000696:     DW_TAG_enumerator
                      DW_AT_name [DW_FORM_string]   ("A")
                      DW_AT_const_value [DW_FORM_data1]     (0x7e)

    0x0000069a:     DW_TAG_enumerator
                      DW_AT_name [DW_FORM_string]   ("B")
                      DW_AT_const_value [DW_FORM_data1]     (0x7f)

    0x0000069e:     DW_TAG_enumerator
                      DW_AT_name [DW_FORM_string]   ("C")
                      DW_AT_const_value [DW_FORM_data1]     (0x80)

... and GDB prints it wrong:

    (gdb) ptype Test
    type = enum class Test : int {
        Test::A = 126,
        Test::B = 127,
        Test::C = -128,
        Test::D = -127,
        Test::G = -101,
        Test::E = -1,
        Test::F = 256
    }

However, note that printing Test::C specifically works:

    (gdb) p Test::C
    $1 = 128

The reason why we see different value for Test::C in the two commands is
that the value for "Test::C" is actually read and stored twice.  The
first one (shown by "ptype") is a `struct field` of the enumeration
type, created in function `update_enumeration_type_from_children`.  The
second (shown by "print Test::C") is a symbol of its own, instantiated
in function `new_symbol`.  And the two disagree on how to interpret the
value of enumerator `Test::C`.

GCC emits those DW_AT_const_value on enumerators in what we internally
call "confused" form (see attribute::confused_constant): a negative
value uses DW_FORM_sdata, while a non-negative one uses a narrow
form, which is then expected to be zero-extended.

From what I saw, LLVM always uses DW_FORM_udata or DW_FORM_sdata for
enumerators, so there is no confusion.

I am therefore myself a bit confused about why commit 5363deffcfb1
changed update_enumeration_type_from_children to have the following
assumption: if the underlying type is signed (or if we don't know the
underlying type), then a DW_FORM_data<n> encodes a signed number and
must therefore be sign-extended.  It sounds good on paper, but that's
not what the producers actually do.

The new_symbol path calls `dwarf2_const_value`, which calls
`dwarf2_const_value_attr`, which calls `attribute::confused_constant`,
which implements the correct logic for what gcc emits.

Fix the `update_enumeration_type_from_children` path by using
attribute::confused_constant there as well.

Add tests in four places to try to cover a mix of enumeration types with
a signed underlying type, an unsigned underlying type or no underlying
type at all, sometimes with and sometimes without negative values.  The
values of the enumerators must look good in "ptype" and when we print
them directly, but the sign of the enumeration type must also be
correct, which we verify by casting -1 to the type.

Without the fix, they would all produce some new failures:

    Running /home/simark/src/binutils-gdb/gdb/testsuite/gdb.cp/typed-enum.exp ...
    FAIL: gdb.cp/typed-enum.exp: print int_val
    FAIL: gdb.cp/typed-enum.exp: ptype int_enum
    Running /home/simark/src/binutils-gdb/gdb/testsuite/gdb.base/enumval.exp ...
    FAIL: gdb.base/enumval.exp: print g
    FAIL: gdb.base/enumval.exp: print M
    FAIL: gdb.base/enumval.exp: ptype enum mixed
    Running /home/simark/src/binutils-gdb/gdb/testsuite/gdb.ada/enum-sign.exp ...
    FAIL: gdb.ada/enum-sign.exp: print se_e
    FAIL: gdb.ada/enum-sign.exp: print se_f
    FAIL: gdb.ada/enum-sign.exp: ptype signed_enumeration
    Running /home/simark/src/binutils-gdb/gdb/testsuite/gdb.dwarf2/enum-type.exp ...
    FAIL: gdb.dwarf2/enum-type.exp: ptype enum ESIGNED
    FAIL: gdb.dwarf2/enum-type.exp: print S_NARROW
    FAIL: gdb.dwarf2/enum-type.exp: print S_WIDE
    FAIL: gdb.dwarf2/enum-type.exp: ptype enum ENOTYPE
    FAIL: gdb.dwarf2/enum-type.exp: print N_NARROW
    FAIL: gdb.dwarf2/enum-type.exp: ptype enum ENOTYPENEG
    FAIL: gdb.dwarf2/enum-type.exp: print NN_NARROW
    FAIL: gdb.dwarf2/enum-type.exp: print (enum ENOTYPE) -1
    FAIL: gdb.dwarf2/enum-type.exp: print (enum ENOTYPENEG) -1

With the fix, they all pass (on my machine™).

I checked whether the forms used to represent enumerator values changed
through the years, with the various compiler version.  I had these
installed on my machine so I tested them:

 - GCC: 4.8.5, 7.5.0, 8.5.0, 9.5.0, 10.5.0, 11.5.0, 13.4.1 and 16.2.1
 - clang: clang 18.1.8, 20.1.8, 21.1.8 and 22.1.8

The encoding has been stable throughout:

 - GCC 7 and up all emit exactly the same thing, a narrow form for
   non-negative values and DW_FORM_sdata for negative ones.

 - GCC 4.8 used DW_FORM_sdata throughout, which is unambiguous.  The
   narrow forms appeared somewhere between 4.8 and 7.

 - clang always names the form explicitly, DW_FORM_udata or
   DW_FORM_sdata, so it never produces the ambiguous encoding.

This is a regression that appeared in GDB 17, meaning there is already
one broken release out there.  Should we consider this patch for the GDB
18 branch?

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=34616
Change-Id: I5fd0dbecc1386511edf07ecc275299985f81828c
---
 gdb/dwarf2/read.c                        |  15 ++-
 gdb/testsuite/gdb.ada/enum-sign.exp      |  13 +-
 gdb/testsuite/gdb.ada/enum-sign/prog.adb |   4 +-
 gdb/testsuite/gdb.base/enumval.c         |  14 +-
 gdb/testsuite/gdb.base/enumval.exp       |  22 +++
 gdb/testsuite/gdb.cp/typed-enum.cc       |  16 ++-
 gdb/testsuite/gdb.cp/typed-enum.exp      |  22 ++-
 gdb/testsuite/gdb.dwarf2/enum-type.exp   | 163 ++++++++++++++++++++++-
 8 files changed, 253 insertions(+), 16 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index c1a7068edf0e..7e1a0d686a9e 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -10864,8 +10864,8 @@ die_byte_order (die_info *die, dwarf2_cu *cu, enum bfd_endian *byte_order)
    children.  In particular, the fields are computed.  If IS_UNSIGNED
    is set, the enumeration type's sign is already known (a true value
    means unsigned), and so examining the constants to determine the
-   sign isn't needed; when this is unset, the enumerator constants are
-   read as signed values.  */
+   sign isn't needed; when this is unset, the sign is deduced from the
+   constants.  */
 
 static void
 update_enumeration_type_from_children (struct die_info *die,
@@ -10904,9 +10904,10 @@ update_enumeration_type_from_children (struct die_info *die,
 	value = attr->unsigned_constant ().value_or (0);
       else
 	{
-	  /* Read as signed, either because we don't know the sign or
-	     because we know it is definitely signed.  */
-	  value = attr->signed_constant ().value_or (0);
+	  /* The known producers describe negative values using DW_FORM_sdata
+	     and the non-negative values using either DW_FORM_udata or
+	     DW_FORM_data<n>.  */
+	  value = attr->confused_constant ().value_or (0);
 
 	  if (value < 0)
 	    {
@@ -10988,8 +10989,8 @@ read_enumeration_type (struct die_info *die, struct dwarf2_cu *cu)
     type->set_is_stub (true);
 
   /* If the underlying type is known, and is unsigned, then we'll
-     assume the enumerator constants are unsigned.  Otherwise we have
-     to assume they are signed.  */
+     assume the enumerator constants are unsigned.  Otherwise, it will
+     depend on the form.  */
   std::optional<bool> is_unsigned;
 
   /* If this type has an underlying type that is not a stub, then we
diff --git a/gdb/testsuite/gdb.ada/enum-sign.exp b/gdb/testsuite/gdb.ada/enum-sign.exp
index 84098027849c..0313cc87dfb1 100644
--- a/gdb/testsuite/gdb.ada/enum-sign.exp
+++ b/gdb/testsuite/gdb.ada/enum-sign.exp
@@ -34,7 +34,18 @@ for {set num 0} {$num <= 300} {incr 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} {
+foreach {name dec} {se_a -1 se_b 0 se_c 1 se_d 2 se_e 128 se_f 255} {
     gdb_test "print $name" " = $name"
     gdb_test "print/d $name" " = $dec"
 }
+
+gdb_test "ptype signed_enumeration" \
+    [string_to_regexp \
+	 "type = (se_a => -1, se_b, se_c, se_d, se_e => 128, se_f => 255)"]
+
+set names {}
+for {set num 0} {$num <= 300} {incr num} {
+    lappend names value_$num
+}
+gdb_test "ptype unsigned_enumeration" \
+    [string_to_regexp "type = ([join $names ", "])"]
diff --git a/gdb/testsuite/gdb.ada/enum-sign/prog.adb b/gdb/testsuite/gdb.ada/enum-sign/prog.adb
index 6ef3c956917f..9087015ce3b9 100644
--- a/gdb/testsuite/gdb.ada/enum-sign/prog.adb
+++ b/gdb/testsuite/gdb.ada/enum-sign/prog.adb
@@ -321,8 +321,8 @@ procedure Prog is
 
    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);
+   type Signed_Enumeration is ( SE_A, SE_B, SE_C, SE_D, SE_E, SE_F);
+   for Signed_Enumeration use (-1, 0, 1, 2, 128, 255);
 
    Y : Signed_Enumeration := SE_D;
 
diff --git a/gdb/testsuite/gdb.base/enumval.c b/gdb/testsuite/gdb.base/enumval.c
index a95403237e15..9a06a49fe1eb 100644
--- a/gdb/testsuite/gdb.base/enumval.c
+++ b/gdb/testsuite/gdb.base/enumval.c
@@ -15,7 +15,19 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-enum e { I, J = 0xffffffffU, K = 0xf000000000000000ULL } e = J, f = K;
+enum wide_values { I, J = 0xffffffffU, K = 0xf000000000000000ULL }
+  e = J, f = K;
+
+/* Enum that mixes a negative enumerator with one that could be misinterpreted
+   if the compiler describes its value with DW_FORM_data1 (see PR
+   symtab/34616).  */
+
+enum mixed { M_NEG = -1, M = 200 } g = M;
+
+/* The same, but with no negative enumerator, so that the enum is
+   unsigned.  */
+
+enum unmixed { U_ZERO, U = 200 } h = U;
 
 enum { ZERO };
 
diff --git a/gdb/testsuite/gdb.base/enumval.exp b/gdb/testsuite/gdb.base/enumval.exp
index 6942d0265b5b..1415a159996c 100644
--- a/gdb/testsuite/gdb.base/enumval.exp
+++ b/gdb/testsuite/gdb.base/enumval.exp
@@ -70,6 +70,28 @@ gdb_test_multiple $test $test {
     }
 }
 
+# enum mixed
+gdb_test "print g" " = M"
+gdb_test "print M" " = M"
+gdb_test "print (int) M" " = 200"
+gdb_test "ptype enum mixed" \
+    [multi_line \
+	 "type = enum mixed {" \
+	 "    M_NEG = -1," \
+	 "    M = 200" \
+	 "}"]
+
+# enum unmixed
+gdb_test "print h" " = U"
+gdb_test "print U" " = U"
+gdb_test "print (int) U" " = 200"
+gdb_test "ptype enum unmixed" \
+    [multi_line \
+	 "type = enum unmixed {" \
+	 "    U_ZERO = 0," \
+	 "    U = 200" \
+	 "}"]
+
 # gold/15021
 # With -fdebug-types-section, Gold's .gdb_index entry for ZERO refers to the
 # CU, but the CU doesn't use the TU (type unit) that defines ZERO.
diff --git a/gdb/testsuite/gdb.cp/typed-enum.cc b/gdb/testsuite/gdb.cp/typed-enum.cc
index bca231a9e38a..5b4c479a9e0b 100644
--- a/gdb/testsuite/gdb.cp/typed-enum.cc
+++ b/gdb/testsuite/gdb.cp/typed-enum.cc
@@ -27,9 +27,23 @@ enum uchar_enum : unsigned char
   uchar_val = 128
 };
 
+enum int_enum : int
+{
+  int_three = 3,
+  int_val = 128
+};
+
+enum schar_enum : signed char
+{
+  schar_neg = -128,
+  schar_val = 127
+};
+
 int main()
 {
   int v1 = byte_val;
   int v2 = uchar_val;
-  return v1 == v2;
+  int v3 = int_val;
+  int v4 = schar_neg;
+  return v1 == v2 && v3 == v4;
 }
diff --git a/gdb/testsuite/gdb.cp/typed-enum.exp b/gdb/testsuite/gdb.cp/typed-enum.exp
index a2021abd64cb..ea2effcab35d 100644
--- a/gdb/testsuite/gdb.cp/typed-enum.exp
+++ b/gdb/testsuite/gdb.cp/typed-enum.exp
@@ -13,7 +13,7 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #
-# Check if unsigned typedef are handled correctly with typed enums.
+# Check various cases of typed enums.
 
 require allow_cplus_tests
 
@@ -30,3 +30,23 @@ if {[prepare_for_testing "failed to prepare" $testfile $srcfile $opts]} {
 
 gdb_test "print (int)byte_val" "= 128"
 gdb_test "print (int)uchar_val" "= 128"
+
+# int_enum
+gdb_test "print (int)int_val" "= 128"
+gdb_test "print int_val" "= int_val"
+gdb_test "ptype int_enum" \
+    [multi_line \
+	 "type = enum int_enum : int \{" \
+	 "    int_three = 3," \
+	 "    int_val = 128" \
+	 "\}"]
+
+# schar_enum
+gdb_test "print (int)schar_neg" "= -128"
+gdb_test "print schar_neg" "= schar_neg"
+gdb_test "ptype schar_enum" \
+    [multi_line \
+	 "type = enum schar_enum : signed char \{" \
+	 "    schar_neg = -128," \
+	 "    schar_val = 127" \
+	 "\}"]
diff --git a/gdb/testsuite/gdb.dwarf2/enum-type.exp b/gdb/testsuite/gdb.dwarf2/enum-type.exp
index 5af889a35441..f062bc50f7d8 100644
--- a/gdb/testsuite/gdb.dwarf2/enum-type.exp
+++ b/gdb/testsuite/gdb.dwarf2/enum-type.exp
@@ -63,6 +63,99 @@ Dwarf::assemble $asm_file {
 		    DW_AT_const_value 2 DW_FORM_sdata
 		}
 	    }
+
+	    # Enumeration with signed underlying type.
+	    DW_TAG_enumeration_type {
+		DW_AT_name ESIGNED
+		DW_AT_type :$integer_label
+	    } {
+		# These must not be interpreted as negative numbers, despite
+		# the underlying type being signed.  This is how gcc emits
+		# (at the time of writing):
+		#
+		# enum ESIGNED
+		#  {
+		#     S_NARROW = 0x80,
+		#     S_WIDE = 0x8000,
+		#     S_NEGATIVE = -10,
+		# };
+
+		DW_TAG_enumerator {
+		    DW_AT_name S_NARROW
+		    DW_AT_const_value 0x80 DW_FORM_data1
+		}
+
+		DW_TAG_enumerator {
+		    DW_AT_name S_WIDE
+		    DW_AT_const_value 0x8000 DW_FORM_data2
+		}
+
+		DW_TAG_enumerator {
+		    DW_AT_name S_NEGATIVE
+		    DW_AT_const_value -10 DW_FORM_sdata
+		}
+	    }
+
+	    # Enumeration with unsigned underlying type.
+	    DW_TAG_enumeration_type {
+		DW_AT_name EUNSIGNED
+		DW_AT_type :$uinteger_label
+	    } {
+		DW_TAG_enumerator {
+		    DW_AT_name U_NARROW
+		    DW_AT_const_value 0x80 DW_FORM_data1
+		}
+
+		DW_TAG_enumerator {
+		    DW_AT_name U_WIDE
+		    # Use this particular value to avoid the enum flags
+		    # mechanism kicking in.
+		    DW_AT_const_value 0x8880 DW_FORM_data2
+		}
+	    }
+
+	    # Enumeration whose underlying type is signed but which has no
+	    # negative enumerator.  It must still be considered a signed type.
+	    DW_TAG_enumeration_type {
+		DW_AT_name ESIGNEDPOS
+		DW_AT_type :$integer_label
+	    } {
+		DW_TAG_enumerator {
+		    DW_AT_name SP_MULTI
+		    DW_AT_const_value 3 DW_FORM_sdata
+		}
+	    }
+
+	    # Enumeration without an underlying type, the sign has to be
+	    # deduced from the constants.  Here they are all non-negative,
+	    # so the enum is unsigned.
+	    DW_TAG_enumeration_type {
+		DW_AT_name ENOTYPE
+		DW_AT_byte_size 4 DW_FORM_sdata
+	    } {
+		DW_TAG_enumerator {
+		    DW_AT_name N_NARROW
+		    DW_AT_const_value 0xff DW_FORM_data1
+		}
+	    }
+
+	    # Same, but with a negative enumerator, which makes the
+	    # enum signed.
+	    DW_TAG_enumeration_type {
+		DW_AT_name ENOTYPENEG
+		DW_AT_byte_size 4 DW_FORM_sdata
+	    } {
+		# This one still must not be sign-extended.
+		DW_TAG_enumerator {
+		    DW_AT_name NN_NARROW
+		    DW_AT_const_value 0xff DW_FORM_data1
+		}
+
+		DW_TAG_enumerator {
+		    DW_AT_name NN_NEGATIVE
+		    DW_AT_const_value -2 DW_FORM_sdata
+		}
+	    }
 	}
     }
 
@@ -114,16 +207,80 @@ gdb_test "ptype enum EU" \
 	"type = enum EU {" \
 	"    TWO = 2" \
 	"}"] "ptype EU in enum C"
-gdb_test_no_output "set lang c++"
-gdb_test "ptype enum EU" \
+
+gdb_test "with lang c++ -- ptype enum EU" \
     [multi_line \
 	"type = enum EU : unsigned int {" \
 	"    TWO = 2" \
 	"}"] "ptype EU in C++"
 
-gdb_test "p ns::val1" \
+gdb_test "with lang c++ -- p ns::val1" \
     " = ns::val1"
 
+# Check that DW_FORM_data* enumerator constants are not sign-extended
+# (PR 34616).
+#
+# Both "ptype" (which reads the enum type's fields) and "print" of an
+# enumerator (which reads the enumerator symbol) must agree.
+gdb_test "ptype enum ESIGNED" \
+    [multi_line \
+	 "type = enum ESIGNED {" \
+	 "    S_NARROW = 128," \
+	 "    S_WIDE = 32768," \
+	 "    S_NEGATIVE = -10" \
+	 "}"]
+gdb_test "print S_NARROW" " = S_NARROW"
+gdb_test "print (int) S_NARROW" " = 128"
+gdb_test "print S_WIDE" " = S_WIDE"
+gdb_test "print (int) S_WIDE" " = 32768"
+gdb_test "print S_NEGATIVE" " = S_NEGATIVE"
+gdb_test "print (int) S_NEGATIVE" " = -10"
+
+gdb_test "ptype enum EUNSIGNED" \
+    [multi_line \
+	 "type = enum EUNSIGNED {" \
+	 "    U_NARROW = 128," \
+	 "    U_WIDE = 34944" \
+	 "}"]
+gdb_test "print U_NARROW" " = U_NARROW"
+gdb_test "print (int) U_NARROW" " = 128"
+gdb_test "print U_WIDE" " = U_WIDE"
+gdb_test "print (int) U_WIDE" " = 34944"
+
+gdb_test "ptype enum ESIGNEDPOS" \
+    [multi_line \
+	 "type = enum ESIGNEDPOS {" \
+	 "    SP_MULTI = 3" \
+	 "}"]
+gdb_test "print SP_MULTI" " = SP_MULTI"
+gdb_test "print (int) SP_MULTI" " = 3"
+
+gdb_test "ptype enum ENOTYPE" \
+    [multi_line \
+	 "type = enum ENOTYPE {" \
+	 "    N_NARROW = 255" \
+	 "}"]
+gdb_test "print N_NARROW" " = N_NARROW"
+gdb_test "print (int) N_NARROW" " = 255"
+
+gdb_test "ptype enum ENOTYPENEG" \
+    [multi_line \
+	 "type = enum ENOTYPENEG {" \
+	 "    NN_NARROW = 255," \
+	 "    NN_NEGATIVE = -2" \
+	 "}"]
+gdb_test "print NN_NARROW" " = NN_NARROW"
+gdb_test "print (int) NN_NARROW" " = 255"
+gdb_test "print NN_NEGATIVE" " = NN_NEGATIVE"
+gdb_test "print (int) NN_NEGATIVE" " = -2"
+
+# Check the sign of the enumeration types themselves.
+gdb_test "print (enum ESIGNED) -1" " = -1"
+gdb_test "print (enum EUNSIGNED) -1" " = 4294967295"
+gdb_test "print (enum ESIGNEDPOS) -1" " = -1"
+gdb_test "print (enum ENOTYPE) -1" " = 4294967295"
+gdb_test "print (enum ENOTYPENEG) -1" " = -1"
+
 require !readnow
 require {string equal [have_index $binfile] ""}
 

base-commit: 6ead2253e6266b226a44190446f576185fc63ded
-- 
2.55.0


^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-09-12  1:47 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-10  2:25 [PATCH] gdb/dwarf2: don't sign-extend DW_FORM_data<n> enumerator constants simon.marchi
2026-09-11 14:58 ` Tom Tromey
2026-09-12  1:46   ` Simon Marchi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox