Re: [PATCH] arm reversible : progress <phase_2_complete>

paawan oza
2011-04-11T04:52:25Z

Hi Yao,

thanks for your comments.

1) will try to use macros for arm insns as you suggesed.

2) displaced_in_arm_mode and arm_frame_is_thumb checks T bit differenly, I do no
follow frame unwind logic and checking T bit;

infact the arm-reference-manual which I referred; I could not find 'M' profile
information too.
hence I just checked current CSPR value.
bu yes as you said; I shall change the code;

3) as of now it I have support for arm and thumb(16) only. no plan for thumb32
support.

4) please also refer to FIX ME in the code, specially how o read SPSR value ?

5) also need to know how to read coprocessor registers' value ?

Regards,
Oza.

----- Original Message ----
From: Yao Qi <yao@codesourcery.com>
To: gdb@sourceware.org
Sent: Mon, April 11, 2011 8:35:42 AM
Subject: Re: [PATCH] arm reversible : progress <phase_2_complete>

On 04/10/2011 05:41 PM, paawan oza wrote:
> Hi,
>
> phase2 (both arm and thumb insn implemenation is complete)
>
> Hi Tom:
> I have taken care of most of your comments: but could nit incorporate numercial
>
> to be replaced by some meaningful symbolic names.
> as I dont know what sort of symbolic names are approproate.
>
>

This one looks much better than previous ones. Thanks for working on
this. I am not the people to approve this patch. Some of my cents below.

> +
> +#define ARM_INSN_SIZE_BYTES 4
> +#define THUMB_INSN_SIZE_BYTES 2
> +#define NO_OF_TYPE_OF_ARM_INSNS 8
> +#define NO_OF_TYPE_OF_THUMB_INSNS 8
> +
> +#define ARM_RECORD_ARCH_LIST_ADD_REG(regnum) \
> + record_arch_list_add_reg (arm_record.regcache, regnum)
> +
> +#define GET_REG_VAL(REGCACHE,NO,BUF) regcache_raw_read (REGCACHE, NO, BUF);
> +
> +#define IS_IT_ARM_INSN(X) ((X & 0x00000020) >> 5)
> +#define ARM_PARSE_INSN(X,BIT_POS,NO_OF_BITS) \
> + ((X >> (BIT_POS-1)) & (0xFFFFFFFF >> ((sizeof(uint32_t)*8) -
\
> + NO_OF_BITS)))

There are some existing macros you can use for parsing instructions.

/* Support routines for instruction parsing. */
#define submask(x) ((1L << ((x) + 1)) - 1)
#define bit(obj,st) (((obj) >> (st)) & 1)
#define bits(obj,st,fn) (((obj) >> (st)) & submask ((fn) - (st)))
#define sbits(obj,st,fn) \
((long) (bits(obj,st,fn) | ((long) bit(obj,fn) * ~ submask (fn - st))))

> +
> +#define INSN_S_L_BIT_NUM 21
> +#define ARM_BIT_SET(X, NUM) (((X >> (NUM-1)) & 0x00000001) == 1)
> +#define GET_BIT(X, NUM) (((X >> (NUM-1)) & 0x00000001))
> +

