This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFC] 01/10 Add "executing" property


A Tuesday 06 May 2008 16:46:19, Pedro Alves wrote:
> The "running" property being added is a frontend view of the thread.
> While doing a step for instance, the inferior starts/stops several
> times while single-stepping.  We only clear the "running" state of
> the inferior when it does a normal_stop.  This is to prevent bombarding
> the frontend with a bunch of internal events.
>
> This patch adds a new field, that altough similar to "running"
> has a different usage.  It replaces the current target_executing
> global with a similar per-thread concept.  We can then use the
> is_executing check whenever we need to be sure the selected thread
> is stopped or executing.

Here's an updated patch.

- is_executing mishandled targets that don't register the main thread.
- There was a call to is_running in macroscope.c that should have
  been to is_executing.
- In all-stop/async with scheduler-locking != off, checking for is_executing 
  in the current  thread is not enough.  We should check if there's any thread 
  running.  thread.c gains a new any_running function for that.

-- 
Pedro Alves
2008-05-19  Pedro Alves  <pedro@codesourcery.com>

	* inferior.h (target_executing): Delete.
	* gdbthread.h (struct thread_info): Add executing_ field.
	(set_executing, is_executing): New.
	* thread.c (main_thread_executing): New.
	(init_thread_list): Clear it and also main_thread_running.
	(any_running, is_executing, set_executing): New.

	* top.c: Include "gdbthread.h".
	(target_executing): Delete.
	(execute_command): Replace target_executing check by any_running.
	* event-top.c: Include "gdbthread.h".
	(display_gdb_prompt): Replace target_executing by is_executing.
	* inf-loop.c: Include "gdbthread.h".  Don't mark as not executing
	here.  Replace target_executing by is_executing.
	* infrun.c (handle_inferior_event): Mark all threads as
	not-executing.
	* linux-nat.c (linux_nat_resume): Don't mark thread as executing
	here.
	* stack.c (get_selected_block): Return null if inferior is
	executing.
	* target.c (target_resume): Mark resumed ptid as executing.
	* breakpoint.c (until_break_command): Replace target_executing
	check by is_executing.
	* remote.c (remote_async_resume): Don't mark inferior as executing
	here.
	* macroscope.c: Include "gdbthread.h".
	(default_macro_scope): Don't read registers if thread is running.
	* mi/mi-interp.c (mi_cmd_interpreter_exec): Replace target_executing
	by is_executing.
	* mi/mi-main.c (mi_cmd_exec_interrupt): Replace target_executing
	by is_executing.

	* Makefile.in (event-top.o, inf-loop.o, macroscope.o, top.o):
	Update.

---
 gdb/Makefile.in    |    8 +++---
 gdb/breakpoint.c   |    2 -
 gdb/event-top.c    |    5 ++--
 gdb/gdbthread.h    |   35 ++++++++++++++++++++++++----
 gdb/inf-loop.c     |   11 +-------
 gdb/inferior.h     |    6 ----
 gdb/infrun.c       |    6 ++++
 gdb/linux-nat.c    |    5 ----
 gdb/macroscope.c   |   12 +++++++--
 gdb/mi/mi-interp.c |    2 -
 gdb/mi/mi-main.c   |    8 +++---
 gdb/remote.c       |    7 -----
 gdb/stack.c        |    4 +++
 gdb/target.c       |    2 -
 gdb/thread.c       |   66 +++++++++++++++++++++++++++++++++++++++++++++++++++--
 gdb/top.c          |   13 ++++------
 16 files changed, 135 insertions(+), 57 deletions(-)

Index: src/gdb/inferior.h
===================================================================
--- src.orig/gdb/inferior.h	2008-05-19 11:53:54.000000000 +0100
+++ src/gdb/inferior.h	2008-05-19 12:18:28.000000000 +0100
@@ -111,12 +111,6 @@ extern const char *get_inferior_io_termi
 
 extern ptid_t inferior_ptid;
 
