Re: [patch] Add support for <struct> and <flags> in target descriptions

["H.J. Lu" <hjl.tools@gmail.com>]

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

On Sun, Feb 28, 2010 at 4:15 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Mon, Feb 22, 2010 at 8:45 AM, Daniel Jacobowitz <dan@codesourcery.com> wrote:
>> Hi H.J.,
>>
>> This patch adds <flags> support to the XML language.  Could you try
>> using this to move the two x86 flags registers from out of
>> target-descriptions.c?
>>
>> The flags support is straightforward and covered by the manual.
>> It's not tested because I couldn't find a way to do so; you can't
>> ptype a flags register, and you can't add dummy registers whose value
>> you can get at, only for ptype.
>>
>> The patch also adds <struct>, which is a little more interesting.
>> It's got two forms:
>>
>> * Register containing integer bitfields.  Each field must be
>> explicitly positioned.  The size must be pre-declared - otherwise
>> the representation GDB uses for big-endian bitfields can't figure out
>> how far from the MSB edge of the register the field is.
>>
>> * Register containing typed non-bitfield structures.  Each field must
>> be implicitly positioned.  There's no support for padding.
>>
>> These are somewhat annoying limitations, but they suffice for
>> everything I've needed this for since I wrote the patch, which was
>> originally several years ago; it's been stuck in my submission queue
>> because it was tangled up with other local patches.  Since they are
>> "must" restrictions, they are easy to lift in the future; we can make
>> GDB more permissive.
>>
>> These I was able to type, although I'd have liked more exhaustive
>> tests... for that, I'd need typeof, which turns out to be annoyingly
>> hard to implement in GDB's parser :-(
>>
>> Eli, how's the documentation?
>>
>> --
>> Daniel Jacobowitz
>> CodeSourcery
>>
>> 2010-02-22  Daniel Jacobowitz  <dan@codesourcery.com>
>>
>>        * gdbtypes.c (append_composite_type_field_raw): New.
>>        (append_composite_type_field_aligned): Use the new function.
>>        * gdbtypes.h (append_composite_type_field_raw): Declare.
>>        * target-descriptions.c (struct tdesc_type_field): Add start and end.
>>        (struct tdesc_type_flag): New type.
>>        (struct tdesc_type): Add TDESC_TYPE_STRUCT and TDESC_TYPE_FLAGS to
>>        kind.  Add size to u.u.  Add u.f for flags.
>>        (tdesc_gdb_type): Handle TDESC_TYPE_STRUCT and TDESC_TYPE_FLAGS.
>>        (tdesc_free_type): Likewise.
>>        (tdesc_create_struct, tdesc_set_struct_size, tdesc_create_flags): New.
>>        (tdesc_add_field): Handle TDESC_TYPE_STRUCT.
>>        (tdesc_add_bitfield, tdesc_add_flag): New.
>>        * target-descriptions.h (tdesc_create_struct, tdesc_set_struct_size)
>>        (tdesc_create_flags, tdesc_add_bitfield, tdesc_add_flag): Declare.
>>        * xml-tdesc.c (struct tdesc_parsing_data): Rename current_union to
>>        current_type.  Add current_type_size and current_type_is_flags.
>>        (tdesc_start_union): Clear the new fields.
>>        (tdesc_start_struct, tdesc_start_flags): New.
>>        (tdesc_start_field): Handle struct fields, including bitfields.
>>        (field_attributes): Make type optional.  Add start and end.
>>        (union_children): Rename to struct_union_children.
>>        (union_attributes): Rename to struct_union_attributes.  Add optional
>>        size.
>>        (flags_attributes): New.
>>        (feature_children): Add struct and flags.
>>        * features/gdb-target.dtd: Add flags and struct to features.
>>        Make field type optional.  Add field start and end.
>>
>>        * gdb.texinfo (Types): Describe <struct> and <flags>.
>>
>>        * gdb.xml/extra-regs.xml: Add struct1, struct2, and flags
>>        types.  Add structreg, bitfields, and flags registers.
>>        * gdb.xml/tdesc-regs.exp: Test structreg and bitfields
>>        registers.
>>
>
> It doesn't work. "case TDESC_TYPE_FLAGS:" is missing in
> maint_print_c_tdesc_cmd.
>
>

