public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [RFA/DWARF] Add array DW_AT_bit_stride and DW_AT_byte_stride support
@ 2014-01-29 14:30 Joel Brobecker
  2014-02-10 14:29 ` PING: " Joel Brobecker
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Joel Brobecker @ 2014-01-29 14:30 UTC (permalink / raw)
  To: gdb-patches

Hello,

Consider the following declarations in Ada...

   type Item is range -32 .. 31;
   for Item'Size use 6;

   type Table is array (Natural range 0 .. 4) of Item;
   pragma Pack (Table);

... which declare a packed array whose elements are 6 bits long.
The debugger currently does not notice that the array is packed,
and thus prints values of this type incorrectly. This can be seen
in the "ptype" output:

    (gdb) ptype table
    type = array (0 .. 4) of foo.item

Normally, the debugger should print:

    (gdb) ptype table
    type = array (0 .. 4) of foo.item <packed: 6-bit elements>

The debugging information for this array looks like this:

        .uleb128 0xf    # (DIE (0x15c) DW_TAG_array_type)
        .long   .LASF9  # DW_AT_name: "pck__table"
        .byte   0x6     # DW_AT_bit_stride
        .long   0x1a9   # DW_AT_type
        .uleb128 0x10   # (DIE (0x16a) DW_TAG_subrange_type)
        .long   0x3b    # DW_AT_type
        .byte   0       # DW_AT_lower_bound
        .byte   0x4     # DW_AT_upper_bound
        .byte   0       # end of children of DIE 0x15c

The interesting part is the DW_AT_bit_stride attribute, which tells
the size of the array elements is 6 bits, rather than the normal
element type's size.

This patch adds support for this attribute by first creating
gdbtypes.c::create_array_type_with_stride, which is an enhanced
version of create_array_type taking an extra parameter as the stride.
The old create_array_type can then be re-implemented very simply
by calling the new create_array_type_with_stride.

We can then use this new function from dwarf2read, to create
arrays with or without stride.

gdb/ChangeLog:

        * gdbtypes.h (create_array_type_with_stride): Add declaration.
        * gdbtypes.c (create_array_type_with_stride): New function,
        renaming create_array_type, but with an added parameter
        called "bit_stride".
        (create_array_type): Re-implement using
        create_array_type_with_stride.
        * dwarf2read.c (read_array_type): Add support for DW_AT_byte_stride
        and DW_AT_bit_stride attributes.

gdb/testsuite/ChangeLog:

        * gdb.dwarf2/arr-stride.c: New file.
        * gdb.dwarf2/arr-stride.exp: New file.

The test, relying purely on generating an assembly file, only
verifies the type description of our array. But I was also
able to verify manually that the debugger print values of these
types correctly as well (which was not the case prior to this
patch).
---
 gdb/dwarf2read.c                        |  18 +++++-
 gdb/gdbtypes.c                          |  27 +++++++-
 gdb/gdbtypes.h                          |   4 ++
 gdb/testsuite/gdb.dwarf2/arr-stride.c   |  20 ++++++
 gdb/testsuite/gdb.dwarf2/arr-stride.exp | 108 ++++++++++++++++++++++++++++++++
 5 files changed, 171 insertions(+), 6 deletions(-)
 create mode 100644 gdb/testsuite/gdb.dwarf2/arr-stride.c
 create mode 100644 gdb/testsuite/gdb.dwarf2/arr-stride.exp

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 54c538a..1c9c188 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -13254,6 +13254,7 @@ read_array_type (struct die_info *die, struct dwarf2_cu *cu)
   int ndim = 0;
   struct cleanup *back_to;
   const char *name;
+  unsigned int bit_stride = 0;
 
   element_type = die_type (die, cu);
 
@@ -13262,13 +13263,22 @@ read_array_type (struct die_info *die, struct dwarf2_cu *cu)
   if (type)
     return type;
 
+  attr = dwarf2_attr (die, DW_AT_byte_stride, cu);
+  if (attr != NULL)
+    bit_stride = DW_UNSND (attr) * 8;
+
+  attr = dwarf2_attr (die, DW_AT_bit_stride, cu);
+  if (attr != NULL)
+    bit_stride = DW_UNSND (attr);
+
   /* Irix 6.2 native cc creates array types without children for
      arrays with unspecified length.  */
   if (die->child == NULL)
     {
       index_type = objfile_type (objfile)->builtin_int;
       range_type = create_range_type (NULL, index_type, 0, -1);
-      type = create_array_type (NULL, element_type, range_type);
+      type = create_array_type_with_stride (NULL, element_type, range_type,
+					    bit_stride);
       return set_die_type (die, type, cu);
     }
 
@@ -13308,12 +13318,14 @@ read_array_type (struct die_info *die, struct dwarf2_cu *cu)
       int i = 0;
 
       while (i < ndim)
-	type = create_array_type (NULL, type, range_types[i++]);
+	type = create_array_type_with_stride (NULL, type, range_types[i++],
+					      bit_stride);
     }
   else
     {
       while (ndim-- > 0)
-	type = create_array_type (NULL, type, range_types[ndim]);
+	type = create_array_type_with_stride (NULL, type, range_types[ndim],
+					      bit_stride);
     }
 
   /* Understand Dwarf2 support for vector types (like they occur on
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 042c17d..03d7ba9 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -951,14 +951,18 @@ get_array_bounds (struct type *type, LONGEST *low_bound, LONGEST *high_bound)
    Elements will be of type ELEMENT_TYPE, the indices will be of type
    RANGE_TYPE.
 
+   If BIT_STRIDE is non-null, build a packed array type whose element
+   size is BIT_STRIDE.  Otherwise, ignore this parameter.
+
    FIXME: Maybe we should check the TYPE_CODE of RESULT_TYPE to make
    sure it is TYPE_CODE_UNDEF before we bash it into an array
    type?  */
 
 struct type *
-create_array_type (struct type *result_type, 
-		   struct type *element_type,
-		   struct type *range_type)
+create_array_type_with_stride (struct type *result_type,
+			       struct type *element_type,
+			       struct type *range_type,
+			       unsigned int bit_stride)
 {
   LONGEST low_bound, high_bound;
 
@@ -975,6 +979,9 @@ create_array_type (struct type *result_type,
      In such cases, the array length should be zero.  */
   if (high_bound < low_bound)
     TYPE_LENGTH (result_type) = 0;
+  else if (bit_stride > 0)
+    TYPE_LENGTH (result_type) =
+      (bit_stride * (high_bound - low_bound + 1) + 7) / 8;
   else
     TYPE_LENGTH (result_type) =
       TYPE_LENGTH (element_type) * (high_bound - low_bound + 1);
@@ -983,6 +990,8 @@ create_array_type (struct type *result_type,
     (struct field *) TYPE_ZALLOC (result_type, sizeof (struct field));
   TYPE_INDEX_TYPE (result_type) = range_type;
   TYPE_VPTR_FIELDNO (result_type) = -1;
+  if (bit_stride > 0)
+    TYPE_FIELD_BITSIZE (result_type, 0) = bit_stride;
 
   /* TYPE_FLAG_TARGET_STUB will take care of zero length arrays.  */
   if (TYPE_LENGTH (result_type) == 0)
@@ -991,6 +1000,18 @@ create_array_type (struct type *result_type,
   return result_type;
 }
 
+/* Same as create_array_type_with_stride but with no bit_stride
+   (BIT_STRIDE = -1), thus building an unpacked array.  */
+
+struct type *
+create_array_type (struct type *result_type,
+		   struct type *element_type,
+		   struct type *range_type)
+{
+  return create_array_type_with_stride (result_type, element_type,
+					range_type, 0);
+}
+
 struct type *
 lookup_array_range_type (struct type *element_type,
 			 LONGEST low_bound, LONGEST high_bound)
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 61ddeff..a1d9921 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -1529,8 +1529,12 @@ extern struct type *lookup_function_type_with_arguments (struct type *,
 extern struct type *create_range_type (struct type *, struct type *, LONGEST,
 				       LONGEST);
 
+extern struct type *create_array_type_with_stride
+  (struct type *, struct type *, struct type *, unsigned int);
+
 extern struct type *create_array_type (struct type *, struct type *,
 				       struct type *);
+
 extern struct type *lookup_array_range_type (struct type *, LONGEST, LONGEST);
 
 extern struct type *create_string_type (struct type *, struct type *,
diff --git a/gdb/testsuite/gdb.dwarf2/arr-stride.c b/gdb/testsuite/gdb.dwarf2/arr-stride.c
new file mode 100644
index 0000000..978ef8d
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/arr-stride.c
@@ -0,0 +1,20 @@
+/* Copyright 2014 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/>.  */
+
+int
+main (void)
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.dwarf2/arr-stride.exp b/gdb/testsuite/gdb.dwarf2/arr-stride.exp
new file mode 100644
index 0000000..f2a1f39
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/arr-stride.exp
@@ -0,0 +1,108 @@
+# Copyright 2014 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 dwarf.exp
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+if {![dwarf2_support]} {
+    return 0
+}
+
+standard_testfile arr-stride.c arr-stride-dw.S
+
+# Make some DWARF for the test.
+set asm_file [standard_output_file $srcfile2]
+Dwarf::assemble $asm_file {
+    cu {} {
+ 	DW_TAG_compile_unit {
+                {DW_AT_language @DW_LANG_Ada95}
+                {DW_AT_name     foo.adb}
+                {DW_AT_comp_dir /tmp}
+        } {
+	    declare_labels integer_label array_elt_label array_label \
+                big_array_label
+
+            integer_label: DW_TAG_base_type {
+                {DW_AT_byte_size 4 DW_FORM_sdata}
+                {DW_AT_encoding  @DW_ATE_signed}
+                {DW_AT_name      integer}
+            }
+
+            array_elt_label: DW_TAG_subrange_type {
+                {DW_AT_lower_bound 0xe0 DW_FORM_data1}
+                {DW_AT_upper_bound 0x1f DW_FORM_data1}
+                {DW_AT_name        pck__item}
+                {DW_AT_type        :$integer_label}
+            }
+
+            DW_TAG_typedef {
+                {DW_AT_name pck__table}
+                {DW_AT_type :$array_label}
+            }
+
+	    array_label: DW_TAG_array_type {
+		{DW_AT_name pck__table}
+                {DW_AT_bit_stride 6 DW_FORM_data1}
+		{DW_AT_type :$array_elt_label}
+	    } {
+		DW_TAG_subrange_type {
+		    {DW_AT_type        :$integer_label}
+		    {DW_AT_lower_bound 0 DW_FORM_data1}
+		    {DW_AT_upper_bound 4 DW_FORM_data1}
+		}
+	    }
+
+            DW_TAG_typedef {
+                {DW_AT_name pck__big_table}
+                {DW_AT_type :$big_array_label}
+            }
+
+	    big_array_label: DW_TAG_array_type {
+		{DW_AT_name pck__big_table}
+                {DW_AT_byte_stride 1 DW_FORM_data1}
+		{DW_AT_type :$array_elt_label}
+	    } {
+		DW_TAG_subrange_type {
+		    {DW_AT_type        :$integer_label}
+		    {DW_AT_lower_bound 0 DW_FORM_data1}
+		    {DW_AT_upper_bound 4 DW_FORM_data1}
+		}
+	    }
+	}
+    }
+}
+
+if  {[gdb_compile ${srcdir}/${subdir}/${srcfile} ${binfile}1.o \
+	  object {nodebug}] != ""} {
+    return -1
+}
+
+if  {[gdb_compile $asm_file ${binfile}2.o object {nodebug}] != ""} {
+    return -1
+}
+
+if  {[gdb_compile [list ${binfile}1.o ${binfile}2.o] \
+	  "${binfile}" executable {}] != ""} {
+    return -1
+}
+
+clean_restart ${testfile}
+
+gdb_test_no_output "set language ada"
+
+gdb_test "ptype pck.table" \
+         "type = array \\(0 \\.\\. 4\\) of pck\\.item <packed: 6-bit elements>"
+
+gdb_test "ptype pck.big_table" \
+         "type = array \\(0 \\.\\. 4\\) of pck\\.item <packed: 8-bit elements>"
-- 
1.8.3.2

^ permalink raw reply	[flat|nested] 7+ messages in thread

* PING: [RFA/DWARF] Add array DW_AT_bit_stride and DW_AT_byte_stride support
  2014-01-29 14:30 [RFA/DWARF] Add array DW_AT_bit_stride and DW_AT_byte_stride support Joel Brobecker
@ 2014-02-10 14:29 ` Joel Brobecker
  2014-02-17 20:20 ` Joel Brobecker
  2014-02-20 12:16 ` Mark Wielaard
  2 siblings, 0 replies; 7+ messages in thread
