Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [RFA/m32r] Fix breakpoint bug
@ 2004-10-07  6:32 Kei Sakamoto
  2004-10-18  6:43 ` [RFA/m32r] Unreviewed patch (breakpoint bug) Kei Sakamoto
  2004-10-25 16:19 ` [RFA/m32r] Fix breakpoint bug Daniel Jacobowitz
  0 siblings, 2 replies; 8+ messages in thread
From: Kei Sakamoto @ 2004-10-07  6:32 UTC (permalink / raw)
  To: gdb-patches

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

Hello,

m32r-tdep.c can't handle breakpoints on instructions executed
in parallel. It does not remove unnecessary parallel execution
bit from instructions and causes illegal instruction errors.

The attached patch fixes this problem.

OK to commit?

2004-10-07    Kei Sakamoto  <sakamoto.kei@renesas.com>

    * m32r-tdep.c (m32r_memory_insert_breakpoint): Remove
    unnecessary parallel execution bit.
    (m32r_memory_remove_breakpoint): Ditto.
    (m32r_breakpoint_from_pc): Update.

[-- Attachment #2: m32r-tdep.patch --]
[-- Type: application/octet-stream, Size: 5182 bytes --]

Index: m32r-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/m32r-tdep.c,v
retrieving revision 1.33
diff -u -r1.33 m32r-tdep.c
--- m32r-tdep.c	7 Oct 2004 01:21:53 -0000	1.33
+++ m32r-tdep.c	7 Oct 2004 04:27:00 -0000
@@ -56,62 +56,56 @@
 }
 
 
