[RFC] h8300 "info registers" fix

[RFC] h8300 "info registers" fix

[Yoshinori Sato]

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

A result of ccr and exr mistakes by a "info registers" of h8300 target.
There was two faults.

- A name of E_CCR_REGNUM is not defined with h8300_register_name.
  Accordingly regcache_raw_read of h8300_pseudo_regitser_read fails.
- Assume size of E_PSEUDO_CCR_REGNUM by 1 byte with h8300_register_type.
  But assume 4 bytes from a remote target.
  Accordingly cannot take response of remote side to be really.

I correct an issue of a thing of the patch box which I attached.
The issue was corrected, but not know you whether this is good technique.

-- 
Yoshinori Sato
<ysato@users.sourceforge.jp>


[-- Attachment #2: h8300-tdep.diff --]
[-- Type: application/octet-stream, Size: 2248 bytes --]

Index: ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/ChangeLog,v
retrieving revision 1.7897
diff -u -r1.7897 ChangeLog
--- ChangeLog	10 Sep 2006 16:21:03 -0000	1.7897
+++ ChangeLog	12 Sep 2006 14:51:35 -0000
@@ -1,3 +1,11 @@
+2006-09-12  Yoshinori Sato  <ysato@users.sourceforge.jp>
+
+	* h8300-tdep.c (h8300_register_name): set E_CCR_REGNUM name.
+	(h8300s_register_name): set E_CCR_REGNUM and E_EXR_REGNUM name.
+	(h8300sx_register_name): Ditto.
+	(h8300_register_type): E_PSEUD_CCR_REGNUM and E_PSEUD_EXR_REGNUM 
+	size fix.
+
 2006-09-10  Daniel Jacobowitz  <dan@codesourcery.com>
 
 	PR threads/2149
Index: h8300-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/h8300-tdep.c,v
retrieving revision 1.103
diff -u -r1.103 h8300-tdep.c
--- h8300-tdep.c	17 Dec 2005 22:34:00 -0000	1.103
+++ h8300-tdep.c	12 Sep 2006 14:51:35 -0000
@@ -949,7 +949,7 @@
      type is selected. */
   static char *register_names[] = {
     "r0", "r1", "r2", "r3", "r4", "r5", "r6",
-    "sp", "", "pc", "cycles", "tick", "inst",
+    "sp", "gdb-ccr", "pc", "cycles", "tick", "inst",
     "ccr",			/* pseudo register */
   };
   if (regno < 0
@@ -965,7 +965,7 @@
 {
   static char *register_names[] = {
     "er0", "er1", "er2", "er3", "er4", "er5", "er6",
-    "sp", "", "pc", "cycles", "", "tick", "inst",
+    "sp", "dbg-ccr", "pc", "cycles", "dbg-exr", "tick", "inst",
     "mach", "macl",
     "ccr", "exr"		/* pseudo registers */
   };
@@ -983,7 +983,7 @@
 {
   static char *register_names[] = {
     "er0", "er1", "er2", "er3", "er4", "er5", "er6",
-    "sp", "", "pc", "cycles", "", "tick", "inst",
+    "sp", "dbg-ccr", "pc", "cycles", "dbg-exr", "tick", "inst",
     "mach", "macl", "sbr", "vbr",
     "ccr", "exr"		/* pseudo registers */
   };
@@ -1131,11 +1131,7 @@
 	case E_FP_REGNUM:
 	  return builtin_type_void_data_ptr;
 	default:
-	  if (regno == E_PSEUDO_CCR_REGNUM)
-	    return builtin_type_uint8;
-	  else if (regno == E_PSEUDO_EXR_REGNUM)
-	    return builtin_type_uint8;
-	  else if (is_h8300hmode (current_gdbarch))
+	  if (is_h8300hmode (current_gdbarch))
 	    return builtin_type_int32;
 	  else
 	    return builtin_type_int16;

RE: [RFC] h8300 "info registers" fix

[Michael Snyder]

> A result of ccr and exr mistakes by a "info registers" of h8300 target.
> There was two faults.
> 
> - A name of E_CCR_REGNUM is not defined with h8300_register_name.
>   Accordingly regcache_raw_read of h8300_pseudo_regitser_read fails.
> - Assume size of E_PSEUDO_CCR_REGNUM by 1 byte with h8300_register_type.
>   But assume 4 bytes from a remote target.
>   Accordingly cannot take response of remote side to be really.
> 
> I correct an issue of a thing of the patch box which I attached.
> The issue was corrected, but not know you whether this is good technique.

I'm sorry, but I don't think this is right.  

E_CCR_REGNUM is the physical register which gdb reads from the target.
This is defined to be 4 bytes, even though only 1 byte is significant.
It's just easier to keep most or all of the registers to the same size
within the register message packet.

E_PSEUDO_CCR_REGNUM is a "pseudo-register", which gdb computes internally
(rather than requesting from the target).  It is defined to be 1 byte, and
it is the register that gdb displays to the user.  In this case, the 
computation of the pseudo-register is simply copying the one significant
byte from the physical register.

The fact that E_CCR_REGNUM has no display-name is deliberate.  That
register is never displayed, it is only fetched from the target and
used to obtain the value of E_PSEUDO_CCR_REGNUM (which is displayed).

If you are working on the target side (gdb stub, simulator, or gdbserver), 
you only have to worry about E_CCR_REGNUM, but you need to supply it as
four bytes.  I forget whether the MSB or the LSB is the significant one,
but you should not have too much trouble to figure it out.  The other
bytes can be any value - they'll be ignored.

Michael

RE: [RFC] h8300 "info registers" fix

[Yoshinori Sato]

>
>> A result of ccr and exr mistakes by a "info registers" of h8300 target.
>> There was two faults.
>>
>> - A name of E_CCR_REGNUM is not defined with h8300_register_name.
>>   Accordingly regcache_raw_read of h8300_pseudo_regitser_read fails.
>> - Assume size of E_PSEUDO_CCR_REGNUM by 1 byte with h8300_register_type.
>>   But assume 4 bytes from a remote target.
>>   Accordingly cannot take response of remote side to be really.
>>
>> I correct an issue of a thing of the patch box which I attached.
>> The issue was corrected, but not know you whether this is good
>> technique.
>
> I'm sorry, but I don't think this is right.
>
> E_CCR_REGNUM is the physical register which gdb reads from the target.
> This is defined to be 4 bytes, even though only 1 byte is significant.
> It's just easier to keep most or all of the registers to the same size
> within the register message packet.
>
> E_PSEUDO_CCR_REGNUM is a "pseudo-register", which gdb computes internally
> (rather than requesting from the target).  It is defined to be 1 byte, and
> it is the register that gdb displays to the user.  In this case, the
> computation of the pseudo-register is simply copying the one significant
> byte from the physical register.

The size is converted by the access of a pseudo register.

> The fact that E_CCR_REGNUM has no display-name is deliberate.  That
> register is never displayed, it is only fetched from the target and
> used to obtain the value of E_PSEUDO_CCR_REGNUM (which is displayed).

Because legacy_register_sim_regno was not able to judge the register
number correctly, it corrected it.
However, it is necessary to evade this problem with register_sim_regno.
It corrects it like that.

> If you are working on the target side (gdb stub, simulator, or gdbserver),
> you only have to worry about E_CCR_REGNUM, but you need to supply it as
> four bytes.  I forget whether the MSB or the LSB is the significant one,
> but you should not have too much trouble to figure it out.  The other
> bytes can be any value - they'll be ignored.
>
> Michael
>

Thank you for the advice that becomes reference very much.

-- 
Yoshinori Sato
<ysato@users.sourceforge.jp>

Re: [RFC] h8300 "info registers" fix

[Yoshinori Sato]

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

At Wed, 13 Sep 2006 00:04:58 +0900,
Yoshinori Sato wrote:
> 
> [1  <text/plain; US-ASCII (7bit)>]
> A result of ccr and exr mistakes by a "info registers" of h8300 target.
> There was two faults.
> 
> - A name of E_CCR_REGNUM is not defined with h8300_register_name.
>   Accordingly regcache_raw_read of h8300_pseudo_regitser_read fails.
> - Assume size of E_PSEUDO_CCR_REGNUM by 1 byte with h8300_register_type.
>   But assume 4 bytes from a remote target.
>   Accordingly cannot take response of remote side to be really.
> 
> I correct an issue of a thing of the patch box which I attached.
> The issue was corrected, but not know you whether this is good technique.
> 
> -- 
> Yoshinori Sato
> <ysato@users.sourceforge.jp>
> 

I modified it in technique to think that I was appropriate.

-- 
Yoshinori Sato
<ysato@users.sourceforge.jp>


[-- Attachment #2: h8300-tdep.diff --]
[-- Type: application/octet-stream, Size: 2877 bytes --]

? h8300-tdep.diff
Index: ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/ChangeLog,v
retrieving revision 1.7897
diff -u -r1.7897 ChangeLog
--- ChangeLog	10 Sep 2006 16:21:03 -0000	1.7897
+++ ChangeLog	13 Sep 2006 15:49:27 -0000
@@ -1,3 +1,11 @@
+2006-09-13  Yoshinori Sato  <ysato@users.sourceforge.jp>
+
+	* h8300-tdep.c (h8300_pseud_register_read): fix E_CCR_REGNUM 
+	and E_EXR_REGNUM size. 
+	(h8300_pseudo_register_write): Ditto.
+	(h8300_sim_register_regno): New function.
+	(h8300_h8300_gdbarch_init): add regist h8300_sim_register_regno.
+
 2006-09-10  Daniel Jacobowitz  <dan@codesourcery.com>
 
 	PR threads/2149
Index: h8300-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/h8300-tdep.c,v
retrieving revision 1.103
diff -u -r1.103 h8300-tdep.c
--- h8300-tdep.c	17 Dec 2005 22:34:00 -0000	1.103
+++ h8300-tdep.c	13 Sep 2006 15:49:28 -0000
@@ -1148,10 +1148,18 @@
 			    struct regcache *regcache, int regno,
 			    gdb_byte *buf)
 {
+  gdb_byte tmp[4];
+
   if (regno == E_PSEUDO_CCR_REGNUM)
-    regcache_raw_read (regcache, E_CCR_REGNUM, buf);
+    {
+      regcache_raw_read (regcache, E_CCR_REGNUM, tmp);
+      *buf = tmp[3];
+    }
   else if (regno == E_PSEUDO_EXR_REGNUM)
-    regcache_raw_read (regcache, E_EXR_REGNUM, buf);
+    {
+      regcache_raw_read (regcache, E_EXR_REGNUM, tmp);
+      *buf = tmp[3];
+    }
   else
     regcache_raw_read (regcache, regno, buf);
 }
@@ -1161,10 +1169,18 @@
 			     struct regcache *regcache, int regno,
 			     const gdb_byte *buf)
 {
+  unsigned long tmp;
+
   if (regno == E_PSEUDO_CCR_REGNUM)
-    regcache_raw_write (regcache, E_CCR_REGNUM, buf);
+    {
+      tmp = extract_unsigned_integer(buf, 1);
+      regcache_raw_write (regcache, E_CCR_REGNUM, (gdb_byte *)&tmp);
+    }
   else if (regno == E_PSEUDO_EXR_REGNUM)
-    regcache_raw_write (regcache, E_EXR_REGNUM, buf);
+    {
+      tmp = extract_unsigned_integer(buf, 1);
+      regcache_raw_write (regcache, E_EXR_REGNUM, (gdb_byte *)&tmp);
+    }
   else
     regcache_raw_write (regcache, regno, buf);
 }
@@ -1205,6 +1221,19 @@
 No floating-point info available for this processor.\n");
 }
 
+static int
+h8300_register_sim_regno (int regno)
+{
+  switch (regno)
+    {
+    case E_CCR_REGNUM:
+    case E_EXR_REGNUM:
+      return regno;
+    default:
+      return legacy_register_sim_regno (regno);
+    }
+}
+
 static struct gdbarch *
 h8300_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 {
@@ -1319,6 +1348,7 @@
   set_gdbarch_register_type (gdbarch, h8300_register_type);
   set_gdbarch_print_registers_info (gdbarch, h8300_print_registers_info);
   set_gdbarch_print_float_info (gdbarch, h8300_print_float_info);
+  set_gdbarch_register_sim_regno(gdbarch, h8300_register_sim_regno);
 
   /*
    * Frame Info

RE: [RFC] h8300 "info registers" fix

[Michael Snyder]

At Wed, 13 Sep 2006 00:04:58 +0900,
Yoshinori Sato wrote:
> I modified it in technique to think that I was appropriate.

OK, much better.  Just one more question.

Your change to h8300_pseudo_register_read (and write): is it because
someone is calling the function with a one-byte buffer?  I assume so, 
but who is making that call?

Thanks,
Michael Snyder

Re: [RFC] h8300 "info registers" fix

[Daniel Jacobowitz]

On Wed, Sep 13, 2006 at 01:00:17PM -0700, Michael Snyder wrote:
> 
> At Wed, 13 Sep 2006 00:04:58 +0900,
> Yoshinori Sato wrote:
> > I modified it in technique to think that I was appropriate.
> 
> OK, much better.  Just one more question.
> 
> Your change to h8300_pseudo_register_read (and write): is it because
> someone is calling the function with a one-byte buffer?  I assume so, 
> but who is making that call?

I'm not familiar with this target, but are there host endianness
problems with this patch?  Casting an unsigned long * pointer to
a gdb_byte * pointer is very suspicious.

-- 
Daniel Jacobowitz
CodeSourcery

RE: [RFC] h8300 "info registers" fix

[Michael Snyder]

Grumble -- is the regcache kept in host order, or target order?
I can never remember.  Anyway, good call.  The read is done using
a gdb_byte[4], while the write is done using an unsigned long.
Surely they can't both be correct -- it should be one or the other.

Besides, a host unsigned long can't be right, 'cause we don't 
even know what size it is.  


-----Original Message-----
From: gdb-patches-owner@sourceware.org on behalf of Daniel Jacobowitz
Sent: Wed 9/13/2006 1:09 PM
To: gdb-patches@sourceware.org
Subject: Re: [RFC] h8300 "info registers" fix
 
On Wed, Sep 13, 2006 at 01:00:17PM -0700, Michael Snyder wrote:
> 
> At Wed, 13 Sep 2006 00:04:58 +0900,
> Yoshinori Sato wrote:
> > I modified it in technique to think that I was appropriate.
> 
> OK, much better.  Just one more question.
> 
> Your change to h8300_pseudo_register_read (and write): is it because
> someone is calling the function with a one-byte buffer?  I assume so, 
> but who is making that call?

I'm not familiar with this target, but are there host endianness
problems with this patch?  Casting an unsigned long * pointer to
a gdb_byte * pointer is very suspicious.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFC] h8300 "info registers" fix

[Daniel Jacobowitz]

On Wed, Sep 13, 2006 at 01:20:09PM -0700, Michael Snyder wrote:
> Grumble -- is the regcache kept in host order, or target order?

Target order.

[Which is apparently a bit weird; most debug interfaces I've seen
lately use host order.]

-- 
Daniel Jacobowitz
CodeSourcery

RE: [RFC] h8300 "info registers" fix

[Michael Snyder]

Daniel: 
>On Wed, Sep 13, 2006 at 01:20:09PM -0700, Michael Snyder wrote:
>> Grumble -- is the regcache kept in host order, or target order?
>
>Target order.
>
>[Which is apparently a bit weird; most debug interfaces I've seen
>lately use host order.]

That's right -- it's a relic of the fact that the original 
"register cache" was the register packet itself, which is 
generated on the target side.  The target doesn't know anything
about the host, so there's no way that can be in host order.

Anyway, in that case, the gdb_byte[4] approach is more likely
to be correct, eh?

I think I understand the problem now -- the pseudo-register is
only one byte, so it's "natural" to call pseudo_register_read
with a one byte buffer.  But the physical register is four bytes,
so you have to have a four byte buffer to read it.

Wonder why it ever worked?   ;-/

Re: [RFC] h8300 "info registers" fix

[Mark Kettenis]

> Date: Wed, 13 Sep 2006 13:49:26 -0700
> From: "Michael Snyder" <Michael.Snyder@palmsource.com>
> 
> Daniel: 
> >On Wed, Sep 13, 2006 at 01:20:09PM -0700, Michael Snyder wrote:
> >> Grumble -- is the regcache kept in host order, or target order?
> >
> >Target order.
> >
> >[Which is apparently a bit weird; most debug interfaces I've seen
> >lately use host order.]
> 
> That's right -- it's a relic of the fact that the original 
> "register cache" was the register packet itself, which is 
> generated on the target side.  The target doesn't know anything
> about the host, so there's no way that can be in host order.
> 
> Anyway, in that case, the gdb_byte[4] approach is more likely
> to be correct, eh?

Yes, gdb_byte is the type we use for target byte buffers.

> I think I understand the problem now -- the pseudo-register is
> only one byte, so it's "natural" to call pseudo_register_read
> with a one byte buffer.  But the physical register is four bytes,
> so you have to have a four byte buffer to read it.
> 
> Wonder why it ever worked?   ;-/

The fact that almost all variables on a 32-bit machines are
word-aligned?

Mark

Re: [RFC] h8300 "info registers" fix

[Mark Kettenis]

> Date: Wed, 13 Sep 2006 16:30:43 -0400
> From: Daniel Jacobowitz <drow@false.org>
> 
> On Wed, Sep 13, 2006 at 01:20:09PM -0700, Michael Snyder wrote:
> > Grumble -- is the regcache kept in host order, or target order?
> 
> Target order.
> 
> [Which is apparently a bit weird; most debug interfaces I've seen
> lately use host order.]

Not weird at all; it's the same convention we use for storing values.

Bet most debug interfaces you've seen lately actually use
little-endian byte order.

Mark

Re: [RFC] h8300 "info registers" fix

[Jim Blandy]

Mark Kettenis <mark.kettenis@xs4all.nl> writes:
>> Date: Wed, 13 Sep 2006 16:30:43 -0400
>> From: Daniel Jacobowitz <drow@false.org>
>> 
>> On Wed, Sep 13, 2006 at 01:20:09PM -0700, Michael Snyder wrote:
>> > Grumble -- is the regcache kept in host order, or target order?
>> 
>> Target order.
>> 
>> [Which is apparently a bit weird; most debug interfaces I've seen
>> lately use host order.]
>
> Not weird at all; it's the same convention we use for storing values.
>
> Bet most debug interfaces you've seen lately actually use
> little-endian byte order.

No, they're actually documented to return them in host byte order.
You read a register, you get an integer.

(Well, Daniel's seen more than me, but this is so of the ones I have
seen.)

Re: [RFC] h8300 "info registers" fix

[Daniel Jacobowitz]

On Fri, Sep 22, 2006 at 08:21:12PM -0700, Jim Blandy wrote:
> 
> Mark Kettenis <mark.kettenis@xs4all.nl> writes:
> > Not weird at all; it's the same convention we use for storing values.
> >
> > Bet most debug interfaces you've seen lately actually use
> > little-endian byte order.

They're frequently organized in integer multiples of the word size,
which are documented to return in host uint32_t's.  If the underlying
"register" is really bigger than that, then the stub is responsible
for composing.

> No, they're actually documented to return them in host byte order.
> You read a register, you get an integer.
> 
> (Well, Daniel's seen more than me, but this is so of the ones I have
> seen.)

Yep.  I can think offhand of four different JTAG-related debug
interfaces I've worked with in the last six months, all of which did
this.  Then, we transform the number from host endianness (on
the machine running the stub) to target endianness (the device)
before passing it over TCP/IP to GDB.  All a bit messy, but you lose
some no matter which order you pick :-)

-- 
Daniel Jacobowitz
CodeSourcery

RE: [RFC] h8300 "info registers" fix

[Yoshinori Sato]

>
> At Wed, 13 Sep 2006 00:04:58 +0900,
> Yoshinori Sato wrote:
>> I modified it in technique to think that I was appropriate.
>
> OK, much better.  Just one more question.
>
> Your change to h8300_pseudo_register_read (and write): is it because
> someone is calling the function with a one-byte buffer?  I assume so,
> but who is making that call?

E_PSEUDO_CCR_REGNUM and E_PSEUDO_EXR_REGNUM are defined in builtin_
type_uint8 by h8300_register_type.

>
> Thanks,
> Michael Snyder
>

-- 
Yoshinori Sato
<ysato@users.sourceforge.jp>

Re: [RFC] h8300 "info registers" fix

[Yoshinori Sato]

I corrected it not to depend on endian.

Index: ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/ChangeLog,v
retrieving revision 1.7901
diff -u -r1.7901 ChangeLog
--- ChangeLog	17 Sep 2006 15:48:34 -0000	1.7901
+++ ChangeLog	19 Sep 2006 15:55:15 -0000
@@ -1,3 +1,11 @@
+2006-09-19  Yoshinori Sato  <ysato@users.sourceforge.jp>
+
+	* h8300-tdep.c (h8300_pseud_register_read): fix E_CCR_REGNUM 
+	and E_EXR_REGNUM size. 
+	(h8300_pseudo_register_write): Ditto.
+	(h8300_sim_register_regno): New function.
+	(h8300_h8300_gdbarch_init): add regist h8300_sim_register_regno.
+
 2006-09-17  Vladimir Prus  <vladimir@codesourcery.com>
 
 	* mi/mi-cmd-stack.c (mi_cmd_stack_list_args): Don't emit error
Index: h8300-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/h8300-tdep.c,v
retrieving revision 1.103
diff -u -r1.103 h8300-tdep.c
--- h8300-tdep.c	17 Dec 2005 22:34:00 -0000	1.103
+++ h8300-tdep.c	19 Sep 2006 15:55:15 -0000
@@ -1148,10 +1148,20 @@
 			    struct regcache *regcache, int regno,
 			    gdb_byte *buf)
 {
+  unsigned long tmp;
+
   if (regno == E_PSEUDO_CCR_REGNUM)
-    regcache_raw_read (regcache, E_CCR_REGNUM, buf);
+    {
+      regcache_raw_read (regcache, E_CCR_REGNUM, (gdb_byte *)&tmp);
+      store_unsigned_integer((gdb_byte *)&tmp, 4, tmp);
+      *buf = tmp;
+    }
   else if (regno == E_PSEUDO_EXR_REGNUM)
-    regcache_raw_read (regcache, E_EXR_REGNUM, buf);
+    {
+      regcache_raw_read (regcache, E_EXR_REGNUM, (gdb_byte *)&tmp);
+      store_unsigned_integer((gdb_byte *)&tmp, 4, tmp);
+      *buf = tmp;
+    }
   else
     regcache_raw_read (regcache, regno, buf);
 }
@@ -1161,10 +1171,18 @@
 			     struct regcache *regcache, int regno,
 			     const gdb_byte *buf)
 {
+  unsigned long tmp;
+
   if (regno == E_PSEUDO_CCR_REGNUM)
-    regcache_raw_write (regcache, E_CCR_REGNUM, buf);
+    {
+      tmp = extract_unsigned_integer(buf, 1);
+      regcache_raw_write (regcache, E_CCR_REGNUM, (gdb_byte *)&tmp);
+    }
   else if (regno == E_PSEUDO_EXR_REGNUM)
-    regcache_raw_write (regcache, E_EXR_REGNUM, buf);
+    {
+      tmp = extract_unsigned_integer(buf, 1);
+      regcache_raw_write (regcache, E_EXR_REGNUM, (gdb_byte *)&tmp);
+    }
   else
     regcache_raw_write (regcache, regno, buf);
 }
@@ -1205,6 +1223,19 @@
 No floating-point info available for this processor.\n");
 }
 
+static int
+h8300_register_sim_regno (int regno)
+{
+  switch (regno)
+    {
+    case E_CCR_REGNUM:
+    case E_EXR_REGNUM:
+      return regno;
+    default:
+      return legacy_register_sim_regno (regno);
+    }
+}
+
 static struct gdbarch *
 h8300_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 {
@@ -1319,6 +1350,7 @@
   set_gdbarch_register_type (gdbarch, h8300_register_type);
   set_gdbarch_print_registers_info (gdbarch, h8300_print_registers_info);
   set_gdbarch_print_float_info (gdbarch, h8300_print_float_info);
+  set_gdbarch_register_sim_regno(gdbarch, h8300_register_sim_regno);
 
   /*
    * Frame Info

-- 
Yoshinori Sato
<ysato@users.sourceforge.jp>

Re: [RFC] h8300 "info registers" fix

[Mark Kettenis]

> Date: Wed, 20 Sep 2006 01:28:02 +0900
> From: Yoshinori Sato <ysato@users.sourceforge.jp>
> 
> I corrected it not to depend on endian.
> 
> Index: h8300-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/h8300-tdep.c,v
> retrieving revision 1.103
> diff -u -r1.103 h8300-tdep.c
> --- h8300-tdep.c	17 Dec 2005 22:34:00 -0000	1.103
> +++ h8300-tdep.c	19 Sep 2006 15:55:15 -0000
> @@ -1148,10 +1148,20 @@
>  			    struct regcache *regcache, int regno,
>  			    gdb_byte *buf)
>  {
> +  unsigned long tmp;
> +
>    if (regno == E_PSEUDO_CCR_REGNUM)
> -    regcache_raw_read (regcache, E_CCR_REGNUM, buf);
> +    {
> +      regcache_raw_read (regcache, E_CCR_REGNUM, (gdb_byte *)&tmp);
> +      store_unsigned_integer((gdb_byte *)&tmp, 4, tmp);
> +      *buf = tmp;
> +    }
>    else if (regno == E_PSEUDO_EXR_REGNUM)
> -    regcache_raw_read (regcache, E_EXR_REGNUM, buf);
> +    {
> +      regcache_raw_read (regcache, E_EXR_REGNUM, (gdb_byte *)&tmp);
> +      store_unsigned_integer((gdb_byte *)&tmp, 4, tmp);
> +      *buf = tmp;
> +    }

This is still wrong.  You'll need to read the raw register into a
properly sized gdb_byte array, take the right bits out of it and then
move it into the buffer.  You probably want something like

  gdb_byte tmp[4];

  regcache_raw_read(regcache, E_CCR_REGNUM, tmp)
  *buf = tmp[0];

or

  gdb_byte tmp[4];

  regcache_raw_read(regcache, E_CCR_REGNUM, tmp)
  *buf = tmp[3];

depending on whether h8300 is little- or big-endian.

Mark

Re: [RFC] h8300 "info registers" fix

[Yoshinori Sato]

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

A response is late sorry.

At Sat, 23 Sep 2006 20:00:16 +0200 (CEST),
Mark Kettenis wrote:
> 
> > Date: Wed, 20 Sep 2006 01:28:02 +0900
> > From: Yoshinori Sato <ysato@users.sourceforge.jp>
> > 
> > I corrected it not to depend on endian.
> > 
> > Index: h8300-tdep.c
> > ===================================================================
> > RCS file: /cvs/src/src/gdb/h8300-tdep.c,v
> > retrieving revision 1.103
> > diff -u -r1.103 h8300-tdep.c
> > --- h8300-tdep.c	17 Dec 2005 22:34:00 -0000	1.103
> > +++ h8300-tdep.c	19 Sep 2006 15:55:15 -0000
> > @@ -1148,10 +1148,20 @@
> >  			    struct regcache *regcache, int regno,
> >  			    gdb_byte *buf)
> >  {
> > +  unsigned long tmp;
> > +
> >    if (regno == E_PSEUDO_CCR_REGNUM)
> > -    regcache_raw_read (regcache, E_CCR_REGNUM, buf);
> > +    {
> > +      regcache_raw_read (regcache, E_CCR_REGNUM, (gdb_byte *)&tmp);
> > +      store_unsigned_integer((gdb_byte *)&tmp, 4, tmp);
> > +      *buf = tmp;
> > +    }
> >    else if (regno == E_PSEUDO_EXR_REGNUM)
> > -    regcache_raw_read (regcache, E_EXR_REGNUM, buf);
> > +    {
> > +      regcache_raw_read (regcache, E_EXR_REGNUM, (gdb_byte *)&tmp);
> > +      store_unsigned_integer((gdb_byte *)&tmp, 4, tmp);
> > +      *buf = tmp;
> > +    }
> 
> This is still wrong.  You'll need to read the raw register into a
> properly sized gdb_byte array, take the right bits out of it and then
> move it into the buffer.  You probably want something like
> 
>   gdb_byte tmp[4];
> 
>   regcache_raw_read(regcache, E_CCR_REGNUM, tmp)
>   *buf = tmp[0];
> 
> or
> 
>   gdb_byte tmp[4];
> 
>   regcache_raw_read(regcache, E_CCR_REGNUM, tmp)
>   *buf = tmp[3];
> 
> depending on whether h8300 is little- or big-endian.
> 
> Mark


[-- Attachment #2: h8300.diff --]
[-- Type: application/octet-stream, Size: 2925 bytes --]

Index: ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/ChangeLog,v
retrieving revision 1.7960
diff -u -r1.7960 ChangeLog
--- ChangeLog	13 Nov 2006 19:05:50 -0000	1.7960
+++ ChangeLog	16 Nov 2006 07:16:29 -0000
@@ -1,3 +1,11 @@
+2006-11-16  Yoshinori Sato  <ysato@users.sourceforge.jp>
+
+	* h8300-tdep.c (h8300_pseud_register_read): fix E_CCR_REGNUM 
+	and E_EXR_REGNUM size. 
+	(h8300_pseudo_register_write): Ditto.
+	(h8300_sim_register_regno): New function.
+	(h8300_h8300_gdbarch_init): add regist h8300_sim_register_regno.
+
 2006-11-13  Paul Gilliam  <pgilliam@us.ibm.com>
 
 	* ppc-linux-tdep.c (ppc_linux_sigtramp_cache): Don't futz with
Index: h8300-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/h8300-tdep.c,v
retrieving revision 1.103
diff -u -r1.103 h8300-tdep.c
--- h8300-tdep.c	17 Dec 2005 22:34:00 -0000	1.103
+++ h8300-tdep.c	16 Nov 2006 07:16:29 -0000
@@ -1148,10 +1148,18 @@
 			    struct regcache *regcache, int regno,
 			    gdb_byte *buf)
 {
+  gdb_byte tmp[4];
+
   if (regno == E_PSEUDO_CCR_REGNUM)
-    regcache_raw_read (regcache, E_CCR_REGNUM, buf);
+    {
+      regcache_raw_read (regcache, E_CCR_REGNUM, tmp);
+      *buf = tmp[3];	/* h8300 is bigendian */
+    }
   else if (regno == E_PSEUDO_EXR_REGNUM)
-    regcache_raw_read (regcache, E_EXR_REGNUM, buf);
+    {
+      regcache_raw_read (regcache, E_EXR_REGNUM, tmp);
+      *buf = tmp[3];
+    }
   else
     regcache_raw_read (regcache, regno, buf);
 }
@@ -1161,10 +1169,18 @@
 			     struct regcache *regcache, int regno,
 			     const gdb_byte *buf)
 {
+  unsigned long tmp;
+
   if (regno == E_PSEUDO_CCR_REGNUM)
-    regcache_raw_write (regcache, E_CCR_REGNUM, buf);
+    {
+      tmp = extract_unsigned_integer(buf, 1);
+      regcache_raw_write (regcache, E_CCR_REGNUM, (gdb_byte *)&tmp);
+    }
   else if (regno == E_PSEUDO_EXR_REGNUM)
-    regcache_raw_write (regcache, E_EXR_REGNUM, buf);
+    {
+      tmp = extract_unsigned_integer(buf, 1);
+      regcache_raw_write (regcache, E_EXR_REGNUM, (gdb_byte *)&tmp);
+    }
   else
     regcache_raw_write (regcache, regno, buf);
 }
@@ -1205,6 +1221,19 @@
 No floating-point info available for this processor.\n");
 }
 
+static int
+h8300_register_sim_regno (int regno)
+{
+  switch (regno)
+    {
+    case E_CCR_REGNUM:
+    case E_EXR_REGNUM:
+      return regno;
+    default:
+      return legacy_register_sim_regno (regno);
+    }
+}
+
 static struct gdbarch *
 h8300_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 {
@@ -1319,6 +1348,7 @@
   set_gdbarch_register_type (gdbarch, h8300_register_type);
   set_gdbarch_print_registers_info (gdbarch, h8300_print_registers_info);
   set_gdbarch_print_float_info (gdbarch, h8300_print_float_info);
+  set_gdbarch_register_sim_regno(gdbarch, h8300_register_sim_regno);
 
   /*
    * Frame Info