From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24603 invoked by alias); 13 Jan 2011 15:16:00 -0000 Received: (qmail 24593 invoked by uid 22791); 13 Jan 2011 15:15:58 -0000 X-SWARE-Spam-Status: No, hits=-2.0 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW X-Spam-Check-By: sourceware.org Received: from service87.mimecast.com (HELO service87.mimecast.com) (94.185.240.25) by sourceware.org (qpsmtpd/0.43rc1) with SMTP; Thu, 13 Jan 2011 15:15:52 +0000 Received: from cam-owa2.Emea.Arm.com (fw-tnat.cambridge.arm.com [217.140.96.21]) by service87.mimecast.com; Thu, 13 Jan 2011 15:15:48 +0000 Received: from [10.1.77.49] ([10.1.255.212]) by cam-owa2.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.3959); Thu, 13 Jan 2011 15:15:47 +0000 Subject: Re: [try 3rd] arm_pc_is_thumb takes displaced stepping into account From: Matthew Gretton-Dann To: Yao Qi Cc: gdb-patches@sourceware.org In-Reply-To: <4D2D27F2.7050303@codesourcery.com> References: <4D0F0ABA.9010506@codesourcery.com> <201012200804.oBK84oPu005379@glazunov.sibelius.xs4all.nl> <4D0F5D36.2040909@codesourcery.com> <4D10D377.8080100@codesourcery.com> <20101223042236.GS2596@adacore.com> <4D12F3A8.3020102@codesourcery.com> <4D2D27F2.7050303@codesourcery.com> Date: Thu, 13 Jan 2011 15:55:00 -0000 Message-ID: <1294931747.9907.137.camel@e102319-lin.cambridge.arm.com> Mime-Version: 1.0 X-MC-Unique: 111011315154806401 Content-Type: text/plain; charset=WINDOWS-1252 Content-Transfer-Encoding: quoted-printable 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-01/txt/msg00296.txt.bz2 On Tue, 2011-01-11 at 22:02 -0600, Yao Qi wrote: > On 12/23/2010 01:00 AM, Yao Qi wrote: > > OK. I have to try the second approach, which is 1) exposing displaced > > stepping state to tdep, and 2) take displaced stepping state into > > account when determining the mode. > > >=20 > After talked with Ulrich, I realize that it is *not* a good idea to=20 > expose displaced stepping outside of infrun, and my patch is a little=20 > bit too intrusive. >=20 > > 2010-12-23 Yao Qi > > > > * arm-tdep.c: (arm_pc_is_thumb): Adjust MEMADDR if it is within > > copy area of displaced stepping. > > * infrun.c (struct displaced_step_inferior_state): Move to ... > > Expose get_displaced_stepping_state. > > * inferior.h: ... here. > > Declare get_displaced_stepping_state. >=20 > This time, instead of exposing displaced_step_inferior_state to tdep, we= =20 > return displaced_step_closure, which is defined by each tdep, instance=20 > to tdep appropriately. >=20 > OK to mainline? I agree with Ulrich, and prefer this latest patch to your previous ones - but I am still not 100% happy with it, as I think it would be better not to need anything from infrun.c at all here. What could be done instead is to have the displaced stepping routines maintain a list of the areas of memory that are being used as scratch space, and the instruction set state of the instructions in those areas. Then arm_pc_is_thumb should check this list to see if the PC falls into one of these areas, and return the appropriate value. This keeps these changes wholly within the ARM backend for GDB, and doesn't require changes to GDB globally. However, this will impact performance and memory more than your patch - so I am not completely opposed to your patch. This is yet another instance of a problem Ulrich and I discussed last year, which is: How do you tell what instruction-set state you are in when you just have a CORE_ADDR, which is insufficient information on ARM. See the thread starting at: http://sourceware.org/ml/gdb-patches/2010-08/msg00271.html The long-term solution is probably either to implement multiple address spaces for code, or to treat ARM & Thumb as different gdbarch's and use multiarch support. However, this is a much more difficult problem to solve in general and shouldn't stop this patch going in. [Note again that I am not a maintainer and so do not have approval rights]. Thanks, Matt --=20 Matthew Gretton-Dann Principal Engineer - PDSW Tools ARM Ltd