-/* BREAKPOINT */
-#define M32R_BE_BREAKPOINT32 {0x10, 0xf1, 0x70, 0x00}
-#define M32R_LE_BREAKPOINT32 {0xf1, 0x10, 0x00, 0x70}
-#define M32R_BE_BREAKPOINT16 {0x10, 0xf1}
-#define M32R_LE_BREAKPOINT16 {0xf1, 0x10}
-
 static int
 m32r_memory_insert_breakpoint (CORE_ADDR addr, char *contents_cache)
 {
   int val;
-  unsigned char *bp;
-  int bplen;
-
-  bplen = (addr & 3) ? 2 : 4;
+  char buf[4];
+  char bp_entry[] = { 0x10, 0xf1 };	/* dpt */
 
   /* Save the memory contents.  */
-  val = target_read_memory (addr, contents_cache, bplen);
+  val = target_read_memory (addr & 0xfffffffc, contents_cache, 4);
   if (val != 0)
     return val;			/* return error */
 
   /* Determine appropriate breakpoint contents and size for this address.  */
   if (TARGET_BYTE_ORDER == BFD_ENDIAN_BIG)
     {
-      if (((addr & 3) == 0)
-	  && ((contents_cache[0] & 0x80) || (contents_cache[2] & 0x80)))
+      if ((addr & 3) == 0)
 	{
-	  static unsigned char insn[] = M32R_BE_BREAKPOINT32;
-	  bp = insn;
-	  bplen = sizeof (insn);
+	  buf[0] = bp_entry[0];
+	  buf[1] = bp_entry[1];
+	  buf[2] = contents_cache[2] & 0x7f;
+	  buf[3] = contents_cache[3];
 	}
       else
 	{
-	  static unsigned char insn[] = M32R_BE_BREAKPOINT16;
-	  bp = insn;
-	  bplen = sizeof (insn);
+	  buf[0] = contents_cache[0];
+	  buf[1] = contents_cache[1];
+	  buf[2] = bp_entry[0];
+	  buf[3] = bp_entry[1];
 	}
     }
-  else
-    {				/* little-endian */
-      if (((addr & 3) == 0)
-	  && ((contents_cache[1] & 0x80) || (contents_cache[3] & 0x80)))
-	{
-	  static unsigned char insn[] = M32R_LE_BREAKPOINT32;
-	  bp = insn;
-	  bplen = sizeof (insn);
+  else				/* little-endian */
+    {
+      if ((addr & 3) == 0)
+	{
+	  buf[0] = contents_cache[0];
+	  buf[1] = contents_cache[1] & 0x7f;
+	  buf[2] = bp_entry[1];
+	  buf[3] = bp_entry[0];
 	}
       else
 	{
-	  static unsigned char insn[] = M32R_LE_BREAKPOINT16;
-	  bp = insn;
-	  bplen = sizeof (insn);
+	  buf[0] = bp_entry[1];
+	  buf[1] = bp_entry[0];
+	  buf[2] = contents_cache[2];
+	  buf[3] = contents_cache[3];
 	}
     }
 
   /* Write the breakpoint.  */
-  val = target_write_memory (addr, (char *) bp, bplen);
+  val = target_write_memory (addr & 0xfffffffc, buf, 4);
   return val;
 }
 
@@ -119,47 +113,35 @@
 m32r_memory_remove_breakpoint (CORE_ADDR addr, char *contents_cache)
 {
   int val;
-  int bplen;
+  char buf[4];
 
-  /* Determine appropriate breakpoint contents and size for this address.  */
+  buf[0] = contents_cache[0];
+  buf[1] = contents_cache[1];
+  buf[2] = contents_cache[2];
+  buf[3] = contents_cache[3];
+
+  /* Remove parallel bit.  */
   if (TARGET_BYTE_ORDER == BFD_ENDIAN_BIG)
     {
-      if (((addr & 3) == 0)
-	  && ((contents_cache[0] & 0x80) || (contents_cache[2] & 0x80)))
-	{
-	  static unsigned char insn[] = M32R_BE_BREAKPOINT32;
-	  bplen = sizeof (insn);
-	}
-      else
-	{
-	  static unsigned char insn[] = M32R_BE_BREAKPOINT16;
-	  bplen = sizeof (insn);
-	}
+      if ((buf[0] & 0x80) == 0 && (buf[2] & 0x80) != 0)
+	buf[2] &= 0x7f;
     }
-  else
+  else				/* little-endian */
     {
-      /* little-endian */
-      if (((addr & 3) == 0)
-	  && ((contents_cache[1] & 0x80) || (contents_cache[3] & 0x80)))
-	{
-	  static unsigned char insn[] = M32R_BE_BREAKPOINT32;
-	  bplen = sizeof (insn);
-	}
-      else
-	{
-	  static unsigned char insn[] = M32R_BE_BREAKPOINT16;
-	  bplen = sizeof (insn);
-	}
+      if ((buf[3] & 0x80) == 0 && (buf[1] & 0x80) != 0)
+	buf[1] &= 0x7f;
     }
 
   /* Write contents.  */
-  val = target_write_memory (addr, contents_cache, bplen);
+  val = target_write_memory (addr & 0xfffffffc, buf, 4);
   return val;
 }
 
 static const unsigned char *
 m32r_breakpoint_from_pc (CORE_ADDR *pcptr, int *lenptr)
 {
+  static char be_bp_entry[] = { 0x10, 0xf1, 0x70, 0x00 };	/* dpt -> nop */
+  static char le_bp_entry[] = { 0x00, 0x70, 0xf1, 0x10 };	/* dpt -> nop */
   unsigned char *bp;
 
   /* Determine appropriate breakpoint.  */
@@ -167,30 +149,26 @@
     {
       if ((*pcptr & 3) == 0)
 	{
-	  static unsigned char insn[] = M32R_BE_BREAKPOINT32;
-	  bp = insn;
-	  *lenptr = sizeof (insn);
+	  bp = be_bp_entry;
+	  *lenptr = 4;
 	}
       else
 	{
-	  static unsigned char insn[] = M32R_BE_BREAKPOINT16;
-	  bp = insn;
-	  *lenptr = sizeof (insn);
+	  bp = be_bp_entry;
+	  *lenptr = 2;
 	}
     }
   else
     {
       if ((*pcptr & 3) == 0)
 	{
-	  static unsigned char insn[] = M32R_LE_BREAKPOINT32;
-	  bp = insn;
-	  *lenptr = sizeof (insn);
+	  bp = le_bp_entry;
+	  *lenptr = 4;
 	}
       else
 	{
-	  static unsigned char insn[] = M32R_LE_BREAKPOINT16;
-	  bp = insn;
-	  *lenptr = sizeof (insn);
+	  bp = le_bp_entry + 2;
+	  *lenptr = 2;
 	}
     }
 

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

* [RFA/m32r] Unreviewed patch (breakpoint bug)
  2004-10-07  6:32 [RFA/m32r] Fix breakpoint bug Kei Sakamoto
@ 2004-10-18  6:43 ` Kei Sakamoto
  2004-10-25  9:02   ` Kei Sakamoto
  2004-10-25 16:19 ` [RFA/m32r] Fix breakpoint bug Daniel Jacobowitz
  1 sibling, 1 reply; 8+ messages in thread
From: Kei Sakamoto @ 2004-10-18  6:43 UTC (permalink / raw)
  To: gdb-patches

Hello,

Would anyone review this patch?
Thank you in advance.

Kei Sakamoto

From: "Kei Sakamoto" <sakamoto.kei@renesas.com>
To: <gdb-patches@sources.redhat.com>
Sent: Thursday, October 07, 2004 3:31 PM
Subject: [RFA/m32r] Fix breakpoint bug


> Hello,
> 
> m32r-tdep.c can't handle breakpoints on instructions executed
> in parallel. It does not remove unnecessary parallel execution
> bit from instructions and causes illegal instruction errors.
> 
> The attached patch fixes this problem.
> 
> OK to commit?
> 
> 2004-10-07    Kei Sakamoto  <sakamoto.kei@renesas.com>
> 
>     * m32r-tdep.c (m32r_memory_insert_breakpoint): Remove
>     unnecessary parallel execution bit.
>     (m32r_memory_remove_breakpoint): Ditto.
>     (m32r_breakpoint_from_pc): Update.
> 


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

* [RFA/m32r] Unreviewed patch (breakpoint bug)
  2004-10-18  6:43 ` [RFA/m32r] Unreviewed patch (breakpoint bug) Kei Sakamoto
@ 2004-10-25  9:02   ` Kei Sakamoto
  2004-11-01  1:08     ` Kei Sakamoto
  0 siblings, 1 reply; 8+ messages in thread
From: Kei Sakamoto @ 2004-10-25  9:02 UTC (permalink / raw)
  To: gdb-patches

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

This is the second ping.
Please review the following patch.

Kei Sakamoto

From: "Kei Sakamoto" <sakamoto.kei@renesas.com>
To: <gdb-patches@sources.redhat.com>
Sent: Monday, October 18, 2004 3:43 PM
Subject: [RFA/m32r] Unreviewed patch (breakpoint bug)


> Hello,
> 
> Would anyone review this patch?
> Thank you in advance.
> 
> Kei Sakamoto
> 
> From: "Kei Sakamoto" <sakamoto.kei@renesas.com>
> To: <gdb-patches@sources.redhat.com>
> Sent: Thursday, October 07, 2004 3:31 PM
> Subject: [RFA/m32r] Fix breakpoint bug
> 
> 
> > Hello,
> > 
> > m32r-tdep.c can't handle breakpoints on instructions executed
> > in parallel. It does not remove unnecessary parallel execution
> > bit from instructions and causes illegal instruction errors.
> > 
> > The attached patch fixes this problem.
> > 
> > OK to commit?
> > 
> > 2004-10-07    Kei Sakamoto  <sakamoto.kei@renesas.com>
> > 
> >     * m32r-tdep.c (m32r_memory_insert_breakpoint): Remove
> >     unnecessary parallel execution bit.
> >     (m32r_memory_remove_breakpoint): Ditto.
> >     (m32r_breakpoint_from_pc): Update.
> > 
> 

[-- Attachment #2: m32r-tdep.patch --]
[-- Type: application/octet-stream, Size: 5182 bytes --]

Index: m32r-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/m32r-tdep.c,v
retrieving revision 1.33
diff -u -r1.33 m32r-tdep.c
--- m32r-tdep.c	7 Oct 2004 01:21:53 -0000	1.33
+++ m32r-tdep.c	7 Oct 2004 04:27:00 -0000
@@ -56,62 +56,56 @@
 }
 
 
-/* BREAKPOINT */
-#define M32R_BE_BREAKPOINT32 {0x10, 0xf1, 0x70, 0x00}
-#define M32R_LE_BREAKPOINT32 {0xf1, 0x10, 0x00, 0x70}
-#define M32R_BE_BREAKPOINT16 {0x10, 0xf1}
-#define M32R_LE_BREAKPOINT16 {0xf1, 0x10}
-
 static int
 m32r_memory_insert_breakpoint (CORE_ADDR addr, char *contents_cache)
 {
   int val;
-  unsigned char *bp;
-  int bplen;
-
-  bplen = (addr & 3) ? 2 : 4;
+  char buf[4];
+  char bp_entry[] = { 0x10, 0xf1 };	/* dpt */
 
   /* Save the memory contents.  */
-  val = target_read_memory (addr, contents_cache, bplen);
+  val = target_read_memory (addr & 0xfffffffc, contents_cache, 4);
   if (val != 0)
     return val;			/* return error */
 
   /* Determine appropriate breakpoint contents and size for this address.  */
   if (TARGET_BYTE_ORDER == BFD_ENDIAN_BIG)
     {
-      if (((addr & 3) == 0)
-	  && ((contents_cache[0] & 0x80) || (contents_cache[2] & 0x80)))
+      if ((addr & 3) == 0)
 	{
-	  static unsigned char insn[] = M32R_BE_BREAKPOINT32;
-	  bp = insn;
-	  bplen = sizeof (insn);
+	  buf[0] = bp_entry[0];
+	  buf[1] = bp_entry[1];
+	  buf[2] = contents_cache[2] & 0x7f;
+	  buf[3] = contents_cache[3];
 	}
       else
 	{
-	  static unsigned char insn[] = M32R_BE_BREAKPOINT16;
-	  bp = insn;
-	  bplen = sizeof (insn);
+	  buf[0] = contents_cache[0];
+	  buf[1] = contents_cache[1];
+	  buf[2] = bp_entry[0];
+	  buf[3] = bp_entry[1];
 	}
     }
-  else
-    {				/* little-endian */
-      if (((addr & 3) == 0)
-	  && ((contents_cache[1] & 0x80) || (contents_cache[3] & 0x80)))
-	{
-	  static unsigned char insn[] = M32R_LE_BREAKPOINT32;
-	  bp = insn;
-	  bplen = sizeof (insn);
+  else				/* little-endian */
+    {
+      if ((addr & 3) == 0)
+	{
+	  buf[0] = contents_cache[0];
+	  buf[1] = contents_cache[1] & 0x7f;
+	  buf[2] = bp_entry[1];
+	  buf[3] = bp_entry[0];
 	}
       else
 	{
-	  static unsigned char insn[] = M32R_LE_BREAKPOINT16;
-	  bp = insn;
-	  bplen = sizeof (insn);
+	  buf[0] = bp_entry[1];
+	  buf[1] = bp_entry[0];
+	  buf[2] = contents_cache[2];
+	  buf[3] = contents_cache[3];
 	}
     }
 
   /* Write the breakpoint.  */
-  val = target_write_memory (addr, (char *) bp, bplen);
+  val = target_write_memory (addr & 0xfffffffc, buf, 4);
   return val;
 }
 
@@ -119,47 +113,35 @@
 m32r_memory_remove_breakpoint (CORE_ADDR addr, char *contents_cache)
 {
   int val;
-  int bplen;
+  char buf[4];
 
-  /* Determine appropriate breakpoint contents and size for this address.  */
+  buf[0] = contents_cache[0];
+  buf[1] = contents_cache[1];
+  buf[2] = contents_cache[2];
+  buf[3] = contents_cache[3];
+
+  /* Remove parallel bit.  */
   if (TARGET_BYTE_ORDER == BFD_ENDIAN_BIG)
     {
-      if (((addr & 3) == 0)
-	  && ((contents_cache[0] & 0x80) || (contents_cache[2] & 0x80)))
-	{
-	  static unsigned char insn[] = M32R_BE_BREAKPOINT32;
-	  bplen = sizeof (insn);
-	}
-      else
-	{
-	  static unsigned char insn[] = M32R_BE_BREAKPOINT16;
-	  bplen = sizeof (insn);
-	}
+      if ((buf[0] & 0x80) == 0 && (buf[2] & 0x80) != 0)
+	buf[2] &= 0x7f;
     }
-  else
+  else				/* little-endian */
     {
-      /* little-endian */
-      if (((addr & 3) == 0)
-	  && ((contents_cache[1] & 0x80) || (contents_cache[3] & 0x80)))
-	{
-	  static unsigned char insn[] = M32R_BE_BREAKPOINT32;
-	  bplen = sizeof (insn);
-	}
-      else
-	{
-	  static unsigned char insn[] = M32R_BE_BREAKPOINT16;
-	  bplen = sizeof (insn);
-	}
+      if ((buf[3] & 0x80) == 0 && (buf[1] & 0x80) != 0)
+	buf[1] &= 0x7f;
     }
 
   /* Write contents.  */
-  val = target_write_memory (addr, contents_cache, bplen);
+  val = target_write_memory (addr & 0xfffffffc, buf, 4);
   return val;
 }
 
 static const unsigned char *
 m32r_breakpoint_from_pc (CORE_ADDR *pcptr, int *lenptr)
 {
+  static char be_bp_entry[] = { 0x10, 0xf1, 0x70, 0x00 };	/* dpt -> nop */
+  static char le_bp_entry[] = { 0x00, 0x70, 0xf1, 0x10 };	/* dpt -> nop */
   unsigned char *bp;
 
   /* Determine appropriate breakpoint.  */
@@ -167,30 +149,26 @@
     {
       if ((*pcptr & 3) == 0)
 	{
-	  static unsigned char insn[] = M32R_BE_BREAKPOINT32;
-	  bp = insn;
-	  *lenptr = sizeof (insn);
+	  bp = be_bp_entry;
+	  *lenptr = 4;
 	}
       else
 	{
-	  static unsigned char insn[] = M32R_BE_BREAKPOINT16;
-	  bp = insn;
-	  *lenptr = sizeof (insn);
+	  bp = be_bp_entry;
+	  *lenptr = 2;
 	}
     }
   else
     {
       if ((*pcptr & 3) == 0)
 	{
-	  static unsigned char insn[] = M32R_LE_BREAKPOINT32;
-	  bp = insn;
-	  *lenptr = sizeof (insn);
+	  bp = le_bp_entry;
+	  *lenptr = 4;
 	}
       else
 	{
-	  static unsigned char insn[] = M32R_LE_BREAKPOINT16;
-	  bp = insn;
-	  *lenptr = sizeof (insn);
+	  bp = le_bp_entry + 2;
+	  *lenptr = 2;
 	}
     }
 

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

* Re: [RFA/m32r] Fix breakpoint bug
  2004-10-07  6:32 [RFA/m32r] Fix breakpoint bug Kei Sakamoto
  2004-10-18  6:43 ` [RFA/m32r] Unreviewed patch (breakpoint bug) Kei Sakamoto
@ 2004-10-25 16:19 ` Daniel Jacobowitz
  2004-10-26  5:45   ` Kei Sakamoto
  1 sibling, 1 reply; 8+ messages in thread
From: Daniel Jacobowitz @ 2004-10-25 16:19 UTC (permalink / raw)
  To: Kei Sakamoto; +Cc: gdb-patches

On Thu, Oct 07, 2004 at 03:31:07PM +0900, Kei Sakamoto wrote:
> Hello,
> 
> m32r-tdep.c can't handle breakpoints on instructions executed
> in parallel. It does not remove unnecessary parallel execution
> bit from instructions and causes illegal instruction errors.
> 
> The attached patch fixes this problem.
> 
> OK to commit?
> 
> 2004-10-07    Kei Sakamoto  <sakamoto.kei@renesas.com>
> 
>     * m32r-tdep.c (m32r_memory_insert_breakpoint): Remove
>     unnecessary parallel execution bit.
>     (m32r_memory_remove_breakpoint): Ditto.
>     (m32r_breakpoint_from_pc): Update.

I'm curious about the little endian case:

+  else				/* little-endian */
+    {
+      if ((addr & 3) == 0)
+	{
+	  buf[0] = contents_cache[0];
+	  buf[1] = contents_cache[1] & 0x7f;
+	  buf[2] = bp_entry[1];
+	  buf[3] = bp_entry[0];
 	}

Shouldn't the breakpoint be placed at buf[0] here rather than buf[2]?


-- 
Daniel Jacobowitz


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

* Re: [RFA/m32r] Fix breakpoint bug
  2004-10-25 16:19 ` [RFA/m32r] Fix breakpoint bug Daniel Jacobowitz
@ 2004-10-26  5:45   ` Kei Sakamoto
  2004-11-04  0:21     ` Andrew Cagney
  0 siblings, 1 reply; 8+ messages in thread
From: Kei Sakamoto @ 2004-10-26  5:45 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: gdb-patches

> I'm curious about the little endian case:
> 
> +  else /* little-endian */
> +    {
> +      if ((addr & 3) == 0)
> + {
> +   buf[0] = contents_cache[0];
> +   buf[1] = contents_cache[1] & 0x7f;
> +   buf[2] = bp_entry[1];
> +   buf[3] = bp_entry[0];
>   }
> 
> Shouldn't the breakpoint be placed at buf[0] here rather than buf[2]?

For most of architectures - yes, it shold be at buf[0]. But the little endian
mode of M32R is a kind of unique.

In other architectures, two 16-bit instructions, A and B, are placed as
the following:

Big endian:
A0 A1 B0 B1

Little endian:
A1 A0  B1 B0

In M32R, they are placed like this:

Big endian:
A0 A1 B0 B1

Little endian:
B1 B0 A1 A0

This is because M32R always fetches instructions in 32-bit.
So the breakpoint should be placed at buf[2].

Kei Sakamoto


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

* [RFA/m32r] Unreviewed patch (breakpoint bug)
  2004-10-25  9:02   ` Kei Sakamoto
@ 2004-11-01  1:08     ` Kei Sakamoto
  0 siblings, 0 replies; 8+ messages in thread
From: Kei Sakamoto @ 2004-11-01  1:08 UTC (permalink / raw)
  To: gdb-patches

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

This is the third ping. It has been three weeks since I posted the
following patch...
Would anyone review it?

Kei Sakamoto

From: "Kei Sakamoto" <sakamoto.kei@renesas.com>
To: <gdb-patches@sources.redhat.com>
Sent: Monday, October 25, 2004 6:03 PM
Subject: [RFA/m32r] Unreviewed patch (breakpoint bug)


> This is the second ping.
> Please review the following patch.
> 
> Kei Sakamoto
> 
> From: "Kei Sakamoto" <sakamoto.kei@renesas.com>
> To: <gdb-patches@sources.redhat.com>
> Sent: Monday, October 18, 2004 3:43 PM
> Subject: [RFA/m32r] Unreviewed patch (breakpoint bug)
> 
> > Hello,
> > 
> > Would anyone review this patch?
> > Thank you in advance.
> > 
> > Kei Sakamoto
> > 
> > From: "Kei Sakamoto" <sakamoto.kei@renesas.com>
> > To: <gdb-patches@sources.redhat.com>
> > Sent: Thursday, October 07, 2004 3:31 PM
> > Subject: [RFA/m32r] Fix breakpoint bug
> > 
> > > Hello,
> > > 
> > > m32r-tdep.c can't handle breakpoints on instructions executed
> > > in parallel. It does not remove unnecessary parallel execution
> > > bit from instructions and causes illegal instruction errors.
> > > 
> > > The attached patch fixes this problem.
> > > 
> > > OK to commit?
> > > 
> > > 2004-10-07    Kei Sakamoto  <sakamoto.kei@renesas.com>
> > > 
> > >     * m32r-tdep.c (m32r_memory_insert_breakpoint): Remove
> > >     unnecessary parallel execution bit.
> > >     (m32r_memory_remove_breakpoint): Ditto.
> > >     (m32r_breakpoint_from_pc): Update.

[-- Attachment #2: m32r-tdep.patch --]
[-- Type: application/octet-stream, Size: 5182 bytes --]

Index: m32r-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/m32r-tdep.c,v
retrieving revision 1.33
diff -u -r1.33 m32r-tdep.c
--- m32r-tdep.c	7 Oct 2004 01:21:53 -0000	1.33
+++ m32r-tdep.c	7 Oct 2004 04:27:00 -0000
@@ -56,62 +56,56 @@
 }
 
 
-/* BREAKPOINT */
-#define M32R_BE_BREAKPOINT32 {0x10, 0xf1, 0x70, 0x00}
-#define M32R_LE_BREAKPOINT32 {0xf1, 0x10, 0x00, 0x70}
-#define M32R_BE_BREAKPOINT16 {0x10, 0xf1}
-#define M32R_LE_BREAKPOINT16 {0xf1, 0x10}
-
 static int
 m32r_memory_insert_breakpoint (CORE_ADDR addr, char *contents_cache)
 {
   int val;
-  unsigned char *bp;
-  int bplen;
-
-  bplen = (addr & 3) ? 2 : 4;
+  char buf[4];
+  char bp_entry[] = { 0x10, 0xf1 };	/* dpt */
 
   /* Save the memory contents.  */
-  val = target_read_memory (addr, contents_cache, bplen);
+  val = target_read_memory (addr & 0xfffffffc, contents_cache, 4);
   if (val != 0)
     return val;			/* return error */
 
   /* Determine appropriate breakpoint contents and size for this address.  */
   if (TARGET_BYTE_ORDER == BFD_ENDIAN_BIG)
     {
-      if (((addr & 3) == 0)
-	  && ((contents_cache[0] & 0x80) || (contents_cache[2] & 0x80)))
+      if ((addr & 3) == 0)
 	{
-	  static unsigned char insn[] = M32R_BE_BREAKPOINT32;
-	  bp = insn;
-	  bplen = sizeof (insn);
+	  buf[0] = bp_entry[0];
+	  buf[1] = bp_entry[1];
+	  buf[2] = contents_cache[2] & 0x7f;
+	  buf[3] = contents_cache[3];
 	}
       else
 	{
-	  static unsigned char insn[] = M32R_BE_BREAKPOINT16;
-	  bp = insn;
-	  bplen = sizeof (insn);
+	  buf[0] = contents_cache[0];
+	  buf[1] = contents_cache[1];
+	  buf[2] = bp_entry[0];
+	  buf[3] = bp_entry[1];
 	}
     }
-  else
-    {				/* little-endian */
-      if (((addr & 3) == 0)
-	  && ((contents_cache[1] & 0x80) || (contents_cache[3] & 0x80)))
-	{
-	  static unsigned char insn[] = M32R_LE_BREAKPOINT32;
-	  bp = insn;
-	  bplen = sizeof (insn);
+  else				/* little-endian */
+    {
+      if ((addr & 3) == 0)
+	{
+	  buf[0] = contents_cache[0];
+	  buf[1] = contents_cache[1] & 0x7f;
+	  buf[2] = bp_entry[1];
+	  buf[3] = bp_entry[0];
 	}
       else
 	{
-	  static unsigned char insn[] = M32R_LE_BREAKPOINT16;
-	  bp = insn;
-	  bplen = sizeof (insn);
+	  buf[0] = bp_entry[1];
+	  buf[1] = bp_entry[0];
+	  buf[2] = contents_cache[2];
+	  buf[3] = contents_cache[3];
 	}
     }
 
   /* Write the breakpoint.  */
-  val = target_write_memory (addr, (char *) bp, bplen);
+  val = target_write_memory (addr & 0xfffffffc, buf, 4);
   return val;
 }
 
@@ -119,47 +113,35 @@
 m32r_memory_remove_breakpoint (CORE_ADDR addr, char *contents_cache)
 {
   int val;
-  int bplen;
+  char buf[4];
 
-  /* Determine appropriate breakpoint contents and size for this address.  */
+  buf[0] = contents_cache[0];
+  buf[1] = contents_cache[1];
+  buf[2] = contents_cache[2];
+  buf[3] = contents_cache[3];
+
+  /* Remove parallel bit.  */
   if (TARGET_BYTE_ORDER == BFD_ENDIAN_BIG)
     {
-      if (((addr & 3) == 0)
-	  && ((contents_cache[0] & 0x80) || (contents_cache[2] & 0x80)))
-	{
-	  static unsigned char insn[] = M32R_BE_BREAKPOINT32;
-	  bplen = sizeof (insn);
-	}
-      else
-	{
-	  static unsigned char insn[] = M32R_BE_BREAKPOINT16;
-	  bplen = sizeof (insn);
-	}
+      if ((buf[0] & 0x80) == 0 && (buf[2] & 0x80) != 0)
+	buf[2] &= 0x7f;
     }
-  else
+  else				/* little-endian */
     {
-      /* little-endian */
-      if (((addr & 3) == 0)
-	  && ((contents_cache[1] & 0x80) || (contents_cache[3] & 0x80)))
-	{
-	  static unsigned char insn[] = M32R_BE_BREAKPOINT32;
-	  bplen = sizeof (insn);
-	}
-      else
-	{
-	  static unsigned char insn[] = M32R_BE_BREAKPOINT16;
-	  bplen = sizeof (insn);
-	}
+      if ((buf[3] & 0x80) == 0 && (buf[1] & 0x80) != 0)
+	buf[1] &= 0x7f;
     }
 
   /* Write contents.  */
-  val = target_write_memory (addr, contents_cache, bplen);
+  val = target_write_memory (addr & 0xfffffffc, buf, 4);
   return val;
 }
 
 static const unsigned char *
 m32r_breakpoint_from_pc (CORE_ADDR *pcptr, int *lenptr)
 {
+  static char be_bp_entry[] = { 0x10, 0xf1, 0x70, 0x00 };	/* dpt -> nop */
+  static char le_bp_entry[] = { 0x00, 0x70, 0xf1, 0x10 };	/* dpt -> nop */
   unsigned char *bp;
 
   /* Determine appropriate breakpoint.  */
@@ -167,30 +149,26 @@
     {
       if ((*pcptr & 3) == 0)
 	{
-	  static unsigned char insn[] = M32R_BE_BREAKPOINT32;
-	  bp = insn;
-	  *lenptr = sizeof (insn);
+	  bp = be_bp_entry;
+	  *lenptr = 4;
 	}
       else
 	{
-	  static unsigned char insn[] = M32R_BE_BREAKPOINT16;
-	  bp = insn;
-	  *lenptr = sizeof (insn);
+	  bp = be_bp_entry;
+	  *lenptr = 2;
 	}
     }
   else
     {
       if ((*pcptr & 3) == 0)
 	{
-	  static unsigned char insn[] = M32R_LE_BREAKPOINT32;
-	  bp = insn;
-	  *lenptr = sizeof (insn);
+	  bp = le_bp_entry;
+	  *lenptr = 4;
 	}
       else
 	{
-	  static unsigned char insn[] = M32R_LE_BREAKPOINT16;
-	  bp = insn;
-	  *lenptr = sizeof (insn);
+	  bp = le_bp_entry + 2;
+	  *lenptr = 2;
 	}
     }
 

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

* Re: [RFA/m32r] Fix breakpoint bug
  2004-10-26  5:45   ` Kei Sakamoto
@ 2004-11-04  0:21     ` Andrew Cagney
  2004-11-04  0:50       ` Kei Sakamoto
  0 siblings, 1 reply; 8+ messages in thread
From: Andrew Cagney @ 2004-11-04  0:21 UTC (permalink / raw)
  To: Kei Sakamoto; +Cc: Daniel Jacobowitz, gdb-patches

Kei Sakamoto wrote:
 >Daniel wrote:
>>I'm curious about the little endian case:
>>
>>+  else /* little-endian */
>>+    {
>>+      if ((addr & 3) == 0)
>>+ {
>>+   buf[0] = contents_cache[0];
>>+   buf[1] = contents_cache[1] & 0x7f;
>>+   buf[2] = bp_entry[1];
>>+   buf[3] = bp_entry[0];
>>  }
>>
>>Shouldn't the breakpoint be placed at buf[0] here rather than buf[2]?
> 
> 
> For most of architectures - yes, it shold be at buf[0]. But the little endian
> mode of M32R is a kind of unique.
> 
> In other architectures, two 16-bit instructions, A and B, are placed as
> the following:
> 
> Big endian:
> A0 A1 B0 B1
> 
> Little endian:
> A1 A0  B1 B0
> 
> In M32R, they are placed like this:
> 
> Big endian:
> A0 A1 B0 B1
> 
> Little endian:
> B1 B0 A1 A0
> 
> This is because M32R always fetches instructions in 32-bit.
> So the breakpoint should be placed at buf[2].

Then this is ok (mainline and 6.3).  I'd add a comment explaining the 
above before committing though.

sorry for the delay,
Andrew


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

* Re: [RFA/m32r] Fix breakpoint bug
  2004-11-04  0:21     ` Andrew Cagney
@ 2004-11-04  0:50       ` Kei Sakamoto
  0 siblings, 0 replies; 8+ messages in thread
From: Kei Sakamoto @ 2004-11-04  0:50 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: Daniel Jacobowitz, gdb-patches

> Then this is ok (mainline and 6.3).  I'd add a comment explaining the 
> above before committing though.
> 
> sorry for the delay,
> Andrew

I added a comment about M32R's unique little endian mode to my patch
and committed it to mainline and 6.3. Thank you!

Kei Sakamoto


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

end of thread, other threads:[~2004-11-04  0:50 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2004-10-07  6:32 [RFA/m32r] Fix breakpoint bug Kei Sakamoto
2004-10-18  6:43 ` [RFA/m32r] Unreviewed patch (breakpoint bug) Kei Sakamoto
2004-10-25  9:02   ` Kei Sakamoto
2004-11-01  1:08     ` Kei Sakamoto
2004-10-25 16:19 ` [RFA/m32r] Fix breakpoint bug Daniel Jacobowitz
2004-10-26  5:45   ` Kei Sakamoto
2004-11-04  0:21     ` Andrew Cagney
2004-11-04  0:50       ` Kei Sakamoto

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox