From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 30419 invoked by alias); 10 Jun 2009 14:58:42 -0000 Received: (qmail 30410 invoked by uid 22791); 10 Jun 2009 14:58:41 -0000 X-SWARE-Spam-Status: No, hits=-2.1 required=5.0 tests=AWL,BAYES_00,J_CHICKENPOX_45,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 10 Jun 2009 14:58:35 +0000 Received: (qmail 9281 invoked from network); 10 Jun 2009 14:58:33 -0000 Received: from unknown (HELO orlando.local) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 10 Jun 2009 14:58:33 -0000 From: Pedro Alves To: Daniel Jacobowitz Subject: Re: [PATCH] Displaced stepping (non-stop debugging) support for ARM Linux Date: Wed, 10 Jun 2009 14:58:00 -0000 User-Agent: KMail/1.9.10 Cc: Julian Brown , gdb-patches@sourceware.org References: <20090120221355.46ac23e6@rex.config> <20090516191910.14820805@rex.config> <20090609173709.GA10846@caradoc.them.org> In-Reply-To: <20090609173709.GA10846@caradoc.them.org> MIME-Version: 1.0 Content-Disposition: inline Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Message-Id: <200906101559.46860.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: 2009-06/txt/msg00253.txt.bz2 On Tuesday 09 June 2009 18:37:09, Daniel Jacobowitz wrote: > On Sat, May 16, 2009 at 07:19:10PM +0100, Julian Brown wrote: > > I'm not sure what the status is here now. For testing purposes, I've > > (still) been using a local patch which uses displaced stepping for all > > single-step operations. > We still can't use software single-stepping simultaneously in multiple > threads. Pedro, should we fix that or always use displaced stepping > for now? It would be nice to have that fixed, for sure, so yes to the we should fix that question. However, it seems to me that this is something that can be worked on mostly independently of the ARM bits as it's a general software single-step issue, not really ARM specific. Unless someone wants to (and has time to) tackle it right now, I'd say go with the always displace-step version. If nothing else, helps in stressing the displaced stepping implementation. :-) > > > Daniel Jacobowitz wrote: > > > > > * What's the point of executing mov on the target for BL? > > > At that point it seems like we ought to skip the target step entirely; > > > just simulate the instruction. We've already got a function to check > > > conditions (condition_true). > > > > I'm now using NOP instructions and condition_true, because the current > > displaced stepping support wants to execute "something" rather than > > nothing. > > From infrun.c: > > One way to work around [software single stepping]... > would be to have gdbarch_displaced_step_copy_insn fully > simulate the effect of PC-relative instructions (and return NULL) > on architectures that use software single-stepping. > > So the interface you need is there; it's just not implemented yet: > > /* We don't support the fully-simulated case at present. */ > gdb_assert (closure); > > I think the implementation strategy will look like: > > * Add another non-zero return value from displaced_step_prepare. The thread should still be marked as running in the frontend/cli's perspective, as it would be stuck doing internal things, so that the user can't try to "continue" it again ... > > * Update should_resume after the call, in resume (currently unused). ... so this bit in resume applies: { if (!displaced_step_prepare (inferior_ptid)) { /* Got placed in displaced stepping queue. Will be resumed later when all the currently queued displaced stepping requests finish. The thread is not executing at this point, and the call to set_executing will be made later. But we need to call set_running here, since from frontend point of view, the thread is running. */ set_running (inferior_ptid, 1); discard_cleanups (old_cleanups); return; } } You need that, since "set_running" is usually called from target_resume, which you'd bypass in the fully similated case. > > * Ask Pedro how to pretend that the inferior resumed and stopped, > for higher levels. See below. > I think this will entail a new queue. Indeed. Every time we required something similar, we got away with doing that shortcut inside target code, keeping infrun agnostic of the trick. But this is gdbarch code, independent of which target_ops is playing (e.g., could be native linux-nat.c, could be remote.c). > Bonus > points if prepare_to_wait and wait_for_inferior do not invalidate > the perfectly good register cache at this point. > > Pedro, thoughts - easy or should we stick with the NOP workaround for > now? In sync mode, I can picture bypassing the target_wait call in wait_for_inferior, by peeking a local event queue first. In async (and non-stop) modes, that would happen in fetch_inferior_event, which is an asynchronous event loop callback. This requires registering a new event source from within infrun.c, and, marking it whenever we have events to handle (that is, when the queue isn't empty). In principle, it shouldn't be hard. Keep a local queue of events in infrun.c (as first approximation, each event consisting of a ptid and a target_waitstatus), and add an async_event_handler to infrun.c (look around in remote.c for event queue and remote_async_inferior_event_token for copy&paste sources). Then, in displaced_step_prepare, if you just detected a fully simulated case, push a new stop event (TARGET_WAITKIND_STOP/SIGTRAP) in this queue, and mark the event source as having something to process. The event source callback would be something like: static void infrun_async_inferior_event_handler (gdb_client_data data) { inferior_event_handler (INF_REG_EVENT, NULL); } Then, just do nothing else, returning back to the event loop, which will eventually call inferior_event_handler->fetch_inferior_event, and that would collect the event from the local event queue, and pass it to handle_inferior_event as usual. The event queue would expose a similar interface to target_wait, so that wait_for_inferior can request a queued event from a specific ptid (and things look neat). Care must be taken to keep the event queue in sync with target reality --- e.g., if the thread that was doing the short-circuit (and so has a pending event in a new infrun.c local event queue) exits (because e.g., we lost connection to the target in between, or the whole process exits due to another thread doing an _exit call or something like that), then we need to remove the event from the queue, otherwise, when you go to process it, things break while referencing a thread that doesn't exist anymore. Should be mostly a matter of taking care of the event queue from within infrun.c:infrun_thread_thread_exit. Another source of care is if there's code out there that tries to resume all threads and the target happens to try to resume such a thread behind infrun event queue's back. This shouldn't be a problem in non-stop mode, though, so, probably not something to worry much about much for now. -- Pedro Alves