RFA/ARM: Switch mode when setting PC

RFA/ARM: Switch mode when setting PC

[Daniel Jacobowitz]

This patch fixes several failures in virtfunc.exp for arm-sim/-mthumb.  The
problem is that the non-virtual thunk for pDe->vg() is emitted in ARM mode
and called via _call_via_r2.  But the rest of the program is Thumb mode, and
nothing tells the simulator (or target; I haven't tested this on hardware
yet but I expect the same result) to switch to ARM.  So it gets very
confused.

This is somewhat suboptimal in that if you want, for some reason, to call
something marked as an ARM function with Thumb mode enabled you will have to
set $ps yourself _after_ setting $pc.  I think it's still a good idea,
though.  OK?

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

2004-01-15  Daniel Jacobowitz  <drow@mvista.com>

	* arm-tdep.c (arm_write_pc): New function.
	(arm_gdbarch_init): Call set_gdbarch_write_pc.

Index: arm-tdep.c
===================================================================
RCS file: /big/fsf/rsync/src-cvs/src/gdb/arm-tdep.c,v
retrieving revision 1.156
diff -u -p -r1.156 arm-tdep.c
--- arm-tdep.c	13 Jan 2004 21:38:45 -0000	1.156
+++ arm-tdep.c	16 Jan 2004 03:37:46 -0000
@@ -2689,6 +2689,21 @@ arm_coff_make_msymbol_special(int val, s
     MSYMBOL_SET_SPECIAL (msym);
 }
 
+static void
+arm_write_pc (CORE_ADDR pc, ptid_t ptid)
+{
+  write_register_pid (ARM_PC_REGNUM, pc, ptid);
+
+  /* If necessary, set the T bit.  */
+  if (arm_apcs_32)
+    {
+      CORE_ADDR val = read_register_pid (ARM_PS_REGNUM, ptid);
+      if (arm_pc_is_thumb (pc))
+	write_register_pid (ARM_PS_REGNUM, val | 0x20, ptid);
+      else
+	write_register_pid (ARM_PS_REGNUM, val & ~(CORE_ADDR) 0x20, ptid);
+    }
+}
 \f
 static enum gdb_osabi
 arm_elf_osabi_sniffer (bfd *abfd)
