RE: [RFA] Unify async setting.

Marc Khouzam
2008-08-15T17:30:40Z

Hi,

it think it is great to have a single command to set async for native
or targets.
Will the command be 'set async' or 'set target-async', the patch
seems to use target-async.

Thanks

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org
> [mailto:gdb-patches-owner@sourceware.org]On Behalf Of Vladimir Prus
> Sent: Friday, August 15, 2008 1:07 PM
> To: gdb-patches@sources.redhat.com
> Subject: [RFA] Unify async setting.
>
>
>
> Presently, one need to use different commands to request
> async mode for linux
> native and for async targets, which is hard on frontends.
> Furthermore, the
> relevant variables merely *allow* async mode, and there's no
> way for frontend
> to make sure that async mode is actually in effect. This
> patch makes two things:
>
> - Converts target-specific variables into a single 'set
> async' command.
> - Adds new MI -list-target-features command that allows to
> check if current
> target is in async mode.
>
> The bulk of the patch is actually written by Pedro, I've only
> added MI/testsuite/doc
> bits.
>
> OK?
>
> - Volodya
>
> * target.c (target_async_permitted, target_async_permitted_1)
> (set_maintenance_target_async_permitted)
> (show_maintenance_target_async_permitted): New.
> (initialize_targets): Register 'set target-async'.
> * target.h (target_async_permitted): Declare.
> * linux-nat.c (linux_nat_async_enabled)
> (linux_nat_async_permitted,
> set_maintenance_linux_async_permitted)
> (show_maintenance_linux_async_permitted): Remove.
> (sigchld_handler, linux_nat_is_async_p, linux_nat_can_async_p)
> (get_pending_events, linux_nat_async): Use
> target_async_permitted.
> (linux_nat_set_async_mode): Remove, moving the only used bits
> into...
> (linux_nat_setup_async): This.
> (_initialize_linux_nat): Do not register 'maint set
> linux-async'.
> Use linux_nat_setup_async.
> * remote.c (remote_async_permitted, remote_async_permitted_set)
> (set_maintenance_remote_async_permitted)
> (show_maintenance_remote_async_permitted): Remove.
> (remote_open_1, remote_terminal_inferior, remote_can_async_p)
> (remote_is_async_p): Use target_async_permitted.
> (_initialize_remote): Don't register 'main set remote-async'.
> * mi/mi-cmds.c (mi_cmds): Register -list-target-features.
> * mi/mi-cmds.h (mi_cmd_list_target_features): New.
> * mi/mi-main.c (mi_cmd_list_target_features): New.
>
> [testsuite/ChangeLog]
> * gdb.mi/mi-async.exp: Use 'set target-async'.
> * lib/mi-support.exp: Use 'set/show target-async'.
>
> [doc/ChangeLog]
> * gdb.texinfo (Background execution): Adjust example
> (GDB/MI Miscellaneous Commands): Document -list-target-features.
> ---
> gdb/doc/gdb.texinfo | 33 +++++++++++--
> gdb/linux-nat.c | 101
> +++++++-----------------------------
> gdb/mi/mi-cmds.c | 1 +
> gdb/mi/mi-cmds.h | 1 +
> gdb/mi/mi-main.c | 20 +++++++-
> gdb/remote.c | 54 +++-----------------
> gdb/target.c | 39 ++++++++++++++
> gdb/target.h | 4 ++
> gdb/testsuite/gdb.mi/mi-async.exp | 2 +-
> gdb/testsuite/lib/mi-support.exp | 4 +-
> 10 files changed, 124 insertions(+), 135 deletions(-)
>
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 1ef4c8a..3ba9a59 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -4572,12 +4572,9 @@ independently and simultaneously.
> To enter non-stop mode, use this sequence of commands before you run
> or attach to your program:
>
> -@c FIXME: can we fix this recipe to avoid the
> linux-async/remote-async details?
> -
> @smallexample
> # Enable the async interface.
> -# For target remote, use remote-async instead of linux-async.
> -maint set linux-async 1
> +set async 1
>
> # If using the CLI, pagination breaks non-stop.
> set pagination off
> @@ -22637,6 +22634,34 @@ option to the @code{-break-insert} command.
>
> @end itemize
>
> +@subheading The @code{-list-target-features} Command
> +@findex -list-target-features
> +
> +Returns a list of particular features that are supported by the
> +target. Those features affect the permitted MI commands, but
> +unlike the features reported by the @code{-list-features}
> command, the
> +features depend on which target GDB is using at the moment. Whenever
> +a target can change, due to commands such as @code{-target-select},
> +@code{-target-attach} or @code{-exec-run}, the list of
> target features
> +may change, and the frontend should obtain it again.
> +Example output:
> +
> +@smallexample
> +(gdb) -list-features
> +^done,result=["async"]
> +@end smallexample
> +
> +The current list of features is:
> +
> +@itemize @minus
> +@item
> +@samp{async}---indicates that the target is capable of asynchronous
> +command execution, which means that GDB will accept further commands
> +even while the target is running.
> +
> +@end itemize
> +
> +
> @subheading The @code{-interpreter-exec} Command
> @findex -interpreter-exec
>
> diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
> index b59a35c..b25104e 100644
> --- a/gdb/linux-nat.c
> +++ b/gdb/linux-nat.c
> @@ -276,9 +276,6 @@ static int
> linux_supports_tracevforkdone_flag = -1;
>
> /* Async mode support */
>
> -/* True if async mode is currently on. */
> -static int linux_nat_async_enabled;
> -
> /* Zero if the async mode, although enabled, is masked, which means
> linux_nat_wait should behave as if async mode was off. */
> static int linux_nat_async_mask_value = 1;
> @@ -3205,7 +3202,7 @@ linux_nat_pid_to_str (ptid_t ptid)
> static void
> sigchld_handler (int signo)
> {
> - if (linux_nat_async_enabled
> + if (target_async_permitted
> && linux_nat_async_events_state != sigchld_sync
> && signo == SIGCHLD)
> /* It is *always* a bug to hit this. */
> @@ -3992,45 +3989,15 @@ linux_trad_target (CORE_ADDR
> (*register_u_offset)(struct gdbarch *, int, int))
> return t;
> }
>
> -/* Controls if async mode is permitted. */
> -static int linux_async_permitted = 0;
> -
> -/* The set command writes to this variable. If the inferior is
> - 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)
> -{
> - if (target_has_execution)
> - {
> - linux_async_permitted_1 = linux_async_permitted;
> - error (_("Cannot change this setting while the
> inferior is running."));
> - }
> -
> - linux_async_permitted = linux_async_permitted_1;
> - linux_nat_set_async_mode (linux_async_permitted);
> -}
> -
> -static void
> -show_maintenance_linux_async_permitted (struct ui_file
> *file, int from_tty,
> - struct cmd_list_element *c, const
> char *value)
> -{
> - fprintf_filtered (file, _("\
> -Controlling the GNU/Linux inferior in asynchronous mode is %s.\n"),
> - value);
> -}
> -
> /* target_is_async_p implementation. */
>
> static int
> linux_nat_is_async_p (void)
> {
> /* NOTE: palves 2008-03-21: We're only async when the user requests
> - it explicitly with the "maintenance set linux-async" command.
> + it explicitly with the "maintenance set target-async" command.
> Someday, linux will always be async. */
> - if (!linux_async_permitted)
> + if (!target_async_permitted)
> return 0;
>
> return 1;
> @@ -4042,9 +4009,9 @@ static int
> linux_nat_can_async_p (void)
> {
> /* NOTE: palves 2008-03-21: We're only async when the user requests
> - it explicitly with the "maintenance set linux-async" command.
> + it explicitly with the "maintenance set target-async" command.
> Someday, linux will always be async. */
> - if (!linux_async_permitted)
> + if (!target_async_permitted)
> return 0;
>
> /* See target.h/target_async_mask. */
> @@ -4125,7 +4092,7 @@ get_pending_events (void)
> {
> int status, options, pid;
>
> - if (!linux_nat_async_enabled
> + if (!target_async_permitted
> || linux_nat_async_events_state != sigchld_async)
> internal_error (__FILE__, __LINE__,
> "get_pending_events called with async masked");
> @@ -4329,7 +4296,7 @@ static void
> linux_nat_async (void (*callback) (enum inferior_event_type
> event_type,
> void *context), void *context)
> {
> - if (linux_nat_async_mask_value == 0 || !linux_nat_async_enabled)
> + if (linux_nat_async_mask_value == 0 || !target_async_permitted)
> internal_error (__FILE__, __LINE__,
> "Calling target_async when async is masked");
>
> @@ -4353,35 +4320,6 @@ linux_nat_async (void (*callback)
> (enum inferior_event_type event_type,
> return;
> }
>
> -/* Enable/Disable async mode. */
> -
> -static void
> -linux_nat_set_async_mode (int on)
> -{
> - if (linux_nat_async_enabled != on)
> - {
> - if (on)
> - {
> - gdb_assert (waitpid_queue == NULL);
> - if (pipe (linux_nat_event_pipe) == -1)
> - internal_error (__FILE__, __LINE__,
> - "creating event pipe failed.");
> - fcntl (linux_nat_event_pipe[0], F_SETFL, O_NONBLOCK);
> - fcntl (linux_nat_event_pipe[1], F_SETFL, O_NONBLOCK);
> - }
> - else
> - {
> - drain_queued_events (-1);
> - linux_nat_num_queued_events = 0;
> - close (linux_nat_event_pipe[0]);
> - close (linux_nat_event_pipe[1]);
> - linux_nat_event_pipe[0] = linux_nat_event_pipe[1] = -1;
> -
> - }
> - }
> - linux_nat_async_enabled = on;
> -}
> -
> static int
> send_sigint_callback (struct lwp_info *lp, void *data)
> {
> @@ -4478,6 +4416,18 @@ linux_nat_get_siginfo (ptid_t ptid)
> return &lp->siginfo;
> }
>
> +/* Enable/Disable async mode. */
> +
> +static void
> +linux_nat_setup_async (void)
> +{
> + if (pipe (linux_nat_event_pipe) == -1)
> + internal_error (__FILE__, __LINE__,
> + "creating event pipe failed.");
> + fcntl (linux_nat_event_pipe[0], F_SETFL, O_NONBLOCK);
> + fcntl (linux_nat_event_pipe[1], F_SETFL, O_NONBLOCK);
> +}
> +
> void
> _initialize_linux_nat (void)
> {
> @@ -4510,16 +4460,6 @@ Enables printf debugging output."),
> show_debug_linux_nat_async,
> &setdebuglist, &showdebuglist);
>
> - add_setshow_boolean_cmd ("linux-async", class_maintenance,
> - &linux_async_permitted_1, _("\
> -Set whether gdb controls the GNU/Linux inferior in
> asynchronous mode."), _("\
> -Show whether gdb controls the GNU/Linux inferior in
> asynchronous mode."), _("\
> -Tells gdb whether to control the GNU/Linux inferior in
> asynchronous mode."),
> - set_maintenance_linux_async_permitted,
> - show_maintenance_linux_async_permitted,
> - &maintenance_set_cmdlist,
> - &maintenance_show_cmdlist);
> -
> /* Get the default SIGCHLD action. Used while forking an inferior
> (see linux_nat_create_inferior/linux_nat_async_events). */
> sigaction (SIGCHLD, NULL, &sigchld_default_action);
> @@ -4551,8 +4491,7 @@ Tells gdb whether to control the
> GNU/Linux inferior in asynchronous mode."),
> sigemptyset (&async_sigchld_action.sa_mask);
> async_sigchld_action.sa_flags = SA_RESTART;
>
> - /* Install the default mode. */
> - linux_nat_set_async_mode (linux_async_permitted);
> + linux_nat_setup_async ();
>
> add_setshow_boolean_cmd ("disable-randomization", class_support,
> &disable_randomization, _("\
> diff --git a/gdb/mi/mi-cmds.c b/gdb/mi/mi-cmds.c
> index 3ae4c95..ca0f428 100644
> --- a/gdb/mi/mi-cmds.c
> +++ b/gdb/mi/mi-cmds.c
> @@ -89,6 +89,7 @@ struct mi_cmd mi_cmds[] =
> { "inferior-tty-show", { NULL, 0 }, mi_cmd_inferior_tty_show},
> { "interpreter-exec", { NULL, 0 }, mi_cmd_interpreter_exec},
> { "list-features", { NULL, 0 }, mi_cmd_list_features},
> + { "list-target-features", { NULL, 0 },
> mi_cmd_list_target_features},
> { "overlay-auto", { NULL, 0 }, NULL },
> { "overlay-list-mapping-state", { NULL, 0 }, NULL },
> { "overlay-list-overlays", { NULL, 0 }, NULL },
> diff --git a/gdb/mi/mi-cmds.h b/gdb/mi/mi-cmds.h
> index 72e18c7..16887ae 100644
> --- a/gdb/mi/mi-cmds.h
> +++ b/gdb/mi/mi-cmds.h
> @@ -66,6 +66,7 @@ extern mi_cmd_argv_ftype mi_cmd_inferior_tty_set;
> extern mi_cmd_argv_ftype mi_cmd_inferior_tty_show;
> extern mi_cmd_argv_ftype mi_cmd_interpreter_exec;
> extern mi_cmd_argv_ftype mi_cmd_list_features;
> +extern mi_cmd_argv_ftype mi_cmd_list_target_features;
> extern mi_cmd_argv_ftype mi_cmd_stack_info_depth;
> extern mi_cmd_argv_ftype mi_cmd_stack_info_frame;
> extern mi_cmd_argv_ftype mi_cmd_stack_list_args;
> diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
> index 733fc47..b18326d 100644
> --- a/gdb/mi/mi-main.c
> +++ b/gdb/mi/mi-main.c
> @@ -915,7 +915,25 @@ mi_cmd_list_features (char *command,
> char **argv, int argc)
>
> error ("-list-features should be passed no arguments");
> }
> -
> +
> +void
> +mi_cmd_list_target_features (char *command, char **argv, int argc)
> +{
> + if (argc == 0)
> + {
> + struct cleanup *cleanup = NULL;
> + cleanup = make_cleanup_ui_out_list_begin_end (uiout,
> "features");
> +
> + if (target_can_async_p ())
> + ui_out_field_string (uiout, NULL, "async");
> +
> + do_cleanups (cleanup);
> + return;
> + }
> +
> + error ("-list-target-features should be passed no arguments");
> +}
> +
> /* Execute a command within a safe environment.
> Return <0 for error; >=0 for ok.
>
> diff --git a/gdb/remote.c b/gdb/remote.c
> index 2782d05..af3b0b8 100644
> --- a/gdb/remote.c
> +++ b/gdb/remote.c
> @@ -209,33 +209,6 @@ static void
> show_remote_protocol_packet_cmd (struct ui_file *file,
>
> void _initialize_remote (void);
>
> -/* Controls if async mode is permitted. */
> -static int remote_async_permitted = 0;
> -
> -static int remote_async_permitted_set = 0;
> -
> -static void
> -set_maintenance_remote_async_permitted (char *args, int from_tty,
> - struct cmd_list_element *c)
> -{
> - if (target_has_execution)
> - {
> - remote_async_permitted_set = remote_async_permitted;
> /* revert */
> - error (_("Cannot change this setting while the
> inferior is running."));
> - }
> -
> - remote_async_permitted = remote_async_permitted_set;
> -}
> -
> -static void
> -show_maintenance_remote_async_permitted (struct ui_file
> *file, int from_tty,
> - struct
> cmd_list_element *c, const char *value)
> -{
> - fprintf_filtered (file, _("\
> -Controlling the remote inferior in asynchronous mode is %s.\n"),
> - value);
> -}
> -
> /* For "remote". */
>
> static struct cmd_list_element *remote_cmdlist;
> @@ -2703,7 +2676,7 @@ remote_open_1 (char *name, int
> from_tty, struct target_ops *target, int extended
> "(e.g. /dev/ttyS0, /dev/ttya, COM1, etc.)."));
>
> /* See FIXME above. */
> - if (!remote_async_permitted)
> + if (!target_async_permitted)
> wait_forever_enabled_p = 1;
>
> /* If we're connected to a running target, target_preopen
> will kill it.
> @@ -2824,7 +2797,7 @@ remote_open_1 (char *name, int
> from_tty, struct target_ops *target, int extended
> this before anything involving memory or registers. */
> target_find_description ();
>
> - if (remote_async_permitted)
> + if (target_async_permitted)
> {
> /* With this target we start out by owning the terminal. */
> remote_async_terminal_ours_p = 1;
> @@ -2869,13 +2842,13 @@ remote_open_1 (char *name, int
> from_tty, struct target_ops *target, int extended
> if (ex.reason < 0)
> {
> pop_target ();
> - if (remote_async_permitted)
> + if (target_async_permitted)
> wait_forever_enabled_p = 1;
> throw_exception (ex);
> }
> }
>
> - if (remote_async_permitted)
> + if (target_async_permitted)
> wait_forever_enabled_p = 1;
>
> if (extended_p)
> @@ -3403,7 +3376,7 @@ Give up (and stop debugging it)? "))
> static void
> remote_terminal_inferior (void)
> {
> - if (!remote_async_permitted)
> + if (!target_async_permitted)
> /* Nothing to do. */
> return;
>
> @@ -3433,7 +3406,7 @@ remote_terminal_inferior (void)
> static void
> remote_terminal_ours (void)
> {
> - if (!remote_async_permitted)
> + if (!target_async_permitted)
> /* Nothing to do. */
> return;
>
> @@ -7265,7 +7238,7 @@ Specify the serial device it is
> connected to (e.g. /dev/ttya).";
> static int
> remote_can_async_p (void)
> {
> - if (!remote_async_permitted)
> + if (!target_async_permitted)
> /* We only enable async when the user specifically asks
> for it. */
> return 0;
>
> @@ -7276,7 +7249,7 @@ remote_can_async_p (void)
> static int
> remote_is_async_p (void)
> {
> - if (!remote_async_permitted)
> + if (!target_async_permitted)
> /* We only enable async when the user specifically asks
> for it. */
> return 0;
>
> @@ -7630,17 +7603,6 @@ Set the remote pathname for \"run\""), _("\
> Show the remote pathname for \"run\""), NULL, NULL, NULL,
> &remote_set_cmdlist,
> &remote_show_cmdlist);
>
> - add_setshow_boolean_cmd ("remote-async", class_maintenance,
> - &remote_async_permitted_set, _("\
> -Set whether gdb controls the remote inferior in asynchronous
> mode."), _("\
> -Show whether gdb controls the remote inferior in
> asynchronous mode."), _("\
> -Tells gdb whether to control the remote inferior in
> asynchronous mode."),
> - set_maintenance_remote_async_permitted,
> - show_maintenance_remote_async_permitted,
> - &maintenance_set_cmdlist,
> - &maintenance_show_cmdlist);
> -
> -
> /* Eventually initialize fileio. See fileio.c */
> initialize_remote_fileio (remote_set_cmdlist, remote_show_cmdlist);
>
> diff --git a/gdb/target.c b/gdb/target.c
> index 6cda095..0dbe49b 100644
> --- a/gdb/target.c
> +++ b/gdb/target.c
> @@ -3136,6 +3136,35 @@ maintenance_print_target_stack (char
> *cmd, int from_tty)
> }
> }
>
> +/* Controls if async mode is permitted. */
> +int target_async_permitted = 0;
> +
> +/* The set command writes to this variable. If the inferior is
> + executing, linux_nat_async_permitted is *not* updated. */
> +static int target_async_permitted_1 = 0;
> +
> +static void
> +set_maintenance_target_async_permitted (char *args, int from_tty,
> + struct cmd_list_element *c)
> +{
> + if (target_has_execution)
> + {
> + target_async_permitted_1 = target_async_permitted;
> + error (_("Cannot change this setting while the
> inferior is running."));
> + }
> +
> + target_async_permitted = target_async_permitted_1;
> +}
> +
> +static void
> +show_maintenance_target_async_permitted (struct ui_file
> *file, int from_tty,
> + struct cmd_list_element *c,
> + const char *value)
> +{
> + fprintf_filtered (file, _("\
> +Controlling the inferior in asynchronous mode is %s.\n"), value);
> +}
> +
> void
> initialize_targets (void)
> {
> @@ -3173,5 +3202,15 @@ result in significant performance
> improvement for remote targets."),
> _("Print the name of each layer of the internal
> target stack."),
> &maintenanceprintlist);
>
> + add_setshow_boolean_cmd ("target-async", no_class,
> + &target_async_permitted_1, _("\
> +Set whether gdb controls the inferior in asynchronous mode."), _("\
> +Show whether gdb controls the inferior in asynchronous mode."), _("\
> +Tells gdb whether to control the inferior in asynchronous mode."),
> + set_maintenance_target_async_permitted,
> + show_maintenance_target_async_permitted,
> + &setlist,
> + &showlist);
> +
> target_dcache = dcache_init ();
> }
> diff --git a/gdb/target.h b/gdb/target.h
> index e242566..3a1d4b7 100644
> --- a/gdb/target.h
> +++ b/gdb/target.h
> @@ -956,6 +956,10 @@ int target_follow_fork (int follow_child);
> #define target_can_lock_scheduler \
> (current_target.to_has_thread_control & tc_schedlock)
>
> +/* Should the target enable async mode if it is supported? Temporary
> + cludge until async mode is a strict superset of sync mode. */
> +extern int target_async_permitted;
> +
> /* Can the target support asynchronous execution? */
> #define target_can_async_p() (current_target.to_can_async_p ())
>
> diff --git a/gdb/testsuite/gdb.mi/mi-async.exp
> b/gdb/testsuite/gdb.mi/mi-async.exp
> index 9ac91fd..a251c32 100644
> --- a/gdb/testsuite/gdb.mi/mi-async.exp
> +++ b/gdb/testsuite/gdb.mi/mi-async.exp
> @@ -27,7 +27,7 @@ if { !([isnative] && [istarget *-linux*]) \
>
> # The plan is for async mode to become the default but
> toggle for now.
> set saved_gdbflags $GDBFLAGS
> -set GDBFLAGS [concat $GDBFLAGS " -ex \"maint set linux-async on\""]
> +set GDBFLAGS [concat $GDBFLAGS " -ex \"set target-async on\""]
>
> load_lib mi-support.exp
>
> diff --git a/gdb/testsuite/lib/mi-support.exp
> b/gdb/testsuite/lib/mi-support.exp
> index 762e4e2..09d3eeb 100644
> --- a/gdb/testsuite/lib/mi-support.exp
> +++ b/gdb/testsuite/lib/mi-support.exp
> @@ -912,10 +912,10 @@ proc detect_async {} {
> global async
> global mi_gdb_prompt
>
> - send_gdb "maint show linux-async\n"
> + send_gdb "show target-async\n"
>
> gdb_expect {
> - -re ".*Controlling the GNU/Linux inferior in
> asynchronous mode is on...*$mi_gdb_prompt$" {
> + -re ".*Controlling the inferior in asynchronous mode
> is on...*$mi_gdb_prompt$" {
> set async 1
> }
> -re ".*$mi_gdb_prompt$" {
> --
> 1.5.3.5
>
>