Re: [RFA] Unify async setting.

Vladimir Prus
2008-08-19T13:08:30Z

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

Daniel Jacobowitz wrote:

> On Fri, Aug 15, 2008 at 09:07:19PM +0400, Vladimir Prus wrote:
>>
>> 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?
>
> OK with Eli's comments, except check for references to "maintenance";
> it's not a maintenance command any more. And the "set async"
> reference in the doc that Marc noticed. Is there a NEWS entry that
> needs updating or is that still to be written?

I've checked in the below. I'll address NEWS immediately.

- Volodya

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

commit 9f1afb3e2925135090e18f40b7539e1f444589bf
Author: Vladimir Prus <vladimir@codesourcery.com>
Date: Thu Aug 14 16:30:08 2008 +0400

Unify async setting.

* 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.

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 21648b7..100d795 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 target-async 1

# If using the CLI, pagination breaks non-stop.
set pagination off
@@ -4710,7 +4707,7 @@ use @code{interrupt -a}.
@end table

You may need to explicitly enable async mode before you can use background
-execution commands. @xref{Maintenance Commands}, for details. If the
+execution commands, with the @code{set target-async 1} command. If the
target doesn't support async mode, @value{GDBN} issues an error message
if you attempt to use the background execution commands.

@@ -22640,6 +22637,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:
+
+@table @samp
+@item async
+Indicates that the target is capable of asynchronous command
+execution, which means that @value{GDBN} will accept further commands
+while the target is running.
+
+@end table
+
+
@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 30e62f6..332ce42 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 575415b..e469905 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -3133,6 +3133,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)
{
@@ -3170,5 +3199,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 ded91a2..350a7c4 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$" {