From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4421 invoked by alias); 7 Apr 2011 08:02:35 -0000 Received: (qmail 4406 invoked by uid 22791); 7 Apr 2011 08:02:30 -0000 X-SWARE-Spam-Status: No, hits=-1.8 required=5.0 tests=AWL,BAYES_00,TW_BL,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; Thu, 07 Apr 2011 08:02:20 +0000 Received: (qmail 5100 invoked from network); 7 Apr 2011 08:02:18 -0000 Received: from unknown (HELO ?192.168.0.102?) (yao@127.0.0.2) by mail.codesourcery.com with ESMTPA; 7 Apr 2011 08:02:18 -0000 Message-ID: <4D9D6F85.1030008@codesourcery.com> Date: Thu, 07 Apr 2011 08:02: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: Ulrich Weigand CC: gdb-patches@sourceware.org Subject: Re: [try 2nd 2/8] Rename copy_* functions to arm_copy_* References: <201104062051.p36KpOLp003493@d06av02.portsmouth.uk.ibm.com> In-Reply-To: <201104062051.p36KpOLp003493@d06av02.portsmouth.uk.ibm.com> Content-Type: multipart/mixed; boundary="------------050709040500050303050503" 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-04/txt/msg00107.txt.bz2 This is a multi-part message in MIME format. --------------050709040500050303050503 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Content-length: 1119 On 04/07/2011 04:51 AM, Ulrich Weigand wrote: > The interface between the copy_ routines and the install_ routines > seems a bit odd in some cases: mostly, values are just passed as > arguments, but in some cases, they are passed via displaced_step_closure > fields. > The parameter list of some of install_* routines is a little bit long, if we pass values as arguments. > I'd prefer if this were handled in a regular manner: the copy_ > routines parse the insn text, extract all required information > and pass it all as arguments to the install_ routine. The > install_ routine then stores all information that is needed > by the cleanup_ routine into the displaced_step_closure struct. >From the consistency's point of view, I am fine with your approach. In this new patch, the following things are changed, 1. Merge 6/8 patch, 2. Pass all values as parameters to install_* routines, 3. Fix install_* routines' arguments order, 4. Change all install_* routines to void method. This patches makes patch 4/8 and 5/8 obsolete. I'll re-send an updated version once this patch is approved. -- Yao (齐尧) --------------050709040500050303050503 Content-Type: text/x-patch; name="0003-refactor-rename-functions.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="0003-refactor-rename-functions.patch" Content-length: 45384 * gdb/arm-tdep.c (copy_unmodified): Rename to ... (arm_copy_unmodified): .. this. New. (copy_preload): Move common part to ... (install_preload): .. this. New. (arm_copy_preload): New. (copy_preload_reg): Move common part to ... (install_preload_reg): ... this. New. (arm_copy_preload_reg): New. (copy_b_bl_blx): Move common part to ... (install_b_bl_blx): .. this. New. (arm_copy_b_bl_blx): New. (copy_bx_blx_reg): Move common part to ... (install_bx_blx_reg): ... this. New. (arm_copy_bx_blx_reg): New. (copy_alu_reg): Move common part to ... (install_alu_reg): ... this. New. (arm_copy_alu_reg): New. (copy_alu_shifted_reg): Move common part to ... (install_alu_shifted_reg): ... this. New. (copy_ldr_str_ldrb_strb): Move common part to ... (install_ldr_str_ldrb_strb): ... this. New. (arm_copy_ldr_str_ldrb_strb): New. * (copy_copro_load_store): Move some common part to ... (install_copy_copro_load_store): ... this. New. (arm_copy_copro_load_store): New. (copy_svc): Delete. (arm_copy_svc): Renamed from copy_svc. (copy_undef): Delete. (arm_copy_undef): Renamed from copy_undef. (decode_ext_reg_ld_st): Delete. (arm_decode_ext_reg_ld_st): Renamed from decode_ext_reg_ld_st. (decode_svc_copro): Delete. (arm_decode_svc_copro): Renamed from decode_svc_copro. (copy_copro_load_store, copy_alu_imm): update callers. (copy_extra_ld_st, copy_block_xfer): Likewise. (decode_misc_memhint_neon, decode_unconditional): Likewise. (decode_miscellaneous, decode_dp_misc): Likewise. (decode_ld_st_word_ubyte, decode_media): Likewise. (decode_b_bl_ldmstm, decode_ext_reg_ld_st): Likewise. (decode_svc_copro, decode_misc_memhint_neon): Likewise. (decode_unconditional, decode_miscellaneous): Likewise. (decode_media, decode_b_bl_ldmstm): Likewise. (arm_process_displaced_insn): Likewise.. --- gdb/arm-tdep.c | 606 +++++++++++++++++++++++++++++++------------------------- 1 files changed, 340 insertions(+), 266 deletions(-) diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c index e284b39..ff1f10f 100644 --- a/gdb/arm-tdep.c +++ b/gdb/arm-tdep.c @@ -5327,8 +5327,8 @@ insn_references_pc (uint32_t insn, uint32_t bitmask) matter what address they are executed at: in those cases, use this. */ static int -copy_unmodified (struct gdbarch *gdbarch, uint32_t insn, - const char *iname, struct displaced_step_closure *dsc) +arm_copy_unmodified (struct gdbarch *gdbarch, uint32_t insn, + const char *iname, struct displaced_step_closure *dsc) { if (debug_displaced) fprintf_unfiltered (gdb_stdlog, "displaced: copying insn %.8lx, " @@ -5351,20 +5351,11 @@ cleanup_preload (struct gdbarch *gdbarch, displaced_write_reg (regs, dsc, 1, dsc->tmp[1], CANNOT_WRITE_PC); } -static int -copy_preload (struct gdbarch *gdbarch, uint32_t insn, struct regcache *regs, - struct displaced_step_closure *dsc) +static void +install_preload (struct gdbarch *gdbarch, struct regcache *regs, + struct displaced_step_closure *dsc, unsigned int rn) { - unsigned int rn = bits (insn, 16, 19); ULONGEST rn_val; - - if (!insn_references_pc (insn, 0x000f0000ul)) - return copy_unmodified (gdbarch, insn, "preload", dsc); - - if (debug_displaced) - fprintf_unfiltered (gdb_stdlog, "displaced: copying preload insn %.8lx\n", - (unsigned long) insn); - /* Preload instructions: {pli/pld} [rn, #+/-imm] @@ -5374,34 +5365,40 @@ copy_preload (struct gdbarch *gdbarch, uint32_t insn, struct regcache *regs, dsc->tmp[0] = displaced_read_reg (regs, dsc, 0); rn_val = displaced_read_reg (regs, dsc, rn); displaced_write_reg (regs, dsc, 0, rn_val, CANNOT_WRITE_PC); - dsc->u.preload.immed = 1; - dsc->modinsn[0] = insn & 0xfff0ffff; - dsc->cleanup = &cleanup_preload; - - return 0; } -/* Preload instructions with register offset. */ - static int -copy_preload_reg (struct gdbarch *gdbarch, uint32_t insn, - struct regcache *regs, +arm_copy_preload (struct gdbarch *gdbarch, uint32_t insn, struct regcache *regs, struct displaced_step_closure *dsc) { unsigned int rn = bits (insn, 16, 19); - unsigned int rm = bits (insn, 0, 3); - ULONGEST rn_val, rm_val; - if (!insn_references_pc (insn, 0x000f000ful)) - return copy_unmodified (gdbarch, insn, "preload reg", dsc); + if (!insn_references_pc (insn, 0x000f0000ul)) + return arm_copy_unmodified (gdbarch, insn, "preload", dsc); if (debug_displaced) fprintf_unfiltered (gdb_stdlog, "displaced: copying preload insn %.8lx\n", (unsigned long) insn); + dsc->modinsn[0] = insn & 0xfff0ffff; + + install_preload (gdbarch, regs, dsc, rn); + + return 0; +} + +/* Preload instructions with register offset. */ + +static void +install_preload_reg(struct gdbarch *gdbarch, struct regcache *regs, + struct displaced_step_closure *dsc, unsigned int rn, + unsigned int rm) +{ + ULONGEST rn_val, rm_val; + /* Preload register-offset instructions: {pli/pld} [rn, rm {, shift}] @@ -5414,13 +5411,30 @@ copy_preload_reg (struct gdbarch *gdbarch, uint32_t insn, rm_val = displaced_read_reg (regs, dsc, rm); displaced_write_reg (regs, dsc, 0, rn_val, CANNOT_WRITE_PC); displaced_write_reg (regs, dsc, 1, rm_val, CANNOT_WRITE_PC); - dsc->u.preload.immed = 0; - dsc->modinsn[0] = (insn & 0xfff0fff0) | 0x1; - dsc->cleanup = &cleanup_preload; +} + +static int +arm_copy_preload_reg (struct gdbarch *gdbarch, uint32_t insn, + struct regcache *regs, + struct displaced_step_closure *dsc) +{ + unsigned int rn = bits (insn, 16, 19); + unsigned int rm = bits (insn, 0, 3); + + + if (!insn_references_pc (insn, 0x000f000ful)) + return arm_copy_unmodified (gdbarch, insn, "preload reg", dsc); + + if (debug_displaced) + fprintf_unfiltered (gdb_stdlog, "displaced: copying preload insn %.8lx\n", + (unsigned long) insn); + + dsc->modinsn[0] = (insn & 0xfff0fff0) | 0x1; + install_preload_reg (gdbarch, regs, dsc, rn, rm); return 0; } @@ -5439,21 +5453,13 @@ cleanup_copro_load_store (struct gdbarch *gdbarch, displaced_write_reg (regs, dsc, dsc->u.ldst.rn, rn_val, LOAD_WRITE_PC); } -static int -copy_copro_load_store (struct gdbarch *gdbarch, uint32_t insn, - struct regcache *regs, - struct displaced_step_closure *dsc) +static void +install_copy_copro_load_store (struct gdbarch *gdbarch, struct regcache *regs, + struct displaced_step_closure *dsc, + int writeback, unsigned int rn) { - unsigned int rn = bits (insn, 16, 19); ULONGEST rn_val; - if (!insn_references_pc (insn, 0x000f0000ul)) - return copy_unmodified (gdbarch, insn, "copro load/store", dsc); - - if (debug_displaced) - fprintf_unfiltered (gdb_stdlog, "displaced: copying coprocessor " - "load/store insn %.8lx\n", (unsigned long) insn); - /* Coprocessor load/store instructions: {stc/stc2} [, #+/-imm] (and other immediate addressing modes) @@ -5462,16 +5468,33 @@ copy_copro_load_store (struct gdbarch *gdbarch, uint32_t insn, ldc/ldc2 are handled identically. */ + dsc->u.ldst.writeback = writeback; + dsc->u.ldst.rn = rn; + dsc->tmp[0] = displaced_read_reg (regs, dsc, 0); - rn_val = displaced_read_reg (regs, dsc, rn); + rn_val = displaced_read_reg (regs, dsc, dsc->u.ldst.rn); displaced_write_reg (regs, dsc, 0, rn_val, CANNOT_WRITE_PC); - dsc->u.ldst.writeback = bit (insn, 25); - dsc->u.ldst.rn = rn; + dsc->cleanup = &cleanup_copro_load_store; +} + +static int +arm_copy_copro_load_store (struct gdbarch *gdbarch, uint32_t insn, + struct regcache *regs, + struct displaced_step_closure *dsc) +{ + unsigned int rn = bits (insn, 16, 19); + + if (!insn_references_pc (insn, 0x000f0000ul)) + return arm_copy_unmodified (gdbarch, insn, "copro load/store", dsc); + + if (debug_displaced) + fprintf_unfiltered (gdb_stdlog, "displaced: copying coprocessor " + "load/store insn %.8lx\n", (unsigned long) insn); dsc->modinsn[0] = insn & 0xfff0ffff; - dsc->cleanup = &cleanup_copro_load_store; + install_copy_copro_load_store (gdbarch, regs, dsc, bit (insn, 25), rn); return 0; } @@ -5510,29 +5533,43 @@ cleanup_branch (struct gdbarch *gdbarch, struct regcache *regs, /* Copy B/BL/BLX instructions with immediate destinations. */ +static void +install_b_bl_blx (struct gdbarch *gdbarch, struct regcache *regs, + struct displaced_step_closure *dsc, + unsigned int cond, int exchange, int link, long offset) +{ + /* Implement "BL