[PATCH] Fix handling of DW_AT_data_bit_offset

[PATCH] Fix handling of DW_AT_data_bit_offset

[Tom Tromey]

A newer version of GCC will now emit member locations using just
DW_AT_data_bit_offset, like:

 <3><14fe>: Abbrev Number: 1 (DW_TAG_member)
    <14ff>   DW_AT_name        : (indirect string, offset: 0x215e): nb_bytes
    <1503>   DW_AT_decl_file   : 1
    <1503>   DW_AT_decl_line   : 10
    <1504>   DW_AT_decl_column : 7
    <1505>   DW_AT_type        : <0x150b>
    <1509>   DW_AT_bit_size    : 31
    <150a>   DW_AT_data_bit_offset: 64

whereas earlier versions would emit something like:

 <3><164f>: Abbrev Number: 7 (DW_TAG_member)
    <1650>   DW_AT_name        : (indirect string, offset: 0x218d): nb_bytes
    <1654>   DW_AT_decl_file   : 1
    <1655>   DW_AT_decl_line   : 10
    <1656>   DW_AT_decl_column : 7
    <1657>   DW_AT_type        : <0x165f>
    <165b>   DW_AT_byte_size   : 4
    <165c>   DW_AT_bit_size    : 31
    <165d>   DW_AT_bit_offset  : 1
    <165e>   DW_AT_data_member_location: 8

That is, DW_AT_data_member_location is not emitted any more.  This is
a change due to the switch to DWARF 5 by default.

This change pointed out an existing bug in gdb, namely that the
attr_to_dynamic_prop depends on the presence of
DW_AT_data_member_location.  This patch moves the handling of
DW_AT_data_bit_offset into handle_data_member_location, and updates
attr_to_dynamic_prop to handle this new case.

A new test case is included.  This test fails with GCC 11, but passes
with an earlier version of GCC.
---
 gdb/dwarf2/read.c                          | 49 ++++++++++++++--------
 gdb/testsuite/gdb.ada/packed_record.exp    | 36 ++++++++++++++++
 gdb/testsuite/gdb.ada/packed_record/pr.adb | 35 ++++++++++++++++
 3 files changed, 103 insertions(+), 17 deletions(-)
 create mode 100644 gdb/testsuite/gdb.ada/packed_record.exp
 create mode 100644 gdb/testsuite/gdb.ada/packed_record/pr.adb

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 2e85deeaa39..c7a5362342d 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -14337,14 +14337,14 @@ dwarf2_access_attribute (struct die_info *die, struct dwarf2_cu *cu)
     }
 }
 
-/* Look for DW_AT_data_member_location.  Set *OFFSET to the byte
-   offset.  If the attribute was not found return 0, otherwise return
-   1.  If it was found but could not properly be handled, set *OFFSET
-   to 0.  */
+/* Look for DW_AT_data_member_location or DW_AT_data_bit_offset.  Set
+   *OFFSET to the byte offset.  If the attribute was not found return
+   0, otherwise return 1.  If it was found but could not properly be
+   handled, set *OFFSET to 0.  */
 
 static int
-handle_data_member_location (struct die_info *die, struct dwarf2_cu *cu,
-			     LONGEST *offset)
+handle_member_location (struct die_info *die, struct dwarf2_cu *cu,
+			LONGEST *offset)
 {
   struct attribute *attr;
 
@@ -14368,15 +14368,25 @@ handle_data_member_location (struct die_info *die, struct dwarf2_cu *cu,
 
       return 1;
     }
+  else
+    {
+      attr = dwarf2_attr (die, DW_AT_data_bit_offset, cu);
+      if (attr != nullptr)
+	{
+	  *offset = attr->constant_value (0);
+	  return 1;
+	}
+    }
 
   return 0;
 }
 
-/* Look for DW_AT_data_member_location and store the results in FIELD.  */
+/* Look for DW_AT_data_member_location or DW_AT_data_bit_offset and
+   store the results in FIELD.  */
 
 static void
-handle_data_member_location (struct die_info *die, struct dwarf2_cu *cu,
-			     struct field *field)
+handle_member_location (struct die_info *die, struct dwarf2_cu *cu,
+			struct field *field)
 {
   struct attribute *attr;
 
@@ -14418,6 +14428,12 @@ handle_data_member_location (struct die_info *die, struct dwarf2_cu *cu,
       else
 	dwarf2_complex_location_expr_complaint ();
     }
+  else
+    {
+      attr = dwarf2_attr (die, DW_AT_data_bit_offset, cu);
+      if (attr != nullptr)
+	SET_FIELD_BITPOS (*field, attr->constant_value (0));
+    }
 }
 
 /* Add an aggregate field to the field list.  */
@@ -14479,7 +14495,7 @@ dwarf2_add_field (struct field_info *fip, struct die_info *die,
 	}
 
       /* Get bit offset of field.  */
-      handle_data_member_location (die, cu, fp);
+      handle_member_location (die, cu, fp);
       attr = dwarf2_attr (die, DW_AT_bit_offset, cu);
       if (attr != nullptr && attr->form_is_constant ())
 	{
@@ -14526,10 +14542,6 @@ dwarf2_add_field (struct field_info *fip, struct die_info *die,
 				 - bit_offset - FIELD_BITSIZE (*fp)));
 	    }
 	}
-      attr = dwarf2_attr (die, DW_AT_data_bit_offset, cu);
-      if (attr != NULL)
-	SET_FIELD_BITPOS (*fp, (FIELD_BITPOS (*fp)
-				+ attr->constant_value (0)));
 
       /* Get name of field.  */
       fieldname = dwarf2_name (die, cu);
