From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19913 invoked by alias); 30 Jun 2008 00:09:24 -0000 Received: (qmail 19843 invoked by uid 22791); 30 Jun 2008 00:09:20 -0000 X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.31) with ESMTP; Mon, 30 Jun 2008 00:08:53 +0000 Received: (qmail 5289 invoked from network); 30 Jun 2008 00:08:49 -0000 Received: from unknown (HELO orlando.local) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 30 Jun 2008 00:08:49 -0000 From: Pedro Alves To: Daniel Jacobowitz Subject: Re: [non-stop] 10/10 split user/internal threads Date: Mon, 30 Jun 2008 12:21:00 -0000 User-Agent: KMail/1.9.9 Cc: gdb-patches@sourceware.org References: <200806152209.45049.pedro@codesourcery.com> <20080626133251.GA22726@caradoc.them.org> In-Reply-To: <20080626133251.GA22726@caradoc.them.org> MIME-Version: 1.0 Content-Disposition: inline Content-Type: Multipart/Mixed; boundary="Boundary-00=_RQCaIgNCIwlqUsr" Message-Id: <200806300108.49392.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-06/txt/msg00584.txt.bz2 --Boundary-00=_RQCaIgNCIwlqUsr Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Content-Disposition: inline Content-length: 12788 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): > > > > =A01 - execute_command, before executing a command > > =A02 - execute_command, after executing a command, > > =A03 - 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 - - 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 (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 ...] =46rom 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.= =20=20=20=20 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 =3D 0; > > THREAD_STOPPED instead of 0? Done. > > > - /* Backup current thread and selected frame. */ > > - if (!is_running (inferior_ptid)) > > - saved_frame_id =3D get_frame_id (get_selected_frame (NULL)); > > - else > > - saved_frame_id =3D null_frame_id; > > - > > - old_chain =3D 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. --=20 Pedro Alves --Boundary-00=_RQCaIgNCIwlqUsr Content-Type: text/x-diff; charset="iso-8859-1"; name="context_thread.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="context_thread.diff" Content-length: 29249 --- 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); --Boundary-00=_RQCaIgNCIwlqUsr--