From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9503 invoked by alias); 10 May 2011 13:58:36 -0000 Received: (qmail 9493 invoked by uid 22791); 10 May 2011 13:58:36 -0000 X-SWARE-Spam-Status: No, hits=-1.3 required=5.0 tests=AWL,BAYES_00,MSGID_FROM_MTA_HEADER,SPF_SOFTFAIL,TW_EG,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mtagate4.uk.ibm.com (HELO mtagate4.uk.ibm.com) (194.196.100.164) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 10 May 2011 13:58:21 +0000 Received: from d06nrmr1307.portsmouth.uk.ibm.com (d06nrmr1307.portsmouth.uk.ibm.com [9.149.38.129]) by mtagate4.uk.ibm.com (8.13.1/8.13.1) with ESMTP id p4ADw9cF014562 for ; Tue, 10 May 2011 13:58:09 GMT Received: from d06av02.portsmouth.uk.ibm.com (d06av02.portsmouth.uk.ibm.com [9.149.37.228]) by d06nrmr1307.portsmouth.uk.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p4ADw9012195700 for ; Tue, 10 May 2011 14:58:09 +0100 Received: from d06av02.portsmouth.uk.ibm.com (loopback [127.0.0.1]) by d06av02.portsmouth.uk.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p4ADw9EH029851 for ; Tue, 10 May 2011 07:58:09 -0600 Received: from tuxmaker.boeblingen.de.ibm.com (tuxmaker.boeblingen.de.ibm.com [9.152.85.9]) by d06av02.portsmouth.uk.ibm.com (8.14.4/8.13.1/NCO v10.0 AVin) with SMTP id p4ADw8D0029817; Tue, 10 May 2011 07:58:08 -0600 Message-Id: <201105101358.p4ADw8D0029817@d06av02.portsmouth.uk.ibm.com> Received: by tuxmaker.boeblingen.de.ibm.com (sSMTP sendmail emulation); Tue, 10 May 2011 15:58:08 +0200 Subject: Re: [try 2nd 4/8] Displaced stepping for Thumb 16-bit insn To: yao@codesourcery.com (Yao Qi) Date: Tue, 10 May 2011 13:58:00 -0000 From: "Ulrich Weigand" Cc: gdb-patches@sourceware.org In-Reply-To: <4DC2A4CE.5030407@codesourcery.com> from "Yao Qi" at May 05, 2011 09:23:26 PM MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit 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-05/txt/msg00240.txt.bz2 Yao Qi wrote: > Support displaced stepping for Thumb 16-bit insns. > * arm-tdep.c (THUMB_NOP) Define. > (thumb_copy_unmodified_16bit): New. > (thumb_copy_b, thumb_copy_bx_blx_reg): New. > (thumb_copy_alu_reg): New. > (arm_copy_svc): Move some common code to ... > (copy_svc): ... here. New. > (thumb_copy_svc): New. > (install_pc_relative): New. > (thumb_copy_pc_relative_16bit): New. > (thumb_decode_pc_relative_16bit): New. > (thumb_copy_16bit_ldr_literal): New. > (thumb_copy_cbnz_cbz): New. > (cleanup_pop_pc_16bit): New. > (thumb_copy_pop_pc_16bit): New. > (thumb_process_displaced_16bit_insn): New. > (thumb_process_displaced_32bit_insn): New. > (thumb_process_displaced_insn): process thumb instruction. This is looking pretty good now, thanks. There is still one problem that I noticed: > +static int > +thumb_copy_pop_pc_16bit (struct gdbarch *gdbarch, unsigned short insn1, > + struct regcache *regs, > + struct displaced_step_closure *dsc) > +{ > + dsc->u.block.regmask = insn1 & 0x00ff; > + > + /* Rewrite instruction: POP {rX, rY, ...,rZ, PC} > + to : > + > + (1) register list is not empty, > + Prepare: tmp[0] <- r8, > + > + POP {rX}; PC is stored in rX > + MOV r8, rX; finally, PC is stored in r8 > + POP {rX, rY, ...., rZ} > + > + Cleanup: PC <-r8, r8 <- tmp[0] It seems this approach is actually incorrect. If you do a POP {rX, rY, ..., rZ, PC} the value at the SP gets restored into rX, the value at rX+4 into rY and so on, and the value at the highest address gets restored into PC. With your replacement sequence, it would appear that you instead get the value at the *lowest* address (just SP) restored into the PC ... Apart from this, just a few minor issues: > +/* Common copy routine for svc instruciton. */ > > +static int > +copy_svc (struct gdbarch *gdbarch, struct regcache *regs, > + struct displaced_step_closure *dsc) > +{ I guess to keep in sync with terminology in the rest of the file, this really should be called install_svc ... > + err = thumb_copy_unmodified_16bit (gdbarch, insn1,"shift/add/sub/mov/cmp", Formatting: space after the comma (here and at a couple other places). > + thumb_process_displaced_32bit_insn(gdbarch, insn1, insn2, regs, dsc); > + } > + else > + thumb_process_displaced_16bit_insn(gdbarch, insn1, regs, dsc); Formatting: space before ( Thanks, Ulrich -- Dr. Ulrich Weigand GNU Toolchain for Linux on System z and Cell BE Ulrich.Weigand@de.ibm.com