-/* Is the inferior running right now, as a result of a 'run&',
-   'continue&' etc command? This is used in asycn gdb to determine
-   whether a command that the user enters while the target is running
-   is allowed or not. */
-extern int target_executing;
-
 /* Are we simulating synchronous execution? This is used in async gdb
    to implement the 'run', 'continue' etc commands, which will not
    redisplay the prompt until the execution is actually over. */
Index: src/gdb/gdbthread.h
===================================================================
--- src.orig/gdb/gdbthread.h	2008-05-19 11:53:54.000000000 +0100
+++ src/gdb/gdbthread.h	2008-05-19 12:31:00.000000000 +0100
@@ -41,6 +41,25 @@ struct thread_info
 				    In fact, this may be overloaded with 
 				    kernel thread id, etc.  */
   int num;			/* Convenient handle (GDB thread id) */
+
+  /* Non-zero means the thread is executing.  Note: this is different
+     from saying that there is an active target and we are stopped at
+     a breakpoint, for instance.  This is a real indicator whether the
+     thread is off and running.  */
+  /* This field is internal to thread.c.  Never access it directly,
+     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), ... }  */
+  /* This field is internal to thread.c.  Never access it directly,
+     use is_running instead.  */
+  int running_;
+
   /* State from wait_for_inferior */
   CORE_ADDR prev_pc;
   struct breakpoint *step_resume_breakpoint;
@@ -63,10 +82,6 @@ struct thread_info
      when we finally do stop stepping.  */
   bpstat stepping_through_solib_catchpoints;
 
-  /* This field is internal for thread.c.  Never access it directly,
-     use is_running instead.  */
-  int running_;
-
   /* Private data used by the target vector implementation.  */
   struct private_thread_info *private;
 };
@@ -154,9 +169,19 @@ extern void switch_to_thread (ptid_t pti
    If PIDGET (PTID) is -1, marks all threads.  */
 extern void set_running (ptid_t ptid, int running);
 
-/* Reports if thread PTID is know to be running right now.  */
+/* Reports if thread PTID is known to be running right now.  */
 extern int is_running (ptid_t ptid);
 
+/* Reports if any thread is known to be running right now.  */
+extern int any_running (void);
+
+/* 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);
+
+/* Reports if thread PTID is executing.  */
+extern int is_executing (ptid_t ptid);
+
 /* Commands with a prefix of `thread'.  */
 extern struct cmd_list_element *thread_cmd_list;
 
Index: src/gdb/thread.c
===================================================================
--- src.orig/gdb/thread.c	2008-05-19 11:53:54.000000000 +0100
+++ src/gdb/thread.c	2008-05-19 12:34:45.000000000 +0100
@@ -62,6 +62,9 @@ static void thread_apply_command (char *
 static void restore_current_thread (ptid_t);
 static void prune_threads (void);
 
+static int main_thread_running = 0;
+static int main_thread_executing = 0;
+
 void
 delete_step_resume_breakpoint (void *arg)
 {
@@ -113,6 +116,9 @@ init_thread_list (void)
     }
 
   thread_list = NULL;
+
+  main_thread_running = 0;
+  main_thread_executing = 0;
 }
 
 struct thread_info *
@@ -412,8 +418,6 @@ prune_threads (void)
     }
 }
 
-static int main_thread_running = 0;
-
 void
 set_running (ptid_t ptid, int running)
 {
@@ -473,6 +477,64 @@ is_running (ptid_t ptid)
   return tp->running_;  
 }
 