This patch makes it to work. However, there is no equivalent of

append_flags_type_flag (type, 1, NULL);

I have

  <flags id="i386_eflags" size="4">
    <field name="CF" start="0" end="0"/>
    <field name="" start="1" end="1"/>
    <field name="PF" start="2" end="2"/>
....

I got

(gdb) p $eflags
$1 = [  PF ZF IF ]
           ^ extra white space
(gdb)

instead of
(gdb) p $eflags
$1 = [ PF ZF IF ]
(gdb)




-- 
H.J.

[-- Attachment #2: gdb-flag-1.patch --]
[-- Type: text/plain, Size: 2730 bytes --]

diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 77dd37b..af87633 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -126,7 +126,6 @@ typedef struct tdesc_type
     TDESC_TYPE_IEEE_DOUBLE,
     TDESC_TYPE_ARM_FPA_EXT,
     TDESC_TYPE_I387_EXT,
-    TDESC_TYPE_I386_EFLAGS,
     TDESC_TYPE_I386_MXCSR,
 
     /* Types defined by a target feature.  */
@@ -484,7 +483,6 @@ static struct tdesc_type tdesc_predefined_types[] =
   { "ieee_double", TDESC_TYPE_IEEE_DOUBLE },
   { "arm_fpa_ext", TDESC_TYPE_ARM_FPA_EXT },
   { "i387_ext", TDESC_TYPE_I387_EXT },
-  { "i386_eflags", TDESC_TYPE_I386_EFLAGS },
   { "i386_mxcsr", TDESC_TYPE_I386_MXCSR }
 };
 
@@ -607,33 +605,6 @@ tdesc_gdb_type (struct gdbarch *gdbarch, struct tdesc_type *tdesc_type)
       return arch_float_type (gdbarch, -1, "builtin_type_i387_ext",
 			      floatformats_i387_ext);
 
-    case TDESC_TYPE_I386_EFLAGS:
-      {
-	struct type *type;
-
-	type = arch_flags_type (gdbarch, "builtin_type_i386_eflags", 4);
-	append_flags_type_flag (type, 0, "CF");
-	append_flags_type_flag (type, 1, NULL);
-	append_flags_type_flag (type, 2, "PF");
-	append_flags_type_flag (type, 4, "AF");
-	append_flags_type_flag (type, 6, "ZF");
-	append_flags_type_flag (type, 7, "SF");
-	append_flags_type_flag (type, 8, "TF");
-	append_flags_type_flag (type, 9, "IF");
-	append_flags_type_flag (type, 10, "DF");
-	append_flags_type_flag (type, 11, "OF");
-	append_flags_type_flag (type, 14, "NT");
-	append_flags_type_flag (type, 16, "RF");
-	append_flags_type_flag (type, 17, "VM");
-	append_flags_type_flag (type, 18, "AC");
-	append_flags_type_flag (type, 19, "VIF");
-	append_flags_type_flag (type, 20, "VIP");
-	append_flags_type_flag (type, 21, "ID");
-
-	return type;
-      }
-    break;
-
     case TDESC_TYPE_I386_MXCSR:
       {
 	struct type *type;
@@ -1602,6 +1573,7 @@ maint_print_c_tdesc_cmd (char *args, int from_tty)
   struct tdesc_reg *reg;
   struct tdesc_type *type;
   struct tdesc_type_field *f;
+  struct tdesc_type_flag *flag;
   int ix, ix2, ix3;
 
   /* Use the global target-supplied description, not the current
@@ -1715,6 +1687,18 @@ maint_print_c_tdesc_cmd (char *args, int from_tty)
 		     f->name);
 		}
 	      break;
+	    case TDESC_TYPE_FLAGS:
+	      printf_unfiltered
+		("  field_type = tdesc_create_flags (feature, \"%s\", %d);\n",
+		 type->name, (int) type->u.f.size);
+	      for (ix3 = 0;
+		   VEC_iterate (tdesc_type_flag, type->u.f.flags, ix3,
+				flag);
+		   ix3++)
+		printf_unfiltered
+		  ("  tdesc_add_flag (field_type, %d, \"%s\");\n",
+		   flag->start, flag->name);
+	      break;
 	    default:
 	      error (_("C output is not supported type \"%s\"."), type->name);
 	    }