[MI non-stop 05/11, RFA] -exec-continue/-exec-interrupt --all

[MI non-stop 05/11, RFA] -exec-continue/-exec-interrupt --all

[Vladimir Prus]

Per MI non-stop spec, continue and interrupt need --all option
to resume every thread. Implemented thusly. OK?

- Volodya

	* infcmd.c (continue_1): New, extracted from
	(continue_command): ...here.
	(interrupt_target_1): New, extracted from
	(interrupt_target_command): ...here.
	* inferior.h (continue_1, interrupt_target_1): New.
	* mi/mi-main.c (mi_cmd_exec_continue)
	(mi_cmd_exec_interrupt): Handle --all.
---
 gdb/infcmd.c     |   58 ++++++++++++++++++++++++++++++++---------------------
 gdb/inferior.h   |    4 +++
 gdb/mi/mi-main.c |   26 +++++++++++++++++++----
 3 files changed, 60 insertions(+), 28 deletions(-)

diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index ae3668f..6ec458c 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -625,6 +625,21 @@ proceed_thread_callback (struct thread_info *thread, void *arg)
   return 0;
 }
 
+void
+continue_1 (int all_threads)
+{
+  if (non_stop && all_threads)
+      /* Don't error out current thread is running, because there may
+	 be other stopped threads.  */
+      iterate_over_threads (proceed_thread_callback, NULL);
+  else
+    {
+      ensure_not_running ();
+      clear_proceed_status ();
+      proceed ((CORE_ADDR) -1, TARGET_SIGNAL_DEFAULT, 0);
+    }
+}
+
 /* continue [-a] [proceed-count] [&]  */
 void
 continue_command (char *args, int from_tty)
@@ -699,16 +714,7 @@ Can't resume all threads and specify proceed count simultaneously."));
   if (from_tty)
     printf_filtered (_("Continuing.\n"));
 
-  if (non_stop && all_threads)
-    /* 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 ();
-      clear_proceed_status ();
-      proceed ((CORE_ADDR) -1, TARGET_SIGNAL_DEFAULT, 0);
-    }
+  continue_1 (all_threads);
 }
 \f
 /* Step until outside of current statement.  */
@@ -2148,6 +2154,24 @@ disconnect_command (char *args, int from_tty)
     deprecated_detach_hook ();
 }
 