+int
+any_running (void)
+{
+  struct thread_info *tp;
+
+  if (!thread_list)
+    return main_thread_running;
+
+  for (tp = thread_list; tp; tp = tp->next)
+    if (tp->running_)
+      return 1;
+
+  return 0;
+}
+
+int
+is_executing (ptid_t ptid)
+{
+  struct thread_info *tp;
+
+  if (!target_has_execution)
+    return 0;
+
+  if (!thread_list)
+    return main_thread_executing;
+
+  tp = find_thread_pid (ptid);
+  gdb_assert (tp);
+  return tp->executing_;
+}
+
+void
+set_executing (ptid_t ptid, int executing)
+{
+  struct thread_info *tp;
+
+  if (!thread_list)
+    {
+      /* This target does not add the main thread to the thread list.
+	 Use a global flag to indicate that the thread is
+	 executing.  */
+      main_thread_executing = executing;
+      return;
+    }
+
+  if (PIDGET (ptid) == -1)
+    {
+      for (tp = thread_list; tp; tp = tp->next)
+	tp->executing_ = executing;
+    }
+  else
+    {
+      tp = find_thread_pid (ptid);
+      gdb_assert (tp);
+      tp->executing_ = executing;
+    }
+}
+
 /* Prints the list of threads and their details on UIOUT.
    This is a version of 'info_thread_command' suitable for
    use from MI.  
Index: src/gdb/top.c
===================================================================
--- src.orig/gdb/top.c	2008-05-19 11:48:16.000000000 +0100
+++ src/gdb/top.c	2008-05-19 12:30:09.000000000 +0100
@@ -46,6 +46,7 @@
 #include "gdb_assert.h"
 #include "main.h"
 #include "event-loop.h"
+#include "gdbthread.h"
 
 /* readline include files */
 #include "readline/readline.h"
@@ -181,12 +182,6 @@ int remote_timeout = 2;
 
 int remote_debug = 0;
 
-/* Non-zero means the target is running. Note: this is different from
-   saying that there is an active target and we are stopped at a
-   breakpoint, for instance. This is a real indicator whether the
-   target is off and running, which gdb is doing something else. */
-int target_executing = 0;
-
 /* Sbrk location on entry to main.  Used for statistics only.  */
 #ifdef HAVE_SBRK
 char *lim_at_start;
@@ -422,7 +417,9 @@ execute_command (char *p, int from_tty)
 
       /* If the target is running, we allow only a limited set of
          commands. */
-      if (target_can_async_p () && target_executing && !get_cmd_async_ok (c))
+      if (target_can_async_p ()
+	  && any_running ()
+	  && !get_cmd_async_ok (c))
 	error (_("Cannot execute this command while the target is running."));
 
       /* Pass null arg rather than an empty one.  */
@@ -486,7 +483,7 @@ execute_command (char *p, int from_tty)
   /* FIXME:  This should be cacheing the frame and only running when
      the frame changes.  */
 