@@ -2850,6 +2865,8 @@ arm_gdbarch_init (struct gdbarch_info in
   set_gdbarch_deprecated_sizeof_call_dummy_words (gdbarch, 0);
 
   set_gdbarch_push_dummy_call (gdbarch, arm_push_dummy_call);
+
+  set_gdbarch_write_pc (gdbarch, arm_write_pc);
 
   /* Frame handling.  */
   set_gdbarch_unwind_dummy_id (gdbarch, arm_unwind_dummy_id);

Re: RFA/ARM: Switch mode when setting PC

[Andrew Cagney]

> This patch fixes several failures in virtfunc.exp for arm-sim/-mthumb.  The
> problem is that the non-virtual thunk for pDe->vg() is emitted in ARM mode
> and called via _call_via_r2.  But the rest of the program is Thumb mode, and
> nothing tells the simulator (or target; I haven't tested this on hardware
> yet but I expect the same result) to switch to ARM.  So it gets very
> confused.
> 
> This is somewhat suboptimal in that if you want, for some reason, to call
> something marked as an ARM function with Thumb mode enabled you will have to
> set $ps yourself _after_ setting $pc.  I think it's still a good idea,
> though.  OK?

(sorry, but I don't understand that paragraph - doesn't that code set 
the $ps after setting the $pc?)

The ari contains this yellow card:

write pc
Replace write_pc() with get_frame_base_address or get_frame_id; at 
present the inferior function call code still uses this when doing a 
DECR_PC_AFTER_BREAK

the concern is with the way write_pc is being called to perform two 
different operations:

- decrement the pc just after the target stops
Arrrg!
- jump to a specific address
As with an inferior function call or jump.

I think it would be better to have two methods so that it's clear that 
this case only applies when doing a jump.

Andrew

Re: RFA/ARM: Switch mode when setting PC

[Daniel Jacobowitz]

On Fri, Jan 16, 2004 at 12:43:46AM -0500, Andrew Cagney wrote:
> >This patch fixes several failures in virtfunc.exp for arm-sim/-mthumb.  The
> >problem is that the non-virtual thunk for pDe->vg() is emitted in ARM mode
> >and called via _call_via_r2.  But the rest of the program is Thumb mode, 
> >and
> >nothing tells the simulator (or target; I haven't tested this on hardware
> >yet but I expect the same result) to switch to ARM.  So it gets very
> >confused.
> >
> >This is somewhat suboptimal in that if you want, for some reason, to call
> >something marked as an ARM function with Thumb mode enabled you will have 
> >to
> >set $ps yourself _after_ setting $pc.  I think it's still a good idea,
> >though.  OK?
> 
> (sorry, but I don't understand that paragraph - doesn't that code set 
> the $ps after setting the $pc?)

Right.  But if you want to start a Thumb function in ARM mode you'll
have to do:
  (gdb) set $pc = start_of_function
  (gdb) set $ps = $ps & ~0x20

> The ari contains this yellow card:
> 
> write pc
> Replace write_pc() with get_frame_base_address or get_frame_id; at 
> present the inferior function call code still uses this when doing a 
> DECR_PC_AFTER_BREAK
> 
> the concern is with the way write_pc is being called to perform two 
> different operations:
> 
> - decrement the pc just after the target stops
> Arrrg!
> - jump to a specific address
> As with an inferior function call or jump.
> 
> I think it would be better to have two methods so that it's clear that 
> this case only applies when doing a jump.

Well, it'd be better to rip out the current DECR_PC_AFTER_BREAK and
handle it completely within the four (roughly) affected targets,
thereby removing the overloading of write_pc.  IMVHO.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: RFA/ARM: Switch mode when setting PC

[Richard Earnshaw]

> On Fri, Jan 16, 2004 at 12:43:46AM -0500, Andrew Cagney wrote:
> > >This patch fixes several failures in virtfunc.exp for arm-sim/-mthumb.  The
> > >problem is that the non-virtual thunk for pDe->vg() is emitted in ARM mode
> > >and called via _call_via_r2.  But the rest of the program is Thumb mode, 
> > >and
> > >nothing tells the simulator (or target; I haven't tested this on hardware
> > >yet but I expect the same result) to switch to ARM.  So it gets very
> > >confused.
> > >
> > >This is somewhat suboptimal in that if you want, for some reason, to call
> > >something marked as an ARM function with Thumb mode enabled you will have 
> > >to
> > >set $ps yourself _after_ setting $pc.  I think it's still a good idea,
> > >though.  OK?
> > 
> > (sorry, but I don't understand that paragraph - doesn't that code set 
> > the $ps after setting the $pc?)
> 
> Right.  But if you want to start a Thumb function in ARM mode you'll
> have to do:
>   (gdb) set $pc = start_of_function
>   (gdb) set $ps = $ps & ~0x20
> 
> > The ari contains this yellow card:
> > 
> > write pc
> > Replace write_pc() with get_frame_base_address or get_frame_id; at 
> > present the inferior function call code still uses this when doing a 
> > DECR_PC_AFTER_BREAK
> > 
> > the concern is with the way write_pc is being called to perform two 
> > different operations:
> > 
> > - decrement the pc just after the target stops
> > Arrrg!
> > - jump to a specific address
> > As with an inferior function call or jump.
> > 
> > I think it would be better to have two methods so that it's clear that 
> > this case only applies when doing a jump.
> 
> Well, it'd be better to rip out the current DECR_PC_AFTER_BREAK and
> handle it completely within the four (roughly) affected targets,
> thereby removing the overloading of write_pc.  IMVHO.
> 

Unless the "Thumb bit" is being stripped out by GDB, then I suspect that 
this is a bug in the gdb/simulator binding layer.  Any attempt to force 
the PC value by the debugger should be taken as a potential state change.  
If that is not happening, then all sorts of things may not work.

I've suspected that there is a problem in the way that gdb drives the 
simulator for a while now.

R.

Re: RFA/ARM: Switch mode when setting PC

[Daniel Jacobowitz]

On Fri, Jan 16, 2004 at 02:14:56PM +0000, Richard Earnshaw wrote:
> Unless the "Thumb bit" is being stripped out by GDB, then I suspect that 
> this is a bug in the gdb/simulator binding layer.  Any attempt to force 
> the PC value by the debugger should be taken as a potential state change.  
> If that is not happening, then all sorts of things may not work.
> 
> I've suspected that there is a problem in the way that gdb drives the 
> simulator for a while now.

My understanding of the ARM architecture is of somewhat recent vintage,
so the following may be a load of crap.  For unrelated reasons I can't
test this in hardware yet.

The bx instruction sets the PC register to reg & 0xfffffffe.  It uses
reg & 0x1 to set the T bit.  So the value that gets written into the PC
register has its low bit clear, and the CPSR gets updated.  The low bit
of the actual PC register is ignored.  Isn't that correct?

If so, I think the interface is fine.  Certainly it corresponds to how
ptrace behaves on Linux; the value specified for the PC is written
directly to the PC, not parsed for the T bit.  If you want to change
the T bit, write to the CPSR.

Right now the address of a Thumb function is marked in the symbol table
by the msymbol "special" flag, not in the low bit of the address.  The
address points at the actual beginning of the instruction, so that's
what GDB writes into $pc.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: RFA/ARM: Switch mode when setting PC

[Richard Earnshaw]

> 
> Unless the "Thumb bit" is being stripped out by GDB, then I suspect that 
> this is a bug in the gdb/simulator binding layer.  Any attempt to force 
> the PC value by the debugger should be taken as a potential state change.  
> If that is not happening, then all sorts of things may not work.
> 
> I've suspected that there is a problem in the way that gdb drives the 
> simulator for a while now.
> 

sim_store_register in sim/arm/wrapper.c is currently usring ARMul_SetReg 
to set the PC.  I think this is wrong.

RDI_CPUwrite in sim/arm/armrdi.c uses ARMul_SetPC in a similar context.

ARMul_SetReg should only be used on the PC in specific circumstances, 
specifically from within the main simulation loop.  Even then it should 
probably be using ARMul_SetR15.

I suspect that this is why several rather gross hacks were introduced over 
the years to make single stepping work, and what you are seeing now may be 
another artifact of this general problem.

Unfortunately, it may be quite difficult to unwind some of these hacks now 
(I'm not sure all the changes are in the public repository -- someone with 
access to the old Cygnus CVS tree may be able to check, but I certainly 
can't), so the knowledge that there is a problem may not help much in the 
search for a solution :-(

R.

Re: RFA/ARM: Switch mode when setting PC

[Daniel Jacobowitz]

On Fri, Jan 16, 2004 at 02:34:23PM +0000, Richard Earnshaw wrote:
> 
> > 
> > Unless the "Thumb bit" is being stripped out by GDB, then I suspect that 
> > this is a bug in the gdb/simulator binding layer.  Any attempt to force 
> > the PC value by the debugger should be taken as a potential state change.  
> > If that is not happening, then all sorts of things may not work.
> > 
> > I've suspected that there is a problem in the way that gdb drives the 
> > simulator for a while now.
> > 
> 
> sim_store_register in sim/arm/wrapper.c is currently usring ARMul_SetReg 
> to set the PC.  I think this is wrong.
> 
> RDI_CPUwrite in sim/arm/armrdi.c uses ARMul_SetPC in a similar context.
> 
> ARMul_SetReg should only be used on the PC in specific circumstances, 
> specifically from within the main simulation loop.  Even then it should 
> probably be using ARMul_SetR15.
> 
> I suspect that this is why several rather gross hacks were introduced over 
> the years to make single stepping work, and what you are seeing now may be 
> another artifact of this general problem.

OK, but I don't think that's relevant.  Take a look at what those
functions do:

void
ARMul_SetR15 (ARMul_State * state, ARMword value)
{
  if (ARMul_MODE32BIT)
    state->Reg[15] = value & PCBITS;
  else
    {
      state->Reg[15] = value;
      ARMul_R15Altered (state);
    }
  FLUSHPIPE;
}

void
ARMul_SetPC (ARMul_State * state, ARMword value)
{ 
  if (ARMul_MODE32BIT)
    state->Reg[15] = value & PCBITS;
  else
    state->Reg[15] = R15CCINTMODE | (value & R15PCBITS);
  FLUSHPIPE;
}

As I said in my other message, none of these have the potential to
switch the simulator from Thumb to ARM mode.  I spent some time looking
yesterday and I believe that none of the functions used to set the PC
do, with the exception of WriteR15Branch; and it's pretty obvious to me
that the sim interface should not be using WriteR15Branch, so the
simulator's client needs to handle setting the CPSR.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: RFA/ARM: Switch mode when setting PC

[Richard Earnshaw]

> On Fri, Jan 16, 2004 at 02:34:23PM +0000, Richard Earnshaw wrote:
> > 
> > > 
> > > Unless the "Thumb bit" is being stripped out by GDB, then I suspect that 
> > > this is a bug in the gdb/simulator binding layer.  Any attempt to force 
> > > the PC value by the debugger should be taken as a potential state change.  
> > > If that is not happening, then all sorts of things may not work.
> > > 
> > > I've suspected that there is a problem in the way that gdb drives the 
> > > simulator for a while now.
> > > 
> > 
> > sim_store_register in sim/arm/wrapper.c is currently usring ARMul_SetReg 
> > to set the PC.  I think this is wrong.
> > 
> > RDI_CPUwrite in sim/arm/armrdi.c uses ARMul_SetPC in a similar context.
> > 
> > ARMul_SetReg should only be used on the PC in specific circumstances, 
> > specifically from within the main simulation loop.  Even then it should 
> > probably be using ARMul_SetR15.
> > 
> > I suspect that this is why several rather gross hacks were introduced over 
> > the years to make single stepping work, and what you are seeing now may be 
> > another artifact of this general problem.
> 
> OK, but I don't think that's relevant.  Take a look at what those
> functions do:
> 
> void
> ARMul_SetR15 (ARMul_State * state, ARMword value)
> {
>   if (ARMul_MODE32BIT)
>     state->Reg[15] = value & PCBITS;
>   else
>     {
>       state->Reg[15] = value;
>       ARMul_R15Altered (state);
>     }
>   FLUSHPIPE;
> }
> 
> void
> ARMul_SetPC (ARMul_State * state, ARMword value)
> { 
>   if (ARMul_MODE32BIT)
>     state->Reg[15] = value & PCBITS;
>   else
>     state->Reg[15] = R15CCINTMODE | (value & R15PCBITS);
>   FLUSHPIPE;
> }
> 
> As I said in my other message, none of these have the potential to
> switch the simulator from Thumb to ARM mode.  I spent some time looking
> yesterday and I believe that none of the functions used to set the PC
> do, with the exception of WriteR15Branch; and it's pretty obvious to me
> that the sim interface should not be using WriteR15Branch, so the
> simulator's client needs to handle setting the CPSR.
> 

Sure, but I think that is just more lossage that has been introduced after 
the ARMulator was released by ARM.  Remember that the code that was 
released didn't have any Thumb support at all, that has all been added at 
a later date.  So the fact that ARMul_SetPC doesn't correctly update the 
Thumb bit is also a bug.  There's no reason why it shouldn't (and, AFAICT, 
every reason why it should).  Then it would be possible to execute an 
image where even the first instruction was in Thumb state.

So, I still think that wrapper.c should be using ARMul_SetPC to update 
R15, which should then be correctly managing the Thumb bit in the CPSR.  
Note that ARMul_SetPC is only called from wrapper.c and armrdi.c, the two 
interfaces to the simulator.  So there's no chance that fixing this will 
break normal free-running simulation.

However, there are other changes (hacks) in the main loop that were 
introduced to overcome the fact that ARMul_SetPC wasn't being used, these 
may have to be tracked down and fixed.

R.

Re: RFA/ARM: Switch mode when setting PC

[Daniel Jacobowitz]

On Fri, Jan 16, 2004 at 02:59:58PM +0000, Richard Earnshaw wrote:
> Sure, but I think that is just more lossage that has been introduced after 
> the ARMulator was released by ARM.  Remember that the code that was 
> released didn't have any Thumb support at all, that has all been added at 
> a later date.  So the fact that ARMul_SetPC doesn't correctly update the 

OK, I didn't know that.

> Thumb bit is also a bug.  There's no reason why it shouldn't (and, AFAICT, 
> every reason why it should).  Then it would be possible to execute an 
> image where even the first instruction was in Thumb state.

It's already possible; you have to set the CPSR by hand, though.  Am I
missing something?

> So, I still think that wrapper.c should be using ARMul_SetPC to update 
> R15, which should then be correctly managing the Thumb bit in the CPSR.  
> Note that ARMul_SetPC is only called from wrapper.c and armrdi.c, the two 
> interfaces to the simulator.  So there's no chance that fixing this will 
> break normal free-running simulation.
> 
> However, there are other changes (hacks) in the main loop that were 
> introduced to overcome the fact that ARMul_SetPC wasn't being used, these 
> may have to be tracked down and fixed.

I guess I just see this differently.  The existing Linux ptrace
interface also predates Thumb, so it's not surprising that it just
writes what you give it into the PC register.  But I can't see any
reason why I should change that.  The remote protocol is a very
low-level protocol; the CPSR and PC are separate writeable registers,
and I would find it extremely surprising if the sequence:
  read CPSR
  read PC
  write PC
  read CPSR

could return two different CPSR values.

Here's what I would find even more surprising.  The sequence:
  read PC
  write same value to PC

would suddenly switch me out of Thumb mode, since the bit is cleared in
the PC!  This would break _all_ uses of the interface (either the sim
interface or the ptrace interface) in Thumb mode.  Right now there are
only problems if you are deliberately trying to mode switch.

In short, I think writing the PC should not change the CPSR, and if the
client wants to change the mode they should do it explicitly.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: RFA/ARM: Switch mode when setting PC

[Richard Earnshaw]

> I guess I just see this differently.  The existing Linux ptrace
> interface also predates Thumb, so it's not surprising that it just
> writes what you give it into the PC register.  But I can't see any
> reason why I should change that.  The remote protocol is a very
> low-level protocol; the CPSR and PC are separate writeable registers,
> and I would find it extremely surprising if the sequence:
>   read CPSR
>   read PC
>   write PC
>   read CPSR
> 
> could return two different CPSR values.
> 
> Here's what I would find even more surprising.  The sequence:
>   read PC
>   write same value to PC
> 
> would suddenly switch me out of Thumb mode, since the bit is cleared in
> the PC!  This would break _all_ uses of the interface (either the sim
> interface or the ptrace interface) in Thumb mode.  Right now there are
> only problems if you are deliberately trying to mode switch.
> 
> In short, I think writing the PC should not change the CPSR, and if the
> client wants to change the mode they should do it explicitly.

You raise an interesting point.  I'd been thinking about it purely from 
the simulator's perspective, rather than in the wider context.

I've just had a conversation with some engineers in our debugger group to 
see if there was a specific opinion.

The consensus seems to be that you are right, the debugger must correctly 
set the 'CPSR' if it wants the inferior to switch states.

All this means that there are effectively 33 bits that have to be updated 
when the PC is written and a state change is needed.  This is complicated 
further by the fact that sometimes one of those bits needs to be 
manufactured, and at other times it doesn't.  (in fact, there can be 34 
bits if you include the CPSR 'J' bit, but let's not even think about going 
there).

For example, if the user writes a 32-bit value into the PC, the CPSR state 
probably shouldn't be changed (even if the bottom bit is altered) -- this 
is how ARM's debuggers behave.  However, if the user 'calls' a function 
that is in the 'other state', then the CPSR should be updated (and 
presumably restored afterwards).

I'm not sure if GDB has a way of separating these two cases.  It's an 
interesting problem.

As a final comment, when it comes to talking directly to real hardware 
(eg, via an ICE box), it isn't generally possible to update the CPSR by 
just writing to it (at least, not for the 'T' and 'J' bits); the only way 
of switching to Thumb state is via a BX instruction or with some other 
PC-modifying instruction that is documented to cause a state change (on 
ARMv4T that normally means 'movs PC, ...' or 'ldm ..., PC}^'; on v5 some 
loads to the PC can also be used).

R.

Re: RFA/ARM: Switch mode when setting PC

[Daniel Jacobowitz]

On Fri, Jan 16, 2004 at 04:54:30PM +0000, Richard Earnshaw wrote:
> 
> > I guess I just see this differently.  The existing Linux ptrace
> > interface also predates Thumb, so it's not surprising that it just
> > writes what you give it into the PC register.  But I can't see any
> > reason why I should change that.  The remote protocol is a very
> > low-level protocol; the CPSR and PC are separate writeable registers,
> > and I would find it extremely surprising if the sequence:
> >   read CPSR
> >   read PC
> >   write PC
> >   read CPSR
> > 
> > could return two different CPSR values.
> > 
> > Here's what I would find even more surprising.  The sequence:
> >   read PC
> >   write same value to PC
> > 
> > would suddenly switch me out of Thumb mode, since the bit is cleared in
> > the PC!  This would break _all_ uses of the interface (either the sim
> > interface or the ptrace interface) in Thumb mode.  Right now there are
> > only problems if you are deliberately trying to mode switch.
> > 
> > In short, I think writing the PC should not change the CPSR, and if the
> > client wants to change the mode they should do it explicitly.
> 
> You raise an interesting point.  I'd been thinking about it purely from 
> the simulator's perspective, rather than in the wider context.
> 
> I've just had a conversation with some engineers in our debugger group to 
> see if there was a specific opinion.
> 
> The consensus seems to be that you are right, the debugger must correctly 
> set the 'CPSR' if it wants the inferior to switch states.

Patch OK then?

> All this means that there are effectively 33 bits that have to be updated 
> when the PC is written and a state change is needed.  This is complicated 
> further by the fact that sometimes one of those bits needs to be 
> manufactured, and at other times it doesn't.  (in fact, there can be 34 
> bits if you include the CPSR 'J' bit, but let's not even think about going 
> there).

Whew! :)