+void 
+interrupt_target_1 (int all_threads)
+{
+      if (non_stop)
+	{
+	  ptid_t ptid;
+
+	  if (all_threads)
+	    ptid = minus_one_ptid;
+	  else
+	    ptid = inferior_ptid;
+
+	  target_stop_ptid (ptid);
+	}
+      else
+	target_stop ();
+}
+
 /* Stop the execution of the target while running in async mode, in
    the backgound.  In all-stop, stop the whole process.  In non-stop
    mode, stop the current thread only by default, or stop all threads
@@ -2170,19 +2194,7 @@ interrupt_target_command (char *args, int from_tty)
       if (!non_stop && all_threads)
 	error (_("-a is meaningless in all-stop mode."));
 
-      if (non_stop)
-	{
-	  ptid_t ptid;
-
-	  if (all_threads)
-	    ptid = minus_one_ptid;
-	  else
-	    ptid = inferior_ptid;
-
-	  target_stop_ptid (ptid);
-	}
-      else
-	target_stop ();
+      interrupt_target_1 (all_threads);
     }
 }
 
diff --git a/gdb/inferior.h b/gdb/inferior.h
index f4c6b83..d0067ce 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -271,10 +271,14 @@ extern void nexti_command (char *, int);
 
 extern void stepi_command (char *, int);
 
+extern void continue_1 (int all_threads);
+
 extern void continue_command (char *, int);
 
 extern void interrupt_target_command (char *args, int from_tty);
 
+extern void interrupt_target_1 (int all_threads);
+
 /* Last signal that the inferior received (why it stopped).  */
 
 extern enum target_signal stop_signal;
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 61ef5a4..87c3257 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -177,8 +177,12 @@ mi_cmd_exec_return (char *command, char **argv, int argc)
 void
 mi_cmd_exec_continue (char *command, char **argv, int argc)
 {
-  /* FIXME: Should call a libgdb function, not a cli wrapper.  */
-  return mi_execute_async_cli_command ("continue", argv, argc);
+  if (argc == 0)
+    continue_1 (0);
+  else if (argc == 1 && strcmp (argv[0], "--all") == 0)
+    continue_1 (1);
+  else
+    error ("Usage: -exec-continue [--all]");
 }
 
 /* Interrupt the execution of the target.  Note how we must play around
@@ -189,10 +193,22 @@ mi_cmd_exec_continue (char *command, char **argv, int argc)
 void
 mi_cmd_exec_interrupt (char *command, char **argv, int argc)
 {
-  if (!is_running (inferior_ptid))
-    error ("mi_cmd_exec_interrupt: Inferior not running.");
+  if (argc == 0)
+    {
+      if (!is_running (inferior_ptid))
+	error ("Current thread is not running.");
 
-  interrupt_target_command (NULL, 0);
+      interrupt_target_1 (0);
+    }
+  else if (argc == 1 && strcmp (argv[0], "--all") == 0)
+    {
+      if (!any_running ())
+	error ("Inferior not running.");
+
+      interrupt_target_1 (1);
+    }
+  else
+    error ("Usage: -exec-interrupt [--all]");
 }
 
 void
-- 
1.5.3.5

RE: [MI non-stop 05/11, RFA] -exec-continue/-exec-interrupt --all

[Marc Khouzam]

> +void
> +interrupt_target_1 (int all_threads)
> +{
> +      if (non_stop)
> +       {
> +         ptid_t ptid;
> +
> +         if (all_threads)
> +           ptid = minus_one_ptid;
> +         else
> +           ptid = inferior_ptid;
> +
> +         target_stop_ptid (ptid);
> +       }
> +      else
> +       target_stop ();
> +}
> +
>  /* Stop the execution of the target while running in async mode, in
>     the backgound.  In all-stop, stop the whole process.  In non-stop
>     mode, stop the current thread only by default, or stop all threads
> @@ -2170,19 +2194,7 @@ interrupt_target_command (char *args, int from_tty)
>        if (!non_stop && all_threads)
>         error (_("-a is meaningless in all-stop mode."));

Can this error be printed for an MI command?
If so, -a is not the flag in MI...

Re: [MI non-stop 05/11, RFA] -exec-continue/-exec-interrupt --all

[Tom Tromey]

>>>>> "Vladimir" == Vladimir Prus <vladimir@codesourcery.com> writes:

Vladimir> Per MI non-stop spec, continue and interrupt need --all option
Vladimir> to resume every thread. Implemented thusly. OK?

Vladimir> +void 
Vladimir> +interrupt_target_1 (int all_threads)
Vladimir> +{
Vladimir> +      if (non_stop)
Vladimir> +	{

It looks to me like there are 2 extra indentation spaces in this
function's body.

Tom

Re: [MI non-stop 05/11, RFA] -exec-continue/-exec-interrupt --all

[Vladimir Prus]

On Saturday 28 June 2008 22:10:19 Marc Khouzam wrote:
> > +void
> > +interrupt_target_1 (int all_threads)
> > +{
> > +      if (non_stop)
> > +       {
> > +         ptid_t ptid;
> > +
> > +         if (all_threads)
> > +           ptid = minus_one_ptid;
> > +         else
> > +           ptid = inferior_ptid;
> > +
> > +         target_stop_ptid (ptid);
> > +       }
> > +      else
> > +       target_stop ();
> > +}
> > +
> >  /* Stop the execution of the target while running in async mode, in
> >     the backgound.  In all-stop, stop the whole process.  In non-stop
> >     mode, stop the current thread only by default, or stop all threads
> > @@ -2170,19 +2194,7 @@ interrupt_target_command (char *args, int from_tty)
> >        if (!non_stop && all_threads)
> >         error (_("-a is meaningless in all-stop mode."));
> 
> Can this error be printed for an MI command?

It cannot. MI invokes interrupt_target_1, never interrupt_target_command.

- Volodya

Re: [MI non-stop 05/11, RFA] -exec-continue/-exec-interrupt --all

[Vladimir Prus]

On Saturday 28 June 2008 22:25:54 Tom Tromey wrote:
> >>>>> "Vladimir" == Vladimir Prus <vladimir@codesourcery.com> writes:
> 
> Vladimir> Per MI non-stop spec, continue and interrupt need --all option
> Vladimir> to resume every thread. Implemented thusly. OK?
> 
> Vladimir> +void 
> Vladimir> +interrupt_target_1 (int all_threads)
> Vladimir> +{
> Vladimir> +      if (non_stop)
> Vladimir> +	{
> 
> It looks to me like there are 2 extra indentation spaces in this
> function's body.

You're right, I've fixed this.

- Volodya

Re: [MI non-stop 05/11, RFA] -exec-continue/-exec-interrupt --all

[Daniel Jacobowitz]

On Sat, Jun 28, 2008 at 08:45:39PM +0400, Vladimir Prus wrote:
> +  if (non_stop && all_threads)
> +      /* Don't error out current thread is running, because there may
> +	 be other stopped threads.  */
> +      iterate_over_threads (proceed_thread_callback, NULL);

"if the current thread"; also extra indentation here.

> +void 
> +interrupt_target_1 (int all_threads)
> +{

Tom pointed out the indentation problem here.

> @@ -177,8 +177,12 @@ mi_cmd_exec_return (char *command, char **argv, int argc)
>  void
>  mi_cmd_exec_continue (char *command, char **argv, int argc)
>  {
> -  /* FIXME: Should call a libgdb function, not a cli wrapper.  */
> -  return mi_execute_async_cli_command ("continue", argv, argc);
> +  if (argc == 0)
> +    continue_1 (0);
> +  else if (argc == 1 && strcmp (argv[0], "--all") == 0)
> +    continue_1 (1);
> +  else
> +    error ("Usage: -exec-continue [--all]");
>  }
>  
>  /* Interrupt the execution of the target.  Note how we must play around

I suggest you use mi_getopt instead, for consistency.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [MI non-stop 05/11, RFA] -exec-continue/-exec-interrupt --all

[Daniel Jacobowitz]

On Fri, Jul 11, 2008 at 09:34:34AM -0400, Daniel Jacobowitz wrote:
> > @@ -177,8 +177,12 @@ mi_cmd_exec_return (char *command, char **argv, int argc)
> >  void
> >  mi_cmd_exec_continue (char *command, char **argv, int argc)
> >  {
> > -  /* FIXME: Should call a libgdb function, not a cli wrapper.  */
> > -  return mi_execute_async_cli_command ("continue", argv, argc);
> > +  if (argc == 0)
> > +    continue_1 (0);
> > +  else if (argc == 1 && strcmp (argv[0], "--all") == 0)
> > +    continue_1 (1);
> > +  else
> > +    error ("Usage: -exec-continue [--all]");
> >  }
> >  
> >  /* Interrupt the execution of the target.  Note how we must play around
> 
> I suggest you use mi_getopt instead, for consistency.

Did you notice this comment?

Also, please remember to post the version of patches that you check
in.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [MI non-stop 05/11, RFA] -exec-continue/-exec-interrupt --all

[Vladimir Prus]

On Sunday 13 July 2008 08:24:47 Daniel Jacobowitz wrote:
> On Fri, Jul 11, 2008 at 09:34:34AM -0400, Daniel Jacobowitz wrote:
> > > @@ -177,8 +177,12 @@ mi_cmd_exec_return (char *command, char **argv, int argc)
> > >  void
> > >  mi_cmd_exec_continue (char *command, char **argv, int argc)
> > >  {
> > > -  /* FIXME: Should call a libgdb function, not a cli wrapper.  */
> > > -  return mi_execute_async_cli_command ("continue", argv, argc);
> > > +  if (argc == 0)
> > > +    continue_1 (0);
> > > +  else if (argc == 1 && strcmp (argv[0], "--all") == 0)
> > > +    continue_1 (1);
> > > +  else
> > > +    error ("Usage: -exec-continue [--all]");
> > >  }
> > >  
> > >  /* Interrupt the execution of the target.  Note how we must play around
> > 
> > I suggest you use mi_getopt instead, for consistency.
> 
> Did you notice this comment?

Yes. mi_getopt is not capable of handling --all. It can only handle -a, which
seems ugly to me.

> Also, please remember to post the version of patches that you check
> in.

OK, will do.

- Volodya

Re: [MI non-stop 05/11, RFA] -exec-continue/-exec-interrupt --all

[Vladimir Prus]

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

Vladimir Prus wrote:

> 
> Per MI non-stop spec, continue and interrupt need --all option
> to resume every thread. Implemented thusly. OK?

Here's what went in. There were mechanical adjustments since
the continue and interrupt code was revised by Pedro per review comments.
No functional changes, though.

- Volodya



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: all.diff --]
[-- Type: text/x-diff; name="all.diff", Size: 5182 bytes --]

commit 426f984015cb596e6119e41a3daa2a4785ed5974
Author: Vladimir Prus <vladimir@codesourcery.com>
Date:   Fri Jun 6 21:26:14 2008 +0400

    -exec-continue/-exec-interrupt --all
    
    	* infcmd.c (continue_1): New, extracted from
    	(continue_command): ...here.
    	(interrupt_target_1): New, extracted from
    	(interrupt_target_command): ...here.
    	* inferior.h (continue_1, interrupt_target_1): New.
    	* mi/mi-main.c (mi_cmd_exec_continue)
    	(mi_cmd_exec_interrupt): Handle --all.

diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index f5fd018..0a40564 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -620,6 +620,31 @@ proceed_thread_callback (struct thread_info *thread, void *arg)
   return 0;
 }
 
+void
+continue_1 (int all_threads)
+{
+  if (non_stop && all_threads)
+    {
+      /* 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.  */
+      old_chain = make_cleanup_restore_current_thread ();
+
+      iterate_over_threads (proceed_thread_callback, NULL);
+
+      /* Restore selected ptid.  */
+      do_cleanups (old_chain);
+    }
+  else
+    {
+      ensure_not_running ();
+      clear_proceed_status ();
+      proceed ((CORE_ADDR) -1, TARGET_SIGNAL_DEFAULT, 0);
+    }
+}
+
 /* continue [-a] [proceed-count] [&]  */
 void
 continue_command (char *args, int from_tty)