GET_BIT can be replaced by `bit' I posted above.

> +
> +static int
> +handle_extension_space (insn_decode_record *arm_record)
> +{
> + insn_decode_record *arm_insn_r = arm_record;
> + struct gdbarch_tdep *tdep = gdbarch_tdep (arm_insn_r->gdbarch);
> + struct regcache *reg_cache = arm_insn_r->regcache;
> +
> + uint32_t reg_src1 = 0, reg_src2 = 0;
> + uint32_t opcode1 = 0, opcode2 = 0;
> +
> + opcode1 = ARM_PARSE_INSN (arm_insn_r->arm_insn,26,3);
> + if ((3 == opcode1) && ARM_BIT_SET(arm_insn_r->arm_insn,5))
> + {
> + /* undefined instruction on ARM V5; need to handle if later versions
> + define it. */
> + }
> +
> + opcode2 = ARM_PARSE_INSN (arm_insn_r->arm_insn,5,4);
> +
> + if ((!opcode1) && (9 == opcode2))
> + {
> + /* handle arithmetic insn extension space. */
> + }
> +
> + opcode1 = ARM_PARSE_INSN (arm_insn_r->arm_insn,27,2);
^ ^
You need an extra space after each comma.

> + opcode2 = ARM_PARSE_INSN (arm_insn_r->arm_insn,24,2);
> +
> + if ((!opcode1) && (2 == opcode2) && !ARM_BIT_SET(arm_insn_r->arm_insn,21))
> + {
> + /* handle control insn extension space. */
> + }
> +
> + opcode1 = ARM_PARSE_INSN (arm_insn_r->arm_insn,26,3);
> + if ((!opcode1) && (ARM_BIT_SET(arm_insn_r->arm_insn,8)) \
> + && (ARM_BIT_SET(arm_insn_r->arm_insn,5)))
> + {
> + /* handle load/store insn extension space. */
> + }
> +
> + opcode1 = ARM_PARSE_INSN (arm_insn_r->arm_insn,24,5);
> + if ((24 == opcode1) && ARM_BIT_SET(arm_insn_r->arm_insn,22))
> + {
> + /* handle coprocessor insn extension space. */
> + }
> +
> + /* to be done for ARMv5 and later; as of now we return -1. */
> + return -1;
> +}
> +

> +
> +/* Parse the current instruction and record the values of the registers and
> + memory that will be changed in current instruction to "record_arch_list".
> + Return -1 if something is wrong.. */
> +
> +int
> +arm_process_record (struct gdbarch *gdbarch, struct regcache *regcache,
> + CORE_ADDR insn_addr)
> +{
> +
> + enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> + struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
> + uint32_t no_of_rec=0;
> + uint32_t ret=0;
> +
> + union
> + {
> + uint32_t s_word;
> + gdb_byte buf[4];
> + } u_buf;
> +
> + insn_decode_record arm_record;
> + memset (&u_buf, 0, sizeof(u_buf));
> +
> + memset (&arm_record, 0, sizeof (insn_decode_record));
> + arm_record.regcache = regcache;
> + arm_record.this_addr = insn_addr;
> + arm_record.gdbarch = gdbarch;
> +
> +
> + if (record_debug > 1)
> + {
> + fprintf_unfiltered (gdb_stdlog, "Process record: arm_process_record "
> + "addr = %s\n",
> + paddress (gdbarch, arm_record.this_addr));
> + }
> +
> + /* check the insn, whether it is thumb or arm one. */
> + GET_REG_VAL (arm_record.regcache, ARM_PS_REGNUM, &u_buf.buf[0]);
> + arm_record.cond = ARM_PARSE_INSN (arm_record.arm_insn,29,4);
> +
> + if (!IS_IT_ARM_INSN (u_buf.s_word))

Please reference to `arm_frame_is_thumb' or `displaced_in_arm_mode' to
see how to check ARM mode or Thumb mode.

> + {
> + /* we are decoding arm insn. */
> + ret = decode_insn (&arm_record, ARM_INSN_SIZE_BYTES);
> + }
> + else
> + {
> + /* we are decoding thumb insn. */
> + ret = decode_insn (&arm_record, THUMB_INSN_SIZE_BYTES);

On some ARM arch, there are 32-bit Thumb instructions, called Thumb-2.
Do you plan to support Thumb-2 insn?

> + }
> +
> + /* record registers. */
> + ARM_RECORD_ARCH_LIST_ADD_REG(ARM_PC_REGNUM);
> + if (arm_record.arm_regs)
> + {
> + for (no_of_rec=1;no_of_rec<=arm_record.arm_regs[0];no_of_rec++)
> + {
> + if (ARM_RECORD_ARCH_LIST_ADD_REG (arm_record.arm_regs[no_of_rec]))
> + ret = -1;
> + }
> + }
> + /* record memories. */
> + if (arm_record.arm_mems)
> + {
> + for (no_of_rec=1;no_of_rec<=arm_record.arm_mems[0].len;no_of_rec++)
> + {
> + if (record_arch_list_add_mem \
> + ((CORE_ADDR)arm_record.arm_mems[no_of_rec].addr,
> + arm_record.arm_mems[no_of_rec].len))
> + ret = -1;
> + }
> + }
> +
> + if (record_arch_list_add_end ())
> + ret = -1;
> +
> + if (arm_record.arm_regs)
> + xfree (arm_record.arm_regs);
> + if (arm_record.arm_mems)
> + xfree (arm_record.arm_mems);
> +
> + return ret;
> +}

--
Yao (齐尧)