This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [reverse] PATCH: Several interface changes
- From: teawater <teawater at gmail dot com>
- To: "Pedro Alves" <pedro at codesourcery dot com>
- Cc: gdb-patches at sourceware dot org, "Michael Snyder" <msnyder at vmware dot com>
- Date: Wed, 8 Oct 2008 15:28:05 +0800
- Subject: Re: [reverse] PATCH: Several interface changes
- References: <200810071709.48346.pedro@codesourcery.com>
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?
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
>