From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5607 invoked by alias); 11 Apr 2011 03:05:57 -0000 Received: (qmail 5598 invoked by uid 22791); 11 Apr 2011 03:05:56 -0000 X-SWARE-Spam-Status: No, hits=-1.8 required=5.0 tests=AWL,BAYES_00,TW_EG,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 11 Apr 2011 03:05:50 +0000 Received: (qmail 13817 invoked from network); 11 Apr 2011 03:05:49 -0000 Received: from unknown (HELO ?192.168.0.102?) (yao@127.0.0.2) by mail.codesourcery.com with ESMTPA; 11 Apr 2011 03:05:49 -0000 Message-ID: <4DA27006.1080607@codesourcery.com> Date: Mon, 11 Apr 2011 03:05:00 -0000 From: Yao Qi User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.14) Gecko/20110223 Lightning/1.0b2 Thunderbird/3.1.8 MIME-Version: 1.0 To: gdb@sourceware.org Subject: Re: [PATCH] arm reversible : progress References: <341905.10459.qm@web112513.mail.gq1.yahoo.com> <208397.95006.qm@web112517.mail.gq1.yahoo.com> In-Reply-To: <208397.95006.qm@web112517.mail.gq1.yahoo.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-IsSubscribed: yes Mailing-List: contact gdb-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-owner@sourceware.org X-SW-Source: 2011-04/txt/msg00028.txt.bz2 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 (齐尧)