From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 14343 invoked by alias); 17 Apr 2006 13:09:46 -0000 Received: (qmail 14272 invoked by uid 22791); 17 Apr 2006 13:09:41 -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; Mon, 17 Apr 2006 13:09:37 +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 k3HD8rvn020379; Mon, 17 Apr 2006 15:08:53 +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 k3HD8qxl028802; Mon, 17 Apr 2006 15:08:52 +0200 (CEST) Received: (from kettenis@localhost) by elgar.sibelius.xs4all.nl (8.13.6/8.13.6/Submit) id k3HD8qdY014104; Mon, 17 Apr 2006 15:08:52 +0200 (CEST) Date: Mon, 17 Apr 2006 13:09:00 -0000 Message-Id: <200604171308.k3HD8qdY014104@elgar.sibelius.xs4all.nl> From: Mark Kettenis To: drow@false.org CC: msnyder@redhat.com, gdb-patches@sourceware.org In-reply-to: <20060417014133.GA4169@nevyn.them.org> (message from Daniel Jacobowitz on Sun, 16 Apr 2006 21:41:33 -0400) 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> <200604162349.k3GNnNhl006336@elgar.sibelius.xs4all.nl> <20060417014133.GA4169@nevyn.them.org> 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/msg00215.txt.bz2 > Date: Sun, 16 Apr 2006 21:41:33 -0400 > From: Daniel Jacobowitz > > On Mon, Apr 17, 2006 at 01:49:23AM +0200, Mark Kettenis wrote: > > 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. > > Have you read any of my arguments as to why the abstraction is not > needless? I haven't seen a response to any of them, just a dismissal. > I think I gave some useful examples; I'm a little frustrated that you > keep ignoring them. Sorry. Came home to a flurry of messages about the subject and semi-randomly chose to reply to some of them. Citing from your earlier message: > I do not think the abstraction is at all unnecessary, or I wouldn't > have added it. I presented three reasons why it could be useful: > one practical concern with the address modifications performed by > the existing BREAKPOINT_FROM_PC interface, one potential use case > for hardware breakpoints, and the considerable simplification of > future changes to these specific functions. I don't think the hardware breakpoints should be a concern here. But the BREAKPOINT_FROM_PC issue is indeed pretty much a showstopper. > The interface is not so well defined as your paragraph suggests. In > fact, the target MAY save the contents, not DOES. I'm pretty sure that > some don't. I let this mislead me into thinking that saving the > contents was optional. In fact it isn't. deprecated_read_memory_nobpt > uses the shadow contents, and uses BREAKPOINT_FROM_PC. Whatever we > decide on, I'll make sure that gets fixed. But this means that actually we should try to make the interface stricter, instead of looser. And the fact that the shadow contents are needed makes it impossible to use an opaque struct. That in combination with the possibility that BREAKPOINT_FROM_PC adjusts the breakpoint address means that besides the length we also need to pass back the address. At that point indeed passing a struct is perhaps a better option. And we should remove the usage of BREAKPOINT_FROM_PC from deprecated_read_memory_nobpt() altogether. I still think we should maintain a strict seperation between the high-level breakpoint code and the low-level target code. So I'd really appreciate it if you'd be willing to change your patch such that instead of struct bp_location, you used a different struct, which for now would have three members: the saved contents, the length and the address. It's perfectly ok with me to make that new struct part of struct bp_location to avoid all memory allocation problems. Mark