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: [reverse] PATCH: Several interface changes


teawater wrote:
I think the idea of this patch is good.
But maybe process record still not need it now because p record still
not support multi-thread. Of course, p record will need it in the
feature.

Michael, how about your target?

No multi-thread support, yet.


Pedro, future enhancement?

On Wed, Oct 8, 2008 at 00:09, Pedro Alves <pedro@codesourcery.com> wrote:
On Monday 06 October 2008 23:11:14, Michael Snyder wrote:
Pedro Alves wrote:
A per-target property may seems to make sense on
single-threaded,single-inferior targets, but when you add support
for multi-inferiors per target (e.g., extended-remote has some of it now,
and I'm going to push more of it), or multi-threaded support, the
per-target setting may not make sense anymore --- explicit requests
at the target resume interface (just like your new packets) may make
more sense.  Imagine forward execution non-stop debugging in all threads
but one, which the user is examining in reverse.  What's the target
direction in this case?
Yakkk!!!
:-)  Here's an alternative interface I was considering and envisioning
when I mentioned the above.  Consider this just a suggestion.  If it
looks bad, let's quickly forget about it.

The question to me is --- when/why does the target (as in, the debug
API abstraction) ever need to know about the current direction that
it couldn't get from the core's request?
So, after last night's discussion, I came up with the attached to
see how it would look like.  The major change is that I consider the
reverse/forward-direction thing a property or the command the user
requested, and as such, belongs together with the other thread
stepping state we keep in struct thread_info, and the
target_ops implementation, adjusts itself to the direction GDB
requests with target_resume.  I've extended target_resume's interface
to accept this instead of a `step' boolean:

 enum target_resume_kind
  {
    /* Continue forward.  */
    rk_continue,

    /* Step forward.  */
    rk_step,

    /* Continue in the reverse direction.  */
    rk_continue_reverse,

    /* Step in the reverse direction.  */
    rk_step_reverse,
  };

(notice that the order of the things in the enum allows me to
miss some conversions --- I'm lazy).

 I can't say if I like this new target_resume interface or
not.  I just tried doing it to see how it would look.

(I can imagine that we're in the future going to extend the
target_resume interface to be more like gdbserver's, but, well,
that's another issue.)

So, the interface at the command level implementation is just
like it was before:

1) call clear_proceed_status ();

2) /* construct the step/continue request */

3) call proceed (...);

Where in #2, you can set the thread to go backwards by
doing:

thread->reverse = 1;

The attached patch applies against the reverse-20080930-branch.

Other things I've done in the patch:

  * Added support for a "Tnn nohistory" stop reply that translates
   to TARGET_WAITKIND_NO_HISTORY.  When going multi-threaded, or
   multi-process this will allow things like "T05;thread:pPID.TID;nohistory"
   for free.  I absolutelly didn't test this.  I've no reverse aware target
   at hand.

  * At places, error out if async + reverse or non-stop + reverse
    was requested.

  * Added a target_can_reverse_p method, so infcmd.c can check if the
    target supports reverse execution before calling into the target.  Not
    strictly necessary, though, but I thought this was nicer this way.

I checked that I can use the record target on x86 (actually x86_64
with -m32) as good as without the patch, but it's quite possible I
broke things badly.

--
Pedro Alves



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