From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23016 invoked by alias); 8 Oct 2008 07:28:45 -0000 Received: (qmail 23007 invoked by uid 22791); 8 Oct 2008 07:28:44 -0000 X-Spam-Check-By: sourceware.org Received: from ti-out-0910.google.com (HELO ti-out-0910.google.com) (209.85.142.191) by sourceware.org (qpsmtpd/0.31) with ESMTP; Wed, 08 Oct 2008 07:28:09 +0000 Received: by ti-out-0910.google.com with SMTP id d10so2464237tib.12 for ; Wed, 08 Oct 2008 00:28:06 -0700 (PDT) Received: by 10.110.109.12 with SMTP id h12mr8987759tic.52.1223450885892; Wed, 08 Oct 2008 00:28:05 -0700 (PDT) Received: by 10.110.42.9 with HTTP; Wed, 8 Oct 2008 00:28:05 -0700 (PDT) Message-ID: Date: Wed, 08 Oct 2008 07:28:00 -0000 From: teawater To: "Pedro Alves" Subject: Re: [reverse] PATCH: Several interface changes Cc: gdb-patches@sourceware.org, "Michael Snyder" In-Reply-To: <200810071709.48346.pedro@codesourcery.com> MIME-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Content-Disposition: inline References: <200810071709.48346.pedro@codesourcery.com> X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2008-10/txt/msg00245.txt.bz2 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 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 >