[rfa/arm] Handle bx and blx

[rfa/arm] Handle bx and blx

[Daniel Jacobowitz]

The software single-step implementation in GDB doesn't know either BX or
BLX.  This results in losing control of the inferior when we single-step
over them.  I based this on the ARM ARM, so I'm pretty sure I've got the
numbers correct.

OK to check in?

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

2004-02-28  Daniel Jacobowitz  <drow@mvista.com>

	* arm-tdep.c (thumb_get_next_pc): Handle BX.
	(arm_get_next_pc): Handle BX and BLX.

Index: gdb/arm-tdep.c
===================================================================
RCS file: /big/fsf/rsync/src-cvs/src/gdb/arm-tdep.c,v
retrieving revision 1.164
diff -u -p -r1.164 arm-tdep.c
--- gdb/arm-tdep.c	16 Feb 2004 21:49:21 -0000	1.164
+++ gdb/arm-tdep.c	28 Feb 2004 03:37:18 -0000
@@ -1657,6 +1658,17 @@ thumb_get_next_pc (CORE_ADDR pc)
       offset = (sbits (inst1, 0, 10) << 12) + (bits (inst2, 0, 10) << 1);
       nextpc = pc_val + offset;
     }
+  else if ((inst1 & 0xff80) == 0x4700)	/* branch and exchange (bx) */
+    {
+      if (bits (inst1, 3, 6) == 0x0f)
+	nextpc = pc_val;
+      else
+	nextpc = read_register (bits (inst1, 3, 6));
+
+      nextpc = ADDR_BITS_REMOVE (nextpc);
+      if (nextpc == pc)
+	error ("Infinite loop detected");
+    }
 
   return nextpc;
 }
@@ -1697,6 +1709,20 @@ arm_get_next_pc (CORE_ADDR pc)
 		&& bits (this_instr, 4, 7) == 9)	/* multiply */
 	      error ("Illegal update to pc in instruction");
 
+	    /* BX <reg>, BLX <reg> */
+	    if (bits (this_instr, 4, 28) == 0x12fff1
+		|| bits (this_instr, 4, 28) == 0x12fff3)
+	      {
+		rn = bits (this_instr, 0, 3);
+		result = (rn == 15) ? pc_val + 8 : read_register (rn);
+		nextpc = (CORE_ADDR) ADDR_BITS_REMOVE (result);
+
+		if (nextpc == pc)
+		  error ("Infinite loop detected");
+
+		return nextpc;
+	      }
+
 	    /* Multiply into PC */
 	    c = (status & FLAG_C) ? 1 : 0;
 	    rn = bits (this_instr, 16, 19);
