* [non-stop] 10/10 split user/internal threads
@ 2008-06-15 21:49 Pedro Alves
2008-06-26 13:41 ` Daniel Jacobowitz
0 siblings, 1 reply; 11+ messages in thread
From: Pedro Alves @ 2008-06-15 21:49 UTC (permalink / raw)
To: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 6942 bytes --]
This is the patch that split the notion of the frontend selected
thread and frame from inferior_ptid. I've come to the conclusion
this is the path to go for several reasons.
- In non-stop mode, the user/frontend will have a stopped thread
and frame selected, while a new event in another thread comes
along. (Aside from having GDB switch threads automatically or
not when another thread stops, ) this may just be an
internal event. The user should not lose its selection. More
on that below.
- We avoided deleting inferior_ptid until the target stopped. This
worked in all-stopped mode, as there, the user is not be able to enter
commands that apply to the thread that has already exited. This
split make it safe to notify the threads module that
inferior_ptid exited. If we don't split the external/internal
thread notion, then cases like switching threads and restoring
the previous thread with a cleanup are tricky, because there is
no reference anywhere to which thread was the previous one, other
than inside the cleanup. Comparing with inferior_ptid only
doesn't cut it.
- The frontend wants to be reported of thread and frame
changes that are interesting to the user, not every internal
thread and selected frame change, used, e.g., while updating
var-objects.
The patch then updates the behaviour of GDB on the case of
the selected thread exiting. It implements a scheme similar
to c) of the options listed here:
http://sourceware.org/ml/gdb/2008-06/msg00013.html
This requires some explanation, so here goes.
After having tried solutions
b) (keep dead threads on the list, until
the user switches threads) and
c) (don't keep dead threads on the list and make inferior_ptid point
to some magic ptid if inferior_ptid exits),
and several combinations of the above, including one that added a new
field to ptid (so dead threads would not match ptids comming from the
events from the target layer),
I came to the conclusion that there was something wrong with
the way GDB manages the current user/frontend/external thread.
Overloading inferior_ptid to mean the current thread/process that GDB
is internally operating on, and also the user selected thread is
really not what we want. It was OK in all-stop mode, but is
not ideal in non-stop.
So, what the patch does is upgrade previous_inferior_ptid (which
already played a small role in the "hey, I've switched to a
new thread" business) to represent the external/user selected thread,
rename it to user_selected_ptid, and add companion user_selected_frame
and user_selected_frame_level, which are used to represent the
frame the user has selected.
The new store_selected_thread_and_frame is used to
store the record which thread and frame the user/frontend has selected,
and restore_selected_thread_and_frame is used to switch inferior_ptid
and its selected frame to point back to what the user/frontend expects.
With this patch in place, the target side can now call
delete_thread without caring if inferior_ptid points to it. The
common code will take care of not deleting it just immediatelly, and
still tagging/notifying of its exit. It will be deleted whenever
it's safe. thread_info's running state also gets a small upgrade into
an enum that stores the running/stopped/exited state.
If add_thread* is called, and the ptid being added is already listed,
it means the OS is reusing the tid. In that case, we delete the old
ptid from the list, and add the new one --- this new one will get a
new GDB thread num. If the thread just exiting was also the user
selected thread, we invalidate it. This means that in all-stop,
if the OS is reusing the selected thread's ptid, and this new thread
has the next event, normal_stop will now notice that this is a
new thread, and print a "switched to thread" notice, although the
ptid didn't change (if we added the GDB thread num to that output,
the user could notice the change).
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.
You'll also notice that the implementations of "info threads",
"thread apply all/tid" and a couple of other places get a clean up,
since now execute_command takes care of restoring the user selected
thread and frame.
In the case were we want to temporarily execute a command
to a thread other than the current thread, we switch the internal
thread to the thread we want with switch_to_thread/context_switch_to,
and invoke execute_command_internal. Unlike execute_command, this
doesn't re-switch the internal thread to the external thread.
There are examples in thread_apply_all_command, and
thread_apply_command. This can be used to implement the
"--thread" switch in MI commands.
Oh, and here's what happens when the current thread exits in
non-stop mode:
(gdb) b 81
Breakpoint 1 at 0x80485fd: file threads.c, line 81.
(gdb) r
Starting program: /home/pedro/gdb/tests/threads32
[Thread debugging using libthread_db enabled]
[New Thread 0xf7e22b90 (LWP 29573)]
[New Thread 0xf7621b90 (LWP 29574)]
Breakpoint 1, thread_function1 (arg=0x1) at threads.c:81
81 usleep (1); /* Loop increment. */
Thread 3 [Thread 0xf7621b90 (LWP 29574)] stopped
(gdb) thread 3
[Switching to thread 3 (Thread 0xf7621b90 (LWP 29574))]#0 thread_function1
(arg=0x1) at threads.c:81
81 usleep (1); /* Loop increment. */
(gdb) info threads
* 3 Thread 0xf7621b90 (LWP 29574) thread_function1 (arg=0x1) at threads.c:81
2 Thread 0xf7e22b90 (LWP 29573) (running)
1 Thread 0xf7e236b0 (LWP 29570) (running)
(gdb) p *myp=0
$1 = 0
(gdb) c&
Continuing.
(gdb) [Thread 0xf7621b90 (LWP 29574) exited]
info threads
2 Thread 0xf7e22b90 (LWP 29573) (running)
1 Thread 0xf7e236b0 (LWP 29570) (running)
No selected thread. See `help thread'
(gdb) p a
Cannot execute this command without a selected thread. See `help thread'
(gdb) thread 2
[Switching to thread 2 (Thread 0xf7e22b90 (LWP 29573))](running)
(gdb) info threads
* 2 Thread 0xf7e22b90 (LWP 29573) (running)
1 Thread 0xf7e236b0 (LWP 29570) (running)
(gdb) p a
Cannot execute this command while the target is running.
(gdb) interrupt
(gdb)
Program received signal SIGINT, Interrupt.
0xffffe410 in __kernel_vsyscall ()
info threads
* 2 Thread 0xf7e22b90 (LWP 29573) 0xffffe410 in __kernel_vsyscall ()
1 Thread 0xf7e236b0 (LWP 29570) (running)
(gdb) p a
No symbol "a" in current context.
(gdb)
--
Pedro Alves
[-- Attachment #2: 010-dont_switch_threads_and_exited_threads.diff --]
[-- Type: text/x-diff, Size: 61228 bytes --]
2008-06-15 Pedro Alves <pedro@codesourcery.com>
Split external/internal threads.
* thread.c (enum thread_state): New.
(thread_state main_thread_running): Delete, in favor of...
(thread_state main_thread_state): ... this. Update throughout.
(clear_thread_inferior_resources): New, split from free_thread.
(free_thread): Call clear_thread_inferior_resources.
(init_thread_list): Set main thread to stopped state. Clear the
selected thread and frame.
(add_thread_silent): Take care of PTID reuses.
(delete_thread): If the selected thread is being deleted, clear
the selected thread. If deleting inferior_ptid, mark it as
exited, and keep it in the list.
(iterate_over_threads): Make it safe to delete threads while
iterating over them.
(do_captured_list_thread_ids): Don't account for exited threads.
(thread_alive): Check for the THREAD_EXITED state, and don't set
ptid to -1 on exited threads.
(set_running): Update to account for extra possible states.
(is_thread_state): New.
(is_stopped, is_exited): New.
(is_running): Implement in terms of is_thread_state.
(any_running): Update.
(print_thread_info): Don't do thread and frame restoring here.
Account for exited threads. Consider user_selected_ptid the
selected thread, not inferior_ptid. If there's no selected
thread, warn the user.
(switch_to_thread): Don't read from a thread that has gone.
(struct current_thread_cleanup): Remove the is_running check.
(thread_apply_all_command): Don't do thread and frame restoring
here. Prune threads. Call execute_command_internal instead of
execute_command.
(thread_command): If there's no selected thread, say so.
(do_captured_thread_select): Check for a running thread. Prune
threads.
(_initialize_thread): Make "info threads", "thread", "thread
apply", and "thread apply all" appliable without a selected thread.
* gdbthread.h (struct thread_info): Replace running_ by state_.
(is_exited, is_stopped): Declare.
* inferior.h (proceed_to_inferior_function_call): Declare.
(user_selected_ptid): Declare.
(store_selected_thread_and_frame)
(restore_selected_thread_and_frame)
(select_thread_if_no_thread_selected): Declare.
* infrun.c (previous_inferior_ptid): Delete.
(proceed_to_inferior_function_call): New.
(proceed): Don't set previous_inferior_ptid.
(init_wait_for_inferior): Clear selected thread and frame.
(fetch_inferior_event): In non-stop mode, restore the user
selected frame after handling an event.
(user_selected_ptid, user_selected_frame_level, frame_id)
(user_selected_frame): New.
(store_selected_thread_and_frame)
(restore_selected_thread_and_frame)
(select_thread_if_no_thread_selected): New.
(normal_stop): Store selected thread and frame. In non-stop mode,
don't switch the selected thread to the thread that had an event.
* top.h (execute_command_internal): Declare.
* gdbcmd.h (execute_command_internal): Declare.
* top.c (do_restore_selected_thread_and_frame_cleanup): New.
(execute_command): Rename to ...
(execute_command_1): ... this. Add INTERNAL parameter. If not
INTERNAL switch to the user selected thread, before and after
running the command. Check for non no-selected-thread-ok
commands.
(execute_command_internal, execute_command): New as wrappers to
execute_command_1.
* stack.c (select_frame_command, up_silently_base)
(down_silently_base): Record new user selected frame.
* infcmd.c (run_command_1): Select thread.
(proceed_thread_callback): Check if !is_stopped instead of
is_running. Remove thread and frame restoring here.
(attach_command_post_wait): Select thread.
* infcall.c (call_function_by_hand): Set and restore
proceed_to_inferior_function_call.
* target.c (generic_mourn_inferior): Clear the selected thread and
frame.
* corelow.c (core_open): Select thread and frame.
* linux-nat.c (struct saved_ptids, threads_to_delete)
(record_dead_thread, prune_lwps): Delete.
(exit_lwp): Unconditionally delete thread.
(linux_nat_resume): Remove prune_lwps call.
* linux-fork.c (fork_load_infrun_state): Store selected thread and
frame.
* cli/cli-decode.h (CMD_NO_SELECTED_THREAD_OK): New.
(set_cmd_no_selected_thread_ok, get_cmd_no_selected_thread_ok):
Declare.
* cli/cli-decode.c (set_cmd_no_selected_thread_ok)
(get_cmd_no_selected_thread_ok): New.
* cli/cli-cmds.c: Set "pwd", "help", "info", "show" as
no-selected-thread ok.
* mi/mi-main.c (mi_cmd_target_download, mi_cmd_target_select): Use
execute_command_internal instead of execute_command.
(do_restore_selected_thread_and_frame_cleanup): New.
(mi_cmd_execute): Restore the user selected thread and frame
before and after running a command. Only "thread-info",
"thread-list-ids", and "thread-select" if there's no selected
thread.
(mi_execute_cli_command, mi_execute_async_cli_command): Use
execute_command_internal instead of execute_command.
* mi/mi-cmd-env.c (env_execute_cli_command): Call
execute_command_internal instead of execute_command.
---
gdb/cli/cli-cmds.c | 7
gdb/cli/cli-decode.c | 12 +
gdb/cli/cli-decode.h | 11 +
gdb/corelow.c | 1
gdb/gdbcmd.h | 2
gdb/gdbthread.h | 22 ++-
gdb/infcall.c | 10 -
gdb/infcmd.c | 40 ++---
gdb/inferior.h | 24 +++
gdb/infrun.c | 225 +++++++++++++++++++++++++++++--
gdb/linux-fork.c | 3
gdb/linux-nat.c | 48 ------
gdb/mi/mi-cmd-env.c | 2
gdb/mi/mi-main.c | 50 ++++++-
gdb/stack.c | 13 +
gdb/target.c | 1
gdb/thread.c | 364 +++++++++++++++++++++++++++++++++------------------
gdb/top.c | 61 +++++++-
gdb/top.h | 5
19 files changed, 673 insertions(+), 228 deletions(-)
Index: src/gdb/thread.c
===================================================================
--- src.orig/gdb/thread.c 2008-06-15 20:56:55.000000000 +0100
+++ src/gdb/thread.c 2008-06-15 20:57:46.000000000 +0100
@@ -65,7 +65,16 @@ static void thread_apply_command (char *
static void restore_current_thread (ptid_t);
static void prune_threads (void);
-static int main_thread_running = 0;
+/* Frontend view of the thread state. Possible extensions: stepping,
+ finishing, until(ling),... */
+enum thread_state
+{
+ THREAD_STOPPED,
+ THREAD_RUNNING,
+ THREAD_EXITED,
+};
+
+static enum thread_state main_thread_state = 0;
static int main_thread_executing = 0;
void
@@ -86,16 +95,25 @@ delete_step_resume_breakpoint (void *arg
}
static void
-free_thread (struct thread_info *tp)
+clear_thread_inferior_resources (struct thread_info *tp)
{
/* NOTE: this will take care of any left-over step_resume breakpoints,
but not any user-specified thread-specific breakpoints. We can not
delete the breakpoint straight-off, because the inferior might not
be stopped at the moment. */
if (tp->step_resume_breakpoint)
- tp->step_resume_breakpoint->disposition = disp_del_at_next_stop;
+ {
+ tp->step_resume_breakpoint->disposition = disp_del_at_next_stop;
+ tp->step_resume_breakpoint = NULL;
+ }
bpstat_clear (&tp->stop_bpstat);
+}
+
+static void
+free_thread (struct thread_info *tp)
+{
+ clear_thread_inferior_resources (tp);
/* FIXME: do I ever need to call the back-end to give it a
chance at this private data before deleting the thread? */
@@ -111,9 +129,11 @@ init_thread_list (void)
struct thread_info *tp, *tpnext;
highest_thread_num = 0;
- main_thread_running = 0;
+ main_thread_state = THREAD_STOPPED;
main_thread_executing = 0;
+ store_selected_thread_and_frame (null_ptid, NULL);
+
if (!thread_list)
return;
@@ -131,6 +151,46 @@ add_thread_silent (ptid_t ptid)
{
struct thread_info *tp;
+ tp = find_thread_pid (ptid);
+ if (tp)
+ /* Found an old thread with the same id. It has to be dead,
+ otherwise we wouldn't see a new thread with the same id. The
+ OS is reusing this id --- delete it, and recreate a new
+ one. */
+ {
+ /* In addition to deleting the thread, if this is the current
+ thread, then we need to also get rid of the current infrun
+ context, and take care that delete_thread doesn't really
+ delete the thread if it is inferior_ptid. Create an invalid
+ thread in the list, context switch to it, delete the original
+ thread, create a new one, and switch to it. */
+
+ if (ptid_equal (inferior_ptid, ptid))
+ {
+ tp = xmalloc (sizeof (*tp));
+ memset (tp, 0, sizeof (*tp));
+ tp->ptid = minus_one_ptid;
+ tp->num = ++highest_thread_num;
+ tp->next = thread_list;
+ thread_list = tp;
+ context_switch_to (minus_one_ptid);
+
+ /* Now we can delete it. */
+ delete_thread (ptid);
+
+ /* Since the context is already set to this new thread,
+ reset it's ptid, and reswitch inferior_ptid to it. */
+ tp->ptid = ptid;
+ switch_to_thread (ptid);
+
+ /* Done. */
+ return tp;
+ }
+ else
+ /* Just go ahead and delete it. */
+ delete_thread (ptid);
+ }
+
tp = (struct thread_info *) xmalloc (sizeof (*tp));
memset (tp, 0, sizeof (*tp));
tp->ptid = ptid;
@@ -163,6 +223,10 @@ add_thread (ptid_t ptid)
return add_thread_with_info (ptid, NULL);
}
+/* Delete thread PTID and notify of thread exit. If this is
+ inferior_ptid, don't actually delete it, but tag it as exited and
+ do the notification. If PTID is the user selected thread, clear
+ it. */
void
delete_thread (ptid_t ptid)
{
@@ -177,12 +241,36 @@ delete_thread (ptid_t ptid)
if (!tp)
return;
+ if (ptid_equal (tp->ptid, user_selected_ptid))
+ /* User selected thread no longer exists. */
+ store_selected_thread_and_frame (null_ptid, NULL);
+
+ if (ptid_equal (tp->ptid, inferior_ptid))
+ {
+ /* Can't delete yet. Mark it as exited, and notify it. */
+ if (tp->state_ != THREAD_EXITED)
+ {
+ observer_notify_thread_exit (tp);
+
+ /* Tag it as exited. */
+ tp->state_ = THREAD_EXITED;
+
+ /* Clear breakpoints, etc. associated with this thread. */
+ clear_thread_inferior_resources (tp);
+ }
+
+ /* Will be really deleted some other time. */
+ return;
+ }
+
if (tpprev)
tpprev->next = tp->next;
else
thread_list = tp->next;
- observer_notify_thread_exit (tp);
+ /* Notify thread exit, but only if we haven't already. */
+ if (tp->state_ != THREAD_EXITED)
+ observer_notify_thread_exit (tp);
free_thread (tp);
}
@@ -230,11 +318,14 @@ struct thread_info *
iterate_over_threads (int (*callback) (struct thread_info *, void *),
void *data)
{
- struct thread_info *tp;
+ struct thread_info *tp, *next;
- for (tp = thread_list; tp; tp = tp->next)
- if ((*callback) (tp, data))
- return tp;
+ for (tp = thread_list; tp; tp = next)
+ {
+ next = tp->next;
+ if ((*callback) (tp, data))
+ return tp;
+ }
return NULL;
}
@@ -301,6 +392,8 @@ do_captured_list_thread_ids (struct ui_o
for (tp = thread_list; tp; tp = tp->next)
{
+ if (tp->state_ == THREAD_EXITED)
+ continue;
num++;
ui_out_field_int (uiout, "thread-id", tp->num);
}
@@ -451,13 +544,10 @@ save_infrun_state (ptid_t ptid,
static int
thread_alive (struct thread_info *tp)
{
- if (PIDGET (tp->ptid) == -1)
+ if (tp->state_ == THREAD_EXITED)
return 0;
if (!target_thread_alive (tp->ptid))
- {
- tp->ptid = pid_to_ptid (-1); /* Mark it as dead */
- return 0;
- }
+ return 0;
return 1;
}
@@ -489,9 +579,11 @@ set_running (ptid_t ptid, int running)
the main thread is always present in the thread list. If it's
not, the first call to context_switch will mess up GDB internal
state. */
- if (running && !main_thread_running && !suppress_resume_observer)
+ if (running
+ && main_thread_state != THREAD_RUNNING
+ && !suppress_resume_observer)
observer_notify_target_resumed (ptid);
- main_thread_running = running;
+ main_thread_state = running ? THREAD_RUNNING : THREAD_STOPPED;
return;
}
@@ -503,25 +595,31 @@ set_running (ptid_t ptid, int running)
int any_started = 0;
for (tp = thread_list; tp; tp = tp->next)
{
- if (running && !tp->running_)
- any_started = 1;
- tp->running_ = running;
+ if (tp->state_ == THREAD_EXITED)
+ continue;
+ if (running && tp->state_ == THREAD_STOPPED)
+ any_started = 1;
+ tp->state_ = running ? THREAD_RUNNING : THREAD_STOPPED;
}
if (any_started && !suppress_resume_observer)
observer_notify_target_resumed (ptid);
}
else
{
+ int started = 0;
tp = find_thread_pid (ptid);
gdb_assert (tp);
- if (running && !tp->running_ && !suppress_resume_observer)
- observer_notify_target_resumed (ptid);
- tp->running_ = running;
- }
+ gdb_assert (tp->state_ != THREAD_EXITED);
+ if (running && tp->state_ == THREAD_STOPPED)
+ started = 1;
+ tp->state_ = running ? THREAD_RUNNING : THREAD_STOPPED;
+ if (started && !suppress_resume_observer)
+ observer_notify_target_resumed (ptid);
+ }
}
-int
-is_running (ptid_t ptid)
+static int
+is_thread_state (ptid_t ptid, enum thread_state state)
{
struct thread_info *tp;
@@ -529,11 +627,41 @@ is_running (ptid_t ptid)
return 0;
if (!thread_list)
- return main_thread_running;
+ return main_thread_state == state;
tp = find_thread_pid (ptid);
gdb_assert (tp);
- return tp->running_;
+ return tp->state_ == state;
+}
+
+int
+is_stopped (ptid_t ptid)
+{
+ /* Without execution, this property is always true. */
+ if (!target_has_execution)
+ return 1;
+
+ return is_thread_state (ptid, THREAD_STOPPED);
+}
+
+int
+is_exited (ptid_t ptid)
+{
+ /* Without execution, this property is always false. */
+ if (!target_has_execution)
+ return 0;
+
+ return is_thread_state (ptid, THREAD_EXITED);
+}
+
+int
+is_running (ptid_t ptid)
+{
+ /* Without execution, this property is always false. */
+ if (!target_has_execution)
+ return 0;
+
+ return is_thread_state (ptid, THREAD_RUNNING);
}
int
@@ -545,10 +673,10 @@ any_running (void)
return 0;
if (!thread_list)
- return main_thread_running;
+ return main_thread_state == THREAD_RUNNING;
for (tp = thread_list; tp; tp = tp->next)
- if (tp->running_)
+ if (tp->state_ == THREAD_RUNNING)
return 1;
return 0;
@@ -600,42 +728,36 @@ set_executing (ptid_t ptid, int executin
/* Prints the list of threads and their details on UIOUT.
This is a version of 'info_thread_command' suitable for
use from MI.
- If REQESTED_THREAD is not -1, it's the GDB id of the thread
+ If REQUESTED_THREAD is not -1, it's the GDB id of the thread
that should be printed. Otherwise, all threads are
printed. */
void
print_thread_info (struct ui_out *uiout, int requested_thread)
{
struct thread_info *tp;
- ptid_t current_ptid;
- struct frame_info *cur_frame;
struct cleanup *old_chain;
- struct frame_id saved_frame_id;
char *extra_info;
int current_thread = -1;
- /* 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);
- make_cleanup_ui_out_list_begin_end (uiout, "threads");
-
prune_threads ();
target_find_new_threads ();
- current_ptid = inferior_ptid;
+
+ old_chain = make_cleanup_ui_out_list_begin_end (uiout, "threads");
+
for (tp = thread_list; tp; tp = tp->next)
{
struct cleanup *chain2;
+ ptid_t ptid;
+
+ if (tp->state_ == THREAD_EXITED)
+ continue;
if (requested_thread != -1 && tp->num != requested_thread)
continue;
chain2 = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
- if (ptid_equal (tp->ptid, current_ptid))
+ if (ptid_equal (tp->ptid, user_selected_ptid))
{
current_thread = tp->num;
ui_out_text (uiout, "* ");
@@ -647,15 +769,19 @@ print_thread_info (struct ui_out *uiout,
ui_out_text (uiout, " ");
ui_out_field_string (uiout, "target-id", target_tid_to_str (tp->ptid));
- extra_info = target_extra_thread_info (tp);
- if (extra_info)
+ if (tp->state_ != THREAD_EXITED)
{
- ui_out_text (uiout, " (");
- ui_out_field_string (uiout, "details", extra_info);
- ui_out_text (uiout, ")");
+ extra_info = target_extra_thread_info (tp);
+ if (extra_info)
+ {
+ ui_out_text (uiout, " (");
+ ui_out_field_string (uiout, "details", extra_info);
+ ui_out_text (uiout, ")");
+ }
+ ui_out_text (uiout, " ");
}
- ui_out_text (uiout, " ");
- if (tp->running_)
+
+ if (tp->state_ == THREAD_RUNNING)
ui_out_text (uiout, "(running)\n");
else
{
@@ -677,24 +803,14 @@ print_thread_info (struct ui_out *uiout,
if (requested_thread == -1)
{
- gdb_assert (current_thread != -1 || !thread_list);
+ gdb_assert (current_thread != -1
+ || !thread_list
+ || ptid_equal (user_selected_ptid, null_ptid));
if (current_thread != -1 && ui_out_is_mi_like_p (uiout))
ui_out_field_int (uiout, "current-thread-id", current_thread);
- }
- if (is_running (inferior_ptid))
- return;
-
- /* If case we were not able to find the original frame, print the
- new selected frame. */
- if (frame_find_by_id (saved_frame_id) == NULL)
- {
- warning (_("Couldn't restore frame in current thread, at frame 0"));
- /* 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. */
- if (!ui_out_is_mi_like_p (uiout))
- print_stack_frame (get_selected_frame (NULL), 0, LOCATION);
+ if (current_thread == -1 && thread_list)
+ ui_out_text (uiout, "\nNo selected thread. See `help thread'\n");
}
}
@@ -724,7 +840,10 @@ switch_to_thread (ptid_t ptid)
reinit_frame_cache ();
registers_changed ();
- if (!is_executing (ptid))
+ /* We don't check for is_stopped, because we're called at times
+ while in the TARGET_RUNNING state, e.g., while handling an
+ internal event. */
+ if (!is_exited (ptid) && !is_executing (ptid))
stop_pc = read_pc ();
else
stop_pc = ~(CORE_ADDR) 0;
@@ -753,6 +872,9 @@ restore_selected_frame (struct frame_id
}
}
+/* Only safe to use this cleanup on a stopped thread, and
+ inferior_ptid isn't ran before running the cleanup. */
+
struct current_thread_cleanup
{
ptid_t inferior_ptid;
@@ -765,11 +887,7 @@ do_restore_current_thread_cleanup (void
struct current_thread_cleanup *old = arg;
restore_current_thread (old->inferior_ptid);
- /* A command like 'thread apply all $exec_command&' may change the
- running state of the originally selected thread, so we have to
- recheck it here. */
- if (!is_running (old->inferior_ptid))
- restore_selected_frame (old->selected_frame_id);
+ restore_selected_frame (old->selected_frame_id);
xfree (old);
}
@@ -799,23 +917,11 @@ thread_apply_all_command (char *cmd, int
struct thread_info *tp;
struct cleanup *old_chain = make_cleanup (null_cleanup, 0);
char *saved_cmd;
- struct frame_id saved_frame_id;
- ptid_t current_ptid;
- int thread_has_changed = 0;
if (cmd == NULL || *cmd == '\000')
error (_("Please specify a command following the thread ID list"));
-
- current_ptid = inferior_ptid;
- if (!is_running (inferior_ptid))
- saved_frame_id = get_frame_id (get_selected_frame (NULL));
- else
- saved_frame_id = null_frame_id;
- make_cleanup_restore_current_thread (inferior_ptid, saved_frame_id);
-
- /* It is safe to update the thread list now, before
- traversing it for "thread apply all". MVS */
+ prune_threads ();
target_find_new_threads ();
/* Save a copy of the command in case it is clobbered by
@@ -832,17 +938,17 @@ thread_apply_all_command (char *cmd, int
printf_filtered (_("\nThread %d (%s):\n"),
tp->num, target_tid_to_str (inferior_ptid));
- execute_command (cmd, from_tty);
+ /* 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 */
}
- if (!ptid_equal (current_ptid, inferior_ptid))
- thread_has_changed = 1;
-
do_cleanups (old_chain);
- /* Print stack frame only if we changed thread. */
- if (thread_has_changed && !is_running (inferior_ptid))
- print_stack_frame (get_current_frame (), 1, SRC_LINE);
}
static void
@@ -851,11 +957,7 @@ thread_apply_command (char *tidlist, int
char *cmd;
char *p;
struct cleanup *old_chain;
- struct cleanup *saved_cmd_cleanup_chain;
char *saved_cmd;
- struct frame_id saved_frame_id;
- ptid_t current_ptid;
- int thread_has_changed = 0;
if (tidlist == NULL || *tidlist == '\000')
error (_("Please specify a thread ID list"));
@@ -865,18 +967,10 @@ thread_apply_command (char *tidlist, int
if (*cmd == '\000')
error (_("Please specify a command following the thread ID list"));
- current_ptid = inferior_ptid;
-
- 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);
-
/* Save a copy of the command in case it is clobbered by
execute_command */
saved_cmd = xstrdup (cmd);
- saved_cmd_cleanup_chain = make_cleanup (xfree, (void *) saved_cmd);
+ old_chain = make_cleanup (xfree, saved_cmd);
while (tidlist < cmd)
{
struct thread_info *tp;
@@ -920,20 +1014,19 @@ thread_apply_command (char *tidlist, int
switch_to_thread (tp->ptid);
printf_filtered (_("\nThread %d (%s):\n"), tp->num,
target_tid_to_str (inferior_ptid));
- execute_command (cmd, from_tty);
+ /* 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 */
}
}
}
- if (!ptid_equal (current_ptid, inferior_ptid))
- thread_has_changed = 1;
-
- do_cleanups (saved_cmd_cleanup_chain);
do_cleanups (old_chain);
- /* Print stack frame only if we changed thread. */
- if (thread_has_changed)
- print_stack_frame (get_current_frame (), 1, SRC_LINE);
}
/* Switch to the specified thread. Will dispatch off to thread_apply_command
@@ -944,11 +1037,17 @@ thread_command (char *tidstr, int from_t
{
if (!tidstr)
{
- /* Don't generate an error, just say which thread is current. */
if (target_has_stack)
- printf_filtered (_("[Current thread is %d (%s)]\n"),
- pid_to_thread_id (inferior_ptid),
- target_tid_to_str (inferior_ptid));
+ {
+ /* Don't generate an error, just say which thread is
+ current. */
+ if (ptid_equal (user_selected_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));
+ }
else
error (_("No stack."));
return;
@@ -996,10 +1095,23 @@ do_captured_thread_select (struct ui_out
ui_out_text (uiout, target_tid_to_str (inferior_ptid));
ui_out_text (uiout, ")]");
- if (!tp->running_)
- print_stack_frame (get_selected_frame (NULL), 1, SRC_AND_LOC);
+ /* Note that we can't reach this with an exited thread, due to the
+ thread_alive check above. */
+ if (tp->state_ == THREAD_RUNNING)
+ {
+ ui_out_text (uiout, "(running)\n");
+ store_selected_thread_and_frame (tp->ptid, NULL);
+ }
else
- ui_out_text (uiout, "(running)\n");
+ {
+ struct frame_info *frame = get_selected_frame (NULL);
+ store_selected_thread_and_frame (tp->ptid, frame);
+ print_stack_frame (frame, 1, SRC_AND_LOC);
+ }
+
+ /* Since the current thread may have changed, see if there is any
+ exited thread we can now delete. */
+ prune_threads ();
return GDB_RC_OK;
}
@@ -1025,19 +1137,25 @@ _initialize_thread (void)
c = add_info ("threads", info_threads_command,
_("IDs of currently known threads."));
set_cmd_async_ok (c);
+ set_cmd_no_selected_thread_ok (c);
c = add_prefix_cmd ("thread", class_run, thread_command, _("\
Use this command to switch between threads.\n\
The new thread ID must be currently known."),
&thread_cmd_list, "thread ", 1, &cmdlist);
set_cmd_async_ok (c);
+ set_cmd_no_selected_thread_ok (c);
- add_prefix_cmd ("apply", class_run, thread_apply_command,
+ c = add_prefix_cmd ("apply", class_run, thread_apply_command,
_("Apply a command to a list of threads."),
&thread_apply_list, "thread apply ", 1, &thread_cmd_list);
+ set_cmd_async_ok (c);
+ set_cmd_no_selected_thread_ok (c);
- add_cmd ("all", class_run, thread_apply_all_command,
- _("Apply a command to all threads."), &thread_apply_list);
+ c = add_cmd ("all", class_run, thread_apply_all_command,
+ _("Apply a command to all threads."), &thread_apply_list);
+ set_cmd_async_ok (c);
+ set_cmd_no_selected_thread_ok (c);
if (!xdb_commands)
add_com_alias ("t", "thread", class_run, 1);
Index: src/gdb/gdbthread.h
===================================================================
--- src.orig/gdb/gdbthread.h 2008-06-15 20:56:55.000000000 +0100
+++ src/gdb/gdbthread.h 2008-06-15 20:57:46.000000000 +0100
@@ -45,15 +45,16 @@ struct thread_info
use is_executing instead. */
int executing_;
- /* Frontend view of the running state. Note that this is different
- from EXECUTING. When the thread is stopped internally while
- handling an internal event, like a software single-step
- breakpoint, executing will be false, but running will still be
- true. As a possible future extension, this could turn into
- enum { stopped, stepping, finishing, until(ling), ... } */
+ /* Frontend view of the thread state. Note that the RUNNING/STOPPED
+ states are different from EXECUTING. When the thread is stopped
+ internally while handling an internal event, like a software
+ single-step breakpoint, EXECUTING will be false, but running will
+ still be true. As a possible future extension, this could turn
+ into enum { stopped, exited, stepping, finishing, until(ling),
+ running ... } */
/* This field is internal to thread.c. Never access it directly,
use is_running instead. */
- int running_;
+ int state_;
/* State from wait_for_inferior */
CORE_ADDR prev_pc;
@@ -200,6 +201,13 @@ extern int is_running (ptid_t ptid);
/* Reports if any thread is known to be running right now. */
extern int any_running (void);
+/* Is this thread listed, but known to have exited? We keep it listed
+ (but not visible) until it's safe to delete. */
+extern int is_exited (ptid_t ptid);
+
+/* Is this thread stopped? */
+extern int is_stopped (ptid_t ptid);
+
/* Marks thread PTID as executing, or as stopped.
If PIDGET (PTID) is -1, marks all threads. */
extern void set_executing (ptid_t ptid, int executing);
Index: src/gdb/inferior.h
===================================================================
--- src.orig/gdb/inferior.h 2008-06-15 20:56:56.000000000 +0100
+++ src/gdb/inferior.h 2008-06-15 20:57:46.000000000 +0100
@@ -388,6 +388,11 @@ extern int proceed_to_finish;
extern struct regcache *stop_registers;
+/* Nonzero if proceed is being used while calling an inferior
+ function. */
+
+extern int proceed_to_inferior_function_call;
+
/* Nonzero if the child process in inferior_ptid was attached rather
than forked. */
@@ -400,6 +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;
+
+/* Records PTID and FRAME as user/frontend/external selected thread
+ and frame. */
+extern void store_selected_thread_and_frame (ptid_t ptid,
+ struct frame_info *frame);
+
+/* Make the internal thread and frame point to the external thread and
+ frame. */
+extern void restore_selected_thread_and_frame (void);
+
+/* If no external thread is selected, make it point to the internal
+ thread. */
+extern void select_thread_if_no_thread_selected (void);
/* When set, normal_stop will not call the normal_stop observer. */
extern int suppress_stop_observer;
Index: src/gdb/infrun.c
===================================================================
--- src.orig/gdb/infrun.c 2008-06-15 20:56:56.000000000 +0100
+++ src/gdb/infrun.c 2008-06-15 20:57:46.000000000 +0100
@@ -45,6 +45,7 @@
#include "language.h"
#include "solib.h"
#include "main.h"
+#include "interps.h"
#include "gdb_assert.h"
#include "mi/mi-common.h"
@@ -97,12 +98,6 @@ show_step_stop_if_no_debug (struct ui_fi
int sync_execution = 0;
-/* wait_for_inferior and normal_stop use this to notify the user
- when the inferior stopped in a different thread than it had been
- running in. */
-
-static ptid_t previous_inferior_ptid;
-
int debug_displaced = 0;
static void
show_debug_displaced (struct ui_file *file, int from_tty,
@@ -275,6 +270,11 @@ int proceed_to_finish;
struct regcache *stop_registers;
+/* Nonzero if proceed is being used while calling an inferior
+ function. */
+
+int proceed_to_inferior_function_call;
+
/* Nonzero after stop if current stack frame should be printed. */
static int stop_print_frame;
@@ -1301,9 +1301,6 @@ proceed (CORE_ADDR addr, enum target_sig
/* Fill in with reasonable starting values. */
init_thread_stepping_state (tss);
- /* We'll update this if & when we switch to a new thread. */
- previous_inferior_ptid = inferior_ptid;
-
/* Reset to normal state. */
init_infwait_state ();
@@ -1380,7 +1377,7 @@ init_wait_for_inferior (void)
target_last_wait_ptid = minus_one_ptid;
init_thread_stepping_state (tss);
- previous_inferior_ptid = null_ptid;
+ store_selected_thread_and_frame (null_ptid, NULL);
init_infwait_state ();
displaced_step_clear ();
@@ -1565,6 +1562,16 @@ fetch_inferior_event (void *client_data)
else
inferior_event_handler (INF_EXEC_COMPLETE, NULL);
}
+
+ /* In non-stop mode, the user/frontend should not notice a thread
+ switch due to internal events. Revert to the user selected
+ thread and frame. */
+ if (non_stop
+ && target_has_execution
+ && ecs->ws.kind != TARGET_WAITKIND_IGNORE
+ && ecs->ws.kind != TARGET_WAITKIND_EXITED
+ && ecs->ws.kind != TARGET_WAITKIND_SIGNALLED)
+ restore_selected_thread_and_frame ();
}
/* Prepare an execution control state for looping through a
@@ -1681,6 +1688,174 @@ context_switch_to (ptid_t ptid)
return current_ptid;
}
+/* These point 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 (inferior_ptid) may change). */
+
+/* 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;
+
+/* 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)
+{
+ /* 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. */
+ gdb_assert (frame == NULL || ptid_equal (inferior_ptid, ptid));
+
+ /* If clearing the selected thread, we should clear the frame
+ too. */
+ 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);
+
+ 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);
+ fprintf_unfiltered (gdb_stdlog, ", #%d.\n",
+ user_selected_frame_level);
+ }
+}
+
+/* Restore the external selected thread. */
+void
+restore_selected_thread_and_frame (void)
+{
+ if (!target_has_registers || !target_has_stack || !target_has_memory)
+ return;
+
+ /* 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 (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);
+ fprintf_unfiltered (gdb_stdlog, ", #%d\n",
+ user_selected_frame_level);
+ }
+
+ if (non_stop)
+ context_switch_to (user_selected_ptid);
+ else
+ switch_to_thread (user_selected_ptid);
+
+ if (is_stopped (user_selected_ptid))
+ {
+ struct frame_info *frame;
+ int count;
+
+ gdb_assert (user_selected_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;
+ frame = find_relative_frame (get_current_frame (), &count);
+ if (count == 0
+ && frame != NULL
+ /* Either the frame ids match, of they're both invalid.
+ The later case is not failsafe, but since it's highly
+ 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)
+ /* 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))))
+ {
+ /* Cool, all is fine. */
+ select_frame (frame);
+ if (debug_infrun)
+ fprintf_unfiltered (gdb_stdlog, "infrun.c: success (level)\n");
+ return;
+ }
+
+ frame = frame_find_by_id (user_selected_frame);
+ if (frame != NULL)
+ {
+ /* Cool, refound it. */
+ select_frame (frame);
+
+ /* The relative level changed, otherwise we would have
+ found it above. */
+ user_selected_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);
+ return;
+ }
+
+ if (debug_infrun)
+ fprintf_unfiltered (gdb_stdlog, "infrun.c: fail");
+
+ /* Nothing else to do, the frame layout really changed.
+ Tell the user. */
+ if (!ui_out_is_mi_like_p (interp_ui_out (top_level_interpreter ())))
+ {
+ warning (_("\
+Couldn't restore frame #%d in current thread, at reparsed frame #0\n"),
+ user_selected_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. */
+ print_stack_frame (frame, 0, LOCATION);
+ }
+
+ frame = get_selected_frame (NULL);
+ user_selected_frame = get_frame_id (frame);
+ user_selected_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);
+ fprintf_unfiltered (gdb_stdlog, ", #%d.\n",
+ user_selected_frame_level);
+ }
+ }
+ }
+}
+
+void
+select_thread_if_no_thread_selected (void)
+{
+ if (ptid_equal (user_selected_ptid, null_ptid)
+ && !ptid_equal (inferior_ptid, null_ptid))
+ {
+ if (is_stopped (inferior_ptid))
+ store_selected_thread_and_frame (inferior_ptid,
+ get_selected_frame (NULL));
+ else
+ store_selected_thread_and_frame (inferior_ptid,
+ NULL);
+ }
+}
+
static void
adjust_pc_after_break (struct execution_control_state *ecs)
{
@@ -2199,7 +2374,7 @@ targets should add new threads to the th
switch_to_thread (deferred_step_ptid);
deferred_step_ptid = null_ptid;
/* Suppress spurious "Switching to ..." message. */
- previous_inferior_ptid = inferior_ptid;
+ store_selected_thread_and_frame (inferior_ptid, NULL);
resume (1, TARGET_SIGNAL_0);
prepare_to_wait (ecs);
@@ -3701,6 +3876,11 @@ normal_stop (void)
get_last_target_status (&last_ptid, &last);
+ /* 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
+ apply to thread x, but GDB switches to thread y before the user
+ finishes entering the command. */
+
/* As with the notification of thread events, we want to delay
notifying the user that we've switched thread context until
the inferior actually stops.
@@ -3708,7 +3888,8 @@ normal_stop (void)
There's no point in saying anything if the inferior has exited.
Note that SIGNALLED here means "exited with a signal", not
"received a signal". */
- if (!ptid_equal (previous_inferior_ptid, inferior_ptid)
+ if (!non_stop
+ && !ptid_equal (user_selected_ptid, inferior_ptid)
&& target_has_execution
&& last.kind != TARGET_WAITKIND_SIGNALLED
&& last.kind != TARGET_WAITKIND_EXITED)
@@ -3717,7 +3898,6 @@ normal_stop (void)
printf_filtered (_("[Switching to %s]\n"),
target_pid_to_str (inferior_ptid));
annotate_thread_changed ();
- previous_inferior_ptid = inferior_ptid;
}
/* NOTE drow/2004-01-17: Is this still necessary? */
@@ -3889,15 +4069,28 @@ done:
Delete any breakpoint that is to be deleted at the next stop. */
breakpoint_auto_delete (stop_bpstat);
- if (target_has_execution
- && last.kind != TARGET_WAITKIND_SIGNALLED
- && last.kind != TARGET_WAITKIND_EXITED)
+ if (target_has_execution)
{
+ if ((!stop_stack_dummy || !proceed_to_inferior_function_call)
+ && (!non_stop
+ || ptid_equal (user_selected_ptid, 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
+ routine, unless we're no longer in call_function_by_hand
+ evaluating an expression --- if we are, then this is
+ considered an internal event, and we should not change the
+ user selected frame. */
+ store_selected_thread_and_frame (inferior_ptid,
+ get_selected_frame (NULL));
if (!non_stop)
set_running (pid_to_ptid (-1), 0);
else
set_running (inferior_ptid, 0);
}
+ else
+ /* execution ended; Clear the selected thread and frame. */
+ store_selected_thread_and_frame (null_ptid, NULL);
}
static int
Index: src/gdb/cli/cli-decode.c
===================================================================
--- src.orig/gdb/cli/cli-decode.c 2008-06-15 20:56:56.000000000 +0100
+++ src/gdb/cli/cli-decode.c 2008-06-15 20:57:46.000000000 +0100
@@ -117,6 +117,18 @@ get_cmd_async_ok (struct cmd_list_elemen
return cmd->flags & CMD_ASYNC_OK;
}
+void
+set_cmd_no_selected_thread_ok (struct cmd_list_element *cmd)
+{
+ cmd->flags |= CMD_NO_SELECTED_THREAD_OK;
+}
+
+int
+get_cmd_no_selected_thread_ok (struct cmd_list_element *cmd)
+{
+ return cmd->flags & CMD_NO_SELECTED_THREAD_OK;
+}
+
enum cmd_types
cmd_type (struct cmd_list_element *cmd)
{
Index: src/gdb/cli/cli-decode.h
===================================================================
--- src.orig/gdb/cli/cli-decode.h 2008-06-15 20:56:56.000000000 +0100
+++ src/gdb/cli/cli-decode.h 2008-06-15 20:57:46.000000000 +0100
@@ -51,6 +51,10 @@ cmd_types;
/* This flag is set if the command is allowed during async execution. */
#define CMD_ASYNC_OK 0x8
+/* This flag is set if the command is allowed to run when the target
+ has execution, but there's no selected thread. */
+#define CMD_NO_SELECTED_THREAD_OK 0x10
+
struct cmd_list_element
{
/* Points to next command in this list. */
@@ -253,6 +257,13 @@ extern void set_cmd_async_ok (struct cmd
/* Return true if command is async-ok. */
extern int get_cmd_async_ok (struct cmd_list_element *);
+/* Mark command as ok to call when there is no selected thread. There
+ is no way to disable this once set. */
+extern void set_cmd_no_selected_thread_ok (struct cmd_list_element *);
+
+/* Return true if command is no-selected-thread-ok. */
+extern int get_cmd_no_selected_thread_ok (struct cmd_list_element *);
+
extern struct cmd_list_element *lookup_cmd (char **,
struct cmd_list_element *, char *,
int, int);
Index: src/gdb/top.c
===================================================================
--- src.orig/gdb/top.c 2008-06-15 20:56:56.000000000 +0100
+++ src/gdb/top.c 2008-06-15 20:57:46.000000000 +0100
@@ -359,11 +359,17 @@ do_chdir_cleanup (void *old_dir)
}
#endif
+static void
+do_restore_selected_thread_and_frame_cleanup (void *arg)
+{
+ restore_selected_thread_and_frame ();
+}
+
/* Execute the line P as a command.
Pass FROM_TTY as second argument to the defining function. */
void
-execute_command (char *p, int from_tty)
+execute_command_1 (char *p, int from_tty, int internal)
{
struct cmd_list_element *c;
enum language flang;
@@ -407,6 +413,7 @@ execute_command (char *p, int from_tty)
p++;
if (*p)
{
+ struct cleanup *old_chain = make_cleanup (null_cleanup, NULL);
char *arg;
line = p;
@@ -415,13 +422,35 @@ execute_command (char *p, int from_tty)
c = lookup_cmd (&p, cmdlist, "", 0, 1);
- /* If the target is running, we allow only a limited set of
- commands. */
+ 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);
+ }
+
+ /* If there is no selected thread, we allow only a limited set
+ of commands. */
if (target_can_async_p ()
&& target_has_execution
- && ((!non_stop && any_running ())
- || (non_stop && is_running (inferior_ptid)))
- && !get_cmd_async_ok (c))
+ && (!internal
+ && ptid_equal (user_selected_ptid, null_ptid))
+ && !get_cmd_no_selected_thread_ok (c))
+ error (_("\
+Cannot execute this command without a selected thread. See `help thread'"));
+ /* If the target is running, we allow only a limited set of
+ commands. */
+ else if (target_can_async_p ()
+ && target_has_execution
+ && ((!non_stop && any_running ())
+ || (non_stop && is_running (inferior_ptid)))
+ && !get_cmd_async_ok (c))
error (_("Cannot execute this command while the target is running."));
/* Pass null arg rather than an empty one. */
@@ -467,6 +496,8 @@ execute_command (char *p, int from_tty)
/* If this command has been post-hooked, run the hook last. */
execute_cmd_post_hook (c);
+ /* Restore user selected thread and frame. */
+ do_cleanups (old_chain);
}
/* Tell the user if the language has changed (except first time). */
@@ -498,6 +529,24 @@ execute_command (char *p, int from_tty)
}
}
+/* 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/linux-nat.c
===================================================================
--- src.orig/gdb/linux-nat.c 2008-06-15 20:57:27.000000000 +0100
+++ src/gdb/linux-nat.c 2008-06-15 20:57:46.000000000 +0100
@@ -977,48 +977,12 @@ linux_nat_switch_fork (ptid_t new_ptid)
{
struct lwp_info *lp;
- init_thread_list ();
init_lwp_list ();
lp = add_lwp (new_ptid);
- add_thread_silent (new_ptid);
lp->stopped = 1;
-}
-
-/* Record a PTID for later deletion. */
-
-struct saved_ptids
-{
- ptid_t ptid;
- struct saved_ptids *next;
-};
-static struct saved_ptids *threads_to_delete;
-
-static void
-record_dead_thread (ptid_t ptid)
-{
- struct saved_ptids *p = xmalloc (sizeof (struct saved_ptids));
- p->ptid = ptid;
- p->next = threads_to_delete;
- threads_to_delete = p;
-}
-/* Delete any dead threads which are not the current thread. */
-
-static void
-prune_lwps (void)
-{
- struct saved_ptids **p = &threads_to_delete;
-
- while (*p)
- if (! ptid_equal ((*p)->ptid, inferior_ptid))
- {
- struct saved_ptids *tmp = *p;
- delete_thread (tmp->ptid);
- *p = tmp->next;
- xfree (tmp);
- }
- else
- p = &(*p)->next;
+ init_thread_list ();
+ add_thread_silent (new_ptid);
}
/* Handle the exit of a single thread LP. */
@@ -1033,11 +997,7 @@ exit_lwp (struct lwp_info *lp)
if (print_thread_events)
printf_unfiltered (_("[%s exited]\n"), target_pid_to_str (lp->ptid));
- /* Core GDB cannot deal with us deleting the current thread. */
- if (!ptid_equal (lp->ptid, inferior_ptid))
- delete_thread (lp->ptid);
- else
- record_dead_thread (lp->ptid);
+ delete_thread (lp->ptid);
}
delete_lwp (lp->ptid);
@@ -1542,8 +1502,6 @@ linux_nat_resume (ptid_t ptid, int step,
signo ? strsignal (signo) : "0",
target_pid_to_str (inferior_ptid));
- prune_lwps ();
-
if (target_can_async_p ())
/* Block events while we're here. */
linux_nat_async_events (0);
Index: src/gdb/stack.c
===================================================================
--- src.orig/gdb/stack.c 2008-06-15 20:56:55.000000000 +0100
+++ src/gdb/stack.c 2008-06-15 20:57:46.000000000 +0100
@@ -1697,6 +1697,11 @@ void
select_frame_command (char *level_exp, int from_tty)
{
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));
}
/* The "frame" command. With no argument, print the selected frame
@@ -1734,6 +1739,10 @@ up_silently_base (char *count_exp)
if (count != 0 && count_exp == NULL)
error (_("Initial frame selected; you cannot go up."));
select_frame (frame);
+
+ /* Record user selected frame. */
+ if (ptid_equal (inferior_ptid, user_selected_ptid))
+ store_selected_thread_and_frame (inferior_ptid, frame);
}
static void
@@ -1772,6 +1781,10 @@ 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);
}
static void
Index: src/gdb/infcmd.c
===================================================================
--- src.orig/gdb/infcmd.c 2008-06-15 20:57:45.000000000 +0100
+++ src/gdb/infcmd.c 2008-06-15 20:57:46.000000000 +0100
@@ -569,6 +569,11 @@ run_command_1 (char *args, int from_tty,
target_create_inferior (exec_file, get_inferior_args (),
environ_vector (inferior_environ), from_tty);
+ /* If no selected thread, select the current one. */
+ if (ptid_equal (user_selected_ptid, null_ptid))
+ /* Going to proceed, no need to record a frame. */
+ store_selected_thread_and_frame (inferior_ptid, NULL);
+
/* Pass zero for FROM_TTY, because at this point the "run" command
has done its thing; now we are setting up the running program. */
post_create_inferior (¤t_target, 0);
@@ -611,7 +616,7 @@ start_command (char *args, int from_tty)
static int
proceed_thread_callback (struct thread_info *thread, void *arg)
{
- if (is_running (thread->ptid))
+ if (!is_stopped (thread->ptid))
return 0;
context_switch_to (thread->ptid);
@@ -695,25 +700,9 @@ Can't resume all threads and specify pro
printf_filtered (_("Continuing.\n"));
if (non_stop && all_threads)
- {
- struct cleanup *old_chain;
- struct frame_id saved_frame_id;
-
- /* Don't error out if the current thread is running, because
- there may be other stopped threads. */
-
- /* 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);
- iterate_over_threads (proceed_thread_callback, NULL);
-
- /* Restore selected ptid. */
- do_cleanups (old_chain);
- }
+ /* Don't error out if the current thread is running, because
+ there may be other stopped threads. */
+ iterate_over_threads (proceed_thread_callback, NULL);
else
{
ensure_not_running ();
@@ -1980,12 +1969,21 @@ attach_command_post_wait (char *args, in
target_terminal_inferior ();
if (async_exec)
- proceed ((CORE_ADDR) -1, TARGET_SIGNAL_0, 0);
+ {
+ /* If no selected thread, select the current one. */
+ if (ptid_equal (user_selected_ptid, 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);
+ }
else
{
if (target_can_async_p ())
async_enable_stdin ();
normal_stop ();
+
+ select_thread_if_no_thread_selected ();
+
if (deprecated_attach_hook)
deprecated_attach_hook ();
}
Index: src/gdb/top.h
===================================================================
--- src.orig/gdb/top.h 2008-06-15 20:56:56.000000000 +0100
+++ src/gdb/top.h 2008-06-15 20:57:46.000000000 +0100
@@ -47,8 +47,13 @@ extern int quit_confirm (void);
extern void quit_force (char *, int);
extern void quit_command (char *, int);
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);
Index: src/gdb/corelow.c
===================================================================
--- src.orig/gdb/corelow.c 2008-06-15 20:56:56.000000000 +0100
+++ src/gdb/corelow.c 2008-06-15 20:57:46.000000000 +0100
@@ -376,6 +376,7 @@ core_open (char *filename, int from_tty)
/* Now, set up the frame cache, and print the top of stack. */
reinit_frame_cache ();
print_stack_frame (get_selected_frame (NULL), 1, SRC_AND_LOC);
+ store_selected_thread_and_frame (inferior_ptid, get_selected_frame (NULL));
}
else
{
Index: src/gdb/gdbcmd.h
===================================================================
--- src.orig/gdb/gdbcmd.h 2008-06-15 20:56:55.000000000 +0100
+++ src/gdb/gdbcmd.h 2008-06-15 20:57:46.000000000 +0100
@@ -122,6 +122,8 @@ 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/cli/cli-cmds.c
===================================================================
--- src.orig/gdb/cli/cli-cmds.c 2008-06-15 20:56:56.000000000 +0100
+++ src/gdb/cli/cli-cmds.c 2008-06-15 20:57:46.000000000 +0100
@@ -1245,6 +1245,8 @@ The commands below can be used to select
c = add_com ("pwd", class_files, pwd_command, _("\
Print working directory. This is used for your program as well."));
set_cmd_async_ok (c);
+ set_cmd_no_selected_thread_ok (c);
+
c = add_cmd ("cd", class_files, cd_command, _("\
Set working directory to DIR for debugger and program being debugged.\n\
The change does not take effect for the program being debugged\n\
@@ -1280,11 +1282,12 @@ when GDB is started."), gdbinit);
source_help_text, &cmdlist);
set_cmd_completer (c, filename_completer);
- add_com ("quit", class_support, quit_command, _("Exit gdb."));
+ c = add_com ("quit", class_support, quit_command, _("Exit gdb."));
c = add_com ("help", class_support, help_command,
_("Print list of commands."));
set_cmd_completer (c, command_completer);
set_cmd_async_ok (c);
+ set_cmd_no_selected_thread_ok (c);
add_com_alias ("q", "quit", class_support, 1);
add_com_alias ("h", "help", class_support, 1);
@@ -1314,6 +1317,7 @@ Without an argument, history expansion i
Generic command for showing things about the program being debugged."),
&infolist, "info ", 0, &cmdlist);
set_cmd_async_ok (c);
+ set_cmd_no_selected_thread_ok (c);
add_com_alias ("i", "info", class_info, 1);
add_com ("complete", class_obscure, complete_command,
@@ -1323,6 +1327,7 @@ Generic command for showing things about
Generic command for showing things about the debugger."),
&showlist, "show ", 0, &cmdlist);
set_cmd_async_ok (c);
+ set_cmd_no_selected_thread_ok (c);
/* Another way to get at the same thing. */
add_info ("set", show_command, _("Show all GDB settings."));
Index: src/gdb/linux-fork.c
===================================================================
--- src.orig/gdb/linux-fork.c 2008-06-15 20:56:56.000000000 +0100
+++ src/gdb/linux-fork.c 2008-06-15 20:57:46.000000000 +0100
@@ -248,6 +248,9 @@ fork_load_infrun_state (struct fork_info
registers_changed ();
reinit_frame_cache ();
+ store_selected_thread_and_frame (inferior_ptid,
+ get_selected_frame (NULL));
+
stop_pc = read_pc ();
nullify_last_target_wait_ptid ();
Index: src/gdb/mi/mi-main.c
===================================================================
--- src.orig/gdb/mi/mi-main.c 2008-06-15 20:56:55.000000000 +0100
+++ src/gdb/mi/mi-main.c 2008-06-15 20:57:46.000000000 +0100
@@ -647,7 +647,7 @@ mi_cmd_target_download (char *command, c
file to download. */
run = xstrprintf ("load %s", argc ? *argv : "");
old_cleanups = make_cleanup (xfree, run);
- execute_command (run, 0);
+ execute_command_internal (run, 0);
do_cleanups (old_cleanups);
return MI_CMD_DONE;
@@ -683,7 +683,7 @@ mi_cmd_target_select (char *command, cha
/* NOTE: At present all targets that are connected are also
(implicitly) talking to a halted target. In the future this may
change. */
- execute_command (run, 0);
+ execute_command_internal (run, 0);
do_cleanups (old_cleanups);
@@ -1186,15 +1186,50 @@ mi_execute_command (char *cmd, int from_
/* ..... */
}
+
+static void
+do_restore_selected_thread_and_frame_cleanup (void *arg)
+{
+ restore_selected_thread_and_frame ();
+}
+
static enum mi_cmd_result
mi_cmd_execute (struct mi_parse *parse)
{
- struct cleanup *cleanup;
+ struct cleanup *cleanup = make_cleanup (null_cleanup, NULL);
enum mi_cmd_result r;
free_all_values ();
+ /* Switch to the frontend selected thread, 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
+ frontend notices immediatelly. Note that the command may change
+ the selected thread. */
+ make_cleanup (do_restore_selected_thread_and_frame_cleanup, NULL);
+
if (parse->cmd->argv_func != NULL)
{
+ if (target_can_async_p ()
+ && target_has_execution
+ && (ptid_equal (user_selected_ptid, null_ptid))
+ && (strcmp (parse->command, "thread-info") != 0
+ && strcmp (parse->command, "thread-list-ids") != 0
+ && strcmp (parse->command, "thread-select") != 0))
+ {
+ struct ui_file *stb;
+ stb = mem_fileopen ();
+
+ fputs_unfiltered ("Cannot execute command ", stb);
+ fputstr_unfiltered (parse->command, '"', stb);
+ fputs_unfiltered (" without a selected thread", stb);
+
+ make_cleanup_ui_file_delete (stb);
+ error_stream (stb);
+ }
+
if ((!non_stop && any_running ())
|| (non_stop && is_running (inferior_ptid)))
{
@@ -1265,7 +1300,12 @@ 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 ( /*ui */ run, 0 /*from_tty */ );
+ /* 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 */ );
do_cleanups (old_cleanups);
return;
}
@@ -1306,7 +1346,7 @@ mi_execute_async_cli_command (char *cli_
}
- execute_command ( /*ui */ run, 0 /*from_tty */ );
+ execute_command_internal ( /*ui */ run, 0 /*from_tty */ );
if (target_can_async_p ())
{
Index: src/gdb/infcall.c
===================================================================
--- src.orig/gdb/infcall.c 2008-06-15 20:56:55.000000000 +0100
+++ src/gdb/infcall.c 2008-06-15 20:57:46.000000000 +0100
@@ -724,14 +724,18 @@ call_function_by_hand (struct value *fun
suppress_resume_observer = 1;
make_cleanup_restore_integer (&suppress_stop_observer);
suppress_stop_observer = 1;
+ make_cleanup_restore_integer (&proceed_to_inferior_function_call);
+ proceed_to_inferior_function_call = 1; /* Don't change the user
+ selected frame in
+ normal_stop. */
proceed (real_pc, TARGET_SIGNAL_0, 0);
do_cleanups (old_cleanups2);
-
+
if (saved_async)
target_async_mask (saved_async);
-
+
enable_watchpoints_after_interactive_call_stop ();
-
+
discard_cleanups (old_cleanups);
}
Index: src/gdb/target.c
===================================================================
--- src.orig/gdb/target.c 2008-06-15 20:56:56.000000000 +0100
+++ src/gdb/target.c 2008-06-15 20:57:46.000000000 +0100
@@ -2238,6 +2238,7 @@ generic_mourn_inferior (void)
extern int show_breakpoint_hit_counts;
inferior_ptid = null_ptid;
+ store_selected_thread_and_frame (null_ptid, NULL);
attach_flag = 0;
breakpoint_init_inferior (inf_exited);
registers_changed ();
Index: src/gdb/mi/mi-cmd-env.c
===================================================================
--- src.orig/gdb/mi/mi-cmd-env.c 2008-06-15 20:56:55.000000000 +0100
+++ src/gdb/mi/mi-cmd-env.c 2008-06-15 20:57:46.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 ( /*ui */ run, 0 /*from_tty */ );
+ execute_command_internal ( /*ui */ run, 0 /*from_tty */ );
do_cleanups (old_cleanups);
return;
}
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [non-stop] 10/10 split user/internal threads
2008-06-15 21:49 [non-stop] 10/10 split user/internal threads Pedro Alves
@ 2008-06-26 13:41 ` Daniel Jacobowitz
2008-06-30 12:21 ` Pedro Alves
0 siblings, 1 reply; 11+ messages in thread
From: Daniel Jacobowitz @ 2008-06-26 13:41 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
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?
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?
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.
> +enum thread_state
> +{
> + THREAD_STOPPED,
> + THREAD_RUNNING,
> + THREAD_EXITED,
> +};
> +
> +static enum thread_state main_thread_state = 0;
THREAD_STOPPED instead of 0?
> - /* 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.
> +/* 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.
> + /* 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
--
Daniel Jacobowitz
CodeSourcery
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [non-stop] 10/10 split user/internal threads
2008-06-26 13:41 ` Daniel Jacobowitz
@ 2008-06-30 12:21 ` Pedro Alves
2008-07-01 0:51 ` Daniel Jacobowitz
0 siblings, 1 reply; 11+ messages in thread
From: Pedro Alves @ 2008-06-30 12:21 UTC (permalink / raw)
To: Daniel Jacobowitz; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 13186 bytes --]
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
[-- Attachment #2: context_thread.diff --]
[-- Type: text/x-diff, Size: 29249 bytes --]
---
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);
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [non-stop] 10/10 split user/internal threads
2008-06-30 12:21 ` Pedro Alves
@ 2008-07-01 0:51 ` Daniel Jacobowitz
2008-07-01 13:38 ` Pedro Alves
0 siblings, 1 reply; 11+ messages in thread
From: Daniel Jacobowitz @ 2008-07-01 0:51 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
On Mon, Jun 30, 2008 at 01:08:48AM +0100, Pedro Alves wrote:
> - 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.
The target interface is async, the inferior program is running - but
GDB retains a single thread of control. So unless the inferior runs
while the cleanup is live, there's no problem here. I suppose it
could be trouble if you enter the expression evaluator, though?
> 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?
IMO it's thread 3 in all-stop and thread 1 in non-stop. This is a
weird user-visible artifact, though.
> 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?
Yes, in non-stop the weird behavior of scoping the "thread" command to
a commands list seems to be the only plausible choice.
> 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.
Isn't this exactly the same as using cleanups, except that there is a
way to invalidate saved cleanups?
If so, then it seems to me there is a simpler and less intrusive
solution. Put a reference count in the thread structure and increment
it when you push a cleanup. I think Vladimir added a hook that will
let us drop the reference even if the cleanup is discarded.
I don't want to proliferate cleanup-like mechanisms if we can avoid
it, the ones we have are confusing enough already :-)
> > > +/* 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?
Does it in "by the time it is" refer to the cleanup? And then the
"it"s on the last line to "the same thread"? Pronouns are so
confusing.
I didn't look at the incremental patch, sorry - let's finish talking
about the problem first.
--
Daniel Jacobowitz
CodeSourcery
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [non-stop] 10/10 split user/internal threads
2008-07-01 0:51 ` Daniel Jacobowitz
@ 2008-07-01 13:38 ` Pedro Alves
2008-07-01 14:03 ` Daniel Jacobowitz
0 siblings, 1 reply; 11+ messages in thread
From: Pedro Alves @ 2008-07-01 13:38 UTC (permalink / raw)
To: Daniel Jacobowitz; +Cc: gdb-patches
A Tuesday 01 July 2008 01:51:28, Daniel Jacobowitz wrote:
> On Mon, Jun 30, 2008 at 01:08:48AM +0100, Pedro Alves wrote:
> > - 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.
>
> The target interface is async, the inferior program is running - but
> GDB retains a single thread of control. So unless the inferior runs
> while the cleanup is live, there's no problem here. I suppose it
> could be trouble if you enter the expression evaluator, though?
Sure it can run while the cleanup is live. It could already be
running when the cleanup was created. Remember that the selected
thread may be running at any time.
If the selected thread was not running, but it is set
running while the cleanup is live, and we enter the expression
evaluator, I don't think there's a problem currently, as
returning from infcalls tries to restore the originally
selected frame.
The other case the current thread may be set running is:
01: foo3()
02: {
03: }
04:
05: foo2()
06: {
07: foo3();
08: }
09:
10: foo1()
11: {
12: foo2();
13: }
14:
15: foo_other_thread()
16: {
17: }
(gdb) info threads
* 1 Thread <foo> at foo2 line 7
(gdb) bt
#0 0xffffe410 in foo2 ()
#1 0xf7e4aff6 in foo1 ()
#2 0xf7e8634c in thread_entry ()
(gdb) up
#1 0xf7e4aff6 in foo1 ()
(gdb) break foo_other_thread
Breakpoint 1 ...
(gdb) commands
>thread 1
>step&
>end
<thread 2 hits break 1>
<steps thread 1>
The user loses the selected frame in thread 1, because when
the cleanup is ran, the thread is running/stepping.
Since the thread stepped into another frame, we could
claim that since the originally selected frame is still live,
it should still be selected. But, it can also be
claimed that, though luck, the thread was set running,
you lose the selected frame. This doesn't seem
like an important enough case to care about. Agree?
> > - 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?
>
> IMO it's thread 3 in all-stop and thread 1 in non-stop. This is a
> weird user-visible artifact, though.
>
Ok, that's my opinion too.
> > 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.
>
> Isn't this exactly the same as using cleanups, except that there is a
> way to invalidate saved cleanups?
>
Yeah, conceptually different, but the end result is close.
In fact, ...
> If so, then it seems to me there is a simpler and less intrusive
> solution. Put a reference count in the thread structure and increment
> it when you push a cleanup. I think Vladimir added a hook that will
> let us drop the reference even if the cleanup is discarded.
>
... I like this.
There's one issue that I'd like to point out. That is, is a
thread exits, and it was inferior_ptid, we can't just switch
inferior_ptid to null_ptid as I could with split user/internal
threads split. A *lot* of things break.
The issue arises with the OS quickly reusing ptids:
- inferior_ptid (ptid1) exits, we can't delete it from the
thread list yet (things break, e.g. context switching...,
but many other things break in target code.)
- We mark it with THREAD_EXITED, but leave it there.
- target notifies new thread with ptid1. There's already
such a thread in the list, and it's also the
current thread -- inferior_ptid.
- To add this new thread to the list, we must get rid
of the old exited thread. Conceptually, this new thread
although it has the target identifier, it should have a
new GDB thread id. So, we could say that in this case,
the "non-stop doesn't change threads" premise breaks. But,
then again, the user couldn't do anything to the exited
thread anyway, and it can only exit is it is running, in which
case the user also couldn't do most things with it. Maybe we
can just live with this exception.
> I don't want to proliferate cleanup-like mechanisms if we can avoid
> it, the ones we have are confusing enough already :-)
>
> > > > +/* 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?
>
> Does it in "by the time it is" refer to the cleanup? And then the
> "it"s on the last line to "the same thread"? Pronouns are so
> confusing.
Oh, sorry. Yes, that's correct.
> I didn't look at the incremental patch, sorry - let's finish talking
> about the problem first.
No problem. I like the refcounting idea. Let me work on it, and
see if there's any other problem.
--
Pedro Alves
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [non-stop] 10/10 split user/internal threads
2008-07-01 13:38 ` Pedro Alves
@ 2008-07-01 14:03 ` Daniel Jacobowitz
2008-07-02 3:29 ` Pedro Alves
2008-07-02 3:39 ` [non-stop] 10/10 handle exited threads [was: Re: [non-stop] 10/10 split user/internal threads] Pedro Alves
0 siblings, 2 replies; 11+ messages in thread
From: Daniel Jacobowitz @ 2008-07-01 14:03 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
On Tue, Jul 01, 2008 at 02:37:44PM +0100, Pedro Alves wrote:
> A Tuesday 01 July 2008 01:51:28, Daniel Jacobowitz wrote:
> > On Mon, Jun 30, 2008 at 01:08:48AM +0100, Pedro Alves wrote:
> > > - 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.
> >
> > The target interface is async, the inferior program is running - but
> > GDB retains a single thread of control. So unless the inferior runs
> > while the cleanup is live, there's no problem here. I suppose it
> > could be trouble if you enter the expression evaluator, though?
>
> Sure it can run while the cleanup is live. It could already be
> running when the cleanup was created. Remember that the selected
> thread may be running at any time.
Doesn't matter if the thread is already running. We won't go looking
for events while normal cleanups are on the stack in most cases - so
even if the thread exits, we won't remove it from the thread list
until later. But the evaluator, and some commands, might cause us to
return to w_f_i. That's all I meant.
> The user loses the selected frame in thread 1, because when
> the cleanup is ran, the thread is running/stepping.
> Since the thread stepped into another frame, we could
> claim that since the originally selected frame is still live,
> it should still be selected. But, it can also be
> claimed that, though luck, the thread was set running,
> you lose the selected frame. This doesn't seem
> like an important enough case to care about. Agree?
Agreed. We can worry about it later. Execution commands in commands
lists already behave weirdly - e.g. they may cause the rest of the
command list to be discarded.
> There's one issue that I'd like to point out. That is, is a
> thread exits, and it was inferior_ptid, we can't just switch
> inferior_ptid to null_ptid as I could with split user/internal
> threads split. A *lot* of things break.
>
> The issue arises with the OS quickly reusing ptids:
>
> - inferior_ptid (ptid1) exits, we can't delete it from the
> thread list yet (things break, e.g. context switching...,
> but many other things break in target code.)
> - We mark it with THREAD_EXITED, but leave it there.
> - target notifies new thread with ptid1. There's already
> such a thread in the list, and it's also the
> current thread -- inferior_ptid.
> - To add this new thread to the list, we must get rid
> of the old exited thread. Conceptually, this new thread
> although it has the target identifier, it should have a
> new GDB thread id. So, we could say that in this case,
> the "non-stop doesn't change threads" premise breaks. But,
> then again, the user couldn't do anything to the exited
> thread anyway, and it can only exit is it is running, in which
> case the user also couldn't do most things with it. Maybe we
> can just live with this exception.
IMO, as long as GDB is not likely to crash, I don't think there's a
big problem here. If we, for instance, fail to report one pair of
thread exit and thread creation events in this case that would be OK.
Any number of other quirky behaviors would too. Most systems have a
sufficiently large ID space and avoid immediate reuse such that this
is very unlikely to ever trigger. If we come across a system that
reuses the first available TID then we'll have to do better but we'll
also have an easier way to test :-)
Is this patch a blocking issue for the other non-stop patches, or
would you like to start merging them?
Also, a request: I know that you and Vlad are going to be filling in
the documentation and test cases before any release goes out with this
code. In the meanwhile, could you create a wiki page listing things
to be documented/tested so that there's a single central list?
--
Daniel Jacobowitz
CodeSourcery
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [non-stop] 10/10 split user/internal threads
2008-07-01 14:03 ` Daniel Jacobowitz
@ 2008-07-02 3:29 ` Pedro Alves
2008-07-02 3:39 ` [non-stop] 10/10 handle exited threads [was: Re: [non-stop] 10/10 split user/internal threads] Pedro Alves
1 sibling, 0 replies; 11+ messages in thread
From: Pedro Alves @ 2008-07-02 3:29 UTC (permalink / raw)
To: gdb-patches; +Cc: Daniel Jacobowitz
A Tuesday 01 July 2008 15:03:11, Daniel Jacobowitz wrote:
> On Tue, Jul 01, 2008 at 02:37:44PM +0100, Pedro Alves wrote:
> > A Tuesday 01 July 2008 01:51:28, Daniel Jacobowitz wrote:
> > > On Mon, Jun 30, 2008 at 01:08:48AM +0100, Pedro Alves wrote:
> > There's one issue that I'd like to point out. That is, is a
> > thread exits, and it was inferior_ptid, we can't just switch
> > inferior_ptid to null_ptid as I could with split user/internal
> > threads split. A *lot* of things break.
> >
> > The issue arises with the OS quickly reusing ptids:
> >
> > - inferior_ptid (ptid1) exits, we can't delete it from the
> > thread list yet (things break, e.g. context switching...,
> > but many other things break in target code.)
> > - We mark it with THREAD_EXITED, but leave it there.
> > - target notifies new thread with ptid1. There's already
> > such a thread in the list, and it's also the
> > current thread -- inferior_ptid.
> > - To add this new thread to the list, we must get rid
> > of the old exited thread. Conceptually, this new thread
> > although it has the target identifier, it should have a
> > new GDB thread id. So, we could say that in this case,
> > the "non-stop doesn't change threads" premise breaks. But,
> > then again, the user couldn't do anything to the exited
> > thread anyway, and it can only exit is it is running, in which
> > case the user also couldn't do most things with it. Maybe we
> > can just live with this exception.
>
> IMO, as long as GDB is not likely to crash, I don't think there's a
> big problem here. If we, for instance, fail to report one pair of
> thread exit and thread creation events in this case that would be OK.
> Any number of other quirky behaviors would too. Most systems have a
> sufficiently large ID space and avoid immediate reuse such that this
> is very unlikely to ever trigger. If we come across a system that
> reuses the first available TID then we'll have to do better but we'll
> also have an easier way to test :-)
>
Well, quickly was perhaps a too much of a quick word. If the user leaves
the selected thread pointing at an already exited thread, and the
app has many short lived threads, the reusing doesn't have to
be that quick. I'd guess that we may see it happen no so unfrequently
if the target implements thread ids as pointers to (reused) thread
control blocks.
> Is this patch a blocking issue for the other non-stop patches, or
> would you like to start merging them?
I'd like to merge them, yes. But I'd prefer to get the first 7
patches of the series checked in togheter. I need another round of
review on patch 7 (and a mechanical follow up to it).
Patch 8 is going to request attention, and the new version of
patch 10 I'm about to post will need review.
Patches 1-6 and 9, you've already OKed.
> Also, a request: I know that you and Vlad are going to be filling in
> the documentation and test cases before any release goes out with this
> code. In the meanwhile, could you create a wiki page listing things
> to be documented/tested so that there's a single central list?
I've created:
http://sourceware.org/gdb/wiki/GDB_Next_Release
linked from the wiki frontpage. It's short in content, but it should
prevent us from forgeting about it.
--
Pedro Alves
^ permalink raw reply [flat|nested] 11+ messages in thread
* [non-stop] 10/10 handle exited threads [was: Re: [non-stop] 10/10 split user/internal threads]
2008-07-01 14:03 ` Daniel Jacobowitz
2008-07-02 3:29 ` Pedro Alves
@ 2008-07-02 3:39 ` Pedro Alves
2008-07-07 18:59 ` Daniel Jacobowitz
1 sibling, 1 reply; 11+ messages in thread
From: Pedro Alves @ 2008-07-02 3:39 UTC (permalink / raw)
To: gdb-patches; +Cc: Daniel Jacobowitz
[-- Attachment #1: Type: text/plain, Size: 506 bytes --]
Here's the rewrite of this patch to use cleanups and refcounting on the
thread_info objects to prevent them from being deleted while the thread
is held in a cleanup. This version completelly drops the
external/internal thread separation, and sticks with inferior_ptid
to mean current thread. Exited threads are kept in the thread list
until safe to delete. Most commands aren't allowed while the
user has an exited thread selected.
Tested on x86_64-unknown-linux-gnu -m32, sync/async.
--
Pedro Alves
[-- Attachment #2: 010-exited_threads_ref_count.diff --]
[-- Type: text/x-diff, Size: 45834 bytes --]
2008-07-02 Pedro Alves <pedro@codesourcery.com>
Exited threads.
* thread.c (enum thread_state): New.
(thread_state main_thread_running): Delete, in favor of...
(thread_state main_thread_state): ... this. Update throughout.
(clear_thread_inferior_resources): New, split from free_thread.
(free_thread): Call clear_thread_inferior_resources.
(init_thread_list): Set main thread to stopped state.
(add_thread_silent): Take care of PTID reuses.
(delete_thread): If deleting inferior_ptid or a thread with
refcount > 0, mark it as exited, but still keep it in the list.
Only notify of thread exits, if we haven't done so yet.
(iterate_over_threads): Make it safe to delete threads while
iterating over them.
(do_captured_list_thread_ids): Don't account for exited threads.
(thread_alive): Check for the THREAD_EXITED state, and don't set
ptid to -1 on exited threads.
(set_running): Update to account for extra possible states.
(is_thread_state): New.
(is_stopped, is_exited): New.
(is_running): Implement in terms of is_thread_state.
(any_running): Update.
(print_thread_info): Update. Account for exited threads. Don't
warn about missed frame restoring here, it's done in the cleanup.
(switch_to_thread): Don't read from a thread that has gone.
(restore_current_thread): In non-stop mode, do a full context
switch.
(restore_selected_frame): Add a frame_level argument. Rewrite.
(struct current_thread_cleanup): Add selected_frame_level and
was_stopped members.
(do_restore_current_thread_cleanup): Check if thread was stopped
and still is, and if the target has registers, stack and memory
before restoring the selected frame. Don't delete the cleanup
argument here.
(restore_current_thread_cleanup_dtor): New.
(make_cleanup_restore_current_thread): Remove all arguments.
Rewrite.
(thread_apply_all_command): Update. Prune threads.
(thread_apply_command): Update.
(thread_command): Account for currently selected exited thread.
(do_captured_thread_select): Check for a running thread. Prune
threads.
(_initialize_thread): Make "info threads", "thread", "thread
apply", and "thread apply all" appliable without a selected thread.
* gdbthread.h (struct thread_info): Replace running_ by state_.
Add refcount.
(is_exited, is_stopped): Declare.
(make_cleanup_restore_current_thread): Remove all arguments.
* infrun.c: Include "event-top.h".
(fetch_inferior_event): In non-stop mode, restore selected thread
and frame after handling the event and running breakpoint
commands. Display GDB prompt if needed.
(normal_stop): In non-stop mode, don't print thread switching
notice.
* cli/cli-decode.c (set_cmd_no_selected_thread_ok)
(get_cmd_no_selected_thread_ok): New.
* cli/cli-decode.h (CMD_NO_SELECTED_THREAD_OK): New.
(set_cmd_no_selected_thread_ok, get_cmd_no_selected_thread_ok):
Declare.
* cli/cli-cmds.c: Set "pwd", "help", "info", "show" as
no-selected-thread ok.
* top.c (execute_command): Check for non no-selected-thread-ok
commands.
* linux-nat.c (struct saved_ptids, threads_to_delete)
(record_dead_thread, prune_lwps): Delete.
(exit_lwp): Unconditionally delete thread.
(linux_nat_resume): Remove prune_lwps call.
* infcmd.c (proceed_thread_callback): Check if !is_stopped instead
of is_running. Adjust to make_cleanup_restore_current_thread
interface change.
* mi/mi-main.c (mi_cmd_execute): Only allow a few commands if the
selected thread has exited.
* inf-loop.c (inferior_event_handler): Don't display the prompt
here.
* varobj.c (c_value_of_root): Update.
* Makefile.in (infrun.o): Depend on $(event_top_h).
* defs.h (make_cleanup_dtor): Declare.
* utils.c (make_cleanup_dtor): New.
---
gdb/Makefile.in | 2
gdb/cli/cli-cmds.c | 5
gdb/cli/cli-decode.c | 12 +
gdb/cli/cli-decode.h | 11 +
gdb/defs.h | 3
gdb/gdbthread.h | 30 ++-
gdb/inf-loop.c | 16 -
gdb/infcmd.c | 12 -
gdb/infrun.c | 26 ++
gdb/linux-nat.c | 48 -----
gdb/mi/mi-main.c | 18 +
gdb/thread.c | 468 +++++++++++++++++++++++++++++++++++----------------
gdb/top.c | 19 +-
gdb/utils.c | 8
gdb/varobj.c | 3
15 files changed, 455 insertions(+), 226 deletions(-)
Index: src/gdb/thread.c
===================================================================
--- src.orig/gdb/thread.c 2008-07-01 23:03:37.000000000 +0100
+++ src/gdb/thread.c 2008-07-02 03:10:06.000000000 +0100
@@ -41,7 +41,6 @@
#include "ui-out.h"
#include "observer.h"
#include "annotate.h"
-
#include "cli/cli-decode.h"
/* Definition of struct thread_info exported to gdbthread.h */
@@ -65,7 +64,16 @@ static void thread_apply_command (char *
static void restore_current_thread (ptid_t);
static void prune_threads (void);
-static int main_thread_running = 0;
+/* Frontend view of the thread state. Possible extensions: stepping,
+ finishing, until(ling),... */
+enum thread_state
+{
+ THREAD_STOPPED,
+ THREAD_RUNNING,
+ THREAD_EXITED,
+};
+
+static enum thread_state main_thread_state = THREAD_STOPPED;
static int main_thread_executing = 0;
void
@@ -86,16 +94,25 @@ delete_step_resume_breakpoint (void *arg
}
static void
-free_thread (struct thread_info *tp)
+clear_thread_inferior_resources (struct thread_info *tp)
{
/* NOTE: this will take care of any left-over step_resume breakpoints,
but not any user-specified thread-specific breakpoints. We can not
delete the breakpoint straight-off, because the inferior might not
be stopped at the moment. */
if (tp->step_resume_breakpoint)
- tp->step_resume_breakpoint->disposition = disp_del_at_next_stop;
+ {
+ tp->step_resume_breakpoint->disposition = disp_del_at_next_stop;
+ tp->step_resume_breakpoint = NULL;
+ }
bpstat_clear (&tp->stop_bpstat);
+}
+
+static void
+free_thread (struct thread_info *tp)
+{
+ clear_thread_inferior_resources (tp);
/* FIXME: do I ever need to call the back-end to give it a
chance at this private data before deleting the thread? */
@@ -111,7 +128,7 @@ init_thread_list (void)
struct thread_info *tp, *tpnext;
highest_thread_num = 0;
- main_thread_running = 0;
+ main_thread_state = THREAD_STOPPED;
main_thread_executing = 0;
if (!thread_list)
@@ -131,6 +148,49 @@ add_thread_silent (ptid_t ptid)
{
struct thread_info *tp;
+ tp = find_thread_pid (ptid);
+ if (tp)
+ /* Found an old thread with the same id. It has to be dead,
+ otherwise we wouldn't be adding a new thread with the same id.
+ The OS is reusing this id --- delete it, and recreate a new
+ one. */
+ {
+ /* In addition to deleting the thread, if this is the current
+ thread, then we need to also get rid of the current infrun
+ context, and take care that delete_thread doesn't really
+ delete the thread if it is inferior_ptid. Create a new
+ template thread in the list with an invalid ptid, context
+ switch to it, delete the original thread, reset the new
+ thread's ptid, and switch to it. */
+
+ if (ptid_equal (inferior_ptid, ptid))
+ {
+ tp = xmalloc (sizeof (*tp));
+ memset (tp, 0, sizeof (*tp));
+ tp->ptid = minus_one_ptid;
+ tp->num = ++highest_thread_num;
+ tp->next = thread_list;
+ thread_list = tp;
+ context_switch_to (minus_one_ptid);
+
+ /* Now we can delete it. */
+ delete_thread (ptid);
+
+ /* Since the context is already set to this new thread,
+ reset it's ptid, and reswitch inferior_ptid to it. */
+ tp->ptid = ptid;
+ switch_to_thread (ptid);
+
+ observer_notify_new_thread (tp);
+
+ /* All done. */
+ return tp;
+ }
+ else
+ /* Just go ahead and delete it. */
+ delete_thread (ptid);
+ }
+
tp = (struct thread_info *) xmalloc (sizeof (*tp));
memset (tp, 0, sizeof (*tp));
tp->ptid = ptid;
@@ -163,6 +223,10 @@ add_thread (ptid_t ptid)
return add_thread_with_info (ptid, NULL);
}
+/* Delete thread PTID and notify of thread exit. If this is
+ inferior_ptid, don't actually delete it, but tag it as exited and
+ do the notification. If PTID is the user selected thread, clear
+ it. */
void
delete_thread (ptid_t ptid)
{
@@ -177,12 +241,35 @@ delete_thread (ptid_t ptid)
if (!tp)
return;
+ /* If this is the current thread, or there's code out there that
+ relies on it existing (refcout > 0) we can't delete yet. Mark it
+ as exited, and notify it. */
+ if (tp->refcount > 0
+ || ptid_equal (tp->ptid, inferior_ptid))
+ {
+ if (tp->state_ != THREAD_EXITED)
+ {
+ observer_notify_thread_exit (tp);
+
+ /* Tag it as exited. */
+ tp->state_ = THREAD_EXITED;
+
+ /* Clear breakpoints, etc. associated with this thread. */
+ clear_thread_inferior_resources (tp);
+ }
+
+ /* Will be really deleted some other time. */
+ return;
+ }
+
if (tpprev)
tpprev->next = tp->next;
else
thread_list = tp->next;
- observer_notify_thread_exit (tp);
+ /* Notify thread exit, but only if we haven't already. */
+ if (tp->state_ != THREAD_EXITED)
+ observer_notify_thread_exit (tp);
free_thread (tp);
}
@@ -230,11 +317,14 @@ struct thread_info *
iterate_over_threads (int (*callback) (struct thread_info *, void *),
void *data)
{
- struct thread_info *tp;
+ struct thread_info *tp, *next;
- for (tp = thread_list; tp; tp = tp->next)
- if ((*callback) (tp, data))
- return tp;
+ for (tp = thread_list; tp; tp = next)
+ {
+ next = tp->next;
+ if ((*callback) (tp, data))
+ return tp;
+ }
return NULL;
}
@@ -313,6 +403,8 @@ do_captured_list_thread_ids (struct ui_o
for (tp = thread_list; tp; tp = tp->next)
{
+ if (tp->state_ == THREAD_EXITED)
+ continue;
num++;
ui_out_field_int (uiout, "thread-id", tp->num);
}
@@ -463,13 +555,10 @@ save_infrun_state (ptid_t ptid,
static int
thread_alive (struct thread_info *tp)
{
- if (PIDGET (tp->ptid) == -1)
+ if (tp->state_ == THREAD_EXITED)
return 0;
if (!target_thread_alive (tp->ptid))
- {
- tp->ptid = pid_to_ptid (-1); /* Mark it as dead */
- return 0;
- }
+ return 0;
return 1;
}
@@ -501,9 +590,11 @@ set_running (ptid_t ptid, int running)
the main thread is always present in the thread list. If it's
not, the first call to context_switch will mess up GDB internal
state. */
- if (running && !main_thread_running && !suppress_resume_observer)
+ if (running
+ && main_thread_state != THREAD_RUNNING
+ && !suppress_resume_observer)
observer_notify_target_resumed (ptid);
- main_thread_running = running;
+ main_thread_state = running ? THREAD_RUNNING : THREAD_STOPPED;
return;
}
@@ -515,25 +606,31 @@ set_running (ptid_t ptid, int running)
int any_started = 0;
for (tp = thread_list; tp; tp = tp->next)
{
- if (running && !tp->running_)
- any_started = 1;
- tp->running_ = running;
+ if (tp->state_ == THREAD_EXITED)
+ continue;
+ if (running && tp->state_ == THREAD_STOPPED)
+ any_started = 1;
+ tp->state_ = running ? THREAD_RUNNING : THREAD_STOPPED;
}
if (any_started && !suppress_resume_observer)
observer_notify_target_resumed (ptid);
}
else
{
+ int started = 0;
tp = find_thread_pid (ptid);
gdb_assert (tp);
- if (running && !tp->running_ && !suppress_resume_observer)
- observer_notify_target_resumed (ptid);
- tp->running_ = running;
- }
+ gdb_assert (tp->state_ != THREAD_EXITED);
+ if (running && tp->state_ == THREAD_STOPPED)
+ started = 1;
+ tp->state_ = running ? THREAD_RUNNING : THREAD_STOPPED;
+ if (started && !suppress_resume_observer)
+ observer_notify_target_resumed (ptid);
+ }
}
-int
-is_running (ptid_t ptid)
+static int
+is_thread_state (ptid_t ptid, enum thread_state state)
{
struct thread_info *tp;
@@ -541,11 +638,41 @@ is_running (ptid_t ptid)
return 0;
if (!thread_list)
- return main_thread_running;
+ return main_thread_state == state;
tp = find_thread_pid (ptid);
gdb_assert (tp);
- return tp->running_;
+ return tp->state_ == state;
+}
+
+int
+is_stopped (ptid_t ptid)
+{
+ /* Without execution, this property is always true. */
+ if (!target_has_execution)
+ return 1;
+
+ return is_thread_state (ptid, THREAD_STOPPED);
+}
+
+int
+is_exited (ptid_t ptid)
+{
+ /* Without execution, this property is always false. */
+ if (!target_has_execution)
+ return 0;
+
+ return is_thread_state (ptid, THREAD_EXITED);
+}
+
+int
+is_running (ptid_t ptid)
+{
+ /* Without execution, this property is always false. */
+ if (!target_has_execution)
+ return 0;
+
+ return is_thread_state (ptid, THREAD_RUNNING);
}
int
@@ -557,10 +684,10 @@ any_running (void)
return 0;
if (!thread_list)
- return main_thread_running;
+ return main_thread_state == THREAD_RUNNING;
for (tp = thread_list; tp; tp = tp->next)
- if (tp->running_)
+ if (tp->state_ == THREAD_RUNNING)
return 1;
return 0;
@@ -612,7 +739,7 @@ set_executing (ptid_t ptid, int executin
/* Prints the list of threads and their details on UIOUT.
This is a version of 'info_thread_command' suitable for
use from MI.
- If REQESTED_THREAD is not -1, it's the GDB id of the thread
+ If REQUESTED_THREAD is not -1, it's the GDB id of the thread
that should be printed. Otherwise, all threads are
printed. */
void
@@ -620,24 +747,18 @@ print_thread_info (struct ui_out *uiout,
{
struct thread_info *tp;
ptid_t current_ptid;
- struct frame_info *cur_frame;
struct cleanup *old_chain;
- struct frame_id saved_frame_id;
char *extra_info;
int current_thread = -1;
- /* 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);
- make_cleanup_ui_out_list_begin_end (uiout, "threads");
-
prune_threads ();
target_find_new_threads ();
current_ptid = inferior_ptid;
+
+ /* We'll be switching threads temporarily. */
+ old_chain = make_cleanup_restore_current_thread ();
+
+ make_cleanup_ui_out_list_begin_end (uiout, "threads");
for (tp = thread_list; tp; tp = tp->next)
{
struct cleanup *chain2;
@@ -645,13 +766,16 @@ print_thread_info (struct ui_out *uiout,
if (requested_thread != -1 && tp->num != requested_thread)
continue;
+ if (ptid_equal (tp->ptid, current_ptid))
+ current_thread = tp->num;
+
+ if (tp->state_ == THREAD_EXITED)
+ continue;
+
chain2 = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
if (ptid_equal (tp->ptid, current_ptid))
- {
- current_thread = tp->num;
- ui_out_text (uiout, "* ");
- }
+ ui_out_text (uiout, "* ");
else
ui_out_text (uiout, " ");
@@ -659,15 +783,19 @@ print_thread_info (struct ui_out *uiout,
ui_out_text (uiout, " ");
ui_out_field_string (uiout, "target-id", target_tid_to_str (tp->ptid));
- extra_info = target_extra_thread_info (tp);
- if (extra_info)
+ if (tp->state_ != THREAD_EXITED)
{
- ui_out_text (uiout, " (");
- ui_out_field_string (uiout, "details", extra_info);
- ui_out_text (uiout, ")");
+ extra_info = target_extra_thread_info (tp);
+ if (extra_info)
+ {
+ ui_out_text (uiout, " (");
+ ui_out_field_string (uiout, "details", extra_info);
+ ui_out_text (uiout, ")");
+ }
+ ui_out_text (uiout, " ");
}
- ui_out_text (uiout, " ");
- if (tp->running_)
+
+ if (tp->state_ == THREAD_RUNNING)
ui_out_text (uiout, "(running)\n");
else
{
@@ -689,24 +817,15 @@ print_thread_info (struct ui_out *uiout,
if (requested_thread == -1)
{
- gdb_assert (current_thread != -1 || !thread_list);
+ gdb_assert (current_thread != -1
+ || !thread_list);
if (current_thread != -1 && ui_out_is_mi_like_p (uiout))
ui_out_field_int (uiout, "current-thread-id", current_thread);
- }
-
- if (is_running (inferior_ptid))
- return;
- /* If case we were not able to find the original frame, print the
- new selected frame. */
- if (frame_find_by_id (saved_frame_id) == NULL)
- {
- warning (_("Couldn't restore frame in current thread, at frame 0"));
- /* 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. */
- if (!ui_out_is_mi_like_p (uiout))
- print_stack_frame (get_selected_frame (NULL), 0, LOCATION);
+ if (current_thread != -1 && is_exited (current_ptid))
+ ui_out_message (uiout, 0, "\n\
+The current thread <Thread ID %d> has terminated. See `help thread'.\n",
+ current_thread);
}
}
@@ -736,7 +855,10 @@ switch_to_thread (ptid_t ptid)
reinit_frame_cache ();
registers_changed ();
- if (!is_executing (ptid))
+ /* We don't check for is_stopped, because we're called at times
+ while in the TARGET_RUNNING state, e.g., while handling an
+ internal event. */
+ if (!is_exited (ptid) && !is_executing (ptid))
stop_pc = read_pc ();
else
stop_pc = ~(CORE_ADDR) 0;
@@ -747,21 +869,64 @@ restore_current_thread (ptid_t ptid)
{
if (!ptid_equal (ptid, inferior_ptid))
{
- switch_to_thread (ptid);
+ if (non_stop)
+ context_switch_to (ptid);
+ else
+ switch_to_thread (ptid);
}
}
static void
-restore_selected_frame (struct frame_id a_frame_id)
+restore_selected_frame (struct frame_id a_frame_id, int frame_level)
{
- struct frame_info *selected_frame_info = NULL;
+ struct frame_info *frame = NULL;
+ int count;
- if (frame_id_eq (a_frame_id, null_frame_id))
- return;
+ gdb_assert (frame_level >= 0);
- if ((selected_frame_info = frame_find_by_id (a_frame_id)) != NULL)
+ /* 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 = frame_level;
+ frame = find_relative_frame (get_current_frame (), &count);
+ if (count == 0
+ && frame != NULL
+ /* Either the frame ids match, of they're both invalid.
+ The later case is not failsafe, but since it's highly
+ 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), a_frame_id)
+ /* Note: could be better to check every frame_id
+ member for equality here. */
+ || (!frame_id_p (get_frame_id (frame))
+ && !frame_id_p (a_frame_id))))
{
- select_frame (selected_frame_info);
+ /* Cool, all is fine. */
+ select_frame (frame);
+ return;
+ }
+
+ frame = frame_find_by_id (a_frame_id);
+ if (frame != NULL)
+ {
+ /* Cool, refound it. */
+ select_frame (frame);
+ return;
+ }
+
+ /* Nothing else to do, the frame layout really changed.
+ Tell the user. */
+ if (!ui_out_is_mi_like_p (uiout))
+ {
+ warning (_("\
+Couldn't restore frame #%d in current thread, at reparsed frame #0\n"),
+ 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. */
+ print_stack_frame (frame, 1, SRC_LINE);
}
}
@@ -769,31 +934,66 @@ struct current_thread_cleanup
{
ptid_t inferior_ptid;
struct frame_id selected_frame_id;
+ int selected_frame_level;
+ int was_stopped;
};
static void
do_restore_current_thread_cleanup (void *arg)
{
+ struct thread_info *tp;
struct current_thread_cleanup *old = arg;
restore_current_thread (old->inferior_ptid);
- /* A command like 'thread apply all $exec_command&' may change the
- running state of the originally selected thread, so we have to
- recheck it here. */
- if (!is_running (old->inferior_ptid))
- restore_selected_frame (old->selected_frame_id);
+ /* The running state of the originally selected thread may have
+ changed, so we have to recheck it here. */
+ if (old->was_stopped
+ && is_stopped (inferior_ptid)
+ && target_has_registers
+ && target_has_stack
+ && target_has_memory)
+ restore_selected_frame (old->selected_frame_id,
+ old->selected_frame_level);
+}
+
+static void
+restore_current_thread_cleanup_dtor (void *arg)
+{
+ struct current_thread_cleanup *old = arg;
+ struct thread_info *tp;
+ tp = find_thread_pid (old->inferior_ptid);
+ if (tp)
+ tp->refcount--;
xfree (old);
}
struct cleanup *
-make_cleanup_restore_current_thread (ptid_t inferior_ptid,
- struct frame_id a_frame_id)
+make_cleanup_restore_current_thread (void)
{
- struct current_thread_cleanup *old
- = xmalloc (sizeof (struct current_thread_cleanup));
+ struct thread_info *tp;
+ struct frame_info *frame;
+ struct current_thread_cleanup *old;
+
+ old = xmalloc (sizeof (struct current_thread_cleanup));
old->inferior_ptid = inferior_ptid;
- old->selected_frame_id = a_frame_id;
- return make_cleanup (do_restore_current_thread_cleanup, old);
+ old->was_stopped = is_stopped (inferior_ptid);
+ if (old->was_stopped
+ && target_has_registers
+ && target_has_stack
+ && target_has_memory)
+ frame = get_selected_frame (NULL);
+ else
+ frame = NULL;
+
+ old->selected_frame_id = get_frame_id (frame);
+ old->selected_frame_level = frame_relative_level (frame);
+
+ tp = find_thread_pid (inferior_ptid);
+ if (tp)
+ tp->refcount++;
+
+ return make_cleanup_dtor (do_restore_current_thread_cleanup, old,
+ restore_current_thread_cleanup_dtor);
}
/* Apply a GDB command to a list of threads. List syntax is a whitespace
@@ -809,27 +1009,17 @@ static void
thread_apply_all_command (char *cmd, int from_tty)
{
struct thread_info *tp;
- struct cleanup *old_chain = make_cleanup (null_cleanup, 0);
+ struct cleanup *old_chain;
char *saved_cmd;
- struct frame_id saved_frame_id;
- ptid_t current_ptid;
- int thread_has_changed = 0;
if (cmd == NULL || *cmd == '\000')
error (_("Please specify a command following the thread ID list"));
-
- current_ptid = inferior_ptid;
-
- if (!is_running (inferior_ptid))
- saved_frame_id = get_frame_id (get_selected_frame (NULL));
- else
- saved_frame_id = null_frame_id;
- make_cleanup_restore_current_thread (inferior_ptid, saved_frame_id);
- /* It is safe to update the thread list now, before
- traversing it for "thread apply all". MVS */
+ prune_threads ();
target_find_new_threads ();
+ old_chain = make_cleanup_restore_current_thread ();
+
/* Save a copy of the command in case it is clobbered by
execute_command */
saved_cmd = xstrdup (cmd);
@@ -848,13 +1038,7 @@ thread_apply_all_command (char *cmd, int
strcpy (cmd, saved_cmd); /* Restore exact command used previously */
}
- if (!ptid_equal (current_ptid, inferior_ptid))
- thread_has_changed = 1;
-
do_cleanups (old_chain);
- /* Print stack frame only if we changed thread. */
- if (thread_has_changed && !is_running (inferior_ptid))
- print_stack_frame (get_current_frame (), 1, SRC_LINE);
}
static void
@@ -863,11 +1047,7 @@ thread_apply_command (char *tidlist, int
char *cmd;
char *p;
struct cleanup *old_chain;
- struct cleanup *saved_cmd_cleanup_chain;
char *saved_cmd;
- struct frame_id saved_frame_id;
- ptid_t current_ptid;
- int thread_has_changed = 0;
if (tidlist == NULL || *tidlist == '\000')
error (_("Please specify a thread ID list"));
@@ -877,18 +1057,10 @@ thread_apply_command (char *tidlist, int
if (*cmd == '\000')
error (_("Please specify a command following the thread ID list"));
- current_ptid = inferior_ptid;
-
- 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);
-
/* Save a copy of the command in case it is clobbered by
execute_command */
saved_cmd = xstrdup (cmd);
- saved_cmd_cleanup_chain = make_cleanup (xfree, (void *) saved_cmd);
+ old_chain = make_cleanup (xfree, saved_cmd);
while (tidlist < cmd)
{
struct thread_info *tp;
@@ -926,26 +1098,24 @@ thread_apply_command (char *tidlist, int
warning (_("Thread %d has terminated."), start);
else
{
+ make_cleanup_restore_current_thread ();
+
if (non_stop)
context_switch_to (tp->ptid);
else
switch_to_thread (tp->ptid);
+
printf_filtered (_("\nThread %d (%s):\n"), tp->num,
target_tid_to_str (inferior_ptid));
execute_command (cmd, from_tty);
- strcpy (cmd, saved_cmd); /* Restore exact command used previously */
+
+ /* Restore exact command used previously. */
+ strcpy (cmd, saved_cmd);
}
}
}
- if (!ptid_equal (current_ptid, inferior_ptid))
- thread_has_changed = 1;
-
- do_cleanups (saved_cmd_cleanup_chain);
do_cleanups (old_chain);
- /* Print stack frame only if we changed thread. */
- if (thread_has_changed)
- print_stack_frame (get_current_frame (), 1, SRC_LINE);
}
/* Switch to the specified thread. Will dispatch off to thread_apply_command
@@ -956,11 +1126,17 @@ thread_command (char *tidstr, int from_t
{
if (!tidstr)
{
- /* Don't generate an error, just say which thread is current. */
if (target_has_stack)
- printf_filtered (_("[Current thread is %d (%s)]\n"),
- pid_to_thread_id (inferior_ptid),
- target_tid_to_str (inferior_ptid));
+ {
+ if (is_exited (inferior_ptid))
+ printf_filtered (_("[Current thread is %d (%s) (exited)]\n"),
+ pid_to_thread_id (inferior_ptid),
+ target_tid_to_str (inferior_ptid));
+ else
+ printf_filtered (_("[Current thread is %d (%s)]\n"),
+ pid_to_thread_id (inferior_ptid),
+ target_tid_to_str (inferior_ptid));
+ }
else
error (_("No stack."));
return;
@@ -1008,10 +1184,16 @@ do_captured_thread_select (struct ui_out
ui_out_text (uiout, target_tid_to_str (inferior_ptid));
ui_out_text (uiout, ")]");
- if (!tp->running_)
- print_stack_frame (get_selected_frame (NULL), 1, SRC_AND_LOC);
- else
+ /* Note that we can't reach this with an exited thread, due to the
+ thread_alive check above. */
+ if (tp->state_ == THREAD_RUNNING)
ui_out_text (uiout, "(running)\n");
+ else
+ print_stack_frame (get_selected_frame (NULL), 1, SRC_AND_LOC);
+
+ /* Since the current thread may have changed, see if there is any
+ exited thread we can now delete. */
+ prune_threads ();
return GDB_RC_OK;
}
@@ -1037,19 +1219,25 @@ _initialize_thread (void)
c = add_info ("threads", info_threads_command,
_("IDs of currently known threads."));
set_cmd_async_ok (c);
+ set_cmd_no_selected_thread_ok (c);
c = add_prefix_cmd ("thread", class_run, thread_command, _("\
Use this command to switch between threads.\n\
The new thread ID must be currently known."),
&thread_cmd_list, "thread ", 1, &cmdlist);
set_cmd_async_ok (c);
+ set_cmd_no_selected_thread_ok (c);
- add_prefix_cmd ("apply", class_run, thread_apply_command,
- _("Apply a command to a list of threads."),
- &thread_apply_list, "thread apply ", 1, &thread_cmd_list);
+ c = add_prefix_cmd ("apply", class_run, thread_apply_command,
+ _("Apply a command to a list of threads."),
+ &thread_apply_list, "thread apply ", 1, &thread_cmd_list);
+ set_cmd_async_ok (c);
+ set_cmd_no_selected_thread_ok (c);
- add_cmd ("all", class_run, thread_apply_all_command,
- _("Apply a command to all threads."), &thread_apply_list);
+ c = add_cmd ("all", class_run, thread_apply_all_command,
+ _("Apply a command to all threads."), &thread_apply_list);
+ set_cmd_async_ok (c);
+ set_cmd_no_selected_thread_ok (c);
if (!xdb_commands)
add_com_alias ("t", "thread", class_run, 1);
Index: src/gdb/gdbthread.h
===================================================================
--- src.orig/gdb/gdbthread.h 2008-07-01 23:03:27.000000000 +0100
+++ src/gdb/gdbthread.h 2008-07-01 23:50:48.000000000 +0100
@@ -45,15 +45,21 @@ struct thread_info
use is_executing instead. */
int executing_;
- /* Frontend view of the running state. Note that this is different
- from EXECUTING. When the thread is stopped internally while
- handling an internal event, like a software single-step
- breakpoint, executing will be false, but running will still be
- true. As a possible future extension, this could turn into
- enum { stopped, stepping, finishing, until(ling), ... } */
+ /* Frontend view of the thread state. Note that the RUNNING/STOPPED
+ states are different from EXECUTING. When the thread is stopped
+ internally while handling an internal event, like a software
+ single-step breakpoint, EXECUTING will be false, but running will
+ still be true. As a possible future extension, this could turn
+ into enum { stopped, exited, stepping, finishing, until(ling),
+ running ... } */
/* This field is internal to thread.c. Never access it directly,
use is_running instead. */
- int running_;
+ int state_;
+
+ /* If this is > 0, then it means there's code out there that relies
+ on this thread being listed. Don't delete it from the lists even
+ if we detect it exiting. */
+ int refcount;
/* State from wait_for_inferior */
CORE_ADDR prev_pc;
@@ -202,6 +208,13 @@ extern int is_running (ptid_t ptid);
/* Reports if any thread is known to be running right now. */
extern int any_running (void);
+/* Is this thread listed, but known to have exited? We keep it listed
+ (but not visible) until it's safe to delete. */
+extern int is_exited (ptid_t ptid);
+
+/* Is this thread stopped? */
+extern int is_stopped (ptid_t ptid);
+
/* Marks thread PTID as executing, or as stopped.
If PIDGET (PTID) is -1, marks all threads. */
extern void set_executing (ptid_t ptid, int executing);
@@ -218,8 +231,7 @@ extern int print_thread_events;
extern void print_thread_info (struct ui_out *uiout, int thread);
-extern struct cleanup *make_cleanup_restore_current_thread (ptid_t,
- struct frame_id);
+extern struct cleanup *make_cleanup_restore_current_thread (void);
#endif /* GDBTHREAD_H */
Index: src/gdb/infrun.c
===================================================================
--- src.orig/gdb/infrun.c 2008-07-01 23:03:37.000000000 +0100
+++ src/gdb/infrun.c 2008-07-01 23:50:48.000000000 +0100
@@ -48,6 +48,7 @@
#include "gdb_assert.h"
#include "mi/mi-common.h"
+#include "event-top.h"
/* Prototypes for local functions */
@@ -1526,11 +1527,20 @@ fetch_inferior_event (void *client_data)
{
struct execution_control_state ecss;
struct execution_control_state *ecs = &ecss;
+ struct cleanup *old_chain = make_cleanup (null_cleanup, NULL);
+ int was_sync = sync_execution;
memset (ecs, 0, sizeof (*ecs));
overlay_cache_invalid = 1;
+ if (non_stop)
+ /* In non-stop mode, the user/frontend should not notice a thread
+ switch due to internal events. Make sure we reverse to the
+ user selected thread and frame after handling the event and
+ running any breakpoint commands. */
+ make_cleanup_restore_current_thread ();
+
/* We have to invalidate the registers BEFORE calling target_wait
because they can be loaded from the target while in target_wait.
This makes remote debugging a bit more efficient for those
@@ -1567,6 +1577,14 @@ fetch_inferior_event (void *client_data)
else
inferior_event_handler (INF_EXEC_COMPLETE, NULL);
}
+
+ /* Revert thread and frame. */
+ do_cleanups (old_chain);
+
+ /* If the inferior was in sync execution mode, and now isn't,
+ restore the prompt. */
+ if (was_sync && !sync_execution)
+ display_gdb_prompt (0);
}
/* Prepare an execution control state for looping through a
@@ -3705,6 +3723,11 @@ normal_stop (void)
get_last_target_status (&last_ptid, &last);
+ /* In non-stop mode, we don't want GDB to switch threads on the
+ user's back, to avoid races where the user is typing a command to
+ apply to thread x, but GDB switches to thread y before the user
+ finishes entering the command. */
+
/* As with the notification of thread events, we want to delay
notifying the user that we've switched thread context until
the inferior actually stops.
@@ -3712,7 +3735,8 @@ normal_stop (void)
There's no point in saying anything if the inferior has exited.
Note that SIGNALLED here means "exited with a signal", not
"received a signal". */
- if (!ptid_equal (previous_inferior_ptid, inferior_ptid)
+ if (!non_stop
+ && !ptid_equal (previous_inferior_ptid, inferior_ptid)
&& target_has_execution
&& last.kind != TARGET_WAITKIND_SIGNALLED
&& last.kind != TARGET_WAITKIND_EXITED)
Index: src/gdb/cli/cli-decode.c
===================================================================
--- src.orig/gdb/cli/cli-decode.c 2008-07-01 23:03:27.000000000 +0100
+++ src/gdb/cli/cli-decode.c 2008-07-01 23:50:48.000000000 +0100
@@ -117,6 +117,18 @@ get_cmd_async_ok (struct cmd_list_elemen
return cmd->flags & CMD_ASYNC_OK;
}
+void
+set_cmd_no_selected_thread_ok (struct cmd_list_element *cmd)
+{
+ cmd->flags |= CMD_NO_SELECTED_THREAD_OK;
+}
+
+int
+get_cmd_no_selected_thread_ok (struct cmd_list_element *cmd)
+{
+ return cmd->flags & CMD_NO_SELECTED_THREAD_OK;
+}
+
enum cmd_types
cmd_type (struct cmd_list_element *cmd)
{
Index: src/gdb/cli/cli-decode.h
===================================================================
--- src.orig/gdb/cli/cli-decode.h 2008-07-01 23:03:27.000000000 +0100
+++ src/gdb/cli/cli-decode.h 2008-07-01 23:50:48.000000000 +0100
@@ -51,6 +51,10 @@ cmd_types;
/* This flag is set if the command is allowed during async execution. */
#define CMD_ASYNC_OK 0x8
+/* This flag is set if the command is allowed to run when the target
+ has execution, but there's no selected thread. */
+#define CMD_NO_SELECTED_THREAD_OK 0x10
+
struct cmd_list_element
{
/* Points to next command in this list. */
@@ -253,6 +257,13 @@ extern void set_cmd_async_ok (struct cmd
/* Return true if command is async-ok. */
extern int get_cmd_async_ok (struct cmd_list_element *);
+/* Mark command as ok to call when there is no selected thread. There
+ is no way to disable this once set. */
+extern void set_cmd_no_selected_thread_ok (struct cmd_list_element *);
+
+/* Return true if command is no-selected-thread-ok. */
+extern int get_cmd_no_selected_thread_ok (struct cmd_list_element *);
+
extern struct cmd_list_element *lookup_cmd (char **,
struct cmd_list_element *, char *,
int, int);
Index: src/gdb/top.c
===================================================================
--- src.orig/gdb/top.c 2008-07-01 23:03:27.000000000 +0100
+++ src/gdb/top.c 2008-07-01 23:50:48.000000000 +0100
@@ -362,6 +362,8 @@ do_chdir_cleanup (void *old_dir)
/* 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 (char *p, int from_tty)
{
@@ -415,12 +417,19 @@ execute_command (char *p, int from_tty)
c = lookup_cmd (&p, cmdlist, "", 0, 1);
- /* If the target is running, we allow only a limited set of
- commands. */
+ /* If the selected thread has terminated, we allow only a
+ limited set of commands. */
if (target_can_async_p ()
- && ((!non_stop && any_running ())
- || (non_stop && is_running (inferior_ptid)))
- && !get_cmd_async_ok (c))
+ && is_exited (inferior_ptid)
+ && !get_cmd_no_selected_thread_ok (c))
+ error (_("\
+Cannot execute this command without a live selected thread. See `help thread'."));
+ /* If the target is running, we allow only a limited set of
+ commands. */
+ else if (target_can_async_p ()
+ && ((!non_stop && any_running ())
+ || (non_stop && is_running (inferior_ptid)))
+ && !get_cmd_async_ok (c))
error (_("Cannot execute this command while the target is running."));
/* Pass null arg rather than an empty one. */
Index: src/gdb/linux-nat.c
===================================================================
--- src.orig/gdb/linux-nat.c 2008-07-01 23:50:47.000000000 +0100
+++ src/gdb/linux-nat.c 2008-07-02 02:47:29.000000000 +0100
@@ -1043,48 +1043,12 @@ linux_nat_switch_fork (ptid_t new_ptid)
{
struct lwp_info *lp;
- init_thread_list ();
init_lwp_list ();
lp = add_lwp (new_ptid);
- add_thread_silent (new_ptid);
lp->stopped = 1;
-}
-
-/* Record a PTID for later deletion. */
-
-struct saved_ptids
-{
- ptid_t ptid;
- struct saved_ptids *next;
-};
-static struct saved_ptids *threads_to_delete;
-
-static void
-record_dead_thread (ptid_t ptid)
-{
- struct saved_ptids *p = xmalloc (sizeof (struct saved_ptids));
- p->ptid = ptid;
- p->next = threads_to_delete;
- threads_to_delete = p;
-}
-/* Delete any dead threads which are not the current thread. */
-
-static void
-prune_lwps (void)
-{
- struct saved_ptids **p = &threads_to_delete;
-
- while (*p)
- if (! ptid_equal ((*p)->ptid, inferior_ptid))
- {
- struct saved_ptids *tmp = *p;
- delete_thread (tmp->ptid);
- *p = tmp->next;
- xfree (tmp);
- }
- else
- p = &(*p)->next;
+ init_thread_list ();
+ add_thread_silent (new_ptid);
}
/* Handle the exit of a single thread LP. */
@@ -1099,11 +1063,7 @@ exit_lwp (struct lwp_info *lp)
if (print_thread_events)
printf_unfiltered (_("[%s exited]\n"), target_pid_to_str (lp->ptid));
- /* Core GDB cannot deal with us deleting the current thread. */
- if (!ptid_equal (lp->ptid, inferior_ptid))
- delete_thread (lp->ptid);
- else
- record_dead_thread (lp->ptid);
+ delete_thread (lp->ptid);
}
delete_lwp (lp->ptid);
@@ -1610,8 +1570,6 @@ linux_nat_resume (ptid_t ptid, int step,
signo ? strsignal (signo) : "0",
target_pid_to_str (inferior_ptid));
- prune_lwps ();
-
if (target_can_async_p ())
/* Block events while we're here. */
linux_nat_async_events (sigchld_sync);
Index: src/gdb/infcmd.c
===================================================================
--- src.orig/gdb/infcmd.c 2008-07-01 23:50:47.000000000 +0100
+++ src/gdb/infcmd.c 2008-07-01 23:50:48.000000000 +0100
@@ -611,7 +611,7 @@ start_command (char *args, int from_tty)
static int
proceed_thread_callback (struct thread_info *thread, void *arg)
{
- if (is_running (thread->ptid))
+ if (!is_stopped (thread->ptid))
return 0;
context_switch_to (thread->ptid);
@@ -696,19 +696,13 @@ Can't resume all threads and specify pro
if (non_stop && all_threads)
{
- struct cleanup *old_chain;
- struct frame_id saved_frame_id;
-
/* Don't error out if the current thread is running, because
there may be other stopped threads. */
+ struct cleanup *old_chain;
/* 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 ();
- old_chain = make_cleanup_restore_current_thread (inferior_ptid, saved_frame_id);
iterate_over_threads (proceed_thread_callback, NULL);
/* Restore selected ptid. */
Index: src/gdb/cli/cli-cmds.c
===================================================================
--- src.orig/gdb/cli/cli-cmds.c 2008-07-01 23:03:27.000000000 +0100
+++ src/gdb/cli/cli-cmds.c 2008-07-01 23:50:48.000000000 +0100
@@ -1245,6 +1245,8 @@ The commands below can be used to select
c = add_com ("pwd", class_files, pwd_command, _("\
Print working directory. This is used for your program as well."));
set_cmd_async_ok (c);
+ set_cmd_no_selected_thread_ok (c);
+
c = add_cmd ("cd", class_files, cd_command, _("\
Set working directory to DIR for debugger and program being debugged.\n\
The change does not take effect for the program being debugged\n\
@@ -1285,6 +1287,7 @@ when GDB is started."), gdbinit);
_("Print list of commands."));
set_cmd_completer (c, command_completer);
set_cmd_async_ok (c);
+ set_cmd_no_selected_thread_ok (c);
add_com_alias ("q", "quit", class_support, 1);
add_com_alias ("h", "help", class_support, 1);
@@ -1314,6 +1317,7 @@ Without an argument, history expansion i
Generic command for showing things about the program being debugged."),
&infolist, "info ", 0, &cmdlist);
set_cmd_async_ok (c);
+ set_cmd_no_selected_thread_ok (c);
add_com_alias ("i", "info", class_info, 1);
add_com ("complete", class_obscure, complete_command,
@@ -1323,6 +1327,7 @@ Generic command for showing things about
Generic command for showing things about the debugger."),
&showlist, "show ", 0, &cmdlist);
set_cmd_async_ok (c);
+ set_cmd_no_selected_thread_ok (c);
/* Another way to get at the same thing. */
add_info ("set", show_command, _("Show all GDB settings."));
Index: src/gdb/mi/mi-main.c
===================================================================
--- src.orig/gdb/mi/mi-main.c 2008-07-01 23:03:27.000000000 +0100
+++ src/gdb/mi/mi-main.c 2008-07-02 02:47:55.000000000 +0100
@@ -1059,6 +1059,24 @@ mi_cmd_execute (struct mi_parse *parse)
if (parse->cmd->argv_func != NULL)
{
+ if (target_can_async_p ()
+ && target_has_execution
+ && (is_exited (inferior_ptid))
+ && (strcmp (parse->command, "thread-info") != 0
+ && strcmp (parse->command, "thread-list-ids") != 0
+ && strcmp (parse->command, "thread-select") != 0))
+ {
+ struct ui_file *stb;
+ stb = mem_fileopen ();
+
+ fputs_unfiltered ("Cannot execute command ", stb);
+ fputstr_unfiltered (parse->command, '"', stb);
+ fputs_unfiltered (" without a selected thread", stb);
+
+ make_cleanup_ui_file_delete (stb);
+ error_stream (stb);
+ }
+
if ((!non_stop && any_running ())
|| (non_stop && is_running (inferior_ptid)))
{
Index: src/gdb/inf-loop.c
===================================================================
--- src.orig/gdb/inf-loop.c 2008-07-01 23:03:37.000000000 +0100
+++ src/gdb/inf-loop.c 2008-07-01 23:50:48.000000000 +0100
@@ -116,20 +116,8 @@ inferior_event_handler (enum inferior_ev
bpstat_do_actions (&stop_bpstat);
}
- /* If no breakpoint command resumed the inferior, prepare for
- interaction with the user. */
- if (!is_running (inferior_ptid))
- {
- if (was_sync)
- {
- display_gdb_prompt (0);
- }
- else
- {
- if (exec_done_display_p)
- printf_unfiltered (_("completed.\n"));
- }
- }
+ if (!was_sync && !is_running (inferior_ptid) && exec_done_display_p)
+ printf_unfiltered (_("completed.\n"));
break;
case INF_EXEC_CONTINUE:
Index: src/gdb/varobj.c
===================================================================
--- src.orig/gdb/varobj.c 2008-07-01 23:03:27.000000000 +0100
+++ src/gdb/varobj.c 2008-07-01 23:50:48.000000000 +0100
@@ -2198,8 +2198,7 @@ c_value_of_root (struct varobj **var_han
/* Not a root var */
return NULL;
- back_to = make_cleanup_restore_current_thread (
- inferior_ptid, get_frame_id (deprecated_safe_get_selected_frame ()));
+ back_to = make_cleanup_restore_current_thread ();
/* Determine whether the variable is still around. */
if (var->root->valid_block == NULL || var->root->floating)
Index: src/gdb/Makefile.in
===================================================================
--- src.orig/gdb/Makefile.in 2008-07-01 23:03:37.000000000 +0100
+++ src/gdb/Makefile.in 2008-07-02 03:11:08.000000000 +0100
@@ -2335,7 +2335,7 @@ infrun.o: infrun.c $(defs_h) $(gdb_strin
$(gdbcore_h) $(gdbcmd_h) $(cli_script_h) $(target_h) $(gdbthread_h) \
$(annotate_h) $(symfile_h) $(top_h) $(inf_loop_h) $(regcache_h) \
$(value_h) $(observer_h) $(language_h) $(solib_h) $(gdb_assert_h) \
- $(mi_common_h) $(main_h)
+ $(mi_common_h) $(main_h) $(event_top_h)
inf-ttrace.o: inf-ttrace.c $(defs_h) $(command_h) $(gdbcore_h) \
$(gdbthread_h) $(inferior_h) $(target_h) \
$(gdb_assert_h) $(gdb_string_h) $(inf_child_h) $(inf_ttrace_h)
Index: src/gdb/defs.h
===================================================================
--- src.orig/gdb/defs.h 2008-07-02 00:32:28.000000000 +0100
+++ src/gdb/defs.h 2008-07-02 00:33:06.000000000 +0100
@@ -333,6 +333,9 @@ typedef void (make_cleanup_ftype) (void
extern struct cleanup *make_cleanup (make_cleanup_ftype *, void *);
+extern struct cleanup *make_cleanup_dtor (make_cleanup_ftype *, void *,
+ void (*dtor) (void *));
+
extern struct cleanup *make_cleanup_freeargv (char **);
struct ui_file;
Index: src/gdb/utils.c
===================================================================
--- src.orig/gdb/utils.c 2008-07-02 00:32:29.000000000 +0100
+++ src/gdb/utils.c 2008-07-02 00:34:00.000000000 +0100
@@ -208,6 +208,14 @@ make_cleanup (make_cleanup_ftype *functi
}
struct cleanup *
+make_cleanup_dtor (make_cleanup_ftype *function, void *arg,
+ void (*dtor) (void *))
+{
+ return make_my_cleanup2 (&cleanup_chain,
+ function, arg, dtor);
+}
+
+struct cleanup *
make_final_cleanup (make_cleanup_ftype *function, void *arg)
{
return make_my_cleanup (&final_cleanup_chain, function, arg);
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [non-stop] 10/10 handle exited threads [was: Re: [non-stop] 10/10 split user/internal threads]
2008-07-02 3:39 ` [non-stop] 10/10 handle exited threads [was: Re: [non-stop] 10/10 split user/internal threads] Pedro Alves
@ 2008-07-07 18:59 ` Daniel Jacobowitz
2008-07-11 11:16 ` Pedro Alves
0 siblings, 1 reply; 11+ messages in thread
From: Daniel Jacobowitz @ 2008-07-07 18:59 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
On Wed, Jul 02, 2008 at 04:39:07AM +0100, Pedro Alves wrote:
> (print_thread_info): Update. Account for exited threads. Don't
> warn about missed frame restoring here, it's done in the cleanup.
its
> + /* Since the context is already set to this new thread,
> + reset it's ptid, and reswitch inferior_ptid to it. */
and here, too.
> @@ -163,6 +223,10 @@ add_thread (ptid_t ptid)
> return add_thread_with_info (ptid, NULL);
> }
>
> +/* Delete thread PTID and notify of thread exit. If this is
> + inferior_ptid, don't actually delete it, but tag it as exited and
> + do the notification. If PTID is the user selected thread, clear
> + it. */
> void
> delete_thread (ptid_t ptid)
> {
> @@ -177,12 +241,35 @@ delete_thread (ptid_t ptid)
> if (!tp)
> return;
>
> + /* If this is the current thread, or there's code out there that
> + relies on it existing (refcout > 0) we can't delete yet. Mark it
> + as exited, and notify it. */
refcount
> @@ -747,21 +869,64 @@ restore_current_thread (ptid_t ptid)
> {
> if (!ptid_equal (ptid, inferior_ptid))
> {
> - switch_to_thread (ptid);
> + if (non_stop)
> + context_switch_to (ptid);
> + else
> + switch_to_thread (ptid);
> }
> }
>
> static void
> -restore_selected_frame (struct frame_id a_frame_id)
> +restore_selected_frame (struct frame_id a_frame_id, int frame_level)
> {
> - struct frame_info *selected_frame_info = NULL;
> + struct frame_info *frame = NULL;
> + int count;
>
> - if (frame_id_eq (a_frame_id, null_frame_id))
> - return;
> + gdb_assert (frame_level >= 0);
>
> - if ((selected_frame_info = frame_find_by_id (a_frame_id)) != NULL)
> + /* 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 = frame_level;
> + frame = find_relative_frame (get_current_frame (), &count);
> + if (count == 0
> + && frame != NULL
> + /* Either the frame ids match, of they're both invalid.
> + The later case is not failsafe, but since it's highly
> + 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), a_frame_id)
> + /* Note: could be better to check every frame_id
> + member for equality here. */
> + || (!frame_id_p (get_frame_id (frame))
> + && !frame_id_p (a_frame_id))))
Indentation, spelling; "latter" and "unlikely" and "of the time".
> @@ -877,18 +1057,10 @@ thread_apply_command (char *tidlist, int
> if (*cmd == '\000')
> error (_("Please specify a command following the thread ID list"));
>
> - current_ptid = inferior_ptid;
> -
> - 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);
> -
> /* Save a copy of the command in case it is clobbered by
> execute_command */
> saved_cmd = xstrdup (cmd);
> - saved_cmd_cleanup_chain = make_cleanup (xfree, (void *) saved_cmd);
> + old_chain = make_cleanup (xfree, saved_cmd);
> while (tidlist < cmd)
> {
> struct thread_info *tp;
> @@ -926,26 +1098,24 @@ thread_apply_command (char *tidlist, int
> warning (_("Thread %d has terminated."), start);
> else
> {
> + make_cleanup_restore_current_thread ();
> +
> if (non_stop)
> context_switch_to (tp->ptid);
> else
> switch_to_thread (tp->ptid);
> +
> printf_filtered (_("\nThread %d (%s):\n"), tp->num,
> target_tid_to_str (inferior_ptid));
> execute_command (cmd, from_tty);
> - strcpy (cmd, saved_cmd); /* Restore exact command used previously */
> +
> + /* Restore exact command used previously. */
> + strcpy (cmd, saved_cmd);
> }
> }
> }
>
> - if (!ptid_equal (current_ptid, inferior_ptid))
> - thread_has_changed = 1;
> -
> - do_cleanups (saved_cmd_cleanup_chain);
> do_cleanups (old_chain);
> - /* Print stack frame only if we changed thread. */
> - if (thread_has_changed)
> - print_stack_frame (get_current_frame (), 1, SRC_LINE);
> }
>
> /* Switch to the specified thread. Will dispatch off to thread_apply_command
You've moved creation of the cleanup into the loop, but not moved the
call to do_cleanups. Do we ever need more than one cleanup for a
thread apply command?
> Index: src/gdb/infrun.c
> ===================================================================
> --- src.orig/gdb/infrun.c 2008-07-01 23:03:37.000000000 +0100
> +++ src/gdb/infrun.c 2008-07-01 23:50:48.000000000 +0100
> @@ -48,6 +48,7 @@
>
> #include "gdb_assert.h"
> #include "mi/mi-common.h"
> +#include "event-top.h"
>
> /* Prototypes for local functions */
>
> @@ -1526,11 +1527,20 @@ fetch_inferior_event (void *client_data)
> {
> struct execution_control_state ecss;
> struct execution_control_state *ecs = &ecss;
> + struct cleanup *old_chain = make_cleanup (null_cleanup, NULL);
> + int was_sync = sync_execution;
>
> memset (ecs, 0, sizeof (*ecs));
>
> overlay_cache_invalid = 1;
>
> + if (non_stop)
> + /* In non-stop mode, the user/frontend should not notice a thread
> + switch due to internal events. Make sure we reverse to the
> + user selected thread and frame after handling the event and
> + running any breakpoint commands. */
> + make_cleanup_restore_current_thread ();
> +
> /* We have to invalidate the registers BEFORE calling target_wait
> because they can be loaded from the target while in target_wait.
> This makes remote debugging a bit more efficient for those
> @@ -1567,6 +1577,14 @@ fetch_inferior_event (void *client_data)
> else
> inferior_event_handler (INF_EXEC_COMPLETE, NULL);
> }
> +
> + /* Revert thread and frame. */
> + do_cleanups (old_chain);
> +
> + /* If the inferior was in sync execution mode, and now isn't,
> + restore the prompt. */
> + if (was_sync && !sync_execution)
> + display_gdb_prompt (0);
> }
>
> /* Prepare an execution control state for looping through a
> @@ -3705,6 +3723,11 @@ normal_stop (void)
>
> get_last_target_status (&last_ptid, &last);
>
> + /* In non-stop mode, we don't want GDB to switch threads on the
> + user's back, to avoid races where the user is typing a command to
> + apply to thread x, but GDB switches to thread y before the user
> + finishes entering the command. */
behind the user's back
Otherwise, OK.
--
Daniel Jacobowitz
CodeSourcery
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [non-stop] 10/10 handle exited threads [was: Re: [non-stop] 10/10 split user/internal threads]
2008-07-07 18:59 ` Daniel Jacobowitz
@ 2008-07-11 11:16 ` Pedro Alves
2008-07-11 11:29 ` Pedro Alves
0 siblings, 1 reply; 11+ messages in thread
From: Pedro Alves @ 2008-07-11 11:16 UTC (permalink / raw)
To: Daniel Jacobowitz; +Cc: gdb-patches
A Monday 07 July 2008 19:59:30, Daniel Jacobowitz wrote:
> On Wed, Jul 02, 2008 at 04:39:07AM +0100, Pedro Alves wrote:
> > (print_thread_info): Update. Account for exited threads. Don't
> > warn about missed frame restoring here, it's done in the cleanup.
>
> its
Fixed.
>
> > + /* Since the context is already set to this new thread,
> > + reset it's ptid, and reswitch inferior_ptid to it. */
>
> and here, too.
Fixed.
>
> > @@ -163,6 +223,10 @@ add_thread (ptid_t ptid)
> > return add_thread_with_info (ptid, NULL);
> > }
> >
> > +/* Delete thread PTID and notify of thread exit. If this is
> > + inferior_ptid, don't actually delete it, but tag it as exited and
> > + do the notification. If PTID is the user selected thread, clear
> > + it. */
> > void
> > delete_thread (ptid_t ptid)
> > {
> > @@ -177,12 +241,35 @@ delete_thread (ptid_t ptid)
> > if (!tp)
> > return;
> >
> > + /* If this is the current thread, or there's code out there that
> > + relies on it existing (refcout > 0) we can't delete yet. Mark it
> > + as exited, and notify it. */
>
> refcount
Fixed.
> > + count = frame_level;
> > + frame = find_relative_frame (get_current_frame (), &count);
> > + if (count == 0
> > + && frame != NULL
> > + /* Either the frame ids match, of they're both invalid.
> > + The later case is not failsafe, but since it's highly
> > + 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), a_frame_id)
> > + /* Note: could be better to check every frame_id
> > + member for equality here. */
> > + || (!frame_id_p (get_frame_id (frame))
> > + && !frame_id_p (a_frame_id))))
>
> Indentation, spelling; "latter" and "unlikely" and "of the time".
Fixed.
>
> > @@ -877,18 +1057,10 @@ thread_apply_command (char *tidlist, int
> > if (*cmd == '\000')
> > error (_("Please specify a command following the thread ID list"));
> >
> > - current_ptid = inferior_ptid;
> > -
> > - 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); -
> > /* Save a copy of the command in case it is clobbered by
> > execute_command */
> > saved_cmd = xstrdup (cmd);
> > - saved_cmd_cleanup_chain = make_cleanup (xfree, (void *) saved_cmd);
> > + old_chain = make_cleanup (xfree, saved_cmd);
> > while (tidlist < cmd)
> > {
> > struct thread_info *tp;
> > @@ -926,26 +1098,24 @@ thread_apply_command (char *tidlist, int
> > warning (_("Thread %d has terminated."), start);
> > else
> > {
> > + make_cleanup_restore_current_thread ();
> > +
> > if (non_stop)
> > context_switch_to (tp->ptid);
> > else
> > switch_to_thread (tp->ptid);
> > +
> > printf_filtered (_("\nThread %d (%s):\n"), tp->num,
> > target_tid_to_str (inferior_ptid));
> > execute_command (cmd, from_tty);
> > - strcpy (cmd, saved_cmd); /* Restore exact command used previously
> > */ +
> > + /* Restore exact command used previously. */
> > + strcpy (cmd, saved_cmd);
> > }
> > }
> > }
> >
> > - if (!ptid_equal (current_ptid, inferior_ptid))
> > - thread_has_changed = 1;
> > -
> > - do_cleanups (saved_cmd_cleanup_chain);
> > do_cleanups (old_chain);
> > - /* Print stack frame only if we changed thread. */
> > - if (thread_has_changed)
> > - print_stack_frame (get_current_frame (), 1, SRC_LINE);
> > }
> >
> > /* Switch to the specified thread. Will dispatch off to
> > thread_apply_command
>
> You've moved creation of the cleanup into the loop, but not moved the
> call to do_cleanups. Do we ever need more than one cleanup for a
> thread apply command?
>
Ooops, sorry, I missed this, and committed without taking care of it. Let
me come back to it in a sec, should be a minor change.
> > + /* In non-stop mode, we don't want GDB to switch threads on the
> > + user's back, to avoid races where the user is typing a command to
> > + apply to thread x, but GDB switches to thread y before the user
> > + finishes entering the command. */
>
> behind the user's back
Fixed.
> Otherwise, OK.
Thanks! I've checked it in.
Next up, docs.
--
Pedro Alves
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [non-stop] 10/10 handle exited threads [was: Re: [non-stop] 10/10 split user/internal threads]
2008-07-11 11:16 ` Pedro Alves
@ 2008-07-11 11:29 ` Pedro Alves
0 siblings, 0 replies; 11+ messages in thread
From: Pedro Alves @ 2008-07-11 11:29 UTC (permalink / raw)
To: gdb-patches; +Cc: Daniel Jacobowitz
[-- Attachment #1: Type: text/plain, Size: 2669 bytes --]
On Friday 11 July 2008 12:16:15, Pedro Alves wrote:
> On Monday 07 July 2008 19:59:30, Daniel Jacobowitz wrote:
> > > @@ -877,18 +1057,10 @@ thread_apply_command (char *tidlist, int
> > > if (*cmd == '\000')
> > > error (_("Please specify a command following the thread ID
> > > list"));
> > >
> > > - current_ptid = inferior_ptid;
> > > -
> > > - 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); -
> > > /* Save a copy of the command in case it is clobbered by
> > > execute_command */
> > > saved_cmd = xstrdup (cmd);
> > > - saved_cmd_cleanup_chain = make_cleanup (xfree, (void *) saved_cmd);
> > > + old_chain = make_cleanup (xfree, saved_cmd);
> > > while (tidlist < cmd)
> > > {
> > > struct thread_info *tp;
> > > @@ -926,26 +1098,24 @@ thread_apply_command (char *tidlist, int
> > > warning (_("Thread %d has terminated."), start);
> > > else
> > > {
> > > + make_cleanup_restore_current_thread ();
> > > +
> > > if (non_stop)
> > > context_switch_to (tp->ptid);
> > > else
> > > switch_to_thread (tp->ptid);
> > > +
> > > printf_filtered (_("\nThread %d (%s):\n"), tp->num,
> > > target_tid_to_str (inferior_ptid));
> > > execute_command (cmd, from_tty);
> > > - strcpy (cmd, saved_cmd); /* Restore exact command used
> > > previously */ +
> > > + /* Restore exact command used previously. */
> > > + strcpy (cmd, saved_cmd);
> > > }
> > > }
> > > }
> > >
> > > - if (!ptid_equal (current_ptid, inferior_ptid))
> > > - thread_has_changed = 1;
> > > -
> > > - do_cleanups (saved_cmd_cleanup_chain);
> > > do_cleanups (old_chain);
> > > - /* Print stack frame only if we changed thread. */
> > > - if (thread_has_changed)
> > > - print_stack_frame (get_current_frame (), 1, SRC_LINE);
> > > }
> > >
> > > /* Switch to the specified thread. Will dispatch off to
> > > thread_apply_command
> >
> > You've moved creation of the cleanup into the loop, but not moved the
> > call to do_cleanups. Do we ever need more than one cleanup for a
> > thread apply command?
>
> Ooops, sorry, I missed this, and committed without taking care of it. Let
> me come back to it in a sec, should be a minor change.
I've checked in the attached, to move making the cleanup out of the loop.
It doesn't need to be sooner, as the only thing we do before is checking
for args and throwing errors.
--
Pedro Alves
[-- Attachment #2: fix_thread_apply.diff --]
[-- Type: text/x-diff, Size: 889 bytes --]
2008-07-11 Pedro Alves <pedro@codesourcery.com>
* thread.c (thread_apply_command): Move making the cleanup out of
the loop.
---
gdb/thread.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Index: src/gdb/thread.c
===================================================================
--- src.orig/gdb/thread.c 2008-07-11 12:16:32.000000000 +0100
+++ src/gdb/thread.c 2008-07-11 12:23:43.000000000 +0100
@@ -1102,6 +1102,8 @@ thread_apply_command (char *tidlist, int
else
end = start;
+ make_cleanup_restore_current_thread ();
+
for (; start <= end; start++)
{
tp = find_thread_id (start);
@@ -1112,8 +1114,6 @@ thread_apply_command (char *tidlist, int
warning (_("Thread %d has terminated."), start);
else
{
- make_cleanup_restore_current_thread ();
-
if (non_stop)
context_switch_to (tp->ptid);
else
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2008-07-11 11:29 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-06-15 21:49 [non-stop] 10/10 split user/internal threads Pedro Alves
2008-06-26 13:41 ` Daniel Jacobowitz
2008-06-30 12:21 ` Pedro Alves
2008-07-01 0:51 ` Daniel Jacobowitz
2008-07-01 13:38 ` Pedro Alves
2008-07-01 14:03 ` Daniel Jacobowitz
2008-07-02 3:29 ` Pedro Alves
2008-07-02 3:39 ` [non-stop] 10/10 handle exited threads [was: Re: [non-stop] 10/10 split user/internal threads] Pedro Alves
2008-07-07 18:59 ` Daniel Jacobowitz
2008-07-11 11:16 ` Pedro Alves
2008-07-11 11:29 ` Pedro Alves
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox