[MI non-stop 06/11, RFA/RFC] Report non-stop availability, and allow to enable everything with one command.

[MI non-stop 06/11, RFA/RFC] Report non-stop availability, and allow to enable everything with one command.

[Vladimir Prus]

This patch allows MI frontend to query for non-stop support, like this:

	-gdb-show non-stop
	^done,value="off",supported="1"

and also allows to set non-stop support with a single command, 
"-gdb-set non-stop 1". The questionable bit of this patch is the 
cli-setshow.c change. Presently, -gdb-show basically is routed via CLI "show"
command. The implementation, when uiout is MI, just outputs a field. For
CLI, a show function hooked to that variable is invoked. I want to
report, in addition to value, the supportedness of the variable. The
only way I found was to output a field in the show function, and make
cli-setshow.c always invoke the show function, even for MI. This will
result in extra "~" output, which is not nice, but tolerable.

Another issue is direct poking at linux async support. I do think that
we need global 'async' variable to implement this cleanly.

Comments?


- Volodya

	* breakpoint.c (breakpoints_set_always_inserted_mode): New.
	* breakpoint.h (breakpoints_set_always_inserted_mode): New.
	* cli/cli-setshow.c (do_setshow_command): Call the setshow
	function even in MI mode.
	* infrun.c (set_non_stop): Auto-enable other necessary bits.
	(show_non_stop): Report if non-stop is supported.
	* linux-nat.c (enable_linux_async): New, extracted from ...
	(set_maintenance_linux_async_permitted): ...this.
	(linux_nat_supports_non_stop): New.
	(linux_nat_add_target): Register to_supports_non_stop.
	* linux-nat.h (enable_linux_async): Declare.
	* target.c (find_default_supports_non_stop)
	(target_supports_non_stop): New.
	(init_dummy_target): Register to_supports_non_stop.
	* target.h (struct target_ops): New field to_supports_non_stop.
	(target_supports_non_stop): New.
---
 gdb/breakpoint.c      |    5 +++++
 gdb/breakpoint.h      |    1 +
 gdb/cli/cli-setshow.c |   19 +++++++++----------
 gdb/infrun.c          |   23 +++++++++++++++++++++++
 gdb/linux-nat.c       |   22 ++++++++++++++++++----
 gdb/linux-nat.h       |    3 +++
 gdb/target.c          |   24 ++++++++++++++++++++++++
 gdb/target.h          |    3 +++
 8 files changed, 86 insertions(+), 14 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 567e212..53b1438 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -8204,6 +8204,11 @@ int breakpoints_always_inserted_mode (void)
   return always_inserted_mode;
 }
 
+void breakpoints_set_always_inserted_mode (int enable)
+{
+  always_inserted_mode = enable;
+}
+
 \f
 /* This help string is used for the break, hbreak, tbreak and thbreak commands.
    It is defined as a macro to prevent duplication.
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 70ab75f..442c9ac 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -875,6 +875,7 @@ void breakpoint_restore_shadows (gdb_byte *buf, ULONGEST memaddr,
 				 LONGEST len);
 
 extern int breakpoints_always_inserted_mode (void);
+extern void breakpoints_set_always_inserted_mode (int enable);
 
 /* Called each time new event from target is processed.
    Retires previously deleted breakpoint locations that
diff --git a/gdb/cli/cli-setshow.c b/gdb/cli/cli-setshow.c
index c5f86fe..0fb7b58 100644
--- a/gdb/cli/cli-setshow.c
+++ b/gdb/cli/cli-setshow.c
@@ -375,16 +375,15 @@ do_setshow_command (char *arg, int from_tty, struct cmd_list_element *c)
 
       if (ui_out_is_mi_like_p (uiout))
 	ui_out_field_stream (uiout, "value", stb);
-      else
-	{
-	  long length;
-	  char *value = ui_file_xstrdup (stb->stream, &length);
-	  make_cleanup (xfree, value);
-	  if (c->show_value_func != NULL)
-	    c->show_value_func (gdb_stdout, from_tty, c, value);
-	  else
-	    deprecated_show_value_hack (gdb_stdout, from_tty, c, value);
-	}
+      {
+	long length;
+	char *value = ui_file_xstrdup (stb->stream, &length);
+	make_cleanup (xfree, value);
+	if (c->show_value_func != NULL)
+	  c->show_value_func (gdb_stdout, from_tty, c, value);
+	else
+	  deprecated_show_value_hack (gdb_stdout, from_tty, c, value);
+      }
       do_cleanups (old_chain);
     }
   else
diff --git a/gdb/infrun.c b/gdb/infrun.c
index e803d1b..567fc2a 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -47,6 +47,9 @@
 #include "main.h"
 #include "interps.h"
 
+/* Oh, dirty! */
+#include "linux-nat.h"
+
 #include "gdb_assert.h"
 #include "mi/mi-common.h"
 
@@ -4798,13 +4801,33 @@ set_non_stop (char *args, int from_tty,
       error (_("Cannot change this setting while the inferior is running."));
     }
 
+  if (!target_supports_non_stop ())
+    {
+      non_stop_1 = non_stop;
+      error (_("Non-stop mode is not supported by the target"));
+    }
+
   non_stop = non_stop_1;