> For example, if the user writes a 32-bit value into the PC, the CPSR state 
> probably shouldn't be changed (even if the bottom bit is altered) -- this 
> is how ARM's debuggers behave.  However, if the user 'calls' a function 
> that is in the 'other state', then the CPSR should be updated (and 
> presumably restored afterwards).
> 
> I'm not sure if GDB has a way of separating these two cases.  It's an 
> interesting problem.

I believe that this will work at present, because setting $pc will not
go through write_pc.  There's some blind luck involved in this, though.

> As a final comment, when it comes to talking directly to real hardware 
> (eg, via an ICE box), it isn't generally possible to update the CPSR by 
> just writing to it (at least, not for the 'T' and 'J' bits); the only way 
> of switching to Thumb state is via a BX instruction or with some other 
> PC-modifying instruction that is documented to cause a state change (on 
> ARMv4T that normally means 'movs PC, ...' or 'ldm ..., PC}^'; on v5 some 
> loads to the PC can also be used).

Really?  Interesting... I don't think GDB handles this at all at the
moment.  For both Linux userland GDB and Linux remote kernel GDB, this
is a non-issue; you can write the CPSR directly and it will be restored
at return from exception (via the SPSR and an ldm instruction).  This
works because the kgdb stub is implemented as an exception handler.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: RFA/ARM: Switch mode when setting PC

