This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [non-stop] 10/10 split user/internal threads
A Thursday 26 June 2008 14:32:51, Daniel Jacobowitz wrote:
> On Sun, Jun 15, 2008 at 10:09:44PM +0100, Pedro Alves wrote:
> > I've added three points where GDB makes the internal (inferior_ptid)
> > thread switch to the user selected thread (user_selected_ptid):
> >
> > ?1 - execute_command, before executing a command
> > ?2 - execute_command, after executing a command,
> > ?3 - fetch_inferior_event, after handling the event.
> >
> > With these in place, the user never notices when GDB switches
> > between threads internally while handing events.
>
> The _internal name is a warning sign to me: it means we have two
> functions that conceptually do the same thing, but one of them is only
> "safe for internal use". We call both of them from internal to GDB,
> so let's try to find a better name. How about
> execute_command_in_current_context?
>
Ack. internal refered to "internal selected thread" vs "user/external
selected thread". Please keep reading, though.
> Is -interpreter-exec ever going to get a --thread argument? We have
> to be careful because it currently restores the globally selected
> thread via cli_interpreter_exec, and even if it didn't it may call
> a user defined command.
>
> There are 22 calls to execute_command in GDB (including two from
> Insight). This patch changes seven of them to
> execute_command_internal: thread.c, mi-main.c, mi-cmd-env.c.
> I checked all the others, just in case - they mostly look correct.
>
> The only cases I'm worried about are breakpoint commands and
> conditions. For instance bpstat_do_actions to execute_control_command
> to execute_command will mess with inferior_ptid. Should it?
Ack. Keep reading.
> What about bpstat_check_breakpoint_conditions to breakpoint_cond_eval
> to evaluate_expression?
>
> Also, hook- and hookpost- commands will still be run after
> execute_command_internal - but every call to execute_command_internal
> manually sets the appropriate thread, so this looks fine.
>
Yeah, this can get twisty. I've been computing these issue
in the background for a while, as these issues have been
worrying me. I'd been through several completelly different versions
of this patch, as it's tricky to get all issues covered correctly. I'd
prefer that the next revision goes in the right direction. :-)
Let me start with a bit of some goals and problem description,
then work my way into a proposal, and then there's a patch
attached that applies on top of this one you've reviewed,
so the incremental changes can be seen better.
Here are some thoughts I keep bumping myself into:
- Ideally, at least the target interfaces and register frame
and stack management components of GDB shouldn't
know about a "current thread" (inferior_ptid) at all.
I count over 750 references to it, so that is very far
from being true.
- In the long run, it should be possible to support more
than one context. Say, the IDE can display two or
even more frame info windows, each with its own
thread selected. Something similar to what described
here (3.4.1):
http://www.sourceware.org/gdb/papers/multi-arch/real-multi-arch/
... should be supportable.
MI's --thread and --frame new switches should enable
implementing that on the frontend side.
- MI is gaining "stateless" --thread and --frame commands.
Commands that take those arguments should not change the
currently user selected thread and frame.
- Non-stop shouldn't change the current user selected thread
and frame. At least if the current thread is stopped.
- In non-stop mode, the current thread may exit while the
user has it selected. Switching current thread, and restoring
it with a cleanup is problematic in non-stop mode.
1) The original thread may exit before we re-select it
- current thread 1 is running
- switch to thread 2
- <thread 1 exits>
- trying to restore thread 1 is messy
* ideally, thread 1 is no longer in the thread list at this point.
The main reason the current patch could keep exited threads listed
was due to context switching needing inferior_ptid to
be listed...)
* restoring by ptid is ambiguous is the app/OS quickly reuses
thread 1's ptid (remember it had exited).
2) The selected thread may change between creating the cleanup, and
running it. E.g.,
To handle an event, we need to switch to the thread that got it
--- Frame parsing/handling only being aplicable to inferior_ptid,
and context switching are the main reasons behind this, off the top
of my head. There's a bunch of cleanup work and core rework to do
to be able to remove this requirement.
Picture these examples:
- User has thread 1 selected
- thread 2 has an event
- GDB switches to thread 2 to handle it
- It was an internal breakpoint, thread 2 is immediatelly resumed,
so the user should not notice the thread switch.
- GDB has finished handling the event, and reverts
to thread 2 (and also the frame the user had selected in it),
(the user didn't notice the thread switching, unless the original
current thread or frame were destroyed by the just handled event.)
In this case, restoring with a cleanup might seem appropriate.
Now this example:
- User has thread 1 selected.
- thread 2 has an event
- GDB switches to thread 2 to handle it
- It was a breakpoint, and has breakpoint commands attached to it.
- The breakpoint has several commands attached actually:
thread 3
print value1
up
print value2
up
print value3
end
- GDB has finished handling the event, so should restore to the
user selected thread and frame. But, which is it now?
thread 1, or thread 3? In all-stop mode? In non-stop mode?
After posting this patch I realized that the _internal variant
indeed isn't good enough in situations like:
(gdb) define mycomm
>up
>print a
>end
(gdb) thread 1
(gdb) thread apply 2 mycomm
mycomm is executed with execute_command_internal (on thread 2),
but "up" and "print a" are ran with execute_command
(hence on thread 1)...
In non stop mode, we don't want stop events to switch
the current thread, but, breakpoint commands should apply to
the context of the thread that just hit the breakpoint, e.g.:
(gdb) b foo.c:100
Breakpoint 1 ...
(gdb) commands
>thread
>print func_arg
>c&
>end
(gdb) thread 1
(gdb) c -a&
(thread 2 hits breakpoint 1)
(GDB switches to it to handle the event)
(GDB calls bpstat_do_actions, which apply in
the context of thread 1) Boo! Breakpoint command
became useless...
Now another example. Imagine in non-stop mode, this case:
(gdb) b foo.c:100
Breakpoint 1 ...
(gdb) commands
>thread 3
>c&
>end
(gdb) interrupt -a
(all threads stopped)
(gdb) thread 2
(gdb) c&
(thread 2 resumes)
(gdb) thread 1
(gdb) print foo_c
(gdb) print foo_b
(gdb) pri
(thread 2 hits breakpoint 1, and runs its commands,
which included switching to thread 3)
nt foo_a</enter>
(note that the breakpoint was hit while the user was
inspecting thread 1, typing a print command)
To which thread should the last "print foo_a" apply to?
My opinion is that is should apply to thread 1. Does
anyone disagree?
We still need something else to make GDB behave
this way.
---------------------------------------------------------------
With all the above in mind, I thought of:
- calling the { selected thread and frame } thing a "context".
- having a context stack
(The top level context being what the user/frontend
considers current.)
Instead of temporarilly switching to another thread and frame;
execute commands in it, and restoring the selected thread and
frame, we do:
- push a new context on the context stack
- switch the selected thread and frame in this
new now top level context
- execute command(s)
which can switch thread and frame at will
- pop context
- Whenever a thread exit is detected, we go through all
contexts and if it was the selected thread in it, invalidate
it.
Note that execute_command_internal is then gone. No need
for it anymore.
E.g. 1 - thread apply, pushing context to execute commands:
(top level context selected)
(gdb) thread 1
(gdb) thread apply 2 thread 3
(push new context)
(selected thread 2)
(execute "thread 3")
(pop context)
(gdb) thread
[Current thread is 1 ...]
From the user point of view, this is the current HEAD's behaviour
actually.
E.g. 2 - thread apply frame restoring broke with execute_command_internal,
if command applied to the already selected thread, with contexts, it doesn't:
(top level context selected)
(gdb) thread 1
(gdb) frame
#0 ...
(gdb) thread apply 1 up
(push new context)
(selected thread 1)
(execute "up")
(pop context)
(gdb) thread
[Current thread is 1 ...]
(gdb) frame
#0 ...
E.g. 2:
(top level context selected)
(gdb) b foo.c:100
Breakpoint 1 ...
(gdb) commands
>print func_arg
>c&
>end
(gdb) thread 1
(thread 1 is running)
(gdb) c -a&
(thread 2 hits breakpoint 1)
(GDB switches to it to handle the event)
(GDB pushes new context)
(GDB switches to the event thread (thread 2))
(calls bpstat_do_actions)
(pops context)
(restores current context, which is thread 1)
E.g. 3 - thread exits:
(gdb) b foo.c:100
Breakpoint 1 ...
(gdb) commands
>print inferior_func ()
>c&
>end
(top level context selected)
(gdb) thread 1
(gdb) c -a&
(all threads running now)
(thread 2 hits breakpoint 1)
(GDB switches to it to handle the event)
(GDB pushes new context)
(GDB switches to the event thread (thread 2))
(calls bpstat_do_actions)
(inferior function call, inferior is set running for a bit
(thread 1 exits
(all contexts with thread 1 selected are invalidated)
)
)
(pops context)
(restores current context, which was thread 1, but is not invalidaded.
i.e., there is now no selected thread in the current context.)
(gdb) interrupt
Cannot execute this command without a selected thread. See `help thread'.
E.g. 4:
(gdb) b foo.c:100
Breakpoint 1 ...
(gdb) commands
>thread 3
>c&
>end
(gdb) interrupt -a
(all threads stopped)
(gdb) thread 2
(gdb) c&
(thread 2 resumes)
(gdb) thread 1
(gdb) print foo_c
(gdb) print foo_b
(gdb) pri
(thread 2 hits breakpoint 1
(a new context is pushed)
(breakpoint commands are ran
(thread 3; c&)
)
(context is poped, now back in thread 1)
nt foo_a
(foo_a is evaluated in thread 1)
Hope I've made some sense.
Attached is a first draft at implementing this, needs a bit
of cleanup, but is does work. What do you guys think? Is this
the right direction? It does regtest regression free on
x86_64-unknown-linux-gnu/all-stop.
Vladimir, I've added a basic --thread switch to MI just for
testing, based on older MI non-stop patches I had
here, just so you could see what would change in your
perspective. As you see, only a couple of lines should change.
I'll remove it from any final patch.
> > +enum thread_state
> > +{
> > + THREAD_STOPPED,
> > + THREAD_RUNNING,
> > + THREAD_EXITED,
> > +};
> > +
> > +static enum thread_state main_thread_state = 0;
>
> THREAD_STOPPED instead of 0?
Done.
>
> > - /* Backup current thread and selected frame. */
> > - if (!is_running (inferior_ptid))
> > - saved_frame_id = get_frame_id (get_selected_frame (NULL));
> > - else
> > - saved_frame_id = null_frame_id;
> > -
> > - old_chain = make_cleanup_restore_current_thread (inferior_ptid,
> > saved_frame_id);
>
> Is this sort of simplification really a good idea? Functions which
> leave inferior_ptid with an essentially unspecified value, while it's
> still global state used by most of GDB.
>
The idea is that execute_command restores the user selected
thread and frame anyway (and MI's equivalent does as well), so
this isn't needed.
Let's discuss the other general issues first, as if the direction
I'm proposing is better, this function will still be restoring
the thread and frame, when the context is poped.
> > +/* Only safe to use this cleanup on a stopped thread, and
> > + inferior_ptid isn't ran before running the cleanup. */
> > +
> > struct current_thread_cleanup
>
> Not sure what you mean by this comment.
>
I've changed the comment to this:
/* It is only safe to use this cleanup iff inferior_ptid is alive and
stopped, and, by if by the time it is ran, inferior_ptid represents
the same thread, it is alive and it is stopped. */
Does it make it clearer?
> > + /* In non-stop mode, we don't want GDB to switch threads on the
> > + users back, to avoid races where the user is typing a command to
>
> user's
Fixed, thanks.
--
Pedro Alves
---
gdb/gdbcmd.h | 2
gdb/inf-loop.c | 12 +++
gdb/infcmd.c | 4 -
gdb/inferior.h | 25 +++++--
gdb/infrun.c | 177 +++++++++++++++++++++++++++++++++++++++++-----------
gdb/mi/mi-cmd-env.c | 2
gdb/mi/mi-main.c | 65 ++++++++++++++++---
gdb/stack.c | 11 +--
gdb/thread.c | 63 +++++++++++-------
gdb/top.c | 46 +++----------
gdb/top.h | 3
11 files changed, 287 insertions(+), 123 deletions(-)
Index: src/gdb/inf-loop.c
===================================================================
--- src.orig/gdb/inf-loop.c 2008-06-29 23:44:19.000000000 +0100
+++ src/gdb/inf-loop.c 2008-06-29 23:47:19.000000000 +0100
@@ -108,6 +108,14 @@ inferior_event_handler (enum inferior_ev
&& language_mode == language_mode_auto)
language_info (1); /* Print what changed. */
+
+ /* Run breakpoint commands in a new context. Note: this means
+ that if a breakpoint command switches threads with "thread
+ n", that change is discarded after the breakpoint commands
+ finish. */
+ if (non_stop)
+ push_thread_context (inferior_ptid, get_selected_frame (NULL));
+
/* Don't propagate breakpoint commands errors. Either we're
stopping or some command resumes the inferior. The user will
be informed. */
@@ -116,6 +124,10 @@ inferior_event_handler (enum inferior_ev
bpstat_do_actions (&stop_bpstat);
}
+ /* Restore the previous context. */
+ if (non_stop)
+ pop_thread_context ();
+
/* If no breakpoint command resumed the inferior, prepare for
interaction with the user. */
if (!is_running (inferior_ptid))
Index: src/gdb/infcmd.c
===================================================================
--- src.orig/gdb/infcmd.c 2008-06-29 23:44:19.000000000 +0100
+++ src/gdb/infcmd.c 2008-06-29 23:44:32.000000000 +0100
@@ -570,7 +570,7 @@ run_command_1 (char *args, int from_tty,
environ_vector (inferior_environ), from_tty);
/* If no selected thread, select the current one. */
- if (ptid_equal (user_selected_ptid, null_ptid))
+ if (is_user_selected_thread (null_ptid))
/* Going to proceed, no need to record a frame. */
store_selected_thread_and_frame (inferior_ptid, NULL);
@@ -1971,7 +1971,7 @@ attach_command_post_wait (char *args, in
if (async_exec)
{
/* If no selected thread, select the current one. */
- if (ptid_equal (user_selected_ptid, null_ptid))
+ if (is_user_selected_thread (null_ptid))
/* Going to proceed, no need to record a frame. */
store_selected_thread_and_frame (inferior_ptid, NULL);
proceed ((CORE_ADDR) -1, TARGET_SIGNAL_0, 0);
Index: src/gdb/inferior.h
===================================================================
--- src.orig/gdb/inferior.h 2008-06-29 23:44:19.000000000 +0100
+++ src/gdb/inferior.h 2008-06-29 23:44:32.000000000 +0100
@@ -405,12 +405,25 @@ extern int debug_displaced;
void displaced_step_dump_bytes (struct ui_file *file,
const gdb_byte *buf, size_t len);
-/* This points to the user/external selected thread and frame. In
- all-stop mode, a stop event changes the external thread to the
- thread that got the event (in normal_stop), while in non-stop mode,
- GDB never changes the external thread automatically (although the
- internal thread may change). */
-extern ptid_t user_selected_ptid;
+/* Return true if PTID points to the user/external selected thread of
+ the current context. */
+extern int is_user_selected_thread (ptid_t ptid);
+
+/* Returns the current context's user selected thread. */
+extern ptid_t get_user_selected_thread (void);
+
+/* Push a new context with PTID and FRAME selected into the context
+ stack. */
+extern void push_thread_context (ptid_t ptid, struct frame_info *frame);
+
+/* Push a new context cloned from the current context. */
+extern void clone_thread_context (void);
+
+/* Pop the current context off the context stack. */
+extern void pop_thread_context (void);
+
+/* Invalidate all contexts that point at PTID. */
+extern void invalidate_contexts_of (ptid_t ptid);
/* Records PTID and FRAME as user/frontend/external selected thread
and frame. */
Index: src/gdb/infrun.c
===================================================================
--- src.orig/gdb/infrun.c 2008-06-29 23:44:19.000000000 +0100
+++ src/gdb/infrun.c 2008-06-29 23:47:07.000000000 +0100
@@ -46,9 +46,10 @@
#include "solib.h"
#include "main.h"
#include "interps.h"
-
#include "gdb_assert.h"
#include "mi/mi-common.h"
+#include "vec.h"
+#include "cli/cli-decode.h"
/* Prototypes for local functions */
@@ -1694,21 +1695,116 @@ context_switch_to (ptid_t ptid)
thread that got the event (in normal_stop), while in non-stop mode,
GDB never changes the external thread automatically (although the
internal thread (inferior_ptid) may change). */
+struct frontend_context
+{
+ /* Selected thread/process. If NULL_PTID, there is no selected
+ thread. */
+ ptid_t ptid;
+ /* Selected frame of the selected thread. We store both frame id
+ and level. See restore_selected_thread_and_frame. */
+ struct frame_id frame;
+ int frame_level;
+};
+typedef struct frontend_context * frontend_context_t;
+
+DEF_VEC_P(frontend_context_t);
+static VEC(frontend_context_t) *thread_context_stack;
+static int thread_context_level = -1;
+
+static struct frontend_context *
+get_current_thread_context (void)
+{
+ struct frontend_context *ctx;
+ ctx = VEC_index (frontend_context_t,
+ thread_context_stack, thread_context_level);
+ gdb_assert (ctx);
+ return ctx;
+}
+
+void
+invalidate_contexts_of (ptid_t ptid)
+{
+ int ix = 0;
+ struct frontend_context *ctx;
+
+ for (ix = 0; VEC_iterate (frontend_context_t, thread_context_stack, ix, ctx); ix++)
+ {
+ if (ptid_equal (ctx->ptid, ptid))
+ {
+ ctx->ptid = null_ptid;
+ ctx->frame = null_frame_id;
+ ctx->frame_level = -1;
+ }
+ }
+}
+
+void
+push_thread_context (ptid_t ptid, struct frame_info *frame)
+{
+ struct frontend_context *ctx;
+ struct frontend_context *new_ctx = XMALLOC (struct frontend_context);
-/* Selected thread/process. If NULL_PTID, there is no selected
- thread. */
-ptid_t user_selected_ptid;
-
-/* Selected frame of the selected thread. We store both frame id and
- level. See restore_selected_thread_and_frame. */
-static struct frame_id user_selected_frame;
-static int user_selected_frame_level;
+ new_ctx->ptid = null_ptid;
+ new_ctx->frame = null_frame_id;
+ new_ctx->frame_level = -1;
+
+ VEC_safe_push (frontend_context_t, thread_context_stack, new_ctx);
+ thread_context_level++;
+
+ store_selected_thread_and_frame (ptid, frame);
+}
+
+void
+clone_thread_context (void)
+{
+ struct frontend_context *ctx = get_current_thread_context ();
+ struct frontend_context *new_ctx = XMALLOC (struct frontend_context);
+
+ new_ctx->ptid = ctx->ptid;
+ new_ctx->frame = ctx->frame;
+ new_ctx->frame_level = ctx->frame_level;
+
+ VEC_safe_push (frontend_context_t, thread_context_stack, new_ctx);
+ thread_context_level++;
+}
+
+void
+pop_thread_context (void)
+{
+ struct frontend_context *ctx = get_current_thread_context ();
+ if (thread_context_level == 0)
+ return;
+
+ ctx = VEC_last (frontend_context_t, thread_context_stack);
+ xfree (ctx);
+
+ VEC_pop (frontend_context_t, thread_context_stack);
+ thread_context_level--;
+
+ /* Restore this context. */
+ restore_selected_thread_and_frame ();
+}
+
+ptid_t
+get_user_selected_thread (void)
+{
+ return get_current_thread_context ()->ptid;
+}
+
+int
+is_user_selected_thread (ptid_t ptid)
+{
+ ptid_t selected_ptid = get_user_selected_thread ();
+ return ptid_equal (selected_ptid, ptid);
+}
/* Register PTID and FRAME as user selected thread and frame. If PTID
is NULL_PTID, FRAME has to be NULL. */
void
store_selected_thread_and_frame (ptid_t ptid, struct frame_info *frame)
{
+ struct frontend_context *ctx = get_current_thread_context ();
+
/* If a frame is passed, we need to be able to build an id from it.
GDB only parses frames of inferior_ptid, and frames don't
registers which thread they belong to. */
@@ -1719,18 +1815,18 @@ store_selected_thread_and_frame (ptid_t
if (ptid_equal (ptid, null_ptid))
gdb_assert (frame == NULL);
- user_selected_ptid = ptid;
- user_selected_frame = get_frame_id (frame);
- user_selected_frame_level = frame_relative_level (frame);
+ ctx->ptid = ptid;
+ ctx->frame = get_frame_id (frame);
+ ctx->frame_level = frame_relative_level (frame);
if (debug_infrun)
{
fprintf_unfiltered (gdb_stdlog, "\
infrun.c: Storing user selected thread and frame [%s], ",
- target_pid_to_str (user_selected_ptid));
- fprint_frame_id (gdb_stdlog, user_selected_frame);
+ target_pid_to_str (ctx->ptid));
+ fprint_frame_id (gdb_stdlog, ctx->frame);
fprintf_unfiltered (gdb_stdlog, ", #%d.\n",
- user_selected_frame_level);
+ ctx->frame_level);
}
}
@@ -1738,42 +1834,46 @@ infrun.c: Storing user selected thread a
void
restore_selected_thread_and_frame (void)
{
+ struct frontend_context *ctx;
+
if (!target_has_registers || !target_has_stack || !target_has_memory)
return;
+ ctx = get_current_thread_context ();
+
/* Don't switch the internal current thread to the null thread. Too
many places require inferior_ptid to point at a valid process
(even if the thread component is invalid) currently. */
- if (!ptid_equal (user_selected_ptid, null_ptid))
+ if (!ptid_equal (ctx->ptid, null_ptid))
{
if (debug_infrun)
{
fprintf_unfiltered (gdb_stdlog, "\
infrun.c: restoring_selected_thread_and_frame: [%s], ",
- target_pid_to_str (user_selected_ptid));
- fprint_frame_id (gdb_stdlog, user_selected_frame);
+ target_pid_to_str (ctx->ptid));
+ fprint_frame_id (gdb_stdlog, ctx->frame);
fprintf_unfiltered (gdb_stdlog, ", #%d\n",
- user_selected_frame_level);
+ ctx->frame_level);
}
if (non_stop)
- context_switch_to (user_selected_ptid);
+ context_switch_to (ctx->ptid);
else
- switch_to_thread (user_selected_ptid);
+ switch_to_thread (ctx->ptid);
- if (is_stopped (user_selected_ptid))
+ if (is_stopped (ctx->ptid))
{
struct frame_info *frame;
int count;
- gdb_assert (user_selected_frame_level >= 0);
+ gdb_assert (ctx->frame_level >= 0);
/* Restore by level first, check if the frame id is the same
as expected. If that fails, try restoring by frame id.
If that fails, nothing to do, just warn the user. */
- count = user_selected_frame_level;
+ count = ctx->frame_level;
frame = find_relative_frame (get_current_frame (), &count);
if (count == 0
&& frame != NULL
@@ -1782,11 +1882,11 @@ infrun.c: restoring_selected_thread_and_
unlikelly the search by level finds the wrong frame,
it's 99.9(9)% of the times (for all practical
purposes) safe. */
- && (frame_id_eq (get_frame_id (frame), user_selected_frame)
+ && (frame_id_eq (get_frame_id (frame), ctx->frame)
/* Note: could be better to check every frame_id
member for equality here. */
|| (!frame_id_p (get_frame_id (frame))
- && !frame_id_p (user_selected_frame))))
+ && !frame_id_p (ctx->frame))))
{
/* Cool, all is fine. */
select_frame (frame);
@@ -1795,7 +1895,7 @@ infrun.c: restoring_selected_thread_and_
return;
}
- frame = frame_find_by_id (user_selected_frame);
+ frame = frame_find_by_id (ctx->frame);
if (frame != NULL)
{
/* Cool, refound it. */
@@ -1803,10 +1903,10 @@ infrun.c: restoring_selected_thread_and_
/* The relative level changed, otherwise we would have
found it above. */
- user_selected_frame_level = frame_relative_level (frame);
+ ctx->frame_level = frame_relative_level (frame);
if (debug_infrun)
fprintf_unfiltered (gdb_stdlog, "\
-infrun.c: success (id): new level #%d\n", user_selected_frame_level);
+infrun.c: success (id): new level #%d\n", ctx->frame_level);
return;
}
@@ -1819,7 +1919,7 @@ infrun.c: success (id): new level #%d\n"
{
warning (_("\
Couldn't restore frame #%d in current thread, at reparsed frame #0\n"),
- user_selected_frame_level);
+ ctx->frame_level);
/* For MI, we should probably have a notification about
current frame change. But this error is not very
likely, so don't bother for now. */
@@ -1827,16 +1927,16 @@ Couldn't restore frame #%d in current th
}
frame = get_selected_frame (NULL);
- user_selected_frame = get_frame_id (frame);
- user_selected_frame_level = frame_relative_level (frame);
+ ctx->frame = get_frame_id (frame);
+ ctx->frame_level = frame_relative_level (frame);
if (debug_infrun)
{
fprintf_unfiltered (gdb_stdlog, "\
infrun.c: user selected frame changed to ");
- fprint_frame_id (gdb_stdlog, user_selected_frame);
+ fprint_frame_id (gdb_stdlog, ctx->frame);
fprintf_unfiltered (gdb_stdlog, ", #%d.\n",
- user_selected_frame_level);
+ ctx->frame_level);
}
}
}
@@ -1845,7 +1945,7 @@ infrun.c: user selected frame changed to
void
select_thread_if_no_thread_selected (void)
{
- if (ptid_equal (user_selected_ptid, null_ptid)
+ if (is_user_selected_thread (null_ptid)
&& !ptid_equal (inferior_ptid, null_ptid))
{
if (is_stopped (inferior_ptid))
@@ -3892,7 +3992,7 @@ normal_stop (void)
Note that SIGNALLED here means "exited with a signal", not
"received a signal". */
if (!non_stop
- && !ptid_equal (user_selected_ptid, inferior_ptid)
+ && !is_user_selected_thread (inferior_ptid)
&& target_has_execution
&& last.kind != TARGET_WAITKIND_SIGNALLED
&& last.kind != TARGET_WAITKIND_EXITED)
@@ -4076,7 +4176,7 @@ done:
{
if ((!stop_stack_dummy || !proceed_to_inferior_function_call)
&& (!non_stop
- || ptid_equal (user_selected_ptid, inferior_ptid)))
+ || is_user_selected_thread (inferior_ptid)))
/* Selected thread just stopped. Record the selected frame.
In all-stop, the thread that has the event is always made
the current. Don't do this on return from a stack dummy
@@ -5002,4 +5102,7 @@ breakpoints, even if such is supported b
inferior_ptid = null_ptid;
target_last_wait_ptid = minus_one_ptid;
displaced_step_ptid = null_ptid;
+
+ /* Top level context. */
+ push_thread_context (null_ptid, NULL);
}
Index: src/gdb/mi/mi-main.c
===================================================================
--- src.orig/gdb/mi/mi-main.c 2008-06-29 23:44:19.000000000 +0100
+++ src/gdb/mi/mi-main.c 2008-06-29 23:44:32.000000000 +0100
@@ -1056,13 +1056,19 @@ do_restore_selected_thread_and_frame_cle
}
static void
+do_pop_context (void *arg)
+{
+ pop_thread_context ();
+}
+
+static void
mi_cmd_execute (struct mi_parse *parse)
{
struct cleanup *cleanup = make_cleanup (null_cleanup, NULL);
free_all_values ();
current_token = xstrdup (parse->token);
- cleanup = make_cleanup (free_current_contents, ¤t_token);
+ make_cleanup (free_current_contents, ¤t_token);
/* Switch to the frontend selected thread, before invoking the
command, so the command implementation sees the correct
@@ -1076,9 +1082,55 @@ mi_cmd_execute (struct mi_parse *parse)
if (parse->cmd->argv_func != NULL)
{
+ int i = 0;
+ for (i = 0; i < parse->argc; ++i)
+ {
+ if (strcmp (parse->argv[i], "--thread") == 0)
+ {
+ if (i + 1 < parse->argc)
+ {
+ enum gdb_rc rc;
+ char *mi_error_message;
+ int j, k;
+ int num;
+ ptid_t ptid;
+
+ num = value_as_long (parse_and_eval (parse->argv[i + 1]));
+
+ if (!valid_thread_id (num))
+ error (_("Thread ID %d not known."), num);
+
+ ptid = thread_id_to_pid (num);
+
+ if (is_exited (ptid))
+ error (_("Thread ID %d has terminated."), num);
+
+ clone_thread_context ();
+ make_cleanup (do_pop_context, NULL);
+
+ /* Silently switch to new thread, and record it as
+ current in the new context. */
+ if (non_stop)
+ context_switch_to (ptid);
+ else
+ switch_to_thread (ptid);
+ store_selected_thread_and_frame (inferior_ptid,
+ get_selected_frame (NULL));
+
+ /* FIXME: should we free the two items? */
+ for (j = i, k = i + 2; k < parse->argc; ++k, ++k)
+ parse->argv[i] = parse->argv[k];
+
+ parse->argc -= 2;
+ }
+ else
+ error ("The --thread option requires a value");
+ }
+ }
+
if (target_can_async_p ()
&& target_has_execution
- && (ptid_equal (user_selected_ptid, null_ptid))
+ && (is_user_selected_thread (null_ptid))
&& (strcmp (parse->command, "thread-info") != 0
&& strcmp (parse->command, "thread-list-ids") != 0
&& strcmp (parse->command, "thread-select") != 0))
@@ -1157,12 +1209,7 @@ mi_execute_cli_command (const char *cmd,
fprintf_unfiltered (gdb_stdout, "cli=%s run=%s\n",
cmd, run);
old_cleanups = make_cleanup (xfree, run);
- /* execute_command switches to the user selected thread. We
- don't want that switch. We want the command to apply to
- inferior_ptid instead, hence use the _internal version. We
- don't change temporarily the user selected thread and restore
- it with a cleanup, because the command may change it. */
- execute_command_internal ( /*ui */ run, 0 /*from_tty */ );
+ execute_command ( /*ui */ run, 0 /*from_tty */ );
do_cleanups (old_cleanups);
return;
}
@@ -1180,7 +1227,7 @@ mi_execute_async_cli_command (char *cli_
run = xstrprintf ("%s %s", cli_command, argc ? *argv : "");
old_cleanups = make_cleanup (xfree, run);
- execute_command_internal ( /*ui */ run, 0 /*from_tty */ );
+ execute_command ( /*ui */ run, 0 /*from_tty */ );
if (target_can_async_p ())
{
Index: src/gdb/stack.c
===================================================================
--- src.orig/gdb/stack.c 2008-06-29 23:44:19.000000000 +0100
+++ src/gdb/stack.c 2008-06-29 23:44:32.000000000 +0100
@@ -1699,9 +1699,8 @@ select_frame_command (char *level_exp, i
select_frame (parse_frame_specification_1 (level_exp, "No stack.", NULL));
/* Record user selected frame. */
- if (ptid_equal (inferior_ptid, user_selected_ptid))
- store_selected_thread_and_frame (inferior_ptid,
- get_selected_frame (NULL));
+ store_selected_thread_and_frame (inferior_ptid,
+ get_selected_frame (NULL));
}
/* The "frame" command. With no argument, print the selected frame
@@ -1741,8 +1740,7 @@ up_silently_base (char *count_exp)
select_frame (frame);
/* Record user selected frame. */
- if (ptid_equal (inferior_ptid, user_selected_ptid))
- store_selected_thread_and_frame (inferior_ptid, frame);
+ store_selected_thread_and_frame (inferior_ptid, frame);
}
static void
@@ -1783,8 +1781,7 @@ down_silently_base (char *count_exp)
select_frame (frame);
/* Record user selected frame. */
- if (ptid_equal (inferior_ptid, user_selected_ptid))
- store_selected_thread_and_frame (inferior_ptid, frame);
+ store_selected_thread_and_frame (inferior_ptid, frame);
}
static void
Index: src/gdb/thread.c
===================================================================
--- src.orig/gdb/thread.c 2008-06-29 23:44:19.000000000 +0100
+++ src/gdb/thread.c 2008-06-30 01:06:22.000000000 +0100
@@ -244,10 +244,12 @@ delete_thread (ptid_t ptid)
if (!tp)
return;
- if (ptid_equal (tp->ptid, user_selected_ptid))
+ if (is_user_selected_thread (tp->ptid))
/* User selected thread no longer exists. */
store_selected_thread_and_frame (null_ptid, NULL);
+ invalidate_contexts_of (tp->ptid);
+
if (ptid_equal (tp->ptid, inferior_ptid))
{
/* Can't delete yet. Mark it as exited, and notify it. */
@@ -740,6 +742,12 @@ set_executing (ptid_t ptid, int executin
}
}
+static void
+do_pop_context (void *arg)
+{
+ pop_thread_context ();
+}
+
/* Prints the list of threads and their details on UIOUT.
This is a version of 'info_thread_command' suitable for
use from MI.
@@ -759,6 +767,9 @@ print_thread_info (struct ui_out *uiout,
old_chain = make_cleanup_ui_out_list_begin_end (uiout, "threads");
+ clone_thread_context ();
+ make_cleanup (do_pop_context, NULL);
+
for (tp = thread_list; tp; tp = tp->next)
{
struct cleanup *chain2;
@@ -772,7 +783,7 @@ print_thread_info (struct ui_out *uiout,
chain2 = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
- if (ptid_equal (tp->ptid, user_selected_ptid))
+ if (is_user_selected_thread (tp->ptid))
{
current_thread = tp->num;
ui_out_text (uiout, "* ");
@@ -812,15 +823,13 @@ print_thread_info (struct ui_out *uiout,
do_cleanups (chain2);
}
- /* Restores the current thread and the frame selected before
- the "info threads" command. */
do_cleanups (old_chain);
if (requested_thread == -1)
{
gdb_assert (current_thread != -1
|| !thread_list
- || ptid_equal (user_selected_ptid, null_ptid));
+ || is_user_selected_thread (null_ptid));
if (current_thread != -1 && ui_out_is_mi_like_p (uiout))
ui_out_field_int (uiout, "current-thread-id", current_thread);
@@ -940,6 +949,9 @@ thread_apply_all_command (char *cmd, int
prune_threads ();
target_find_new_threads ();
+ clone_thread_context ();
+ make_cleanup (do_pop_context, NULL);
+
/* Save a copy of the command in case it is clobbered by
execute_command */
saved_cmd = xstrdup (cmd);
@@ -947,20 +959,19 @@ thread_apply_all_command (char *cmd, int
for (tp = thread_list; tp; tp = tp->next)
if (thread_alive (tp))
{
+ /* Silently switch to new thread, and record it as current in
+ the new context. */
if (non_stop)
context_switch_to (tp->ptid);
else
switch_to_thread (tp->ptid);
+ store_selected_thread_and_frame (inferior_ptid,
+ get_selected_frame (NULL));
printf_filtered (_("\nThread %d (%s):\n"),
tp->num, target_tid_to_str (inferior_ptid));
- /* execute_command switches to the user selected thread. We
- don't want that switch. We want the command to apply to
- inferior_ptid instead, hence use the _internal version. We
- don't change temporarily the user selected thread and
- restore it with a cleanup, because the command may change
- it. */
- execute_command_internal (cmd, from_tty);
+ /* Now execute the command in this new context. */
+ execute_command (cmd, from_tty);
strcpy (cmd, saved_cmd); /* Restore exact command used previously */
}
@@ -1024,20 +1035,25 @@ thread_apply_command (char *tidlist, int
warning (_("Thread %d has terminated."), start);
else
{
+ clone_thread_context ();
+ make_cleanup (do_pop_context, NULL);
+
+ /* Silently switch to new thread, and record it as current in
+ the new context. */
if (non_stop)
context_switch_to (tp->ptid);
else
switch_to_thread (tp->ptid);
+ store_selected_thread_and_frame (inferior_ptid,
+ get_selected_frame (NULL));
+
printf_filtered (_("\nThread %d (%s):\n"), tp->num,
target_tid_to_str (inferior_ptid));
- /* execute_command switches to the user selected thread.
- We don't want that switch. We want the command to
- apply to inferior_ptid instead, hence use the
- _internal version. We don't change temporarily the
- user selected thread and restore it with a cleanup,
- because the command may change it. */
- execute_command_internal (cmd, from_tty);
- strcpy (cmd, saved_cmd); /* Restore exact command used previously */
+ /* Now execute the command in this new context. */
+ execute_command (cmd, from_tty);
+
+ /* Restore exact command used previously. */
+ strcpy (cmd, saved_cmd);
}
}
}
@@ -1055,14 +1071,15 @@ thread_command (char *tidstr, int from_t
{
if (target_has_stack)
{
+ ptid_t ptid = get_user_selected_thread ();
/* Don't generate an error, just say which thread is
current. */
- if (ptid_equal (user_selected_ptid, null_ptid))
+ if (ptid_equal (ptid, null_ptid))
printf_filtered (_("No selected thread.\n"));
else
printf_filtered (_("[Current thread is %d (%s)]\n"),
- pid_to_thread_id (user_selected_ptid),
- target_tid_to_str (user_selected_ptid));
+ pid_to_thread_id (ptid),
+ target_tid_to_str (ptid));
}
else
error (_("No stack."));
Index: src/gdb/top.c
===================================================================
--- src.orig/gdb/top.c 2008-06-29 23:44:19.000000000 +0100
+++ src/gdb/top.c 2008-06-29 23:44:32.000000000 +0100
@@ -368,8 +368,10 @@ do_restore_selected_thread_and_frame_cle
/* Execute the line P as a command.
Pass FROM_TTY as second argument to the defining function. */
+/* Execute command P, in the current user context. */
+
void
-execute_command_1 (char *p, int from_tty, int internal)
+execute_command (char *p, int from_tty)
{
struct cmd_list_element *c;
enum language flang;
@@ -422,25 +424,21 @@ execute_command_1 (char *p, int from_tty
c = lookup_cmd (&p, cmdlist, "", 0, 1);
- if (!internal)
- {
- /* Command applies to the external thread. Switch the
- internal thread to it before invoking the command, so the
- command implementation sees the correct context. */
- restore_selected_thread_and_frame ();
- /* Switch to the external thread again after invoking the
- command, so if for some reason restoring the selected
- frame fails, the user notices immediatelly. Note that
- the command may change the selected thread. */
- make_cleanup (do_restore_selected_thread_and_frame_cleanup, NULL);
- }
+ /* Make sure the internal thread points to the user selected
+ thread, so the command implementation sees the correct
+ context. */
+ restore_selected_thread_and_frame ();
+ /* Switch to the external thread again after invoking the
+ command, so if for some reason restoring the selected frame
+ fails, the user notices immediatelly. Note that the command
+ may change the selected thread and/or frame */
+ make_cleanup (do_restore_selected_thread_and_frame_cleanup, NULL);
/* If there is no selected thread, we allow only a limited set
of commands. */
if (target_can_async_p ()
&& target_has_execution
- && (!internal
- && ptid_equal (user_selected_ptid, null_ptid))
+ && is_user_selected_thread (null_ptid)
&& !get_cmd_no_selected_thread_ok (c))
error (_("\
Cannot execute this command without a selected thread. See `help thread'"));
@@ -528,24 +526,6 @@ Cannot execute this command without a se
}
}
-/* Execute command P, and use the already selected internal thread
- (inferior_ptid) as current context. Use in situations like the
- "thread apply" command. */
-
-void
-execute_command_internal (char *p, int from_tty)
-{
- execute_command_1 (p, from_tty, 1);
-}
-
-/* Execute command P, with the user/frontend/external thread as
- context. */
-void
-execute_command (char *p, int from_tty)
-{
- execute_command_1 (p, from_tty, 0);
-}
-
/* Read commands from `instream' and execute them
until end of file or error reading instream. */
Index: src/gdb/gdbcmd.h
===================================================================
--- src.orig/gdb/gdbcmd.h 2008-06-29 23:44:19.000000000 +0100
+++ src/gdb/gdbcmd.h 2008-06-29 23:44:32.000000000 +0100
@@ -122,8 +122,6 @@ extern struct cmd_list_element *showchec
extern void execute_command (char *, int);
-extern void execute_command_internal (char *, int);
-
enum command_control_type execute_control_command (struct command_line *);
extern void print_command_line (struct command_line *, unsigned int,
Index: src/gdb/mi/mi-cmd-env.c
===================================================================
--- src.orig/gdb/mi/mi-cmd-env.c 2008-06-29 23:44:19.000000000 +0100
+++ src/gdb/mi/mi-cmd-env.c 2008-06-29 23:44:32.000000000 +0100
@@ -56,7 +56,7 @@ env_execute_cli_command (const char *cmd
else
run = xstrdup (cmd);
old_cleanups = make_cleanup (xfree, run);
- execute_command_internal ( /*ui */ run, 0 /*from_tty */ );
+ execute_command ( /*ui */ run, 0 /*from_tty */ );
do_cleanups (old_cleanups);
return;
}
Index: src/gdb/top.h
===================================================================
--- src.orig/gdb/top.h 2008-06-29 23:44:19.000000000 +0100
+++ src/gdb/top.h 2008-06-29 23:44:32.000000000 +0100
@@ -51,9 +51,6 @@ extern int quit_cover (void *);
/* Execute command, after changing to the user selected thread. */
extern void execute_command (char *, int);
-/* Execute command, without changing to the user selected thread. */
-extern void execute_command_internal (char *, int);
-
/* This function returns a pointer to the string that is used
by gdb for its command prompt. */
extern char *get_prompt (void);