gdbserver target description: info reg displays or not according to group presence ???

gdbserver target description: info reg displays or not according to group presence ???

[Philippe Waroquiers]

Hello,
I am busy integrating a gdbserver within valgrind.

Valgrind has shadow registers (indicating if a register contains 
"uninitialized" data).
To have gdb be able to display and modify these registers, I am defining
a target description that includes the normal register set and the 2
valgrind shadow register sets.
I would like that these registers are not shown in "info regs" but
only shown in "info all-reg".
According to the manual (gdb gdb-7.2.50.20100730), it is enough
to *not* specify a group and then these are not displayed:

   GROUP
        The register group to which this register belongs.  GROUP must be
        either `general', `float', or `vector'.  If no GROUP is specified,
        GDB will not display the register in `info registers'.

However, that does not seem to match what I observe.

The file gdb/features/i386/32bit-core.xml defines e.g. eax as:
  <reg name="eax" bitsize="32" type="int32"/>
This register is shown in info reg.
According to the manual above, it should not.
I have defined another file for valgrind, called 32bit-core-valgrind-s1.xml
where I have a.o. the following two lines:
  <reg name="eaxs1" bitsize="32" type="int32" group="s1"/>
  <reg name="ecxs1" bitsize="32" type="int32"/>

When I am doing info regs, gdb shows both eaxs1 and ecxs1 (and all others
shadow registers).

Looking at the trace, I see that the correct description is returned
to gdb. I see that gdb properly has understood the target description
as it shows the normal and shadow registers.
However, it does always shows all the shadow registers, not taking
into account the presence or not of a group.
Trying to understand what is happening, I am using maint print
to look at what gdb has understood:
   (gdb) maint print c-tdesc
   The current target description did not come from an XML file.
-- this looks normal as the target description arrives from gdbserver.

   (gdb) maint print register-groups
  ... (removed some)
    ''           47   47    308       0 int0_t          general
    ''           48   48    308       0 int0_t          general
    orig_eax     49   49    308       4 long            save,restore,system
    eaxs1        50   50    312       4 int32_t         general,all,save,restore
    ecxs1        51   51    316       4 int32_t         general,all,save,restore
    edxs1        52   52    320       4 int32_t         general,all,save,restore
  ... (removed some)
-- this looks strange: the s1 group is not theree for eaxs1, it rather has general/all/save/restore
indication.

   (gdb) maint print raw-registers 
  ... (removed some)
    ''           47   47    308       0 int0_t          <invalid>
    ''           48   48    308       0 int0_t          <invalid>
    orig_eax     49   49    308       4 long            0x00000000
    eaxs1        50   50    312       4 int32_t         0x00000000
    ecxs1        51   51    316       4 int32_t         0x00000000
    edxs1        52   52    320       4 int32_t         0x00000000
  ... (removed some)


So, there is something that looks wrong.
Any hint/idea ?

Re: gdbserver target description: info reg displays or not according to group presence ???

[Daniel Jacobowitz]

On Sat, Jul 31, 2010 at 12:16:57AM +0200, Philippe Waroquiers wrote:
>   GROUP
>        The register group to which this register belongs.  GROUP must be
>        either `general', `float', or `vector'.  If no GROUP is specified,
>        GDB will not display the register in `info registers'.

Separately, note "must be".  Don't use group="s1".  Target-defined
register groups aren't supported.

> Looking at the trace, I see that the correct description is returned
> to gdb. I see that gdb properly has understood the target description
> as it shows the normal and shadow registers.
> However, it does always shows all the shadow registers, not taking
> into account the presence or not of a group.

Yes, it looks like a bug in i386_register_reggroup_p, in the last bit
(group == general_reggroup_p).  One way to fix it would be to add a
range check (&& regnum < I386_NUM_GREGS).

-- 
Daniel Jacobowitz
CodeSourcery

Re: gdbserver target description: info reg displays or not according to group presence ???

[Philippe Waroquiers]

On Sun, 1 Aug 2010 22:39:47 -0400, Daniel Jacobowitz wrote:
> Separately, note "must be".  Don't use group="s1".  Target-defined
> register groups aren't supported.

In target-description.c, I found the following comment:
   Arbitrary strings (other than "general", "float", and "vector")
   from the description are not used; they cause the register to be
   displayed in "info all-registers" but excluded from "info
   registers" et al.  The names of containing features are also not
   used.  This might be extended to display registers in some more
   useful groupings.

I guess the idea is that one day, something like
    info registers a_target_defined_abitrary_group
will show the registers of this group.

So, it looks like the code currently already accepts arbitrary
register group but that the doc about GROUP does not mention this.


> Yes, it looks like a bug in i386_register_reggroup_p, in the last bit
> (group == general_reggroup_p).  One way to fix it would be to add a
> range check (&& regnum < I386_NUM_GREGS).

Not too sure of what I did but a small test seems ok with the below code:
  if (group == general_reggroup)
    return (!fp_regnum_p
     && !mmx_regnum_p
     && !mxcsr_regnum_p
     && !xmm_regnum_p
     && !ymm_regnum_p
     && !ymmh_regnum_p
            && regnum < I386_NUM_GREGS);
Note that the tests above seems redundant: none of the registers 
below I386_NUM_GREGS are fp or mmx or ...


Should I do a bug entry for this and also for clarifying the doc/comment ?

The documentation of GROUP could probably be clarified and/or the comment
in the code could be changed :
   * the code accepts any string as group name, doc does not mention this.
   * also the target xml files for i386 does not specify any
     group for e.g. eax, but eax is shown in info registers.
     I imagine we need an additional sentence such as 
       "Pre-defined standard registers have an hard-coded set of groups. 
        Use 'maint print register-groups' to look at these. The standard
        registers belonging to general group will be shown by 
        'info registers'."


Philippe

Re: gdbserver target description: info reg displays or not according to group presence ???

[Daniel Jacobowitz]

On Mon, Aug 02, 2010 at 08:07:52PM +0200, Philippe Waroquiers wrote:
> So, it looks like the code currently already accepts arbitrary
> register group but that the doc about GROUP does not mention this.

IMO, the documentation is authoritative here; it's a specification for
what a valid description looks like.  On the other hand, the attached
patch may not work unless you have some other named group... I'm not
sure what to do.

The problem with letting people specify random strings as group is
that GDB might grab those strings for a specific purpose in some
future version.  There's no registry or namespace.

> >Yes, it looks like a bug in i386_register_reggroup_p, in the last bit
> >(group == general_reggroup_p).  One way to fix it would be to add a
> >range check (&& regnum < I386_NUM_GREGS).
> 
> Not too sure of what I did but a small test seems ok with the below code:
>  if (group == general_reggroup)
>    return (!fp_regnum_p
>     && !mmx_regnum_p
>     && !mxcsr_regnum_p
>     && !xmm_regnum_p
>     && !ymm_regnum_p
>     && !ymmh_regnum_p
>            && regnum < I386_NUM_GREGS);
> Note that the tests above seems redundant: none of the registers
> below I386_NUM_GREGS are fp or mmx or ...

Turns out there's a better fix too.  Does this work?

-- 
Daniel Jacobowitz
CodeSourcery

2010-08-11  Daniel Jacobowitz  <dan@codesourcery.com>

	* i386-tdep.c (i386_register_reggroup_p): Use
	tdesc_register_in_reggroup_p.