@@ -1862,6 +1888,10 @@ arm_get_next_pc (CORE_ADDR pc)
 	  {
 	    nextpc = BranchDest (pc, this_instr);
 
+	    /* BLX */
+	    if (bits (this_instr, 28, 31) == INST_NV)
+	      nextpc |= bit (this_instr, 24) << 1;
+
 	    nextpc = ADDR_BITS_REMOVE (nextpc);
 	    if (nextpc == pc)
 	      error ("Infinite loop detected");

Re: [rfa/arm] Handle bx and blx

[Richard Earnshaw]

> The software single-step implementation in GDB doesn't know either BX or
> BLX.  This results in losing control of the inferior when we single-step
> over them.  I based this on the ARM ARM, so I'm pretty sure I've got the
> numbers correct.
> 
> OK to check in?
> 
> -- 
> Daniel Jacobowitz
> MontaVista Software                         Debian GNU/Linux Developer
> 
> 2004-02-28  Daniel Jacobowitz  <drow@mvista.com>
> 
> 	* arm-tdep.c (thumb_get_next_pc): Handle BX.
> 	(arm_get_next_pc): Handle BX and BLX.

Yikes!  Yes, this is OK.  However, Thumb has BLX (2 variants) as well.

R.

Re: [rfa/arm] Handle bx and blx

[Daniel Jacobowitz]

On Wed, Mar 03, 2004 at 04:01:55PM +0000, Richard Earnshaw wrote:
> > The software single-step implementation in GDB doesn't know either BX or
> > BLX.  This results in losing control of the inferior when we single-step
> > over them.  I based this on the ARM ARM, so I'm pretty sure I've got the
> > numbers correct.
> > 
> > OK to check in?
> > 
> > -- 
> > Daniel Jacobowitz
> > MontaVista Software                         Debian GNU/Linux Developer
> > 
> > 2004-02-28  Daniel Jacobowitz  <drow@mvista.com>
> > 
> > 	* arm-tdep.c (thumb_get_next_pc): Handle BX.
> > 	(arm_get_next_pc): Handle BX and BLX.
> 
> Yikes!  Yes, this is OK.  However, Thumb has BLX (2 variants) as well.

Right you are.  I've checked in the above; if I'm reading
thumb_get_next_pc and the ARM correctly, then the below is all I need
for BLX.  The first form is already handled since we don't check H.
The second form can be handled identically to BX by relaxing a test.

OK?

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

2004-03-07  Daniel Jacobowitz  <drow@mvista.com>

	* arm-tdep.c (thumb_get_next_pc): Handle Thumb BLX.

Index: arm-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/arm-tdep.c,v
retrieving revision 1.165
diff -u -p -r1.165 arm-tdep.c
--- arm-tdep.c	7 Mar 2004 20:03:12 -0000	1.165
+++ arm-tdep.c	7 Mar 2004 20:13:34 -0000
@@ -1651,13 +1651,13 @@ thumb_get_next_pc (CORE_ADDR pc)
     {
       nextpc = pc_val + (sbits (inst1, 0, 10) << 1);
     }
-  else if ((inst1 & 0xf800) == 0xf000)	/* long branch with link */
+  else if ((inst1 & 0xf800) == 0xf000)	/* long branch with link, and blx */
     {
       unsigned short inst2 = read_memory_integer (pc + 2, 2);
       offset = (sbits (inst1, 0, 10) << 12) + (bits (inst2, 0, 10) << 1);
       nextpc = pc_val + offset;
     }
-  else if ((inst1 & 0xff80) == 0x4700)	/* branch and exchange (bx) */
+  else if ((inst1 & 0xff00) == 0x4700)	/* bx REG, blx REG */
     {
       if (bits (inst1, 3, 6) == 0x0f)
 	nextpc = pc_val;

Re: [rfa/arm] Handle bx and blx

[Richard Earnshaw]

> On Wed, Mar 03, 2004 at 04:01:55PM +0000, Richard Earnshaw wrote:
> > > The software single-step implementation in GDB doesn't know either BX or
> > > BLX.  This results in losing control of the inferior when we single-step
> > > over them.  I based this on the ARM ARM, so I'm pretty sure I've got the
> > > numbers correct.
> > > 
> > > OK to check in?
> > > 
> > > -- 
> > > Daniel Jacobowitz
> > > MontaVista Software                         Debian GNU/Linux Developer
> > > 
> > > 2004-02-28  Daniel Jacobowitz  <drow@mvista.com>
> > > 
> > > 	* arm-tdep.c (thumb_get_next_pc): Handle BX.
> > > 	(arm_get_next_pc): Handle BX and BLX.
> > 
> > Yikes!  Yes, this is OK.  However, Thumb has BLX (2 variants) as well.
> 
> Right you are.  I've checked in the above; if I'm reading
> thumb_get_next_pc and the ARM correctly, then the below is all I need
> for BLX.  The first form is already handled since we don't check H.
> The second form can be handled identically to BX by relaxing a test.
> 
> OK?
> 
> -- 
> Daniel Jacobowitz
> MontaVista Software                         Debian GNU/Linux Developer
> 
> 2004-03-07  Daniel Jacobowitz  <drow@mvista.com>
> 
> 	* arm-tdep.c (thumb_get_next_pc): Handle Thumb BLX.

Very close, and possibly good enough for most purposes.  But the ARM ARM 
says that in the blx(1) case, the resulting address should be masked with 
0xfffffffc.  That means that there are two theoretical encodings for each 
target ARM-state instruction.  I think you need to add a test for H=01 and 
if so, to apply the mask to nextpc.

R.

Re: [rfa/arm] Handle bx and blx

[Daniel Jacobowitz]

On Mon, Mar 08, 2004 at 10:17:53AM +0000, Richard Earnshaw wrote:
> > On Wed, Mar 03, 2004 at 04:01:55PM +0000, Richard Earnshaw wrote:
> > > > The software single-step implementation in GDB doesn't know either BX or
> > > > BLX.  This results in losing control of the inferior when we single-step
> > > > over them.  I based this on the ARM ARM, so I'm pretty sure I've got the
> > > > numbers correct.
> > > > 
> > > > OK to check in?
> > > > 
> > > > -- 
> > > > Daniel Jacobowitz
> > > > MontaVista Software                         Debian GNU/Linux Developer
> > > > 
> > > > 2004-02-28  Daniel Jacobowitz  <drow@mvista.com>
> > > > 
> > > > 	* arm-tdep.c (thumb_get_next_pc): Handle BX.
> > > > 	(arm_get_next_pc): Handle BX and BLX.
> > > 
> > > Yikes!  Yes, this is OK.  However, Thumb has BLX (2 variants) as well.
> > 
> > Right you are.  I've checked in the above; if I'm reading
> > thumb_get_next_pc and the ARM correctly, then the below is all I need
> > for BLX.  The first form is already handled since we don't check H.
> > The second form can be handled identically to BX by relaxing a test.
> > 
> > OK?
> > 
> > -- 
> > Daniel Jacobowitz
> > MontaVista Software                         Debian GNU/Linux Developer
> > 
> > 2004-03-07  Daniel Jacobowitz  <drow@mvista.com>
> > 
> > 	* arm-tdep.c (thumb_get_next_pc): Handle Thumb BLX.
> 
> Very close, and possibly good enough for most purposes.  But the ARM ARM 
> says that in the blx(1) case, the resulting address should be masked with 
> 0xfffffffc.  That means that there are two theoretical encodings for each 
> target ARM-state instruction.  I think you need to add a test for H=01 and 
> if so, to apply the mask to nextpc.

Except it also says:
           Bit[0] for BLX     If H == 01, then bit[0] of the instruction must
			      be zero, or the instruction is UNDEFINED.
                              The offset calculation method described
                              in Usage above ensures that the offset
                              calculated for a BLX instruction is a
                              multiple of four, and that this
                              restriction is obeyed.

So I think the mask really isn't needed, or am I reading that wrong?

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [rfa/arm] Handle bx and blx

[Richard Earnshaw]

> On Mon, Mar 08, 2004 at 10:17:53AM +0000, Richard Earnshaw wrote:
> > > On Wed, Mar 03, 2004 at 04:01:55PM +0000, Richard Earnshaw wrote:
> > > > > The software single-step implementation in GDB doesn't know either BX or
> > > > > BLX.  This results in losing control of the inferior when we single-step
> > > > > over them.  I based this on the ARM ARM, so I'm pretty sure I've got the
> > > > > numbers correct.
> > > > > 
> > > > > OK to check in?
> > > > > 
> > > > > -- 
> > > > > Daniel Jacobowitz
> > > > > MontaVista Software                         Debian GNU/Linux Developer
> > > > > 
> > > > > 2004-02-28  Daniel Jacobowitz  <drow@mvista.com>
> > > > > 
> > > > > 	* arm-tdep.c (thumb_get_next_pc): Handle BX.
> > > > > 	(arm_get_next_pc): Handle BX and BLX.
> > > > 
> > > > Yikes!  Yes, this is OK.  However, Thumb has BLX (2 variants) as well.
> > > 
> > > Right you are.  I've checked in the above; if I'm reading
> > > thumb_get_next_pc and the ARM correctly, then the below is all I need
> > > for BLX.  The first form is already handled since we don't check H.
> > > The second form can be handled identically to BX by relaxing a test.
> > > 
> > > OK?
> > > 
> > > -- 
> > > Daniel Jacobowitz
> > > MontaVista Software                         Debian GNU/Linux Developer
> > > 
> > > 2004-03-07  Daniel Jacobowitz  <drow@mvista.com>
> > > 
> > > 	* arm-tdep.c (thumb_get_next_pc): Handle Thumb BLX.
> > 
> > Very close, and possibly good enough for most purposes.  But the ARM ARM 
> > says that in the blx(1) case, the resulting address should be masked with 
> > 0xfffffffc.  That means that there are two theoretical encodings for each 
> > target ARM-state instruction.  I think you need to add a test for H=01 and 
> > if so, to apply the mask to nextpc.
> 
> Except it also says:
>            Bit[0] for BLX     If H == 01, then bit[0] of the instruction must
> 			      be zero, or the instruction is UNDEFINED.
>                               The offset calculation method described
>                               in Usage above ensures that the offset
>                               calculated for a BLX instruction is a
>                               multiple of four, and that this
>                               restriction is obeyed.
> 
> So I think the mask really isn't needed, or am I reading that wrong?

Ah, missed that bit.  However, we could be starting with a pc value where 
pc[1] != 0, so we still need the mask.

R.

Re: [rfa/arm] Handle bx and blx

[Daniel Jacobowitz]

On Mon, Mar 08, 2004 at 02:19:29PM +0000, Richard Earnshaw wrote:
> > On Mon, Mar 08, 2004 at 10:17:53AM +0000, Richard Earnshaw wrote:
> > > > On Wed, Mar 03, 2004 at 04:01:55PM +0000, Richard Earnshaw wrote:
> > > > > > The software single-step implementation in GDB doesn't know either BX or
> > > > > > BLX.  This results in losing control of the inferior when we single-step
> > > > > > over them.  I based this on the ARM ARM, so I'm pretty sure I've got the
> > > > > > numbers correct.
> > > > > > 
> > > > > > OK to check in?
> > > > > > 
> > > > > > -- 
> > > > > > Daniel Jacobowitz
> > > > > > MontaVista Software                         Debian GNU/Linux Developer
> > > > > > 
> > > > > > 2004-02-28  Daniel Jacobowitz  <drow@mvista.com>
> > > > > > 
> > > > > > 	* arm-tdep.c (thumb_get_next_pc): Handle BX.
> > > > > > 	(arm_get_next_pc): Handle BX and BLX.
> > > > > 
> > > > > Yikes!  Yes, this is OK.  However, Thumb has BLX (2 variants) as well.
> > > > 
> > > > Right you are.  I've checked in the above; if I'm reading
> > > > thumb_get_next_pc and the ARM correctly, then the below is all I need
> > > > for BLX.  The first form is already handled since we don't check H.
> > > > The second form can be handled identically to BX by relaxing a test.
> > > > 
> > > > OK?
> > > > 
> > > > -- 
> > > > Daniel Jacobowitz
> > > > MontaVista Software                         Debian GNU/Linux Developer
> > > > 
> > > > 2004-03-07  Daniel Jacobowitz  <drow@mvista.com>
> > > > 
> > > > 	* arm-tdep.c (thumb_get_next_pc): Handle Thumb BLX.
> > > 
> > > Very close, and possibly good enough for most purposes.  But the ARM ARM 
> > > says that in the blx(1) case, the resulting address should be masked with 
> > > 0xfffffffc.  That means that there are two theoretical encodings for each 
> > > target ARM-state instruction.  I think you need to add a test for H=01 and 
> > > if so, to apply the mask to nextpc.
> > 
> > Except it also says:
> >            Bit[0] for BLX     If H == 01, then bit[0] of the instruction must
> > 			      be zero, or the instruction is UNDEFINED.
> >                               The offset calculation method described
> >                               in Usage above ensures that the offset
> >                               calculated for a BLX instruction is a
> >                               multiple of four, and that this
> >                               restriction is obeyed.
> > 
> > So I think the mask really isn't needed, or am I reading that wrong?
> 
> Ah, missed that bit.  However, we could be starting with a pc value where 
> pc[1] != 0, so we still need the mask.

Ahh, that's right.  The offset will be a multiple of four but the
PC+offset may not be.  This OK then?

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

2004-03-08  Daniel Jacobowitz  <drow@mvista.com>

	* arm-tdep.c (thumb_get_next_pc): Handle Thumb BLX.

Index: arm-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/arm-tdep.c,v
retrieving revision 1.165
diff -u -p -r1.165 arm-tdep.c
--- arm-tdep.c	7 Mar 2004 20:03:12 -0000	1.165
+++ arm-tdep.c	8 Mar 2004 14:24:49 -0000
@@ -1651,13 +1651,16 @@ thumb_get_next_pc (CORE_ADDR pc)
     {
       nextpc = pc_val + (sbits (inst1, 0, 10) << 1);
     }
-  else if ((inst1 & 0xf800) == 0xf000)	/* long branch with link */
+  else if ((inst1 & 0xf800) == 0xf000)	/* long branch with link, and blx */
     {
       unsigned short inst2 = read_memory_integer (pc + 2, 2);
       offset = (sbits (inst1, 0, 10) << 12) + (bits (inst2, 0, 10) << 1);
       nextpc = pc_val + offset;
+      /* For BLX make sure to clear the low bits.  */
+      if (bits (inst2, 11, 12) == 1)
+	nextpc = nextpc & 0xfffffffc;
     }
-  else if ((inst1 & 0xff80) == 0x4700)	/* branch and exchange (bx) */
+  else if ((inst1 & 0xff00) == 0x4700)	/* bx REG, blx REG */
     {
       if (bits (inst1, 3, 6) == 0x0f)
 	nextpc = pc_val;

Re: [rfa/arm] Handle bx and blx

[Richard Earnshaw]

> Ahh, that's right.  The offset will be a multiple of four but the
> PC+offset may not be.  This OK then?
> 
> -- 
> Daniel Jacobowitz
> MontaVista Software                         Debian GNU/Linux Developer
> 
> 2004-03-08  Daniel Jacobowitz  <drow@mvista.com>
> 
> 	* arm-tdep.c (thumb_get_next_pc): Handle Thumb BLX.

Ok, thanks.

R.

Re: [rfa/arm] Handle bx and blx

[Daniel Jacobowitz]

On Mon, Mar 08, 2004 at 02:28:03PM +0000, Richard Earnshaw wrote:
> 
> > Ahh, that's right.  The offset will be a multiple of four but the
> > PC+offset may not be.  This OK then?
> > 
> > -- 
> > Daniel Jacobowitz
> > MontaVista Software                         Debian GNU/Linux Developer
> > 
> > 2004-03-08  Daniel Jacobowitz  <drow@mvista.com>
> > 
> > 	* arm-tdep.c (thumb_get_next_pc): Handle Thumb BLX.
> 
> Ok, thanks.

Thanks.  I've checked this in.  Upon reflection, I also checked in both
patches to the 6.1 branch.  GDB's failure mode without them is a real
nuisance.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [rfa/arm] Handle bx and blx

[Richard Earnshaw]

> On Wed, Mar 03, 2004 at 04:01:55PM +0000, Richard Earnshaw wrote:
> > > The software single-step implementation in GDB doesn't know either BX or
> > > BLX.  This results in losing control of the inferior when we single-step
> > > over them.  I based this on the ARM ARM, so I'm pretty sure I've got the
> > > numbers correct.
> > > 
> > > OK to check in?
> > > 
> > > -- 
> > > Daniel Jacobowitz
> > > MontaVista Software                         Debian GNU/Linux Developer
> > > 
> > > 2004-02-28  Daniel Jacobowitz  <drow@mvista.com>
> > > 
> > > 	* arm-tdep.c (thumb_get_next_pc): Handle BX.
> > > 	(arm_get_next_pc): Handle BX and BLX.
> > 
> > Yikes!  Yes, this is OK.  However, Thumb has BLX (2 variants) as well.
> 
> Right you are.  I've checked in the above; if I'm reading
> thumb_get_next_pc and the ARM correctly, then the below is all I need
> for BLX.  The first form is already handled since we don't check H.
> The second form can be handled identically to BX by relaxing a test.
> 
> OK?
> 
> -- 
> Daniel Jacobowitz
> MontaVista Software                         Debian GNU/Linux Developer
> 
> 2004-03-07  Daniel Jacobowitz  <drow@mvista.com>
> 
> 	* arm-tdep.c (thumb_get_next_pc): Handle Thumb BLX.

Very close, and possibly good enough for most purposes.  But the ARM ARM 
says that in the blx(1) case, the resulting address should be masked with 
0xfffffffc.  That means that there are two theoretical encodings for each 
target ARM-state instruction.  I think you need to add a test for H=01 and 
if so, to apply the mask to nextpc.

R.

Re: [rfa/arm] Handle bx and blx

[Daniel Jacobowitz]

On Mon, Mar 08, 2004 at 10:17:53AM +0000, Richard Earnshaw wrote:
> > On Wed, Mar 03, 2004 at 04:01:55PM +0000, Richard Earnshaw wrote:
> > > > The software single-step implementation in GDB doesn't know either BX or
> > > > BLX.  This results in losing control of the inferior when we single-step
> > > > over them.  I based this on the ARM ARM, so I'm pretty sure I've got the
> > > > numbers correct.
> > > > 
> > > > OK to check in?
> > > > 
> > > > -- 
> > > > Daniel Jacobowitz
> > > > MontaVista Software                         Debian GNU/Linux Developer
> > > > 
> > > > 2004-02-28  Daniel Jacobowitz  <drow@mvista.com>
> > > > 
> > > > 	* arm-tdep.c (thumb_get_next_pc): Handle BX.
> > > > 	(arm_get_next_pc): Handle BX and BLX.
> > > 
> > > Yikes!  Yes, this is OK.  However, Thumb has BLX (2 variants) as well.
> > 
> > Right you are.  I've checked in the above; if I'm reading
> > thumb_get_next_pc and the ARM correctly, then the below is all I need
> > for BLX.  The first form is already handled since we don't check H.
> > The second form can be handled identically to BX by relaxing a test.
> > 
> > OK?
> > 
> > -- 
> > Daniel Jacobowitz
> > MontaVista Software                         Debian GNU/Linux Developer
> > 
> > 2004-03-07  Daniel Jacobowitz  <drow@mvista.com>
> > 
> > 	* arm-tdep.c (thumb_get_next_pc): Handle Thumb BLX.
> 
> Very close, and possibly good enough for most purposes.  But the ARM ARM 
> says that in the blx(1) case, the resulting address should be masked with 
> 0xfffffffc.  That means that there are two theoretical encodings for each 
> target ARM-state instruction.  I think you need to add a test for H=01 and 
> if so, to apply the mask to nextpc.

Except it also says:
           Bit[0] for BLX     If H == 01, then bit[0] of the instruction must
			      be zero, or the instruction is UNDEFINED.
                              The offset calculation method described
                              in Usage above ensures that the offset
                              calculated for a BLX instruction is a
                              multiple of four, and that this
                              restriction is obeyed.

So I think the mask really isn't needed, or am I reading that wrong?

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [rfa/arm] Handle bx and blx

[Daniel Jacobowitz]

On Mon, Mar 08, 2004 at 02:19:29PM +0000, Richard Earnshaw wrote:
> > On Mon, Mar 08, 2004 at 10:17:53AM +0000, Richard Earnshaw wrote:
> > > > On Wed, Mar 03, 2004 at 04:01:55PM +0000, Richard Earnshaw wrote:
> > > > > > The software single-step implementation in GDB doesn't know either BX or
> > > > > > BLX.  This results in losing control of the inferior when we single-step
> > > > > > over them.  I based this on the ARM ARM, so I'm pretty sure I've got the
> > > > > > numbers correct.
> > > > > > 
> > > > > > OK to check in?
> > > > > > 
> > > > > > -- 
> > > > > > Daniel Jacobowitz
> > > > > > MontaVista Software                         Debian GNU/Linux Developer
> > > > > > 
> > > > > > 2004-02-28  Daniel Jacobowitz  <drow@mvista.com>
> > > > > > 
> > > > > > 	* arm-tdep.c (thumb_get_next_pc): Handle BX.
> > > > > > 	(arm_get_next_pc): Handle BX and BLX.
> > > > > 
> > > > > Yikes!  Yes, this is OK.  However, Thumb has BLX (2 variants) as well.
> > > > 
> > > > Right you are.  I've checked in the above; if I'm reading
> > > > thumb_get_next_pc and the ARM correctly, then the below is all I need
> > > > for BLX.  The first form is already handled since we don't check H.
> > > > The second form can be handled identically to BX by relaxing a test.
> > > > 
> > > > OK?
> > > > 
> > > > -- 
> > > > Daniel Jacobowitz
> > > > MontaVista Software                         Debian GNU/Linux Developer
> > > > 
> > > > 2004-03-07  Daniel Jacobowitz  <drow@mvista.com>
> > > > 
> > > > 	* arm-tdep.c (thumb_get_next_pc): Handle Thumb BLX.
> > > 
> > > Very close, and possibly good enough for most purposes.  But the ARM ARM 
> > > says that in the blx(1) case, the resulting address should be masked with 
> > > 0xfffffffc.  That means that there are two theoretical encodings for each 
> > > target ARM-state instruction.  I think you need to add a test for H=01 and 
> > > if so, to apply the mask to nextpc.
> > 
> > Except it also says:
> >            Bit[0] for BLX     If H == 01, then bit[0] of the instruction must
> > 			      be zero, or the instruction is UNDEFINED.
> >                               The offset calculation method described
> >                               in Usage above ensures that the offset
> >                               calculated for a BLX instruction is a
> >                               multiple of four, and that this
> >                               restriction is obeyed.
> > 
> > So I think the mask really isn't needed, or am I reading that wrong?
> 
> Ah, missed that bit.  However, we could be starting with a pc value where 
> pc[1] != 0, so we still need the mask.

Ahh, that's right.  The offset will be a multiple of four but the
PC+offset may not be.  This OK then?

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

2004-03-08  Daniel Jacobowitz  <drow@mvista.com>

	* arm-tdep.c (thumb_get_next_pc): Handle Thumb BLX.

Index: arm-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/arm-tdep.c,v
retrieving revision 1.165
diff -u -p -r1.165 arm-tdep.c
--- arm-tdep.c	7 Mar 2004 20:03:12 -0000	1.165
+++ arm-tdep.c	8 Mar 2004 14:24:49 -0000
@@ -1651,13 +1651,16 @@ thumb_get_next_pc (CORE_ADDR pc)
     {
       nextpc = pc_val + (sbits (inst1, 0, 10) << 1);
     }
-  else if ((inst1 & 0xf800) == 0xf000)	/* long branch with link */
+  else if ((inst1 & 0xf800) == 0xf000)	/* long branch with link, and blx */
     {
       unsigned short inst2 = read_memory_integer (pc + 2, 2);
       offset = (sbits (inst1, 0, 10) << 12) + (bits (inst2, 0, 10) << 1);
       nextpc = pc_val + offset;
+      /* For BLX make sure to clear the low bits.  */
+      if (bits (inst2, 11, 12) == 1)
+	nextpc = nextpc & 0xfffffffc;
     }
-  else if ((inst1 & 0xff80) == 0x4700)	/* branch and exchange (bx) */
+  else if ((inst1 & 0xff00) == 0x4700)	/* bx REG, blx REG */
     {
       if (bits (inst1, 3, 6) == 0x0f)
 	nextpc = pc_val;

Re: [rfa/arm] Handle bx and blx

[Richard Earnshaw]

> The software single-step implementation in GDB doesn't know either BX or
> BLX.  This results in losing control of the inferior when we single-step
> over them.  I based this on the ARM ARM, so I'm pretty sure I've got the
> numbers correct.
> 
> OK to check in?
> 
> -- 
> Daniel Jacobowitz
> MontaVista Software                         Debian GNU/Linux Developer
> 
> 2004-02-28  Daniel Jacobowitz  <drow@mvista.com>
> 
> 	* arm-tdep.c (thumb_get_next_pc): Handle BX.
> 	(arm_get_next_pc): Handle BX and BLX.

Yikes!  Yes, this is OK.  However, Thumb has BLX (2 variants) as well.

R.

Re: [rfa/arm] Handle bx and blx

[Daniel Jacobowitz]

On Wed, Mar 03, 2004 at 04:01:55PM +0000, Richard Earnshaw wrote:
> > The software single-step implementation in GDB doesn't know either BX or
> > BLX.  This results in losing control of the inferior when we single-step
> > over them.  I based this on the ARM ARM, so I'm pretty sure I've got the
> > numbers correct.
> > 
> > OK to check in?
> > 
> > -- 
> > Daniel Jacobowitz
> > MontaVista Software                         Debian GNU/Linux Developer
> > 
> > 2004-02-28  Daniel Jacobowitz  <drow@mvista.com>
> > 
> > 	* arm-tdep.c (thumb_get_next_pc): Handle BX.
> > 	(arm_get_next_pc): Handle BX and BLX.
> 
> Yikes!  Yes, this is OK.  However, Thumb has BLX (2 variants) as well.

Right you are.  I've checked in the above; if I'm reading
thumb_get_next_pc and the ARM correctly, then the below is all I need
for BLX.  The first form is already handled since we don't check H.
The second form can be handled identically to BX by relaxing a test.

OK?

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

2004-03-07  Daniel Jacobowitz  <drow@mvista.com>

	* arm-tdep.c (thumb_get_next_pc): Handle Thumb BLX.

Index: arm-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/arm-tdep.c,v
retrieving revision 1.165
diff -u -p -r1.165 arm-tdep.c
--- arm-tdep.c	7 Mar 2004 20:03:12 -0000	1.165
+++ arm-tdep.c	7 Mar 2004 20:13:34 -0000
@@ -1651,13 +1651,13 @@ thumb_get_next_pc (CORE_ADDR pc)
     {
       nextpc = pc_val + (sbits (inst1, 0, 10) << 1);
     }
-  else if ((inst1 & 0xf800) == 0xf000)	/* long branch with link */
+  else if ((inst1 & 0xf800) == 0xf000)	/* long branch with link, and blx */
     {
       unsigned short inst2 = read_memory_integer (pc + 2, 2);
       offset = (sbits (inst1, 0, 10) << 12) + (bits (inst2, 0, 10) << 1);
       nextpc = pc_val + offset;
     }
-  else if ((inst1 & 0xff80) == 0x4700)	/* branch and exchange (bx) */
+  else if ((inst1 & 0xff00) == 0x4700)	/* bx REG, blx REG */
     {
       if (bits (inst1, 3, 6) == 0x0f)
 	nextpc = pc_val;

Re: [rfa/arm] Handle bx and blx

[Richard Earnshaw]

> Ahh, that's right.  The offset will be a multiple of four but the
> PC+offset may not be.  This OK then?
> 
> -- 
> Daniel Jacobowitz
> MontaVista Software                         Debian GNU/Linux Developer
> 
> 2004-03-08  Daniel Jacobowitz  <drow@mvista.com>
> 
> 	* arm-tdep.c (thumb_get_next_pc): Handle Thumb BLX.

Ok, thanks.

R.

Re: [rfa/arm] Handle bx and blx

[Richard Earnshaw]

> On Mon, Mar 08, 2004 at 10:17:53AM +0000, Richard Earnshaw wrote:
> > > On Wed, Mar 03, 2004 at 04:01:55PM +0000, Richard Earnshaw wrote:
> > > > > The software single-step implementation in GDB doesn't know either BX or
> > > > > BLX.  This results in losing control of the inferior when we single-step
> > > > > over them.  I based this on the ARM ARM, so I'm pretty sure I've got the
> > > > > numbers correct.
> > > > > 
> > > > > OK to check in?
> > > > > 
> > > > > -- 
> > > > > Daniel Jacobowitz
> > > > > MontaVista Software                         Debian GNU/Linux Developer
> > > > > 
> > > > > 2004-02-28  Daniel Jacobowitz  <drow@mvista.com>
> > > > > 
> > > > > 	* arm-tdep.c (thumb_get_next_pc): Handle BX.
> > > > > 	(arm_get_next_pc): Handle BX and BLX.
> > > > 
> > > > Yikes!  Yes, this is OK.  However, Thumb has BLX (2 variants) as well.
> > > 
> > > Right you are.  I've checked in the above; if I'm reading
> > > thumb_get_next_pc and the ARM correctly, then the below is all I need
> > > for BLX.  The first form is already handled since we don't check H.
> > > The second form can be handled identically to BX by relaxing a test.
> > > 
> > > OK?
> > > 
> > > -- 
> > > Daniel Jacobowitz
> > > MontaVista Software                         Debian GNU/Linux Developer
> > > 
> > > 2004-03-07  Daniel Jacobowitz  <drow@mvista.com>
> > > 
> > > 	* arm-tdep.c (thumb_get_next_pc): Handle Thumb BLX.
> > 
> > Very close, and possibly good enough for most purposes.  But the ARM ARM 
> > says that in the blx(1) case, the resulting address should be masked with 
> > 0xfffffffc.  That means that there are two theoretical encodings for each 
> > target ARM-state instruction.  I think you need to add a test for H=01 and 
> > if so, to apply the mask to nextpc.
> 
> Except it also says:
>            Bit[0] for BLX     If H == 01, then bit[0] of the instruction must
> 			      be zero, or the instruction is UNDEFINED.
>                               The offset calculation method described
>                               in Usage above ensures that the offset
>                               calculated for a BLX instruction is a
>                               multiple of four, and that this
>                               restriction is obeyed.
> 
> So I think the mask really isn't needed, or am I reading that wrong?

Ah, missed that bit.  However, we could be starting with a pc value where 
pc[1] != 0, so we still need the mask.

R.

Re: [rfa/arm] Handle bx and blx

[Daniel Jacobowitz]

On Mon, Mar 08, 2004 at 02:28:03PM +0000, Richard Earnshaw wrote:
> 
> > Ahh, that's right.  The offset will be a multiple of four but the
> > PC+offset may not be.  This OK then?
> > 
> > -- 
> > Daniel Jacobowitz
> > MontaVista Software                         Debian GNU/Linux Developer
> > 
> > 2004-03-08  Daniel Jacobowitz  <drow@mvista.com>
> > 
> > 	* arm-tdep.c (thumb_get_next_pc): Handle Thumb BLX.
> 
> Ok, thanks.

Thanks.  I've checked this in.  Upon reflection, I also checked in both
patches to the 6.1 branch.  GDB's failure mode without them is a real
nuisance.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer