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

[Pedro Alves <pedro at codesourcery dot com>]

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;
> +}
> +
>  
>  /* 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