From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24135 invoked by alias); 1 Mar 2011 09:01:47 -0000 Received: (qmail 24124 invoked by uid 22791); 1 Mar 2011 09:01:45 -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; Tue, 01 Mar 2011 09:01:41 +0000 Received: (qmail 23513 invoked from network); 1 Mar 2011 09:01:38 -0000 Received: from unknown (HELO ?192.168.0.101?) (yao@127.0.0.2) by mail.codesourcery.com with ESMTPA; 1 Mar 2011 09:01:38 -0000 Message-ID: <4D6CB5F0.4070203@codesourcery.com> Date: Tue, 01 Mar 2011 09:01:00 -0000 From: Yao Qi User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.13) Gecko/20101208 Lightning/1.0b2 Thunderbird/3.1.7 MIME-Version: 1.0 To: Ulrich Weigand CC: gdb-patches@sourceware.org Subject: Re: [patch 2/3] Displaced stepping for 16-bit Thumb instructions References: <201102281707.p1SH7rjs002745@d06av02.portsmouth.uk.ibm.com> In-Reply-To: <201102281707.p1SH7rjs002745@d06av02.portsmouth.uk.ibm.com> Content-Type: multipart/mixed; boundary="------------010407010401050003060200" X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2011-03/txt/msg00024.txt.bz2 This is a multi-part message in MIME format. --------------010407010401050003060200 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Content-length: 1972 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 (齐尧) --------------010407010401050003060200 Content-Type: text/x-patch; name="0000-handle-both-32-bit-and-16-bit.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="0000-handle-both-32-bit-and-16-bit.patch" Content-length: 4954 gdb/ * arm-tdep.h (struct displaced_step_closure): Add two new fields is_thumb and insn_size. * arm-tdep.c (displaced_read_reg): Adjust correct pipeline offset on both ARM and Thumb mode. (arm_process_displaced_insn): Set is_thumb and insn_size. (arm_displaced_init_closure): Handle both 16-bit and 32-bit. (arm_displaced_step_fixup): Likewise. diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c index f0e9435..555a6eb 100644 --- a/gdb/arm-tdep.c +++ b/gdb/arm-tdep.c @@ -5106,6 +5106,8 @@ arm_adjust_breakpoint_address (struct gdbarch *gdbarch, CORE_ADDR bpaddr) /* NOP instruction (mov r0, r0). */ #define ARM_NOP 0xe1a00000 +static int displaced_in_arm_mode (struct regcache *regs); + /* Helper for register reads for displaced stepping. In particular, this returns the PC as it would be seen by the instruction at its original location. */ @@ -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)) + from += 8; + else + from += 4; + if (debug_displaced) fprintf_unfiltered (gdb_stdlog, "displaced: read pc value %.8lx\n", - (unsigned long) from + 8); - return (ULONGEST) from + 8; /* Pipeline offset. */ + (unsigned long) from); + return (ULONGEST) from; } else { @@ -6861,6 +6874,8 @@ arm_process_displaced_insn (struct gdbarch *gdbarch, CORE_ADDR from, if (!displaced_in_arm_mode (regs)) return thumb_process_displaced_insn (gdbarch, from, to, regs, dsc); + dsc->is_thumb = 0; + dsc->insn_size = 4; insn = read_memory_unsigned_integer (from, 4, byte_order_for_code); if (debug_displaced) fprintf_unfiltered (gdb_stdlog, "displaced: stepping insn %.8lx " @@ -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->is_thumb? 2 : 4; + const unsigned char *bkp_insn; + offset = 0; /* Poke modified instruction(s). */ for (i = 0; i < dsc->numinsns; i++) { if (debug_displaced) - fprintf_unfiltered (gdb_stdlog, "displaced: writing insn %.8lx at " - "%.8lx\n", (unsigned long) dsc->modinsn[i], - (unsigned long) to + i * 4); - write_memory_unsigned_integer (to + i * 4, 4, byte_order_for_code, + { + fprintf_unfiltered (gdb_stdlog, "displaced: writing insn "); + if (size == 4) + fprintf_unfiltered (gdb_stdlog, "%.8lx", + dsc->modinsn[i]); + else if (size == 2) + fprintf_unfiltered (gdb_stdlog, "%.4x", + (unsigned short)dsc->modinsn[i]); + + fprintf_unfiltered (gdb_stdlog, " at %.8lx\n", + (unsigned long) to + offset); + + } + write_memory_unsigned_integer (to + offset, size, + byte_order_for_code, dsc->modinsn[i]); + offset += size; + } + + /* Choose the correct breakpoint instruction. */ + if (dsc->is_thumb) + { + bkp_insn = tdep->thumb_breakpoint; + len = tdep->thumb_breakpoint_size; + } + else + { + bkp_insn = tdep->arm_breakpoint; + len = tdep->arm_breakpoint_size; } /* Put breakpoint afterwards. */ - write_memory (to + dsc->numinsns * 4, tdep->arm_breakpoint, - tdep->arm_breakpoint_size); + write_memory (to + offset, bkp_insn, len); if (debug_displaced) fprintf_unfiltered (gdb_stdlog, "displaced: copy %s->%s: ", @@ -6956,7 +6997,9 @@ arm_displaced_step_fixup (struct gdbarch *gdbarch, dsc->cleanup (gdbarch, regs, dsc); if (!dsc->wrote_to_pc) - regcache_cooked_write_unsigned (regs, ARM_PC_REGNUM, dsc->insn_addr + 4); + regcache_cooked_write_unsigned (regs, ARM_PC_REGNUM, + dsc->insn_addr + dsc->insn_size); + } #include "bfd-in2.h" diff --git a/gdb/arm-tdep.h b/gdb/arm-tdep.h index ef02002..a2293ba 100644 --- a/gdb/arm-tdep.h +++ b/gdb/arm-tdep.h @@ -262,6 +262,17 @@ struct displaced_step_closure struct displaced_step_closure *dsc); } svc; } u; + + /* The size of original instruction, 2 or 4. */ + unsigned int insn_size; + /* True if the original insn (and thus all replacement insns) are Thumb + instead of ARM. */ + unsigned int is_thumb; + + /* The slots in the array is used in this way below, + - ARM instruction occupies one slot, + - Thumb 16 bit instruction occupies one slot, + - Thumb 32-bit instruction occupies *two* slots, one part for each. */ unsigned long modinsn[DISPLACED_MODIFIED_INSNS]; int numinsns; CORE_ADDR insn_addr; --------------010407010401050003060200--