@@ -694,26 +719,7 @@ Can't resume all threads and specify proceed count simultaneously."));
   if (from_tty)
     printf_filtered (_("Continuing.\n"));
 
-  if (non_stop && all_threads)
-    {
-      /* 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.  */
-      old_chain = make_cleanup_restore_current_thread ();
-
-      iterate_over_threads (proceed_thread_callback, NULL);
-
-      /* Restore selected ptid.  */
-      do_cleanups (old_chain);
-    }
-  else
-    {
-      ensure_not_running ();
-      clear_proceed_status ();
-      proceed ((CORE_ADDR) -1, TARGET_SIGNAL_DEFAULT, 0);
-    }
+  continue_1 (all_threads);
 }
 \f
 /* Step until outside of current statement.  */
@@ -2146,6 +2152,17 @@ disconnect_command (char *args, int from_tty)
     deprecated_detach_hook ();
 }
 
+void 
+interrupt_target_1 (int all_threads)
+{
+  ptid_t ptid;
+  if (all_threads)
+    ptid = minus_one_ptid;
+  else
+    ptid = inferior_ptid;
+  target_stop (ptid);
+}
+
 /* Stop the execution of the target while running in async mode, in
    the backgound.  In all-stop, stop the whole process.  In non-stop
    mode, stop the current thread only by default, or stop all threads
@@ -2157,7 +2174,6 @@ interrupt_target_command (char *args, int from_tty)
 {
   if (target_can_async_p ())
     {
-      ptid_t ptid;
       int all_threads = 0;
 
       dont_repeat ();		/* Not for the faint of heart */
@@ -2169,12 +2185,7 @@ interrupt_target_command (char *args, int from_tty)
       if (!non_stop && all_threads)
 	error (_("-a is meaningless in all-stop mode."));
 