@@ -14590,7 +14602,7 @@ dwarf2_add_field (struct field_info *fip, struct die_info *die,
   else if (die->tag == DW_TAG_inheritance)
     {
       /* C++ base class field.  */
-      handle_data_member_location (die, cu, fp);
+      handle_member_location (die, cu, fp);
       FIELD_BITSIZE (*fp) = 0;
       fp->set_type (die_type (die, cu));
       FIELD_NAME (*fp) = fp->type ()->name ();
@@ -18305,6 +18317,9 @@ attr_to_dynamic_prop (const struct attribute *attr, struct die_info *die,
       if (target_attr == NULL)
 	target_attr = dwarf2_attr (target_die, DW_AT_data_member_location,
 				   target_cu);
+      if (target_attr == nullptr)
+	target_attr = dwarf2_attr (target_die, DW_AT_data_bit_offset,
+				   target_cu);
       if (target_attr == NULL)
 	{
 	  const char *name = var_decl_name (target_die, target_cu);
@@ -18348,11 +18363,11 @@ attr_to_dynamic_prop (const struct attribute *attr, struct die_info *die,
 	      }
 	    break;
 	  case DW_AT_data_member_location:
+	  case DW_AT_data_bit_offset:
 	    {
 	      LONGEST offset;
 
-	      if (!handle_data_member_location (target_die, target_cu,
-						&offset))
+	      if (!handle_member_location (target_die, target_cu, &offset))
 		return 0;
 
 	      baton = XOBNEW (obstack, struct dwarf2_property_baton);
diff --git a/gdb/testsuite/gdb.ada/packed_record.exp b/gdb/testsuite/gdb.ada/packed_record.exp
new file mode 100644
index 00000000000..20793067d06
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/packed_record.exp
@@ -0,0 +1,36 @@
+# Copyright 2021 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"
+
+if { [skip_ada_tests] } { return -1 }
+
+standard_ada_testfile pr
+
+foreach_with_prefix scenario {all minimal} {
+    set flags [list debug additional_flags=-fgnat-encodings=$scenario]
+
+    if {[gdb_compile_ada "${srcfile}" "${binfile}" executable $flags] != ""} {
+	return -1
+    }
+
+    clean_restart ${testfile}
+
+    set bp_location [gdb_get_line_number "STOP" ${testdir}/pr.adb]
+    runto "pr.adb:$bp_location"
+
+    gdb_test "print var" \
+	"= \\(length => 11, length_t => 23, bytes => 13, msg => hello, val => \"abcdefghijk\"\\)"
+}
diff --git a/gdb/testsuite/gdb.ada/packed_record/pr.adb b/gdb/testsuite/gdb.ada/packed_record/pr.adb
new file mode 100644
index 00000000000..7abfda9085b
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/packed_record/pr.adb
@@ -0,0 +1,35 @@
+--  Copyright 2021 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 pr is
+
+   type Report is (Hello, Goodbye);
+
+   type Response (Length : integer ) is record
+      Length_t : Integer;
+      Bytes : Natural;
+      Msg : Report;
+      Val : String(1..Length);
+   end record;
+
+   pragma pack (Response);
+
+   Var : Response(11) := (11, 23, 13, Hello, "abcdefghijk");
+
+begin
+
+   null; -- STOP
+
+end pr;
-- 
2.26.3

Re: [PATCH] Fix handling of DW_AT_data_bit_offset

[Tom Tromey]

>>>>> "Tom" == Tom Tromey <tromey@adacore.com> writes:

Tom> A newer version of GCC will now emit member locations using just
Tom> DW_AT_data_bit_offset, like:
[...]

Tom> This change pointed out an existing bug in gdb, namely that the
Tom> attr_to_dynamic_prop depends on the presence of
Tom> DW_AT_data_member_location.  This patch moves the handling of
Tom> DW_AT_data_bit_offset into handle_data_member_location, and updates
Tom> attr_to_dynamic_prop to handle this new case.

Tom> A new test case is included.  This test fails with GCC 11, but passes
Tom> with an earlier version of GCC.

I'm checking this in now.

Tom

Re: [PATCH] Fix handling of DW_AT_data_bit_offset

[Tom Tromey]

Tom> A new test case is included.  This test fails with GCC 11, but passes
Tom> with an earlier version of GCC.

> I'm checking this in now.

I spoke too soon - the new test fails now with the Fedora 34 system gcc.
I'll debug it a bit and see what sort of change is needed.

Tom

Re: [PATCH] Fix handling of DW_AT_data_bit_offset

[Tom Tromey]

>>>>> "Tom" == Tom Tromey <tromey@adacore.com> writes:

Tom> A new test case is included.  This test fails with GCC 11, but passes
Tom> with an earlier version of GCC.

>> I'm checking this in now.

Tom> I spoke too soon - the new test fails now with the Fedora 34 system gcc.
Tom> I'll debug it a bit and see what sort of change is needed.

I finally looked.  The working DWARF has an array in a structure that
looks like:

 <3><11f9>: Abbrev Number: 10 (DW_TAG_array_type)
    <11fa>   DW_AT_name        : (indirect string, offset: 0x1a4d): pr__response__T2b
    <11fe>   DW_AT_type        : <0x1248>
    <1202>   DW_AT_sibling     : <0x1210>
 <4><1206>: Abbrev Number: 11 (DW_TAG_subrange_type)
    <1207>   DW_AT_type        : <0x1241>
    <120b>   DW_AT_upper_bound : <0x11a7>

... where the upper bound points at the first member of the enclosing
struct.

However, the failing DWARF does:

 <3><1058>: Abbrev Number: 11 (DW_TAG_array_type)
    <1059>   DW_AT_name        : (indirect string, offset: 0x1a87): pr__response__T2b
    <105d>   DW_AT_type        : <0x10a7>
    <1061>   DW_AT_sibling     : <0x106f>
 <4><1065>: Abbrev Number: 12 (DW_TAG_subrange_type)
    <1066>   DW_AT_type        : <0x10a0>
    <106a>   DW_AT_upper_bound : 3 byte block: 97 94 4 	(DW_OP_push_object_address; DW_OP_deref_size: 4)


Here, and I think correctly, gdb uses the address of the array in the
struct as the object address -- but gcc apparently expects the object
address to be the address of the enclosing structure.

This fix is in gcc 12, so I'm going to update the test case to xfail for
earlier versions of gcc, and then check it in.

Tom