Process exit in multi-process, and gdb's selected thread.

Process exit in multi-process, and gdb's selected thread.

[Pedro Alves]

[-- Attachment #1: Type: text/plain, Size: 3174 bytes --]

Hi!

I've been thinking about issues related to what to do with GDB's
notion of selected thread, if we're debugging more than one process
simultaneously, and the current processes exits.

What would you think if GDB could get into this state,
after a process exit? :

 (gdb) info threads
   2 Thread 31176.31176  0x00007f0706154796 in ?? ()

 No selected thread.  See `help thread'.
 (gdb) info inferiors
   3 31176
 (gdb) c
 Continuing.
 Cannot execute this command without a live selected thread.
 (gdb) detach
 The program is not being run.
 (gdb) kill
 The program is not being run.
 (gdb)   

This would happen, e.g., if the current process (it was some other
pid, not 31176) had exited or was detached from.  Keep in mind that in
non-stop mode, we prefer that GDB doesn't switch threads behind the
user's back while he is typing some command. This way, when the
current thread and inferior no longer exist, GDB switches to a state that
should be safe, and the onus is on the frontend to switch to
whatever thread it wants.

In the past, I had solved this by spreading around some hacks
that tried to detect the current inferior exiting, and switching
to any other random live thread, but, that turned out to be: first,
surprising in non-stop mode, in the case mentioned above; and
second, surprisingly difficult to get right.  I think this usually
means that GDB shouldn't try to be smart (well, or I shouldn't).

I've tried avoiding before getting to a state where GDB has
no thread selected, when there are threads in the thread list.  But,
I don't think we'll be able to avoid it in the long run.  E.g., when
GDB gains ability to load more than on program (exec) at the same time,
(say, "cat", and "grep"), even if "cat" is running already, it will fell
natural to be able to select the "grep" executable as focus, thus,
the selected thread in that case should be "null".   (Also thinking about
the possibility of GDB being able in the future to switch to some form
of post-or-quasi-mortem debugging mode when a process exits, I'd want GDB
to not switch threads to some other process on a process exit.)

A bit related to this, is the hack we have currently that tags a
few commands as being "safe to apply without a valid
selected thread".  This served its purpose when non-stop was first
introduced, but, I think it's time we remove them, and start making
the checks at a lower level.  They are currently done at a too high
level currently, which ends up getting in the way many times.  We need
more finer grained control over which operations can and can't be
applied without a valid thread selected, so we need to push those
checks down, more close to core code, or even to target code.

I'm thinking that we may need to extend the =thread-selected
notification to tell the frontend that there's no thread selected,
and perhaps the -thread-info,current-thread-id, output too.

What do you think of all this, am I making sense?  Or, does it
sound like "let's hope he comes back to senses soon, for he's
not making any"?  :-)

Here's my current patch that implements this, in case you
have a stub around that implements multi-process (Hi Marc!).

-- 
Pedro Alves

[-- Attachment #2: thread_selections.diff --]
[-- Type: text/x-diff, Size: 20386 bytes --]

2009-02-17  Pedro Alves  <pedro@codesourcery.com>

	* infrun.c (normal_stop): Use has_stack_frames instead of
	target_has_stack.
	* mi/mi-main.c (mi_execute_command): Avoid calling inferior_thread
	when there is no thread selected.
	(mi_cmd_execute): Don't special case commands that can run without
	a valid selected thread.
	* top.c (execute_command): Don't special case commands that can
	run without a valid selected thread.  Use has_stack_frames.
	* infcmd.c (ensure_valid_thread): New.
	(continue_1, step_1, jump_command, signal_command): Use it.
	(detach_command): Error out if there's no selected thread/inferior.
	* thread.c (print_thread_info): Allow having no thread selected.
	(switch_to_thread): Don't read the PC if there is no current thread.
	(do_restore_current_thread_cleanup): Don't record the current
	frame if there is no current thread.
	(make_cleanup_restore_current_thread): Don't read frame info if
	there is no selected thread.
	(_initialize_thread): Don't mark commands as
	"no_selected_thread_ok".
	* frame.c (get_current_frame): Error out if there is no valid
	selected thread.
	(has_stack_frames): Return false if there is no valid
	selected thread.
	* cli/cli-cmds.c (init_cli_cmds): Don't mark commands as
	"no_selected_thread_ok".
	* cli/cli-decode.c (set_cmd_no_selected_thread_ok)
	(get_cmd_no_selected_thread_ok): Delete.
	* cli/cli-decode.h (CMD_NO_SELECTED_THREAD_OK): Delete.
	(set_cmd_no_selected_thread_ok, get_cmd_no_selected_thread_ok):
	Delete declaration.
	* stack.c (get_selected_block): Use has_stack_frames.

---
 gdb/cli/cli-cmds.c   |   14 ++------
 gdb/cli/cli-decode.c |   12 ------
 gdb/cli/cli-decode.h |   11 ------
 gdb/frame.c          |   15 +++++++-
 gdb/infcmd.c         |   17 +++++++++
 gdb/infrun.c         |    8 ++--
 gdb/mi/mi-main.c     |   35 ++++----------------
 gdb/remote.c         |   11 ------
 gdb/stack.c          |    8 ----
 gdb/thread.c         |   89 ++++++++++++++++++++++++++++++++-------------------
 gdb/top.c            |   10 -----
 11 files changed, 106 insertions(+), 124 deletions(-)

Index: src/gdb/infrun.c
===================================================================
--- src.orig/gdb/infrun.c	2009-02-15 15:19:29.000000000 +0000
+++ src/gdb/infrun.c	2009-02-16 22:55:02.000000000 +0000
@@ -4213,10 +4213,10 @@ normal_stop (void)
   if (target_has_execution)
     {
       if (!non_stop)
-	old_chain = make_cleanup (finish_thread_state_cleanup, &minus_one_ptid);
+	make_cleanup (finish_thread_state_cleanup, &minus_one_ptid);
       else if (last.kind != TARGET_WAITKIND_SIGNALLED
 	       && last.kind != TARGET_WAITKIND_EXITED)
-	old_chain = make_cleanup (finish_thread_state_cleanup, &inferior_ptid);
+	make_cleanup (finish_thread_state_cleanup, &inferior_ptid);
     }
 
   /* In non-stop mode, we don't want GDB to switch threads behind the
@@ -4275,7 +4275,7 @@ Further execution is probably impossible
   /* Set the current source location.  This will also happen if we
      display the frame below, but the current SAL will be incorrect
      during a user hook-stop function.  */
-  if (target_has_stack && !stop_stack_dummy)
+  if (has_stack_frames () && !stop_stack_dummy)
     set_current_sal_from_frame (get_current_frame (), 1);
 
   /* Let the user/frontend see the threads as stopped.  */
@@ -4287,7 +4287,7 @@ Further execution is probably impossible
     catch_errors (hook_stop_stub, stop_command,
 		  "Error while running hook_stop:\n", RETURN_MASK_ALL);
 
-  if (!target_has_stack)
+  if (!has_stack_frames ())
     goto done;
 
   if (last.kind == TARGET_WAITKIND_SIGNALLED
Index: src/gdb/mi/mi-main.c
===================================================================
--- src.orig/gdb/mi/mi-main.c	2009-01-15 13:15:32.000000000 +0000
+++ src/gdb/mi/mi-main.c	2009-02-16 22:53:42.000000000 +0000
@@ -1279,21 +1279,23 @@ mi_execute_command (char *cmd, int from_
 	  && strcmp (command->command, "thread-select") != 0)
 	{
 	  struct mi_interp *mi = top_level_interpreter_data ();
-	  struct thread_info *ti = inferior_thread ();
-	  int report_change;
+	  int report_change = 0;
 
 	  if (command->thread == -1)
 	    {
-	      report_change = !ptid_equal (previous_ptid, null_ptid)
-		&& !ptid_equal (inferior_ptid, previous_ptid);
+	      report_change = (!ptid_equal (previous_ptid, null_ptid)
+			       && !ptid_equal (inferior_ptid, previous_ptid)
+			       && !ptid_equal (inferior_ptid, null_ptid));
 	    }
-	  else
+	  else if (!ptid_equal (inferior_ptid, null_ptid))
 	    {
+	      struct thread_info *ti = inferior_thread ();
 	      report_change = (ti->num != command->thread);
 	    }
 
 	  if (report_change)
 	    {     
+	      struct thread_info *ti = inferior_thread ();
 	      target_terminal_ours ();
 	      fprintf_unfiltered (mi->event_channel, 
 				  "thread-selected,id=\"%d\"",
@@ -1349,28 +1351,7 @@ 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
-	      && strcmp (parse->command, "list-thread-groups") != 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);
-	}
-
-      parse->cmd->argv_func (parse->command, parse->argv, parse->argc);
-    }
+    parse->cmd->argv_func (parse->command, parse->argv, parse->argc);
   else if (parse->cmd->cli.cmd != 0)
     {
       /* FIXME: DELETE THIS. */
Index: src/gdb/top.c
===================================================================
--- src.orig/gdb/top.c	2009-01-14 13:37:58.000000000 +0000
+++ src/gdb/top.c	2009-02-17 00:46:05.000000000 +0000
@@ -400,14 +400,6 @@ execute_command (char *p, int from_tty)
 
       c = lookup_cmd (&p, cmdlist, "", 0, 1);
 
-      /* If the selected thread has terminated, we allow only a
-	 limited set of commands.  */
-      if (target_can_async_p ()
-	  && is_exited (inferior_ptid)
-	  && !get_cmd_no_selected_thread_ok (c))
-	error (_("\
-Cannot execute this command without a live selected thread.  See `help thread'."));
-
       /* Pass null arg rather than an empty one.  */
       arg = *p ? p : 0;
 
@@ -469,7 +461,7 @@ Cannot execute this command without a li
   /* FIXME:  This should be cacheing the frame and only running when
      the frame changes.  */
 
-  if (target_has_stack && is_stopped (inferior_ptid))
+  if (has_stack_frames ())
     {
       flang = get_frame_language ();
       if (!warned
Index: src/gdb/infcmd.c
===================================================================
--- src.orig/gdb/infcmd.c	2009-02-15 15:19:29.000000000 +0000
+++ src/gdb/infcmd.c	2009-02-16 22:53:42.000000000 +0000
@@ -622,6 +622,15 @@ proceed_thread_callback (struct thread_i
 }
 
 void
+ensure_valid_thread (void)
+{
+  if (ptid_equal (inferior_ptid, null_ptid)
+      || is_exited (inferior_ptid))
+    error (_("\
+Cannot execute this command without a live selected thread."));
+}
+
+void
 continue_1 (int all_threads)
 {
   ERROR_NO_INFERIOR;
@@ -642,6 +651,7 @@ continue_1 (int all_threads)
     }
   else
     {
+      ensure_valid_thread ();
       ensure_not_running ();
       clear_proceed_status ();
       proceed ((CORE_ADDR) -1, TARGET_SIGNAL_DEFAULT, 0);
@@ -786,6 +796,7 @@ step_1 (int skip_subroutines, int single
   int thread = -1;
 
   ERROR_NO_INFERIOR;
+  ensure_valid_thread ();
   ensure_not_running ();
 
   if (count_string)
@@ -996,6 +1007,7 @@ jump_command (char *arg, int from_tty)
   int async_exec = 0;
 
   ERROR_NO_INFERIOR;
+  ensure_valid_thread ();
   ensure_not_running ();
 
   /* Find out whether we must run in the background. */
@@ -1097,6 +1109,7 @@ signal_command (char *signum_exp, int fr
 
   dont_repeat ();		/* Too dangerous.  */
   ERROR_NO_INFERIOR;
+  ensure_valid_thread ();
   ensure_not_running ();
 
   /* Find out whether we must run in the background.  */
@@ -2343,6 +2356,10 @@ void
 detach_command (char *args, int from_tty)
 {
   dont_repeat ();		/* Not for the faint of heart.  */
+
+  if (ptid_equal (inferior_ptid, null_ptid))
+    error (_("The program is not being run."));
+
   target_detach (args, from_tty);
 
   /* If the solist is global across inferiors, don't clear it when we
Index: src/gdb/thread.c
===================================================================
--- src.orig/gdb/thread.c	2009-02-06 23:43:12.000000000 +0000
+++ src/gdb/thread.c	2009-02-17 00:47:51.000000000 +0000
@@ -785,10 +785,11 @@ print_thread_info (struct ui_out *uiout,
      the "info threads" command.  */
   do_cleanups (old_chain);
 
-  if (pid == -1 && requested_thread == -1 )
+  if (pid == -1 && requested_thread == -1)
     {
       gdb_assert (current_thread != -1
-		  || !thread_list);
+		  || !thread_list
+		  || ptid_equal (inferior_ptid, null_ptid));
       if (current_thread != -1 && ui_out_is_mi_like_p (uiout))
 	ui_out_field_int (uiout, "current-thread-id", current_thread);
 
@@ -796,6 +797,11 @@ print_thread_info (struct ui_out *uiout,
 	ui_out_message (uiout, 0, "\n\
 The current thread <Thread ID %d> has terminated.  See `help thread'.\n",
 			current_thread);
+      else if (thread_list
+	       && current_thread == -1
+	       && ptid_equal (current_ptid, null_ptid))
+	ui_out_message (uiout, 0, "\n\
+No selected thread.  See `help thread'.\n");
     }
 }
 
@@ -828,7 +834,9 @@ switch_to_thread (ptid_t 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))
+  if (!ptid_equal (inferior_ptid, null_ptid)
+      && !is_exited (ptid)
+      && !is_executing (ptid))
     stop_pc = read_pc ();
   else
     stop_pc = ~(CORE_ADDR) 0;
@@ -909,11 +917,24 @@ do_restore_current_thread_cleanup (void 
 {
   struct thread_info *tp;
   struct current_thread_cleanup *old = arg;
-  restore_current_thread (old->inferior_ptid);
+
+  tp = find_thread_pid (old->inferior_ptid);
+
+  /* If the previously selected thread belonged to a process that has
+     in the mean time been deleted (due to normal exit, detach, etc.),
+     then don't revert back to it, but instead simply drop back to no
+     thread selected.  */
+  if (tp
+      && is_exited (tp->ptid)
+      && find_inferior_pid (ptid_get_pid (tp->ptid)) == NULL)
+    restore_current_thread (null_ptid);
+  else
+    restore_current_thread (old->inferior_ptid);
 
   /* The running state of the originally selected thread may have
      changed, so we have to recheck it here.  */
-  if (old->was_stopped
+  if (!ptid_equal (inferior_ptid, null_ptid)
+      && old->was_stopped
       && is_stopped (inferior_ptid)
       && target_has_registers
       && target_has_stack
@@ -942,21 +963,25 @@ make_cleanup_restore_current_thread (voi
 
   old = xmalloc (sizeof (struct current_thread_cleanup));
   old->inferior_ptid = inferior_ptid;
-  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);
+  if (!ptid_equal (inferior_ptid, null_ptid))
+    {
+      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;
 
-  tp = find_thread_pid (inferior_ptid);
-  if (tp)
-    tp->refcount++;
+      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);
@@ -1086,6 +1111,9 @@ thread_command (char *tidstr, int from_t
 {
   if (!tidstr)
     {
+      if (ptid_equal (inferior_ptid, null_ptid))
+	error (_("No thread selected"));
+
       if (target_has_stack)
 	{
 	  if (is_exited (inferior_ptid))
@@ -1172,26 +1200,21 @@ void
 _initialize_thread (void)
 {
   static struct cmd_list_element *thread_apply_list = NULL;
-  struct cmd_list_element *c;
 
-  c = add_info ("threads", info_threads_command,
-		_("IDs of currently known threads."));
-  set_cmd_no_selected_thread_ok (c);
+  add_info ("threads", info_threads_command,
+	    _("IDs of currently known threads."));
 
-  c = add_prefix_cmd ("thread", class_run, thread_command, _("\
+  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_no_selected_thread_ok (c);
+		  &thread_cmd_list, "thread ", 1, &cmdlist);
+
+  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_no_selected_thread_ok (c);
-
-  c = add_cmd ("all", class_run, thread_apply_all_command,
-	       _("Apply a command to all threads."), &thread_apply_list);
-  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);
 
   if (!xdb_commands)
     add_com_alias ("t", "thread", class_run, 1);
Index: src/gdb/frame.c
===================================================================
--- src.orig/gdb/frame.c	2009-02-05 17:29:04.000000000 +0000
+++ src/gdb/frame.c	2009-02-16 23:04:27.000000000 +0000
@@ -974,6 +974,10 @@ get_current_frame (void)
     error (_("No stack."));
   if (!target_has_memory)
     error (_("No memory."));
+  if (ptid_equal (inferior_ptid, null_ptid))
+    error (_("No selected thread."));
+  if (is_exited (inferior_ptid))
+    error (_("Invalid selected thread."));
   if (is_executing (inferior_ptid))
     error (_("Target is executing."));
 
@@ -1003,8 +1007,15 @@ has_stack_frames (void)
   if (!target_has_registers || !target_has_stack || !target_has_memory)
     return 0;
 
-  /* If the current thread is executing, don't try to read from
-     it.  */
+  /* No current inferior, no frame.  */
+  if (ptid_equal (inferior_ptid, null_ptid))
+    return 0;
+
+  /* Don't try to read from a dead thread.  */
+  if (is_exited (inferior_ptid))
+    return 0;
+
+  /* ... or from a spinning thread.  */
   if (is_executing (inferior_ptid))
     return 0;
 
Index: src/gdb/cli/cli-cmds.c
===================================================================
--- src.orig/gdb/cli/cli-cmds.c	2009-01-18 01:18:45.000000000 +0000
+++ src/gdb/cli/cli-cmds.c	2009-02-16 22:53:42.000000000 +0000
@@ -1248,9 +1248,8 @@ The commands below can be used to select
 
   /* Define general commands. */
 
-  c = add_com ("pwd", class_files, pwd_command, _("\
+  add_com ("pwd", class_files, pwd_command, _("\
 Print working directory.  This is used for your program as well."));
-  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\
@@ -1291,7 +1290,6 @@ when GDB is started."), gdbinit);
   c = add_com ("help", class_support, help_command,
 	       _("Print list of commands."));
   set_cmd_completer (c, command_completer);
-  set_cmd_no_selected_thread_ok (c);
   add_com_alias ("q", "quit", class_support, 1);
   add_com_alias ("h", "help", class_support, 1);
 
@@ -1317,19 +1315,17 @@ Without an argument, history expansion i
 			   show_history_expansion_p,
 			   &sethistlist, &showhistlist);
 
-  c = add_prefix_cmd ("info", class_info, info_command, _("\
+  add_prefix_cmd ("info", class_info, info_command, _("\
 Generic command for showing things about the program being debugged."),
-		      &infolist, "info ", 0, &cmdlist);
-  set_cmd_no_selected_thread_ok (c);
+		  &infolist, "info ", 0, &cmdlist);
   add_com_alias ("i", "info", class_info, 1);
 
   add_com ("complete", class_obscure, complete_command,
 	   _("List the completions for the rest of the line as a command."));
 
-  c = add_prefix_cmd ("show", class_info, show_command, _("\
+  add_prefix_cmd ("show", class_info, show_command, _("\
 Generic command for showing things about the debugger."),
-		      &showlist, "show ", 0, &cmdlist);
-  set_cmd_no_selected_thread_ok (c);
+		  &showlist, "show ", 0, &cmdlist);
   /* Another way to get at the same thing.  */
   add_info ("set", show_command, _("Show all GDB settings."));
 
Index: src/gdb/cli/cli-decode.c
===================================================================
--- src.orig/gdb/cli/cli-decode.c	2009-02-06 23:43:12.000000000 +0000
+++ src/gdb/cli/cli-decode.c	2009-02-16 22:53:42.000000000 +0000
@@ -112,18 +112,6 @@ get_cmd_context (struct cmd_list_element
   return cmd->context;
 }
 
-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	2009-02-06 23:43:12.000000000 +0000
+++ src/gdb/cli/cli-decode.h	2009-02-16 22:53:42.000000000 +0000
@@ -48,10 +48,6 @@ cmd_types;
 #define DEPRECATED_WARN_USER      0x2
 #define MALLOCED_REPLACEMENT      0x4
 
-/* 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.  */
@@ -259,13 +255,6 @@ extern int cmd_cfunc_eq (struct cmd_list
 extern void set_cmd_context (struct cmd_list_element *cmd, void *context);
 extern void *get_cmd_context (struct cmd_list_element *cmd);
 
-/* 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/remote.c
===================================================================
--- src.orig/gdb/remote.c	2009-02-16 22:51:20.000000000 +0000
+++ src/gdb/remote.c	2009-02-17 00:39:51.000000000 +0000
@@ -6583,16 +6583,7 @@ extended_remote_mourn_1 (struct target_o
   /* Call common code to mark the inferior as not running.	*/
   generic_mourn_inferior ();
 
-  if (have_inferiors ())
-    {
-      extern void nullify_last_target_wait_ptid ();
-      /* Multi-process case.  The current process has exited, but
-	 there are other processes to debug.  Switch to the first
-	 available.  */
-      iterate_over_threads (select_new_thread_callback, NULL);
-      nullify_last_target_wait_ptid ();
-    }
-  else
+  if (!have_inferiors ())
     {
       if (!remote_multi_process_p (rs))
 	{
Index: src/gdb/stack.c
===================================================================
--- src.orig/gdb/stack.c	2009-02-17 00:50:22.000000000 +0000
+++ src/gdb/stack.c	2009-02-17 00:50:46.000000000 +0000
@@ -1629,13 +1629,7 @@ select_and_print_frame (struct frame_inf
 struct block *
 get_selected_block (CORE_ADDR *addr_in_block)
 {
-  if (!target_has_stack)
-    return 0;
-
-  if (is_exited (inferior_ptid))
-    return 0;
-
-  if (is_executing (inferior_ptid))
+  if (!has_stack_frames ())
     return 0;
 
   return get_frame_block (get_selected_frame (NULL), addr_in_block);

RE: Process exit in multi-process, and gdb's selected thread.

[Marc Khouzam]

 

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org 
> [mailto:gdb-patches-owner@sourceware.org] On Behalf Of Pedro Alves
> Sent: Monday, February 16, 2009 7:59 PM
> To: gdb-patches@sourceware.org
> Cc: Marc Khouzam
> Subject: Process exit in multi-process, and gdb's selected thread.
> 
> Hi!
> 
> I've been thinking about issues related to what to do with GDB's
> notion of selected thread, if we're debugging more than one process
> simultaneously, and the current processes exits.
> 
> What would you think if GDB could get into this state,
> after a process exit? :
> 
>  (gdb) info threads
>    2 Thread 31176.31176  0x00007f0706154796 in ?? ()
> 
>  No selected thread.  See `help thread'.
>  (gdb) info inferiors
>    3 31176
>  (gdb) c
>  Continuing.
>  Cannot execute this command without a live selected thread.
>  (gdb) detach
>  The program is not being run.
>  (gdb) kill
>  The program is not being run.
>  (gdb)   
> 
> This would happen, e.g., if the current process (it was some other
> pid, not 31176) had exited or was detached from.  Keep in mind that in
> non-stop mode, we prefer that GDB doesn't switch threads behind the
> user's back while he is typing some command. This way, when the
> current thread and inferior no longer exist, GDB switches to 
> a state that
> should be safe, and the onus is on the frontend to switch to
> whatever thread it wants.
> 
> In the past, I had solved this by spreading around some hacks
> that tried to detect the current inferior exiting, and switching
> to any other random live thread, but, that turned out to be: first,
> surprising in non-stop mode, in the case mentioned above; and
> second, surprisingly difficult to get right.  I think this usually
> means that GDB shouldn't try to be smart (well, or I shouldn't).
> 
> I've tried avoiding before getting to a state where GDB has
> no thread selected, when there are threads in the thread list.  But,
> I don't think we'll be able to avoid it in the long run.  E.g., when
> GDB gains ability to load more than on program (exec) at the 
> same time,
> (say, "cat", and "grep"), even if "cat" is running already, 
> it will fell
> natural to be able to select the "grep" executable as focus, thus,
> the selected thread in that case should be "null".   (Also 
> thinking about
> the possibility of GDB being able in the future to switch to some form
> of post-or-quasi-mortem debugging mode when a process exits, 
> I'd want GDB
> to not switch threads to some other process on a process exit.)
> 
> A bit related to this, is the hack we have currently that tags a
> few commands as being "safe to apply without a valid
> selected thread".  This served its purpose when non-stop was first
> introduced, but, I think it's time we remove them, and start making
> the checks at a lower level.  They are currently done at a too high
> level currently, which ends up getting in the way many times.  We need
> more finer grained control over which operations can and can't be
> applied without a valid thread selected, so we need to push those
> checks down, more close to core code, or even to target code.

For what it's worth, I always thought it was risky to have such
a list of 'safe' commands.  It just seemed to be asking for trouble.
 
> I'm thinking that we may need to extend the =thread-selected
> notification to tell the frontend that there's no thread selected,
> and perhaps the -thread-info,current-thread-id, output too.

Your patches does not do this yet, right?
 
> What do you think of all this, am I making sense?  Or, does it
> sound like "let's hope he comes back to senses soon, for he's
> not making any"?  :-)
> 
> Here's my current patch that implements this, in case you
> have a stub around that implements multi-process (Hi Marc!).

I don't think you explicitely said it (or maybe I've read too
many emails today), but I believe you are suggesting that GDB
be allowed to be in a state where no thread is selected and
this state should be handled properly when receiving commands.

I didn't quite understand what responsibility falls on the 
frontend with this suggestion.  

I wanted to try to patch to see what you meant more clearly.
However, I think this patch applies to HEAD but HEAD does
not work with my stub yet (the -list-thread-groups --available
problem).

But I'm quite interested...

Marc

Re: Process exit in multi-process, and gdb's selected thread.

[Pedro Alves]

On Tuesday 17 February 2009 16:19:55, Marc Khouzam wrote:

> > I'm thinking that we may need to extend the =thread-selected
> > notification to tell the frontend that there's no thread selected,
> > and perhaps the -thread-info,current-thread-id, output too.
> 
> Your patches does not do this yet, right?

Right.

> > What do you think of all this, am I making sense?  Or, does it
> > sound like "let's hope he comes back to senses soon, for he's
> > not making any"?  :-)
> > 
> > Here's my current patch that implements this, in case you
> > have a stub around that implements multi-process (Hi Marc!).
> 
> I don't think you explicitely said it (or maybe I've read too
> many emails today), but I believe you are suggesting that GDB
> be allowed to be in a state where no thread is selected and
> this state should be handled properly when receiving commands.

Yes.

> 
> I didn't quite understand what responsibility falls on the 
> frontend with this suggestion.  

E.g., I'd like to understand what does eclipse do when it
receives a "=thread-group-exited" notification, and the thread
that eclipse had selected disappeared.  Was it expecting that
GDB changed to another random thread (and emit a =thread-selected
notification), or was it supposed to select another thread itself?
Or, does it also have a state of "no thread selected" in the UI?

> 
> I wanted to try to patch to see what you meant more clearly.
> However, I think this patch applies to HEAD but HEAD does
> not work with my stub yet (the -list-thread-groups --available
> problem).

Oh, bummer.  I thought you'd have some way to manually specify which
process to attach to without going through that listing.

-- 
Pedro Alves

RE: Process exit in multi-process, and gdb's selected thread.

[Marc Khouzam]

 

> -----Original Message-----
> From: Pedro Alves [mailto:pedro@codesourcery.com] 
> Sent: Tuesday, February 17, 2009 11:33 AM
> To: Marc Khouzam
> Cc: gdb-patches@sourceware.org
> Subject: Re: Process exit in multi-process, and gdb's selected thread.
> 
> On Tuesday 17 February 2009 16:19:55, Marc Khouzam wrote:
> > I didn't quite understand what responsibility falls on the 
> > frontend with this suggestion.  
> 
> E.g., I'd like to understand what does eclipse do when it
> receives a "=thread-group-exited" notification, and the thread
> that eclipse had selected disappeared.  Was it expecting that
> GDB changed to another random thread (and emit a =thread-selected
> notification), or was it supposed to select another thread itself?
> Or, does it also have a state of "no thread selected" in the UI?

We use the --thread flag for all our MI commands where a thread
makes sense.  Therefore we don't need GDB to have a currently 
selected thread.

This may be a little more tricky when dealing with the console
that the user writes too.  However, the user could simply
select a thread if none are currently selected.

> > I wanted to try to patch to see what you meant more clearly.
> > However, I think this patch applies to HEAD but HEAD does
> > not work with my stub yet (the -list-thread-groups --available
> > problem).
> 
> Oh, bummer.  I thought you'd have some way to manually specify which
> process to attach to without going through that listing.

Now that you mention it... :-)
So I was able to try it with HEAD.
At first glance, things look very good.  I was able to detach from
all processes and re-attach.  When detached from all processes, I ran
the 'info threads' commands and the result was empty, so I know there
was not thread selected.  But I did not get any errors and was able
to attach/detach, multiple times.

The only thing that gave me trouble was that auto-attach was triggered
from my Stub but I don't think HEAD deals with it perfectly, so I got
"No registers" when running -list-thread-groups after an auto-attach.

Bottom line is that this patch is very promising.
Good stuff!

Marc

Re: Process exit in multi-process, and gdb's selected thread.

[Pedro Alves]

On Tuesday 17 February 2009 17:08:04, Marc Khouzam wrote:

> At first glance, things look very good.  I was able to detach from
> all processes and re-attach.  When detached from all processes, I ran
> the 'info threads' commands and the result was empty, so I know there
> was not thread selected.  But I did not get any errors and was able
> to attach/detach, multiple times.
> 

That's very good to know, thanks!

> The only thing that gave me trouble was that auto-attach was triggered
> from my Stub but I don't think HEAD deals with it perfectly, so I got
> "No registers" when running -list-thread-groups after an auto-attach.

Yep, that's expected with HEAD.

> Bottom line is that this patch is very promising.
> Good stuff!
> 

Great!

-- 
Pedro Alves

Re: Process exit in multi-process, and gdb's selected thread.

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:

I meant to reply to this earlier...

Pedro> What would you think if GDB could get into this state,
Pedro> after a process exit? :
Pedro>  (gdb) info threads
Pedro>    2 Thread 31176.31176  0x00007f0706154796 in ?? ()
[...]

I think it is a reasonable outcome given the model.  If users find it
too confusing, we can try to add some extra output somewhere -- for
instance, when gdb says "The program is not being run.", it could
check for multiple inferiors and print something about how to switch
to another inferior.

I tend to doubt that we will need to do this, though, because I think
this is the most logical way for multi-inferior debugging to work.

Pedro> In the past, I had solved this by spreading around some hacks
Pedro> that tried to detect the current inferior exiting, and switching
Pedro> to any other random live thread, but, that turned out to be: first,
Pedro> surprising in non-stop mode, in the case mentioned above; and
Pedro> second, surprisingly difficult to get right.  I think this usually
Pedro> means that GDB shouldn't try to be smart (well, or I shouldn't).

I agree.

Pedro> What do you think of all this, am I making sense?

Yeah, I think your choices here make sense, particularly not having
gdb switch contexts behind the user's back, and that what you wrote up
is the logical outcome of this decision.

Tom

Re: Process exit in multi-process, and gdb's selected thread.

[Pedro Alves]

On Tuesday 24 February 2009 19:56:17, Tom Tromey wrote:
> >>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:
> 
> I meant to reply to this earlier...
> 

Makes two of us now.  :-)

> Pedro> What would you think if GDB could get into this state,
> Pedro> after a process exit? :
> Pedro>  (gdb) info threads
> Pedro>    2 Thread 31176.31176  0x00007f0706154796 in ?? ()
> [...]
> 
> I think it is a reasonable outcome given the model.  If users find it
> too confusing, we can try to add some extra output somewhere -- for
> instance, when gdb says "The program is not being run.", it could
> check for multiple inferiors and print something about how to switch
> to another inferior.
> 
> I tend to doubt that we will need to do this, though, because I think
> this is the most logical way for multi-inferior debugging to work.
> 
> Pedro> In the past, I had solved this by spreading around some hacks
> Pedro> that tried to detect the current inferior exiting, and switching
> Pedro> to any other random live thread, but, that turned out to be: first,
> Pedro> surprising in non-stop mode, in the case mentioned above; and
> Pedro> second, surprisingly difficult to get right.  I think this usually
> Pedro> means that GDB shouldn't try to be smart (well, or I shouldn't).
> 
> I agree.
> 
> Pedro> What do you think of all this, am I making sense?
> 
> Yeah, I think your choices here make sense, particularly not having
> gdb switch contexts behind the user's back, and that what you wrote up
> is the logical outcome of this decision.

Great then.   Since there were no objections to this, and Marc
has been using this patch against his multi-process aware system
for a while now without problems, I checked it in.

-- 
Pedro Alves