Re: RFA/ARM: Switch mode when setting PC

[Richard Earnshaw <rearnsha at arm dot com>]

> 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.