From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11657 invoked by alias); 20 Apr 2006 19:50:53 -0000 Received: (qmail 11649 invoked by uid 22791); 20 Apr 2006 19:50:53 -0000 X-Spam-Check-By: sourceware.org Received: from nevyn.them.org (HELO nevyn.them.org) (66.93.172.17) by sourceware.org (qpsmtpd/0.31.1) with ESMTP; Thu, 20 Apr 2006 19:50:50 +0000 Received: from drow by nevyn.them.org with local (Exim 4.54) id 1FWfAt-00060i-Az; Thu, 20 Apr 2006 15:50:47 -0400 Date: Thu, 20 Apr 2006 19:50:00 -0000 From: Daniel Jacobowitz To: Michael Snyder Cc: Eli Zaretskii , gdb-patches@sources.redhat.com Subject: Re: [RFA] Reverse debugging, part 1/3: target interface Message-ID: <20060420195047.GA22563@nevyn.them.org> Mail-Followup-To: Michael Snyder , Eli Zaretskii , gdb-patches@sources.redhat.com References: <442DAA70.5070203@redhat.com> <444426C7.6020604@redhat.com> <20060418125836.GB10130@nevyn.them.org> <20060418152443.GA13825@nevyn.them.org> <44456356.8090706@redhat.com> <444680C3.1010007@redhat.com> <20060420134343.GC11710@nevyn.them.org> <4447DF7D.6070506@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4447DF7D.6070506@redhat.com> User-Agent: Mutt/1.5.8i X-IsSubscribed: yes 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/msg00295.txt.bz2 On Thu, Apr 20, 2006 at 12:22:37PM -0700, Michael Snyder wrote: > >This seems to be one of the stray FIXMEs in this code which really > > ought to be fixed before we merge it: > > > >+ /* FIXME: check target for capability. */ > > > >I'm guessing you haven't tried this with a remote target that didn't > >support it. It looks like it would fail messily. > > Hmmmmm... ok, your guess is correct. > > I propose a deal -- I'll try it, and if it does fail messily, > I will fix it. If it doesn't, will you let me check it in > with the expectation of future enhancement? I feel that for > a major new functionality, useability rather than completeness > should be the mark. > > It's not like we're not going to add to what's been > implemented so far. I'm happy to be flexible about what gets committed, for new features; but this is a protocol issue. It'll change what other projects have to implement to take advantage of this new functionality. I'm less happy being flexible about that, having had to handle backwards compatibility for beta protocol proposals a time or two :-) Let's see what it does. Not hard at all; patched, built, and a handy gdbserver which doesn't support reversing... /space/fsf/commit/src/gdb/infcmd.c:1392: warning: 'breakpoint' may be used uninitialized in this function It's right, too. The logic in finish_backward is broken. With that fixed I can try it: (gdb) tar rem :1234 Remote debugging using :1234 0x00002aaaaaaaba80 in ?? () (gdb) rsi 0x00002aaaaaaaba80 in ?? () (gdb) 0x00002aaaaaaaba83 in ?? () OK, that's two bugs. One is the vCont bug I mentioned in another message; it's stepping forward instead of backwards. The other has nothing to do with this at all; apparently PTRACE_STEP isn't working for the first instruction in the program and we're taking two steps to go one instruction. OK, let's avoid the vCont bug and try again: (gdb) set remote verbose-resume-packet 0 (gdb) rsi Sending packet: $m2aaaaaab7320,1#70...Ack Packet received: f3 Sending packet: $M2aaaaaab7320,1:cc#50...Ack Packet received: OK Sending packet: $Hc0#db...Ack Packet received: E01 Sending packet: $bs#d5...Ack Packet received: warning: Invalid remote reply: *hang* It thinks that the attempt to step has succeeded and that the program is now running. That's pretty messy failing. > >Are there other hard cases that should be discussed in the remote > >protocol documentation, or handled by the protocol? The first one that > >comes to mind is "what do we do with threads" - is it ever going to be > >relevant to do this in a multi-threaded system, and if so, are we going > >to reverse all threads at once? > > I think in previous discussion, we have recognized that this is a > hard problem, and I have simply deferred addressing it. I don't > even claim to know if it *can* be addressed. I'd like to say, > we can now use this feature on the non-trivial subset of non- > threaded programs, and put threaded programs down as a hoped- > for future enhancement. Sure, but, a certain amount of future proofing is still in order. Let's at least say that their behavior for threaded programs is not yet defined? If we want something more complex, maybe vCont can help later. > >Oh, and refering to Stop Reply Packets is not complete for the reply; > >that doesn't cover error codes. For instance, you have one error code > >designated to mean "no more history". That should be in the > >documentation, please. > > Well, yes, I thought about that -- but in fact, AFAICT there is > *no* documentation for error codes, and I didn't feel like starting > it. Yes, I think we need some, but it's probably worth having a > discussion of how we would like to do it. > > And, self-interest not withstanding, I don't think there's a good > reason for holding up this patch for lack of something that is > lacking throughout. You misunderstand me. Right now, GDB doesn't generally assign meaning to the error numbers. They just mean "error" and they're all treated the same. This isn't like that. You've set things up so that the target is supposed to return "E06" when it runs out of history. That's protocol; it should be in the protocol documentation! Other errors, in general, sure. They're undocumented. We'll have to contend with that at a later date. Is rc/rs allowed to return other errors (whatever they may mean)? If so, the packet description ought to say "Enn: Another error has occured". > > I've only skimmed the patch; are there others? > > None that I'm deliberately withholding. ;-) Other error numbers, I meant. -- Daniel Jacobowitz CodeSourcery