On 03/01/2011 01:07 AM, Ulrich Weigand wrote:
> OK, so at this point I think it's really not necessary to have those
> as macros in the first place.  Instead, code should just continue to
> fill in dsc->modinsn, which will shorten this patch significantly :-)
> 

I am hesitant to remove these macros, because my following patches are
using these macros.  Now, since most of my following patches should be
re-written, I am fine to remove these macros.

>> > @@ -5117,10 +5119,21 @@ displaced_read_reg (struct regcache *regs, CORE_ADDR from, int regno)
>> >  
>> >    if (regno == 15)
>> >      {
>> > +      /* Compute pipeline offset:
>> > +	 - When executing an ARM instruction, PC reads as the address of the
>> > +	 current instruction plus 8.
>> > +	 - When executing a Thumb instruction, PC reads as the address of the
>> > +	 current instruction plus 4.  */
>> > +
>> > +      if (displaced_in_arm_mode (regs))
> It would be somewhat nicer here to use dsc->is_thumb instead of re-computing
> its value.  However, the displaced_read_reg function doesn't have the dsc
> argument, which is annoying (and asymmetrical to displaced_write_reg ...).
> 
> So if you want to make the effort to change all call sites to pass in dsc,
> this would be nice, but I guess I'm also OK with doing it as above.
> 

Let us move this change to another patch.

>> > @@ -6904,23 +6919,49 @@ arm_displaced_init_closure (struct gdbarch *gdbarch, CORE_ADDR from,
>> >  			    CORE_ADDR to, struct displaced_step_closure *dsc)
>> >  {
>> >    struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
>> > -  unsigned int i;
>> > +  unsigned int i, len, offset;
>> >    enum bfd_endian byte_order_for_code = gdbarch_byte_order_for_code (gdbarch);
>> > +  int size = dsc->insn_size;
> Ah, this is wrong: it needs to be "dsc->is_thumb? 2 : 4".  Note that if the
> original instruction was 32-bit Thumb2, insn_size will be 4, but we still
> need to copy 2-byte chunks here.

Right.

-- 
Yao (齐尧)