+  if (non_stop)
+    {
+      /* Automatically enable a couple of features required for non-stop
+	 to operate.  
+	 FIXME: what should we do if non-stop is disabled? Leave async
+	 enabled, or disable it back?  */
+      breakpoints_set_always_inserted_mode (1);
+      /* This is dirty. I think we better have a global 'async' 
+	 flag that instructs an async-capable target to actually
+	 be async.  */
+      enable_linux_async (1);
+    }
 }
 
 static void
 show_non_stop (struct ui_file *file, int from_tty,
 	       struct cmd_list_element *c, const char *value)
 {
+  if (ui_out_is_mi_like_p (uiout))
+    ui_out_field_int (uiout, "supported", target_supports_non_stop ());
   fprintf_filtered (file,
 		    _("Controlling the inferior in non-stop mode is %s.\n"),
 		    value);
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 612fad4..9b2bbf2 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -3936,10 +3936,10 @@ static int linux_async_permitted = 0;
    executing, linux_nat_async_permitted is *not* updated.  */
 static int linux_async_permitted_1 = 0;
 
-static void
-set_maintenance_linux_async_permitted (char *args, int from_tty,
-			       struct cmd_list_element *c)
+void
+enable_linux_async (int enable)
 {
+  linux_async_permitted_1 = enable;
   if (target_has_execution)
     {
       linux_async_permitted_1 = linux_async_permitted;
@@ -3947,7 +3947,14 @@ set_maintenance_linux_async_permitted (char *args, int from_tty,
     }
 
   linux_async_permitted = linux_async_permitted_1;
-  linux_nat_set_async_mode (linux_async_permitted);
+  linux_nat_set_async_mode (linux_async_permitted);  
+}
+
+static void
+set_maintenance_linux_async_permitted (char *args, int from_tty,
+			       struct cmd_list_element *c)
+{
+  enable_linux_async (linux_async_permitted_1);
 }
 
 static void
@@ -3988,6 +3995,12 @@ linux_nat_can_async_p (void)
   return linux_nat_async_mask_value;
 }
 
+static int
+linux_nat_supports_non_stop (void)
+{
+  return 1;
+}
+
 /* target_async_mask implementation.  */
 
 static int
@@ -4371,6 +4384,7 @@ linux_nat_add_target (struct target_ops *t)
 
   t->to_can_async_p = linux_nat_can_async_p;
   t->to_is_async_p = linux_nat_is_async_p;
+  t->to_supports_non_stop = linux_nat_supports_non_stop;
   t->to_async = linux_nat_async;
   t->to_async_mask = linux_nat_async_mask;
   t->to_terminal_inferior = linux_nat_terminal_inferior;
diff --git a/gdb/linux-nat.h b/gdb/linux-nat.h
index 3bdb48a..8d1eccc 100644
--- a/gdb/linux-nat.h
+++ b/gdb/linux-nat.h
@@ -135,3 +135,6 @@ void linux_nat_switch_fork (ptid_t new_ptid);
 
 /* Return the saved siginfo associated with PTID.  */
 struct siginfo *linux_nat_get_siginfo (ptid_t ptid);
+
+void enable_linux_async (int enable);
+
diff --git a/gdb/target.c b/gdb/target.c
index 3ac3e30..9f49886 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -2113,6 +2113,29 @@ find_default_is_async_p (void)
   return 0;
 }
 
