From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 18728 invoked by alias); 8 Oct 2008 20:21:49 -0000 Received: (qmail 18713 invoked by uid 22791); 8 Oct 2008 20:21:48 -0000 X-Spam-Check-By: sourceware.org Received: from smtp-outbound-2.vmware.com (HELO smtp-outbound-2.vmware.com) (65.115.85.73) by sourceware.org (qpsmtpd/0.31) with ESMTP; Wed, 08 Oct 2008 20:21:13 +0000 Received: from mailhost3.vmware.com (mailhost3.vmware.com [10.16.27.45]) by smtp-outbound-2.vmware.com (Postfix) with ESMTP id CEE9F20004; Wed, 8 Oct 2008 13:21:10 -0700 (PDT) Received: from [10.20.92.59] (promb-2s-dhcp59.eng.vmware.com [10.20.92.59]) by mailhost3.vmware.com (Postfix) with ESMTP id BAA20C9A56; Wed, 8 Oct 2008 13:21:10 -0700 (PDT) Message-ID: <48ED15B4.9040401@vmware.com> Date: Wed, 08 Oct 2008 20:21:00 -0000 From: Michael Snyder User-Agent: Thunderbird 1.5.0.12 (X11/20080411) MIME-Version: 1.0 To: teawater CC: Pedro Alves , "gdb-patches@sourceware.org" Subject: Re: [reverse] PATCH: Several interface changes References: <200810071709.48346.pedro@codesourcery.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit 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/msg00255.txt.bz2 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 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 >>