Re: [patch 2/3] Displaced stepping for 16-bit Thumb instructions

Ulrich Weigand
2011-02-28T17:08:06Z

Yao Qi wrote:

> +/* Record an ARM mode instruction in one slot. */
> +#define RECORD_ARM_MODE_INSN(INDEX, INSN) do \
> +{\
> + dsc->modinsn[INDEX] = INSN;\
> + } while (0)
> +
> +#define RECORD_THUMB_MODE_INSN(INDEX, INSN) do \
> +{\
> + dsc->modinsn[INDEX] = INSN;\
> + } while (0)
> +
> +/* Record the two parts of 32-bit Thumb-2 instruction. Each part occupies
> + one array element. */
> +#define RECORD_THUMB2_MODE_INSN(INDEX, INSN1, INSN2) do \
> +{ \
> + dsc->modinsn[INDEX] = INSN1;\
> + dsc->modinsn[INDEX + 1] = INSN2;\
> +} while (0)

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 :-)

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

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

Otherwise, this looks OK to me now. Thanks for your continued effort to work
on this feature!

Bye,
Ulrich

--
Dr. Ulrich Weigand
GNU Toolchain for Linux on System z and Cell BE
Ulrich.Weigand@de.ibm.com