Handle DW_AT_endianity on enumeration types
Checks
Context |
Check |
Description |
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_gdb_build--master-arm |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_gdb_check--master-arm |
success
|
Testing passed
|
Commit Message
A user found that gdb would not correctly print a field from an Ada
record using the scalar storage order feature. We tracked this down
to a combination of problems.
First, GCC did not emit DW_AT_endianity on the enumeration type.
DWARF does not specify this, but it is an obvious and harmless
extension. This was fixed in GCC recently:
https://gcc.gnu.org/pipermail/gcc-patches/2024-January/642347.html
https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=5d8b60effc7268448a94fbbbad923ab6871252cd
Second, GDB did not handle this attribute on enumeration types. This
patch makes this change and adds a test case that will pass with the
patched GCC.
So far, the GCC patch isn't on the gcc-13 branch; but if it ever goes
in, the test case in this patch can be updated to reflect that.
---
gdb/dwarf2/read.c | 66 ++++++++++++-------
gdb/testsuite/gdb.ada/scalar_storage.exp | 9 ++-
.../gdb.ada/scalar_storage/storage.adb | 9 ++-
3 files changed, 57 insertions(+), 27 deletions(-)
Comments
On 1/12/24 06:13, Tom Tromey wrote:
> A user found that gdb would not correctly print a field from an Ada
> record using the scalar storage order feature. We tracked this down
> to a combination of problems.
>
> First, GCC did not emit DW_AT_endianity on the enumeration type.
> DWARF does not specify this, but it is an obvious and harmless
> extension. This was fixed in GCC recently:
>
> https://gcc.gnu.org/pipermail/gcc-patches/2024-January/642347.html
> https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=5d8b60effc7268448a94fbbbad923ab6871252cd
>
> Second, GDB did not handle this attribute on enumeration types. This
> patch makes this change and adds a test case that will pass with the
> patched GCC.
>
> So far, the GCC patch isn't on the gcc-13 branch; but if it ever goes
> in, the test case in this patch can be updated to reflect that.
I appreciate the KFAIL here to keep the test suite from throwing
transient FAILS awaiting compiler updates.
This all looks pretty straight-forward.
Feel free to add my Reviewed-by:.
Keith
>>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes:
Keith> This all looks pretty straight-forward.
Keith> Feel free to add my Reviewed-by:.
Thank you. I'm going to check this in.
Tom
@@ -13170,6 +13170,44 @@ process_structure_scope (struct die_info *die, struct dwarf2_cu *cu)
}
}
+/* Read DW_AT_endianity from DIE and compute the byte order that
+ should be used. The CU's arch is used as the default. The result
+ is true if the returned arch differs from the default, and false if
+ they are the same. If provided, the out parameter BYTE_ORDER is
+ also set. */
+
+static bool
+die_byte_order (die_info *die, dwarf2_cu *cu, enum bfd_endian *byte_order)
+{
+ gdbarch *arch = cu->per_objfile->objfile->arch ();
+ enum bfd_endian arch_order = gdbarch_byte_order (arch);
+ enum bfd_endian new_order = arch_order;
+
+ attribute *attr = dwarf2_attr (die, DW_AT_endianity, cu);
+ if (attr != nullptr && attr->form_is_constant ())
+ {
+ int endianity = attr->constant_value (0);
+
+ switch (endianity)
+ {
+ case DW_END_big:
+ new_order = BFD_ENDIAN_BIG;
+ break;
+ case DW_END_little:
+ new_order = BFD_ENDIAN_LITTLE;
+ break;
+ default:
+ complaint (_("DW_AT_endianity has unrecognized value %d"), endianity);
+ break;
+ }
+ }
+
+ if (byte_order != nullptr)
+ *byte_order = new_order;
+
+ return new_order != arch_order;
+}
+
/* Assuming DIE is an enumeration type, and TYPE is its associated
type, update TYPE using some information only available in DIE's
children. In particular, the fields are computed. */
@@ -13317,6 +13355,8 @@ read_enumeration_type (struct die_info *die, struct dwarf2_cu *cu)
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));
+
set_die_type (die, type, cu);
/* Finish the creation of this type by using the enum's children.
@@ -15220,7 +15260,6 @@ read_base_type (struct die_info *die, struct dwarf2_cu *cu)
struct attribute *attr;
int encoding = 0, bits = 0;
const char *name;
- gdbarch *arch;
attr = dwarf2_attr (die, DW_AT_encoding, cu);
if (attr != nullptr && attr->form_is_constant ())
@@ -15232,27 +15271,8 @@ read_base_type (struct die_info *die, struct dwarf2_cu *cu)
if (!name)
complaint (_("DW_AT_name missing from DW_TAG_base_type"));
- arch = objfile->arch ();
- enum bfd_endian byte_order = gdbarch_byte_order (arch);
-
- attr = dwarf2_attr (die, DW_AT_endianity, cu);
- if (attr != nullptr && attr->form_is_constant ())
- {
- int endianity = attr->constant_value (0);
-
- switch (endianity)
- {
- case DW_END_big:
- byte_order = BFD_ENDIAN_BIG;
- break;
- case DW_END_little:
- byte_order = BFD_ENDIAN_LITTLE;
- break;
- default:
- complaint (_("DW_AT_endianity has unrecognized value %d"), endianity);
- break;
- }
- }
+ enum bfd_endian byte_order;
+ bool not_default = die_byte_order (die, cu, &byte_order);
if ((encoding == DW_ATE_signed_fixed || encoding == DW_ATE_unsigned_fixed)
&& cu->lang () == language_ada
@@ -15390,7 +15410,7 @@ read_base_type (struct die_info *die, struct dwarf2_cu *cu)
maybe_set_alignment (cu, die, type);
- type->set_endianity_is_not_default (gdbarch_byte_order (arch) != byte_order);
+ type->set_endianity_is_not_default (not_default);
if (TYPE_SPECIFIC_FIELD (type) == TYPE_SPECIFIC_INT)
{
@@ -33,5 +33,10 @@ if {![runto "storage.adb:$bp_location"]} {
return
}
-gdb_test "print V_LE" "= \\(value => 126, another_value => 12\\)"
-gdb_test "print V_BE" "= \\(value => 126, another_value => 12\\)"
+gdb_test "print V_LE" "= \\(value => 126, another_value => 12, color => green\\)"
+
+# This requires a compiler fix that is in GCC 14.
+if {[gcc_major_version] < 14} {
+ setup_kfail "DW_AT_endianity on enum types" *-*-*
+}
+gdb_test "print V_BE" "= \\(value => 126, another_value => 12, color => green\\)"
@@ -20,14 +20,19 @@ procedure Storage is
subtype Some_Range is Natural range 0..127;
subtype Another_Range is Natural range 0..15;
+ type Colors is (Red, Green, Blue);
+ for Colors use (Red => 0, Green => 1, Blue => 2);
+
type Rec is record
Value : Some_Range;
Another_Value : Another_Range;
+ Color : Colors;
end record;
for Rec use record
Value at 0 range 0..6;
Another_Value at 0 range 7..10;
+ Color at 0 range 11..13;
end record;
type Rec_LE is new Rec;
@@ -42,8 +47,8 @@ procedure Storage is
V_BE : Rec_BE;
begin
- V_LE := (126, 12);
- V_BE := (126, 12);
+ V_LE := (126, 12, Green);
+ V_BE := (126, 12, Green);
Do_Nothing (V_LE'Address); -- START
Do_Nothing (V_BE'Address);