[RFA/commit DWARF 1/2] Add support for dynamic DW_AT_byte_stride.

[RFA/commit DWARF 1/2] Add support for dynamic DW_AT_byte_stride.

[Joel Brobecker]

Hello,

This patch adds support for DW_AT_byte_stride, using Ada as one
example of where this would be useful. However, the implementation
is language-agnostic.

Consider the following Ada code:

   procedure Nested (L, U : Integer) is
      subtype Small_Type is Integer range L .. U;
      type Record_Type (I : Small_Type := L) is record
         S : String (1 .. I);
      end record;
      type Array_Type is array (Integer range <>) of Record_Type;

      A1 : Array_Type :=
        (1 => (I => 0, S => <>),
         2 => (I => 1, S => "A"),
         3 => (I => 2, S => "AB"));

      procedure Discard (R : Record_Type) is
      begin
         null;
      end Discard;

   begin
      Discard (A1 (1));  -- STOP
   end;

It defines an array A1 of Record_Type, which is a variant record
type whose maximum size actually depends on the value of the
parameters passed when calling Nested. As a result, the stride
of the array A1 cannot be known statically, which leads the compiler
to generate a dynamic DW_AT_byte_stride attribute for our type.
Here is what the debugging info looks like with GNAT:

        .uleb128 0x10   # (DIE (0x14e) DW_TAG_array_type)
        .long   .LASF17 # DW_AT_name: "foo__nested__T18b"
        .long   0x141   # DW_AT_byte_stride
        .long   0xdc    # DW_AT_type
        .uleb128 0x11   # (DIE (0x15f) DW_TAG_subrange_type)
        .long   0x166   # DW_AT_type
        .byte   0x3     # DW_AT_upper_bound
        .byte   0       # end of children of DIE 0x14e

There DW_AT_byte_stride is a reference to a local (internal)
variable:

        .uleb128 0x9    # (DIE (0x141) DW_TAG_variable)
        .long   .LASF6  # DW_AT_name: "foo__nested__T18b___PAD___XVZ"

This patch enhances GDB to handle this dynamic byte stride attribute
by first adding a new dynamic_prop_node_kind (DYN_PROP_BYTE_STRIDE)
to store the array dynamic stride info (when dynamic). It then enhances
the dynamic type resolver to handle this dynamic property.

Before applying this patch, trying to print the value of some of
A1's elements after having stopped at the "STOP" comment does not
work. For instance:

    (gdb) p a1(2)
    Cannot access memory at address 0x80000268dec0

With this patch applied, GDB now prints the value of all 3 elements
correctly:

    (gdb) print A1(1)
    $1 = (i => 0, s => "")
    (gdb) print A1(2)
    $2 = (i => 1, s => "A")
    (gdb) print A1(3)
    $3 = (i => 2, s => "AB")

gdb/ChangeLog:

        * gdbtypes.h (enum dynamic_prop_node_kind) <DYN_PROP_BYTE_STRIDE>:
        New enum value.
        (create_array_type_with_stride): Add byte_stride_prop parameter.
        * gdbtypes.c (create_array_type_with_stride) <byte_stride_prop>:
        New parameter.  Update all callers in this file.
        (array_type_has_dynamic_stride): New function.
        (is_dynamic_type_internal, resolve_dynamic_array): Add handling
        of arrays with dynamic byte strides.
        * dwarf2read.c (read_array_type): Add support for dynamic
        DW_AT_byte_stride attributes.

gdb/testsuite/ChangeLog:

        * gdb.ada/dyn_stride: New testcase.

Tested on x86_64-linux.
This is an area where I worked quite a bit, so I feel relatively
confident self approving; but I would still appreciate all comments!

Thank you,
-- 
Joel

---
 gdb/dwarf2read.c                         | 27 +++++++++--
 gdb/gdbtypes.c                           | 80 ++++++++++++++++++++++++++++----
 gdb/gdbtypes.h                           |  6 ++-
 gdb/testsuite/gdb.ada/dyn_stride.exp     | 38 +++++++++++++++
 gdb/testsuite/gdb.ada/dyn_stride/foo.adb | 42 +++++++++++++++++
 5 files changed, 180 insertions(+), 13 deletions(-)
 create mode 100644 gdb/testsuite/gdb.ada/dyn_stride.exp
 create mode 100644 gdb/testsuite/gdb.ada/dyn_stride/foo.adb

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 8aed7f4..d5a55ea 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -16300,6 +16300,7 @@ read_array_type (struct die_info *die, struct dwarf2_cu *cu)
   struct type *element_type, *range_type, *index_type;
   struct attribute *attr;
   const char *name;
