Re: [try 3rd] arm_pc_is_thumb takes displaced stepping into account

[Matthew Gretton-Dann <matthew.gretton-dann@arm.com>]

On Tue, 2011-01-11 at 22:02 -0600, Yao Qi wrote:
> On 12/23/2010 01:00 AM, Yao Qi wrote:
> > OK.  I have to try the second approach, which is 1) exposing displaced
> > stepping state to tdep, and 2) take displaced stepping state into
> > account when determining the mode.
> >
> 
> After talked with Ulrich, I realize that it is *not* a good idea to 
> expose displaced stepping outside of infrun, and my patch is a little 
> bit too intrusive.
> 
> > 2010-12-23  Yao Qi<yao@codesourcery.com>
> >
> > 	* arm-tdep.c: (arm_pc_is_thumb):  Adjust MEMADDR if it is within
> > 	copy area of displaced stepping.
> > 	* infrun.c (struct displaced_step_inferior_state): Move to ...
> > 	Expose get_displaced_stepping_state.
> > 	* inferior.h: ... here.
> > 	Declare get_displaced_stepping_state.
> 
> This time, instead of exposing displaced_step_inferior_state to tdep, we 
> return displaced_step_closure, which is defined by each tdep, instance 
> to tdep appropriately.
> 
> OK to mainline?

I agree with Ulrich, and prefer this latest patch to your previous ones
- but I am still not 100% happy with it, as I think it would be better
not to need anything from infrun.c at all here.

What could be done instead is to have the displaced stepping routines
maintain a list of the areas of memory that are being used as scratch
space, and the instruction set state of the instructions in those areas.

Then arm_pc_is_thumb should check this list to see if the PC falls into
one of these areas, and return the appropriate value.

This keeps these changes wholly within the ARM backend for GDB, and
doesn't require changes to GDB globally.

However, this will impact performance and memory more than your patch -
so I am not completely opposed to your patch.

This is yet another instance of a problem Ulrich and I discussed last
year, which is: How do you tell what instruction-set state you are in
when you just have a CORE_ADDR, which is insufficient information on
ARM.  See the thread starting at:
http://sourceware.org/ml/gdb-patches/2010-08/msg00271.html

The long-term solution is probably either to implement multiple address
spaces for code, or to treat ARM & Thumb as different gdbarch's and use
multiarch support.  However, this is a much more difficult problem to
solve in general and shouldn't stop this patch going in.

[Note again that I am not a maintainer and so do not have approval
rights].

Thanks,

Matt

-- 
Matthew Gretton-Dann
Principal Engineer - PDSW Tools
ARM Ltd