Index: i386-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/i386-tdep.c,v
retrieving revision 1.316
diff -u -p -r1.316 i386-tdep.c
--- i386-tdep.c	22 Jun 2010 02:15:45 -0000	1.316
+++ i386-tdep.c	11 Aug 2010 20:11:31 -0000
@@ -3106,6 +3106,7 @@ i386_register_reggroup_p (struct gdbarch
   const struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   int fp_regnum_p, mmx_regnum_p, xmm_regnum_p, mxcsr_regnum_p,
       ymm_regnum_p, ymmh_regnum_p;
+  int ret;
 
   /* Don't include pseudo registers, except for MMX, in any register
      groups.  */
@@ -3122,6 +3123,10 @@ i386_register_reggroup_p (struct gdbarch
   if (group == i386_mmx_reggroup)
     return mmx_regnum_p;
 
+  ret = tdesc_register_in_reggroup_p (gdbarch, regnum, group);
+  if (ret != -1)
+    return ret;
+
   xmm_regnum_p = i386_xmm_regnum_p (gdbarch, regnum);
   mxcsr_regnum_p = i386_mxcsr_regnum_p (gdbarch, regnum);
   if (group == i386_sse_reggroup)

Re: gdbserver target description: info reg displays or not according to group presence ???

[Philippe Waroquiers]

Thanks for the reply/help.

> The problem with letting people specify random strings as group is
> that GDB might grab those strings for a specific purpose in some
> future version.  There's no registry or namespace.
Is this really a problem ? 
From what I understand, a group is (only?) useful to (in the future)
allow to do 'info register <group>".
If there is really some behaviour linked with a register group,
then it is enough to specify something like:  
"general", "all", "float", "vector", "save"  and group names starting 
with "g" are standard group names reserved for gdb usage.

> Turns out there's a better fix too.  Does this work?
It does not work according to the doc, but it provides
a consistent behaviour. The behaviour is: 
   if *no* group is given, then 'info reg' shows the registers.

With the patch, the behaviour is:
   * if no group is specified, then a register belongs to the general group.
   * the general group is shown by 'info reg'
   * other (gdb known) groups are shown by 'info reg <group>'
   * info all-reg shows all registers
    * and in the future, we could have
         info reg <non standard group name> shows the registers of this group.

Here is some more detailed info about the behaviour with the patch:

With the below (extract of) target description file:
 ...
    <field name="VIP" start="20" end="20"/>
    <field name="ID" start="21" end="21"/>
  </flags>

  <reg name="eaxs1" bitsize="32" type="int32" group="float"/>
  <reg name="ecxs1" bitsize="32" type="int32" group="groupecxs1"/>
  <reg name="edxs1" bitsize="32" type="int32"/>
  <reg name="ebxs1" bitsize="32" type="int32"/>
  ...

info register gives
  ...
  es             0x0 0
  fs             0x0 0
  gs             0x7b0000 8060928
  edxs1          0x0 0
  ebxs1          0x0 0
  esps1          0x0 0x0
  ...


So, specifying either a standard (float) or unknown (groupecxs1)
group makes these registers not shown.
But having no group at all (e.g. edxs1) still shows them.

maint print register-groups gives:
 ...
 orig_eax     49   49    308       4 long            save,restore,system
 eaxs1        50   50    312       4 int32_t         float,all,save,restore
 ecxs1        51   51    316       4 int32_t         all,save,restore
 edxs1        52   52    320       4 int32_t         general,all,save,restore
 ...

Philippe

Re: gdbserver target description: info reg displays or not according to group presence ???

[Philippe Waroquiers]

Can I give any additional info for the below ?
Or help ?

Thanks

----- Original Message ----- 
From: "Philippe Waroquiers" <philippe.waroquiers@skynet.be>
To: "Daniel Jacobowitz" <dan@codesourcery.com>
Cc: <gdb@sourceware.org>
Sent: Thursday, August 12, 2010 9:08 PM
Subject: Re: gdbserver target description: info reg displays or not according to group presence ???


> Thanks for the reply/help.
> 
>> The problem with letting people specify random strings as group is
>> that GDB might grab those strings for a specific purpose in some
>> future version.  There's no registry or namespace.
> Is this really a problem ? 
> From what I understand, a group is (only?) useful to (in the future)
> allow to do 'info register <group>".
> If there is really some behaviour linked with a register group,
> then it is enough to specify something like:  
> "general", "all", "float", "vector", "save"  and group names starting 
> with "g" are standard group names reserved for gdb usage.
> 
>> Turns out there's a better fix too.  Does this work?
> It does not work according to the doc, but it provides
> a consistent behaviour. The behaviour is: 
>   if *no* group is given, then 'info reg' shows the registers.
> 
> With the patch, the behaviour is:
>   * if no group is specified, then a register belongs to the general group.
>   * the general group is shown by 'info reg'
>   * other (gdb known) groups are shown by 'info reg <group>'
>   * info all-reg shows all registers
>    * and in the future, we could have
>         info reg <non standard group name> shows the registers of this group.
> 
> Here is some more detailed info about the behaviour with the patch:
> 
> With the below (extract of) target description file:
> ...
>    <field name="VIP" start="20" end="20"/>
>    <field name="ID" start="21" end="21"/>
>  </flags>
> 
>  <reg name="eaxs1" bitsize="32" type="int32" group="float"/>
>  <reg name="ecxs1" bitsize="32" type="int32" group="groupecxs1"/>
>  <reg name="edxs1" bitsize="32" type="int32"/>
>  <reg name="ebxs1" bitsize="32" type="int32"/>
>  ...
> 
> info register gives
>  ...
>  es             0x0 0
>  fs             0x0 0
>  gs             0x7b0000 8060928
>  edxs1          0x0 0
>  ebxs1          0x0 0
>  esps1          0x0 0x0
>  ...
> 
> 
> So, specifying either a standard (float) or unknown (groupecxs1)
> group makes these registers not shown.
> But having no group at all (e.g. edxs1) still shows them.
> 
> maint print register-groups gives:
> ...
> orig_eax     49   49    308       4 long            save,restore,system
> eaxs1        50   50    312       4 int32_t         float,all,save,restore
> ecxs1        51   51    316       4 int32_t         all,save,restore
> edxs1        52   52    320       4 int32_t         general,all,save,restore
> ...
> 
> Philippe
>