From: Joel Brobecker @ 2014-02-10 14:29 UTC (permalink / raw)
  To: gdb-patches

Ping?

Thank you!

On Wed, Jan 29, 2014 at 06:30:30PM +0400, Joel Brobecker wrote:
> Hello,
> 
> Consider the following declarations in Ada...
> 
>    type Item is range -32 .. 31;
>    for Item'Size use 6;
> 
>    type Table is array (Natural range 0 .. 4) of Item;
>    pragma Pack (Table);
> 
> ... which declare a packed array whose elements are 6 bits long.
> The debugger currently does not notice that the array is packed,
> and thus prints values of this type incorrectly. This can be seen
> in the "ptype" output:
> 
>     (gdb) ptype table
>     type = array (0 .. 4) of foo.item
> 
> Normally, the debugger should print:
> 
>     (gdb) ptype table
>     type = array (0 .. 4) of foo.item <packed: 6-bit elements>
> 
> The debugging information for this array looks like this:
> 
>         .uleb128 0xf    # (DIE (0x15c) DW_TAG_array_type)
>         .long   .LASF9  # DW_AT_name: "pck__table"
>         .byte   0x6     # DW_AT_bit_stride
>         .long   0x1a9   # DW_AT_type
>         .uleb128 0x10   # (DIE (0x16a) DW_TAG_subrange_type)
>         .long   0x3b    # DW_AT_type
>         .byte   0       # DW_AT_lower_bound
>         .byte   0x4     # DW_AT_upper_bound
>         .byte   0       # end of children of DIE 0x15c
> 
> The interesting part is the DW_AT_bit_stride attribute, which tells
> the size of the array elements is 6 bits, rather than the normal
> element type's size.
> 
> This patch adds support for this attribute by first creating
> gdbtypes.c::create_array_type_with_stride, which is an enhanced
> version of create_array_type taking an extra parameter as the stride.
> The old create_array_type can then be re-implemented very simply
> by calling the new create_array_type_with_stride.
> 
> We can then use this new function from dwarf2read, to create
> arrays with or without stride.
> 
> gdb/ChangeLog:
> 
>         * gdbtypes.h (create_array_type_with_stride): Add declaration.
>         * gdbtypes.c (create_array_type_with_stride): New function,
>         renaming create_array_type, but with an added parameter
>         called "bit_stride".
>         (create_array_type): Re-implement using
>         create_array_type_with_stride.
>         * dwarf2read.c (read_array_type): Add support for DW_AT_byte_stride
>         and DW_AT_bit_stride attributes.
> 
> gdb/testsuite/ChangeLog:
> 
>         * gdb.dwarf2/arr-stride.c: New file.
>         * gdb.dwarf2/arr-stride.exp: New file.
> 
> The test, relying purely on generating an assembly file, only
> verifies the type description of our array. But I was also
> able to verify manually that the debugger print values of these
> types correctly as well (which was not the case prior to this
> patch).
> ---
>  gdb/dwarf2read.c                        |  18 +++++-
>  gdb/gdbtypes.c                          |  27 +++++++-
>  gdb/gdbtypes.h                          |   4 ++
>  gdb/testsuite/gdb.dwarf2/arr-stride.c   |  20 ++++++
>  gdb/testsuite/gdb.dwarf2/arr-stride.exp | 108 ++++++++++++++++++++++++++++++++
>  5 files changed, 171 insertions(+), 6 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.dwarf2/arr-stride.c
>  create mode 100644 gdb/testsuite/gdb.dwarf2/arr-stride.exp
> 
> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index 54c538a..1c9c188 100644
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -13254,6 +13254,7 @@ read_array_type (struct die_info *die, struct dwarf2_cu *cu)
>    int ndim = 0;
>    struct cleanup *back_to;
>    const char *name;
> +  unsigned int bit_stride = 0;
>  
>    element_type = die_type (die, cu);
>  
> @@ -13262,13 +13263,22 @@ read_array_type (struct die_info *die, struct dwarf2_cu *cu)
>    if (type)
>      return type;
>  
> +  attr = dwarf2_attr (die, DW_AT_byte_stride, cu);
> +  if (attr != NULL)
> +    bit_stride = DW_UNSND (attr) * 8;
> +
> +  attr = dwarf2_attr (die, DW_AT_bit_stride, cu);
> +  if (attr != NULL)
> +    bit_stride = DW_UNSND (attr);
> +
>    /* Irix 6.2 native cc creates array types without children for
>       arrays with unspecified length.  */
>    if (die->child == NULL)
>      {
>        index_type = objfile_type (objfile)->builtin_int;
>        range_type = create_range_type (NULL, index_type, 0, -1);
> -      type = create_array_type (NULL, element_type, range_type);
> +      type = create_array_type_with_stride (NULL, element_type, range_type,
> +					    bit_stride);
>        return set_die_type (die, type, cu);
>      }
>  
> @@ -13308,12 +13318,14 @@ read_array_type (struct die_info *die, struct dwarf2_cu *cu)
>        int i = 0;
>  
>        while (i < ndim)
> -	type = create_array_type (NULL, type, range_types[i++]);
> +	type = create_array_type_with_stride (NULL, type, range_types[i++],
> +					      bit_stride);
>      }
>    else
>      {
>        while (ndim-- > 0)
> -	type = create_array_type (NULL, type, range_types[ndim]);
> +	type = create_array_type_with_stride (NULL, type, range_types[ndim],
> +					      bit_stride);
>      }
>  
>    /* Understand Dwarf2 support for vector types (like they occur on
> diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
> index 042c17d..03d7ba9 100644
> --- a/gdb/gdbtypes.c
> +++ b/gdb/gdbtypes.c
> @@ -951,14 +951,18 @@ get_array_bounds (struct type *type, LONGEST *low_bound, LONGEST *high_bound)
>     Elements will be of type ELEMENT_TYPE, the indices will be of type
>     RANGE_TYPE.
>  
> +   If BIT_STRIDE is non-null, build a packed array type whose element
> +   size is BIT_STRIDE.  Otherwise, ignore this parameter.
> +
>     FIXME: Maybe we should check the TYPE_CODE of RESULT_TYPE to make
>     sure it is TYPE_CODE_UNDEF before we bash it into an array
>     type?  */
>  
>  struct type *
> -create_array_type (struct type *result_type, 
> -		   struct type *element_type,
> -		   struct type *range_type)
> +create_array_type_with_stride (struct type *result_type,
> +			       struct type *element_type,
> +			       struct type *range_type,
> +			       unsigned int bit_stride)
>  {
>    LONGEST low_bound, high_bound;
>  
> @@ -975,6 +979,9 @@ create_array_type (struct type *result_type,
>       In such cases, the array length should be zero.  */
>    if (high_bound < low_bound)
>      TYPE_LENGTH (result_type) = 0;
> +  else if (bit_stride > 0)
> +    TYPE_LENGTH (result_type) =
> +      (bit_stride * (high_bound - low_bound + 1) + 7) / 8;
>    else
>      TYPE_LENGTH (result_type) =
>        TYPE_LENGTH (element_type) * (high_bound - low_bound + 1);
> @@ -983,6 +990,8 @@ create_array_type (struct type *result_type,
>      (struct field *) TYPE_ZALLOC (result_type, sizeof (struct field));
>    TYPE_INDEX_TYPE (result_type) = range_type;
>    TYPE_VPTR_FIELDNO (result_type) = -1;
> +  if (bit_stride > 0)
> +    TYPE_FIELD_BITSIZE (result_type, 0) = bit_stride;
>  
>    /* TYPE_FLAG_TARGET_STUB will take care of zero length arrays.  */
>    if (TYPE_LENGTH (result_type) == 0)
> @@ -991,6 +1000,18 @@ create_array_type (struct type *result_type,
>    return result_type;
>  }
>  
> +/* Same as create_array_type_with_stride but with no bit_stride
> +   (BIT_STRIDE = -1), thus building an unpacked array.  */
> +
> +struct type *
> +create_array_type (struct type *result_type,
> +		   struct type *element_type,
> +		   struct type *range_type)
> +{
> +  return create_array_type_with_stride (result_type, element_type,
> +					range_type, 0);
> +}
> +
>  struct type *
>  lookup_array_range_type (struct type *element_type,
>  			 LONGEST low_bound, LONGEST high_bound)
> diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
> index 61ddeff..a1d9921 100644
> --- a/gdb/gdbtypes.h
> +++ b/gdb/gdbtypes.h
> @@ -1529,8 +1529,12 @@ extern struct type *lookup_function_type_with_arguments (struct type *,
>  extern struct type *create_range_type (struct type *, struct type *, LONGEST,
>  				       LONGEST);
>  
> +extern struct type *create_array_type_with_stride
> +  (struct type *, struct type *, struct type *, unsigned int);
> +
>  extern struct type *create_array_type (struct type *, struct type *,
>  				       struct type *);
> +
>  extern struct type *lookup_array_range_type (struct type *, LONGEST, LONGEST);
>  
>  extern struct type *create_string_type (struct type *, struct type *,
> diff --git a/gdb/testsuite/gdb.dwarf2/arr-stride.c b/gdb/testsuite/gdb.dwarf2/arr-stride.c
> new file mode 100644
> index 0000000..978ef8d
> --- /dev/null
> +++ b/gdb/testsuite/gdb.dwarf2/arr-stride.c
> @@ -0,0 +1,20 @@
> +/* Copyright 2014 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/>.  */
> +
> +int
> +main (void)
> +{
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.dwarf2/arr-stride.exp b/gdb/testsuite/gdb.dwarf2/arr-stride.exp
> new file mode 100644
> index 0000000..f2a1f39
> --- /dev/null
> +++ b/gdb/testsuite/gdb.dwarf2/arr-stride.exp
> @@ -0,0 +1,108 @@
> +# Copyright 2014 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 dwarf.exp
> +
> +# This test can only be run on targets which support DWARF-2 and use gas.
> +if {![dwarf2_support]} {
> +    return 0
> +}
> +
> +standard_testfile arr-stride.c arr-stride-dw.S
> +
> +# Make some DWARF for the test.
> +set asm_file [standard_output_file $srcfile2]
> +Dwarf::assemble $asm_file {
> +    cu {} {
> + 	DW_TAG_compile_unit {
> +                {DW_AT_language @DW_LANG_Ada95}
> +                {DW_AT_name     foo.adb}
> +                {DW_AT_comp_dir /tmp}
> +        } {
> +	    declare_labels integer_label array_elt_label array_label \
> +                big_array_label
> +
> +            integer_label: DW_TAG_base_type {
> +                {DW_AT_byte_size 4 DW_FORM_sdata}
> +                {DW_AT_encoding  @DW_ATE_signed}
> +                {DW_AT_name      integer}
> +            }
> +
> +            array_elt_label: DW_TAG_subrange_type {
> +                {DW_AT_lower_bound 0xe0 DW_FORM_data1}
> +                {DW_AT_upper_bound 0x1f DW_FORM_data1}
> +                {DW_AT_name        pck__item}
> +                {DW_AT_type        :$integer_label}
> +            }
> +
> +            DW_TAG_typedef {
> +                {DW_AT_name pck__table}
> +                {DW_AT_type :$array_label}
> +            }
> +
> +	    array_label: DW_TAG_array_type {
> +		{DW_AT_name pck__table}
> +                {DW_AT_bit_stride 6 DW_FORM_data1}
> +		{DW_AT_type :$array_elt_label}
> +	    } {
> +		DW_TAG_subrange_type {
> +		    {DW_AT_type        :$integer_label}
> +		    {DW_AT_lower_bound 0 DW_FORM_data1}
> +		    {DW_AT_upper_bound 4 DW_FORM_data1}
> +		}
> +	    }
> +
> +            DW_TAG_typedef {
> +                {DW_AT_name pck__big_table}
> +                {DW_AT_type :$big_array_label}
> +            }
> +
> +	    big_array_label: DW_TAG_array_type {
> +		{DW_AT_name pck__big_table}
> +                {DW_AT_byte_stride 1 DW_FORM_data1}
> +		{DW_AT_type :$array_elt_label}
> +	    } {
> +		DW_TAG_subrange_type {
> +		    {DW_AT_type        :$integer_label}
> +		    {DW_AT_lower_bound 0 DW_FORM_data1}
> +		    {DW_AT_upper_bound 4 DW_FORM_data1}
> +		}
> +	    }
> +	}
> +    }
> +}
> +
> +if  {[gdb_compile ${srcdir}/${subdir}/${srcfile} ${binfile}1.o \
> +	  object {nodebug}] != ""} {
> +    return -1
> +}
> +
> +if  {[gdb_compile $asm_file ${binfile}2.o object {nodebug}] != ""} {
> +    return -1
> +}
> +
> +if  {[gdb_compile [list ${binfile}1.o ${binfile}2.o] \
> +	  "${binfile}" executable {}] != ""} {
> +    return -1
> +}
> +
> +clean_restart ${testfile}
> +
> +gdb_test_no_output "set language ada"
> +
> +gdb_test "ptype pck.table" \
> +         "type = array \\(0 \\.\\. 4\\) of pck\\.item <packed: 6-bit elements>"
> +
> +gdb_test "ptype pck.big_table" \
> +         "type = array \\(0 \\.\\. 4\\) of pck\\.item <packed: 8-bit elements>"
> -- 
> 1.8.3.2

-- 
Joel

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFA/DWARF] Add array DW_AT_bit_stride and DW_AT_byte_stride support
  2014-01-29 14:30 [RFA/DWARF] Add array DW_AT_bit_stride and DW_AT_byte_stride support Joel Brobecker
  2014-02-10 14:29 ` PING: " Joel Brobecker
@ 2014-02-17 20:20 ` Joel Brobecker
  2014-02-20 12:16 ` Mark Wielaard
  2 siblings, 0 replies; 7+ messages in thread
From: Joel Brobecker @ 2014-02-17 20:20 UTC (permalink / raw)
  To: gdb-patches

Ping?

Thank you :)

On Wed, Jan 29, 2014 at 06:30:30PM +0400, Joel Brobecker wrote:
> Hello,
> 
> Consider the following declarations in Ada...
> 
>    type Item is range -32 .. 31;
>    for Item'Size use 6;
> 
>    type Table is array (Natural range 0 .. 4) of Item;
>    pragma Pack (Table);
> 
> ... which declare a packed array whose elements are 6 bits long.
> The debugger currently does not notice that the array is packed,
> and thus prints values of this type incorrectly. This can be seen
> in the "ptype" output:
> 
>     (gdb) ptype table
>     type = array (0 .. 4) of foo.item
> 
> Normally, the debugger should print:
> 
>     (gdb) ptype table
>     type = array (0 .. 4) of foo.item <packed: 6-bit elements>
> 
> The debugging information for this array looks like this:
> 
>         .uleb128 0xf    # (DIE (0x15c) DW_TAG_array_type)
>         .long   .LASF9  # DW_AT_name: "pck__table"
>         .byte   0x6     # DW_AT_bit_stride
>         .long   0x1a9   # DW_AT_type
>         .uleb128 0x10   # (DIE (0x16a) DW_TAG_subrange_type)
>         .long   0x3b    # DW_AT_type
>         .byte   0       # DW_AT_lower_bound
>         .byte   0x4     # DW_AT_upper_bound
>         .byte   0       # end of children of DIE 0x15c
> 
> The interesting part is the DW_AT_bit_stride attribute, which tells
> the size of the array elements is 6 bits, rather than the normal
> element type's size.
> 
> This patch adds support for this attribute by first creating
> gdbtypes.c::create_array_type_with_stride, which is an enhanced
> version of create_array_type taking an extra parameter as the stride.
> The old create_array_type can then be re-implemented very simply
> by calling the new create_array_type_with_stride.
> 
> We can then use this new function from dwarf2read, to create
> arrays with or without stride.
> 
> gdb/ChangeLog:
> 
>         * gdbtypes.h (create_array_type_with_stride): Add declaration.
>         * gdbtypes.c (create_array_type_with_stride): New function,
>         renaming create_array_type, but with an added parameter
>         called "bit_stride".
>         (create_array_type): Re-implement using
>         create_array_type_with_stride.
>         * dwarf2read.c (read_array_type): Add support for DW_AT_byte_stride
>         and DW_AT_bit_stride attributes.
> 
> gdb/testsuite/ChangeLog:
> 
>         * gdb.dwarf2/arr-stride.c: New file.
>         * gdb.dwarf2/arr-stride.exp: New file.
> 
> The test, relying purely on generating an assembly file, only
> verifies the type description of our array. But I was also
> able to verify manually that the debugger print values of these
> types correctly as well (which was not the case prior to this
> patch).
> ---
>  gdb/dwarf2read.c                        |  18 +++++-
>  gdb/gdbtypes.c                          |  27 +++++++-
>  gdb/gdbtypes.h                          |   4 ++
>  gdb/testsuite/gdb.dwarf2/arr-stride.c   |  20 ++++++
>  gdb/testsuite/gdb.dwarf2/arr-stride.exp | 108 ++++++++++++++++++++++++++++++++
>  5 files changed, 171 insertions(+), 6 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.dwarf2/arr-stride.c
>  create mode 100644 gdb/testsuite/gdb.dwarf2/arr-stride.exp
> 
> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index 54c538a..1c9c188 100644
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -13254,6 +13254,7 @@ read_array_type (struct die_info *die, struct dwarf2_cu *cu)
>    int ndim = 0;
>    struct cleanup *back_to;
>    const char *name;
> +  unsigned int bit_stride = 0;
>  
>    element_type = die_type (die, cu);
>  
> @@ -13262,13 +13263,22 @@ read_array_type (struct die_info *die, struct dwarf2_cu *cu)
>    if (type)
>      return type;
>  
> +  attr = dwarf2_attr (die, DW_AT_byte_stride, cu);
> +  if (attr != NULL)
> +    bit_stride = DW_UNSND (attr) * 8;
> +
> +  attr = dwarf2_attr (die, DW_AT_bit_stride, cu);
> +  if (attr != NULL)
> +    bit_stride = DW_UNSND (attr);
> +
>    /* Irix 6.2 native cc creates array types without children for
>       arrays with unspecified length.  */
>    if (die->child == NULL)
>      {
>        index_type = objfile_type (objfile)->builtin_int;
>        range_type = create_range_type (NULL, index_type, 0, -1);
> -      type = create_array_type (NULL, element_type, range_type);
> +      type = create_array_type_with_stride (NULL, element_type, range_type,
> +					    bit_stride);
>        return set_die_type (die, type, cu);
>      }
>  
> @@ -13308,12 +13318,14 @@ read_array_type (struct die_info *die, struct dwarf2_cu *cu)
>        int i = 0;
>  
>        while (i < ndim)
> -	type = create_array_type (NULL, type, range_types[i++]);
> +	type = create_array_type_with_stride (NULL, type, range_types[i++],
> +					      bit_stride);
>      }
>    else
>      {
>        while (ndim-- > 0)
> -	type = create_array_type (NULL, type, range_types[ndim]);
> +	type = create_array_type_with_stride (NULL, type, range_types[ndim],
> +					      bit_stride);
>      }
>  
>    /* Understand Dwarf2 support for vector types (like they occur on
> diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
> index 042c17d..03d7ba9 100644
> --- a/gdb/gdbtypes.c
> +++ b/gdb/gdbtypes.c
> @@ -951,14 +951,18 @@ get_array_bounds (struct type *type, LONGEST *low_bound, LONGEST *high_bound)
>     Elements will be of type ELEMENT_TYPE, the indices will be of type
>     RANGE_TYPE.
>  
> +   If BIT_STRIDE is non-null, build a packed array type whose element
> +   size is BIT_STRIDE.  Otherwise, ignore this parameter.
> +
>     FIXME: Maybe we should check the TYPE_CODE of RESULT_TYPE to make
>     sure it is TYPE_CODE_UNDEF before we bash it into an array
>     type?  */
>  
>  struct type *
> -create_array_type (struct type *result_type, 
> -		   struct type *element_type,
> -		   struct type *range_type)
> +create_array_type_with_stride (struct type *result_type,
> +			       struct type *element_type,
> +			       struct type *range_type,
> +			       unsigned int bit_stride)
>  {
>    LONGEST low_bound, high_bound;
>  
> @@ -975,6 +979,9 @@ create_array_type (struct type *result_type,
>       In such cases, the array length should be zero.  */
>    if (high_bound < low_bound)
>      TYPE_LENGTH (result_type) = 0;
> +  else if (bit_stride > 0)
> +    TYPE_LENGTH (result_type) =
> +      (bit_stride * (high_bound - low_bound + 1) + 7) / 8;
>    else
>      TYPE_LENGTH (result_type) =
>        TYPE_LENGTH (element_type) * (high_bound - low_bound + 1);
> @@ -983,6 +990,8 @@ create_array_type (struct type *result_type,
>      (struct field *) TYPE_ZALLOC (result_type, sizeof (struct field));
>    TYPE_INDEX_TYPE (result_type) = range_type;
>    TYPE_VPTR_FIELDNO (result_type) = -1;
> +  if (bit_stride > 0)
> +    TYPE_FIELD_BITSIZE (result_type, 0) = bit_stride;
>  
>    /* TYPE_FLAG_TARGET_STUB will take care of zero length arrays.  */
>    if (TYPE_LENGTH (result_type) == 0)
> @@ -991,6 +1000,18 @@ create_array_type (struct type *result_type,
>    return result_type;
>  }
>  
> +/* Same as create_array_type_with_stride but with no bit_stride
> +   (BIT_STRIDE = -1), thus building an unpacked array.  */
> +
> +struct type *
> +create_array_type (struct type *result_type,
> +		   struct type *element_type,
> +		   struct type *range_type)
> +{
> +  return create_array_type_with_stride (result_type, element_type,
> +					range_type, 0);
> +}
> +
>  struct type *
>  lookup_array_range_type (struct type *element_type,
>  			 LONGEST low_bound, LONGEST high_bound)
> diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
> index 61ddeff..a1d9921 100644
> --- a/gdb/gdbtypes.h
> +++ b/gdb/gdbtypes.h
> @@ -1529,8 +1529,12 @@ extern struct type *lookup_function_type_with_arguments (struct type *,
>  extern struct type *create_range_type (struct type *, struct type *, LONGEST,
>  				       LONGEST);
>  
> +extern struct type *create_array_type_with_stride
> +  (struct type *, struct type *, struct type *, unsigned int);
> +
>  extern struct type *create_array_type (struct type *, struct type *,
>  				       struct type *);
> +
>  extern struct type *lookup_array_range_type (struct type *, LONGEST, LONGEST);
>  
>  extern struct type *create_string_type (struct type *, struct type *,
> diff --git a/gdb/testsuite/gdb.dwarf2/arr-stride.c b/gdb/testsuite/gdb.dwarf2/arr-stride.c
> new file mode 100644
> index 0000000..978ef8d
> --- /dev/null
> +++ b/gdb/testsuite/gdb.dwarf2/arr-stride.c
> @@ -0,0 +1,20 @@
> +/* Copyright 2014 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/>.  */
> +
> +int
> +main (void)
> +{
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.dwarf2/arr-stride.exp b/gdb/testsuite/gdb.dwarf2/arr-stride.exp
> new file mode 100644
> index 0000000..f2a1f39
> --- /dev/null
> +++ b/gdb/testsuite/gdb.dwarf2/arr-stride.exp
> @@ -0,0 +1,108 @@
> +# Copyright 2014 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 dwarf.exp
> +
> +# This test can only be run on targets which support DWARF-2 and use gas.
> +if {![dwarf2_support]} {
> +    return 0
> +}
> +
> +standard_testfile arr-stride.c arr-stride-dw.S
> +
> +# Make some DWARF for the test.
> +set asm_file [standard_output_file $srcfile2]
> +Dwarf::assemble $asm_file {
> +    cu {} {
> + 	DW_TAG_compile_unit {
> +                {DW_AT_language @DW_LANG_Ada95}
> +                {DW_AT_name     foo.adb}
> +                {DW_AT_comp_dir /tmp}
> +        } {
> +	    declare_labels integer_label array_elt_label array_label \
> +                big_array_label
> +
> +            integer_label: DW_TAG_base_type {
> +                {DW_AT_byte_size 4 DW_FORM_sdata}
> +                {DW_AT_encoding  @DW_ATE_signed}
> +                {DW_AT_name      integer}
> +            }
> +
> +            array_elt_label: DW_TAG_subrange_type {
> +                {DW_AT_lower_bound 0xe0 DW_FORM_data1}
> +                {DW_AT_upper_bound 0x1f DW_FORM_data1}
> +                {DW_AT_name        pck__item}
> +                {DW_AT_type        :$integer_label}
> +            }
> +
> +            DW_TAG_typedef {
> +                {DW_AT_name pck__table}
> +                {DW_AT_type :$array_label}
> +            }
> +
> +	    array_label: DW_TAG_array_type {
> +		{DW_AT_name pck__table}
> +                {DW_AT_bit_stride 6 DW_FORM_data1}
> +		{DW_AT_type :$array_elt_label}
> +	    } {
> +		DW_TAG_subrange_type {
> +		    {DW_AT_type        :$integer_label}
> +		    {DW_AT_lower_bound 0 DW_FORM_data1}
> +		    {DW_AT_upper_bound 4 DW_FORM_data1}
> +		}
> +	    }
> +
> +            DW_TAG_typedef {
> +                {DW_AT_name pck__big_table}
> +                {DW_AT_type :$big_array_label}
> +            }
> +
> +	    big_array_label: DW_TAG_array_type {
> +		{DW_AT_name pck__big_table}
> +                {DW_AT_byte_stride 1 DW_FORM_data1}
> +		{DW_AT_type :$array_elt_label}
> +	    } {
> +		DW_TAG_subrange_type {
> +		    {DW_AT_type        :$integer_label}
> +		    {DW_AT_lower_bound 0 DW_FORM_data1}
> +		    {DW_AT_upper_bound 4 DW_FORM_data1}
> +		}
> +	    }
> +	}
> +    }
> +}
> +
> +if  {[gdb_compile ${srcdir}/${subdir}/${srcfile} ${binfile}1.o \
> +	  object {nodebug}] != ""} {
> +    return -1
> +}
> +
> +if  {[gdb_compile $asm_file ${binfile}2.o object {nodebug}] != ""} {
> +    return -1
> +}
> +
> +if  {[gdb_compile [list ${binfile}1.o ${binfile}2.o] \
> +	  "${binfile}" executable {}] != ""} {
> +    return -1
> +}
> +
> +clean_restart ${testfile}
> +
> +gdb_test_no_output "set language ada"
> +
> +gdb_test "ptype pck.table" \
> +         "type = array \\(0 \\.\\. 4\\) of pck\\.item <packed: 6-bit elements>"
> +
> +gdb_test "ptype pck.big_table" \
> +         "type = array \\(0 \\.\\. 4\\) of pck\\.item <packed: 8-bit elements>"
> -- 
> 1.8.3.2

-- 
Joel

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFA/DWARF] Add array DW_AT_bit_stride and DW_AT_byte_stride support
  2014-01-29 14:30 [RFA/DWARF] Add array DW_AT_bit_stride and DW_AT_byte_stride support Joel Brobecker
  2014-02-10 14:29 ` PING: " Joel Brobecker
  2014-02-17 20:20 ` Joel Brobecker
@ 2014-02-20 12:16 ` Mark Wielaard
  2014-02-21  9:31   ` Joel Brobecker
  2 siblings, 1 reply; 7+ messages in thread
From: Mark Wielaard @ 2014-02-20 12:16 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

On Wed, 2014-01-29 at 18:30 +0400, Joel Brobecker wrote:
> gdb/ChangeLog:
> 
>         * gdbtypes.h (create_array_type_with_stride): Add declaration.
>         * gdbtypes.c (create_array_type_with_stride): New function,
>         renaming create_array_type, but with an added parameter
>         called "bit_stride".
>         (create_array_type): Re-implement using
>         create_array_type_with_stride.
>         * dwarf2read.c (read_array_type): Add support for DW_AT_byte_stride
>         and DW_AT_bit_stride attributes.
> 
> gdb/testsuite/ChangeLog:
> 
>         * gdb.dwarf2/arr-stride.c: New file.
>         * gdb.dwarf2/arr-stride.exp: New file.

I know little of the GDB code base, just looking at it because it using
some interesting DWARF, but this patch does look fine to me. Just a few
small nitpicks below.

> +++ b/gdb/gdbtypes.c
> @@ -951,14 +951,18 @@ get_array_bounds (struct type *type, LONGEST *low_bound, LONGEST *high_bound)
>     Elements will be of type ELEMENT_TYPE, the indices will be of type
>     RANGE_TYPE.
>  
> +   If BIT_STRIDE is non-null, build a packed array type whose element
> +   size is BIT_STRIDE.  Otherwise, ignore this parameter.

s/non-null/not zero/ ?

> +/* Same as create_array_type_with_stride but with no bit_stride
> +   (BIT_STRIDE = -1), thus building an unpacked array.  */

Shouldn't that be BIT_STRIDE = 0?

> diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
> +extern struct type *create_array_type_with_stride
> +  (struct type *, struct type *, struct type *, unsigned int);
> +
>  extern struct type *create_array_type (struct type *, struct type *,
>  				       struct type *);

Personally I would have just added the new argument and fixed up all
callers, just so I had an overview of all the callers that might have a
bit_stride concept that could be added.

Cheers,

Mark

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFA/DWARF] Add array DW_AT_bit_stride and DW_AT_byte_stride support
  2014-02-20 12:16 ` Mark Wielaard
@ 2014-02-21  9:31   ` Joel Brobecker
  2014-02-26 11:12     ` Mark Wielaard
  0 siblings, 1 reply; 7+ messages in thread
From: Joel Brobecker @ 2014-02-21  9:31 UTC (permalink / raw)
  To: Mark Wielaard; +Cc: gdb-patches

> On Wed, 2014-01-29 at 18:30 +0400, Joel Brobecker wrote:
> > gdb/ChangeLog:
> > 
> >         * gdbtypes.h (create_array_type_with_stride): Add declaration.
> >         * gdbtypes.c (create_array_type_with_stride): New function,
> >         renaming create_array_type, but with an added parameter
> >         called "bit_stride".
> >         (create_array_type): Re-implement using
> >         create_array_type_with_stride.
> >         * dwarf2read.c (read_array_type): Add support for DW_AT_byte_stride
> >         and DW_AT_bit_stride attributes.
> > 
> > gdb/testsuite/ChangeLog:
> > 
> >         * gdb.dwarf2/arr-stride.c: New file.
> >         * gdb.dwarf2/arr-stride.exp: New file.
> 
> I know little of the GDB code base, just looking at it because it using
> some interesting DWARF, but this patch does look fine to me. Just a few
> small nitpicks below.

Thanks :)

> > +++ b/gdb/gdbtypes.c
> > @@ -951,14 +951,18 @@ get_array_bounds (struct type *type, LONGEST *low_bound, LONGEST *high_bound)
> >     Elements will be of type ELEMENT_TYPE, the indices will be of type
> >     RANGE_TYPE.
> >  
> > +   If BIT_STRIDE is non-null, build a packed array type whose element
> > +   size is BIT_STRIDE.  Otherwise, ignore this parameter.
> 
> s/non-null/not zero/ ?

Correct, and ...

> 
> > +/* Same as create_array_type_with_stride but with no bit_stride
> > +   (BIT_STRIDE = -1), thus building an unpacked array.  */
> 
> Shouldn't that be BIT_STRIDE = 0?

... correct! I will fix those.

> > diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
> > +extern struct type *create_array_type_with_stride
> > +  (struct type *, struct type *, struct type *, unsigned int);
> > +
> >  extern struct type *create_array_type (struct type *, struct type *,
> >  				       struct type *);
> 
> Personally I would have just added the new argument and fixed up all
> callers, just so I had an overview of all the callers that might have a
> bit_stride concept that could be added.

I briefely considered this idea, but since most uses seem to be
creating arrays where the context suggested that bit-strides was
not relevant, I went for the dual approach; it also reduces the patch
size with noise from updating all calls (~30 of them). That being said,
if people prefer your suggestion, it can certainly be done easily.

-- 
Joel

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFA/DWARF] Add array DW_AT_bit_stride and DW_AT_byte_stride support
  2014-02-21  9:31   ` Joel Brobecker
@ 2014-02-26 11:12     ` Mark Wielaard
  2014-02-26 14:35       ` pushed: " Joel Brobecker
  0 siblings, 1 reply; 7+ messages in thread
From: Mark Wielaard @ 2014-02-26 11:12 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

On Fri, 2014-02-21 at 10:31 +0100, Joel Brobecker wrote:
> > Personally I would have just added the new argument and fixed up all
> > callers, just so I had an overview of all the callers that might
> have a
> > bit_stride concept that could be added.
> 
> I briefely considered this idea, but since most uses seem to be
> creating arrays where the context suggested that bit-strides was
> not relevant, I went for the dual approach; it also reduces the patch
> size with noise from updating all calls (~30 of them). That being
> said,
> if people prefer your suggestion, it can certainly be done easily.

I think your approach is fine. For someone who already knows the code
base and knows the stride of an array is an odd concept in most cases
anyway. I would just have done it because I am unfamiliar with the code
base, so it would have given me an opportunity to go through it all
because the compiler would yell at me and made me inspect and understand
it all.

Cheers,

Mark

^ permalink raw reply	[flat|nested] 7+ messages in thread

* pushed: [RFA/DWARF] Add array DW_AT_bit_stride and DW_AT_byte_stride support
  2014-02-26 11:12     ` Mark Wielaard
@ 2014-02-26 14:35       ` Joel Brobecker
  0 siblings, 0 replies; 7+ messages in thread
From: Joel Brobecker @ 2014-02-26 14:35 UTC (permalink / raw)
  To: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 674 bytes --]

Attached is the patch I ended up pushing. Many thanks to MarkW for
his insightful comments.

gdb/ChangeLog:

        * gdbtypes.h (create_array_type_with_stride): Add declaration.
        * gdbtypes.c (create_array_type_with_stride): New function,
        renaming create_array_type, but with an added parameter
        called "bit_stride".
        (create_array_type): Re-implement using
        create_array_type_with_stride.
        * dwarf2read.c (read_array_type): Add support for DW_AT_byte_stride
        and DW_AT_bit_stride attributes.

gdb/testsuite/ChangeLog:

        * gdb.dwarf2/arr-stride.c: New file.
        * gdb.dwarf2/arr-stride.exp: New file.

-- 
Joel

[-- Attachment #2: 0001-DWARF-Add-array-DW_AT_bit_stride-and-DW_AT_byte_stri.patch --]
[-- Type: text/x-diff, Size: 13654 bytes --]

From dc53a7adb516adbf2f646a078a1140b1044a39f5 Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Wed, 29 Jan 2014 17:39:56 +0400
Subject: [PATCH] DWARF: Add array DW_AT_bit_stride and DW_AT_byte_stride
 support

Consider the following declarations in Ada...

   type Item is range -32 .. 31;
   for Item'Size use 6;

   type Table is array (Natural range 0 .. 4) of Item;
   pragma Pack (Table);

... which declare a packed array whose elements are 6 bits long.
The debugger currently does not notice that the array is packed,
and thus prints values of this type incorrectly. This can be seen
in the "ptype" output:

    (gdb) ptype table
    type = array (0 .. 4) of foo.item

Normally, the debugger should print:

    (gdb) ptype table
    type = array (0 .. 4) of foo.item <packed: 6-bit elements>

The debugging information for this array looks like this:

        .uleb128 0xf    # (DIE (0x15c) DW_TAG_array_type)
        .long   .LASF9  # DW_AT_name: "pck__table"
        .byte   0x6     # DW_AT_bit_stride
        .long   0x1a9   # DW_AT_type
        .uleb128 0x10   # (DIE (0x16a) DW_TAG_subrange_type)
        .long   0x3b    # DW_AT_type
        .byte   0       # DW_AT_lower_bound
        .byte   0x4     # DW_AT_upper_bound
        .byte   0       # end of children of DIE 0x15c

The interesting part is the DW_AT_bit_stride attribute, which tells
the size of the array elements is 6 bits, rather than the normal
element type's size.

This patch adds support for this attribute by first creating
gdbtypes.c::create_array_type_with_stride, which is an enhanced
version of create_array_type taking an extra parameter as the stride.
The old create_array_type can then be re-implemented very simply
by calling the new create_array_type_with_stride.

We can then use this new function from dwarf2read, to create
arrays with or without stride.

gdb/ChangeLog:

        * gdbtypes.h (create_array_type_with_stride): Add declaration.
        * gdbtypes.c (create_array_type_with_stride): New function,
        renaming create_array_type, but with an added parameter
        called "bit_stride".
        (create_array_type): Re-implement using
        create_array_type_with_stride.
        * dwarf2read.c (read_array_type): Add support for DW_AT_byte_stride
        and DW_AT_bit_stride attributes.

gdb/testsuite/ChangeLog:

        * gdb.dwarf2/arr-stride.c: New file.
        * gdb.dwarf2/arr-stride.exp: New file.

The test, relying purely on generating an assembly file, only
verifies the type description of our array. But I was also
able to verify manually that the debugger print values of these
types correctly as well (which was not the case prior to this
patch).
---
 gdb/ChangeLog                           |  11 ++++
 gdb/dwarf2read.c                        |  18 +++++-
 gdb/gdbtypes.c                          |  27 +++++++-
 gdb/gdbtypes.h                          |   4 ++
 gdb/testsuite/ChangeLog                 |   5 ++
 gdb/testsuite/gdb.dwarf2/arr-stride.c   |  20 ++++++
 gdb/testsuite/gdb.dwarf2/arr-stride.exp | 108 ++++++++++++++++++++++++++++++++
 7 files changed, 187 insertions(+), 6 deletions(-)
 create mode 100644 gdb/testsuite/gdb.dwarf2/arr-stride.c
 create mode 100644 gdb/testsuite/gdb.dwarf2/arr-stride.exp

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 3a02a49..a4377a9 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,14 @@
+2014-02-26  Joel Brobecker  <brobecker@adacore.com>
+
+	* gdbtypes.h (create_array_type_with_stride): Add declaration.
+	* gdbtypes.c (create_array_type_with_stride): New function,
+	renaming create_array_type, but with an added parameter
+	called "bit_stride".
+	(create_array_type): Re-implement using
+	create_array_type_with_stride.
+	* dwarf2read.c (read_array_type): Add support for DW_AT_byte_stride
+	and DW_AT_bit_stride attributes.
+
 2014-02-26  Pedro Alves  <palves@redhat.com>
 
 	* breakpoint.c (bpstat_check_breakpoint_conditions): Handle
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 3eaa0b1..d7dc38c 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -13281,6 +13281,7 @@ read_array_type (struct die_info *die, struct dwarf2_cu *cu)
   int ndim = 0;
   struct cleanup *back_to;
   const char *name;
+  unsigned int bit_stride = 0;
 
   element_type = die_type (die, cu);
 
@@ -13289,13 +13290,22 @@ read_array_type (struct die_info *die, struct dwarf2_cu *cu)
   if (type)
     return type;
 
+  attr = dwarf2_attr (die, DW_AT_byte_stride, cu);
+  if (attr != NULL)
+    bit_stride = DW_UNSND (attr) * 8;
+
+  attr = dwarf2_attr (die, DW_AT_bit_stride, cu);
+  if (attr != NULL)
+    bit_stride = DW_UNSND (attr);
+
   /* Irix 6.2 native cc creates array types without children for
      arrays with unspecified length.  */
   if (die->child == NULL)
     {
       index_type = objfile_type (objfile)->builtin_int;
       range_type = create_range_type (NULL, index_type, 0, -1);
-      type = create_array_type (NULL, element_type, range_type);
+      type = create_array_type_with_stride (NULL, element_type, range_type,
+					    bit_stride);
       return set_die_type (die, type, cu);
     }
 
@@ -13335,12 +13345,14 @@ read_array_type (struct die_info *die, struct dwarf2_cu *cu)
       int i = 0;
 
       while (i < ndim)
-	type = create_array_type (NULL, type, range_types[i++]);
+	type = create_array_type_with_stride (NULL, type, range_types[i++],
+					      bit_stride);
     }
   else
     {
       while (ndim-- > 0)
-	type = create_array_type (NULL, type, range_types[ndim]);
+	type = create_array_type_with_stride (NULL, type, range_types[ndim],
+					      bit_stride);
     }
 
   /* Understand Dwarf2 support for vector types (like they occur on
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 622eff0..98cb873 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -951,14 +951,18 @@ get_array_bounds (struct type *type, LONGEST *low_bound, LONGEST *high_bound)
    Elements will be of type ELEMENT_TYPE, the indices will be of type
    RANGE_TYPE.
 
+   If BIT_STRIDE is not zero, build a packed array type whose element
+   size is BIT_STRIDE.  Otherwise, ignore this parameter.
+
    FIXME: Maybe we should check the TYPE_CODE of RESULT_TYPE to make
    sure it is TYPE_CODE_UNDEF before we bash it into an array
    type?  */
 
 struct type *
-create_array_type (struct type *result_type, 
-		   struct type *element_type,
-		   struct type *range_type)
+create_array_type_with_stride (struct type *result_type,
+			       struct type *element_type,
+			       struct type *range_type,
+			       unsigned int bit_stride)
 {
   LONGEST low_bound, high_bound;
 
@@ -975,6 +979,9 @@ create_array_type (struct type *result_type,
      In such cases, the array length should be zero.  */
   if (high_bound < low_bound)
     TYPE_LENGTH (result_type) = 0;
+  else if (bit_stride > 0)
+    TYPE_LENGTH (result_type) =
+      (bit_stride * (high_bound - low_bound + 1) + 7) / 8;
   else
     TYPE_LENGTH (result_type) =
       TYPE_LENGTH (element_type) * (high_bound - low_bound + 1);
@@ -983,6 +990,8 @@ create_array_type (struct type *result_type,
     (struct field *) TYPE_ZALLOC (result_type, sizeof (struct field));
   TYPE_INDEX_TYPE (result_type) = range_type;
   TYPE_VPTR_FIELDNO (result_type) = -1;
+  if (bit_stride > 0)
+    TYPE_FIELD_BITSIZE (result_type, 0) = bit_stride;
 
   /* TYPE_FLAG_TARGET_STUB will take care of zero length arrays.  */
   if (TYPE_LENGTH (result_type) == 0)
@@ -991,6 +1000,18 @@ create_array_type (struct type *result_type,
   return result_type;
 }
 
+/* Same as create_array_type_with_stride but with no bit_stride
+   (BIT_STRIDE = 0), thus building an unpacked array.  */
+
+struct type *
+create_array_type (struct type *result_type,
+		   struct type *element_type,
+		   struct type *range_type)
+{
+  return create_array_type_with_stride (result_type, element_type,
+					range_type, 0);
+}
+
 struct type *
 lookup_array_range_type (struct type *element_type,
 			 LONGEST low_bound, LONGEST high_bound)
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 643c610..080c3eb 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -1529,8 +1529,12 @@ extern struct type *lookup_function_type_with_arguments (struct type *,
 extern struct type *create_range_type (struct type *, struct type *, LONGEST,
 				       LONGEST);
 
+extern struct type *create_array_type_with_stride
+  (struct type *, struct type *, struct type *, unsigned int);
+
 extern struct type *create_array_type (struct type *, struct type *,
 				       struct type *);
+
 extern struct type *lookup_array_range_type (struct type *, LONGEST, LONGEST);
 
 extern struct type *create_string_type (struct type *, struct type *,
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 09cc8a3..476e1a3 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2014-02-26  Joel Brobecker  <brobecker@adacore.com>
+
+	* gdb.dwarf2/arr-stride.c: New file.
+	* gdb.dwarf2/arr-stride.exp: New file.
+
 2014-02-26  Pedro Alves  <palves@redhat.com>
 
 	* gdb.ada/tasks.exp: Set a task-specific breakpoint at break_me
diff --git a/gdb/testsuite/gdb.dwarf2/arr-stride.c b/gdb/testsuite/gdb.dwarf2/arr-stride.c
new file mode 100644
index 0000000..978ef8d
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/arr-stride.c
@@ -0,0 +1,20 @@
+/* Copyright 2014 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/>.  */
+
+int
+main (void)
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.dwarf2/arr-stride.exp b/gdb/testsuite/gdb.dwarf2/arr-stride.exp
new file mode 100644
index 0000000..f2a1f39
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/arr-stride.exp
@@ -0,0 +1,108 @@
+# Copyright 2014 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 dwarf.exp
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+if {![dwarf2_support]} {
+    return 0
+}
+
+standard_testfile arr-stride.c arr-stride-dw.S
+
+# Make some DWARF for the test.
+set asm_file [standard_output_file $srcfile2]
+Dwarf::assemble $asm_file {
+    cu {} {
+ 	DW_TAG_compile_unit {
+                {DW_AT_language @DW_LANG_Ada95}
+                {DW_AT_name     foo.adb}
+                {DW_AT_comp_dir /tmp}
+        } {
+	    declare_labels integer_label array_elt_label array_label \
+                big_array_label
+
+            integer_label: DW_TAG_base_type {
+                {DW_AT_byte_size 4 DW_FORM_sdata}
+                {DW_AT_encoding  @DW_ATE_signed}
+                {DW_AT_name      integer}
+            }
+
+            array_elt_label: DW_TAG_subrange_type {
+                {DW_AT_lower_bound 0xe0 DW_FORM_data1}
+                {DW_AT_upper_bound 0x1f DW_FORM_data1}
+                {DW_AT_name        pck__item}
+                {DW_AT_type        :$integer_label}
+            }
+
+            DW_TAG_typedef {
+                {DW_AT_name pck__table}
+                {DW_AT_type :$array_label}
+            }
+
+	    array_label: DW_TAG_array_type {
+		{DW_AT_name pck__table}
+                {DW_AT_bit_stride 6 DW_FORM_data1}
+		{DW_AT_type :$array_elt_label}
+	    } {
+		DW_TAG_subrange_type {
+		    {DW_AT_type        :$integer_label}
+		    {DW_AT_lower_bound 0 DW_FORM_data1}
+		    {DW_AT_upper_bound 4 DW_FORM_data1}
+		}
+	    }
+
+            DW_TAG_typedef {
+                {DW_AT_name pck__big_table}
+                {DW_AT_type :$big_array_label}
+            }
+
+	    big_array_label: DW_TAG_array_type {
+		{DW_AT_name pck__big_table}
+                {DW_AT_byte_stride 1 DW_FORM_data1}
+		{DW_AT_type :$array_elt_label}
+	    } {
+		DW_TAG_subrange_type {
+		    {DW_AT_type        :$integer_label}
+		    {DW_AT_lower_bound 0 DW_FORM_data1}
+		    {DW_AT_upper_bound 4 DW_FORM_data1}
+		}
+	    }
+	}
+    }
+}
+
+if  {[gdb_compile ${srcdir}/${subdir}/${srcfile} ${binfile}1.o \
+	  object {nodebug}] != ""} {
+    return -1
+}
+
+if  {[gdb_compile $asm_file ${binfile}2.o object {nodebug}] != ""} {
+    return -1
+}
+
+if  {[gdb_compile [list ${binfile}1.o ${binfile}2.o] \
+	  "${binfile}" executable {}] != ""} {
+    return -1
+}
+
+clean_restart ${testfile}
+
+gdb_test_no_output "set language ada"
+
+gdb_test "ptype pck.table" \
+         "type = array \\(0 \\.\\. 4\\) of pck\\.item <packed: 6-bit elements>"
+
+gdb_test "ptype pck.big_table" \
+         "type = array \\(0 \\.\\. 4\\) of pck\\.item <packed: 8-bit elements>"
-- 
1.8.3.2


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2014-02-26 14:35 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-29 14:30 [RFA/DWARF] Add array DW_AT_bit_stride and DW_AT_byte_stride support Joel Brobecker
2014-02-10 14:29 ` PING: " Joel Brobecker
2014-02-17 20:20 ` Joel Brobecker
2014-02-20 12:16 ` Mark Wielaard
2014-02-21  9:31   ` Joel Brobecker
2014-02-26 11:12     ` Mark Wielaard
2014-02-26 14:35       ` pushed: " Joel Brobecker

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).