-      if (all_threads)
-	ptid = minus_one_ptid;
-      else
-	ptid = inferior_ptid;
-
-      target_stop (ptid);
+      interrupt_target_1 (all_threads);
     }
 }
 
diff --git a/gdb/inferior.h b/gdb/inferior.h
index 60d0352..f53af8b 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -271,10 +271,14 @@ extern void nexti_command (char *, int);
 
 extern void stepi_command (char *, int);
 
+extern void continue_1 (int all_threads);
+
 extern void continue_command (char *, int);
 
 extern void interrupt_target_command (char *args, int from_tty);
 
+extern void interrupt_target_1 (int all_threads);
+
 /* Last signal that the inferior received (why it stopped).  */
 
 extern enum target_signal stop_signal;
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index f829a93..43f3a5f 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -163,8 +163,12 @@ mi_cmd_exec_return (char *command, char **argv, int argc)
 void
 mi_cmd_exec_continue (char *command, char **argv, int argc)
 {
-  /* FIXME: Should call a libgdb function, not a cli wrapper.  */
-  return mi_execute_async_cli_command ("continue", argv, argc);
+  if (argc == 0)
+    continue_1 (0);
+  else if (argc == 1 && strcmp (argv[0], "--all") == 0)
+    continue_1 (1);
+  else
+    error ("Usage: -exec-continue [--all]");
 }
 
 /* Interrupt the execution of the target.  Note how we must play around
@@ -175,10 +179,22 @@ mi_cmd_exec_continue (char *command, char **argv, int argc)
 void
 mi_cmd_exec_interrupt (char *command, char **argv, int argc)
 {
-  if (!is_running (inferior_ptid))
-    error ("mi_cmd_exec_interrupt: Inferior not running.");
+  if (argc == 0)
+    {
+      if (!is_running (inferior_ptid))
+	error ("Current thread is not running.");
 
-  interrupt_target_command (NULL, 0);
+      interrupt_target_1 (0);
+    }
+  else if (argc == 1 && strcmp (argv[0], "--all") == 0)
+    {
+      if (!any_running ())
+	error ("Inferior not running.");
+
+      interrupt_target_1 (1);
+    }
+  else
+    error ("Usage: -exec-interrupt [--all]");
 }
 
 void

Re: [MI non-stop 05/11, RFA] -exec-continue/-exec-interrupt --all

[Daniel Jacobowitz]

On Sun, Jul 13, 2008 at 08:36:22AM +0400, Vladimir Prus wrote:
> Yes. mi_getopt is not capable of handling --all. It can only handle -a, which
> seems ugly to me.

Are --thread, --frame, and --all the only long options?  Maybe
mi_getopt can be improved, if we expect more in the future.

> > Also, please remember to post the version of patches that you check
> > in.
> 
> OK, will do.

Thanks.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [MI non-stop 05/11, RFA] -exec-continue/-exec-interrupt --all

[Vladimir Prus]

Daniel Jacobowitz wrote:

> On Sun, Jul 13, 2008 at 08:36:22AM +0400, Vladimir Prus wrote:
>> Yes. mi_getopt is not capable of handling --all. It can only handle -a, which
>> seems ugly to me.
> 
> Are --thread, --frame, and --all the only long options?  

Yes, for the time been.

> Maybe 
> mi_getopt can be improved, if we expect more in the future.

I actually had a second function, that returns a value of an --option; I'm not
yet sure such a function should be added; the thing is that --thread and --frame
are specially processed, so such a function will only handle --all.

- Volodya

Re: [MI non-stop 05/11, RFA] -exec-continue/-exec-interrupt --all

[Nick Roberts]

 > > > I suggest you use mi_getopt instead, for consistency.
 > > 
 > > Did you notice this comment?
 > 
 > Yes. mi_getopt is not capable of handling --all. It can only handle -a, which
 > seems ugly to me.

ISTR that Daniel said something similar many years ago but that wasn't my
experience with this patch:

   http://sourceware.org/ml/gdb-patches/2008-04/msg00615.html

which seemed to work with both long and short options.

-- 
Nick                                           http://www.inet.net.nz/~nickrob