This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFA] Reverse debugging, part 1/3: target interface


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]