[Richard Earnshaw]

> > The consensus seems to be that you are right, the debugger must correctly 
> > set the 'CPSR' if it wants the inferior to switch states.
> 
> Patch OK then?
> 

I'm happy with the ARM part, if you can convince Andrew that the MI part 
is ok.

> > For example, if the user writes a 32-bit value into the PC, the CPSR state 
> > probably shouldn't be changed (even if the bottom bit is altered) -- this 
> > is how ARM's debuggers behave.  However, if the user 'calls' a function 
> > that is in the 'other state', then the CPSR should be updated (and 
> > presumably restored afterwards).
> > 
> > I'm not sure if GDB has a way of separating these two cases.  It's an 
> > interesting problem.
> 
> I believe that this will work at present, because setting $pc will not
> go through write_pc.  There's some blind luck involved in this, though.

In the past we've tried to distinguish R15 from PC.  This was especially 
useful in the legacy 26-bit mode where the CPSR bits *were* in R15.

This would probably all have been much simpler if I'd been able to 
complete my code for handling the banked register; sadly I never got far 
enough, and I think the code is probably too bit-rotten to be worth trying 
to resurrect directly at this point.

> 
> > As a final comment, when it comes to talking directly to real hardware 
> > (eg, via an ICE box), it isn't generally possible to update the CPSR by 
> > just writing to it (at least, not for the 'T' and 'J' bits); the only way 
> > of switching to Thumb state is via a BX instruction or with some other 
> > PC-modifying instruction that is documented to cause a state change (on 
> > ARMv4T that normally means 'movs PC, ...' or 'ldm ..., PC}^'; on v5 some 
> > loads to the PC can also be used).
> 
> Really?  Interesting... I don't think GDB handles this at all at the
> moment.  For both Linux userland GDB and Linux remote kernel GDB, this
> is a non-issue; you can write the CPSR directly and it will be restored
> at return from exception (via the SPSR and an ldm instruction).  This
> works because the kgdb stub is implemented as an exception handler.

