This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
fix "[Switching to FOO_thread]" in async mode.
- From: Pedro Alves <pedro at codesourcery dot com>
- To: gdb-patches at sourceware dot org
- Date: Fri, 20 May 2011 20:10:12 +0100
- Subject: fix "[Switching to FOO_thread]" in async mode.
Another sync vs async difference.
In async mode, the
[Switching to Thread FOOBAR]
notice is broken, because we reset
previous_inferior_ptid whenever we have
a new event to handle... So, e.g., if you have two
threads, t1, t2, and you have thread t1 selected
when you "next":
- "next", t1 is current.
- t2 reports an event, previous_inferior_ptid is reset
to t1 (happens to be the current thread before handling
the event). handle_inferior_event switches current thread
to t2. The event doesn't cause a user visible stop, so
it is kept going.
- t2 reports another event, previous_inferior_ptid is reset
to t2 (happens to be the current thread before handling
the event). handle_inferior_event switches current thread
to t2 (same as before). The event causes a user visible
stop, but when we get to normal_stop, previous_inferior_ptid
is the same as inferior_ptid, so the "switching to foo..."
message isn't output.
In sync mode, the "previously selected thread"
(previous_inferior_ptid) is reset usually just once per command run,
at the top of wait_for_inferior, before going into the event
handling loop, but, in async mode, there's no loop (other than
the main event loop, that is).
To fix this, I've moved the previous_inferior_ptid
sets conceptually a layer up, into `proceed'.
This is actually more correct. For example, in the case that
proceed happens to decide to switch to another thread before
resuming (prepare_to_proceed), when we get to wait_for_inferior,
inferior_ptid would already not be the previous user
selected thread. This lead to this bit in
handle_inferior_event's handling of deferred_step_ptid:
/* Suppress spurious "Switching to ..." message. */
previous_inferior_ptid = inferior_ptid;
to work around it. But that bit only runs if prepare_to_proceed
switched threads _and_ proceed was requested to step, _and_
the step-off finished sucessfully (wasn't interrupted by a
signal, for example, see my yesterday's patch).
Setting previous_inferior_ptid in wait_for_inferior
also handled the cases of "attach", and "target remote"
(and similars; start_remote). We can easily handle those
by setting previous_inferior_ptid from inferior_ptid
in init_wait_for_inferior.
Actually, to be even more correct, we'd probably set
previous_inferior_ptid at even a higher conceptual
layer, at the command level. For example, breakpoint commands
may switch the current thread before giving the prompt
to the user, which happens _after_ normal_stop (the
place that print the "switching to ..." message. Or
even infcalls can appear in breakpoint commands or user
commands, and those call proceed as well, resetting
previous_inferior_ptid (same thing without my patch,
as proceed calls wait_for_inferior). Anyway, I'm
more concerned with making async look like sync, so
I'm leaving that more complicated change for a rainy day.
Tested on x86_64-linux, sync and async, and checked in.
Pedro Alves
2011-05-20 Pedro Alves <pedro@codesourcery.com>
gdb/
* infrun.c (proceed): Set previous_inferior_ptid here.
(init_wait_for_inferior): Initialize previous_inferior_ptid from
inferior_ptid, not null_ptid.
(wait_for_inferior): Don't initialize previous_inferior_ptid here.
(fetch_inferior_event): Nor here.
---
gdb/infrun.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
Index: src/gdb/infrun.c
===================================================================
--- src.orig/gdb/infrun.c 2011-05-20 17:41:47.568819003 +0100
+++ src/gdb/infrun.c 2011-05-20 19:27:23.028819003 +0100
@@ -2071,6 +2071,9 @@ proceed (CORE_ADDR addr, enum target_sig
return;
}
+ /* We'll update this if & when we switch to a new thread. */
+ previous_inferior_ptid = inferior_ptid;
+
regcache = get_current_regcache ();
gdbarch = get_regcache_arch (regcache);
aspace = get_regcache_aspace (regcache);
@@ -2290,7 +2293,7 @@ init_wait_for_inferior (void)
target_last_wait_ptid = minus_one_ptid;
- previous_inferior_ptid = null_ptid;
+ previous_inferior_ptid = inferior_ptid;
init_infwait_state ();
/* Discard any skipped inlined frames. */
@@ -2654,9 +2657,6 @@ wait_for_inferior (void)
ecs = &ecss;
memset (ecs, 0, sizeof (*ecs));
- /* We'll update this if & when we switch to a new thread. */
- previous_inferior_ptid = inferior_ptid;
-
while (1)
{
struct cleanup *old_chain;
@@ -2720,9 +2720,6 @@ fetch_inferior_event (void *client_data)
memset (ecs, 0, sizeof (*ecs));
- /* We'll update this if & when we switch to a new thread. */
- previous_inferior_ptid = inferior_ptid;
-
/* We're handling a live event, so make sure we're doing live
debugging. If we're looking at traceframes while the target is
running, we're going to need to get back to that mode after