From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11420 invoked by alias); 16 Apr 2006 23:50:13 -0000 Received: (qmail 11412 invoked by uid 22791); 16 Apr 2006 23:50:13 -0000 X-Spam-Check-By: sourceware.org Received: from sibelius.xs4all.nl (HELO sibelius.xs4all.nl) (82.92.89.47) by sourceware.org (qpsmtpd/0.31) with ESMTP; Sun, 16 Apr 2006 23:50:11 +0000 Received: from elgar.sibelius.xs4all.nl (root@elgar.sibelius.xs4all.nl [192.168.0.2]) by sibelius.xs4all.nl (8.13.4/8.13.4) with ESMTP id k3GNnOjq029349; Mon, 17 Apr 2006 01:49:24 +0200 (CEST) Received: from elgar.sibelius.xs4all.nl (kettenis@localhost.sibelius.xs4all.nl [127.0.0.1]) by elgar.sibelius.xs4all.nl (8.13.6/8.13.6) with ESMTP id k3GNnN79017401; Mon, 17 Apr 2006 01:49:23 +0200 (CEST) Received: (from kettenis@localhost) by elgar.sibelius.xs4all.nl (8.13.6/8.13.6/Submit) id k3GNnNhl006336; Mon, 17 Apr 2006 01:49:23 +0200 (CEST) Date: Sun, 16 Apr 2006 23:50:00 -0000 Message-Id: <200604162349.k3GNnNhl006336@elgar.sibelius.xs4all.nl> From: Mark Kettenis To: msnyder@redhat.com CC: drow@false.org, mark.kettenis@xs4all.nl, gdb-patches@sourceware.org In-reply-to: <443EC947.9060109@redhat.com> (message from Michael Snyder on Thu, 13 Apr 2006 14:57:27 -0700) Subject: Re: Save the length of inserted breakpoints References: <20060302221711.GB18830@nevyn.them.org> <200603022301.k22N1qEt008208@elgar.sibelius.xs4all.nl> <20060411214613.GA702@nevyn.them.org> <200604120943.k3C9hYJ8012016@elgar.sibelius.xs4all.nl> <20060412125712.GA22145@nevyn.them.org> <200604121837.k3CIbMwu004466@elgar.sibelius.xs4all.nl> <20060412184717.GA29980@nevyn.them.org> <443EC947.9060109@redhat.com> Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2006-04/txt/msg00210.txt.bz2 > Date: Thu, 13 Apr 2006 14:57:27 -0700 > From: Michael Snyder > > Daniel Jacobowitz wrote: > > On Wed, Apr 12, 2006 at 08:37:22PM +0200, Mark Kettenis wrote: > > > >>>Would a new "struct bp_target_info", defined and allocated centrally > >>>for convenience, allay this concern? [Conveniently I can do the bulk > >>>of the changes for that with sed :-)] > >> > >>Why hide things away if all you're going to need is a buffer and a > >>length? I've tried really hard to see why one would need more than > >>that, but failed completely. > >> > >>So I think we should have: > >> > >>int target_insert_breakpoint(CORE_ADDR addr, gdb_byte *buf, int *size); > >>int target_remove_breakpoint(CORE_ADDR addr, gdb_byte *buf, int size); > > > > > > And then if you come up with a reason, you're going to need to hand > > edit every one of these targets again. It's not a bundle of fun. Is > > that really necessary? > > > > You need an address because the address at which the breakpoint is > > inserted may not match the requested address. This happens in several > > different places in the breakpoint infrastructure (I believe I counted > > three disjoint hooks for it), but I am particularly looking at > > BREAKPOINT_FROM_PC, which takes the PC by reference. In the ARM case, > > given 0x4001, it strips the low bit off and returns a two byte > > breakpoint. If we don't allow the target to save the > > actually-inserted-at address, then it has to call BREAKPOINT_FROM_PC > > again. It feels much more robust to me to save this address when we > > initially adjust it. Here's where we inserted the breakpoint, so > > that's where we should remove it from. > > > > I can think of plenty of other places where another constant might > > be useful. You might want to record which hardware breakpoint > > registers were used, for instance, instead of digging around > > to figure out which ones to clear. Adding a new member to > > "struct bp_target" for that would be easy. > > I haven't followed this discussion closely, so forgive me > if I'm recapitulating something that's already been said. > > What about something like "void *target_data" in the breakpoint struct? > The target can add whatever it likes, and the core breakpoint code > doesn't need to know what it is. If it's non-null when the bp is > freed, then the target should be given an opportunity to delete it. And I presume only TARGET_DATA would be passed down to the target. That's certainly better than passing down "struct bp_location" to the target, exposing the internals of the high-level breakpoint code to the low-level target code. However, my point is that we're creating needless abstraction layers here, and opening the way for potentially big changes in the target interface. We currently have a well defined interface that inserts a software breakpoint at a specific address, and saves the contents into a buffer provided by the high-level code. That interface has a problem in the sense that it doesn't record actually how many valid bytes are in that buffer. This simplest fix is simply adding that missing length to the interface. Mark