If you are talking directly to a core through a hardware channel such as 
an ICE, there's all sorts of restrictions and limitations.  It's usually 
the job of a further layer to map the high(ish)-level directives from the 
debugger onto commands that can be done on the target (in many instances 
you have to insert instructions directly into the core pipeline -- well 
the fetch unit -- and then clock them through).  It's not an area I know 
much about beyond the very basics.

R.

Re: RFA/ARM: Switch mode when setting PC

[Andrew Cagney]

> For example, if the user writes a 32-bit value into the PC, the CPSR state 
>> > probably shouldn't be changed (even if the bottom bit is altered) -- this 
>> > is how ARM's debuggers behave.  However, if the user 'calls' a function 
>> > that is in the 'other state', then the CPSR should be updated (and 
>> > presumably restored afterwards).
>> > 
>> > I'm not sure if GDB has a way of separating these two cases.  It's an 
>> > interesting problem.
> 
>> 
>> I believe that this will work at present, because setting $pc will not
>> go through write_pc.  There's some blind luck involved in this, though.

Or a lack of design, Arm needs to ensure that it doesn't define PC_REGNUM.

> In the past we've tried to distinguish R15 from PC.  This was especially 
> useful in the legacy 26-bit mode where the CPSR bits *were* in R15.
> 
> This would probably all have been much simpler if I'd been able to 
> complete my code for handling the banked register; sadly I never got far 
> enough, and I think the code is probably too bit-rotten to be worth trying 
> to resurrect directly at this point.