+int
+find_default_supports_non_stop (void)
+{
+  struct target_ops *t;
+
+  t = find_default_run_target (NULL);
+  if (t && t->to_supports_non_stop)
+    return (t->to_supports_non_stop) ();
+  return 0;
+}
+
+int
+target_supports_non_stop ()
+{
+  struct target_ops *t;
+  for (t = &current_target; t != NULL; t = t->beneath)
+    if (t->to_supports_non_stop)
+      return t->to_supports_non_stop ();
+
+  return 0;
+}
+
+
 static int
 default_region_ok_for_hw_watchpoint (CORE_ADDR addr, int len)
 {
@@ -2388,6 +2411,7 @@ init_dummy_target (void)
   dummy_target.to_create_inferior = find_default_create_inferior;
   dummy_target.to_can_async_p = find_default_can_async_p;
   dummy_target.to_is_async_p = find_default_is_async_p;
+  dummy_target.to_supports_non_stop = find_default_supports_non_stop;
   dummy_target.to_pid_to_str = normal_pid_to_str;
   dummy_target.to_stratum = dummy_stratum;
   dummy_target.to_find_memory_regions = dummy_find_memory_regions;
diff --git a/gdb/target.h b/gdb/target.h
index 81ced29..1f454e5 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -422,6 +422,7 @@ struct target_ops
     int (*to_is_async_p) (void);
     void (*to_async) (void (*) (enum inferior_event_type, void *), void *);
     int (*to_async_mask) (int);
+    int (*to_supports_non_stop) (void);
     int (*to_find_memory_regions) (int (*) (CORE_ADDR,
 					    unsigned long,
 					    int, int, int,
@@ -961,6 +962,8 @@ int target_follow_fork (int follow_child);
 /* Is the target in asynchronous execution mode? */
 #define target_is_async_p() (current_target.to_is_async_p ())
 
+int target_supports_non_stop (void);
+
 /* Put the target in async mode with the specified callback function. */
 #define target_async(CALLBACK,CONTEXT) \
      (current_target.to_async ((CALLBACK), (CONTEXT)))
-- 
1.5.3.5

Re: [MI non-stop 06/11, RFA/RFC] Report non-stop availability, and allow to enable everything with one command.

[Pedro Alves]

A Saturday 28 June 2008 17:54:03, Vladimir Prus wrote:
> This patch allows MI frontend to query for non-stop support, like this:

> Another issue is direct poking at linux async support. I do think that
> we need global 'async' variable to implement this cleanly.

Yeah, I think so too.  Shouldn't be hard to do.

> Comments?
>

Since you're adding a target method, let's think a bit about this
issue:

What happens in these cases:

 GDB build as native linux debugger, we don't support non-stop in remote yet:

 (gdb) set non-stop 1
                         OK, the default run target supports non-stop.

 (gdb) tar rem foo:9999
                         OK, connected

Is the non_stop global on now?  The debug session will be badly broken if
so.

----

Fast forward a couple of weeks, when non-stop in the remote target
is committed,

GDB built as mingw32 native debugger, 

 (gdb) set non-stop 1
                         failure, the default run target doesn't support
                         non-stop.

 (gdb) tar rem foo:9999
                         OK, connected in all-stop.

 (gdb) set non-stop 1
                         Error, target has execution.

Basically, no way to enable non-stop in this case.  :-(

----

Again, fast forward a couple of weeks, when non-stop in the remote target
is enabled,

GDB built as linux native debugger, 

 (gdb) set non-stop 1
                         OK, the default run target does support
                         non-stop.

 (gdb) tar rem foo:9999
                         OK, connected in non-stop.

Great, but only in remote connected from linux.  :-(

----

Also, if you go the combo command sets everything up for non-stop,
we unfortunatelly we'll have to disable pagination as well,
otherwise non-stop will break badly when the output blocks
in an inner event loop due to the pagination prompt  :-(.

Also, ideally we should check if the arch or target support
stepping over breakpoints without removing them, but that's
even another story.  We need to have the issues above solved
first.  Are they already?

>
> - Volodya
>
> 	* breakpoint.c (breakpoints_set_always_inserted_mode): New.
> 	* breakpoint.h (breakpoints_set_always_inserted_mode): New.
> 	* cli/cli-setshow.c (do_setshow_command): Call the setshow
> 	function even in MI mode.
> 	* infrun.c (set_non_stop): Auto-enable other necessary bits.
> 	(show_non_stop): Report if non-stop is supported.
> 	* linux-nat.c (enable_linux_async): New, extracted from ...
> 	(set_maintenance_linux_async_permitted): ...this.
> 	(linux_nat_supports_non_stop): New.
> 	(linux_nat_add_target): Register to_supports_non_stop.
> 	* linux-nat.h (enable_linux_async): Declare.
> 	* target.c (find_default_supports_non_stop)
> 	(target_supports_non_stop): New.
> 	(init_dummy_target): Register to_supports_non_stop.
> 	* target.h (struct target_ops): New field to_supports_non_stop.
> 	(target_supports_non_stop): New.
> ---
>  gdb/breakpoint.c      |    5 +++++
>  gdb/breakpoint.h      |    1 +
>  gdb/cli/cli-setshow.c |   19 +++++++++----------
>  gdb/infrun.c          |   23 +++++++++++++++++++++++
>  gdb/linux-nat.c       |   22 ++++++++++++++++++----
>  gdb/linux-nat.h       |    3 +++
>  gdb/target.c          |   24 ++++++++++++++++++++++++
>  gdb/target.h          |    3 +++
>  8 files changed, 86 insertions(+), 14 deletions(-)
>
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index 567e212..53b1438 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -8204,6 +8204,11 @@ int breakpoints_always_inserted_mode (void)
>    return always_inserted_mode;
>  }
>
> +void breakpoints_set_always_inserted_mode (int enable)
> +{
> +  always_inserted_mode = enable;
> +}
> +
>  \f
>  /* This help string is used for the break, hbreak, tbreak and thbreak
> commands. It is defined as a macro to prevent duplication.
> diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
> index 70ab75f..442c9ac 100644
> --- a/gdb/breakpoint.h
> +++ b/gdb/breakpoint.h
> @@ -875,6 +875,7 @@ void breakpoint_restore_shadows (gdb_byte *buf,
> ULONGEST memaddr, LONGEST len);
>
>  extern int breakpoints_always_inserted_mode (void);
> +extern void breakpoints_set_always_inserted_mode (int enable);
>
>  /* Called each time new event from target is processed.
>     Retires previously deleted breakpoint locations that
> diff --git a/gdb/cli/cli-setshow.c b/gdb/cli/cli-setshow.c
> index c5f86fe..0fb7b58 100644
> --- a/gdb/cli/cli-setshow.c
> +++ b/gdb/cli/cli-setshow.c
> @@ -375,16 +375,15 @@ do_setshow_command (char *arg, int from_tty, struct
> cmd_list_element *c)
>
>        if (ui_out_is_mi_like_p (uiout))
>  	ui_out_field_stream (uiout, "value", stb);
> -      else
> -	{
> -	  long length;
> -	  char *value = ui_file_xstrdup (stb->stream, &length);
> -	  make_cleanup (xfree, value);
> -	  if (c->show_value_func != NULL)
> -	    c->show_value_func (gdb_stdout, from_tty, c, value);
> -	  else
> -	    deprecated_show_value_hack (gdb_stdout, from_tty, c, value);
> -	}
> +      {
> +	long length;
> +	char *value = ui_file_xstrdup (stb->stream, &length);
> +	make_cleanup (xfree, value);
> +	if (c->show_value_func != NULL)
> +	  c->show_value_func (gdb_stdout, from_tty, c, value);
> +	else
> +	  deprecated_show_value_hack (gdb_stdout, from_tty, c, value);
> +      }
>        do_cleanups (old_chain);
>      }
>    else
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index e803d1b..567fc2a 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -47,6 +47,9 @@
>  #include "main.h"
>  #include "interps.h"
>
> +/* Oh, dirty! */
> +#include "linux-nat.h"
> +
>  #include "gdb_assert.h"
>  #include "mi/mi-common.h"
>
> @@ -4798,13 +4801,33 @@ set_non_stop (char *args, int from_tty,
>        error (_("Cannot change this setting while the inferior is
> running.")); }
>
> +  if (!target_supports_non_stop ())
> +    {
> +      non_stop_1 = non_stop;
> +      error (_("Non-stop mode is not supported by the target"));
> +    }
> +
>    non_stop = non_stop_1;
> +  if (non_stop)
> +    {
> +      /* Automatically enable a couple of features required for non-stop
> +	 to operate.
> +	 FIXME: what should we do if non-stop is disabled? Leave async
> +	 enabled, or disable it back?  */
> +      breakpoints_set_always_inserted_mode (1);
> +      /* This is dirty. I think we better have a global 'async'
> +	 flag that instructs an async-capable target to actually
> +	 be async.  */
> +      enable_linux_async (1);
> +    }
>  }
>
>  static void
>  show_non_stop (struct ui_file *file, int from_tty,
>  	       struct cmd_list_element *c, const char *value)
>  {
> +  if (ui_out_is_mi_like_p (uiout))
> +    ui_out_field_int (uiout, "supported", target_supports_non_stop ());
>    fprintf_filtered (file,
>  		    _("Controlling the inferior in non-stop mode is %s.\n"),
>  		    value);
> diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
> index 612fad4..9b2bbf2 100644
> --- a/gdb/linux-nat.c
> +++ b/gdb/linux-nat.c
> @@ -3936,10 +3936,10 @@ static int linux_async_permitted = 0;
>     executing, linux_nat_async_permitted is *not* updated.  */
>  static int linux_async_permitted_1 = 0;
>
> -static void
> -set_maintenance_linux_async_permitted (char *args, int from_tty,
> -			       struct cmd_list_element *c)
> +void
> +enable_linux_async (int enable)
>  {
> +  linux_async_permitted_1 = enable;
>    if (target_has_execution)
>      {
>        linux_async_permitted_1 = linux_async_permitted;
> @@ -3947,7 +3947,14 @@ set_maintenance_linux_async_permitted (char *args,
> int from_tty, }
>
>    linux_async_permitted = linux_async_permitted_1;
> -  linux_nat_set_async_mode (linux_async_permitted);
> +  linux_nat_set_async_mode (linux_async_permitted);
> +}
> +
> +static void
> +set_maintenance_linux_async_permitted (char *args, int from_tty,
> +			       struct cmd_list_element *c)
> +{
> +  enable_linux_async (linux_async_permitted_1);
>  }
>
>  static void
> @@ -3988,6 +3995,12 @@ linux_nat_can_async_p (void)
>    return linux_nat_async_mask_value;
>  }
>
> +static int
> +linux_nat_supports_non_stop (void)
> +{
> +  return 1;
> +}
> +
>  /* target_async_mask implementation.  */
>
>  static int
> @@ -4371,6 +4384,7 @@ linux_nat_add_target (struct target_ops *t)
>
>    t->to_can_async_p = linux_nat_can_async_p;
>    t->to_is_async_p = linux_nat_is_async_p;
> +  t->to_supports_non_stop = linux_nat_supports_non_stop;
>    t->to_async = linux_nat_async;
>    t->to_async_mask = linux_nat_async_mask;
>    t->to_terminal_inferior = linux_nat_terminal_inferior;
> diff --git a/gdb/linux-nat.h b/gdb/linux-nat.h
> index 3bdb48a..8d1eccc 100644
> --- a/gdb/linux-nat.h
> +++ b/gdb/linux-nat.h
> @@ -135,3 +135,6 @@ void linux_nat_switch_fork (ptid_t new_ptid);
>
>  /* Return the saved siginfo associated with PTID.  */
>  struct siginfo *linux_nat_get_siginfo (ptid_t ptid);
> +
> +void enable_linux_async (int enable);
> +
> diff --git a/gdb/target.c b/gdb/target.c
> index 3ac3e30..9f49886 100644
> --- a/gdb/target.c
> +++ b/gdb/target.c
> @@ -2113,6 +2113,29 @@ find_default_is_async_p (void)
>    return 0;
>  }
>
> +int
> +find_default_supports_non_stop (void)
> +{
> +  struct target_ops *t;
> +
> +  t = find_default_run_target (NULL);
> +  if (t && t->to_supports_non_stop)
> +    return (t->to_supports_non_stop) ();
> +  return 0;
> +}
> +
> +int
> +target_supports_non_stop ()
> +{
> +  struct target_ops *t;
> +  for (t = &current_target; t != NULL; t = t->beneath)
> +    if (t->to_supports_non_stop)
> +      return t->to_supports_non_stop ();
> +
> +  return 0;
> +}
> +
> +
>  static int
>  default_region_ok_for_hw_watchpoint (CORE_ADDR addr, int len)
>  {
> @@ -2388,6 +2411,7 @@ init_dummy_target (void)
>    dummy_target.to_create_inferior = find_default_create_inferior;
>    dummy_target.to_can_async_p = find_default_can_async_p;
>    dummy_target.to_is_async_p = find_default_is_async_p;
> +  dummy_target.to_supports_non_stop = find_default_supports_non_stop;
>    dummy_target.to_pid_to_str = normal_pid_to_str;
>    dummy_target.to_stratum = dummy_stratum;
>    dummy_target.to_find_memory_regions = dummy_find_memory_regions;
> diff --git a/gdb/target.h b/gdb/target.h
> index 81ced29..1f454e5 100644
> --- a/gdb/target.h
> +++ b/gdb/target.h
> @@ -422,6 +422,7 @@ struct target_ops
>      int (*to_is_async_p) (void);
>      void (*to_async) (void (*) (enum inferior_event_type, void *), void
> *); int (*to_async_mask) (int);
> +    int (*to_supports_non_stop) (void);
>      int (*to_find_memory_regions) (int (*) (CORE_ADDR,
>  					    unsigned long,
>  					    int, int, int,
> @@ -961,6 +962,8 @@ int target_follow_fork (int follow_child);
>  /* Is the target in asynchronous execution mode? */
>  #define target_is_async_p() (current_target.to_is_async_p ())
>
> +int target_supports_non_stop (void);
> +
>  /* Put the target in async mode with the specified callback function. */
>  #define target_async(CALLBACK,CONTEXT) \
>       (current_target.to_async ((CALLBACK), (CONTEXT)))



-- 
Pedro Alves

Re: [MI non-stop 06/11, RFA/RFC] Report non-stop availability, and allow to enable everything with one command.

[Vladimir Prus]

On Friday 11 July 2008 17:34:17 you wrote:
> A Saturday 28 June 2008 17:54:03, Vladimir Prus wrote:
> > This patch allows MI frontend to query for non-stop support, like this:
> 
> > Another issue is direct poking at linux async support. I do think that
> > we need global 'async' variable to implement this cleanly.
> 
> Yeah, I think so too.  Shouldn't be hard to do.
> 
> > Comments?
> >
> 
> Since you're adding a target method, let's think a bit about this
> issue:
> 
> What happens in these cases:
> 
>  GDB build as native linux debugger, we don't support non-stop in remote yet:
> 
>  (gdb) set non-stop 1
>                          OK, the default run target supports non-stop.
> 
>  (gdb) tar rem foo:9999
>                          OK, connected
> 
> Is the non_stop global on now?  The debug session will be badly broken if
> so.
> 
> ----
> 
> Fast forward a couple of weeks, when non-stop in the remote target
> is committed,
> 
> GDB built as mingw32 native debugger, 
> 
>  (gdb) set non-stop 1
>                          failure, the default run target doesn't support
>                          non-stop.
> 
>  (gdb) tar rem foo:9999
>                          OK, connected in all-stop.
> 
>  (gdb) set non-stop 1
>                          Error, target has execution.
> 
> Basically, no way to enable non-stop in this case.  :-(

Gah; I've forgotten that presently, you cannot change to non-stop when
target has execution. I assumed the frontend would first select a target
(or implicitly select the default one), then set non-stop, and never
touch the target again.

I think we have two possible approaches now:

1. Make 'non-stop' variable mean 'I want non-stop, from all targets that
support it'. Then, the internal 'non_stop' variable will be true when the
current target does support non-stop, and the user-level non-stop variable will
correspond to some non_stop_request variable, which will be consulted each time
we change targets. There should be a way to query if the target really supports
non-stop.

2. Make 'set non-stop 1' work even if the target has execution. I actually not
sure why the error is emitted currently. If the target is fully stopped when
we do "target remote foo:9999", then changing non-stop setting should not have
any bad effect. We'd have to reset non-stop back to 0 when pushing targets --
either unconditionally, or when the new target is not non-stop capable.

What do you think? (2) seems more clear to me.

- Volodya

Re: [MI non-stop 06/11, RFA/RFC] Report non-stop availability, and allow to enable everything with one command.

[Pedro Alves]

On Wednesday 23 July 2008 08:48:26, Vladimir Prus wrote:
> Gah; I've forgotten that presently, you cannot change to non-stop when
> target has execution. I assumed the frontend would first select a target
> (or implicitly select the default one), then set non-stop, and never
> touch the target again.

In most targets that allow selection with "target remote/sim/etc", when
you select them, you also implicitly get execution.  E.g., 
"target remote" vs "target extended-remote".  Even connecting
with extended-remote can find that the stub was already controling
some inferiors, (due to the "?" packet).  We may even have
a stub that only supports non-stop, and doesn't stop the inferior on
connection (say "target remote" into a running OS).  The connection setup
phase already needs to know if we're trying to use non-stop or not, so,
chosing non-stop only after target selection will not work out.

>
> I think we have two possible approaches now:
>
> 1. Make 'non-stop' variable mean 'I want non-stop, from all targets that
> support it'. Then, the internal 'non_stop' variable will be true when the
> current target does support non-stop, and the user-level non-stop variable
> will correspond to some non_stop_request variable, which will be consulted
> each time we change targets. There should be a way to query if the target
> really supports non-stop.

The thing is that with native targets, we can mostly (*) tell upfront if
they support non-stop, but with remote targets, only after connection can
we tell if the stub supports it (stub will report with qSupported).  We
have to assume no support by default.

(*) Not quite true.  We can ask the default run target (process_stratum),
but support for non-stop, which requires scheduler-locking and tweaking
with threads, may or may not be supported by the thread_stratum target
that ends up being pushed.  There are OSs where there is more than
one possible thread_stratum target (e.g., BSD's and kernel vs user
threads).

> 2. Make 'set non-stop 1' work even if the target has execution. I actually
> not sure why the error is emitted currently. 

Because at the current state of inferior control, it is hard to move
between non-stop <-> all-stop when there's already an inferior under
control.  That may change, especially after getting rid of context-switching
(Real Soon Now (TM)) but that's how things are currently.

> If the target is fully stopped 
> when we do "target remote foo:9999", then changing non-stop setting should
> not have any bad effect. We'd have to reset non-stop back to 0 when pushing
> targets -- either unconditionally, or when the new target is not non-stop
> capable.

> What do you think? (2) seems more clear to me.

I still can't tell what might be better.  E.g., this is how
currently connection with the remote target with non-stop support looks
like:

 remote.c:remote_open_1 ()
 {
 (...)
   /* The first packet we send to the target is the optional "supported
      packets" request.  If the target can answer this, it will tell us
      which later probes to skip.  */
   remote_query_supported ();

   if (non_stop && !rs->non_stop_aware)
     error ("Non-stop mode requested, but remote does not support non-stop");

 (...)
   remote_start_remote ();
 }

 remote_start_remote()
 {
   (several checks for non_stop)  
 }

That is, we can't drop non-stop before connection, and only setting it
after connection after target_open returning breaks too.  Events may
need to be handled before returning from target_open.

I'll either have to think more about this, or perhaps you could try it
out against a DICOS stub.  ;-)

Also, if we set non-stop and it enables async + breakpoints-always-in,
plus whatnot, and then the target refuses it, do we get to revert the
async,breakpoints always-in,whatnot settings?

-- 
Pedro Alves

Re: [MI non-stop 06/11, RFA/RFC] Report non-stop availability, and allow to enable everything with one command.

[Vladimir Prus]

On Monday 28 July 2008 21:57:48 Pedro Alves wrote:
> On Wednesday 23 July 2008 08:48:26, Vladimir Prus wrote:
> > Gah; I've forgotten that presently, you cannot change to non-stop when
> > target has execution. I assumed the frontend would first select a target
> > (or implicitly select the default one), then set non-stop, and never
> > touch the target again.
> 
> In most targets that allow selection with "target remote/sim/etc", when
> you select them, you also implicitly get execution.  E.g., 
> "target remote" vs "target extended-remote".  Even connecting
> with extended-remote can find that the stub was already controling
> some inferiors, (due to the "?" packet).  We may even have
> a stub that only supports non-stop, and doesn't stop the inferior on
> connection (say "target remote" into a running OS).  The connection setup
> phase already needs to know if we're trying to use non-stop or not, so,
> chosing non-stop only after target selection will not work out.

The question is why the connection setup needs to know if we're in non-stop,
and why -- which question we discuss below.

> > I think we have two possible approaches now:
> >
> > 1. Make 'non-stop' variable mean 'I want non-stop, from all targets that
> > support it'. Then, the internal 'non_stop' variable will be true when the
> > current target does support non-stop, and the user-level non-stop variable
> > will correspond to some non_stop_request variable, which will be consulted
> > each time we change targets. There should be a way to query if the target
> > really supports non-stop.
> 
> The thing is that with native targets, we can mostly (*) tell upfront if
> they support non-stop, but with remote targets, only after connection can
> we tell if the stub supports it (stub will report with qSupported).  We
> have to assume no support by default.

I think we can implement a scheme where a target say it does not know
if non-stop is supported until connecting, and have the target set non_stop
to zero if it's actually not supported. Something like probing for packets
in remote protocol.

> (*) Not quite true.  We can ask the default run target (process_stratum),
> but support for non-stop, which requires scheduler-locking and tweaking
> with threads, may or may not be supported by the thread_stratum target
> that ends up being pushed.  There are OSs where there is more than
> one possible thread_stratum target (e.g., BSD's and kernel vs user
> threads).
> 
> > 2. Make 'set non-stop 1' work even if the target has execution. I actually
> > not sure why the error is emitted currently. 
> 
> Because at the current state of inferior control, it is hard to move
> between non-stop <-> all-stop when there's already an inferior under
> control.  That may change, especially after getting rid of context-switching
> (Real Soon Now (TM)) but that's how things are currently.

Well, if that might change then probably (2) is the right approach, and we should
wait for your changes to land?
 
> > If the target is fully stopped 
> > when we do "target remote foo:9999", then changing non-stop setting should
> > not have any bad effect. We'd have to reset non-stop back to 0 when pushing
> > targets -- either unconditionally, or when the new target is not non-stop
> > capable.
> 
> > What do you think? (2) seems more clear to me.
> 
> I still can't tell what might be better.  E.g., this is how
> currently connection with the remote target with non-stop support looks
> like:
> 
>  remote.c:remote_open_1 ()
>  {
>  (...)
>    /* The first packet we send to the target is the optional "supported
>       packets" request.  If the target can answer this, it will tell us
>       which later probes to skip.  */
>    remote_query_supported ();
> 
>    if (non_stop && !rs->non_stop_aware)
>      error ("Non-stop mode requested, but remote does not support non-stop");
> 
>  (...)
>    remote_start_remote ();
>  }
> 
>  remote_start_remote()
>  {
>    (several checks for non_stop)  
>  }

Or maybe not -- the logic in remote_start_remote seems not very trivial.
 
> That is, we can't drop non-stop before connection, and only setting it
> after connection after target_open returning breaks too.  Events may
> need to be handled before returning from target_open.
> 
> I'll either have to think more about this, or perhaps you could try it
> out against a DICOS stub.  ;-)

I surely can try it against a DICOS stub -- but it's not clear what to 
try :-) 

Basically, from the point of frontend we need a way to say that non-stop,
and async, are desired, and to understand if that request is respected.
Whether frontend sets 'non-stop' to 1 after 'target remote' and gets the
error immediately, or sets it to 1 before target remote, and later queries
if non-stop is actually enabled, is not very important. However, the behaviour
of non-stop and of various ways to enable async should be consistent, I think.

That is, the options are:

1. We do

    set non-stop 1
    set async 1
    target remote xxx

and if the target does not support either non-stop, or async, we get error.

2. We do:

    set non-stop 1
    set async 1
    target remote xxx

and if the target does not support either non-stop or async, we'll operate
in all-stop, or sync more, respectively. There will be some way to check if
we're really in non-stop, or async, mode.

3. We do:

    target remote xxxx
    set non-stop 1
    set async 1

and then if non-stop or async is not supported, we get an error from the "set"
command. 

For (3), the question is what to do if we send another "target XXX" command. Should
we reset both variables, or should we do either (1) or (2). For (1) I don't like
the hard error -- if the frontend really demands non-stop mode, it can always bark
itself. So, I think (2) is best. OTOH, it's not how non-stop mode works now --
remote will error out if non stop is not supported by the stub, so we'd have to split
non_stop into non_stop_requested and non_stop_actually_enabled, which is mechanical,
but big change. What do you think is best here?

> Also, if we set non-stop and it enables async + breakpoints-always-in,
> plus whatnot, and then the target refuses it, do we get to revert the
> async,breakpoints always-in,whatnot settings?

Well, I now think individual toggles are better -- with non-stop mode checking
if async and breakpoints-always-inserted are on. This way, if we can't enable
non-stop, the frontend is in control of whether the others two should still
be enabled.

- Volodya

 

Re: [MI non-stop 06/11, RFA/RFC] Report non-stop availability, and allow to enable everything with one command.

[Pedro Alves]

On Monday 04 August 2008 13:58:57, Vladimir Prus wrote:
> The question is why the connection setup needs to know if we're in
> non-stop, and why -- which question we discuss below.

To tell the stub which mode it should enable, and to give it the
option to refuse working in all-stop mode.

> I think we can implement a scheme where a target say it does not know
> if non-stop is supported until connecting, and have the target set non_stop
> to zero if it's actually not supported. Something like probing for packets
> in remote protocol.

We for sure know that no target supports it, except for linux and
remote.  The issue is which target do you ask, before connecting, since
the default run target may or may not support it.  (e.g., linux -> remote,
vs mingw32 -> remote, where the stub on the remote side is the same, and
gdb is also configured with a native debugger included).

> > > 2. Make 'set non-stop 1' work even if the target has execution. I
> > > actually not sure why the error is emitted currently.
> >
> > Because at the current state of inferior control, it is hard to move
> > between non-stop <-> all-stop when there's already an inferior under
> > control.  That may change, especially after getting rid of
> > context-switching (Real Soon Now (TM)) but that's how things are
> > currently.
>

> Well, if that might change then probably (2) is the right approach, and we
> should wait for your changes to land?

Let's just plan the commands and state vars to be able to do it in
the future.

> Or maybe not -- the logic in remote_start_remote seems not very trivial.

Right. :-/

> Basically, from the point of frontend we need a way to say that non-stop,
> and async, are desired, and to understand if that request is respected.
> Whether frontend sets 'non-stop' to 1 after 'target remote' and gets the
> error immediately, or sets it to 1 before target remote, and later queries
> if non-stop is actually enabled, is not very important. However, the
> behaviour of non-stop and of various ways to enable async should be
> consistent, I think.
>
> That is, the options are:
>
> 1. We do
>
>     set non-stop 1
>     set async 1
>     target remote xxx
>
> and if the target does not support either non-stop, or async, we get error.
>
> 2. We do:
>
>     set non-stop 1
>     set async 1
>     target remote xxx
>
> and if the target does not support either non-stop or async, we'll operate
> in all-stop, or sync more, respectively. There will be some way to check if
> we're really in non-stop, or async, mode.
>
> 3. We do:
>
>     target remote xxxx
>     set non-stop 1
>     set async 1
>
> and then if non-stop or async is not supported, we get an error from the
> "set" command.

"set async 1" should just not exist (that's why it's a maintainance
commands today), so let's imagine it's not needed.

> For (3), the question is what to do if we send another "target XXX"
> command. Should we reset both variables, or should we do either (1) or (2).

> For (1) I don't like the hard error -- if the frontend really demands
> non-stop mode, it can always bark itself.  So, I think (2) is best. OTOH, 
> it's not how non-stop mode works now -- remote will error out if non stop
> is not supported by the stub, so we'd have to split non_stop into
> non_stop_requested and non_stop_actually_enabled, which is mechanical, but
> big change. What do you think is best here?

It's not a big change if we leave the non_stop global to mean that we're
in non_stop, and come up with a new non_stop_willingness global.

(2) is fine with me.  It means that both linux-nat and remote
will have to set the non_stop global themselves, after checking the
non_stop_willingness global.

The core code that checks for non_stop always checks it when there's
already execution (e.g., interrupt and continue commands), unlike
asyncness, which is checked before creating an inferior in run
and attach, so it looks safe enough.

We'd then in the future make the non_stop inferior control global a
per-target property, or perhaps even per-inferior, but for now,
there's no harm in leaving it a global.

While we're on to it, perhaps we should rename the
command to:

 set execution-mode "all-stop", "non-stop".

 "all-stop"
    prefer all-stop, but if the target doesn't support it, fine,
    fall-back to what the target wants.
 "non-stop"
    prefer non-stop, but if the target doesn't support it, fine,
    fall-back to what the target wants.

?

We leaves some slack to add new modes like this, which would
combine (1) and (2):

set prefered-execution-mode
 "all-stop"
    prefer all-stop, but if the target doesn't support it, fine.
 "non-stop"
    prefer non-stop, but if the target doesn't support it, fine.
 "force-all-stop"
    require all-stop, fail if the target refuses it.
 "force-non-stop"
    require all-stop, fail if the target refuses it.

?

In remote_open_1, the initial connection setup would change to
something similar to:

 if (!rs->non_stop_aware)
   {
      if (prefered_execution_mode == FORCE_NON_STOP)
        error ("boo!, target does not support non-stop");
      non_stop = 0;
   }
 else
   {
    if (prefered_execution_mode == PREFER_NON_STOP)
      {
        putpkt ("QNonStop:0");
        getpkt (&rs->buf, &rs->buf_size, 0);
        if (strcmp (rs->buf, "OK") == 0)
          non_stop = 1;
      }
     else if (prefered_execution_mode == PREFER_ALL_STOP)
       {
         putpkt ("QNonStop:0");
         getpkt (&rs->buf, &rs->buf_size, 0);
         if (strcmp (rs->buf, "OK") != 0)
           non_stop = 1;
       }
    else if (prefered_execution_mode == FORCE_NON_STOP)
      {
        putpkt ("QNonStop:1");
        getpkt (&rs->buf, &rs->buf_size, 0);
        if (strcmp (rs->buf, "OK") != 0)
          error ("boo!, target refused non-stop");
        non_stop = 1;
      }
     else if (prefered_execution_mode == FORCE_ALL_STOP)
       {
         putpkt ("QNonStop:0");
         getpkt (&rs->buf, &rs->buf_size, 0);
         if (strcmp (rs->buf, "OK") != 0)
          error ("boo!, target refused all-stop");
         non_stop = 0;
       }
  }

If you want to check the current execution mode, you'd obviously
then read from the non_stop global (or whatever we rename it to).

This still leaves making targets other than linux and remote
refuse the FORCE_NON_STOP setting, but it doesn't sound as bad
if we fail to do that.  Note that I'm just suggesting the FORCE_*
variants as a possible future direction, if it turns out required.
We may or not do it now, it's fine with me either way.

BTW, what is it that currently makes the frontend want to check if
non-stop has been activated?  If it possible to remove that need,
other than for informational purposes?

> > Also, if we set non-stop and it enables async + breakpoints-always-in,
> > plus whatnot, and then the target refuses it, do we get to revert the
> > async,breakpoints always-in,whatnot settings?
>
> Well, I now think individual toggles are better -- with non-stop mode
> checking if async and breakpoints-always-inserted are on. This way, if we
> can't enable non-stop, the frontend is in control of whether the others two
> should still be enabled.

Re: always-inserted, make it a three-state?

 - follow all/non-stop mode (default)
 - on
 - off

-- 
Pedro Alves

Re: [MI non-stop 06/11, RFA/RFC] Report non-stop availability, and allow to enable everything with one command.

[Daniel Jacobowitz]

On Tue, Aug 05, 2008 at 05:30:50PM +0100, Pedro Alves wrote:
> On Monday 04 August 2008 13:58:57, Vladimir Prus wrote:
> > The question is why the connection setup needs to know if we're in
> > non-stop, and why -- which question we discuss below.

You have to know whether you're connecting in non-stop, because if you
connect in all-stop the target might have to stop a running thread
that it would leave running if you connected in non-stop.

I can't follow all the possibilities being juggled in this
conversation.  But why not just set non-stop in advance, regardless of
the target, and then issue an error at run / target remote / wherever
if non-stop is not available?

> We leaves some slack to add new modes like this, which would
> combine (1) and (2):
> 
> set prefered-execution-mode
>  "all-stop"
>     prefer all-stop, but if the target doesn't support it, fine.
>  "non-stop"
>     prefer non-stop, but if the target doesn't support it, fine.
>  "force-all-stop"
>     require all-stop, fail if the target refuses it.
>  "force-non-stop"
>     require all-stop, fail if the target refuses it.

Please don't, the user should know what they get.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [MI non-stop 06/11, RFA/RFC] Report non-stop availability, and allow to enable everything with one command.

[Pedro Alves]

A Tuesday 05 August 2008 19:27:35, Daniel Jacobowitz wrote:

> I can't follow all the possibilities being juggled in this
> conversation.  But why not just set non-stop in advance, regardless of
> the target, and then issue an error at run / target remote / wherever
> if non-stop is not available?

Yes, that's what I've been proposing/saying we must do.  Clearly, I
can't express myself that well.

> > We leaves some slack to add new modes like this, which would
> > combine (1) and (2):
> >
> > set prefered-execution-mode
> >  "all-stop"
> >     prefer all-stop, but if the target doesn't support it, fine.
> >  "non-stop"
> >     prefer non-stop, but if the target doesn't support it, fine.
> >  "force-all-stop"
> >     require all-stop, fail if the target refuses it.
> >  "force-non-stop"
> >     require all-stop, fail if the target refuses it.
>
> Please don't, the user should know what they get.

Ok, then we're back to what we have currently.  I only proposed
that, because Vladimir didn't like the exception/error that is
currenly thrown.  (In the unsubmited remote target; linux
doesn't do it yet).  I'll leave it to Vladimir to justify
not having an error and falling back to all-stop/non-stop, if he
still wants it.

-- 
Pedro Alves

Re: [MI non-stop 06/11, RFA/RFC] Report non-stop availability, and allow to enable everything with one command.

[Vladimir Prus]

On Tuesday 05 August 2008 23:08:46 Pedro Alves wrote:
> A Tuesday 05 August 2008 19:27:35, Daniel Jacobowitz wrote:
> 
> > I can't follow all the possibilities being juggled in this
> > conversation.  But why not just set non-stop in advance, regardless of
> > the target, and then issue an error at run / target remote / wherever
> > if non-stop is not available?
> 
> Yes, that's what I've been proposing/saying we must do.  Clearly, I
> can't express myself that well.
> 
> > > We leaves some slack to add new modes like this, which would
> > > combine (1) and (2):
> > >
> > > set prefered-execution-mode
> > >  "all-stop"
> > >     prefer all-stop, but if the target doesn't support it, fine.
> > >  "non-stop"
> > >     prefer non-stop, but if the target doesn't support it, fine.
> > >  "force-all-stop"
> > >     require all-stop, fail if the target refuses it.
> > >  "force-non-stop"
> > >     require all-stop, fail if the target refuses it.
> >
> > Please don't, the user should know what they get.
> 
> Ok, then we're back to what we have currently.  I only proposed
> that, because Vladimir didn't like the exception/error that is
> currenly thrown.  (In the unsubmited remote target; linux
> doesn't do it yet).  I'll leave it to Vladimir to justify
> not having an error and falling back to all-stop/non-stop, if he
> still wants it.

My motivation was that the most intuitive model is that of
immediate application of non-stop flag, with error produced
immediately. This is hard to implement.

Next most intuitive model is "I prefer non-stop mode", which is
what I propose.

The model where 'non-stop' variable is a hard request, 
but the error is delayed seems fairly unconventional to me.

- Volodya

Re: [MI non-stop 06/11, RFA/RFC] Report non-stop availability, and allow to enable everything with one command.

[Daniel Jacobowitz]

On Tue, Aug 12, 2008 at 10:09:11AM +0400, Vladimir Prus wrote:
> My motivation was that the most intuitive model is that of
> immediate application of non-stop flag, with error produced
> immediately. This is hard to implement.
> 
> Next most intuitive model is "I prefer non-stop mode", which is
> what I propose.

Actually I think this is very unintuitive.  You'll have to know
whether you get non-stop or not because commands act very differently
between all-stop and non-stop.  Scripts written for the one won't work
with the other, for example.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [MI non-stop 06/11, RFA/RFC] Report non-stop availability, and allow to enable everything with one command.

[Vladimir Prus]

On Tuesday 12 August 2008 16:08:35 Daniel Jacobowitz wrote:
> On Tue, Aug 12, 2008 at 10:09:11AM +0400, Vladimir Prus wrote:
> > My motivation was that the most intuitive model is that of
> > immediate application of non-stop flag, with error produced
> > immediately. This is hard to implement.
> > 
> > Next most intuitive model is "I prefer non-stop mode", which is
> > what I propose.
> 
> Actually I think this is very unintuitive.  You'll have to know
> whether you get non-stop or not because commands act very differently
> between all-stop and non-stop.  Scripts written for the one won't work
> with the other, for example.

Yes, you have to know whether you get non-stop or not -- does this contradict
to anything I've said?

- Volodya


 

Re: [MI non-stop 06/11, RFA/RFC] Report non-stop availability, and allow to enable everything with one command.

[Daniel Jacobowitz]

On Tue, Aug 12, 2008 at 04:46:38PM +0400, Vladimir Prus wrote:
> On Tuesday 12 August 2008 16:08:35 Daniel Jacobowitz wrote:
> > On Tue, Aug 12, 2008 at 10:09:11AM +0400, Vladimir Prus wrote:
> > > My motivation was that the most intuitive model is that of
> > > immediate application of non-stop flag, with error produced
> > > immediately. This is hard to implement.
> > > 
> > > Next most intuitive model is "I prefer non-stop mode", which is
> > > what I propose.
> > 
> > Actually I think this is very unintuitive.  You'll have to know
> > whether you get non-stop or not because commands act very differently
> > between all-stop and non-stop.  Scripts written for the one won't work
> > with the other, for example.
> 
> Yes, you have to know whether you get non-stop or not -- does this contradict
> to anything I've said?

Yes.  If enabling non-stop means "I prefer non-stop" then every script
and front end has to query "I asked for non-stop but did I really get
it?" and the user has to be paying attention to GDB's messages.  I
don't think that's a good idea.

This is mostly a problem for the testsuite.  Non-stop will be awesome
and useful so the targets where people want it, it'll get implemented :-)

-- 
Daniel Jacobowitz
CodeSourcery