[PATCH] Fix display of structures/bitfields in register description.

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] Fix display of structures/bitfields in register description.

Tedeschi, Walfred
Add support for displaying structures and bitfields for registers when
executing the "maint print c-tdesc". Command is also used when
converting the xml target description file into c file.

Example of the behaviour is given below reporting a snipet of the xml file
and a snippet of the c code generated.

XML file contains:
...
    <union id="vecint">
      <field name="v4" type="v4int8"/>
      <field name="v2" type="v2int16"/>
    </union>

    <struct id="struct1">
      <field name="v4" type="v4int8"/>
      <field name="v2" type="v2int16"/>
    </struct>

    <struct id="struct2" size="8">
      <field name="f1" start="0" end="34"/>
      <field name="f2" start="63" end="63"/>
    </struct>
...

Setting this xml file as target description file and
issuing the maintenance print c-tdesc the following output
is obtained:

  feature = tdesc_create_feature (result, "extra");
  field_type = tdesc_named_type (feature, "int8");
  tdesc_create_vector (feature, "v4int8", field_type, 4);

  field_type = tdesc_named_type (feature, "int16");
  tdesc_create_vector (feature, "v2int16", field_type, 2);

  type = tdesc_create_union (feature, "vecint");
  field_type = tdesc_named_type (feature, "v4int8");
  tdesc_add_field (type, "v4", field_type);
  field_type = tdesc_named_type (feature, "v2int16");
  tdesc_add_field (type, "v2", field_type);

C output is not supported type "struct1".

This is finally the issue.

2013-03-27  Walfred Tedeschi  <[hidden email]>

        * target-descriptions.c (maint_print_c_tdesc_cmd):
        Add case to parse structures as register types and
        bitfields.

testsuite/gdb.xml/

        * maint_print_struct.exp: New file.
        * maint_print_struct.xml: New file.

Signed-off-by: Walfred Tedeschi <[hidden email]>
---
 gdb/target-descriptions.c                    |   30 +++++++++++++++++++++
 gdb/testsuite/gdb.xml/maint_print_struct.exp |   37 ++++++++++++++++++++++++++
 gdb/testsuite/gdb.xml/maint_print_struct.xml |   26 ++++++++++++++++++
 3 files changed, 93 insertions(+)
 create mode 100644 gdb/testsuite/gdb.xml/maint_print_struct.exp
 create mode 100644 gdb/testsuite/gdb.xml/maint_print_struct.xml

diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 95980c5..88a08ad 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -1746,6 +1746,36 @@ feature = tdesc_create_feature (result, \"%s\");\n",
  ("  tdesc_create_vector (feature, \"%s\", field_type, %d);\n",
  type->name, type->u.v.count);
       break;
+    case TDESC_TYPE_STRUCT:
+      printf_unfiltered
+ ("  type = tdesc_create_struct (feature, \"%s\");\n",
+ type->name);
+      if (type->u.u.size != 0)
+ printf_unfiltered
+  ("  tdesc_set_struct_size (type, %s);\n",
+   plongest (type->u.u.size));
+      for (ix3 = 0;
+   VEC_iterate (tdesc_type_field, type->u.u.fields, ix3, f);
+   ix3++)
+ {
+  /* Going first for implicitly sized types, else part handles
+     bitfields.  As reported on xml-tdesc.c implicitly sized types
+     cannot contain a bitfield.  */
+  if (f->start == 0 && f->end == 0)
+    {
+      printf_unfiltered
+ ("  field_type = tdesc_named_type (feature, \"%s\");\n",
+ f->type->name);
+      printf_unfiltered
+ ("  tdesc_add_field (type, \"%s\", field_type);\n",
+ f->name);
+    }
+  else
+    printf_unfiltered
+      ("  tdesc_add_bitfield (type, \"%s\", %d, %d);\n",
+       f->name, f->start, f->end);
+ }
+      break;
     case TDESC_TYPE_UNION:
       printf_unfiltered
  ("  type = tdesc_create_union (feature, \"%s\");\n",
diff --git a/gdb/testsuite/gdb.xml/maint_print_struct.exp b/gdb/testsuite/gdb.xml/maint_print_struct.exp
new file mode 100644
index 0000000..f444451
--- /dev/null
+++ b/gdb/testsuite/gdb.xml/maint_print_struct.exp
@@ -0,0 +1,37 @@
+# This testcase is part of GDB, the GNU debugger.
+#
+# Copyright 2013 Free Software Foundation, Inc.
+#
+# Contributed by Intel Corp. <[hidden email]>
+#
+# 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/>.
+
+if {[gdb_skip_xml_test]} {
+    unsupported "maint_print_struct.exp"
+    return -1
+}
+
+gdb_start
+global srcdir
+global subdir
+
+# Required registers are not present so it is expected a warning.
+#
+gdb_test "set tdesc filename $srcdir/$subdir/maint_print_struct.xml" "
+warning:.*" "setting a new tdesc having only a structure"
+
+gdb_test "maint print c-tdesc" "
+.*tdesc_create_reg \\(feature, \"bad_reg1\", \[0-9\]+, 1, NULL, 128, \"two_fielded\"\\);\r
+.*tdesc_create_reg \\(feature, \"bad_reg2\", \[0-9\]+, 1, NULL, 64, \"bitfield\"\\);\r
+.*" "printing tdesc with a structure and a bitfield"
diff --git a/gdb/testsuite/gdb.xml/maint_print_struct.xml b/gdb/testsuite/gdb.xml/maint_print_struct.xml
new file mode 100644
index 0000000..dab71fa
--- /dev/null
+++ b/gdb/testsuite/gdb.xml/maint_print_struct.xml
@@ -0,0 +1,26 @@
+<?xml version="1.0"?>
+<!-- Copyright (C) 2013 Free Software Foundation, Inc.
+
+     Copying and distribution of this file, with or without modification,
+     are permitted in any medium without royalty provided the copyright
+     notice and this notice are preserved.  -->
+
+<!DOCTYPE feature SYSTEM "gdb-target.dtd">
+<target>
+<feature name="test">
+
+  <struct id="two_fielded">
+       <field name="field1" type="data_ptr"/>
+       <field name="field2" type="data_ptr"/>
+  </struct>
+
+  <struct id="bitfield" size="8">
+    <field name="field1" start="24" end="63"/>
+    <field name="field2" start="16" end="24"/>
+  </struct>
+
+  <reg name="bad_reg1" bitsize="128" type="two_fielded"/>
+  <reg name="bad_reg2" bitsize="64"  type="bitfield"/>
+</feature>
+</target>
+
--
1.7.10.4

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix display of structures/bitfields in register description.

Tom Tromey
>>>>> "Walfred" == Walfred Tedeschi <[hidden email]> writes:

Walfred> Add support for displaying structures and bitfields for
Walfred> registers when executing the "maint print c-tdesc". Command is
Walfred> also used when converting the xml target description file into
Walfred> c file.

Thanks for the patch.

I think it looks pretty good.  I have a few notes.

Walfred> +    case TDESC_TYPE_STRUCT:
Walfred> +      printf_unfiltered
Walfred> + ("  type = tdesc_create_struct (feature, \"%s\");\n",

This emits code to assign to the variable "type".
But is this variable defined?
Earlier I see:

          if (type->kind == TDESC_TYPE_UNION
              && VEC_length (tdesc_type_field, type->u.u.fields) > 0)
            {
              printf_unfiltered ("  struct tdesc_type *type;\n");

... which I think is the only spot where it is defined.

If this is correct then I think changing this to also check
TDESC_TYPE_STRUCT would be appropriate.

If it isn't correct, then I'm curious to know what defines 'type' in the
output.

Walfred> +global srcdir
Walfred> +global subdir

I don't think these are needed.

Walfred> +<!-- Copyright (C) 2013 Free Software Foundation, Inc.
Walfred> +
Walfred> +     Copying and distribution of this file, with or without modification,
Walfred> +     are permitted in any medium without royalty provided the copyright
Walfred> +     notice and this notice are preserved.  -->

I think this should be the usual GPL comment.

Tom
Reply | Threaded
Open this post in threaded view
|

RE: [PATCH] Fix display of structures/bitfields in register description.

Tedeschi, Walfred

smime.p7m (15K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix display of structures/bitfields in register description.

Tom Tromey
Walfred> Copyrights:
Walfred> I took a look on several xml files and all of them had the same
Walfred> copyright I have used.  Though, would you recommend to use the
Walfred> common GPL header?

I don't know why the other .xml files have this other header.
However, seeing as the new file is in the test suite, I think the plain
GPL comment is preferable.

Tom
Reply | Threaded
Open this post in threaded view
|

RE: [PATCH] Fix display of structures/bitfields in register description.

Tedeschi, Walfred

smime.p7m (12K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix display of structures/bitfields in register description.

Daniel Jacobowitz-2
In reply to this post by Tom Tromey
They have a different header because they're supposed to be embedded
in things that run on the target which are not necessarily
GPL-compatible.  I figured out with the FSF what an appropriate notice
would be for that case.  I'd generally rather we use that notice for
all target XML files, even in the testsuite, to prevent someone
copying the GPL'd version into something by accident - not a big deal,
though.

On Thu, Apr 25, 2013 at 9:49 AM, Tom Tromey <[hidden email]> wrote:

> Walfred> Copyrights:
> Walfred> I took a look on several xml files and all of them had the same
> Walfred> copyright I have used.  Though, would you recommend to use the
> Walfred> common GPL header?
>
> I don't know why the other .xml files have this other header.
> However, seeing as the new file is in the test suite, I think the plain
> GPL comment is preferable.
>
> Tom



--
Thanks,
Daniel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix display of structures/bitfields in register description.

Tom Tromey
>>>>> "Daniel" == Daniel Jacobowitz <[hidden email]> writes:

Daniel> They have a different header because they're supposed to be embedded
Daniel> in things that run on the target which are not necessarily
Daniel> GPL-compatible.  I figured out with the FSF what an appropriate notice
Daniel> would be for that case.  I'd generally rather we use that notice for
Daniel> all target XML files, even in the testsuite, to prevent someone
Daniel> copying the GPL'd version into something by accident - not a big deal,
Daniel> though.

It is totally fine with me.
Thanks for explaining this.

Tom
Reply | Threaded
Open this post in threaded view
|

RE: [PATCH] Fix display of structures/bitfields in register description.

Tedeschi, Walfred
In reply to this post by Daniel Jacobowitz-2

smime.p7m (13K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix display of structures/bitfields in register description.

Tom Tromey
>>>>> "Walfred" == Tedeschi, Walfred <[hidden email]> writes:

Walfred> Thanks for this clarification.  Since it's not checked-in yet,
Walfred> I think it would make sense using the usual xml header, or?
Walfred> (Keeping all xml files homogeneous)

Yeah, I think it is fine to change it back.
Sorry about that.

Tom