If there's an explicit "set_resume_address", separate to write_pc, this 
should happen:

	(gdb) set $r15 = 0x123
	- target sees:
		$r15=0x123
	(gdb) call foo()   OR (gdb) jump foo
	- target, via "set_resume_address", sees:
		$r15=&foo
		$ps&|=<magic-bits>

and significantly no other write_pc calls.

Andrew

Re: RFA/ARM: Switch mode when setting PC

[Daniel Jacobowitz]

On Fri, Jan 16, 2004 at 09:10:40AM -0500, Daniel Jacobowitz wrote:
> > The ari contains this yellow card:
> > 
> > write pc
> > Replace write_pc() with get_frame_base_address or get_frame_id; at 
> > present the inferior function call code still uses this when doing a 
> > DECR_PC_AFTER_BREAK
> > 
> > the concern is with the way write_pc is being called to perform two 
> > different operations:
> > 
> > - decrement the pc just after the target stops
> > Arrrg!
> > - jump to a specific address
> > As with an inferior function call or jump.
> > 
> > I think it would be better to have two methods so that it's clear that 
> > this case only applies when doing a jump.
> 
> Well, it'd be better to rip out the current DECR_PC_AFTER_BREAK and
> handle it completely within the four (roughly) affected targets,
> thereby removing the overloading of write_pc.  IMVHO.

Hi Andrew,