+  struct dynamic_prop *byte_stride_prop = NULL;
   unsigned int bit_stride = 0;
 
   element_type = die_type (die, cu);
@@ -16311,7 +16312,25 @@ read_array_type (struct die_info *die, struct dwarf2_cu *cu)
 
   attr = dwarf2_attr (die, DW_AT_byte_stride, cu);
   if (attr != NULL)
-    bit_stride = DW_UNSND (attr) * 8;
+    {
+      int stride_ok;
+
+      byte_stride_prop
+	= (struct dynamic_prop *) alloca (sizeof (struct dynamic_prop));
+      stride_ok = attr_to_dynamic_prop (attr, die, cu, byte_stride_prop);
+      if (!stride_ok)
+	{
+	  complaint (&symfile_complaints,
+		     _("unable to read array DW_AT_byte_stride "
+		       " - DIE at 0x%x [in module %s]"),
+		     to_underlying (die->sect_off),
+		     objfile_name (cu->objfile));
+	  /* Ignore this attribute.  We will likely not be able to print
+	     arrays of this type correctly, but there is little we can do
+	     to help if we cannot read the attribute's value.  */
+	  byte_stride_prop = NULL;
+	}
+    }
 
   attr = dwarf2_attr (die, DW_AT_bit_stride, cu);
   if (attr != NULL)
@@ -16324,7 +16343,7 @@ read_array_type (struct die_info *die, struct dwarf2_cu *cu)
       index_type = objfile_type (objfile)->builtin_int;
       range_type = create_static_range_type (NULL, index_type, 0, -1);
       type = create_array_type_with_stride (NULL, element_type, range_type,
-					    bit_stride);
+					    byte_stride_prop, bit_stride);
       return set_die_type (die, type, cu);
     }
 
@@ -16357,14 +16376,14 @@ read_array_type (struct die_info *die, struct dwarf2_cu *cu)
 
       while (i < range_types.size ())
 	type = create_array_type_with_stride (NULL, type, range_types[i++],
-					      bit_stride);
+					      byte_stride_prop, bit_stride);
     }
   else
     {
       size_t ndim = range_types.size ();
       while (ndim-- > 0)
 	type = create_array_type_with_stride (NULL, type, range_types[ndim],
-					      bit_stride);
+					      byte_stride_prop, bit_stride);
     }
 
   /* Understand Dwarf2 support for vector types (like they occur on
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 01ab6fa..6ca342b 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -1090,6 +1090,14 @@ discrete_position (struct type *type, LONGEST val, LONGEST *pos)
    Elements will be of type ELEMENT_TYPE, the indices will be of type
    RANGE_TYPE.
 
+   BYTE_STRIDE_PROP, when not NULL, provides the array's byte stride.
+   This byte stride property is added to the resulting array type
+   as a DYN_PROP_BYTE_STRIDE.  As a consequence, the BYTE_STRIDE_PROP
+   argument can only be used to create types that are objfile-owned
+   (see add_dyn_prop), meaning that either this function must be called
+   with an objfile-owned RESULT_TYPE, or an objfile-owned RANGE_TYPE.
+
+   BIT_STRIDE is taken into account only when BYTE_STRIDE_PROP is NULL.
    If BIT_STRIDE is not zero, build a packed array type whose element
    size is BIT_STRIDE.  Otherwise, ignore this parameter.
 
@@ -1101,14 +1109,27 @@ struct type *
 create_array_type_with_stride (struct type *result_type,
 			       struct type *element_type,
 			       struct type *range_type,
+			       struct dynamic_prop *byte_stride_prop,
 			       unsigned int bit_stride)
 {
+  if (byte_stride_prop != NULL
+      && byte_stride_prop->kind == PROP_CONST)
+    {
+      /* The byte stride is actually not dynamic.  Pretend we were
+	 called with bit_stride set instead of byte_stride_prop.
+	 This will give us the same result type, while avoiding
+	 the need to handle this as a special case.  */
+      bit_stride = byte_stride_prop->data.const_val * 8;
+      byte_stride_prop = NULL;
+    }
+
   if (result_type == NULL)
     result_type = alloc_type_copy (range_type);
 
   TYPE_CODE (result_type) = TYPE_CODE_ARRAY;
   TYPE_TARGET_TYPE (result_type) = element_type;
-  if (has_static_range (TYPE_RANGE_DATA (range_type))
+  if (byte_stride_prop == NULL
+      && has_static_range (TYPE_RANGE_DATA (range_type))
       && (!type_not_associated (result_type)
 	  && !type_not_allocated (result_type)))
     {
@@ -1144,7 +1165,10 @@ create_array_type_with_stride (struct type *result_type,
   TYPE_FIELDS (result_type) =
     (struct field *) TYPE_ZALLOC (result_type, sizeof (struct field));
   TYPE_INDEX_TYPE (result_type) = range_type;
-  if (bit_stride > 0)
+  if (byte_stride_prop != NULL)
+    add_dyn_prop (DYN_PROP_BYTE_STRIDE, *byte_stride_prop, result_type,
+		  TYPE_OBJFILE (result_type));
+  else if (bit_stride > 0)
     TYPE_FIELD_BITSIZE (result_type, 0) = bit_stride;
 
   /* TYPE_TARGET_STUB will take care of zero length arrays.  */
@@ -1163,7 +1187,7 @@ create_array_type (struct type *result_type,
 		   struct type *range_type)
 {
   return create_array_type_with_stride (result_type, element_type,
-					range_type, 0);
+					range_type, NULL, 0);
 }
 
 struct type *
@@ -1824,6 +1848,17 @@ stub_noname_complaint (void)
   complaint (&symfile_complaints, _("stub type has NULL name"));
 }
 
+/* Return nonzero if TYPE has a DYN_PROP_BYTE_STRIDE dynamic property
+   attached to it, and that property has a non-constant value.  */
+
+static int
+array_type_has_dynamic_stride (struct type *type)
+{
+  struct dynamic_prop *prop = get_dyn_prop (DYN_PROP_BYTE_STRIDE, type);
+
+  return (prop != NULL && prop->kind != PROP_CONST);
+}
+
 /* Worker for is_dynamic_type.  */
 
 static int
@@ -1869,11 +1904,16 @@ is_dynamic_type_internal (struct type *type, int top_level)
       {
 	gdb_assert (TYPE_NFIELDS (type) == 1);
 
-	/* The array is dynamic if either the bounds are dynamic,
-	   or the elements it contains have a dynamic contents.  */
+	/* The array is dynamic if either the bounds are dynamic...  */
 	if (is_dynamic_type_internal (TYPE_INDEX_TYPE (type), 0))
 	  return 1;
-	return is_dynamic_type_internal (TYPE_TARGET_TYPE (type), 0);
+	/* ... or the elements it contains have a dynamic contents...  */
+	if (is_dynamic_type_internal (TYPE_TARGET_TYPE (type), 0))
+	  return 1;
+	/* ... or if it has a dynamic stride...  */
+	if (array_type_has_dynamic_stride (type))
+	  return 1;
+	return 0;
       }
 
     case TYPE_CODE_STRUCT:
@@ -1969,6 +2009,7 @@ resolve_dynamic_array (struct type *type,
   struct type *range_type;
   struct type *ary_dim;
   struct dynamic_prop *prop;
+  unsigned int bit_stride = 0;
 
   gdb_assert (TYPE_CODE (type) == TYPE_CODE_ARRAY);
 
@@ -2000,8 +2041,31 @@ resolve_dynamic_array (struct type *type,
   else
     elt_type = TYPE_TARGET_TYPE (type);
 
-  return create_array_type_with_stride (type, elt_type, range_type,
-                                        TYPE_FIELD_BITSIZE (type, 0));
+  prop = get_dyn_prop (DYN_PROP_BYTE_STRIDE, type);
+  if (prop != NULL)
+    {
+      int prop_eval_ok
+	= dwarf2_evaluate_property (prop, NULL, addr_stack, &value);
+
+      if (prop_eval_ok)
+	{
+	  remove_dyn_prop (DYN_PROP_BYTE_STRIDE, type);
+	  bit_stride = (unsigned int) (value * 8);
+	}
+      else
+	{
+	  /* Could be a bug in our code, but it could also happen
+	     if the DWARF info is not correct.  Issue a warning,
+	     and assume no byte/bit stride (leave bit_stride = 0).  */
+	  warning (_("cannot determine array stride for type %s"),
+		   TYPE_NAME (type) ? TYPE_NAME (type) : "<no name>");
+	}
+    }
+  else
+    bit_stride = TYPE_FIELD_BITSIZE (type, 0);
+
+  return create_array_type_with_stride (type, elt_type, range_type, NULL,
+                                        bit_stride);
 }
 
 /* Resolve dynamic bounds of members of the union TYPE to static
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 68ffaf8..3725462 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -418,6 +418,9 @@ enum dynamic_prop_node_kind
   /* A property representing DW_AT_allocated.  The presence of this attribute
      indicated that the object of the type can be associated.  */
   DYN_PROP_ASSOCIATED,
+
+  /* A property providing an array's byte stride.  */
+  DYN_PROP_BYTE_STRIDE,
 };
 
 /* * List for dynamic type attributes.  */
@@ -1791,7 +1794,8 @@ extern struct type *create_static_range_type (struct type *, struct type *,
 
 
 extern struct type *create_array_type_with_stride
-  (struct type *, struct type *, struct type *, unsigned int);
+  (struct type *, struct type *, struct type *,
+   struct dynamic_prop *, unsigned int);
 
 extern struct type *create_range_type (struct type *, struct type *,
 				       const struct dynamic_prop *,
diff --git a/gdb/testsuite/gdb.ada/dyn_stride.exp b/gdb/testsuite/gdb.ada/dyn_stride.exp
new file mode 100644
index 0000000..4dd1177
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/dyn_stride.exp
@@ -0,0 +1,38 @@
+# Copyright 2015-2017 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"
+
+standard_ada_testfile foo
+
+if {[gdb_compile_ada "${srcfile}" "${binfile}" executable [list debug ]] != "" } {
+  return -1
+}
+
+clean_restart ${testfile}
+
+set bp_location [gdb_get_line_number "STOP" ${testdir}/foo.adb]
+if ![runto "foo.adb:$bp_location" ] then {
+  return -1
+}
+
+gdb_test "print A1(1)" \
+         "\\(i => 0, s => \"\"\\)"
+
+gdb_test "print A1(2)" \
+         "\\(i => 1, s => \"A\"\\)"
+
+gdb_test "print A1(3)" \
+         "\\(i => 2, s => \"AB\"\\)"
diff --git a/gdb/testsuite/gdb.ada/dyn_stride/foo.adb b/gdb/testsuite/gdb.ada/dyn_stride/foo.adb
new file mode 100644
index 0000000..7ee80b2
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/dyn_stride/foo.adb
@@ -0,0 +1,42 @@
+--  Copyright 2015-2017 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 Foo is
+
+   procedure Nested (L, U : Integer) is
+      subtype Small_Type is Integer range L .. U;
+      type Record_Type (I : Small_Type := L) is record
+         S : String (1 .. I);
+      end record;
+      type Array_Type is array (Integer range <>) of Record_Type;
+
+      A1 : Array_Type :=
+        (1 => (I => 0, S => <>),
+         2 => (I => 1, S => "A"),
+         3 => (I => 2, S => "AB"));
+
+      procedure Discard (R : Record_Type) is
+      begin
+         null;
+      end Discard;
+
+   begin
+      Discard (A1 (1));  -- STOP
+   end;
+
+begin
+   Nested (0, 10);
+   Nested (-10, 10);
+end Foo;
-- 
2.1.4

[commit/Ada 2/2] slices of arrays with dynamic strides

[Joel Brobecker]

Hello,

This is a patch which fixes an isses with array slices in Ada
when the array has a dynamic stride.  It depends on the patch
adding support for DW_AT_byte_stride, so I will push after
the first one gets approved.

Consider the following Ada code:

       procedure Nested (L, U : Integer) is
          subtype Small_Type is Integer range L .. U;
          type Record_Type (I : Small_Type := L) is record
             S : String (1 .. I);
          end record;
          type Array_Type is array (Integer range <>) of Record_Type;

          A1 : Array_Type :=
            (1 => (I => 0, S => <>),
             2 => (I => 1, S => "A"),
             3 => (I => 2, S => "AB"));

          procedure Discard (R : Record_Type) is
          begin
             null;
          end Discard;

       begin
          Discard (A1 (1));  -- STOP
       end;

Trying to print a slice of that array currently yields:

    (gdb) p a1(1..3)
    $1 = ((i => 0, s => ""), (i => 0, s => ""), (i => 0, s => ""))

We expected instead:

    (gdb) p a1(1..3)
    $1 = ((i => 0, s => ""), (i => 1, s => "A"), (i => 2, s => "AB"))

This is because the functions we use in ada-lang.c to create the type
of the array slice (ada_value_slice and ada_value_slice_from_ptr) was
not taking into account the stride of the array. This patch fixes this.

gdb/ChangeLog:

        * ada-lang.c (ada_value_slice_from_ptr): Take array stride into
        account when creating the array type of the slice.
        (ada_value_slice): Likewise.

gdb/testsuite/ChangeLog:

        * gdb.ada/dyn_stride.exp: Add slice test.

Note that, with the current use of ada_value_slice, the enhancement
to handle dynamic array strides seems unnecessary, because I do not
see how an array with a dynamic stride can be referenced by either
by reference or pointer. Since references are coerced to array pointers,
in both cases, the slice is performed by ada_value_slice_from_ptr.
But ada_value_slice is enhanced nonetheless, in the spirit of making
the code more robust, in case we missed something, and also as similar
as possible with its from_ptr counterpart.

tested on x86_64-linux.

Thanks,
-- 
Joel

---
 gdb/ada-lang.c                       | 12 ++++++++----
 gdb/testsuite/gdb.ada/dyn_stride.exp |  3 +++
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 9e637eb..c3b1cd3 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -2910,8 +2910,10 @@ ada_value_slice_from_ptr (struct value *array_ptr, struct type *type,
   struct type *base_index_type = TYPE_TARGET_TYPE (TYPE_INDEX_TYPE (type0));
   struct type *index_type
     = create_static_range_type (NULL, base_index_type, low, high);
-  struct type *slice_type =
-    create_array_type (NULL, TYPE_TARGET_TYPE (type0), index_type);
+  struct type *slice_type = create_array_type_with_stride
+			      (NULL, TYPE_TARGET_TYPE (type0), index_type,
+			       get_dyn_prop (DYN_PROP_BYTE_STRIDE, type0),
+			       TYPE_FIELD_BITSIZE (type0, 0));
   int base_low =  ada_discrete_type_low_bound (TYPE_INDEX_TYPE (type0));
   LONGEST base_low_pos, low_pos;
   CORE_ADDR base;
@@ -2938,8 +2940,10 @@ ada_value_slice (struct value *array, int low, int high)
   struct type *base_index_type = TYPE_TARGET_TYPE (TYPE_INDEX_TYPE (type));
   struct type *index_type
     = create_static_range_type (NULL, TYPE_INDEX_TYPE (type), low, high);
-  struct type *slice_type =
-    create_array_type (NULL, TYPE_TARGET_TYPE (type), index_type);
+  struct type *slice_type = create_array_type_with_stride
+			      (NULL, TYPE_TARGET_TYPE (type), index_type,
+			       get_dyn_prop (DYN_PROP_BYTE_STRIDE, type),
+			       TYPE_FIELD_BITSIZE (type, 0));
   LONGEST low_pos, high_pos;
 
   if (!discrete_position (base_index_type, low, &low_pos)
diff --git a/gdb/testsuite/gdb.ada/dyn_stride.exp b/gdb/testsuite/gdb.ada/dyn_stride.exp
index 4dd1177..19be9cb 100644
--- a/gdb/testsuite/gdb.ada/dyn_stride.exp
+++ b/gdb/testsuite/gdb.ada/dyn_stride.exp
@@ -36,3 +36,6 @@ gdb_test "print A1(2)" \
 
 gdb_test "print A1(3)" \
          "\\(i => 2, s => \"AB\"\\)"
+
+gdb_test "print A1(1..3)" \
+         "\\(\\(i => 0, s => \"\"\\), \\(i => 1, s => \"A\"\\), \\(i => 2, s => \"AB\"\\)\\)"
-- 
2.1.4

pushed: [RFA/commit DWARF 1/2] Add support for dynamic DW_AT_byte_stride.

[Joel Brobecker]

Hello,

> gdb/ChangeLog:
> 
>         * gdbtypes.h (enum dynamic_prop_node_kind) <DYN_PROP_BYTE_STRIDE>:
>         New enum value.
>         (create_array_type_with_stride): Add byte_stride_prop parameter.
>         * gdbtypes.c (create_array_type_with_stride) <byte_stride_prop>:
>         New parameter.  Update all callers in this file.
>         (array_type_has_dynamic_stride): New function.
>         (is_dynamic_type_internal, resolve_dynamic_array): Add handling
>         of arrays with dynamic byte strides.
>         * dwarf2read.c (read_array_type): Add support for dynamic
>         DW_AT_byte_stride attributes.
> 
> gdb/testsuite/ChangeLog:
> 
>         * gdb.ada/dyn_stride: New testcase.

I pushed this patch (after having updated the copyright year range
for the new files -- in the new testcase).

-- 
Joel

pushed: [commit/Ada 2/2] slices of arrays with dynamic strides

[Joel Brobecker]

> gdb/ChangeLog:
> 
>         * ada-lang.c (ada_value_slice_from_ptr): Take array stride into
>         account when creating the array type of the slice.
>         (ada_value_slice): Likewise.
> 
> gdb/testsuite/ChangeLog:
> 
>         * gdb.ada/dyn_stride.exp: Add slice test.

Pushed as well :).

-- 
Joel