-  if (!target_executing && target_has_stack)
+  if (target_has_stack && !is_executing (inferior_ptid))
     {
       flang = get_frame_language ();
       if (!warned
Index: src/gdb/event-top.c
===================================================================
--- src.orig/gdb/event-top.c	2008-05-19 11:48:16.000000000 +0100
+++ src/gdb/event-top.c	2008-05-19 11:54:02.000000000 +0100
@@ -32,6 +32,7 @@
 #include "exceptions.h"
 #include "cli/cli-script.h"     /* for reset_command_nest_depth */
 #include "main.h"
+#include "gdbthread.h"
 
 /* For dont_repeat() */
 #include "gdbcmd.h"
@@ -268,7 +269,7 @@ display_gdb_prompt (char *new_prompt)
   if (!current_interp_display_prompt_p ())
     return;
 
-  if (target_executing && sync_execution)
+  if (sync_execution && is_executing (inferior_ptid))
     {
       /* This is to trick readline into not trying to display the
          prompt.  Even though we display the prompt using this
@@ -516,7 +517,7 @@ command_handler (char *command)
   /* Do any commands attached to breakpoint we stopped at. Only if we
      are always running synchronously. Or if we have just executed a
      command that doesn't start the target. */
-  if (!target_can_async_p () || !target_executing)
+  if (!target_can_async_p () || !is_executing (inferior_ptid))
     {
       bpstat_do_actions (&stop_bpstat);
       do_cleanups (old_chain);
Index: src/gdb/inf-loop.c
===================================================================
--- src.orig/gdb/inf-loop.c	2008-05-19 11:48:16.000000000 +0100
+++ src/gdb/inf-loop.c	2008-05-19 12:06:51.000000000 +0100
@@ -26,6 +26,7 @@
 #include "remote.h"
 #include "exceptions.h"
 #include "language.h"
+#include "gdbthread.h"
 
 static int fetch_inferior_event_wrapper (gdb_client_data client_data);
 
@@ -72,14 +73,6 @@ inferior_event_handler (enum inferior_ev
       break;
 
     case INF_EXEC_COMPLETE:
-
-      /* This is the first thing to do -- so that continuations know that
-	 the target is stopped.  For example, command_line_handler_continuation
-	 will run breakpoint commands, and if we think that the target is
-	 running, we'll refuse to execute most commands.  MI continuation
-	 presently uses target_executing to either print or not print *stopped.  */
-      target_executing = 0;
-
       /* Unregister the inferior from the event loop. This is done so that
 	 when the inferior is not running we don't get distracted by
 	 spurious inferior output.  */
@@ -121,7 +114,7 @@ inferior_event_handler (enum inferior_ev
 
       /* If no breakpoint command resumed the inferior, prepare for
 	 interaction with the user.  */
-      if (!target_executing)
+      if (!is_executing (inferior_ptid))
 	{              
 	  if (was_sync)
 	    {
Index: src/gdb/infrun.c
===================================================================
--- src.orig/gdb/infrun.c	2008-05-19 11:53:54.000000000 +0100
+++ src/gdb/infrun.c	2008-05-19 12:19:49.000000000 +0100
@@ -1766,6 +1766,12 @@ handle_inferior_event (struct execution_
       && ecs->ws.kind != TARGET_WAITKIND_SIGNALLED && ecs->new_thread_event)
     add_thread (ecs->ptid);
 
+  /* Mark all threads as not-executing.  In non-stop, this should be
+     adjusted to only mark ecs->ptid.  */
+  if (ecs->ws.kind != TARGET_WAITKIND_IGNORE
+      && stop_soon != STOP_QUIETLY)
+    set_executing (pid_to_ptid (-1), 0);
+
   switch (ecs->ws.kind)
     {
     case TARGET_WAITKIND_LOADED:
Index: src/gdb/linux-nat.c
===================================================================
--- src.orig/gdb/linux-nat.c	2008-05-19 11:50:37.000000000 +0100
+++ src/gdb/linux-nat.c	2008-05-19 12:19:49.000000000 +0100
@@ -1554,10 +1554,7 @@ linux_nat_resume (ptid_t ptid, int step,
 			signo ? strsignal (signo) : "0");
 
   if (target_can_async_p ())
-    {
-      target_executing = 1;
-      target_async (inferior_event_handler, 0);
-    }
+    target_async (inferior_event_handler, 0);
 }
 
 /* Issue kill to specified lwp.  */
Index: src/gdb/stack.c
===================================================================
--- src.orig/gdb/stack.c	2008-05-19 11:50:38.000000000 +0100
+++ src/gdb/stack.c	2008-05-19 11:54:02.000000000 +0100
@@ -43,6 +43,7 @@
 #include "regcache.h"
 #include "solib.h"
 #include "valprint.h"
+#include "gdbthread.h"
 
 #include "gdb_assert.h"
 #include <ctype.h>
@@ -1653,6 +1654,9 @@ get_selected_block (CORE_ADDR *addr_in_b
   if (!target_has_stack)
     return 0;
 
+  if (is_executing (inferior_ptid))
+    return 0;
+
   return get_frame_block (get_selected_frame (NULL), addr_in_block);
 }
 
Index: src/gdb/target.c
===================================================================
--- src.orig/gdb/target.c	2008-05-19 11:53:54.000000000 +0100
+++ src/gdb/target.c	2008-05-19 12:06:51.000000000 +0100
@@ -1721,8 +1721,8 @@ target_resume (ptid_t ptid, int step, en
 {
   dcache_invalidate (target_dcache);
   (*current_target.to_resume) (ptid, step, signal);
+  set_executing (ptid, 1);
   set_running (ptid, 1);
-
 }
 /* Look through the list of possible targets for a target that can
    follow forks.  */
Index: src/gdb/breakpoint.c
===================================================================
--- src.orig/gdb/breakpoint.c	2008-05-19 11:53:47.000000000 +0100
+++ src/gdb/breakpoint.c	2008-05-19 12:19:49.000000000 +0100
@@ -6247,7 +6247,7 @@ until_break_command (char *arg, int from
      deleted when the target stops.  Otherwise, we're already stopped and
      delete breakpoints via cleanup chain.  */
 
-  if (target_can_async_p () && target_executing)
+  if (target_can_async_p () && is_executing (inferior_ptid))
     {
       arg1 =
 	(struct continuation_arg *) xmalloc (sizeof (struct continuation_arg));
Index: src/gdb/remote.c
===================================================================
--- src.orig/gdb/remote.c	2008-05-19 11:50:38.000000000 +0100
+++ src/gdb/remote.c	2008-05-19 11:54:02.000000000 +0100
@@ -3147,13 +3147,6 @@ remote_async_resume (ptid_t ptid, int st
      NOT asynchronously.  */
   if (target_can_async_p ())
     target_async (inferior_event_handler, 0);
-  /* Tell the world that the target is now executing.  */
-  /* FIXME: cagney/1999-09-23: Is it the targets responsibility to set
-     this?  Instead, should the client of target just assume (for
-     async targets) that the target is going to start executing?  Is
-     this information already found in the continuation block?  */
-  if (target_is_async_p ())
-    target_executing = 1;
 }
 
 
Index: src/gdb/macroscope.c
===================================================================
--- src.orig/gdb/macroscope.c	2008-05-19 11:48:16.000000000 +0100
+++ src/gdb/macroscope.c	2008-05-19 12:35:41.000000000 +0100
@@ -26,7 +26,7 @@
 #include "frame.h"
 #include "inferior.h"
 #include "complaints.h"
-
+#include "gdbthread.h"
 
 struct macro_scope *
 sal_macro_scope (struct symtab_and_line sal)
@@ -84,8 +84,14 @@ default_macro_scope (void)
   struct macro_scope *ms;
   struct frame_info *frame;
 
-  /* If there's a selected frame, use its PC.  */
-  frame = deprecated_safe_get_selected_frame ();
+  /* If there's a selected frame, use its PC, except if the current
+     thread is executing, which which case, we can't read its
+     registers.  */
+  if (!is_executing (inferior_ptid))
+    frame = deprecated_safe_get_selected_frame ();
+  else
+    frame = NULL;
+
   if (frame)
     sal = find_pc_line (get_frame_pc (frame), 0);
   
Index: src/gdb/mi/mi-interp.c
===================================================================
--- src.orig/gdb/mi/mi-interp.c	2008-05-19 11:53:54.000000000 +0100
+++ src/gdb/mi/mi-interp.c	2008-05-19 11:54:02.000000000 +0100
@@ -222,7 +222,7 @@ mi_cmd_interpreter_exec (char *command, 
      changing the interpreter will clear out all the continuations for
      that interpreter... */
 
-  if (target_can_async_p () && target_executing)
+  if (target_can_async_p () && is_executing (inferior_ptid))
     {
       fputs_unfiltered ("^running\n", raw_stdout);
     }
Index: src/gdb/mi/mi-main.c
===================================================================
--- src.orig/gdb/mi/mi-main.c	2008-05-19 11:53:47.000000000 +0100
+++ src/gdb/mi/mi-main.c	2008-05-19 11:54:02.000000000 +0100
@@ -215,7 +215,7 @@ mi_cmd_exec_continue (char *command, cha
 enum mi_cmd_result
 mi_cmd_exec_interrupt (char *command, char **argv, int argc)
 {
-  if (!target_executing)
+  if (!is_executing (inferior_ptid))
     error ("mi_cmd_exec_interrupt: Inferior not executing.");
 
   interrupt_target_command (NULL, 0);
@@ -1063,7 +1063,7 @@ captured_mi_execute_command (struct ui_o
       if (do_timings)
 	timestamp (&cmd_finished);
 
-      if (!target_can_async_p () || !target_executing)
+      if (!target_can_async_p () || !is_executing (inferior_ptid))
 	{
 	  /* Print the result if there were no errors.
 
@@ -1196,7 +1196,7 @@ mi_cmd_execute (struct mi_parse *parse)
 
   if (parse->cmd->argv_func != NULL)
     {
-      if (target_executing)
+      if (is_executing (inferior_ptid))
 	{
 	  if (strcmp (parse->command, "exec-interrupt"))
 	    {
@@ -1311,7 +1311,7 @@ mi_execute_async_cli_command (char *cli_
   if (target_can_async_p ())
     {
       /* If we're not executing, an exception should have been throw.  */
-      gdb_assert (target_executing);
+      gdb_assert (is_executing (inferior_ptid));
       do_cleanups (old_cleanups);
     }
   else
Index: src/gdb/Makefile.in
===================================================================
--- src.orig/gdb/Makefile.in	2008-05-19 11:53:54.000000000 +0100
+++ src/gdb/Makefile.in	2008-05-19 12:18:17.000000000 +0100
@@ -2118,7 +2118,7 @@ event-loop.o: event-loop.c $(defs_h) $(e
 event-top.o: event-top.c $(defs_h) $(top_h) $(inferior_h) $(target_h) \
 	$(terminal_h) $(event_loop_h) $(event_top_h) $(interps_h) \
 	$(exceptions_h) $(cli_script_h) $(gdbcmd_h) $(readline_h) \
-	$(readline_history_h) $(main_h)
+	$(readline_history_h) $(main_h) $(gdbthread_h)
 exceptions.o: exceptions.c $(defs_h) $(exceptions_h) $(breakpoint_h) \
 	$(target_h) $(inferior_h) $(annotate_h) $(ui_out_h) $(gdb_assert_h) \
 	$(gdb_string_h) $(serial_h)
@@ -2334,7 +2334,7 @@ infcmd.o: infcmd.c $(defs_h) $(gdb_strin
 	$(user_regs_h) $(exceptions_h) $(cli_decode_h) $(gdbthread_h)
 inf-loop.o: inf-loop.c $(defs_h) $(inferior_h) $(target_h) $(event_loop_h) \
 	$(event_top_h) $(inf_loop_h) $(remote_h) $(exceptions_h) \
-	$(language_h)
+	$(language_h) $(gdbthread_h)
 inflow.o: inflow.c $(defs_h) $(frame_h) $(inferior_h) $(command_h) \
 	$(serial_h) $(terminal_h) $(target_h) $(gdbthread_h) $(gdb_string_h) \
 	$(inflow_h) $(gdb_select_h)
@@ -2471,7 +2471,7 @@ macrocmd.o: macrocmd.c $(defs_h) $(macro
 macroexp.o: macroexp.c $(defs_h) $(gdb_obstack_h) $(bcache_h) $(macrotab_h) \
 	$(macroexp_h) $(gdb_assert_h)
 macroscope.o: macroscope.c $(defs_h) $(macroscope_h) $(symtab_h) $(source_h) \
-	$(target_h) $(frame_h) $(inferior_h) $(complaints_h)
+	$(target_h) $(frame_h) $(inferior_h) $(complaints_h) $(gdbthread_h)
 macrotab.o: macrotab.c $(defs_h) $(gdb_obstack_h) $(splay_tree_h) \
 	$(symtab_h) $(symfile_h) $(objfiles_h) $(macrotab_h) $(gdb_assert_h) \
 	$(bcache_h) $(complaints_h)
@@ -2940,7 +2940,7 @@ top.o: top.c $(defs_h) $(gdbcmd_h) $(cal
 	$(annotate_h) $(completer_h) $(top_h) $(version_h) $(serial_h) \
 	$(doublest_h) $(gdb_assert_h) $(readline_h) $(readline_history_h) \
 	$(event_top_h) $(gdb_string_h) $(gdb_stat_h) $(ui_out_h) \
-	$(cli_out_h) $(main_h) $(event_loop_h)
+	$(cli_out_h) $(main_h) $(event_loop_h) $(gdbthread_h)
 tracepoint.o: tracepoint.c $(defs_h) $(symtab_h) $(frame_h) $(gdbtypes_h) \
 	$(expression_h) $(gdbcmd_h) $(value_h) $(target_h) $(language_h) \
 	$(gdb_string_h) $(inferior_h) $(tracepoint_h) $(remote_h) \

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]