Is this patch OK (write_pc isn't deprecated yet!)?  Cleaning up the
existing DECR_PC_AFTER_BREAK handling is going to be a touchy job, and
I don't really want to try it today :)  I'll try to look into it later,
though.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: RFA/ARM: Switch mode when setting PC

[Andrew Cagney]

> On Fri, Jan 16, 2004 at 09:10:40AM -0500, Daniel Jacobowitz wrote:
> 
>> > The ari contains this yellow card:
>> > 
>> > write pc
>> > Replace write_pc() with get_frame_base_address or get_frame_id; at 
>> > present the inferior function call code still uses this when doing a 
>> > DECR_PC_AFTER_BREAK
>> > 
>> > the concern is with the way write_pc is being called to perform two 
>> > different operations:
>> > 
>> > - decrement the pc just after the target stops
>> > Arrrg!
>> > - jump to a specific address
>> > As with an inferior function call or jump.
>> > 
>> > I think it would be better to have two methods so that it's clear that 
>> > this case only applies when doing a jump.
> 
>> 
>> Well, it'd be better to rip out the current DECR_PC_AFTER_BREAK and
>> handle it completely within the four (roughly) affected targets,
>> thereby removing the overloading of write_pc.  IMVHO.

It's been on everyones wish list for too long.  Last time I tried I got 
bogged down due to a lack of ia32 doco - that's now been fixed though.

> Hi Andrew,
> 
> Is this patch OK (write_pc isn't deprecated yet!)?  Cleaning up the
> existing DECR_PC_AFTER_BREAK handling is going to be a touchy job, and
> I don't really want to try it today :)  I'll try to look into it later,
> though.

I was suggesting "two methods so that it's clear that this case only 
applies when doing a jump".  This won't involve anything like 
deprecating /removing decr_pc_after_break _+ write_pc but will involve 
the addition of a new method like:
	set_resume_address (arch, targ or tpid or regs)
that could somehow default to a legacy call to write_pc.  Significantly, 
this will avoid making the changes conditional on the elimination of 
decr-pc (your concern).

Why is this better?  It clearly separates the [apparently] legetimate 
resume case from the decr-pc case.  This in turn opens the way for the 
deprecate / delete decr-pc "write_pc" code while at the same time 
ensuring that the work can't break the arm.

enjoy,
Andrew

Re: RFA/ARM: Switch mode when setting PC

[Daniel Jacobowitz]

On Fri, Jan 16, 2004 at 02:12:18PM -0500, Andrew Cagney wrote:
> Or a lack of design, Arm needs to ensure that it doesn't define PC_REGNUM.

OK, I'll keep that in mind.

> >In the past we've tried to distinguish R15 from PC.  This was especially 
> >useful in the legacy 26-bit mode where the CPSR bits *were* in R15.

Perhaps, in that case, we should have the PC as a pseudoregister.  That
would simplify a lot of arm-tdep.c.  Is it possible to use the existing
ARM compiler and simulator (and newlib) in 26-bit mode without a lot of
hassle, or would it be the work of months to make it happen?  I'm
reluctant to dig my fingers in too deeply if I can't test both code
paths.

> >This would probably all have been much simpler if I'd been able to 
> >complete my code for handling the banked register; sadly I never got far 
> >enough, and I think the code is probably too bit-rotten to be worth trying 
> >to resurrect directly at this point.
> 
> If there's an explicit "set_resume_address", separate to write_pc, this 
> should happen:
> 
> 	(gdb) set $r15 = 0x123
> 	- target sees:
> 		$r15=0x123
> 	(gdb) call foo()   OR (gdb) jump foo
> 	- target, via "set_resume_address", sees:
> 		$r15=&foo
> 		$ps&|=<magic-bits>
> 
> and significantly no other write_pc calls.

And at this point, is write_pc used for anything besides
DECR_PC_AFTER_BREAK?  I would prefer to add an adjust_pc_after_break,
and then possibly rename the existing write_pc.  Most of the write_pc
implementations we have currently are really set-resume-address
semantics.

On Fri, Jan 16, 2004 at 01:57:40PM -0500, Andrew Cagney wrote:
> >On Fri, Jan 16, 2004 at 09:10:40AM -0500, Daniel Jacobowitz wrote:

> >Is this patch OK (write_pc isn't deprecated yet!)?  Cleaning up the
> >existing DECR_PC_AFTER_BREAK handling is going to be a touchy job, and
> >I don't really want to try it today :)  I'll try to look into it later,
> >though.

> I was suggesting "two methods so that it's clear that this case only 
> applies when doing a jump".  This won't involve anything like 
> deprecating /removing decr_pc_after_break _+ write_pc but will involve 
> the addition of a new method like:
> 	set_resume_address (arch, targ or tpid or regs)
> that could somehow default to a legacy call to write_pc.  Significantly, 
> this will avoid making the changes conditional on the elimination of 
> decr-pc (your concern).
> 
> Why is this better?  It clearly separates the [apparently] legetimate 
> resume case from the decr-pc case.  This in turn opens the way for the 
> deprecate / delete decr-pc "write_pc" code while at the same time 
> ensuring that the work can't break the arm.

Sorry, but you did not answer my question.  My question was, are you
specifically objecting to this patch pending the, IMO tangentially
related, architectural cleanup?

I understand your suggested change and your reasoning for it, I just
don't understand your answer to my question.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: RFA/ARM: Switch mode when setting PC

[Richard Earnshaw]

> On Fri, Jan 16, 2004 at 02:12:18PM -0500, Andrew Cagney wrote:
> > Or a lack of design, Arm needs to ensure that it doesn't define PC_REGNUM.
> 
> OK, I'll keep that in mind.
> 
> > >In the past we've tried to distinguish R15 from PC.  This was especially 
> > >useful in the legacy 26-bit mode where the CPSR bits *were* in R15.
> 
> Perhaps, in that case, we should have the PC as a pseudoregister.  That
> would simplify a lot of arm-tdep.c.

That was my feeling too.

>  Is it possible to use the existing
> ARM compiler and simulator (and newlib) in 26-bit mode without a lot of
> hassle, or would it be the work of months to make it happen?  I'm
> reluctant to dig my fingers in too deeply if I can't test both code
> paths.

It should be possible.  At least, it used to be possible.  In GDB you 
should be able to do

	set arm apcs32 off

and then you should get the debugger to think it is talking to a 26-bit 
mode host.  I doubt the code has been tested much recently.

In fact, I want to deprecate support for compiling 26-bit mode code in gcc 
fairly soon -- but that isn't quite the same thing as deprecating 26-bit 
mode execution; a processor running in 26-bit mode can generally run 
apcs32 code as long as certain rare sequences are avoided.

R.

Re: RFA/ARM: Switch mode when setting PC

[Andrew Cagney]

>> On Fri, Jan 16, 2004 at 02:12:18PM -0500, Andrew Cagney wrote:
> 
>> > Or a lack of design, Arm needs to ensure that it doesn't define PC_REGNUM.
> 
>> 
>> OK, I'll keep that in mind.
>> 
> 
>> > >In the past we've tried to distinguish R15 from PC.  This was especially 
>> > >useful in the legacy 26-bit mode where the CPSR bits *were* in R15.
> 
>> 
>> Perhaps, in that case, we should have the PC as a pseudoregister.  That
>> would simplify a lot of arm-tdep.c.
> 
> 
> That was my feeling too.

BTW, the user visible $pc is now typically per-frame and tied to the 
"unwound return address".  If that pseudo-register were made visible 
things could get confusing.

Andrew

Re: RFA/ARM: Switch mode when setting PC

[Andrew Cagney]

>> If there's an explicit "set_resume_address", separate to write_pc, this 
>> should happen:
>> 
>> 	(gdb) set $r15 = 0x123
>> 	- target sees:
>> 		$r15=0x123
>> 	(gdb) call foo()   OR (gdb) jump foo
>> 	- target, via "set_resume_address", sees:
>> 		$r15=&foo
>> 		$ps&|=<magic-bits>
>> 
>> and significantly no other write_pc calls.
> 
> 
> And at this point, is write_pc used for anything besides
> DECR_PC_AFTER_BREAK?

Hopefully not (well ignoring -tdep files).

>  I would prefer to add an adjust_pc_after_break,
> and then possibly rename the existing write_pc.  Most of the write_pc
> implementations we have currently are really set-resume-address
> semantics.

The change wouldn't be tested, and would be more work.

> On Fri, Jan 16, 2004 at 01:57:40PM -0500, Andrew Cagney wrote:
> 
>> >On Fri, Jan 16, 2004 at 09:10:40AM -0500, Daniel Jacobowitz wrote:
> 
> 
>> >Is this patch OK (write_pc isn't deprecated yet!)?  Cleaning up the
>> >existing DECR_PC_AFTER_BREAK handling is going to be a touchy job, and
>> >I don't really want to try it today :)  I'll try to look into it later,
>> >though.
> 
> 
>> I was suggesting "two methods so that it's clear that this case only 
>> applies when doing a jump".  This won't involve anything like 
>> deprecating /removing decr_pc_after_break _+ write_pc but will involve 
>> the addition of a new method like:
>> 	set_resume_address (arch, targ or tpid or regs)
>> that could somehow default to a legacy call to write_pc.  Significantly, 
>> this will avoid making the changes conditional on the elimination of 
>> decr-pc (your concern).
>> 
>> Why is this better?  It clearly separates the [apparently] legetimate 
>> resume case from the decr-pc case.  This in turn opens the way for the 
>> deprecate / delete decr-pc "write_pc" code while at the same time 
>> ensuring that the work can't break the arm.
> 
> 
> Sorry, but you did not answer my question.  My question was, are you
> specifically objecting to this patch pending the, IMO tangentially
> related, architectural cleanup?

What ever.

Andrew

Re: RFA/ARM: Switch mode when setting PC

[Daniel Jacobowitz]

On Sat, Jan 17, 2004 at 11:12:45AM -0500, Andrew Cagney wrote:
> 
> >>If there's an explicit "set_resume_address", separate to write_pc, this 
> >>should happen:
> >>
> >>	(gdb) set $r15 = 0x123
> >>	- target sees:
> >>		$r15=0x123
> >>	(gdb) call foo()   OR (gdb) jump foo
> >>	- target, via "set_resume_address", sees:
> >>		$r15=&foo
> >>		$ps&|=<magic-bits>
> >>
> >>and significantly no other write_pc calls.
> >
> >
> >And at this point, is write_pc used for anything besides
> >DECR_PC_AFTER_BREAK?
> 
> Hopefully not (well ignoring -tdep files).

That's my thought too.

> > I would prefer to add an adjust_pc_after_break,
> >and then possibly rename the existing write_pc.  Most of the write_pc
> >implementations we have currently are really set-resume-address
> >semantics.
> 
> The change wouldn't be tested, and would be more work.

What wouldn't be tested?  I have this handy DECR_PC_AFTER_BREAK target
machine to test on... in fact, I have access to m68k, alpha, s390,
i386, and a d10v simulator.  Definitely more work, though.

I've now begun work on the cleanup of DECR_PC_AFTER_BREAK in a local
tree.  More to follow.  It's going to require painful testing...

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: RFA/ARM: Switch mode when setting PC

[Daniel Jacobowitz]

On Thu, Jan 15, 2004 at 10:54:02PM -0500, Daniel Jacobowitz wrote:
> This patch fixes several failures in virtfunc.exp for arm-sim/-mthumb.  The
> problem is that the non-virtual thunk for pDe->vg() is emitted in ARM mode
> and called via _call_via_r2.  But the rest of the program is Thumb mode, and
> nothing tells the simulator (or target; I haven't tested this on hardware
> yet but I expect the same result) to switch to ARM.  So it gets very
> confused.
> 
> This is somewhat suboptimal in that if you want, for some reason, to call
> something marked as an ARM function with Thumb mode enabled you will have to
> set $ps yourself _after_ setting $pc.  I think it's still a good idea,
> though.  OK?

Checked in, with copyright date update.  I will come back to look at
separating r15 and PC some more, later.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer