* [PATCH v6 0/2] enable target-async by default
@ 2014-05-23 20:59 Pedro Alves
2014-05-23 20:59 ` [PATCH v6 1/2] Make display_gdb_prompt CLI-only Pedro Alves
` (2 more replies)
0 siblings, 3 replies; 42+ messages in thread
From: Pedro Alves @ 2014-05-23 20:59 UTC (permalink / raw)
To: gdb-patches
This is version 6 of the patch series to enable target-async by
default. Most of the series is now in.
There's actually a third patch this depends on, here:
https://sourceware.org/ml/gdb-patches/2014-05/msg00590.html
As I had already posted it today, I figured moving it to the series
and posting it again would possibly confuse things.
Tested on x86_64 Fedora 20, with different permutations of target
async and mi async. No regressions.
Pedro Alves (2):
Make display_gdb_prompt CLI-only.
enable target async by default; separate MI and target notions of
async
gdb/NEWS | 31 ++++++++++
gdb/cli/cli-interp.c | 31 ++++++----
gdb/doc/gdb.texinfo | 54 +++++++++++------
gdb/doc/observer.texi | 8 +++
gdb/event-loop.c | 6 +-
gdb/event-top.c | 5 --
gdb/inf-loop.c | 3 +-
gdb/infcmd.c | 6 +-
gdb/inferior.h | 7 +++
gdb/infrun.c | 3 +-
gdb/interps.c | 30 ++-------
gdb/interps.h | 5 +-
gdb/mi/mi-interp.c | 61 ++++++++++++++-----
gdb/mi/mi-main.c | 77 +++++++++++++++++++++---
gdb/mi/mi-main.h | 4 ++
gdb/target.c | 25 ++++----
gdb/target.h | 3 +-
gdb/testsuite/gdb.base/async-shell.exp | 1 -
gdb/testsuite/gdb.base/async.exp | 2 -
gdb/testsuite/gdb.base/corefile.exp | 17 +-----
gdb/testsuite/gdb.base/dprintf-non-stop.exp | 1 -
gdb/testsuite/gdb.base/gdb-sigterm.exp | 12 +---
gdb/testsuite/gdb.base/inferior-died.exp | 1 -
gdb/testsuite/gdb.base/interrupt-noterm.exp | 1 -
gdb/testsuite/gdb.mi/mi-async.exp | 2 +-
gdb/testsuite/gdb.mi/mi-nonstop-exit.exp | 2 +-
gdb/testsuite/gdb.mi/mi-nonstop.exp | 2 +-
gdb/testsuite/gdb.mi/mi-ns-stale-regcache.exp | 2 +-
gdb/testsuite/gdb.mi/mi-nsintrall.exp | 2 +-
gdb/testsuite/gdb.mi/mi-nsmoribund.exp | 2 +-
gdb/testsuite/gdb.mi/mi-nsthrexec.exp | 2 +-
gdb/testsuite/gdb.mi/mi-watch-nonstop.exp | 2 +-
gdb/testsuite/gdb.multi/watchpoint-multi.exp | 2 +-
gdb/testsuite/gdb.python/py-evsignal.exp | 1 -
gdb/testsuite/gdb.python/py-evthreads.exp | 1 -
gdb/testsuite/gdb.python/py-prompt.exp | 6 +-
gdb/testsuite/gdb.reverse/break-precsave.exp | 6 --
gdb/testsuite/gdb.server/solib-list.exp | 1 -
gdb/testsuite/gdb.threads/thread-specific-bp.exp | 1 -
gdb/testsuite/lib/mi-support.exp | 4 +-
gdb/tui/tui-interp.c | 32 ++++++----
41 files changed, 282 insertions(+), 182 deletions(-)
--
1.9.0
^ permalink raw reply [flat|nested] 42+ messages in thread* [PATCH v6 1/2] Make display_gdb_prompt CLI-only. 2014-05-23 20:59 [PATCH v6 0/2] enable target-async by default Pedro Alves @ 2014-05-23 20:59 ` Pedro Alves 2014-05-23 20:59 ` [PATCH v6 2/2] enable target async by default; separate MI and target notions of async Pedro Alves 2014-05-29 13:44 ` [pushed] Re: [PATCH v6 0/2] enable target-async by default Pedro Alves 2 siblings, 0 replies; 42+ messages in thread From: Pedro Alves @ 2014-05-23 20:59 UTC (permalink / raw) To: gdb-patches Enabling target-async by default will require implementing sync execution on top of an async target, much like foreground command are implemented on the CLI in async mode. In order to do that, we will need better control of when to print the MI prompt. Currently the interp->display_prompt_p hook is all we have, and MI just always returns false, meaning, make display_gdb_prompt a no-op. We'll need to be able to know to print the MI prompt in some of the conditions that display_gdb_prompt is called from the core, but not all. This is all a litte twisted currently. As we can see, display_gdb_prompt is really CLI specific, so make the console interpreters (console/tui) themselves call it. To be able to do that, and add a few different observers that the interpreters can use to distinguish when or why the the prompt is being printed: #1 - one called whenever a command is cancelled due to an error. #2 - another for when a foreground command just finished. In both cases, CLI wants to print the prompt, while MI doesn't. MI will want to print the prompt in the second case when in a special MI mode. The display_gdb_prompt call in interp_set made me pause. The comment there reads: /* Finally, put up the new prompt to show that we are indeed here. Also, display_gdb_prompt for the console does some readline magic which is needed for the console interpreter, at least... */ But, that looks very much like a no-op to me currently: - the MI interpreter always return false in the prompt hook, meaning actually display no prompt. - the interpreter used at that point is still quiet. And the console/tui interpreters return false in the prompt hook if they're quiet, meaning actually display no prompt. The only remaining possible use would then be the readline magic. But whatever that might have been, it's not reacheable today either, because display_gdb_prompt returns early, before touching readline if the interpreter returns false in the display_prompt_p hook. Tested on x86_64 Fedora 20, sync and async modes. gdb/ 2014-05-23 Pedro Alves <palves@redhat.com> * cli/cli-interp.c (cli_interpreter_display_prompt_p): Delete. (_initialize_cli_interp): Adjust. * event-loop.c: Include "observer.h". (start_event_loop): Notify 'command_error' observers instead of calling display_gdb_prompt. Remove FIXME comment. * event-top.c (display_gdb_prompt): Remove call into the interpreters. * inf-loop.c: Include "observer.h". (inferior_event_handler): Notify 'command_error' observers instead of calling display_gdb_prompt. * infrun.c (fetch_inferior_event): Notify 'sync_execution_done' observers instead of calling display_gdb_prompt. * interps.c (interp_set): Don't call display_gdb_prompt. (current_interp_display_prompt_p): Delete. * interps.h (interp_prompt_p): Delete declaration. (interp_prompt_p_ftype): Delete. (struct interp_procs) <prompt_proc_p>: Delete field. (current_interp_display_prompt_p): Delete declaration. * mi-interp.c (mi_interpreter_prompt_p): Delete. (_initialize_mi_interp): Adjust. * tui-interp.c (tui_init): Install 'sync_execution_done' and 'command_error' observers. (tui_on_sync_execution_done, tui_on_command_error): New functions. (tui_display_prompt_p): Delete. (_initialize_tui_interp): Adjust. gdb/doc/ 2014-05-23 Pedro Alves <palves@redhat.com> * observer.texi (synchronous_command_done, command_error): New subjects. --- gdb/cli/cli-interp.c | 31 ++++++++++++++++++++----------- gdb/doc/observer.texi | 8 ++++++++ gdb/event-loop.c | 6 ++---- gdb/event-top.c | 5 ----- gdb/inf-loop.c | 3 ++- gdb/infrun.c | 2 +- gdb/interps.c | 30 ++++-------------------------- gdb/interps.h | 5 +---- gdb/mi/mi-interp.c | 9 --------- gdb/tui/tui-interp.c | 32 ++++++++++++++++++++------------ 10 files changed, 58 insertions(+), 73 deletions(-) diff --git a/gdb/cli/cli-interp.c b/gdb/cli/cli-interp.c index d3badcd..dc09b24 100644 --- a/gdb/cli/cli-interp.c +++ b/gdb/cli/cli-interp.c @@ -87,6 +87,24 @@ cli_on_no_history (void) print_no_history_reason (cli_uiout); } +/* Observer for the sync_execution_done notification. */ + +static void +cli_on_sync_execution_done (void) +{ + if (!interp_quiet_p (cli_interp)) + display_gdb_prompt (NULL); +} + +/* Observer for the command_error notification. */ + +static void +cli_on_command_error (void) +{ + if (!interp_quiet_p (cli_interp)) + display_gdb_prompt (NULL); +} + /* These implement the cli out interpreter: */ static void * @@ -98,6 +116,8 @@ cli_interpreter_init (struct interp *self, int top_level) observer_attach_signal_exited (cli_on_signal_exited); observer_attach_exited (cli_on_exited); observer_attach_no_history (cli_on_no_history); + observer_attach_sync_execution_done (cli_on_sync_execution_done); + observer_attach_command_error (cli_on_command_error); return NULL; } @@ -135,16 +155,6 @@ cli_interpreter_suspend (void *data) return 1; } -/* Don't display the prompt if we are set quiet. */ -static int -cli_interpreter_display_prompt_p (void *data) -{ - if (interp_quiet_p (NULL)) - return 0; - else - return 1; -} - static struct gdb_exception cli_interpreter_exec (void *data, const char *command_str) { @@ -209,7 +219,6 @@ _initialize_cli_interp (void) cli_interpreter_resume, /* resume_proc */ cli_interpreter_suspend, /* suspend_proc */ cli_interpreter_exec, /* exec_proc */ - cli_interpreter_display_prompt_p, /* prompt_proc_p */ cli_ui_out, /* ui_out_proc */ NULL, /* set_logging_proc */ cli_command_loop /* command_loop_proc */ diff --git a/gdb/doc/observer.texi b/gdb/doc/observer.texi index ee43fc5..2757587 100644 --- a/gdb/doc/observer.texi +++ b/gdb/doc/observer.texi @@ -114,6 +114,14 @@ The inferior program is finished. Reverse execution: target ran out of history info. @end deftypefun +@deftypefun void sync_execution_done (void) +A synchronous command finished. +@end deftypefun + +@deftypefun void command_error (void) +An error was caught while executing a command. +@end deftypefun + @deftypefun void target_changed (struct target_ops *@var{target}) The target's register contents have changed. @end deftypefun diff --git a/gdb/event-loop.c b/gdb/event-loop.c index 7939739..5999c97 100644 --- a/gdb/event-loop.c +++ b/gdb/event-loop.c @@ -37,6 +37,7 @@ #include "exceptions.h" #include "gdb_assert.h" #include "gdb_select.h" +#include "observer.h" /* Tell create_file_handler what events we are interested in. This is used by the select version of the event loop. */ @@ -441,10 +442,7 @@ start_event_loop (void) /* If we long-jumped out of do_one_event, we probably didn't get around to resetting the prompt, which leaves readline in a messed-up state. Reset it here. */ - /* FIXME: this should really be a call to a hook that is - interface specific, because interfaces can display the - prompt in their own way. */ - display_gdb_prompt (0); + observer_notify_command_error (); /* This call looks bizarre, but it is required. If the user entered a command that caused an error, after_char_processing_hook won't be called from diff --git a/gdb/event-top.c b/gdb/event-top.c index f690bc6..833f49d 100644 --- a/gdb/event-top.c +++ b/gdb/event-top.c @@ -243,11 +243,6 @@ display_gdb_prompt (char *new_prompt) /* Reset the nesting depth used when trace-commands is set. */ reset_command_nest_depth (); - /* Each interpreter has its own rules on displaying the command - prompt. */ - if (!current_interp_display_prompt_p ()) - return; - old_chain = make_cleanup (free_current_contents, &actual_gdb_prompt); /* Do not call the python hook on an explicit prompt change as diff --git a/gdb/inf-loop.c b/gdb/inf-loop.c index f5293ae..247e9d6 100644 --- a/gdb/inf-loop.c +++ b/gdb/inf-loop.c @@ -31,6 +31,7 @@ #include "continuations.h" #include "interps.h" #include "top.h" +#include "observer.h" static int fetch_inferior_event_wrapper (gdb_client_data client_data); @@ -58,7 +59,7 @@ inferior_event_handler (enum inferior_event_type event_type, do_all_intermediate_continuations (1); do_all_continuations (1); async_enable_stdin (); - display_gdb_prompt (0); + observer_notify_command_error (); } break; diff --git a/gdb/infrun.c b/gdb/infrun.c index 461f6e7..25626d9 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -2938,7 +2938,7 @@ fetch_inferior_event (void *client_data) restore the prompt (a synchronous execution command has finished, and we're ready for input). */ if (interpreter_async && was_sync && !sync_execution) - display_gdb_prompt (0); + observer_notify_sync_execution_done (); if (cmd_done && !was_sync diff --git a/gdb/interps.c b/gdb/interps.c index f6b941c..5a93095 100644 --- a/gdb/interps.c +++ b/gdb/interps.c @@ -204,19 +204,11 @@ interp_set (struct interp *interp, int top_level) return 0; } - /* Finally, put up the new prompt to show that we are indeed here. - Also, display_gdb_prompt for the console does some readline magic - which is needed for the console interpreter, at least... */ - - if (!first_time) + if (!first_time && !interp_quiet_p (interp)) { - if (!interp_quiet_p (interp)) - { - xsnprintf (buffer, sizeof (buffer), - "Switching to interpreter \"%.24s\".\n", interp->name); - ui_out_text (current_uiout, buffer); - } - display_gdb_prompt (NULL); + xsnprintf (buffer, sizeof (buffer), + "Switching to interpreter \"%.24s\".\n", interp->name); + ui_out_text (current_uiout, buffer); } return 1; @@ -304,20 +296,6 @@ current_interp_named_p (const char *interp_name) return 0; } -/* This is called in display_gdb_prompt. If the proc returns a zero - value, display_gdb_prompt will return without displaying the - prompt. */ -int -current_interp_display_prompt_p (void) -{ - if (current_interpreter == NULL - || current_interpreter->procs->prompt_proc_p == NULL) - return 0; - else - return current_interpreter->procs->prompt_proc_p (current_interpreter-> - data); -} - /* The interpreter that is active while `interp_exec' is active, NULL at all other times. */ static struct interp *command_interpreter; diff --git a/gdb/interps.h b/gdb/interps.h index 13edf42..197fa55 100644 --- a/gdb/interps.h +++ b/gdb/interps.h @@ -29,7 +29,6 @@ struct interp; extern int interp_resume (struct interp *interp); extern int interp_suspend (struct interp *interp); -extern int interp_prompt_p (struct interp *interp); extern struct gdb_exception interp_exec (struct interp *interp, const char *command); extern int interp_quiet_p (struct interp *interp); @@ -37,7 +36,6 @@ extern int interp_quiet_p (struct interp *interp); typedef void *(interp_init_ftype) (struct interp *self, int top_level); typedef int (interp_resume_ftype) (void *data); typedef int (interp_suspend_ftype) (void *data); -typedef int (interp_prompt_p_ftype) (void *data); typedef struct gdb_exception (interp_exec_ftype) (void *data, const char *command); typedef void (interp_command_loop_ftype) (void *data); @@ -53,7 +51,6 @@ struct interp_procs interp_resume_ftype *resume_proc; interp_suspend_ftype *suspend_proc; interp_exec_ftype *exec_proc; - interp_prompt_p_ftype *prompt_proc_p; /* Returns the ui_out currently used to collect results for this interpreter. It can be a formatter for stdout, as is the case @@ -79,7 +76,7 @@ extern const char *interp_name (struct interp *interp); extern struct interp *interp_set_temp (const char *name); extern int current_interp_named_p (const char *name); -extern int current_interp_display_prompt_p (void); + extern void current_interp_command_loop (void); /* Call this function to give the current interpreter an opportunity diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c index e8e30e2..e1dd97f 100644 --- a/gdb/mi/mi-interp.c +++ b/gdb/mi/mi-interp.c @@ -223,14 +223,6 @@ mi_interpreter_exec (void *data, const char *command) return exception_none; } -/* Never display the default GDB prompt in MI case. */ - -static int -mi_interpreter_prompt_p (void *data) -{ - return 0; -} - void mi_cmd_interpreter_exec (char *command, char **argv, int argc) { @@ -1142,7 +1134,6 @@ _initialize_mi_interp (void) mi_interpreter_resume, /* resume_proc */ mi_interpreter_suspend, /* suspend_proc */ mi_interpreter_exec, /* exec_proc */ - mi_interpreter_prompt_p, /* prompt_proc_p */ mi_ui_out, /* ui_out_proc */ mi_set_logging, /* set_logging_proc */ mi_command_loop /* command_loop_proc */ diff --git a/gdb/tui/tui-interp.c b/gdb/tui/tui-interp.c index cd11148..147a424 100644 --- a/gdb/tui/tui-interp.c +++ b/gdb/tui/tui-interp.c @@ -104,6 +104,24 @@ tui_on_no_history (void) print_no_history_reason (tui_ui_out (tui_interp)); } +/* Observer for the sync_execution_done notification. */ + +static void +tui_on_sync_execution_done (void) +{ + if (!interp_quiet_p (tui_interp)) + display_gdb_prompt (NULL); +} + +/* Observer for the command_error notification. */ + +static void +tui_on_command_error (void) +{ + if (!interp_quiet_p (tui_interp)) + display_gdb_prompt (NULL); +} + /* These implement the TUI interpreter. */ static void * @@ -127,6 +145,8 @@ tui_init (struct interp *self, int top_level) observer_attach_signal_exited (tui_on_signal_exited); observer_attach_exited (tui_on_exited); observer_attach_no_history (tui_on_no_history); + observer_attach_sync_execution_done (tui_on_sync_execution_done); + observer_attach_command_error (tui_on_command_error); return NULL; } @@ -177,17 +197,6 @@ tui_suspend (void *data) return 1; } -/* Display the prompt if we are silent. */ - -static int -tui_display_prompt_p (void *data) -{ - if (interp_quiet_p (NULL)) - return 0; - else - return 1; -} - static struct ui_out * tui_ui_out (struct interp *self) { @@ -214,7 +223,6 @@ _initialize_tui_interp (void) tui_resume, tui_suspend, tui_exec, - tui_display_prompt_p, tui_ui_out, NULL, cli_command_loop -- 1.9.0 ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v6 2/2] enable target async by default; separate MI and target notions of async 2014-05-23 20:59 [PATCH v6 0/2] enable target-async by default Pedro Alves 2014-05-23 20:59 ` [PATCH v6 1/2] Make display_gdb_prompt CLI-only Pedro Alves @ 2014-05-23 20:59 ` Pedro Alves 2014-05-24 7:03 ` Eli Zaretskii 2014-05-29 13:44 ` [pushed] Re: [PATCH v6 0/2] enable target-async by default Pedro Alves 2 siblings, 1 reply; 42+ messages in thread From: Pedro Alves @ 2014-05-23 20:59 UTC (permalink / raw) To: gdb-patches This finally makes background execution commands possible by default. However, in order to do that, there's one last thing we need to do -- we need to separate the MI and target notions of "async". Unlike the CLI, where the user explicitly requests foreground vs background execution in the execution command itself (c vs c&), MI chose to treat "set target-async" specially -- setting it changes the default behavior of execution commands. So, we can't simply "set target-async" default to on, as that would affect MI frontends. Instead we have to make the setting MI-specific, and teach MI about sync commands on top of an async target. Because the "target" word in "set target-async" ends up as a potential source of confusion, the patch adds a "set mi-async" option, and makes "set target-async" a deprecated alias. Rather than make the targets always async, this patch introduces a new "maint set target-async" option so that the GDB developer can control whether the target is async. This makes it simpler to debug issues arising only in the synchronous mode; important because sync mode seems unlikely to go away. Unlike in previous revisions, "set target-async" does not affect this new maint parameter. The rationale for this is that then one can easily run the test suite in the "maint set target-async off" mode and have tests that enable mi-async fail just like they fail on non-async-capable targets. This emulation is exactly the point of the maint option. I had asked Tom in a previous iteration to split the actual change of the target async default to a separate patch, but it turns out that that is quite awkward in this version of the patch, because with MI async and target async decoupled (unlike in previous versions), if we don't flip the default at the same time, then just "set target-async on" alone never actually manages to do anything. It's best to not have that transitory state in the tree. Given "set target-async on" now only has effect for MI, the patch goes through the testsuite removing it from non-MI tests. MI tests are adjusted to use the new and less confusing "mi-async" spelling. 2014-05-23 Pedro Alves <palves@redhat.com> Tom Tromey <tromey@redhat.com> * NEWS: Mention "maint set target-async", "set mi-async", and that background execution commands are now always available. * target.h (target_async_permitted): Update comment. * target.c (target_async_permitted, target_async_permitted_1): Default to 1. (set_target_async_command): Rename to ... (maint_set_target_async_command): ... this. (show_target_async_command): Rename to ... (maint_show_target_async_command): ... this. (_initialize_target): Adjust. * infcmd.c (prepare_execution_command): Make extern. * inferior.h (prepare_execution_command): Declare. * infrun.c (set_observer_mode): Leave target async alone. * mi/mi-interp.c (mi_interpreter_init): Install mi_on_synchronous_command_done as synchronous_command_done observer. (mi_on_synchronous_command_done): New function. (mi_execute_command_input_handler): Don't print the prompt if we just started a synchronous command with an async target. (mi_on_resume): Check sync_execution before printing prompt. * mi/mi-main.h (mi_async_p): Declare. * mi/mi-main.c: Include gdbcmd.h. (mi_async_p): New function. (mi_async, mi_async_1): New globals. (set_mi_async_command, show_mi_async_command, mi_async): New functions. (exec_continue): Call prepare_execution_command. (run_one_inferior, mi_cmd_exec_run, mi_cmd_list_target_features) (mi_execute_async_cli_command): Use mi_async_p. (_initialize_mi_main): Install "set mi-async". Make "target-async" a deprecated alias. 2014-02-26 Pedro Alves <palves@redhat.com> Tom Tromey <tromey@redhat.com> * gdb.texinfo (Non-Stop Mode): Remove "set target-async 1" from example. (Asynchronous and non-stop modes): Document '-gdb-set mi-async'. Mention that target-async is now deprecated. (Maintenance Commands): Document maint set/show target-async. 2013-10-30 Pedro Alves <palves@redhat.com> Tom Tromey <tromey@redhat.com> * gdb.base/async-shell.exp: Don't enable target-async. * gdb.base/async.exp * gdb.base/corefile.exp (corefile_test_attach): Remove 'async' parameter. Adjust. (top level): Don't test with "target-async". * gdb.base/dprintf-non-stop.exp: Don't enable target-async. * gdb.base/gdb-sigterm.exp: Don't test with "target-async". * gdb.base/inferior-died.exp: Don't enable target-async. * gdb.base/interrupt-noterm.exp: Likewise. * gdb.mi/mi-async.exp: Use "mi-async" instead of "target-async". * gdb.mi/mi-nonstop-exit.exp: Likewise. * gdb.mi/mi-nonstop.exp: Likewise. * gdb.mi/mi-ns-stale-regcache.exp: Likewise. * gdb.mi/mi-nsintrall.exp: Likewise. * gdb.mi/mi-nsmoribund.exp: Likewise. * gdb.mi/mi-nsthrexec.exp: Likewise. * gdb.mi/mi-watch-nonstop.exp: Likewise. * gdb.multi/watchpoint-multi.exp: Adjust comment. * gdb.python/py-evsignal.exp: Don't enable target-async. * gdb.python/py-evthreads.exp: Likewise. * gdb.python/py-prompt.exp: Likewise. * gdb.reverse/break-precsave.exp: Don't test with "target-async". * gdb.server/solib-list.exp: Don't enable target-async. * gdb.threads/thread-specific-bp.exp: Likewise. * lib/mi-support.exp: Adjust to use mi-async. --- gdb/NEWS | 31 ++++++++++ gdb/doc/gdb.texinfo | 54 +++++++++++------ gdb/infcmd.c | 6 +- gdb/inferior.h | 7 +++ gdb/infrun.c | 1 - gdb/mi/mi-interp.c | 52 ++++++++++++++-- gdb/mi/mi-main.c | 77 +++++++++++++++++++++--- gdb/mi/mi-main.h | 4 ++ gdb/target.c | 25 ++++---- gdb/target.h | 3 +- gdb/testsuite/gdb.base/async-shell.exp | 1 - gdb/testsuite/gdb.base/async.exp | 2 - gdb/testsuite/gdb.base/corefile.exp | 17 +----- gdb/testsuite/gdb.base/dprintf-non-stop.exp | 1 - gdb/testsuite/gdb.base/gdb-sigterm.exp | 12 +--- gdb/testsuite/gdb.base/inferior-died.exp | 1 - gdb/testsuite/gdb.base/interrupt-noterm.exp | 1 - gdb/testsuite/gdb.mi/mi-async.exp | 2 +- gdb/testsuite/gdb.mi/mi-nonstop-exit.exp | 2 +- gdb/testsuite/gdb.mi/mi-nonstop.exp | 2 +- gdb/testsuite/gdb.mi/mi-ns-stale-regcache.exp | 2 +- gdb/testsuite/gdb.mi/mi-nsintrall.exp | 2 +- gdb/testsuite/gdb.mi/mi-nsmoribund.exp | 2 +- gdb/testsuite/gdb.mi/mi-nsthrexec.exp | 2 +- gdb/testsuite/gdb.mi/mi-watch-nonstop.exp | 2 +- gdb/testsuite/gdb.multi/watchpoint-multi.exp | 2 +- gdb/testsuite/gdb.python/py-evsignal.exp | 1 - gdb/testsuite/gdb.python/py-evthreads.exp | 1 - gdb/testsuite/gdb.python/py-prompt.exp | 6 +- gdb/testsuite/gdb.reverse/break-precsave.exp | 6 -- gdb/testsuite/gdb.server/solib-list.exp | 1 - gdb/testsuite/gdb.threads/thread-specific-bp.exp | 1 - gdb/testsuite/lib/mi-support.exp | 4 +- 33 files changed, 224 insertions(+), 109 deletions(-) diff --git a/gdb/NEWS b/gdb/NEWS index 4663650..30c8c41 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -67,6 +67,26 @@ set auto-connect-native-target native target for the run, attach, etc. commands when not connected to any target yet. See also "target native" below. +maint set target-async (on|off) +maint show target-async + This controls whether GDB targets operate in syncronous or + asyncronous mode. Normally the default is asyncronous, if it is + available; but this can be changed to more easily debug problems + occurring only in syncronous mode. + +set mi-async (on|off) +show mi-async + Control whether MI asynchronous mode is preferred. This supersedes + "set target-async" of previous GDB versions. + +* "set target-async" is deprecated as a CLI option and is now an alias + for "set mi-async" (only puts MI into async mode). + +* Background execution commands (e.g., "c&", "s&", etc.) are now + possible ``out of the box'' if the target supports them. Previously + the user would need to explicitly enable the possibility with the + "set target-async on" command. + * New features in the GDB remote stub, GDBserver ** New option --debug-format=option1[,option2,...] allows one to add @@ -137,6 +157,17 @@ PowerPC64 GNU/Linux little-endian powerpc64le-*-linux* and "assf"), have been deprecated. Use the "sharedlibrary" command, or its alias "share", instead. +* MI changes + + ** A new option "-gdb-set mi-async" replaces "-gdb-set + target-async". The latter is left as a deprecated alias of the + former for backward compatibility. If the target supports it, + CLI background execution commands are now always possible by + default, independently of whether the frontend stated a + preference for asynchronous execution with "-gdb-set mi-async". + Previously "-gdb-set target-async off" affected both MI execution + commands and CLI execution commands. + *** Changes in GDB 7.7 * Improved support for process record-replay and reverse debugging on diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index 47d4bb7..77deff4 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -5778,9 +5778,6 @@ To enter non-stop mode, use this sequence of commands before you run or attach to your program: @smallexample -# Enable the async interface. -set target-async 1 - # If using the CLI, pagination breaks non-stop. set pagination off @@ -5850,21 +5847,6 @@ the program to report that some thread has stopped before prompting for another command. In background execution, @value{GDBN} immediately gives a command prompt so that you can issue other commands while your program runs. -You need to explicitly enable asynchronous mode before you can use -background execution commands. You can use these commands to -manipulate the asynchronous mode setting: - -@table @code -@kindex set target-async -@item set target-async on -Enable asynchronous mode. -@item set target-async off -Disable asynchronous mode. -@kindex show target-async -@item show target-async -Show the current target-async setting. -@end table - If the target doesn't support async mode, @value{GDBN} issues an error message if you attempt to use the background execution commands. @@ -24794,12 +24776,37 @@ On some targets, @value{GDBN} is capable of processing MI commands even while the target is running. This is called @dfn{asynchronous command execution} (@pxref{Background Execution}). The frontend may specify a preferrence for asynchronous execution using the -@code{-gdb-set target-async 1} command, which should be emitted before +@code{-gdb-set mi-async 1} command, which should be emitted before either running the executable or attaching to the target. After the frontend has started the executable or attached to the target, it can find if asynchronous execution is enabled using the @code{-list-target-features} command. +@table @code +@item -gdb-set mi-async on +@item -gdb-set mi-async off +Set whether MI is in asynchronous mode. + +When @code{off}, which is the default, MI execution commands (e.g., +@code{-exec-continue}) are foreground commands, and @value{GDBN} waits +for the program to stop before processing further commands. + +When @code{on}, MI execution commands are background execution +commands (e.g., @code{-exec-continue} becomes the equivalent of the +@code{c&} CLI command), and so @value{GDBN} is capable of processing +MI commands even while the target is running. + +@item -gdb-show mi-async +Show whether MI asynchronous mode is enabled. +@end table + +Note: In previous @value{GDBN} versions this option was called +@code{target-async} instead of @code{mi-async}, and it had the effect +of both putting MI in asynchronous mode and making CLI background +commands possible. CLI background commands are now always possible +``out of the box'' if the target supports them. The old spelling is +kept as a deprecated alias for backwards compatibility. + Even if @value{GDBN} can accept a command while target is running, many commands that access the target do not work when the target is running. Therefore, asynchronous command execution is most useful @@ -33508,6 +33515,15 @@ Control whether to show all non zero areas within a 1k block starting at thread local base, when using the @samp{info w32 thread-information-block} command. +@kindex maint set target-async +@kindex maint show target-async +@item maint set target-async +@itemx maint show target-async +This controls whether @value{GDBN} targets operate in synchronous or +asynchronous mode (@pxref{Background Execution}). Normally the +default is asynchronous, if it is available; but this can be changed +to more easily debug problems occurring only in synchronous mode. + @kindex maint set per-command @kindex maint show per-command @item maint set per-command diff --git a/gdb/infcmd.c b/gdb/infcmd.c index 6511d64..df4fd40 100644 --- a/gdb/infcmd.c +++ b/gdb/infcmd.c @@ -497,11 +497,9 @@ Start it from the beginning? "))) } } -/* Prepare for execution command. TARGET is the target that will run - the command. BACKGROUND determines whether this is a foreground - (synchronous) or background (asynchronous) command. */ +/* See inferior.h. */ -static void +void prepare_execution_command (struct target_ops *target, int background) { /* If we get a request for running in the bg but the target diff --git a/gdb/inferior.h b/gdb/inferior.h index 14f4ec8..668c888 100644 --- a/gdb/inferior.h +++ b/gdb/inferior.h @@ -164,6 +164,13 @@ extern void notice_new_inferior (ptid_t, int, int); extern struct value *get_return_value (struct value *function, struct type *value_type); +/* Prepare for execution command. TARGET is the target that will run + the command. BACKGROUND determines whether this is a foreground + (synchronous) or background (asynchronous) command. */ + +extern void prepare_execution_command (struct target_ops *target, + int background); + /* Whether to start up the debuggee under a shell. If startup-with-shell is set, GDB's "run" will attempt to start up diff --git a/gdb/infrun.c b/gdb/infrun.c index 25626d9..5172c13 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -235,7 +235,6 @@ set_observer_mode (char *args, int from_tty, going out we leave it that way. */ if (observer_mode) { - target_async_permitted = 1; pagination_enabled = 0; non_stop = non_stop_1 = 1; } diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c index e1dd97f..1ce0f60 100644 --- a/gdb/mi/mi-interp.c +++ b/gdb/mi/mi-interp.c @@ -85,6 +85,7 @@ static void mi_breakpoint_modified (struct breakpoint *b); static void mi_command_param_changed (const char *param, const char *value); static void mi_memory_changed (struct inferior *inf, CORE_ADDR memaddr, ssize_t len, const bfd_byte *myaddr); +static void mi_on_synchronous_command_done (void); static int report_initial_inferior (struct inferior *inf, void *closure); @@ -158,6 +159,7 @@ mi_interpreter_init (struct interp *interp, int top_level) observer_attach_breakpoint_modified (mi_breakpoint_modified); observer_attach_command_param_changed (mi_command_param_changed); observer_attach_memory_changed (mi_memory_changed); + observer_attach_synchronous_command_done (mi_on_synchronous_command_done); /* The initial inferior is created before this function is called, so we need to report it explicitly. Use iteration in @@ -304,6 +306,28 @@ mi_execute_command_wrapper (const char *cmd) mi_execute_command (cmd, stdin == instream); } +/* Observer for the synchronous_command_done notification. */ + +static void +mi_on_synchronous_command_done (void) +{ + /* MI generally prints a prompt after a command, indicating it's + ready for further input. However, due to an historical wart, if + MI async, and a (CLI) synchronous command was issued, then we + will print the prompt right after printing "^running", even if we + cannot actually accept any input until the target stops. See + mi_on_resume. However, if the target is async but MI is sync, + then we need to output the MI prompt now, to replicate gdb's + behavior when neither the target nor MI are async. (Note this + observer is only called by the asynchronous target event handling + code.) */ + if (!mi_async_p ()) + { + fputs_unfiltered ("(gdb) \n", raw_stdout); + gdb_flush (raw_stdout); + } +} + /* mi_execute_command_wrapper wrapper suitable for INPUT_HANDLER. */ static void @@ -311,8 +335,24 @@ mi_execute_command_input_handler (char *cmd) { mi_execute_command_wrapper (cmd); - fputs_unfiltered ("(gdb) \n", raw_stdout); - gdb_flush (raw_stdout); + /* MI generally prints a prompt after a command, indicating it's + ready for further input. However, due to an historical wart, if + MI is async, and a synchronous command was issued, then we will + print the prompt right after printing "^running", even if we + cannot actually accept any input until the target stops. See + mi_on_resume. + + If MI is not async, then we print the prompt when the command + finishes. If the target is sync, that means output the prompt + now, as in that case executing a command doesn't return until the + command is done. However, if the target is async, we go back to + the event loop and output the prompt in the + 'synchronous_command_done' observer. */ + if (!target_is_async_p () || !sync_execution) + { + fputs_unfiltered ("(gdb) \n", raw_stdout); + gdb_flush (raw_stdout); + } } static void @@ -928,10 +968,10 @@ mi_on_resume (ptid_t ptid) running_result_record_printed = 1; /* This is what gdb used to do historically -- printing prompt even if it cannot actually accept any input. This will be surely removed - for MI3, and may be removed even earler. */ - /* FIXME: review the use of target_is_async_p here -- is that - what we want? */ - if (!target_is_async_p ()) + for MI3, and may be removed even earlier. SYNC_EXECUTION is + checked here because we only need to emit a prompt if a + synchronous command was issued when the target is async. */ + if (!target_is_async_p () || sync_execution) fputs_unfiltered ("(gdb) \n", raw_stdout); } gdb_flush (raw_stdout); diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c index 9ab4886..96dfc71 100644 --- a/gdb/mi/mi-main.c +++ b/gdb/mi/mi-main.c @@ -54,6 +54,7 @@ #include "ada-lang.h" #include "linespec.h" #include "extension.h" +#include "gdbcmd.h" #include <ctype.h> #include <sys/time.h> @@ -105,6 +106,45 @@ static int register_changed_p (int regnum, struct regcache *, static void output_register (struct frame_info *, int regnum, int format, int skip_unavailable); +/* Controls whether the frontend wants MI in async mode. */ +static int mi_async = 0; + +/* The set command writes to this variable. If the inferior is + executing, mi_async is *not* updated. */ +static int mi_async_1 = 0; + +static void +set_mi_async_command (char *args, int from_tty, + struct cmd_list_element *c) +{ + if (have_live_inferiors ()) + { + mi_async_1 = mi_async; + error (_("Cannot change this setting while the inferior is running.")); + } + + mi_async = mi_async_1; +} + +static void +show_mi_async_command (struct ui_file *file, int from_tty, + struct cmd_list_element *c, + const char *value) +{ + fprintf_filtered (file, + _("Whether MI is in asynchronous mode is %s.\n"), + value); +} + +/* A wrapper for target_can_async_p that takes the MI setting into + account. */ + +int +mi_async_p (void) +{ + return mi_async && target_can_async_p (); +} + /* Command implementations. FIXME: Is this libgdb? No. This is the MI layer that calls libgdb. Any operation used in the below should be formalized. */ @@ -229,6 +269,8 @@ proceed_thread_callback (struct thread_info *thread, void *arg) static void exec_continue (char **argv, int argc) { + prepare_execution_command (¤t_target, mi_async_p ()); + if (non_stop) { /* In non-stop mode, 'resume' always resumes a single thread. @@ -395,8 +437,8 @@ run_one_inferior (struct inferior *inf, void *arg) switch_to_thread (null_ptid); set_current_program_space (inf->pspace); } - mi_execute_cli_command (run_cmd, target_can_async_p (), - target_can_async_p () ? "&" : NULL); + mi_execute_cli_command (run_cmd, mi_async_p (), + mi_async_p () ? "&" : NULL); return 0; } @@ -450,8 +492,8 @@ mi_cmd_exec_run (char *command, char **argv, int argc) { const char *run_cmd = start_p ? "start" : "run"; - mi_execute_cli_command (run_cmd, target_can_async_p (), - target_can_async_p () ? "&" : NULL); + mi_execute_cli_command (run_cmd, mi_async_p (), + mi_async_p () ? "&" : NULL); } } @@ -1838,11 +1880,10 @@ mi_cmd_list_target_features (char *command, char **argv, int argc) struct ui_out *uiout = current_uiout; cleanup = make_cleanup_ui_out_list_begin_end (uiout, "features"); - if (target_can_async_p ()) + if (mi_async_p ()) ui_out_field_string (uiout, NULL, "async"); if (target_can_execute_reverse) ui_out_field_string (uiout, NULL, "reverse"); - do_cleanups (cleanup); return; } @@ -2269,7 +2310,7 @@ mi_execute_async_cli_command (char *cli_command, char **argv, int argc) struct cleanup *old_cleanups; char *run; - if (target_can_async_p ()) + if (mi_async_p ()) run = xstrprintf ("%s %s&", cli_command, argc ? *argv : ""); else run = xstrprintf ("%s %s", cli_command, argc ? *argv : ""); @@ -2920,3 +2961,25 @@ mi_cmd_trace_frame_collected (char *command, char **argv, int argc) do_cleanups (old_chain); } + +void +_initialize_mi_main (void) +{ + struct cmd_list_element *c; + + add_setshow_boolean_cmd ("mi-async", class_run, + &mi_async_1, _("\ +Set whether MI asynchronous mode is enabled."), _("\ +Show whether MI asynchronous mode is enabled."), _("\ +Tells GDB whether MI should be in asynchronous mode."), + set_mi_async_command, + show_mi_async_command, + &setlist, + &showlist); + + /* Alias old "target-async" to "mi-async". */ + c = add_alias_cmd ("target-async", "mi-async", class_run, 0, &setlist); + deprecate_cmd (c, "set mi-async"); + c = add_alias_cmd ("target-async", "mi-async", class_run, 0, &showlist); + deprecate_cmd (c, "show mi-async"); +} diff --git a/gdb/mi/mi-main.h b/gdb/mi/mi-main.h index c32845d..530aeb7 100644 --- a/gdb/mi/mi-main.h +++ b/gdb/mi/mi-main.h @@ -28,6 +28,10 @@ extern void mi_load_progress (const char *section_name, extern void mi_print_timing_maybe (void); +/* Whether MI is in async mode. */ + +extern int mi_async_p (void); + extern char *current_token; extern int running_result_record_printed; diff --git a/gdb/target.c b/gdb/target.c index cf86e73..2fe3269 100644 --- a/gdb/target.c +++ b/gdb/target.c @@ -4104,16 +4104,17 @@ maintenance_print_target_stack (char *cmd, int from_tty) } } -/* Controls if async mode is permitted. */ -int target_async_permitted = 0; +/* Controls if targets can report that they can/are async. This is + just for maintainers to use when debugging gdb. */ +int target_async_permitted = 1; /* The set command writes to this variable. If the inferior is executing, target_async_permitted is *not* updated. */ -static int target_async_permitted_1 = 0; +static int target_async_permitted_1 = 1; static void -set_target_async_command (char *args, int from_tty, - struct cmd_list_element *c) +maint_set_target_async_command (char *args, int from_tty, + struct cmd_list_element *c) { if (have_live_inferiors ()) { @@ -4125,9 +4126,9 @@ set_target_async_command (char *args, int from_tty, } static void -show_target_async_command (struct ui_file *file, int from_tty, - struct cmd_list_element *c, - const char *value) +maint_show_target_async_command (struct ui_file *file, int from_tty, + struct cmd_list_element *c, + const char *value) { fprintf_filtered (file, _("Controlling the inferior in " @@ -4232,10 +4233,10 @@ result in significant performance improvement for remote targets."), 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_target_async_command, - show_target_async_command, - &setlist, - &showlist); + maint_set_target_async_command, + maint_show_target_async_command, + &maintenance_set_cmdlist, + &maintenance_show_cmdlist); add_setshow_boolean_cmd ("may-write-registers", class_support, &may_write_registers_1, _("\ diff --git a/gdb/target.h b/gdb/target.h index 9371529..face210 100644 --- a/gdb/target.h +++ b/gdb/target.h @@ -1619,8 +1619,7 @@ extern int default_child_has_execution (struct target_ops *ops, #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. */ +/* Controls whether async mode is permitted. */ extern int target_async_permitted; /* Can the target support asynchronous execution? */ diff --git a/gdb/testsuite/gdb.base/async-shell.exp b/gdb/testsuite/gdb.base/async-shell.exp index 4890a59..f0550bc 100644 --- a/gdb/testsuite/gdb.base/async-shell.exp +++ b/gdb/testsuite/gdb.base/async-shell.exp @@ -31,7 +31,6 @@ if { [prepare_for_testing ${testfile}.exp ${testfile} $srcfile] } { set gdbindex_warning_re "warning: Skipping \[^\r\n\]+ \\.gdb_index section \[^\r\n\]*\r\nDo \"set use-deprecated-index-sections on\" before the file is read\r\nto use the section anyway\\." -gdb_test_no_output "set target-async on " gdb_test_no_output "set non-stop on" gdb_test "run &" "Starting program: \[^\r\n\]*(\r\n$gdbindex_warning_re)?" diff --git a/gdb/testsuite/gdb.base/async.exp b/gdb/testsuite/gdb.base/async.exp index f0a18e8..0f99b01 100644 --- a/gdb/testsuite/gdb.base/async.exp +++ b/gdb/testsuite/gdb.base/async.exp @@ -25,8 +25,6 @@ if {[prepare_for_testing $testfile.exp $testfile $srcfile debug]} { return -1 } -gdb_test_no_output "set target-async on" - # # set it up at a breakpoint so we can play with it # diff --git a/gdb/testsuite/gdb.base/corefile.exp b/gdb/testsuite/gdb.base/corefile.exp index 6eb1f02..bde2de8 100644 --- a/gdb/testsuite/gdb.base/corefile.exp +++ b/gdb/testsuite/gdb.base/corefile.exp @@ -241,7 +241,7 @@ gdb_exit # Test an attach command will clear any loaded core file. -proc corefile_test_attach {{async 0}} { +proc corefile_test_attach {} { global binfile corefile gdb_prompt if ![is_remote target] { @@ -257,11 +257,6 @@ proc corefile_test_attach {{async 0}} { gdb_start - if {$async} { - gdb_test_no_output "set target-async on" \ - "enable target-async for attach tests" - } - gdb_test "core-file $corefile" "Core was generated by .*" "attach: load core again" gdb_test "info files" "\r\nLocal core dump file:\r\n.*" "attach: sanity check we see the core file" @@ -298,13 +293,3 @@ gdb_test_multiple "core-file $corefile" $test { pass $test } } - - -# Try a couple tests again with target-async. -with_test_prefix "target-async" { - clean_restart ${testfile} - - gdb_test_no_output "set target-async on" - corefile_test_run - corefile_test_attach 1 -} diff --git a/gdb/testsuite/gdb.base/dprintf-non-stop.exp b/gdb/testsuite/gdb.base/dprintf-non-stop.exp index fdaa5c1..df1e270 100644 --- a/gdb/testsuite/gdb.base/dprintf-non-stop.exp +++ b/gdb/testsuite/gdb.base/dprintf-non-stop.exp @@ -26,7 +26,6 @@ if [prepare_for_testing "failed to prepare for dprintf with non-stop" \ return -1 } -gdb_test_no_output "set target-async on" gdb_test_no_output "set non-stop on" if ![runto main] { diff --git a/gdb/testsuite/gdb.base/gdb-sigterm.exp b/gdb/testsuite/gdb.base/gdb-sigterm.exp index f52517c..df0de42 100644 --- a/gdb/testsuite/gdb.base/gdb-sigterm.exp +++ b/gdb/testsuite/gdb.base/gdb-sigterm.exp @@ -79,17 +79,7 @@ proc do_test { pass } { # 50 runs should be approx. a safe number to be sure it is fixed now. for {set pass 0} {$pass < 50} {incr pass} { - clean_restart ${testfile} - gdb_test_no_output "set target-async off" "" - with_test_prefix "sync" { - do_test $pass - } - - clean_restart ${testfile} - gdb_test_no_output "set target-async on" "" - with_test_prefix "async" { - do_test $pass - } + do_test $pass } pass "$pass SIGTERM passes" diff --git a/gdb/testsuite/gdb.base/inferior-died.exp b/gdb/testsuite/gdb.base/inferior-died.exp index 33f92e9..152bdd8 100644 --- a/gdb/testsuite/gdb.base/inferior-died.exp +++ b/gdb/testsuite/gdb.base/inferior-died.exp @@ -38,7 +38,6 @@ if { [prepare_for_testing ${testfile}.exp ${testfile} ${testfile}.c] } { } gdb_test_no_output "set detach-on-fork off" -gdb_test_no_output "set target-async on" gdb_test_no_output "set non-stop on" if ![runto_main] { diff --git a/gdb/testsuite/gdb.base/interrupt-noterm.exp b/gdb/testsuite/gdb.base/interrupt-noterm.exp index a22acd2..5c92b97 100644 --- a/gdb/testsuite/gdb.base/interrupt-noterm.exp +++ b/gdb/testsuite/gdb.base/interrupt-noterm.exp @@ -22,7 +22,6 @@ if [prepare_for_testing "failed to prepare for testing" \ # Pretend there's no terminal. gdb_test_no_output "set interactive-mode off" -gdb_test_no_output "set target-async on" if ![runto main] { fail "Can't run to main" diff --git a/gdb/testsuite/gdb.mi/mi-async.exp b/gdb/testsuite/gdb.mi/mi-async.exp index e41701d..0df4c98 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*]) } then { # The plan is for async mode to become the default but toggle for now. set saved_gdbflags $GDBFLAGS -set GDBFLAGS [concat $GDBFLAGS " -ex \"set target-async on\""] +set GDBFLAGS [concat $GDBFLAGS " -ex \"set mi-async on\""] load_lib mi-support.exp diff --git a/gdb/testsuite/gdb.mi/mi-nonstop-exit.exp b/gdb/testsuite/gdb.mi/mi-nonstop-exit.exp index 3727d81..5187b40 100644 --- a/gdb/testsuite/gdb.mi/mi-nonstop-exit.exp +++ b/gdb/testsuite/gdb.mi/mi-nonstop-exit.exp @@ -40,7 +40,7 @@ mi_gdb_reinitialize_dir $srcdir/$subdir mi_gdb_load $binfile mi_gdb_test "-gdb-set non-stop 1" ".*" -mi_gdb_test "-gdb-set target-async 1" ".*" +mi_gdb_test "-gdb-set mi-async 1" ".*" mi_detect_async if { [mi_run_to_main] < 0 } { diff --git a/gdb/testsuite/gdb.mi/mi-nonstop.exp b/gdb/testsuite/gdb.mi/mi-nonstop.exp index 5ef74ed..ba7dfd4 100644 --- a/gdb/testsuite/gdb.mi/mi-nonstop.exp +++ b/gdb/testsuite/gdb.mi/mi-nonstop.exp @@ -50,7 +50,7 @@ mi_gdb_reinitialize_dir $srcdir/$subdir mi_gdb_load $binfile mi_gdb_test "-gdb-set non-stop 1" ".*" -mi_gdb_test "-gdb-set target-async 1" ".*" +mi_gdb_test "-gdb-set mi-async 1" ".*" mi_detect_async if { [mi_run_to_main] < 0 } { diff --git a/gdb/testsuite/gdb.mi/mi-ns-stale-regcache.exp b/gdb/testsuite/gdb.mi/mi-ns-stale-regcache.exp index 754689c..ae9e5f2 100644 --- a/gdb/testsuite/gdb.mi/mi-ns-stale-regcache.exp +++ b/gdb/testsuite/gdb.mi/mi-ns-stale-regcache.exp @@ -53,7 +53,7 @@ mi_gdb_reinitialize_dir $srcdir/$subdir mi_gdb_load $binfile mi_gdb_test "-gdb-set non-stop 1" ".*" -mi_gdb_test "-gdb-set target-async 1" ".*" +mi_gdb_test "-gdb-set mi-async 1" ".*" mi_detect_async if { [mi_run_to_main] < 0 } { diff --git a/gdb/testsuite/gdb.mi/mi-nsintrall.exp b/gdb/testsuite/gdb.mi/mi-nsintrall.exp index f613e7f..ac1209e 100644 --- a/gdb/testsuite/gdb.mi/mi-nsintrall.exp +++ b/gdb/testsuite/gdb.mi/mi-nsintrall.exp @@ -40,7 +40,7 @@ mi_gdb_reinitialize_dir $srcdir/$subdir mi_gdb_load $binfile mi_gdb_test "-gdb-set non-stop 1" ".*" -mi_gdb_test "-gdb-set target-async 1" ".*" +mi_gdb_test "-gdb-set mi-async 1" ".*" mi_detect_async if { [mi_run_to_main] < 0 } { diff --git a/gdb/testsuite/gdb.mi/mi-nsmoribund.exp b/gdb/testsuite/gdb.mi/mi-nsmoribund.exp index 350c060..0ff04ca 100644 --- a/gdb/testsuite/gdb.mi/mi-nsmoribund.exp +++ b/gdb/testsuite/gdb.mi/mi-nsmoribund.exp @@ -40,7 +40,7 @@ mi_gdb_reinitialize_dir $srcdir/$subdir mi_gdb_load $binfile mi_gdb_test "-gdb-set non-stop 1" ".*" -mi_gdb_test "-gdb-set target-async 1" ".*" +mi_gdb_test "-gdb-set mi-async 1" ".*" mi_detect_async if { [mi_run_to_main] < 0 } { diff --git a/gdb/testsuite/gdb.mi/mi-nsthrexec.exp b/gdb/testsuite/gdb.mi/mi-nsthrexec.exp index 85617d0..008a831 100644 --- a/gdb/testsuite/gdb.mi/mi-nsthrexec.exp +++ b/gdb/testsuite/gdb.mi/mi-nsthrexec.exp @@ -50,7 +50,7 @@ mi_gdb_reinitialize_dir $srcdir/$subdir mi_gdb_load $binfile mi_gdb_test "-gdb-set non-stop 1" ".*" -mi_gdb_test "-gdb-set target-async 1" ".*" +mi_gdb_test "-gdb-set mi-async 1" ".*" mi_detect_async if { [mi_run_to_main] < 0 } { diff --git a/gdb/testsuite/gdb.mi/mi-watch-nonstop.exp b/gdb/testsuite/gdb.mi/mi-watch-nonstop.exp index 3dbf4c7..44371c8 100644 --- a/gdb/testsuite/gdb.mi/mi-watch-nonstop.exp +++ b/gdb/testsuite/gdb.mi/mi-watch-nonstop.exp @@ -52,7 +52,7 @@ mi_gdb_reinitialize_dir $srcdir/$subdir mi_gdb_load $binfile mi_gdb_test "-gdb-set non-stop 1" ".*" -mi_gdb_test "-gdb-set target-async 1" ".*" +mi_gdb_test "-gdb-set mi-async 1" ".*" mi_detect_async if { [mi_run_to_main] < 0 } { diff --git a/gdb/testsuite/gdb.multi/watchpoint-multi.exp b/gdb/testsuite/gdb.multi/watchpoint-multi.exp index b7fb0a9..e6a8957 100644 --- a/gdb/testsuite/gdb.multi/watchpoint-multi.exp +++ b/gdb/testsuite/gdb.multi/watchpoint-multi.exp @@ -37,7 +37,7 @@ if { [gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" executa clean_restart $executable -# Simulate non-stop+target-async which also uses breakpoint always-inserted. +# Simulate non-stop which also uses breakpoint always-inserted. gdb_test_no_output "set breakpoint always-inserted on" # displaced-stepping is also needed as other GDB sometimes still removes the # breakpoints, even with always-inserted on. diff --git a/gdb/testsuite/gdb.python/py-evsignal.exp b/gdb/testsuite/gdb.python/py-evsignal.exp index af3d53c..530fd07 100644 --- a/gdb/testsuite/gdb.python/py-evsignal.exp +++ b/gdb/testsuite/gdb.python/py-evsignal.exp @@ -35,7 +35,6 @@ gdb_test_no_output "python exec (open ('${pyfile}').read ())" "" gdb_test "test-events" "Event testers registered." gdb_test_no_output "set non-stop on" -gdb_test_no_output "set target-async on" gdb_run_cmd gdb_test_multiple "" "Signal Thread 3" { diff --git a/gdb/testsuite/gdb.python/py-evthreads.exp b/gdb/testsuite/gdb.python/py-evthreads.exp index ffa322f..d62624e 100644 --- a/gdb/testsuite/gdb.python/py-evthreads.exp +++ b/gdb/testsuite/gdb.python/py-evthreads.exp @@ -40,7 +40,6 @@ gdb_test_no_output "python exec (open ('${pyfile}').read ())" "" gdb_test "test-events" "Event testers registered." gdb_test_no_output "set non-stop on" -gdb_test_no_output "set target-async on" gdb_breakpoint "main" gdb_breakpoint "thread2" diff --git a/gdb/testsuite/gdb.python/py-prompt.exp b/gdb/testsuite/gdb.python/py-prompt.exp index e376c05..ebe4cb6 100644 --- a/gdb/testsuite/gdb.python/py-prompt.exp +++ b/gdb/testsuite/gdb.python/py-prompt.exp @@ -90,8 +90,7 @@ if { [istarget "*-*-cygwin*"] } { set testpid [ exec ps -e | gawk "{ if (\$1 == $testpid) print \$4; }" ] } -set GDBFLAGS [concat $tmp_gdbflags " -ex \"set target-async on\""] -set GDBFLAGS [concat $GDBFLAGS " -ex \"set pagination off\""] +set GDBFLAGS [concat $tmp_gdbflags " -ex \"set pagination off\""] set GDBFLAGS [concat $GDBFLAGS " -ex \"set editing on\""] set GDBFLAGS [concat $GDBFLAGS " -ex \"attach $testpid\""] set GDBFLAGS [concat $GDBFLAGS " -ex \"continue&\""] @@ -107,8 +106,7 @@ gdb_test "python print (\"'\" + str(p\[0\]) + \"'\")" "'$gdb_prompt_fail '" \ "prompt_hook argument is default prompt. 3" gdb_exit -set GDBFLAGS [concat $tmp_gdbflags " -ex \"set target-async on\""] -set GDBFLAGS [concat $GDBFLAGS " -ex \"set pagination off\""] +set GDBFLAGS [concat $tmp_gdbflags " -ex \"set pagination off\""] set GDBFLAGS [concat $GDBFLAGS " -ex \"set editing on\""] set GDBFLAGS [concat $GDBFLAGS " -ex \"attach $testpid\""] set GDBFLAGS [concat $GDBFLAGS " -ex \"interrupt\""] diff --git a/gdb/testsuite/gdb.reverse/break-precsave.exp b/gdb/testsuite/gdb.reverse/break-precsave.exp index f1e6c69..3206e3c 100644 --- a/gdb/testsuite/gdb.reverse/break-precsave.exp +++ b/gdb/testsuite/gdb.reverse/break-precsave.exp @@ -110,9 +110,3 @@ proc precsave_tests {} { } precsave_tests - -with_test_prefix "target-async" { - clean_restart $testfile - gdb_test_no_output "set target-async on" - precsave_tests -} diff --git a/gdb/testsuite/gdb.server/solib-list.exp b/gdb/testsuite/gdb.server/solib-list.exp index 8cc3edc..4504a23 100644 --- a/gdb/testsuite/gdb.server/solib-list.exp +++ b/gdb/testsuite/gdb.server/solib-list.exp @@ -66,7 +66,6 @@ foreach nonstop { 0 1 } { with_test_prefix "non-stop $nonstop" { gdb_test "disconnect" ".*" gdb_test "set non-stop $nonstop" - gdb_test "set target-async $nonstop" # It is required for the non-stop mode, GDB would try to step over # _dl_debug_state breakpoint will still only ld.so loaded in gdbserver. diff --git a/gdb/testsuite/gdb.threads/thread-specific-bp.exp b/gdb/testsuite/gdb.threads/thread-specific-bp.exp index b8416d7..4273e1d 100644 --- a/gdb/testsuite/gdb.threads/thread-specific-bp.exp +++ b/gdb/testsuite/gdb.threads/thread-specific-bp.exp @@ -122,6 +122,5 @@ check_thread_specific_breakpoint "all-stop" clean_restart ${binfile} # Test non-stop mode. -gdb_test_no_output "set target-async on" "set async mode" gdb_test_no_output "set non-stop on" "set non-stop mode" check_thread_specific_breakpoint "non-stop" diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp index 09a514b..e5d2de3 100644 --- a/gdb/testsuite/lib/mi-support.exp +++ b/gdb/testsuite/lib/mi-support.exp @@ -1002,10 +1002,10 @@ proc mi_detect_async {} { global async global mi_gdb_prompt - send_gdb "show target-async\n" + send_gdb "show mi-async\n" gdb_expect { - -re ".*Controlling the inferior in asynchronous mode is on...*$mi_gdb_prompt$" { + -re "asynchronous mode is on...*$mi_gdb_prompt$" { set async 1 } -re ".*$mi_gdb_prompt$" { -- 1.9.0 ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v6 2/2] enable target async by default; separate MI and target notions of async 2014-05-23 20:59 ` [PATCH v6 2/2] enable target async by default; separate MI and target notions of async Pedro Alves @ 2014-05-24 7:03 ` Eli Zaretskii 2014-05-29 13:49 ` Pedro Alves 0 siblings, 1 reply; 42+ messages in thread From: Eli Zaretskii @ 2014-05-24 7:03 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches > From: Pedro Alves <palves@redhat.com> > Date: Fri, 23 May 2014 21:59:13 +0100 > > This finally makes background execution commands possible by default. Thanks. > +Note: In previous @value{GDBN} versions this option was called It is best to state here the last GDB version where that was true, as in In @value{GDBN} version X.YZ and earlier, this option was called ... That's because "previous versions" is a relative reference, which will be inaccurate in the version after the one where this text will be included. The documentation parts are OK with this fixed. ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v6 2/2] enable target async by default; separate MI and target notions of async 2014-05-24 7:03 ` Eli Zaretskii @ 2014-05-29 13:49 ` Pedro Alves 2014-07-30 18:40 ` Doug Evans 0 siblings, 1 reply; 42+ messages in thread From: Pedro Alves @ 2014-05-29 13:49 UTC (permalink / raw) To: Eli Zaretskii; +Cc: gdb-patches On 05/24/2014 08:03 AM, Eli Zaretskii wrote: >> From: Pedro Alves <palves@redhat.com> >> Date: Fri, 23 May 2014 21:59:13 +0100 >> >> This finally makes background execution commands possible by default. > > Thanks. > >> +Note: In previous @value{GDBN} versions this option was called > > It is best to state here the last GDB version where that was true, as > in > > In @value{GDBN} version X.YZ and earlier, this option was called ... > > That's because "previous versions" is a relative reference, which will > be inaccurate in the version after the one where this text will be > included. Indeed, wholeheartedly agreed. I'll often complain about the same. It goes to show how easy it is to fall into the trap. > > The documentation parts are OK with this fixed. Thanks Eli. I did that exactly: +Note: In @value{GDBN} version 7.7 and earlier, this option was called +@code{target-async} instead of @code{mi-async}, and it had the effect +of both putting MI in asynchronous mode and making CLI background Below's what I pushed. 8<------------ Subject: [PATCH] enable target async by default; separate MI and target notions of async This finally makes background execution commands possible by default. However, in order to do that, there's one last thing we need to do -- we need to separate the MI and target notions of "async". Unlike the CLI, where the user explicitly requests foreground vs background execution in the execution command itself (c vs c&), MI chose to treat "set target-async" specially -- setting it changes the default behavior of execution commands. So, we can't simply "set target-async" default to on, as that would affect MI frontends. Instead we have to make the setting MI-specific, and teach MI about sync commands on top of an async target. Because the "target" word in "set target-async" ends up as a potential source of confusion, the patch adds a "set mi-async" option, and makes "set target-async" a deprecated alias. Rather than make the targets always async, this patch introduces a new "maint set target-async" option so that the GDB developer can control whether the target is async. This makes it simpler to debug issues arising only in the synchronous mode; important because sync mode seems unlikely to go away. Unlike in previous revisions, "set target-async" does not affect this new maint parameter. The rationale for this is that then one can easily run the test suite in the "maint set target-async off" mode and have tests that enable mi-async fail just like they fail on non-async-capable targets. This emulation is exactly the point of the maint option. I had asked Tom in a previous iteration to split the actual change of the target async default to a separate patch, but it turns out that that is quite awkward in this version of the patch, because with MI async and target async decoupled (unlike in previous versions), if we don't flip the default at the same time, then just "set target-async on" alone never actually manages to do anything. It's best to not have that transitory state in the tree. Given "set target-async on" now only has effect for MI, the patch goes through the testsuite removing it from non-MI tests. MI tests are adjusted to use the new and less confusing "mi-async" spelling. 2014-05-29 Pedro Alves <palves@redhat.com> Tom Tromey <tromey@redhat.com> * NEWS: Mention "maint set target-async", "set mi-async", and that background execution commands are now always available. * target.h (target_async_permitted): Update comment. * target.c (target_async_permitted, target_async_permitted_1): Default to 1. (set_target_async_command): Rename to ... (maint_set_target_async_command): ... this. (show_target_async_command): Rename to ... (maint_show_target_async_command): ... this. (_initialize_target): Adjust. * infcmd.c (prepare_execution_command): Make extern. * inferior.h (prepare_execution_command): Declare. * infrun.c (set_observer_mode): Leave target async alone. * mi/mi-interp.c (mi_interpreter_init): Install mi_on_sync_execution_done as sync_execution_done observer. (mi_on_sync_execution_done): New function. (mi_execute_command_input_handler): Don't print the prompt if we just started a synchronous command with an async target. (mi_on_resume): Check sync_execution before printing prompt. * mi/mi-main.h (mi_async_p): Declare. * mi/mi-main.c: Include gdbcmd.h. (mi_async_p): New function. (mi_async, mi_async_1): New globals. (set_mi_async_command, show_mi_async_command, mi_async): New functions. (exec_continue): Call prepare_execution_command. (run_one_inferior, mi_cmd_exec_run, mi_cmd_list_target_features) (mi_execute_async_cli_command): Use mi_async_p. (_initialize_mi_main): Install "set mi-async". Make "target-async" a deprecated alias. 2014-05-29 Pedro Alves <palves@redhat.com> Tom Tromey <tromey@redhat.com> * gdb.texinfo (Non-Stop Mode): Remove "set target-async 1" from example. (Asynchronous and non-stop modes): Document '-gdb-set mi-async'. Mention that target-async is now deprecated. (Maintenance Commands): Document maint set/show target-async. 2014-05-29 Pedro Alves <palves@redhat.com> Tom Tromey <tromey@redhat.com> * gdb.base/async-shell.exp: Don't enable target-async. * gdb.base/async.exp * gdb.base/corefile.exp (corefile_test_attach): Remove 'async' parameter. Adjust. (top level): Don't test with "target-async". * gdb.base/dprintf-non-stop.exp: Don't enable target-async. * gdb.base/gdb-sigterm.exp: Don't test with "target-async". * gdb.base/inferior-died.exp: Don't enable target-async. * gdb.base/interrupt-noterm.exp: Likewise. * gdb.mi/mi-async.exp: Use "mi-async" instead of "target-async". * gdb.mi/mi-nonstop-exit.exp: Likewise. * gdb.mi/mi-nonstop.exp: Likewise. * gdb.mi/mi-ns-stale-regcache.exp: Likewise. * gdb.mi/mi-nsintrall.exp: Likewise. * gdb.mi/mi-nsmoribund.exp: Likewise. * gdb.mi/mi-nsthrexec.exp: Likewise. * gdb.mi/mi-watch-nonstop.exp: Likewise. * gdb.multi/watchpoint-multi.exp: Adjust comment. * gdb.python/py-evsignal.exp: Don't enable target-async. * gdb.python/py-evthreads.exp: Likewise. * gdb.python/py-prompt.exp: Likewise. * gdb.reverse/break-precsave.exp: Don't test with "target-async". * gdb.server/solib-list.exp: Don't enable target-async. * gdb.threads/thread-specific-bp.exp: Likewise. * lib/mi-support.exp: Adjust to use mi-async. --- gdb/ChangeLog | 34 +++++++++++ gdb/doc/ChangeLog | 9 +++ gdb/testsuite/ChangeLog | 29 +++++++++ gdb/NEWS | 31 ++++++++++ gdb/doc/gdb.texinfo | 54 +++++++++++------ gdb/infcmd.c | 6 +- gdb/inferior.h | 7 +++ gdb/infrun.c | 1 - gdb/mi/mi-interp.c | 52 ++++++++++++++-- gdb/mi/mi-main.c | 77 +++++++++++++++++++++--- gdb/mi/mi-main.h | 4 ++ gdb/target.c | 25 ++++---- gdb/target.h | 3 +- gdb/testsuite/gdb.base/async-shell.exp | 1 - gdb/testsuite/gdb.base/async.exp | 2 - gdb/testsuite/gdb.base/corefile.exp | 17 +----- gdb/testsuite/gdb.base/dprintf-non-stop.exp | 1 - gdb/testsuite/gdb.base/gdb-sigterm.exp | 12 +--- gdb/testsuite/gdb.base/inferior-died.exp | 1 - gdb/testsuite/gdb.base/interrupt-noterm.exp | 1 - gdb/testsuite/gdb.mi/mi-async.exp | 2 +- gdb/testsuite/gdb.mi/mi-nonstop-exit.exp | 2 +- gdb/testsuite/gdb.mi/mi-nonstop.exp | 2 +- gdb/testsuite/gdb.mi/mi-ns-stale-regcache.exp | 2 +- gdb/testsuite/gdb.mi/mi-nsintrall.exp | 2 +- gdb/testsuite/gdb.mi/mi-nsmoribund.exp | 2 +- gdb/testsuite/gdb.mi/mi-nsthrexec.exp | 2 +- gdb/testsuite/gdb.mi/mi-watch-nonstop.exp | 2 +- gdb/testsuite/gdb.multi/watchpoint-multi.exp | 2 +- gdb/testsuite/gdb.python/py-evsignal.exp | 1 - gdb/testsuite/gdb.python/py-evthreads.exp | 1 - gdb/testsuite/gdb.python/py-prompt.exp | 6 +- gdb/testsuite/gdb.reverse/break-precsave.exp | 6 -- gdb/testsuite/gdb.server/solib-list.exp | 1 - gdb/testsuite/gdb.threads/thread-specific-bp.exp | 1 - gdb/testsuite/lib/mi-support.exp | 4 +- 36 files changed, 296 insertions(+), 109 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index f270c61..341698b 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,4 +1,38 @@ 2014-05-29 Pedro Alves <palves@redhat.com> + Tom Tromey <tromey@redhat.com> + + * NEWS: Mention "maint set target-async", "set mi-async", and that + background execution commands are now always available. + * target.h (target_async_permitted): Update comment. + * target.c (target_async_permitted, target_async_permitted_1): + Default to 1. + (set_target_async_command): Rename to ... + (maint_set_target_async_command): ... this. + (show_target_async_command): Rename to ... + (maint_show_target_async_command): ... this. + (_initialize_target): Adjust. + * infcmd.c (prepare_execution_command): Make extern. + * inferior.h (prepare_execution_command): Declare. + * infrun.c (set_observer_mode): Leave target async alone. + * mi/mi-interp.c (mi_interpreter_init): Install + mi_on_sync_execution_done as sync_execution_done observer. + (mi_on_sync_execution_done): New function. + (mi_execute_command_input_handler): Don't print the prompt if we + just started a synchronous command with an async target. + (mi_on_resume): Check sync_execution before printing prompt. + * mi/mi-main.h (mi_async_p): Declare. + * mi/mi-main.c: Include gdbcmd.h. + (mi_async_p): New function. + (mi_async, mi_async_1): New globals. + (set_mi_async_command, show_mi_async_command, mi_async): New + functions. + (exec_continue): Call prepare_execution_command. + (run_one_inferior, mi_cmd_exec_run, mi_cmd_list_target_features) + (mi_execute_async_cli_command): Use mi_async_p. + (_initialize_mi_main): Install "set mi-async". Make + "target-async" a deprecated alias. + +2014-05-29 Pedro Alves <palves@redhat.com> * cli/cli-interp.c (cli_interpreter_display_prompt_p): Delete. (_initialize_cli_interp): Adjust. diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog index f0e5804..18fb241 100644 --- a/gdb/doc/ChangeLog +++ b/gdb/doc/ChangeLog @@ -1,4 +1,13 @@ 2014-05-29 Pedro Alves <palves@redhat.com> + Tom Tromey <tromey@redhat.com> + + * gdb.texinfo (Non-Stop Mode): Remove "set target-async 1" + from example. + (Asynchronous and non-stop modes): Document '-gdb-set mi-async'. + Mention that target-async is now deprecated. + (Maintenance Commands): Document maint set/show target-async. + +2014-05-29 Pedro Alves <palves@redhat.com> * observer.texi (sync_execution_done, command_error): New subjects. diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 3dc8f5f..0297ec6 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,4 +1,33 @@ 2014-05-29 Pedro Alves <palves@redhat.com> + Tom Tromey <tromey@redhat.com> + + * gdb.base/async-shell.exp: Don't enable target-async. + * gdb.base/async.exp + * gdb.base/corefile.exp (corefile_test_attach): Remove 'async' + parameter. Adjust. + (top level): Don't test with "target-async". + * gdb.base/dprintf-non-stop.exp: Don't enable target-async. + * gdb.base/gdb-sigterm.exp: Don't test with "target-async". + * gdb.base/inferior-died.exp: Don't enable target-async. + * gdb.base/interrupt-noterm.exp: Likewise. + * gdb.mi/mi-async.exp: Use "mi-async" instead of "target-async". + * gdb.mi/mi-nonstop-exit.exp: Likewise. + * gdb.mi/mi-nonstop.exp: Likewise. + * gdb.mi/mi-ns-stale-regcache.exp: Likewise. + * gdb.mi/mi-nsintrall.exp: Likewise. + * gdb.mi/mi-nsmoribund.exp: Likewise. + * gdb.mi/mi-nsthrexec.exp: Likewise. + * gdb.mi/mi-watch-nonstop.exp: Likewise. + * gdb.multi/watchpoint-multi.exp: Adjust comment. + * gdb.python/py-evsignal.exp: Don't enable target-async. + * gdb.python/py-evthreads.exp: Likewise. + * gdb.python/py-prompt.exp: Likewise. + * gdb.reverse/break-precsave.exp: Don't test with "target-async". + * gdb.server/solib-list.exp: Don't enable target-async. + * gdb.threads/thread-specific-bp.exp: Likewise. + * lib/mi-support.exp: Adjust to use mi-async. + +2014-05-29 Pedro Alves <palves@redhat.com> PR gdb/13860 * gdb.mi/mi-cli.exp: Always expect "end-stepping-range" stop diff --git a/gdb/NEWS b/gdb/NEWS index 6683bc0..8b57bd9 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -71,6 +71,26 @@ set record btrace replay-memory-access (read-only|read-write) show record btrace replay-memory-access Control what memory accesses are allowed during replay. +maint set target-async (on|off) +maint show target-async + This controls whether GDB targets operate in syncronous or + asyncronous mode. Normally the default is asyncronous, if it is + available; but this can be changed to more easily debug problems + occurring only in syncronous mode. + +set mi-async (on|off) +show mi-async + Control whether MI asynchronous mode is preferred. This supersedes + "set target-async" of previous GDB versions. + +* "set target-async" is deprecated as a CLI option and is now an alias + for "set mi-async" (only puts MI into async mode). + +* Background execution commands (e.g., "c&", "s&", etc.) are now + possible ``out of the box'' if the target supports them. Previously + the user would need to explicitly enable the possibility with the + "set target-async on" command. + * New features in the GDB remote stub, GDBserver ** New option --debug-format=option1[,option2,...] allows one to add @@ -145,6 +165,17 @@ PowerPC64 GNU/Linux little-endian powerpc64le-*-linux* supported. Use "set serial baud" and "show serial baud" (respectively) instead. +* MI changes + + ** A new option "-gdb-set mi-async" replaces "-gdb-set + target-async". The latter is left as a deprecated alias of the + former for backward compatibility. If the target supports it, + CLI background execution commands are now always possible by + default, independently of whether the frontend stated a + preference for asynchronous execution with "-gdb-set mi-async". + Previously "-gdb-set target-async off" affected both MI execution + commands and CLI execution commands. + *** Changes in GDB 7.7 * Improved support for process record-replay and reverse debugging on diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index 1ee62e0..9f7fa18 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -5778,9 +5778,6 @@ To enter non-stop mode, use this sequence of commands before you run or attach to your program: @smallexample -# Enable the async interface. -set target-async 1 - # If using the CLI, pagination breaks non-stop. set pagination off @@ -5850,21 +5847,6 @@ the program to report that some thread has stopped before prompting for another command. In background execution, @value{GDBN} immediately gives a command prompt so that you can issue other commands while your program runs. -You need to explicitly enable asynchronous mode before you can use -background execution commands. You can use these commands to -manipulate the asynchronous mode setting: - -@table @code -@kindex set target-async -@item set target-async on -Enable asynchronous mode. -@item set target-async off -Disable asynchronous mode. -@kindex show target-async -@item show target-async -Show the current target-async setting. -@end table - If the target doesn't support async mode, @value{GDBN} issues an error message if you attempt to use the background execution commands. @@ -24818,12 +24800,37 @@ On some targets, @value{GDBN} is capable of processing MI commands even while the target is running. This is called @dfn{asynchronous command execution} (@pxref{Background Execution}). The frontend may specify a preferrence for asynchronous execution using the -@code{-gdb-set target-async 1} command, which should be emitted before +@code{-gdb-set mi-async 1} command, which should be emitted before either running the executable or attaching to the target. After the frontend has started the executable or attached to the target, it can find if asynchronous execution is enabled using the @code{-list-target-features} command. +@table @code +@item -gdb-set mi-async on +@item -gdb-set mi-async off +Set whether MI is in asynchronous mode. + +When @code{off}, which is the default, MI execution commands (e.g., +@code{-exec-continue}) are foreground commands, and @value{GDBN} waits +for the program to stop before processing further commands. + +When @code{on}, MI execution commands are background execution +commands (e.g., @code{-exec-continue} becomes the equivalent of the +@code{c&} CLI command), and so @value{GDBN} is capable of processing +MI commands even while the target is running. + +@item -gdb-show mi-async +Show whether MI asynchronous mode is enabled. +@end table + +Note: In @value{GDBN} version 7.7 and earlier, this option was called +@code{target-async} instead of @code{mi-async}, and it had the effect +of both putting MI in asynchronous mode and making CLI background +commands possible. CLI background commands are now always possible +``out of the box'' if the target supports them. The old spelling is +kept as a deprecated alias for backwards compatibility. + Even if @value{GDBN} can accept a command while target is running, many commands that access the target do not work when the target is running. Therefore, asynchronous command execution is most useful @@ -33532,6 +33539,15 @@ Control whether to show all non zero areas within a 1k block starting at thread local base, when using the @samp{info w32 thread-information-block} command. +@kindex maint set target-async +@kindex maint show target-async +@item maint set target-async +@itemx maint show target-async +This controls whether @value{GDBN} targets operate in synchronous or +asynchronous mode (@pxref{Background Execution}). Normally the +default is asynchronous, if it is available; but this can be changed +to more easily debug problems occurring only in synchronous mode. + @kindex maint set per-command @kindex maint show per-command @item maint set per-command diff --git a/gdb/infcmd.c b/gdb/infcmd.c index 6511d64..df4fd40 100644 --- a/gdb/infcmd.c +++ b/gdb/infcmd.c @@ -497,11 +497,9 @@ Start it from the beginning? "))) } } -/* Prepare for execution command. TARGET is the target that will run - the command. BACKGROUND determines whether this is a foreground - (synchronous) or background (asynchronous) command. */ +/* See inferior.h. */ -static void +void prepare_execution_command (struct target_ops *target, int background) { /* If we get a request for running in the bg but the target diff --git a/gdb/inferior.h b/gdb/inferior.h index 14f4ec8..668c888 100644 --- a/gdb/inferior.h +++ b/gdb/inferior.h @@ -164,6 +164,13 @@ extern void notice_new_inferior (ptid_t, int, int); extern struct value *get_return_value (struct value *function, struct type *value_type); +/* Prepare for execution command. TARGET is the target that will run + the command. BACKGROUND determines whether this is a foreground + (synchronous) or background (asynchronous) command. */ + +extern void prepare_execution_command (struct target_ops *target, + int background); + /* Whether to start up the debuggee under a shell. If startup-with-shell is set, GDB's "run" will attempt to start up diff --git a/gdb/infrun.c b/gdb/infrun.c index 9086cd2..e6540ad 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -235,7 +235,6 @@ set_observer_mode (char *args, int from_tty, going out we leave it that way. */ if (observer_mode) { - target_async_permitted = 1; pagination_enabled = 0; non_stop = non_stop_1 = 1; } diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c index e1dd97f..52a3a62 100644 --- a/gdb/mi/mi-interp.c +++ b/gdb/mi/mi-interp.c @@ -85,6 +85,7 @@ static void mi_breakpoint_modified (struct breakpoint *b); static void mi_command_param_changed (const char *param, const char *value); static void mi_memory_changed (struct inferior *inf, CORE_ADDR memaddr, ssize_t len, const bfd_byte *myaddr); +static void mi_on_sync_execution_done (void); static int report_initial_inferior (struct inferior *inf, void *closure); @@ -158,6 +159,7 @@ mi_interpreter_init (struct interp *interp, int top_level) observer_attach_breakpoint_modified (mi_breakpoint_modified); observer_attach_command_param_changed (mi_command_param_changed); observer_attach_memory_changed (mi_memory_changed); + observer_attach_sync_execution_done (mi_on_sync_execution_done); /* The initial inferior is created before this function is called, so we need to report it explicitly. Use iteration in @@ -304,6 +306,28 @@ mi_execute_command_wrapper (const char *cmd) mi_execute_command (cmd, stdin == instream); } +/* Observer for the synchronous_command_done notification. */ + +static void +mi_on_sync_execution_done (void) +{ + /* MI generally prints a prompt after a command, indicating it's + ready for further input. However, due to an historical wart, if + MI async, and a (CLI) synchronous command was issued, then we + will print the prompt right after printing "^running", even if we + cannot actually accept any input until the target stops. See + mi_on_resume. However, if the target is async but MI is sync, + then we need to output the MI prompt now, to replicate gdb's + behavior when neither the target nor MI are async. (Note this + observer is only called by the asynchronous target event handling + code.) */ + if (!mi_async_p ()) + { + fputs_unfiltered ("(gdb) \n", raw_stdout); + gdb_flush (raw_stdout); + } +} + /* mi_execute_command_wrapper wrapper suitable for INPUT_HANDLER. */ static void @@ -311,8 +335,24 @@ mi_execute_command_input_handler (char *cmd) { mi_execute_command_wrapper (cmd); - fputs_unfiltered ("(gdb) \n", raw_stdout); - gdb_flush (raw_stdout); + /* MI generally prints a prompt after a command, indicating it's + ready for further input. However, due to an historical wart, if + MI is async, and a synchronous command was issued, then we will + print the prompt right after printing "^running", even if we + cannot actually accept any input until the target stops. See + mi_on_resume. + + If MI is not async, then we print the prompt when the command + finishes. If the target is sync, that means output the prompt + now, as in that case executing a command doesn't return until the + command is done. However, if the target is async, we go back to + the event loop and output the prompt in the + 'synchronous_command_done' observer. */ + if (!target_is_async_p () || !sync_execution) + { + fputs_unfiltered ("(gdb) \n", raw_stdout); + gdb_flush (raw_stdout); + } } static void @@ -928,10 +968,10 @@ mi_on_resume (ptid_t ptid) running_result_record_printed = 1; /* This is what gdb used to do historically -- printing prompt even if it cannot actually accept any input. This will be surely removed - for MI3, and may be removed even earler. */ - /* FIXME: review the use of target_is_async_p here -- is that - what we want? */ - if (!target_is_async_p ()) + for MI3, and may be removed even earlier. SYNC_EXECUTION is + checked here because we only need to emit a prompt if a + synchronous command was issued when the target is async. */ + if (!target_is_async_p () || sync_execution) fputs_unfiltered ("(gdb) \n", raw_stdout); } gdb_flush (raw_stdout); diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c index 9ab4886..96dfc71 100644 --- a/gdb/mi/mi-main.c +++ b/gdb/mi/mi-main.c @@ -54,6 +54,7 @@ #include "ada-lang.h" #include "linespec.h" #include "extension.h" +#include "gdbcmd.h" #include <ctype.h> #include <sys/time.h> @@ -105,6 +106,45 @@ static int register_changed_p (int regnum, struct regcache *, static void output_register (struct frame_info *, int regnum, int format, int skip_unavailable); +/* Controls whether the frontend wants MI in async mode. */ +static int mi_async = 0; + +/* The set command writes to this variable. If the inferior is + executing, mi_async is *not* updated. */ +static int mi_async_1 = 0; + +static void +set_mi_async_command (char *args, int from_tty, + struct cmd_list_element *c) +{ + if (have_live_inferiors ()) + { + mi_async_1 = mi_async; + error (_("Cannot change this setting while the inferior is running.")); + } + + mi_async = mi_async_1; +} + +static void +show_mi_async_command (struct ui_file *file, int from_tty, + struct cmd_list_element *c, + const char *value) +{ + fprintf_filtered (file, + _("Whether MI is in asynchronous mode is %s.\n"), + value); +} + +/* A wrapper for target_can_async_p that takes the MI setting into + account. */ + +int +mi_async_p (void) +{ + return mi_async && target_can_async_p (); +} + /* Command implementations. FIXME: Is this libgdb? No. This is the MI layer that calls libgdb. Any operation used in the below should be formalized. */ @@ -229,6 +269,8 @@ proceed_thread_callback (struct thread_info *thread, void *arg) static void exec_continue (char **argv, int argc) { + prepare_execution_command (¤t_target, mi_async_p ()); + if (non_stop) { /* In non-stop mode, 'resume' always resumes a single thread. @@ -395,8 +437,8 @@ run_one_inferior (struct inferior *inf, void *arg) switch_to_thread (null_ptid); set_current_program_space (inf->pspace); } - mi_execute_cli_command (run_cmd, target_can_async_p (), - target_can_async_p () ? "&" : NULL); + mi_execute_cli_command (run_cmd, mi_async_p (), + mi_async_p () ? "&" : NULL); return 0; } @@ -450,8 +492,8 @@ mi_cmd_exec_run (char *command, char **argv, int argc) { const char *run_cmd = start_p ? "start" : "run"; - mi_execute_cli_command (run_cmd, target_can_async_p (), - target_can_async_p () ? "&" : NULL); + mi_execute_cli_command (run_cmd, mi_async_p (), + mi_async_p () ? "&" : NULL); } } @@ -1838,11 +1880,10 @@ mi_cmd_list_target_features (char *command, char **argv, int argc) struct ui_out *uiout = current_uiout; cleanup = make_cleanup_ui_out_list_begin_end (uiout, "features"); - if (target_can_async_p ()) + if (mi_async_p ()) ui_out_field_string (uiout, NULL, "async"); if (target_can_execute_reverse) ui_out_field_string (uiout, NULL, "reverse"); - do_cleanups (cleanup); return; } @@ -2269,7 +2310,7 @@ mi_execute_async_cli_command (char *cli_command, char **argv, int argc) struct cleanup *old_cleanups; char *run; - if (target_can_async_p ()) + if (mi_async_p ()) run = xstrprintf ("%s %s&", cli_command, argc ? *argv : ""); else run = xstrprintf ("%s %s", cli_command, argc ? *argv : ""); @@ -2920,3 +2961,25 @@ mi_cmd_trace_frame_collected (char *command, char **argv, int argc) do_cleanups (old_chain); } + +void +_initialize_mi_main (void) +{ + struct cmd_list_element *c; + + add_setshow_boolean_cmd ("mi-async", class_run, + &mi_async_1, _("\ +Set whether MI asynchronous mode is enabled."), _("\ +Show whether MI asynchronous mode is enabled."), _("\ +Tells GDB whether MI should be in asynchronous mode."), + set_mi_async_command, + show_mi_async_command, + &setlist, + &showlist); + + /* Alias old "target-async" to "mi-async". */ + c = add_alias_cmd ("target-async", "mi-async", class_run, 0, &setlist); + deprecate_cmd (c, "set mi-async"); + c = add_alias_cmd ("target-async", "mi-async", class_run, 0, &showlist); + deprecate_cmd (c, "show mi-async"); +} diff --git a/gdb/mi/mi-main.h b/gdb/mi/mi-main.h index c32845d..530aeb7 100644 --- a/gdb/mi/mi-main.h +++ b/gdb/mi/mi-main.h @@ -28,6 +28,10 @@ extern void mi_load_progress (const char *section_name, extern void mi_print_timing_maybe (void); +/* Whether MI is in async mode. */ + +extern int mi_async_p (void); + extern char *current_token; extern int running_result_record_printed; diff --git a/gdb/target.c b/gdb/target.c index 6a1a2ff..cb154ab 100644 --- a/gdb/target.c +++ b/gdb/target.c @@ -4105,16 +4105,17 @@ maintenance_print_target_stack (char *cmd, int from_tty) } } -/* Controls if async mode is permitted. */ -int target_async_permitted = 0; +/* Controls if targets can report that they can/are async. This is + just for maintainers to use when debugging gdb. */ +int target_async_permitted = 1; /* The set command writes to this variable. If the inferior is executing, target_async_permitted is *not* updated. */ -static int target_async_permitted_1 = 0; +static int target_async_permitted_1 = 1; static void -set_target_async_command (char *args, int from_tty, - struct cmd_list_element *c) +maint_set_target_async_command (char *args, int from_tty, + struct cmd_list_element *c) { if (have_live_inferiors ()) { @@ -4126,9 +4127,9 @@ set_target_async_command (char *args, int from_tty, } static void -show_target_async_command (struct ui_file *file, int from_tty, - struct cmd_list_element *c, - const char *value) +maint_show_target_async_command (struct ui_file *file, int from_tty, + struct cmd_list_element *c, + const char *value) { fprintf_filtered (file, _("Controlling the inferior in " @@ -4233,10 +4234,10 @@ result in significant performance improvement for remote targets."), 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_target_async_command, - show_target_async_command, - &setlist, - &showlist); + maint_set_target_async_command, + maint_show_target_async_command, + &maintenance_set_cmdlist, + &maintenance_show_cmdlist); add_setshow_boolean_cmd ("may-write-registers", class_support, &may_write_registers_1, _("\ diff --git a/gdb/target.h b/gdb/target.h index 9371529..face210 100644 --- a/gdb/target.h +++ b/gdb/target.h @@ -1619,8 +1619,7 @@ extern int default_child_has_execution (struct target_ops *ops, #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. */ +/* Controls whether async mode is permitted. */ extern int target_async_permitted; /* Can the target support asynchronous execution? */ diff --git a/gdb/testsuite/gdb.base/async-shell.exp b/gdb/testsuite/gdb.base/async-shell.exp index 4890a59..f0550bc 100644 --- a/gdb/testsuite/gdb.base/async-shell.exp +++ b/gdb/testsuite/gdb.base/async-shell.exp @@ -31,7 +31,6 @@ if { [prepare_for_testing ${testfile}.exp ${testfile} $srcfile] } { set gdbindex_warning_re "warning: Skipping \[^\r\n\]+ \\.gdb_index section \[^\r\n\]*\r\nDo \"set use-deprecated-index-sections on\" before the file is read\r\nto use the section anyway\\." -gdb_test_no_output "set target-async on " gdb_test_no_output "set non-stop on" gdb_test "run &" "Starting program: \[^\r\n\]*(\r\n$gdbindex_warning_re)?" diff --git a/gdb/testsuite/gdb.base/async.exp b/gdb/testsuite/gdb.base/async.exp index f0a18e8..0f99b01 100644 --- a/gdb/testsuite/gdb.base/async.exp +++ b/gdb/testsuite/gdb.base/async.exp @@ -25,8 +25,6 @@ if {[prepare_for_testing $testfile.exp $testfile $srcfile debug]} { return -1 } -gdb_test_no_output "set target-async on" - # # set it up at a breakpoint so we can play with it # diff --git a/gdb/testsuite/gdb.base/corefile.exp b/gdb/testsuite/gdb.base/corefile.exp index 6eb1f02..bde2de8 100644 --- a/gdb/testsuite/gdb.base/corefile.exp +++ b/gdb/testsuite/gdb.base/corefile.exp @@ -241,7 +241,7 @@ gdb_exit # Test an attach command will clear any loaded core file. -proc corefile_test_attach {{async 0}} { +proc corefile_test_attach {} { global binfile corefile gdb_prompt if ![is_remote target] { @@ -257,11 +257,6 @@ proc corefile_test_attach {{async 0}} { gdb_start - if {$async} { - gdb_test_no_output "set target-async on" \ - "enable target-async for attach tests" - } - gdb_test "core-file $corefile" "Core was generated by .*" "attach: load core again" gdb_test "info files" "\r\nLocal core dump file:\r\n.*" "attach: sanity check we see the core file" @@ -298,13 +293,3 @@ gdb_test_multiple "core-file $corefile" $test { pass $test } } - - -# Try a couple tests again with target-async. -with_test_prefix "target-async" { - clean_restart ${testfile} - - gdb_test_no_output "set target-async on" - corefile_test_run - corefile_test_attach 1 -} diff --git a/gdb/testsuite/gdb.base/dprintf-non-stop.exp b/gdb/testsuite/gdb.base/dprintf-non-stop.exp index fdaa5c1..df1e270 100644 --- a/gdb/testsuite/gdb.base/dprintf-non-stop.exp +++ b/gdb/testsuite/gdb.base/dprintf-non-stop.exp @@ -26,7 +26,6 @@ if [prepare_for_testing "failed to prepare for dprintf with non-stop" \ return -1 } -gdb_test_no_output "set target-async on" gdb_test_no_output "set non-stop on" if ![runto main] { diff --git a/gdb/testsuite/gdb.base/gdb-sigterm.exp b/gdb/testsuite/gdb.base/gdb-sigterm.exp index f52517c..df0de42 100644 --- a/gdb/testsuite/gdb.base/gdb-sigterm.exp +++ b/gdb/testsuite/gdb.base/gdb-sigterm.exp @@ -79,17 +79,7 @@ proc do_test { pass } { # 50 runs should be approx. a safe number to be sure it is fixed now. for {set pass 0} {$pass < 50} {incr pass} { - clean_restart ${testfile} - gdb_test_no_output "set target-async off" "" - with_test_prefix "sync" { - do_test $pass - } - - clean_restart ${testfile} - gdb_test_no_output "set target-async on" "" - with_test_prefix "async" { - do_test $pass - } + do_test $pass } pass "$pass SIGTERM passes" diff --git a/gdb/testsuite/gdb.base/inferior-died.exp b/gdb/testsuite/gdb.base/inferior-died.exp index 33f92e9..152bdd8 100644 --- a/gdb/testsuite/gdb.base/inferior-died.exp +++ b/gdb/testsuite/gdb.base/inferior-died.exp @@ -38,7 +38,6 @@ if { [prepare_for_testing ${testfile}.exp ${testfile} ${testfile}.c] } { } gdb_test_no_output "set detach-on-fork off" -gdb_test_no_output "set target-async on" gdb_test_no_output "set non-stop on" if ![runto_main] { diff --git a/gdb/testsuite/gdb.base/interrupt-noterm.exp b/gdb/testsuite/gdb.base/interrupt-noterm.exp index a22acd2..5c92b97 100644 --- a/gdb/testsuite/gdb.base/interrupt-noterm.exp +++ b/gdb/testsuite/gdb.base/interrupt-noterm.exp @@ -22,7 +22,6 @@ if [prepare_for_testing "failed to prepare for testing" \ # Pretend there's no terminal. gdb_test_no_output "set interactive-mode off" -gdb_test_no_output "set target-async on" if ![runto main] { fail "Can't run to main" diff --git a/gdb/testsuite/gdb.mi/mi-async.exp b/gdb/testsuite/gdb.mi/mi-async.exp index e41701d..0df4c98 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*]) } then { # The plan is for async mode to become the default but toggle for now. set saved_gdbflags $GDBFLAGS -set GDBFLAGS [concat $GDBFLAGS " -ex \"set target-async on\""] +set GDBFLAGS [concat $GDBFLAGS " -ex \"set mi-async on\""] load_lib mi-support.exp diff --git a/gdb/testsuite/gdb.mi/mi-nonstop-exit.exp b/gdb/testsuite/gdb.mi/mi-nonstop-exit.exp index 3727d81..5187b40 100644 --- a/gdb/testsuite/gdb.mi/mi-nonstop-exit.exp +++ b/gdb/testsuite/gdb.mi/mi-nonstop-exit.exp @@ -40,7 +40,7 @@ mi_gdb_reinitialize_dir $srcdir/$subdir mi_gdb_load $binfile mi_gdb_test "-gdb-set non-stop 1" ".*" -mi_gdb_test "-gdb-set target-async 1" ".*" +mi_gdb_test "-gdb-set mi-async 1" ".*" mi_detect_async if { [mi_run_to_main] < 0 } { diff --git a/gdb/testsuite/gdb.mi/mi-nonstop.exp b/gdb/testsuite/gdb.mi/mi-nonstop.exp index 5ef74ed..ba7dfd4 100644 --- a/gdb/testsuite/gdb.mi/mi-nonstop.exp +++ b/gdb/testsuite/gdb.mi/mi-nonstop.exp @@ -50,7 +50,7 @@ mi_gdb_reinitialize_dir $srcdir/$subdir mi_gdb_load $binfile mi_gdb_test "-gdb-set non-stop 1" ".*" -mi_gdb_test "-gdb-set target-async 1" ".*" +mi_gdb_test "-gdb-set mi-async 1" ".*" mi_detect_async if { [mi_run_to_main] < 0 } { diff --git a/gdb/testsuite/gdb.mi/mi-ns-stale-regcache.exp b/gdb/testsuite/gdb.mi/mi-ns-stale-regcache.exp index 754689c..ae9e5f2 100644 --- a/gdb/testsuite/gdb.mi/mi-ns-stale-regcache.exp +++ b/gdb/testsuite/gdb.mi/mi-ns-stale-regcache.exp @@ -53,7 +53,7 @@ mi_gdb_reinitialize_dir $srcdir/$subdir mi_gdb_load $binfile mi_gdb_test "-gdb-set non-stop 1" ".*" -mi_gdb_test "-gdb-set target-async 1" ".*" +mi_gdb_test "-gdb-set mi-async 1" ".*" mi_detect_async if { [mi_run_to_main] < 0 } { diff --git a/gdb/testsuite/gdb.mi/mi-nsintrall.exp b/gdb/testsuite/gdb.mi/mi-nsintrall.exp index f613e7f..ac1209e 100644 --- a/gdb/testsuite/gdb.mi/mi-nsintrall.exp +++ b/gdb/testsuite/gdb.mi/mi-nsintrall.exp @@ -40,7 +40,7 @@ mi_gdb_reinitialize_dir $srcdir/$subdir mi_gdb_load $binfile mi_gdb_test "-gdb-set non-stop 1" ".*" -mi_gdb_test "-gdb-set target-async 1" ".*" +mi_gdb_test "-gdb-set mi-async 1" ".*" mi_detect_async if { [mi_run_to_main] < 0 } { diff --git a/gdb/testsuite/gdb.mi/mi-nsmoribund.exp b/gdb/testsuite/gdb.mi/mi-nsmoribund.exp index 350c060..0ff04ca 100644 --- a/gdb/testsuite/gdb.mi/mi-nsmoribund.exp +++ b/gdb/testsuite/gdb.mi/mi-nsmoribund.exp @@ -40,7 +40,7 @@ mi_gdb_reinitialize_dir $srcdir/$subdir mi_gdb_load $binfile mi_gdb_test "-gdb-set non-stop 1" ".*" -mi_gdb_test "-gdb-set target-async 1" ".*" +mi_gdb_test "-gdb-set mi-async 1" ".*" mi_detect_async if { [mi_run_to_main] < 0 } { diff --git a/gdb/testsuite/gdb.mi/mi-nsthrexec.exp b/gdb/testsuite/gdb.mi/mi-nsthrexec.exp index 85617d0..008a831 100644 --- a/gdb/testsuite/gdb.mi/mi-nsthrexec.exp +++ b/gdb/testsuite/gdb.mi/mi-nsthrexec.exp @@ -50,7 +50,7 @@ mi_gdb_reinitialize_dir $srcdir/$subdir mi_gdb_load $binfile mi_gdb_test "-gdb-set non-stop 1" ".*" -mi_gdb_test "-gdb-set target-async 1" ".*" +mi_gdb_test "-gdb-set mi-async 1" ".*" mi_detect_async if { [mi_run_to_main] < 0 } { diff --git a/gdb/testsuite/gdb.mi/mi-watch-nonstop.exp b/gdb/testsuite/gdb.mi/mi-watch-nonstop.exp index 3dbf4c7..44371c8 100644 --- a/gdb/testsuite/gdb.mi/mi-watch-nonstop.exp +++ b/gdb/testsuite/gdb.mi/mi-watch-nonstop.exp @@ -52,7 +52,7 @@ mi_gdb_reinitialize_dir $srcdir/$subdir mi_gdb_load $binfile mi_gdb_test "-gdb-set non-stop 1" ".*" -mi_gdb_test "-gdb-set target-async 1" ".*" +mi_gdb_test "-gdb-set mi-async 1" ".*" mi_detect_async if { [mi_run_to_main] < 0 } { diff --git a/gdb/testsuite/gdb.multi/watchpoint-multi.exp b/gdb/testsuite/gdb.multi/watchpoint-multi.exp index b7fb0a9..e6a8957 100644 --- a/gdb/testsuite/gdb.multi/watchpoint-multi.exp +++ b/gdb/testsuite/gdb.multi/watchpoint-multi.exp @@ -37,7 +37,7 @@ if { [gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" executa clean_restart $executable -# Simulate non-stop+target-async which also uses breakpoint always-inserted. +# Simulate non-stop which also uses breakpoint always-inserted. gdb_test_no_output "set breakpoint always-inserted on" # displaced-stepping is also needed as other GDB sometimes still removes the # breakpoints, even with always-inserted on. diff --git a/gdb/testsuite/gdb.python/py-evsignal.exp b/gdb/testsuite/gdb.python/py-evsignal.exp index af3d53c..530fd07 100644 --- a/gdb/testsuite/gdb.python/py-evsignal.exp +++ b/gdb/testsuite/gdb.python/py-evsignal.exp @@ -35,7 +35,6 @@ gdb_test_no_output "python exec (open ('${pyfile}').read ())" "" gdb_test "test-events" "Event testers registered." gdb_test_no_output "set non-stop on" -gdb_test_no_output "set target-async on" gdb_run_cmd gdb_test_multiple "" "Signal Thread 3" { diff --git a/gdb/testsuite/gdb.python/py-evthreads.exp b/gdb/testsuite/gdb.python/py-evthreads.exp index ffa322f..d62624e 100644 --- a/gdb/testsuite/gdb.python/py-evthreads.exp +++ b/gdb/testsuite/gdb.python/py-evthreads.exp @@ -40,7 +40,6 @@ gdb_test_no_output "python exec (open ('${pyfile}').read ())" "" gdb_test "test-events" "Event testers registered." gdb_test_no_output "set non-stop on" -gdb_test_no_output "set target-async on" gdb_breakpoint "main" gdb_breakpoint "thread2" diff --git a/gdb/testsuite/gdb.python/py-prompt.exp b/gdb/testsuite/gdb.python/py-prompt.exp index e376c05..ebe4cb6 100644 --- a/gdb/testsuite/gdb.python/py-prompt.exp +++ b/gdb/testsuite/gdb.python/py-prompt.exp @@ -90,8 +90,7 @@ if { [istarget "*-*-cygwin*"] } { set testpid [ exec ps -e | gawk "{ if (\$1 == $testpid) print \$4; }" ] } -set GDBFLAGS [concat $tmp_gdbflags " -ex \"set target-async on\""] -set GDBFLAGS [concat $GDBFLAGS " -ex \"set pagination off\""] +set GDBFLAGS [concat $tmp_gdbflags " -ex \"set pagination off\""] set GDBFLAGS [concat $GDBFLAGS " -ex \"set editing on\""] set GDBFLAGS [concat $GDBFLAGS " -ex \"attach $testpid\""] set GDBFLAGS [concat $GDBFLAGS " -ex \"continue&\""] @@ -107,8 +106,7 @@ gdb_test "python print (\"'\" + str(p\[0\]) + \"'\")" "'$gdb_prompt_fail '" \ "prompt_hook argument is default prompt. 3" gdb_exit -set GDBFLAGS [concat $tmp_gdbflags " -ex \"set target-async on\""] -set GDBFLAGS [concat $GDBFLAGS " -ex \"set pagination off\""] +set GDBFLAGS [concat $tmp_gdbflags " -ex \"set pagination off\""] set GDBFLAGS [concat $GDBFLAGS " -ex \"set editing on\""] set GDBFLAGS [concat $GDBFLAGS " -ex \"attach $testpid\""] set GDBFLAGS [concat $GDBFLAGS " -ex \"interrupt\""] diff --git a/gdb/testsuite/gdb.reverse/break-precsave.exp b/gdb/testsuite/gdb.reverse/break-precsave.exp index f1e6c69..3206e3c 100644 --- a/gdb/testsuite/gdb.reverse/break-precsave.exp +++ b/gdb/testsuite/gdb.reverse/break-precsave.exp @@ -110,9 +110,3 @@ proc precsave_tests {} { } precsave_tests - -with_test_prefix "target-async" { - clean_restart $testfile - gdb_test_no_output "set target-async on" - precsave_tests -} diff --git a/gdb/testsuite/gdb.server/solib-list.exp b/gdb/testsuite/gdb.server/solib-list.exp index 8cc3edc..4504a23 100644 --- a/gdb/testsuite/gdb.server/solib-list.exp +++ b/gdb/testsuite/gdb.server/solib-list.exp @@ -66,7 +66,6 @@ foreach nonstop { 0 1 } { with_test_prefix "non-stop $nonstop" { gdb_test "disconnect" ".*" gdb_test "set non-stop $nonstop" - gdb_test "set target-async $nonstop" # It is required for the non-stop mode, GDB would try to step over # _dl_debug_state breakpoint will still only ld.so loaded in gdbserver. diff --git a/gdb/testsuite/gdb.threads/thread-specific-bp.exp b/gdb/testsuite/gdb.threads/thread-specific-bp.exp index b8416d7..4273e1d 100644 --- a/gdb/testsuite/gdb.threads/thread-specific-bp.exp +++ b/gdb/testsuite/gdb.threads/thread-specific-bp.exp @@ -122,6 +122,5 @@ check_thread_specific_breakpoint "all-stop" clean_restart ${binfile} # Test non-stop mode. -gdb_test_no_output "set target-async on" "set async mode" gdb_test_no_output "set non-stop on" "set non-stop mode" check_thread_specific_breakpoint "non-stop" diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp index 9f07cda..f1b5749 100644 --- a/gdb/testsuite/lib/mi-support.exp +++ b/gdb/testsuite/lib/mi-support.exp @@ -1014,10 +1014,10 @@ proc mi_detect_async {} { global async global mi_gdb_prompt - send_gdb "show target-async\n" + send_gdb "show mi-async\n" gdb_expect { - -re ".*Controlling the inferior in asynchronous mode is on...*$mi_gdb_prompt$" { + -re "asynchronous mode is on...*$mi_gdb_prompt$" { set async 1 } -re ".*$mi_gdb_prompt$" { -- 1.9.0 ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v6 2/2] enable target async by default; separate MI and target notions of async 2014-05-29 13:49 ` Pedro Alves @ 2014-07-30 18:40 ` Doug Evans 0 siblings, 0 replies; 42+ messages in thread From: Doug Evans @ 2014-07-30 18:40 UTC (permalink / raw) To: Pedro Alves; +Cc: Eli Zaretskii, gdb-patches On Thu, May 29, 2014 at 6:48 AM, Pedro Alves <palves@redhat.com> wrote: > [...] > Below's what I pushed. > > 8<------------ > Subject: [PATCH] enable target async by default; separate MI and target > notions of async > > This finally makes background execution commands possible by default. > > However, in order to do that, there's one last thing we need to do -- > we need to separate the MI and target notions of "async". Unlike the > CLI, where the user explicitly requests foreground vs background > execution in the execution command itself (c vs c&), MI chose to treat > "set target-async" specially -- setting it changes the default > behavior of execution commands. > > So, we can't simply "set target-async" default to on, as that would > affect MI frontends. Instead we have to make the setting MI-specific, > and teach MI about sync commands on top of an async target. > > Because the "target" word in "set target-async" ends up as a potential > source of confusion, the patch adds a "set mi-async" option, and makes > "set target-async" a deprecated alias. > > Rather than make the targets always async, this patch introduces a new > "maint set target-async" option so that the GDB developer can control > whether the target is async. This makes it simpler to debug issues > arising only in the synchronous mode; important because sync mode > seems unlikely to go away. > > Unlike in previous revisions, "set target-async" does not affect this > new maint parameter. The rationale for this is that then one can > easily run the test suite in the "maint set target-async off" mode and > have tests that enable mi-async fail just like they fail on > non-async-capable targets. This emulation is exactly the point of the > maint option. > > I had asked Tom in a previous iteration to split the actual change of > the target async default to a separate patch, but it turns out that > that is quite awkward in this version of the patch, because with MI > async and target async decoupled (unlike in previous versions), if we > don't flip the default at the same time, then just "set target-async > on" alone never actually manages to do anything. It's best to not > have that transitory state in the tree. > > Given "set target-async on" now only has effect for MI, the patch goes > through the testsuite removing it from non-MI tests. MI tests are > adjusted to use the new and less confusing "mi-async" spelling. > > 2014-05-29 Pedro Alves <palves@redhat.com> > Tom Tromey <tromey@redhat.com> > > * NEWS: Mention "maint set target-async", "set mi-async", and that > background execution commands are now always available. > * target.h (target_async_permitted): Update comment. > * target.c (target_async_permitted, target_async_permitted_1): > Default to 1. > (set_target_async_command): Rename to ... > (maint_set_target_async_command): ... this. > (show_target_async_command): Rename to ... > (maint_show_target_async_command): ... this. > (_initialize_target): Adjust. > * infcmd.c (prepare_execution_command): Make extern. > * inferior.h (prepare_execution_command): Declare. > * infrun.c (set_observer_mode): Leave target async alone. > * mi/mi-interp.c (mi_interpreter_init): Install > mi_on_sync_execution_done as sync_execution_done observer. > (mi_on_sync_execution_done): New function. > (mi_execute_command_input_handler): Don't print the prompt if we > just started a synchronous command with an async target. > (mi_on_resume): Check sync_execution before printing prompt. > * mi/mi-main.h (mi_async_p): Declare. > * mi/mi-main.c: Include gdbcmd.h. > (mi_async_p): New function. > (mi_async, mi_async_1): New globals. > (set_mi_async_command, show_mi_async_command, mi_async): New > functions. > (exec_continue): Call prepare_execution_command. > (run_one_inferior, mi_cmd_exec_run, mi_cmd_list_target_features) > (mi_execute_async_cli_command): Use mi_async_p. > (_initialize_mi_main): Install "set mi-async". Make > "target-async" a deprecated alias. > > 2014-05-29 Pedro Alves <palves@redhat.com> > Tom Tromey <tromey@redhat.com> > > * gdb.texinfo (Non-Stop Mode): Remove "set target-async 1" > from example. > (Asynchronous and non-stop modes): Document '-gdb-set mi-async'. > Mention that target-async is now deprecated. > (Maintenance Commands): Document maint set/show target-async. > > 2014-05-29 Pedro Alves <palves@redhat.com> > Tom Tromey <tromey@redhat.com> > > * gdb.base/async-shell.exp: Don't enable target-async. > * gdb.base/async.exp > * gdb.base/corefile.exp (corefile_test_attach): Remove 'async' > parameter. Adjust. > (top level): Don't test with "target-async". > * gdb.base/dprintf-non-stop.exp: Don't enable target-async. > * gdb.base/gdb-sigterm.exp: Don't test with "target-async". > * gdb.base/inferior-died.exp: Don't enable target-async. > * gdb.base/interrupt-noterm.exp: Likewise. > * gdb.mi/mi-async.exp: Use "mi-async" instead of "target-async". > * gdb.mi/mi-nonstop-exit.exp: Likewise. > * gdb.mi/mi-nonstop.exp: Likewise. > * gdb.mi/mi-ns-stale-regcache.exp: Likewise. > * gdb.mi/mi-nsintrall.exp: Likewise. > * gdb.mi/mi-nsmoribund.exp: Likewise. > * gdb.mi/mi-nsthrexec.exp: Likewise. > * gdb.mi/mi-watch-nonstop.exp: Likewise. > * gdb.multi/watchpoint-multi.exp: Adjust comment. > * gdb.python/py-evsignal.exp: Don't enable target-async. > * gdb.python/py-evthreads.exp: Likewise. > * gdb.python/py-prompt.exp: Likewise. > * gdb.reverse/break-precsave.exp: Don't test with "target-async". > * gdb.server/solib-list.exp: Don't enable target-async. > * gdb.threads/thread-specific-bp.exp: Likewise. > * lib/mi-support.exp: Adjust to use mi-async. Hi. I happened to notice a couple of comments that still need updating. linux-nat.c: /* target_is_async_p implementation. */ static int linux_nat_is_async_p (struct target_ops *ops) { /* NOTE: palves 2008-03-21: We're only async when the user requests it explicitly with the "set target-async" command. Someday, linux will always be async. */ return target_async_permitted; } /* target_can_async_p implementation. */ static int linux_nat_can_async_p (struct target_ops *ops) { /* NOTE: palves 2008-03-21: We're only async when the user requests it explicitly with the "set target-async" command. Someday, linux will always be async. */ return target_async_permitted; } ^ permalink raw reply [flat|nested] 42+ messages in thread
* [pushed] Re: [PATCH v6 0/2] enable target-async by default 2014-05-23 20:59 [PATCH v6 0/2] enable target-async by default Pedro Alves 2014-05-23 20:59 ` [PATCH v6 1/2] Make display_gdb_prompt CLI-only Pedro Alves 2014-05-23 20:59 ` [PATCH v6 2/2] enable target async by default; separate MI and target notions of async Pedro Alves @ 2014-05-29 13:44 ` Pedro Alves 2014-07-01 16:28 ` Regression for attach from stdin [Re: [pushed] Re: [PATCH v6 0/2] enable target-async by default] Jan Kratochvil 2014-10-05 14:00 ` Crash regression for annota1.exp w/vDSO debuginfo [Re: [pushed] Re: [PATCH v6 0/2] enable target-async by default] Jan Kratochvil 2 siblings, 2 replies; 42+ messages in thread From: Pedro Alves @ 2014-05-29 13:44 UTC (permalink / raw) To: gdb-patches On 05/23/2014 09:59 PM, Pedro Alves wrote: > There's actually a third patch this depends on, here: > > https://sourceware.org/ml/gdb-patches/2014-05/msg00590.html I went ahead and pushed this all in, with Eli's comments addressed. "set target-async" is dead, long live "set target-async"! I think this is a significant milestone, which has been quite long in the making, with several people on and off pushing for it little by little. Thanks everyone. Apologies for the haste, but, the 7.8 branch point date is approaching, and I'd like to get async-by-default into 7.8, and I think the sooner this gets into the tree before the branch, the better. If something goes badly wrong, we can flip the default back to target-async off, and make target-async depend on mi-async again (make -gdb-set target-async flip both globals), which makes most of the new MI code dead. (Or we can always just revert.) Thanks, -- Pedro Alves ^ permalink raw reply [flat|nested] 42+ messages in thread
* Regression for attach from stdin [Re: [pushed] Re: [PATCH v6 0/2] enable target-async by default] 2014-05-29 13:44 ` [pushed] Re: [PATCH v6 0/2] enable target-async by default Pedro Alves @ 2014-07-01 16:28 ` Jan Kratochvil 2014-07-02 8:59 ` Mark Wielaard 2014-10-05 14:00 ` Crash regression for annota1.exp w/vDSO debuginfo [Re: [pushed] Re: [PATCH v6 0/2] enable target-async by default] Jan Kratochvil 1 sibling, 1 reply; 42+ messages in thread From: Jan Kratochvil @ 2014-07-01 16:28 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches On Thu, 29 May 2014 15:44:02 +0200, Pedro Alves wrote: > I went ahead and pushed this all in, with Eli's comments addressed. 329ea57934a9d4b250a0b417af1ec47bc2d0ceb6 is the first bad commit commit 329ea57934a9d4b250a0b417af1ec47bc2d0ceb6 Author: Pedro Alves <palves@redhat.com> Date: Thu May 29 19:58:57 2014 +0100 enable target async by default; separate MI and target notions of async This affects GCC testsuite where guality.exp does not start at all with: gdb: took too long to attach cat >var2.c <<HERE #include <stdio.h> volatile int xxx; int main(void) { while (!xxx); printf("xxx=%d\n",xxx); return 0; } HERE gcc -o var2 var2.c -Wall -g ./var2 &p=$!;sleep 0.1;echo -e "set trace-commands\nattach $p\nset xxx=1\nprint xxx"|gdb -q;kill -9 $p ---> [1] 31729 (gdb) (gdb) +attach 31729 Attaching to process 31729 +set xxx=1 No symbol table is loaded. Use the "file" command. ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ (gdb) Reading symbols from /home/jkratoch/t/var2...done. Reading symbols from /lib64/libc.so.6...Reading symbols from /usr/lib/debug/usr/lib64/libc-2.19.90.so.debug...done. done. Loaded symbols for /lib64/libc.so.6 Reading symbols from /lib64/ld-linux-x86-64.so.2...Reading symbols from /usr/lib/debug/usr/lib64/ld-2.19.90.so.debug...done. done. Loaded symbols for /lib64/ld-linux-x86-64.so.2 0x0000000000400541 in main () at var2.c:4 4 while (!xxx); +print xxx $1 = 0 Jan ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: Regression for attach from stdin [Re: [pushed] Re: [PATCH v6 0/2] enable target-async by default] 2014-07-01 16:28 ` Regression for attach from stdin [Re: [pushed] Re: [PATCH v6 0/2] enable target-async by default] Jan Kratochvil @ 2014-07-02 8:59 ` Mark Wielaard 2014-07-02 9:16 ` Pedro Alves 0 siblings, 1 reply; 42+ messages in thread From: Mark Wielaard @ 2014-07-02 8:59 UTC (permalink / raw) To: Jan Kratochvil; +Cc: Pedro Alves, gdb-patches On Tue, 2014-07-01 at 18:28 +0200, Jan Kratochvil wrote: > On Thu, 29 May 2014 15:44:02 +0200, Pedro Alves wrote: > > I went ahead and pushed this all in, with Eli's comments addressed. > > 329ea57934a9d4b250a0b417af1ec47bc2d0ceb6 is the first bad commit > commit 329ea57934a9d4b250a0b417af1ec47bc2d0ceb6 > Author: Pedro Alves <palves@redhat.com> > Date: Thu May 29 19:58:57 2014 +0100 > enable target async by default; separate MI and target notions of async > > This affects GCC testsuite where guality.exp does not start at all with: > gdb: took too long to attach Thanks for tracking this down Jan. Should this patch be reverted or should gcc guality.h be patched to work around the new async default somehow? Currently I am using the following patch to guality.h in gcc which seems to work around it, but I don't really understand why: diff --git a/gcc/testsuite/gcc.dg/guality/guality.h b/gcc/testsuite/gcc.dg/guality/guality.h index 8b657f2..d83e270 100644 --- a/gcc/testsuite/gcc.dg/guality/guality.h +++ b/gcc/testsuite/gcc.dg/guality/guality.h @@ -243,6 +243,7 @@ main (int argc, char *argv[]) || fprintf (guality_gdb_input, "\ set height 0\n\ attach %i\n\ +print guality_attached\n\ set guality_attached = 1\n\ b %i\n\ continue\n\ Thanks, Mark ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: Regression for attach from stdin [Re: [pushed] Re: [PATCH v6 0/2] enable target-async by default] 2014-07-02 8:59 ` Mark Wielaard @ 2014-07-02 9:16 ` Pedro Alves 2014-07-03 15:39 ` Pedro Alves 0 siblings, 1 reply; 42+ messages in thread From: Pedro Alves @ 2014-07-02 9:16 UTC (permalink / raw) To: Mark Wielaard, Jan Kratochvil; +Cc: gdb-patches On 07/02/2014 09:59 AM, Mark Wielaard wrote: > Thanks for tracking this down Jan. Should this patch be reverted Unless it turns into a deep rat hole, I'd like to fix the issue instead. I haven't investigated this particular issue yet, but I've been busy fixing PR17072, and I ended up fixing several issues related to execution commands entered directly in the command line, before GDB's main event loop starts. This one sounds related. > or > should gcc guality.h be patched to work around the new async default > somehow? No, the async default in an internal implementation detail. It should cause no visible change of behavior, other than enabling features. -- Pedro Alves ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: Regression for attach from stdin [Re: [pushed] Re: [PATCH v6 0/2] enable target-async by default] 2014-07-02 9:16 ` Pedro Alves @ 2014-07-03 15:39 ` Pedro Alves 2014-07-04 13:48 ` [PATCH] Fix "attach" command vs user input race [Re: Regression for attach from stdin [Re: [pushed] Re: [PATCH v6 0/2] enable target-async by default]] Pedro Alves 0 siblings, 1 reply; 42+ messages in thread From: Pedro Alves @ 2014-07-03 15:39 UTC (permalink / raw) To: Mark Wielaard, Jan Kratochvil; +Cc: gdb-patches On 07/02/2014 10:15 AM, Pedro Alves wrote: > On 07/02/2014 09:59 AM, Mark Wielaard wrote: >> Thanks for tracking this down Jan. Should this patch be reverted > > Unless it turns into a deep rat hole, I'd like to fix the issue > instead. I haven't investigated this particular issue yet, but I've > been busy fixing PR17072, and I ended up fixing several issues > related to execution commands entered directly in the command line, > before GDB's main event loop starts. This one sounds related. Looking at this now. The other series unfortunately doesn't fix this. -- Pedro Alves ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH] Fix "attach" command vs user input race [Re: Regression for attach from stdin [Re: [pushed] Re: [PATCH v6 0/2] enable target-async by default]] 2014-07-03 15:39 ` Pedro Alves @ 2014-07-04 13:48 ` Pedro Alves 2014-07-04 21:13 ` Mark Wielaard ` (2 more replies) 0 siblings, 3 replies; 42+ messages in thread From: Pedro Alves @ 2014-07-04 13:48 UTC (permalink / raw) To: Mark Wielaard, Jan Kratochvil; +Cc: gdb-patches On 07/03/2014 04:38 PM, Pedro Alves wrote: > On 07/02/2014 10:15 AM, Pedro Alves wrote: >> On 07/02/2014 09:59 AM, Mark Wielaard wrote: >>> Thanks for tracking this down Jan. Should this patch be reverted >> >> Unless it turns into a deep rat hole, I'd like to fix the issue >> instead. I haven't investigated this particular issue yet, but I've >> been busy fixing PR17072, and I ended up fixing several issues >> related to execution commands entered directly in the command line, >> before GDB's main event loop starts. This one sounds related. > > Looking at this now. The other series unfortunately doesn't fix this. > Here's a fix. Let me know whether it fixes guality testing. You can also find it at: git@github.com:palves/gdb.git palves/attach_from_stdin I've also pushed it on top of the palves/pr17072_pagination_async branch, in case the other fixes on that branch (posted yesterday) are necessary. Thanks, Pedro Alves 8<---------- From f93645049c02459bedada7b61fea633f59a47817 Mon Sep 17 00:00:00 2001 From: Pedro Alves <palves@redhat.com> Date: Fri, 4 Jul 2014 13:47:53 +0100 Subject: [PATCH] Fix "attach" command vs user input race On async targets, a synchronous attach is done like this: #1 - target_attach is called (PTRACE_ATTACH is issued) #2 - a continuation is installed #3 - we go back to the event loop #4 - target reports stop (SIGSTOP), event loop wakes up, and attach continuation is called #5 - among other things, the continuation calls target_terminal_inferior, which removes stdin from the event loop Note that in #3, GDB is still processing user input. If the user is fast enough, e.g., with something like: echo -e "attach PID\nset xxx=1" | gdb ... then the "set" command is processed before the attach completes. We get worse behavior even, if input is a tty and therefore readline/editing is enabled, with e.g.,: (gdb) attach PID\nset xxx=1 we then crash readline/gdb, with: Attaching to program: attach-wait-input, process 14537 readline: readline_callback_read_char() called with no handler! Aborted $ Fix this by calling target_terminal_inferior before #3 above. The test covers both scenarios by running with editing/readline forced to both on and off. gdb/ 2014-07-04 Pedro Alves <palves@redhat.com> * infcmd.c (attach_command_post_wait): Don't call target_terminal_inferior here. (attach_command): Call it here instead. gdb/testsuite/ 2014-07-04 Pedro Alves <palves@redhat.com> * gdb.base/attach-fg-no-stdin.exp: New file. * gdb.base/attach-fg-no-stdin.c: New file. --- gdb/infcmd.c | 9 +- gdb/testsuite/gdb.base/attach-wait-input.c | 47 +++++++++++ gdb/testsuite/gdb.base/attach-wait-input.exp | 119 +++++++++++++++++++++++++++ 3 files changed, 170 insertions(+), 5 deletions(-) create mode 100644 gdb/testsuite/gdb.base/attach-wait-input.c create mode 100644 gdb/testsuite/gdb.base/attach-wait-input.exp diff --git a/gdb/infcmd.c b/gdb/infcmd.c index c4bb401..76ab12b 100644 --- a/gdb/infcmd.c +++ b/gdb/infcmd.c @@ -2381,9 +2381,6 @@ attach_command_post_wait (char *args, int from_tty, int async_exec) post_create_inferior (¤t_target, from_tty); - /* Install inferior's terminal modes. */ - target_terminal_inferior (); - if (async_exec) { /* The user requested an `attach&', so be sure to leave threads @@ -2495,9 +2492,11 @@ attach_command (char *args, int from_tty) shouldn't refer to attach_target again. */ attach_target = NULL; - /* Set up the "saved terminal modes" of the inferior - based on what modes we are starting it with. */ + /* Set up the "saved terminal modes" of the inferior based on what + modes we are starting it with, and remove stdin from the event + loop. */ target_terminal_init (); + target_terminal_inferior (); /* Set up execution context to know that we should return from wait_for_inferior as soon as the target reports a stop. */ diff --git a/gdb/testsuite/gdb.base/attach-wait-input.c b/gdb/testsuite/gdb.base/attach-wait-input.c new file mode 100644 index 0000000..bb99b1f --- /dev/null +++ b/gdb/testsuite/gdb.base/attach-wait-input.c @@ -0,0 +1,47 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2014 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see <http://www.gnu.org/licenses/>. */ + +/* This program is intended to be started outside of gdb, and then + attached to by gdb. Thus, it simply spins in a loop. The loop + is exited when & if the variable 'should_exit' is non-zero. (It + is initialized to zero in this program, so the loop will never + exit unless/until gdb sets the variable to non-zero.) + */ + +#include <sys/types.h> +#include <unistd.h> + +volatile int should_exit = 0; +int mypid; + +static void +setup_done (void) +{ +} + +int +main (void) +{ + unsigned int counter = 1; + + mypid = getpid (); + setup_done (); + + for (counter = 0; !should_exit && counter < 100; counter++) + sleep (1); + return 0; +} diff --git a/gdb/testsuite/gdb.base/attach-wait-input.exp b/gdb/testsuite/gdb.base/attach-wait-input.exp new file mode 100644 index 0000000..3120778 --- /dev/null +++ b/gdb/testsuite/gdb.base/attach-wait-input.exp @@ -0,0 +1,119 @@ +# Copyright 1997-2014 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. */ + +# Verify that GDB waits for the "attach" command to finish before +# processing the following command. +# +# GDB used to have a race where on async targets, in the small window +# between the attach request and the initial stop for the attach, GDB +# was still processing user input. +# +# The issue was originally detected with: +# +# echo -e "attach PID\nset xxx=1" | gdb +# +# In that scenario, stdin is not a tty, which disables readline. +# Explicitly turning off editing exercises the same code path, and is +# simpler to do, so we test with both editing on and off. + +# The test uses the "attach" command. +if [target_info exists use_gdb_stub] { + return +} + +standard_testfile + +if {[build_executable "failed to build" $testfile $srcfile debug]} { + return -1 +} + +# Start the program running, and return its PID, ready for attaching. + +proc start_program {binfile} { + global gdb_prompt + global decimal + + clean_restart $binfile + + if ![runto setup_done] then { + fail "Can't run to setup_done" + return 0 + } + + # Get the PID of the test process. + set testpid "" + set test "get inferior process ID" + gdb_test_multiple "p mypid" $test { + -re " = ($decimal)\r\n$gdb_prompt $" { + set testpid $expect_out(1,string) + pass $test + } + } + + gdb_test "detach" "Detaching from program: .*" + + if {$testpid == ""} { + return + } + + return $testpid +} + +# Do test proper. EDITING indicates whether "set editing" is on or +# off. + +proc test { editing } { + global gdb_prompt + global binfile + global decimal + + with_test_prefix "editing $editing" { + + set testpid [start_program $binfile] + if {$testpid == ""} { + return + } + + # Enable/disable readline. + gdb_test_no_output "set editing $editing" + + # Send both commands at once. + send_gdb "attach $testpid\nprint should_exit = 1\n" + + # Use gdb_expect directly instead of gdb_test_multiple to + # avoid races with the double prompt. + set test "attach and print" + gdb_expect { + -re "Attaching to program.*process $testpid\r\n.*$gdb_prompt.*$decimal = 1\r\n$gdb_prompt $" { + pass "$test" + } + timeout { + fail "$test (timeout)" + } + } + + # As we've used attach, on quit, we'll detach from the + # program. Explicitly kill it in case we failed above. + gdb_test "kill" \ + "" \ + "after attach, exit" \ + "Kill the program being debugged.*y or n. $" \ + "y" + } +} + +foreach editing {"on" "off"} { + test $editing +} -- 1.9.3 ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] Fix "attach" command vs user input race [Re: Regression for attach from stdin [Re: [pushed] Re: [PATCH v6 0/2] enable target-async by default]] 2014-07-04 13:48 ` [PATCH] Fix "attach" command vs user input race [Re: Regression for attach from stdin [Re: [pushed] Re: [PATCH v6 0/2] enable target-async by default]] Pedro Alves @ 2014-07-04 21:13 ` Mark Wielaard 2014-07-07 16:39 ` Doug Evans 2014-07-07 17:02 ` [PATCH] Fix "attach" command vs user input race [Re: Regression for attach from stdin [Re: [pushed] Re: [PATCH v6 0/2] enable target-async by default]] Jan Kratochvil 2 siblings, 0 replies; 42+ messages in thread From: Mark Wielaard @ 2014-07-04 21:13 UTC (permalink / raw) To: Pedro Alves; +Cc: Jan Kratochvil, gdb-patches On Fri, 2014-07-04 at 14:48 +0100, Pedro Alves wrote: > Here's a fix. Let me know whether it fixes guality testing. Yes, that patch works for me. Now I can run make check-c RUNTESTFLAGS=guality.exp in gcc without needing any guality.h workarounds. Thanks, Mark ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] Fix "attach" command vs user input race [Re: Regression for attach from stdin [Re: [pushed] Re: [PATCH v6 0/2] enable target-async by default]] 2014-07-04 13:48 ` [PATCH] Fix "attach" command vs user input race [Re: Regression for attach from stdin [Re: [pushed] Re: [PATCH v6 0/2] enable target-async by default]] Pedro Alves 2014-07-04 21:13 ` Mark Wielaard @ 2014-07-07 16:39 ` Doug Evans 2014-07-08 15:24 ` Pedro Alves 2014-07-07 17:02 ` [PATCH] Fix "attach" command vs user input race [Re: Regression for attach from stdin [Re: [pushed] Re: [PATCH v6 0/2] enable target-async by default]] Jan Kratochvil 2 siblings, 1 reply; 42+ messages in thread From: Doug Evans @ 2014-07-07 16:39 UTC (permalink / raw) To: Pedro Alves; +Cc: Mark Wielaard, Jan Kratochvil, gdb-patches Pedro Alves writes: > gdb/ > 2014-07-04 Pedro Alves <palves@redhat.com> > > * infcmd.c (attach_command_post_wait): Don't call > target_terminal_inferior here. > (attach_command): Call it here instead. > [...] > diff --git a/gdb/infcmd.c b/gdb/infcmd.c > index c4bb401..76ab12b 100644 > --- a/gdb/infcmd.c > +++ b/gdb/infcmd.c > @@ -2381,9 +2381,6 @@ attach_command_post_wait (char *args, int from_tty, int async_exec) > > post_create_inferior (¤t_target, from_tty); > > - /* Install inferior's terminal modes. */ > - target_terminal_inferior (); > - > if (async_exec) > { > /* The user requested an `attach&', so be sure to leave threads > @@ -2495,9 +2492,11 @@ attach_command (char *args, int from_tty) > shouldn't refer to attach_target again. */ > attach_target = NULL; > > - /* Set up the "saved terminal modes" of the inferior > - based on what modes we are starting it with. */ > + /* Set up the "saved terminal modes" of the inferior based on what > + modes we are starting it with, and remove stdin from the event > + loop. */ > target_terminal_init (); > + target_terminal_inferior (); > > /* Set up execution context to know that we should return from > wait_for_inferior as soon as the target reports a stop. */ Our nomenclature here is problematic. I always do a double take when I see attach and target_terminal_foo mentioned in the same sentence. Bleah. For the case at hand, and at least until we have something more readable, can I ask for a slight change here? target_terminal_inferior can do a lot more than just "remove stdin from the event loop", thus as a reader I'm left being unsure if there aren't still more bugs here. Plus, while I can see how to map the comment to the code in the patch, "Set up the "saved terminal modes" of the inferior based on what modes we are starting it with" --> target_terminal_init (); and "remove stdin from the event loop" --> target_terminal_inferior (); If I look at the result after the patch, combining the two steps into one sentence doesn't help clarify things given that target_terminal_inferior can do more than just "remove stdin from the event loop". E.g., this is a marginal improvement: /* Set up the "saved terminal modes" of the inferior based on what modes we are starting it with. */ target_terminal_init (); /* And remove stdin from the event loop. */ target_terminal_inferior (); But I'm hoping for more text in the second comment to explain things better. ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] Fix "attach" command vs user input race [Re: Regression for attach from stdin [Re: [pushed] Re: [PATCH v6 0/2] enable target-async by default]] 2014-07-07 16:39 ` Doug Evans @ 2014-07-08 15:24 ` Pedro Alves 2014-07-09 16:37 ` Doug Evans 0 siblings, 1 reply; 42+ messages in thread From: Pedro Alves @ 2014-07-08 15:24 UTC (permalink / raw) To: Doug Evans; +Cc: Mark Wielaard, Jan Kratochvil, gdb-patches On 07/07/2014 05:39 PM, Doug Evans wrote: > Our nomenclature here is problematic. I always do a double take when > I see attach and target_terminal_foo mentioned in the same sentence. > Bleah. Why? "run; detach; attach SAME_PID" is a trivial case where we end up attached to an inferior that is sharing the same tty as GDB. In fact, the test does that. > But I'm hoping for more text in the second comment to explain things > better. See updated patch and let me know what you think. FYI, I wrote a test that sends a ctrl-c right after "attach", which triggers the issue mentioned in the comment, but it also trips on a set of other races (orthogonal to sync/async), which made the test not even KFAILable, and so worthless. ------ From ef85a658ee8ff26c0b3b18db7ea7ee3da3d3650b Mon Sep 17 00:00:00 2001 From: Pedro Alves <palves@redhat.com> Date: Tue, 8 Jul 2014 13:25:58 +0100 Subject: [PATCH] Fix "attach" command vs user input race On async targets, a synchronous attach is done like this: #1 - target_attach is called (PTRACE_ATTACH is issued) #2 - a continuation is installed #3 - we go back to the event loop #4 - target reports stop (SIGSTOP), event loop wakes up, and attach continuation is called #5 - among other things, the continuation calls target_terminal_inferior, which removes stdin from the event loop Note that in #3, GDB is still processing user input. If the user is fast enough, e.g., with something like: echo -e "attach PID\nset xxx=1" | gdb ... then the "set" command is processed before the attach completes. We get worse behavior even, if input is a tty and therefore readline/editing is enabled, with e.g.,: (gdb) attach PID\nset xxx=1 we then crash readline/gdb, with: Attaching to program: attach-wait-input, process 14537 readline: readline_callback_read_char() called with no handler! Aborted $ Fix this by calling target_terminal_inferior before #3 above. The test covers both scenarios by running with editing/readline forced to both on and off. gdb/ 2014-07-04 Pedro Alves <palves@redhat.com> * infcmd.c (attach_command_post_wait): Don't call target_terminal_inferior here. (attach_command): Call it here instead. gdb/testsuite/ 2014-07-04 Pedro Alves <palves@redhat.com> * gdb.base/attach-wait-input.exp: New file. * gdb.base/attach-wait-input.c: New file. --- gdb/infcmd.c | 23 ++++-- gdb/testsuite/gdb.base/attach-wait-input.c | 40 +++++++++ gdb/testsuite/gdb.base/attach-wait-input.exp | 119 +++++++++++++++++++++++++++ 3 files changed, 177 insertions(+), 5 deletions(-) create mode 100644 gdb/testsuite/gdb.base/attach-wait-input.c create mode 100644 gdb/testsuite/gdb.base/attach-wait-input.exp diff --git a/gdb/infcmd.c b/gdb/infcmd.c index c4bb401..df7a6cd 100644 --- a/gdb/infcmd.c +++ b/gdb/infcmd.c @@ -2381,9 +2381,6 @@ attach_command_post_wait (char *args, int from_tty, int async_exec) post_create_inferior (¤t_target, from_tty); - /* Install inferior's terminal modes. */ - target_terminal_inferior (); - if (async_exec) { /* The user requested an `attach&', so be sure to leave threads @@ -2495,10 +2492,26 @@ attach_command (char *args, int from_tty) shouldn't refer to attach_target again. */ attach_target = NULL; - /* Set up the "saved terminal modes" of the inferior - based on what modes we are starting it with. */ + /* Set up the "saved terminal modes" of the inferior based on what + modes we are starting it with. */ target_terminal_init (); + /* Install inferior's terminal modes. This may look like a no-op, + as we've just saved them above, however, this does more than + restore terminal settings: + + - installs a SIGINT handler that forwards SIGINT to the inferior. + Otherwise a Ctrl-C pressed just while waiting for that initial + stop would end up as a spurious Quit. + + - removes stdin from the event loop, which we need if attaching + in the foreground, otherwise on targets that report an initial + stop on attach (which are most) we'd process input/commands + while we're in the event loop waiting for that stop. That is, + before the attach continuation runs and the command is really + finished. */ + target_terminal_inferior (); + /* Set up execution context to know that we should return from wait_for_inferior as soon as the target reports a stop. */ init_wait_for_inferior (); diff --git a/gdb/testsuite/gdb.base/attach-wait-input.c b/gdb/testsuite/gdb.base/attach-wait-input.c new file mode 100644 index 0000000..bddf2e1 --- /dev/null +++ b/gdb/testsuite/gdb.base/attach-wait-input.c @@ -0,0 +1,40 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2014 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see <http://www.gnu.org/licenses/>. */ + +#include <sys/types.h> +#include <unistd.h> + +volatile int should_exit = 0; +int mypid; + +static void +setup_done (void) +{ +} + +int +main (void) +{ + unsigned int counter = 1; + + mypid = getpid (); + setup_done (); + + for (counter = 0; !should_exit && counter < 100; counter++) + sleep (1); + return 0; +} diff --git a/gdb/testsuite/gdb.base/attach-wait-input.exp b/gdb/testsuite/gdb.base/attach-wait-input.exp new file mode 100644 index 0000000..d6d572e --- /dev/null +++ b/gdb/testsuite/gdb.base/attach-wait-input.exp @@ -0,0 +1,119 @@ +# Copyright 2014 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. */ + +# Verify that GDB waits for the "attach" command to finish before +# processing the following command. +# +# GDB used to have a race where on async targets, in the small window +# between the attach request and the initial stop for the attach, GDB +# was still processing user input. +# +# The issue was originally detected with: +# +# echo -e "attach PID\nset xxx=1" | gdb +# +# In that scenario, stdin is not a tty, which disables readline. +# Explicitly turning off editing exercises the same code path, and is +# simpler to do, so we test with both editing on and off. + +# The test uses the "attach" command. +if [target_info exists use_gdb_stub] { + return +} + +standard_testfile + +if {[build_executable "failed to build" $testfile $srcfile debug]} { + return -1 +} + +# Start the program running, and return its PID, ready for attaching. + +proc start_program {binfile} { + global gdb_prompt + global decimal + + clean_restart $binfile + + if ![runto setup_done] then { + fail "Can't run to setup_done" + return 0 + } + + # Get the PID of the test process. + set testpid "" + set test "get inferior process ID" + gdb_test_multiple "p mypid" $test { + -re " = ($decimal)\r\n$gdb_prompt $" { + set testpid $expect_out(1,string) + pass $test + } + } + + gdb_test "detach" "Detaching from program: .*" + + if {$testpid == ""} { + return + } + + return $testpid +} + +# Do test proper. EDITING indicates whether "set editing" is on or +# off. + +proc test { editing } { + global gdb_prompt + global binfile + global decimal + + with_test_prefix "editing $editing" { + + set testpid [start_program $binfile] + if {$testpid == ""} { + return + } + + # Enable/disable readline. + gdb_test_no_output "set editing $editing" + + # Send both commands at once. + send_gdb "attach $testpid\nprint should_exit = 1\n" + + # Use gdb_expect directly instead of gdb_test_multiple to + # avoid races with the double prompt. + set test "attach and print" + gdb_expect { + -re "Attaching to program.*process $testpid\r\n.*$gdb_prompt.*$decimal = 1\r\n$gdb_prompt $" { + pass "$test" + } + timeout { + fail "$test (timeout)" + } + } + + # As we've used attach, on quit, we'll detach from the + # program. Explicitly kill it in case we failed above. + gdb_test "kill" \ + "" \ + "after attach, exit" \ + "Kill the program being debugged.*y or n. $" \ + "y" + } +} + +foreach editing {"on" "off"} { + test $editing +} -- 1.9.3 ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] Fix "attach" command vs user input race [Re: Regression for attach from stdin [Re: [pushed] Re: [PATCH v6 0/2] enable target-async by default]] 2014-07-08 15:24 ` Pedro Alves @ 2014-07-09 16:37 ` Doug Evans 2014-07-09 17:09 ` [pushed+7.8] " Pedro Alves 0 siblings, 1 reply; 42+ messages in thread From: Doug Evans @ 2014-07-09 16:37 UTC (permalink / raw) To: Pedro Alves; +Cc: Mark Wielaard, Jan Kratochvil, gdb-patches Pedro Alves writes: > On 07/07/2014 05:39 PM, Doug Evans wrote: > > > Our nomenclature here is problematic. I always do a double take when > > I see attach and target_terminal_foo mentioned in the same sentence. > > Bleah. > > Why? "run; detach; attach SAME_PID" is a trivial case where we end > up attached to an inferior that is sharing the same tty as GDB. In > fact, the test does that. I know. But for me it's not the normal case. I wish the naming/wording could better reflect what's happening. [but that's not something that has to be addressed here of course] > @@ -2495,10 +2492,26 @@ attach_command (char *args, int from_tty) > shouldn't refer to attach_target again. */ > attach_target = NULL; > > - /* Set up the "saved terminal modes" of the inferior > - based on what modes we are starting it with. */ > + /* Set up the "saved terminal modes" of the inferior based on what > + modes we are starting it with. */ > target_terminal_init (); spurious change > > + /* Install inferior's terminal modes. This may look like a no-op, > + as we've just saved them above, however, this does more than > + restore terminal settings: > + > + - installs a SIGINT handler that forwards SIGINT to the inferior. > + Otherwise a Ctrl-C pressed just while waiting for that initial > + stop would end up as a spurious Quit. > + > + - removes stdin from the event loop, which we need if attaching > + in the foreground, otherwise on targets that report an initial > + stop on attach (which are most) we'd process input/commands > + while we're in the event loop waiting for that stop. That is, > + before the attach continuation runs and the command is really > + finished. */ > + target_terminal_inferior (); > + > /* Set up execution context to know that we should return from > wait_for_inferior as soon as the target reports a stop. */ > init_wait_for_inferior (); I like this a lot better. Thanks. The patch is ok with me, modulo removing the spurious change. ^ permalink raw reply [flat|nested] 42+ messages in thread
* [pushed+7.8] Re: [PATCH] Fix "attach" command vs user input race [Re: Regression for attach from stdin [Re: [pushed] Re: [PATCH v6 0/2] enable target-async by default]] 2014-07-09 16:37 ` Doug Evans @ 2014-07-09 17:09 ` Pedro Alves 2014-07-29 22:03 ` Doug Evans 2014-09-03 7:59 ` Regression: GDB stopped on run with attached process (PR 17347) [Re: [pushed+7.8] Re: [PATCH] Fix "attach" command vs user input race [Re: Regression for attach from stdin [Re: [pushed] Re: [PATCH v6 0/2] enable target-async by default]]] Jan Kratochvil 0 siblings, 2 replies; 42+ messages in thread From: Pedro Alves @ 2014-07-09 17:09 UTC (permalink / raw) To: Doug Evans; +Cc: Mark Wielaard, Jan Kratochvil, gdb-patches On 07/09/2014 05:37 PM, Doug Evans wrote: > spurious change Fixed. > I like this a lot better. Thanks. > The patch is ok with me, modulo removing the spurious change. Here's what I pushed to both master and gdb-7.8-branch. Thanks. ------------- From 1fe2833b6dd03602ba86aa334e81466ea9abe66a Mon Sep 17 00:00:00 2001 From: Pedro Alves <palves@redhat.com> Date: Wed, 9 Jul 2014 17:52:58 +0100 Subject: [PATCH] Fix "attach" command vs user input race On async targets, a synchronous attach is done like this: #1 - target_attach is called (PTRACE_ATTACH is issued) #2 - a continuation is installed #3 - we go back to the event loop #4 - target reports stop (SIGSTOP), event loop wakes up, and attach continuation is called #5 - among other things, the continuation calls target_terminal_inferior, which removes stdin from the event loop Note that in #3, GDB is still processing user input. If the user is fast enough, e.g., with something like: echo -e "attach PID\nset xxx=1" | gdb ... then the "set" command is processed before the attach completes. We get worse behavior even, if input is a tty and therefore readline/editing is enabled, with e.g.,: (gdb) attach PID\nset xxx=1 we then crash readline/gdb, with: Attaching to program: attach-wait-input, process 14537 readline: readline_callback_read_char() called with no handler! Aborted $ Fix this by calling target_terminal_inferior before #3 above. The test covers both scenarios by running with editing/readline forced to both on and off. gdb/ 2014-07-09 Pedro Alves <palves@redhat.com> * infcmd.c (attach_command_post_wait): Don't call target_terminal_inferior here. (attach_command): Call it here instead. gdb/testsuite/ 2014-07-09 Pedro Alves <palves@redhat.com> * gdb.base/attach-wait-input.exp: New file. * gdb.base/attach-wait-input.c: New file. --- gdb/ChangeLog | 6 ++ gdb/testsuite/ChangeLog | 5 ++ gdb/infcmd.c | 19 ++++- gdb/testsuite/gdb.base/attach-wait-input.c | 40 +++++++++ gdb/testsuite/gdb.base/attach-wait-input.exp | 119 +++++++++++++++++++++++++++ 5 files changed, 186 insertions(+), 3 deletions(-) create mode 100644 gdb/testsuite/gdb.base/attach-wait-input.c create mode 100644 gdb/testsuite/gdb.base/attach-wait-input.exp diff --git a/gdb/ChangeLog b/gdb/ChangeLog index d4304e6..1bd19fc 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,9 @@ +2014-07-09 Pedro Alves <palves@redhat.com> + + * infcmd.c (attach_command_post_wait): Don't call + target_terminal_inferior here. + (attach_command): Call it here instead. + 2014-07-07 Pedro Alves <palves@redhat.com> PR gdb/17096 diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 7667969..66dcc95 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2014-07-09 Pedro Alves <palves@redhat.com> + + * gdb.base/attach-wait-input.exp: New file. + * gdb.base/attach-wait-input.c: New file. + 2014-06-23 Siva Chandra Reddy <sivachandra@google.com> * gdb.python/py-xmethods.exp: Use "progspace" instead of the diff --git a/gdb/infcmd.c b/gdb/infcmd.c index df4fd40..a712b6f 100644 --- a/gdb/infcmd.c +++ b/gdb/infcmd.c @@ -2381,9 +2381,6 @@ attach_command_post_wait (char *args, int from_tty, int async_exec) post_create_inferior (¤t_target, from_tty); - /* Install inferior's terminal modes. */ - target_terminal_inferior (); - if (async_exec) { /* The user requested an `attach&', so be sure to leave threads @@ -2499,6 +2496,22 @@ attach_command (char *args, int from_tty) based on what modes we are starting it with. */ target_terminal_init (); + /* Install inferior's terminal modes. This may look like a no-op, + as we've just saved them above, however, this does more than + restore terminal settings: + + - installs a SIGINT handler that forwards SIGINT to the inferior. + Otherwise a Ctrl-C pressed just while waiting for the initial + stop would end up as a spurious Quit. + + - removes stdin from the event loop, which we need if attaching + in the foreground, otherwise on targets that report an initial + stop on attach (which are most) we'd process input/commands + while we're in the event loop waiting for that stop. That is, + before the attach continuation runs and the command is really + finished. */ + target_terminal_inferior (); + /* Set up execution context to know that we should return from wait_for_inferior as soon as the target reports a stop. */ init_wait_for_inferior (); diff --git a/gdb/testsuite/gdb.base/attach-wait-input.c b/gdb/testsuite/gdb.base/attach-wait-input.c new file mode 100644 index 0000000..bddf2e1 --- /dev/null +++ b/gdb/testsuite/gdb.base/attach-wait-input.c @@ -0,0 +1,40 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2014 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see <http://www.gnu.org/licenses/>. */ + +#include <sys/types.h> +#include <unistd.h> + +volatile int should_exit = 0; +int mypid; + +static void +setup_done (void) +{ +} + +int +main (void) +{ + unsigned int counter = 1; + + mypid = getpid (); + setup_done (); + + for (counter = 0; !should_exit && counter < 100; counter++) + sleep (1); + return 0; +} diff --git a/gdb/testsuite/gdb.base/attach-wait-input.exp b/gdb/testsuite/gdb.base/attach-wait-input.exp new file mode 100644 index 0000000..d6d572e --- /dev/null +++ b/gdb/testsuite/gdb.base/attach-wait-input.exp @@ -0,0 +1,119 @@ +# Copyright 2014 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. */ + +# Verify that GDB waits for the "attach" command to finish before +# processing the following command. +# +# GDB used to have a race where on async targets, in the small window +# between the attach request and the initial stop for the attach, GDB +# was still processing user input. +# +# The issue was originally detected with: +# +# echo -e "attach PID\nset xxx=1" | gdb +# +# In that scenario, stdin is not a tty, which disables readline. +# Explicitly turning off editing exercises the same code path, and is +# simpler to do, so we test with both editing on and off. + +# The test uses the "attach" command. +if [target_info exists use_gdb_stub] { + return +} + +standard_testfile + +if {[build_executable "failed to build" $testfile $srcfile debug]} { + return -1 +} + +# Start the program running, and return its PID, ready for attaching. + +proc start_program {binfile} { + global gdb_prompt + global decimal + + clean_restart $binfile + + if ![runto setup_done] then { + fail "Can't run to setup_done" + return 0 + } + + # Get the PID of the test process. + set testpid "" + set test "get inferior process ID" + gdb_test_multiple "p mypid" $test { + -re " = ($decimal)\r\n$gdb_prompt $" { + set testpid $expect_out(1,string) + pass $test + } + } + + gdb_test "detach" "Detaching from program: .*" + + if {$testpid == ""} { + return + } + + return $testpid +} + +# Do test proper. EDITING indicates whether "set editing" is on or +# off. + +proc test { editing } { + global gdb_prompt + global binfile + global decimal + + with_test_prefix "editing $editing" { + + set testpid [start_program $binfile] + if {$testpid == ""} { + return + } + + # Enable/disable readline. + gdb_test_no_output "set editing $editing" + + # Send both commands at once. + send_gdb "attach $testpid\nprint should_exit = 1\n" + + # Use gdb_expect directly instead of gdb_test_multiple to + # avoid races with the double prompt. + set test "attach and print" + gdb_expect { + -re "Attaching to program.*process $testpid\r\n.*$gdb_prompt.*$decimal = 1\r\n$gdb_prompt $" { + pass "$test" + } + timeout { + fail "$test (timeout)" + } + } + + # As we've used attach, on quit, we'll detach from the + # program. Explicitly kill it in case we failed above. + gdb_test "kill" \ + "" \ + "after attach, exit" \ + "Kill the program being debugged.*y or n. $" \ + "y" + } +} + +foreach editing {"on" "off"} { + test $editing +} -- 1.9.3 ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [pushed+7.8] Re: [PATCH] Fix "attach" command vs user input race [Re: Regression for attach from stdin [Re: [pushed] Re: [PATCH v6 0/2] enable target-async by default]] 2014-07-09 17:09 ` [pushed+7.8] " Pedro Alves @ 2014-07-29 22:03 ` Doug Evans 2014-07-29 23:10 ` Doug Evans 2014-07-30 12:38 ` Pedro Alves 2014-09-03 7:59 ` Regression: GDB stopped on run with attached process (PR 17347) [Re: [pushed+7.8] Re: [PATCH] Fix "attach" command vs user input race [Re: Regression for attach from stdin [Re: [pushed] Re: [PATCH v6 0/2] enable target-async by default]]] Jan Kratochvil 1 sibling, 2 replies; 42+ messages in thread From: Doug Evans @ 2014-07-29 22:03 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches On Wed, Jul 9, 2014 at 10:09 AM, Pedro Alves <palves@redhat.com> wrote: > On 07/09/2014 05:37 PM, Doug Evans wrote: > >> spurious change > > Fixed. > >> I like this a lot better. Thanks. >> The patch is ok with me, modulo removing the spurious change. > > Here's what I pushed to both master and gdb-7.8-branch. > > Thanks. > > ------------- > From 1fe2833b6dd03602ba86aa334e81466ea9abe66a Mon Sep 17 00:00:00 2001 > From: Pedro Alves <palves@redhat.com> > Date: Wed, 9 Jul 2014 17:52:58 +0100 > Subject: [PATCH] Fix "attach" command vs user input race > > On async targets, a synchronous attach is done like this: > > #1 - target_attach is called (PTRACE_ATTACH is issued) > #2 - a continuation is installed > #3 - we go back to the event loop > #4 - target reports stop (SIGSTOP), event loop wakes up, and > attach continuation is called > #5 - among other things, the continuation calls > target_terminal_inferior, which removes stdin from the event > loop > > Note that in #3, GDB is still processing user input. If the user is > fast enough, e.g., with something like: > > echo -e "attach PID\nset xxx=1" | gdb > > ... then the "set" command is processed before the attach completes. > > We get worse behavior even, if input is a tty and therefore > readline/editing is enabled, with e.g.,: > > (gdb) attach PID\nset xxx=1 > > we then crash readline/gdb, with: > > Attaching to program: attach-wait-input, process 14537 > readline: readline_callback_read_char() called with no handler! > Aborted > $ > > Fix this by calling target_terminal_inferior before #3 above. > > The test covers both scenarios by running with editing/readline forced > to both on and off. > > gdb/ > 2014-07-09 Pedro Alves <palves@redhat.com> > > * infcmd.c (attach_command_post_wait): Don't call > target_terminal_inferior here. > (attach_command): Call it here instead. > > gdb/testsuite/ > 2014-07-09 Pedro Alves <palves@redhat.com> > > * gdb.base/attach-wait-input.exp: New file. > * gdb.base/attach-wait-input.c: New file. Hi. Is this TODO still needed after this patch? infcmd.c: /* * TODO: * Should save/restore the tty state since it might be that the * program to be debugged was started on this tty and it wants * the tty in some state other than what we want. If it's running * on another terminal or without a terminal, then saving and * restoring the tty state is a harmless no-op. * This only needs to be done if we are attaching to a process. */ ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [pushed+7.8] Re: [PATCH] Fix "attach" command vs user input race [Re: Regression for attach from stdin [Re: [pushed] Re: [PATCH v6 0/2] enable target-async by default]] 2014-07-29 22:03 ` Doug Evans @ 2014-07-29 23:10 ` Doug Evans 2014-07-30 12:46 ` Pedro Alves 2014-07-30 12:38 ` Pedro Alves 1 sibling, 1 reply; 42+ messages in thread From: Doug Evans @ 2014-07-29 23:10 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches On Tue, Jul 29, 2014 at 2:48 PM, Doug Evans <dje@google.com> wrote: > On Wed, Jul 9, 2014 at 10:09 AM, Pedro Alves <palves@redhat.com> wrote: >> On 07/09/2014 05:37 PM, Doug Evans wrote: >> >>> spurious change >> >> Fixed. >> >>> I like this a lot better. Thanks. >>> The patch is ok with me, modulo removing the spurious change. >> >> Here's what I pushed to both master and gdb-7.8-branch. >> >> Thanks. >> >> ------------- >> From 1fe2833b6dd03602ba86aa334e81466ea9abe66a Mon Sep 17 00:00:00 2001 >> From: Pedro Alves <palves@redhat.com> >> Date: Wed, 9 Jul 2014 17:52:58 +0100 >> Subject: [PATCH] Fix "attach" command vs user input race >> >> On async targets, a synchronous attach is done like this: >> >> #1 - target_attach is called (PTRACE_ATTACH is issued) >> #2 - a continuation is installed >> #3 - we go back to the event loop >> #4 - target reports stop (SIGSTOP), event loop wakes up, and >> attach continuation is called >> #5 - among other things, the continuation calls >> target_terminal_inferior, which removes stdin from the event >> loop >> >> Note that in #3, GDB is still processing user input. If the user is >> fast enough, e.g., with something like: >> >> echo -e "attach PID\nset xxx=1" | gdb >> >> ... then the "set" command is processed before the attach completes. >> >> We get worse behavior even, if input is a tty and therefore >> readline/editing is enabled, with e.g.,: >> >> (gdb) attach PID\nset xxx=1 >> >> we then crash readline/gdb, with: >> >> Attaching to program: attach-wait-input, process 14537 >> readline: readline_callback_read_char() called with no handler! >> Aborted >> $ >> >> Fix this by calling target_terminal_inferior before #3 above. >> >> The test covers both scenarios by running with editing/readline forced >> to both on and off. >> >> gdb/ >> 2014-07-09 Pedro Alves <palves@redhat.com> >> >> * infcmd.c (attach_command_post_wait): Don't call >> target_terminal_inferior here. >> (attach_command): Call it here instead. >> >> gdb/testsuite/ >> 2014-07-09 Pedro Alves <palves@redhat.com> >> >> * gdb.base/attach-wait-input.exp: New file. >> * gdb.base/attach-wait-input.c: New file. > > Hi. > > Is this TODO still needed after this patch? > > infcmd.c: > > /* > * TODO: > * Should save/restore the tty state since it might be that the > * program to be debugged was started on this tty and it wants > * the tty in some state other than what we want. If it's running > * on another terminal or without a terminal, then saving and > * restoring the tty state is a harmless no-op. > * This only needs to be done if we are attaching to a process. > */ A related issue (or the same one if one prefers): post_create_inferior does this: /* Be sure we own the terminal in case write operations are performed. */ target_terminal_ours (); but post_create_inferior is called *after* target_post_attach in attach_command_post_wait: /* Take any necessary post-attaching actions for this platform. */ target_post_attach (ptid_get_pid (inferior_ptid)); post_create_inferior (¤t_target, from_tty); What if target_post_attach does some writes? Seems like it can, e.g., some of the ptrace checking stuff may print a warning. Plus attach_command_post_wait calls some other stuff before post_create_inferior that could cause some writes to the terminal. Question: Is there a specific terminal state that needs to be in effect when attach_command_post_wait returns? Or can we just call target_terminal_ours at its start? [and leave it to other code to switch back to target_terminal_inferior as needed - e.g. proceed calls resume which will call target_terminal_inferior] ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [pushed+7.8] Re: [PATCH] Fix "attach" command vs user input race [Re: Regression for attach from stdin [Re: [pushed] Re: [PATCH v6 0/2] enable target-async by default]] 2014-07-29 23:10 ` Doug Evans @ 2014-07-30 12:46 ` Pedro Alves 0 siblings, 0 replies; 42+ messages in thread From: Pedro Alves @ 2014-07-30 12:46 UTC (permalink / raw) To: Doug Evans; +Cc: gdb-patches On 07/29/2014 11:03 PM, Doug Evans wrote: > A related issue (or the same one if one prefers): > > post_create_inferior does this: > > /* Be sure we own the terminal in case write operations are performed. */ > target_terminal_ours (); Yeah. As I mentioned before, when I wrote a test that sends a ctrl-c right after "attach", which triggered the issue mentioned in the new comment, - installs a SIGINT handler that forwards SIGINT to the inferior. Otherwise a Ctrl-C pressed just while waiting for the initial stop would end up as a spurious Quit. it tripped on a set of other races. This call was one of them. We should actually be calling target_terminal_ours_for_output, not target_terminal_ours, so that stdin/ctrl-c is still redirected to the inferior. As is, IIRC, if a ctrl-c arrives after than target_terminal_ours and before the next target_terminal_inferior, we end up in the prompt, with a Quit, but with the target still running. I gave up fixing those at the time, as they're orthogonal to sync/async. > > but post_create_inferior is called *after* target_post_attach > in attach_command_post_wait: > > /* Take any necessary post-attaching actions for this platform. */ > target_post_attach (ptid_get_pid (inferior_ptid)); > > post_create_inferior (¤t_target, from_tty); > > What if target_post_attach does some writes? Something should call target_terminal_ours_for_output before. > Seems like it can, e.g., some of the ptrace checking stuff may print a warning. > Plus attach_command_post_wait calls some other stuff before > post_create_inferior that could cause some writes to the terminal. > > Question: Is there a specific terminal state that needs to be in > effect when attach_command_post_wait returns? > Or can we just call target_terminal_ours at its start? I think we can. > [and leave it to other code to switch back to target_terminal_inferior > as needed - e.g. proceed calls resume which will call > target_terminal_inferior] Even with that, it'll still be possible to have a ctrl-c pressed after to_attach is called, and before target_terminal_inferior is called (and therefore set_sigint_trap is called, which sets up ctrl-c forwarding). When I last looked at this, it seemed to be that ideally, we should only have a single SIGINT handler, which just records that a ctrl-c was pressed, and then the event loop decides whether a ctrl-c is a Quit or whether it should be forwarded to the inferior, depending on whether the target is running on the foreground, or not. -- Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [pushed+7.8] Re: [PATCH] Fix "attach" command vs user input race [Re: Regression for attach from stdin [Re: [pushed] Re: [PATCH v6 0/2] enable target-async by default]] 2014-07-29 22:03 ` Doug Evans 2014-07-29 23:10 ` Doug Evans @ 2014-07-30 12:38 ` Pedro Alves 2014-07-30 16:59 ` Doug Evans 1 sibling, 1 reply; 42+ messages in thread From: Pedro Alves @ 2014-07-30 12:38 UTC (permalink / raw) To: Doug Evans; +Cc: gdb-patches On 07/29/2014 10:48 PM, Doug Evans wrote: > On Wed, Jul 9, 2014 at 10:09 AM, Pedro Alves <palves@redhat.com> wrote: >> On 07/09/2014 05:37 PM, Doug Evans wrote: >> >>> spurious change >> >> Fixed. >> >>> I like this a lot better. Thanks. >>> The patch is ok with me, modulo removing the spurious change. >> >> Here's what I pushed to both master and gdb-7.8-branch. >> >> Thanks. >> >> ------------- >> From 1fe2833b6dd03602ba86aa334e81466ea9abe66a Mon Sep 17 00:00:00 2001 >> From: Pedro Alves <palves@redhat.com> >> Date: Wed, 9 Jul 2014 17:52:58 +0100 >> Subject: [PATCH] Fix "attach" command vs user input race >> >> On async targets, a synchronous attach is done like this: >> >> #1 - target_attach is called (PTRACE_ATTACH is issued) >> #2 - a continuation is installed >> #3 - we go back to the event loop >> #4 - target reports stop (SIGSTOP), event loop wakes up, and >> attach continuation is called >> #5 - among other things, the continuation calls >> target_terminal_inferior, which removes stdin from the event >> loop >> >> Note that in #3, GDB is still processing user input. If the user is >> fast enough, e.g., with something like: >> >> echo -e "attach PID\nset xxx=1" | gdb >> >> ... then the "set" command is processed before the attach completes. >> >> We get worse behavior even, if input is a tty and therefore >> readline/editing is enabled, with e.g.,: >> >> (gdb) attach PID\nset xxx=1 >> >> we then crash readline/gdb, with: >> >> Attaching to program: attach-wait-input, process 14537 >> readline: readline_callback_read_char() called with no handler! >> Aborted >> $ >> >> Fix this by calling target_terminal_inferior before #3 above. >> >> The test covers both scenarios by running with editing/readline forced >> to both on and off. >> >> gdb/ >> 2014-07-09 Pedro Alves <palves@redhat.com> >> >> * infcmd.c (attach_command_post_wait): Don't call >> target_terminal_inferior here. >> (attach_command): Call it here instead. >> >> gdb/testsuite/ >> 2014-07-09 Pedro Alves <palves@redhat.com> >> >> * gdb.base/attach-wait-input.exp: New file. >> * gdb.base/attach-wait-input.c: New file. > > Hi. > > Is this TODO still needed after this patch? > > infcmd.c: > > /* > * TODO: > * Should save/restore the tty state since it might be that the > * program to be debugged was started on this tty and it wants > * the tty in some state other than what we want. If it's running > * on another terminal or without a terminal, then saving and > * restoring the tty state is a harmless no-op. > * This only needs to be done if we are attaching to a process. > */ > As usual, git blame/log is your friend... That's been in place for over 20 years. In bd5635a1 (1991), we see: ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /* * TODO: * Should save/restore the tty state since it might be that the * program to be debugged was started on this tty and it wants * the tty in some state other than what we want. If it's running * on another terminal or without a terminal, then saving and * restoring the tty state is a harmless no-op. * This only needs to be done if we are attaching to a process. */ /* * attach_command -- * takes a program started up outside of gdb and ``attaches'' to it. * This stops it cold in its tracks and allows us to start tracing it. * For this to work, we must be able to send the process a * signal and we must have the same effective uid as the program. */ void attach_command (args, from_tty) char *args; int from_tty; { target_attach (args, from_tty); } ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ So clearly the TODO has been stale for a long while. We've been saving/restoring the tty state way before my patch. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [pushed+7.8] Re: [PATCH] Fix "attach" command vs user input race [Re: Regression for attach from stdin [Re: [pushed] Re: [PATCH v6 0/2] enable target-async by default]] 2014-07-30 12:38 ` Pedro Alves @ 2014-07-30 16:59 ` Doug Evans 2014-08-21 16:34 ` [PUSHED] infcmd.c: Remove stale TODO Pedro Alves 0 siblings, 1 reply; 42+ messages in thread From: Doug Evans @ 2014-07-30 16:59 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches On Wed, Jul 30, 2014 at 5:05 AM, Pedro Alves <palves@redhat.com> wrote: >>> [...] >>> gdb/ >>> 2014-07-09 Pedro Alves <palves@redhat.com> >>> >>> * infcmd.c (attach_command_post_wait): Don't call >>> target_terminal_inferior here. >>> (attach_command): Call it here instead. >>> >>> gdb/testsuite/ >>> 2014-07-09 Pedro Alves <palves@redhat.com> >>> >>> * gdb.base/attach-wait-input.exp: New file. >>> * gdb.base/attach-wait-input.c: New file. >> >> Hi. >> >> Is this TODO still needed after this patch? >> >> infcmd.c: >> >> /* >> * TODO: >> * Should save/restore the tty state since it might be that the >> * program to be debugged was started on this tty and it wants >> * the tty in some state other than what we want. If it's running >> * on another terminal or without a terminal, then saving and >> * restoring the tty state is a harmless no-op. >> * This only needs to be done if we are attaching to a process. >> */ >> > > As usual, git blame/log is your friend... > > That's been in place for over 20 years. In bd5635a1 (1991), we see: > > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > /* > * TODO: > * Should save/restore the tty state since it might be that the > * program to be debugged was started on this tty and it wants > * the tty in some state other than what we want. If it's running > * on another terminal or without a terminal, then saving and > * restoring the tty state is a harmless no-op. > * This only needs to be done if we are attaching to a process. > */ > > /* > * attach_command -- > * takes a program started up outside of gdb and ``attaches'' to it. > * This stops it cold in its tracks and allows us to start tracing it. > * For this to work, we must be able to send the process a > * signal and we must have the same effective uid as the program. > */ > void > attach_command (args, from_tty) > char *args; > int from_tty; > { > target_attach (args, from_tty); > } > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > > So clearly the TODO has been stale for a long while. 23 years? Zoinks! :) > We've been saving/restoring the tty state way before > my patch. > > Thanks, > Pedro Alves > It wasn't clear to me whether the comment was (trying to) refer to something more specific for the task at hand. Just being too literal I guess. Thanks. ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PUSHED] infcmd.c: Remove stale TODO 2014-07-30 16:59 ` Doug Evans @ 2014-08-21 16:34 ` Pedro Alves 0 siblings, 0 replies; 42+ messages in thread From: Pedro Alves @ 2014-08-21 16:34 UTC (permalink / raw) To: gdb-patches; +Cc: Doug Evans This TODO has been stale for over 2 years. In bd5635a1 (1991), we already see the comment, when we only had a bare attach_command: ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /* * TODO: * Should save/restore the tty state since it might be that the * program to be debugged was started on this tty and it wants * the tty in some state other than what we want. If it's running * on another terminal or without a terminal, then saving and * restoring the tty state is a harmless no-op. * This only needs to be done if we are attaching to a process. */ /* * attach_command -- * takes a program started up outside of gdb and ``attaches'' to it. * This stops it cold in its tracks and allows us to start tracing it. * For this to work, we must be able to send the process a * signal and we must have the same effective uid as the program. */ void attach_command (args, from_tty) char *args; int from_tty; { target_attach (args, from_tty); } ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Later in b5a3d2aa (1992) target_terminal_init, etc. calls are added to attach_command, and in 7e97eb28 (1992) we see: + /* If we attached to the process, we might or might not be sharing + a terminal. Avoid printing error msg if we are unable to set our + terminal's process group to his process group ID. */ + if (!attach_flag) { + OOPSY ("ioctl TIOCSPGRP"); Clearly the TODO has been stale for a long while. I considered preserving the text elsewhere, but then thought the comments in inflow.c already have all the necessary info. gdb/ChangeLog: * infcmd.c (attach_command): Remove comment. --- gdb/ChangeLog | 4 ++++ gdb/infcmd.c | 10 ---------- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index fae78e5..bf7f618 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,7 @@ +2014-08-21 Pedro Alves <palves@redhat.com> + + * infcmd.c (attach_command): Remove comment. + 2014-08-21 Bin Cheng <bin.cheng@arm.com> * aarch64-linux-nat.c (dr_changed_t): Change the type from diff --git a/gdb/infcmd.c b/gdb/infcmd.c index bc42cea..b6aba04 100644 --- a/gdb/infcmd.c +++ b/gdb/infcmd.c @@ -2365,16 +2365,6 @@ proceed_after_attach (int pid) do_cleanups (old_chain); } -/* - * TODO: - * Should save/restore the tty state since it might be that the - * program to be debugged was started on this tty and it wants - * the tty in some state other than what we want. If it's running - * on another terminal or without a terminal, then saving and - * restoring the tty state is a harmless no-op. - * This only needs to be done if we are attaching to a process. - */ - /* attach_command -- takes a program started up outside of gdb and ``attaches'' to it. This stops it cold in its tracks and allows us to start debugging it. -- 1.9.3 ^ permalink raw reply [flat|nested] 42+ messages in thread
* Regression: GDB stopped on run with attached process (PR 17347) [Re: [pushed+7.8] Re: [PATCH] Fix "attach" command vs user input race [Re: Regression for attach from stdin [Re: [pushed] Re: [PATCH v6 0/2] enable target-async by default]]] 2014-07-09 17:09 ` [pushed+7.8] " Pedro Alves 2014-07-29 22:03 ` Doug Evans @ 2014-09-03 7:59 ` Jan Kratochvil 2014-09-03 20:11 ` Regression: GDB stopped on run with attached process (PR 17347) [Re: [pushed+7.8] Re: [PATCH] Fix "attach" command vs user input race Pedro Alves 1 sibling, 1 reply; 42+ messages in thread From: Jan Kratochvil @ 2014-09-03 7:59 UTC (permalink / raw) To: Pedro Alves; +Cc: Doug Evans, Mark Wielaard, gdb-patches On Wed, 09 Jul 2014 19:09:29 +0200, Pedro Alves wrote: > Here's what I pushed to both master and gdb-7.8-branch. https://sourceware.org/bugzilla/show_bug.cgi?id=17347 sleep 1h&p=$!;sleep 0.1;gdb -batch sleep $p -ex r PASS: [2] 22970 0x0000003fdf8bc780 in __nanosleep_nocancel () at ../sysdeps/unix/syscall-template.S:81 81 T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS) /usr/bin/sleep: missing operand Try '/usr/bin/sleep --help' for more information. [Inferior 1 (process 23030) exited with code 01] [2]+ Killed sleep 1h FAIL: [2] Killed sleep 1h [3]+ Stopped ./gdb/gdb -batch sleep $! -ex r 7180e04a36d812bbea2c280f2db33a7e8ce6b07b is the first bad commit commit 7180e04a36d812bbea2c280f2db33a7e8ce6b07b Author: Pedro Alves <palves@redhat.com> Date: Wed Jul 9 15:59:02 2014 +0100 Fix "attach" command vs user input race private Red Hat reference: https://bugzilla.redhat.com/show_bug.cgi?id=1136704 ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: Regression: GDB stopped on run with attached process (PR 17347) [Re: [pushed+7.8] Re: [PATCH] Fix "attach" command vs user input race 2014-09-03 7:59 ` Regression: GDB stopped on run with attached process (PR 17347) [Re: [pushed+7.8] Re: [PATCH] Fix "attach" command vs user input race [Re: Regression for attach from stdin [Re: [pushed] Re: [PATCH v6 0/2] enable target-async by default]]] Jan Kratochvil @ 2014-09-03 20:11 ` Pedro Alves 2014-09-07 19:28 ` Jan Kratochvil 0 siblings, 1 reply; 42+ messages in thread From: Pedro Alves @ 2014-09-03 20:11 UTC (permalink / raw) To: Jan Kratochvil; +Cc: Doug Evans, Mark Wielaard, gdb-patches On 09/03/2014 08:58 AM, Jan Kratochvil wrote: > https://sourceware.org/bugzilla/show_bug.cgi?id=17347 Thanks Jan. Here's a fix, test included. Comments? Thanks, Pedro Alves -------------------------- [PATCH] gdb/17347 - Regression: GDB stopped on run with attached process Doing: gdb --pid=PID -ex run Results in GDB getting a SIGTTIN, and thus ending stopped. That's usually indicative of a missing target_terminal_ours call. E.g., from the PR: $ sleep 1h & p=$!; sleep 0.1; gdb -batch sleep $p -ex run [1] 28263 [1] Killed sleep 1h [2]+ Stopped gdb -batch sleep $p -ex run The workaround is doing: gdb -ex "attach $PID" -ex "run" instead of gdb [-p] $PID -ex "run" With the former, gdb waits for the attach command to complete before moving on to the "run" command, because the interpreter is in sync mode at this point, within execute_command. But for the latter, attach_command is called directly from captured_main, and thus misses that waiting. IOW, "run" is running before the attach continuation has run, before the program stops and attach completes. The broken terminal settings are just one symptom of that. Any command that queries or requires input results in the same. The fix is to wait in catch_command_errors (which is specific to main.c nowadays), just like we wait in execute_command. gdb/ChangeLog: 2014-09-03 Pedro Alves <palves@redhat.com> PR gdb/17347 * main.c: Include "infrun.h". (catch_command_errors, catch_command_errors_const): Wait for the foreground command to complete. * top.c (maybe_wait_sync_command_done): New function, factored out from ... (maybe_wait_sync_command_done): ... here. * top.h (maybe_wait_sync_command_done): New declaration. gdb/testsuite/ChangeLog: 2014-09-03 Pedro Alves <palves@redhat.com> PR gdb/17347 * gdb.base/attach.exp (spawn_test_prog): New, factored out from ... (do_attach_tests, do_call_attach_tests, do_command_attach_tests): ... here. (gdb_spawn_with_cmdline_opts): New procedure. (test_command_line_attach_run): New procedure. (top level): Call it. --- gdb/main.c | 9 +++ gdb/testsuite/gdb.base/attach.exp | 118 ++++++++++++++++++++++++++------------ gdb/top.c | 26 +++++---- gdb/top.h | 8 +++ 4 files changed, 114 insertions(+), 47 deletions(-) diff --git a/gdb/main.c b/gdb/main.c index 12f0146..756dd4f 100644 --- a/gdb/main.c +++ b/gdb/main.c @@ -45,6 +45,7 @@ #include "filestuff.h" #include <signal.h> #include "event-top.h" +#include "infrun.h" /* The selected interpreter. This will be used as a set command variable, so it should always be malloc'ed - since @@ -369,7 +370,11 @@ catch_command_errors (catch_command_errors_ftype *command, TRY_CATCH (e, mask) { + int was_sync = sync_execution; + command (arg, from_tty); + + maybe_wait_sync_command_done (was_sync); } return handle_command_errors (e); } @@ -388,7 +393,11 @@ catch_command_errors_const (catch_command_errors_const_ftype *command, TRY_CATCH (e, mask) { + int was_sync = sync_execution; + command (arg, from_tty); + + maybe_wait_sync_command_done (was_sync); } return handle_command_errors (e); } diff --git a/gdb/testsuite/gdb.base/attach.exp b/gdb/testsuite/gdb.base/attach.exp index 9714c29..c94c127 100644 --- a/gdb/testsuite/gdb.base/attach.exp +++ b/gdb/testsuite/gdb.base/attach.exp @@ -58,6 +58,37 @@ if [get_compiler_info] { return -1 } +# Start the program running and then wait for a bit, to be sure that +# it can be attached to. Return the process's PID. + +proc spawn_test_prog { executable } { + set testpid [eval exec $executable &] + exec sleep 2 + if { [istarget "*-*-cygwin*"] } { + # testpid is the Cygwin PID, GDB uses the Windows PID, which might be + # different due to the way fork/exec works. + set testpid [ exec ps -e | gawk "{ if (\$1 == $testpid) print \$4; }" ] + } + + return $testpid +} + +# Spawn GDB with CMDLINE_FLAGS appended to the GDBFLAGS global. + +proc gdb_spawn_with_cmdline_opts { cmdline_flags } { + global GDBFLAGS + + set saved_gdbflags $GDBFLAGS + + append GDBFLAGS $cmdline_flags + + set res [gdb_spawn] + + set GDBFLAGS $saved_gdbflags + + return $res +} + proc do_attach_tests {} { global gdb_prompt global binfile @@ -70,13 +101,7 @@ proc do_attach_tests {} { # Start the program running and then wait for a bit, to be sure # that it can be attached to. - set testpid [eval exec $binfile &] - exec sleep 2 - if { [istarget "*-*-cygwin*"] } { - # testpid is the Cygwin PID, GDB uses the Windows PID, which might be - # different due to the way fork/exec works. - set testpid [ exec ps -e | gawk "{ if (\$1 == $testpid) print \$4; }" ] - } + set testpid [spawn_test_prog $binfile] # Verify that we cannot attach to nonsense. @@ -279,16 +304,7 @@ proc do_attach_tests {} { remote_exec build "kill -9 ${testpid}" - # Start the program running and then wait for a bit, to be sure - # that it can be attached to. - - set testpid [eval exec $binfile &] - exec sleep 2 - if { [istarget "*-*-cygwin*"] } { - # testpid is the Cygwin PID, GDB uses the Windows PID, which might be - # different due to the way fork/exec works. - set testpid [ exec ps -e | gawk "{ if (\$1 == $testpid) print \$4; }" ] - } + set testpid [spawn_test_prog $binfile] # Verify that we can attach to the process, and find its a.out # when we're cd'd to some directory that doesn't contain the @@ -335,16 +351,7 @@ proc do_call_attach_tests {} { global gdb_prompt global binfile2 - # Start the program running and then wait for a bit, to be sure - # that it can be attached to. - - set testpid [eval exec $binfile2 &] - exec sleep 2 - if { [istarget "*-*-cygwin*"] } { - # testpid is the Cygwin PID, GDB uses the Windows PID, which might be - # different due to the way fork/exec works. - set testpid [ exec ps -e | gawk "{ if (\$1 == $testpid) print \$4; }" ] - } + set testpid [spawn_test_prog $binfile2] # Attach @@ -397,16 +404,7 @@ proc do_command_attach_tests {} { return 0 } - # Start the program running and then wait for a bit, to be sure - # that it can be attached to. - - set testpid [eval exec $binfile &] - exec sleep 2 - if { [istarget "*-*-cygwin*"] } { - # testpid is the Cygwin PID, GDB uses the Windows PID, which might be - # different due to the way fork/exec works. - set testpid [ exec ps -e | gawk "{ if (\$1 == $testpid) print \$4; }" ] - } + set testpid [spawn_test_prog $binfile] gdb_exit if $verbose>1 then { @@ -429,6 +427,50 @@ proc do_command_attach_tests {} { remote_exec build "kill -9 ${testpid}" } +# Test ' gdb --pid PID -ex "run" '. GDB used to have a bug where +# "run" would run before the attach finished - PR17347. + +proc test_command_line_attach_run {} { + global gdb_prompt + global binfile + global verbose + global GDB + global INTERNAL_GDBFLAGS + + if ![isnative] then { + unsupported "commandline attach run test" + return 0 + } + + with_test_prefix "cmdline attach run" { + set testpid [spawn_test_prog $binfile] + + set test "run to prompt" + gdb_exit + set res [gdb_spawn_with_cmdline_opts "--pid=$testpid -ex \"start\""] + if { $res != 0} { + fail $test + return $res + } + gdb_test_multiple "" $test { + -re {Attaching to.*Start it from the beginning\? \(y or n\) } { + pass $test + } + } + + send_gdb "y\n" + + set test "run to main" + gdb_test_multiple "" $test { + -re "Temporary breakpoint .* main .*$gdb_prompt $" { + pass $test + } + } + + # Get rid of the process + remote_exec build "kill -9 ${testpid}" + } +} # Start with a fresh gdb @@ -453,4 +495,6 @@ do_call_attach_tests do_command_attach_tests +test_command_line_attach_run + return 0 diff --git a/gdb/top.c b/gdb/top.c index fc2b84d..ba71f8f 100644 --- a/gdb/top.c +++ b/gdb/top.c @@ -373,6 +373,21 @@ check_frame_language_change (void) } } +void +maybe_wait_sync_command_done (int was_sync) +{ + /* If the interpreter is in sync mode (we're running a user + command's list, running command hooks or similars), and we + just ran a synchronous command that started the target, wait + for that command to end. */ + if (!interpreter_async && !was_sync && sync_execution) + { + while (gdb_do_one_event () >= 0) + if (!sync_execution) + break; + } +} + /* Execute the line P as a command, in the current user context. Pass FROM_TTY as second argument to the defining function. */ @@ -459,16 +474,7 @@ execute_command (char *p, int from_tty) else cmd_func (c, arg, from_tty); - /* If the interpreter is in sync mode (we're running a user - command's list, running command hooks or similars), and we - just ran a synchronous command that started the target, wait - for that command to end. */ - if (!interpreter_async && !was_sync && sync_execution) - { - while (gdb_do_one_event () >= 0) - if (!sync_execution) - break; - } + maybe_wait_sync_command_done (was_sync); /* If this command has been post-hooked, run the hook last. */ execute_cmd_post_hook (c); diff --git a/gdb/top.h b/gdb/top.h index c322c33..94f6c48 100644 --- a/gdb/top.h +++ b/gdb/top.h @@ -42,6 +42,14 @@ extern void quit_command (char *, int); extern void quit_cover (void); extern void execute_command (char *, int); +/* If the interpreter is in sync mode (we're running a user command's + list, running command hooks or similars), and we just ran a + synchronous command that started the target, wait for that command + to end. WAS_SYNC indicates whether sync_execution was set before + the command was run. */ + +extern void maybe_wait_sync_command_done (int was_sync); + extern void check_frame_language_change (void); /* Prepare for execution of a command. -- 1.9.3 ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: Regression: GDB stopped on run with attached process (PR 17347) [Re: [pushed+7.8] Re: [PATCH] Fix "attach" command vs user input race 2014-09-03 20:11 ` Regression: GDB stopped on run with attached process (PR 17347) [Re: [pushed+7.8] Re: [PATCH] Fix "attach" command vs user input race Pedro Alves @ 2014-09-07 19:28 ` Jan Kratochvil 2014-09-08 16:19 ` [PATCH 1/2] testsuite: refactor spawn and wait for attach (was: Re: Regression: GDB stopped on run with attached process) Pedro Alves 2014-09-08 16:27 ` Regression: GDB stopped on run with attached process (PR 17347) [Re: [pushed+7.8] Re: [PATCH] Fix "attach" command vs user input race Pedro Alves 0 siblings, 2 replies; 42+ messages in thread From: Jan Kratochvil @ 2014-09-07 19:28 UTC (permalink / raw) To: Pedro Alves; +Cc: Doug Evans, Mark Wielaard, gdb-patches [-- Attachment #1: Type: text/plain, Size: 5673 bytes --] On Wed, 03 Sep 2014 22:11:03 +0200, Pedro Alves wrote: > On 09/03/2014 08:58 AM, Jan Kratochvil wrote: > > > https://sourceware.org/bugzilla/show_bug.cgi?id=17347 > > Thanks Jan. > > Here's a fix, test included. Comments? In the testsuite run from the Fedora rpm packaging I get: GNU gdb (GDB) Fedora 7.8-21.fc21^M Copyright (C) 2014 Free Software Foundation, Inc.^M License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>^M This is free software: you are free to change and redistribute it.^M There is NO WARRANTY, to the extent permitted by law. Type "show copying"^M and "show warranty" for details.^M This GDB was configured as "i686-redhat-linux-gnu".^M Type "show configuration" for configuration details.^M For bug reporting instructions, please see:^M <http://www.gnu.org/software/gdb/bugs/>.^M Find the GDB manual and other documentation resources online at:^M <http://www.gnu.org/software/gdb/documentation/>.^M For help, type "help".^M Type "apropos word" to search for commands related to "word".^M Attaching to process 27028^M Reading symbols from /unsafebuild-i686-redhat-linux-gnu/gdb/testsuite.unix.-m32/outputs/gdb.base/attach/attach...done.^M Reading symbols from /lib/libm.so.6...Reading symbols from /usr/lib/debug/usr/lib/libm-2.19.90.so.debug...done.^M done.^M Loaded symbols for /lib/libm.so.6^M Reading symbols from /lib/libc.so.6...Reading symbols from /usr/lib/debug/usr/lib/libc-2.19.90.so.debug...done.^M done.^M Loaded symbols for /lib/libc.so.6^M Reading symbols from /lib/ld-linux.so.2...Reading symbols from /usr/lib/debug/usr/lib/ld-2.19.90.so.debug...done.^M done.^M Loaded symbols for /lib/ld-linux.so.2^M main ()^M at gdb/testsuite/gdb.base/attach.c:15^M 15 while (! should_exit)^M The program being debugged has been started already.^M Start it from the beginning? (y or n) PASS: gdb.base/attach.exp: cmdline attach run: run to prompt y^M ^M Temporary breakpoint 1 at 0x8048481: file gdb/testsuite/gdb.base/attach.c, line 13.^M Starting program: /unsafe/home/jkratoch/hammock/20140907fedorarel21-f21/fedora-2---Type <return> to continue, or q <return> to quit---ERROR: Window too small. UNRESOLVED: gdb.base/attach.exp: cmdline attach run: run to main While I do not fully understand why that happens in every run of that Fedora testsuite while it never happens during my reproducibility attempts by hand I find it understandable and the Fedora testsuite issues does get fixed by the attached patch. > --- a/gdb/testsuite/gdb.base/attach.exp > +++ b/gdb/testsuite/gdb.base/attach.exp > @@ -58,6 +58,37 @@ if [get_compiler_info] { > return -1 > } > > +# Start the program running and then wait for a bit, to be sure that > +# it can be attached to. Return the process's PID. > + > +proc spawn_test_prog { executable } { > + set testpid [eval exec $executable &] > + exec sleep 2 Unrelated to this patch - this patch only moved the code. Also the code move could be a separate patch: I do not see why the testsuite commonly uses "exec sleep" while it also uses "sleep" itself which also works fine. > + if { [istarget "*-*-cygwin*"] } { > + # testpid is the Cygwin PID, GDB uses the Windows PID, which might be > + # different due to the way fork/exec works. > + set testpid [ exec ps -e | gawk "{ if (\$1 == $testpid) print \$4; }" ] > + } > + > + return $testpid > +} [...] > @@ -429,6 +427,50 @@ proc do_command_attach_tests {} { > remote_exec build "kill -9 ${testpid}" > } > > +# Test ' gdb --pid PID -ex "run" '. GDB used to have a bug where > +# "run" would run before the attach finished - PR17347. > + > +proc test_command_line_attach_run {} { > + global gdb_prompt > + global binfile > + global verbose > + global GDB > + global INTERNAL_GDBFLAGS These 3 lines are unused. > + > + if ![isnative] then { > + unsupported "commandline attach run test" > + return 0 > + } > + > + with_test_prefix "cmdline attach run" { > + set testpid [spawn_test_prog $binfile] > + > + set test "run to prompt" > + gdb_exit > + set res [gdb_spawn_with_cmdline_opts "--pid=$testpid -ex \"start\""] Here see the attachment. > + if { $res != 0} { > + fail $test > + return $res > + } > + gdb_test_multiple "" $test { > + -re {Attaching to.*Start it from the beginning\? \(y or n\) } { > + pass $test > + } > + } > + > + send_gdb "y\n" > + > + set test "run to main" > + gdb_test_multiple "" $test { > + -re "Temporary breakpoint .* main .*$gdb_prompt $" { > + pass $test > + } > + } > + > + # Get rid of the process > + remote_exec build "kill -9 ${testpid}" > + } > +} > > # Start with a fresh gdb > > @@ -453,4 +495,6 @@ do_call_attach_tests > > do_command_attach_tests > > +test_command_line_attach_run > + > return 0 > diff --git a/gdb/top.c b/gdb/top.c > index fc2b84d..ba71f8f 100644 > --- a/gdb/top.c > +++ b/gdb/top.c > @@ -373,6 +373,21 @@ check_frame_language_change (void) > } > } > Missing: /* See top.h. */ Unless that rule from me has been abandoned. > +void > +maybe_wait_sync_command_done (int was_sync) > +{ > + /* If the interpreter is in sync mode (we're running a user > + command's list, running command hooks or similars), and we > + just ran a synchronous command that started the target, wait > + for that command to end. */ > + if (!interpreter_async && !was_sync && sync_execution) > + { > + while (gdb_do_one_event () >= 0) > + if (!sync_execution) > + break; > + } > +} > + > /* Execute the line P as a command, in the current user context. > Pass FROM_TTY as second argument to the defining function. */ > Thanks, Jan [-- Attachment #2: 1 --] [-- Type: text/plain, Size: 538 bytes --] diff --git a/gdb/testsuite/gdb.base/attach.exp b/gdb/testsuite/gdb.base/attach.exp index c94c127..6e566a3 100644 --- a/gdb/testsuite/gdb.base/attach.exp +++ b/gdb/testsuite/gdb.base/attach.exp @@ -447,7 +444,8 @@ proc test_command_line_attach_run {} { set test "run to prompt" gdb_exit - set res [gdb_spawn_with_cmdline_opts "--pid=$testpid -ex \"start\""] + set res [gdb_spawn_with_cmdline_opts \ + "-iex set\\ height\\ 0 -iex set\\ width\\ 0 --pid=$testpid -ex \"start\""] if { $res != 0} { fail $test return $res ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH 1/2] testsuite: refactor spawn and wait for attach (was: Re: Regression: GDB stopped on run with attached process) 2014-09-07 19:28 ` Jan Kratochvil @ 2014-09-08 16:19 ` Pedro Alves 2014-09-09 17:29 ` Jan Kratochvil 2014-09-08 16:27 ` Regression: GDB stopped on run with attached process (PR 17347) [Re: [pushed+7.8] Re: [PATCH] Fix "attach" command vs user input race Pedro Alves 1 sibling, 1 reply; 42+ messages in thread From: Pedro Alves @ 2014-09-08 16:19 UTC (permalink / raw) To: Jan Kratochvil; +Cc: Doug Evans, Mark Wielaard, gdb-patches On 09/07/2014 08:28 PM, Jan Kratochvil wrote: >> > --- a/gdb/testsuite/gdb.base/attach.exp >> > +++ b/gdb/testsuite/gdb.base/attach.exp >> > @@ -58,6 +58,37 @@ if [get_compiler_info] { >> > return -1 >> > } >> > >> > +# Start the program running and then wait for a bit, to be sure that >> > +# it can be attached to. Return the process's PID. >> > + >> > +proc spawn_test_prog { executable } { >> > + set testpid [eval exec $executable &] >> > + exec sleep 2 > Unrelated to this patch - this patch only moved the code. Also the code move > could be a separate patch: Indeed it could. I've done that now. See below. There were actually more places around the testsuite that have that same code. Let me know how it looks to you. > I do not see why the testsuite commonly uses "exec sleep" while it also uses > "sleep" itself which also works fine. No idea, though I'd guess that there's really no good reason. ------ 8< ------- [PATCH 1/2] testsuite: refactor spawn and wait for attach Several places in the testsuite have a copy of a snippet of code that spawns a test program, waits a bit, and then does some PID munging for Cygwin. This is in order to have GDB attach to the spawned program. This refactors all that to a common procedure. (multi-attach.exp wants to spawn multiple processes, so this makes the new procedure's interface work with lists.) Tested on x86_64 Fedora 20. gdb/testsuite/ChangeLog: 2014-09-08 Pedro Alves <palves@redhat.com> * lib/gdb.exp (spawn_wait_for_attach): New procedure. * gdb.base/attach.exp (do_attach_tests, do_call_attach_tests) (do_command_attach_tests): Use spawn_wait_for_attach. * gdb.base/solib-overlap.exp: Likewise. * gdb.multi/multi-attach.exp: Likewise. * gdb.python/py-prompt.exp: Likewise. * gdb.python/py-sync-interp.exp: Likewise. * gdb.server/ext-attach.exp: Likewise. --- gdb/testsuite/gdb.base/attach.exp | 41 +++-------------------------- gdb/testsuite/gdb.base/solib-overlap.exp | 11 +------- gdb/testsuite/gdb.multi/multi-attach.exp | 13 +++------ gdb/testsuite/gdb.python/py-prompt.exp | 10 +------ gdb/testsuite/gdb.python/py-sync-interp.exp | 10 +------ gdb/testsuite/gdb.server/ext-attach.exp | 10 +------ gdb/testsuite/lib/gdb.exp | 25 ++++++++++++++++++ 7 files changed, 37 insertions(+), 83 deletions(-) diff --git a/gdb/testsuite/gdb.base/attach.exp b/gdb/testsuite/gdb.base/attach.exp index 9714c29..a20c51a 100644 --- a/gdb/testsuite/gdb.base/attach.exp +++ b/gdb/testsuite/gdb.base/attach.exp @@ -70,13 +70,7 @@ proc do_attach_tests {} { # Start the program running and then wait for a bit, to be sure # that it can be attached to. - set testpid [eval exec $binfile &] - exec sleep 2 - if { [istarget "*-*-cygwin*"] } { - # testpid is the Cygwin PID, GDB uses the Windows PID, which might be - # different due to the way fork/exec works. - set testpid [ exec ps -e | gawk "{ if (\$1 == $testpid) print \$4; }" ] - } + set testpid [spawn_wait_for_attach $binfile] # Verify that we cannot attach to nonsense. @@ -279,16 +273,7 @@ proc do_attach_tests {} { remote_exec build "kill -9 ${testpid}" - # Start the program running and then wait for a bit, to be sure - # that it can be attached to. - - set testpid [eval exec $binfile &] - exec sleep 2 - if { [istarget "*-*-cygwin*"] } { - # testpid is the Cygwin PID, GDB uses the Windows PID, which might be - # different due to the way fork/exec works. - set testpid [ exec ps -e | gawk "{ if (\$1 == $testpid) print \$4; }" ] - } + set testpid [spawn_wait_for_attach $binfile] # Verify that we can attach to the process, and find its a.out # when we're cd'd to some directory that doesn't contain the @@ -335,16 +320,7 @@ proc do_call_attach_tests {} { global gdb_prompt global binfile2 - # Start the program running and then wait for a bit, to be sure - # that it can be attached to. - - set testpid [eval exec $binfile2 &] - exec sleep 2 - if { [istarget "*-*-cygwin*"] } { - # testpid is the Cygwin PID, GDB uses the Windows PID, which might be - # different due to the way fork/exec works. - set testpid [ exec ps -e | gawk "{ if (\$1 == $testpid) print \$4; }" ] - } + set testpid [spawn_wait_for_attach $binfile2] # Attach @@ -397,16 +373,7 @@ proc do_command_attach_tests {} { return 0 } - # Start the program running and then wait for a bit, to be sure - # that it can be attached to. - - set testpid [eval exec $binfile &] - exec sleep 2 - if { [istarget "*-*-cygwin*"] } { - # testpid is the Cygwin PID, GDB uses the Windows PID, which might be - # different due to the way fork/exec works. - set testpid [ exec ps -e | gawk "{ if (\$1 == $testpid) print \$4; }" ] - } + set testpid [spawn_wait_for_attach $binfile] gdb_exit if $verbose>1 then { diff --git a/gdb/testsuite/gdb.base/solib-overlap.exp b/gdb/testsuite/gdb.base/solib-overlap.exp index 68731be..7486b07 100644 --- a/gdb/testsuite/gdb.base/solib-overlap.exp +++ b/gdb/testsuite/gdb.base/solib-overlap.exp @@ -82,16 +82,7 @@ foreach prelink_lib1 {0x40000000 0x50000000} { with_test_prefix "$prelink_lib1" return -1 } - # Start the program running and then wait for a bit, to be sure - # that it can be attached to. - - set testpid [eval exec $binfile &] - sleep 2 - if { [istarget "*-*-cygwin*"] } { - # testpid is the Cygwin PID, GDB uses the Windows PID, which might be - # different due to the way fork/exec works. - set testpid [ exec ps -e | gawk "{ if (\$1 == $testpid) print \$4; }" ] - } + set testpid [spawn_wait_for_attach $binfile] remote_exec build "mv -f ${binfile_lib1} ${binfile_lib1}-running" remote_exec build "mv -f ${binfile_lib2} ${binfile_lib2}-running" diff --git a/gdb/testsuite/gdb.multi/multi-attach.exp b/gdb/testsuite/gdb.multi/multi-attach.exp index e933520..eaff2c9 100644 --- a/gdb/testsuite/gdb.multi/multi-attach.exp +++ b/gdb/testsuite/gdb.multi/multi-attach.exp @@ -30,15 +30,10 @@ if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug additiona # Start the programs running and then wait for a bit, to be sure that # they can be attached to. -set testpid1 [eval exec $binfile &] -set testpid2 [eval exec $binfile &] -exec sleep 2 -if { [istarget "*-*-cygwin*"] } { - # testpid{1,2} are the Cygwin PID, GDB uses the Windows PID, which might be - # different due to the way fork/exec works. - set testpid1 [ exec ps -e | gawk "{ if (\$1 == $testpid1) print \$4; }" ] - set testpid2 [ exec ps -e | gawk "{ if (\$1 == $testpid2) print \$4; }" ] -} + +set pid_list [spawn_wait_for_attach [list $binfile $binfile]] +set testpid1 [lindex $pid_list 0] +set testpid2 [lindex $pid_list 1] gdb_test "attach $testpid1" \ "Attaching to program: .*, process $testpid1.*(in|at).*" \ diff --git a/gdb/testsuite/gdb.python/py-prompt.exp b/gdb/testsuite/gdb.python/py-prompt.exp index ebe4cb6..1c53c03 100644 --- a/gdb/testsuite/gdb.python/py-prompt.exp +++ b/gdb/testsuite/gdb.python/py-prompt.exp @@ -80,15 +80,7 @@ gdb_test "python print (\"'\" + str(p\[0\]) + \"'\")" "'$gdb_prompt_fail '" \ "prompt_hook argument is default prompt. 2" gdb_exit -# Start the program running and then wait for a bit, to be sure -# that it can be attached to. -set testpid [eval exec $binfile &] -exec sleep 2 -if { [istarget "*-*-cygwin*"] } { - # testpid is the Cygwin PID, GDB uses the Windows PID, which might be - # different due to the way fork/exec works. - set testpid [ exec ps -e | gawk "{ if (\$1 == $testpid) print \$4; }" ] -} +set testpid [spawn_wait_for_attach $binfile] set GDBFLAGS [concat $tmp_gdbflags " -ex \"set pagination off\""] set GDBFLAGS [concat $GDBFLAGS " -ex \"set editing on\""] diff --git a/gdb/testsuite/gdb.python/py-sync-interp.exp b/gdb/testsuite/gdb.python/py-sync-interp.exp index b9b86bc..d62f966 100644 --- a/gdb/testsuite/gdb.python/py-sync-interp.exp +++ b/gdb/testsuite/gdb.python/py-sync-interp.exp @@ -41,15 +41,7 @@ if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile}] } { return -1 } -# Start the program running and then wait for a bit, to be sure -# that it can be attached to. -set testpid [eval exec $binfile &] -exec sleep 2 -if { [istarget "*-*-cygwin*"] } { - # testpid is the Cygwin PID, GDB uses the Windows PID, which might be - # different due to the way fork/exec works. - set testpid [ exec ps -e | gawk "{ if (\$1 == $testpid) print \$4; }" ] -} +set testpid [spawn_wait_for_attach $binfile] # Test command 'where' is executed when command 'attach' is done, otherwise # function 'sleep' may not show up in backtrace. diff --git a/gdb/testsuite/gdb.server/ext-attach.exp b/gdb/testsuite/gdb.server/ext-attach.exp index 5f7bac4..9baeeb7 100644 --- a/gdb/testsuite/gdb.server/ext-attach.exp +++ b/gdb/testsuite/gdb.server/ext-attach.exp @@ -44,15 +44,7 @@ gdbserver_start_extended gdb_test_no_output "set remote exec-file $target_exec" "set remote exec-file" -# Start the program running and then wait for a bit, to be sure -# that it can be attached to. -set testpid [eval exec $binfile &] -exec sleep 2 -if { [istarget "*-*-cygwin*"] } { - # testpid is the Cygwin PID, GDB uses the Windows PID, which might be - # different due to the way fork/exec works. - set testpid [ exec ps -e | gawk "{ if (\$1 == $testpid) print \$4; }" ] -} +set testpid [spawn_wait_for_attach $binfile] gdb_test "attach $testpid" \ "Attaching to program: .*, process $testpid.*(in|at).*" \ diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp index 1019ecd..ecf942e 100644 --- a/gdb/testsuite/lib/gdb.exp +++ b/gdb/testsuite/lib/gdb.exp @@ -3347,6 +3347,31 @@ proc gdb_exit { } { catch default_gdb_exit } +# Start a set of programs running and then wait for a bit, to be sure +# that they can be attached to. Return a list of the processes' PIDs. + +proc spawn_wait_for_attach { executable_list } { + set pid_list {} + + foreach {executable} $executable_list { + lappend pid_list [eval exec $executable &] + } + + sleep 2 + + if { [istarget "*-*-cygwin*"] } { + for {set i 0} {$i < [llength $pid_list]} {incr i} { + # testpid is the Cygwin PID, GDB uses the Windows PID, + # which might be different due to the way fork/exec works. + set testpid [lindex $pid_list $i] + set testpid [ exec ps -e | gawk "{ if (\$1 == $testpid) print \$4; }" ] + lreplace $i $i $testpid + } + } + + return $pid_list +} + # # gdb_load_cmd -- load a file into the debugger. # ARGS - additional args to load command. -- 1.9.3 ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 1/2] testsuite: refactor spawn and wait for attach (was: Re: Regression: GDB stopped on run with attached process) 2014-09-08 16:19 ` [PATCH 1/2] testsuite: refactor spawn and wait for attach (was: Re: Regression: GDB stopped on run with attached process) Pedro Alves @ 2014-09-09 17:29 ` Jan Kratochvil 2014-09-09 17:35 ` [PATCH 1/2] testsuite: refactor spawn and wait for attach Pedro Alves 0 siblings, 1 reply; 42+ messages in thread From: Jan Kratochvil @ 2014-09-09 17:29 UTC (permalink / raw) To: Pedro Alves; +Cc: Doug Evans, Mark Wielaard, gdb-patches On Mon, 08 Sep 2014 18:19:17 +0200, Pedro Alves wrote: > Indeed it could. I've done that now. See below. There were actually more > places around the testsuite that have that same code. Let me know how it looks > to you. As long as it has been tested on *-*-cygwin* I do not see anything wrong there. Thanks, Jan ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 1/2] testsuite: refactor spawn and wait for attach 2014-09-09 17:29 ` Jan Kratochvil @ 2014-09-09 17:35 ` Pedro Alves 2014-09-10 21:25 ` Pedro Alves 0 siblings, 1 reply; 42+ messages in thread From: Pedro Alves @ 2014-09-09 17:35 UTC (permalink / raw) To: Jan Kratochvil; +Cc: Doug Evans, Mark Wielaard, gdb-patches On 09/09/2014 06:29 PM, Jan Kratochvil wrote: > On Mon, 08 Sep 2014 18:19:17 +0200, Pedro Alves wrote: >> Indeed it could. I've done that now. See below. There were actually more >> places around the testsuite that have that same code. Let me know how it looks >> to you. > > As long as it has been tested on *-*-cygwin* I do not see anything wrong > there. It hasn't. I wonder if someone can help with this? Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 1/2] testsuite: refactor spawn and wait for attach 2014-09-09 17:35 ` [PATCH 1/2] testsuite: refactor spawn and wait for attach Pedro Alves @ 2014-09-10 21:25 ` Pedro Alves 2014-09-11 12:34 ` Pedro Alves 0 siblings, 1 reply; 42+ messages in thread From: Pedro Alves @ 2014-09-10 21:25 UTC (permalink / raw) To: Pedro Alves, Jan Kratochvil; +Cc: Doug Evans, Mark Wielaard, gdb-patches On 09/09/2014 06:35 PM, Pedro Alves wrote: > On 09/09/2014 06:29 PM, Jan Kratochvil wrote: >> On Mon, 08 Sep 2014 18:19:17 +0200, Pedro Alves wrote: >>> Indeed it could. I've done that now. See below. There were actually more >>> places around the testsuite that have that same code. Let me know how it looks >>> to you. >> >> As long as it has been tested on *-*-cygwin* I do not see anything wrong >> there. > > It hasn't. I wonder if someone can help with this? FYI, Keith was kind enough to run testing and sent me the logs, though I haven't analyzed them yet. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 1/2] testsuite: refactor spawn and wait for attach 2014-09-10 21:25 ` Pedro Alves @ 2014-09-11 12:34 ` Pedro Alves 0 siblings, 0 replies; 42+ messages in thread From: Pedro Alves @ 2014-09-11 12:34 UTC (permalink / raw) To: Pedro Alves, Jan Kratochvil; +Cc: Doug Evans, Mark Wielaard, gdb-patches On 09/10/2014 10:25 PM, Pedro Alves wrote: > On 09/09/2014 06:35 PM, Pedro Alves wrote: >> On 09/09/2014 06:29 PM, Jan Kratochvil wrote: >>> On Mon, 08 Sep 2014 18:19:17 +0200, Pedro Alves wrote: >>>> Indeed it could. I've done that now. See below. There were actually more >>>> places around the testsuite that have that same code. Let me know how it looks >>>> to you. >>> >>> As long as it has been tested on *-*-cygwin* I do not see anything wrong >>> there. >> >> It hasn't. I wonder if someone can help with this? > > FYI, Keith was kind enough to run testing and sent me the logs, > though I haven't analyzed them yet. There was a silly bug, causing new Cygwin fails. lreplace was being misused. This fixed it: +++ w/gdb/testsuite/lib/gdb.exp @@ -3381,7 +3381,7 @@ proc spawn_wait_for_attach { executable_list } { # which might be different due to the way fork/exec works. set testpid [lindex $pid_list $i] set testpid [ exec ps -e | gawk "{ if (\$1 == $testpid) print \$4; }" ] - lreplace $i $i $testpid + set pid_list [lreplace $pid_list $i $i $testpid] } } I borrowed the wife's laptop and managed to test the fix on Cygwin myself (boy is that slow...). Here's what I pushed to both master and 7.8. ---- From 7ec5670becb3f978f4d2f4df252ad0cbf805e37a Mon Sep 17 00:00:00 2001 From: Pedro Alves <palves@redhat.com> Date: Thu, 11 Sep 2014 13:14:47 +0100 Subject: [PATCH] testsuite: refactor spawn and wait for attach Several places in the testsuite have a copy of a snippet of code that spawns a test program, waits a bit, and then does some PID munging for Cygwin. This is in order to have GDB attach to the spawned program. This refactors all that to a common procedure. (multi-attach.exp wants to spawn multiple processes, so this makes the new procedure's interface work with lists.) Tested on x86_64 Fedora 20. gdb/testsuite/ChangeLog: 2014-09-11 Pedro Alves <palves@redhat.com> * lib/gdb.exp (spawn_wait_for_attach): New procedure. * gdb.base/attach.exp (do_attach_tests, do_call_attach_tests) (do_command_attach_tests): Use spawn_wait_for_attach. * gdb.base/solib-overlap.exp: Likewise. * gdb.multi/multi-attach.exp: Likewise. * gdb.python/py-prompt.exp: Likewise. * gdb.python/py-sync-interp.exp: Likewise. * gdb.server/ext-attach.exp: Likewise. --- gdb/testsuite/ChangeLog | 11 ++++++++ gdb/testsuite/gdb.base/attach.exp | 41 +++-------------------------- gdb/testsuite/gdb.base/solib-overlap.exp | 11 +------- gdb/testsuite/gdb.multi/multi-attach.exp | 13 +++------ gdb/testsuite/gdb.python/py-prompt.exp | 10 +------ gdb/testsuite/gdb.python/py-sync-interp.exp | 10 +------ gdb/testsuite/gdb.server/ext-attach.exp | 10 +------ gdb/testsuite/lib/gdb.exp | 25 ++++++++++++++++++ 8 files changed, 48 insertions(+), 83 deletions(-) diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index f85f9b5..a388194 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,14 @@ +2014-09-11 Pedro Alves <palves@redhat.com> + + * lib/gdb.exp (spawn_wait_for_attach): New procedure. + * gdb.base/attach.exp (do_attach_tests, do_call_attach_tests) + (do_command_attach_tests): Use spawn_wait_for_attach. + * gdb.base/solib-overlap.exp: Likewise. + * gdb.multi/multi-attach.exp: Likewise. + * gdb.python/py-prompt.exp: Likewise. + * gdb.python/py-sync-interp.exp: Likewise. + * gdb.server/ext-attach.exp: Likewise. + 2014-09-07 Jan Kratochvil <jan.kratochvil@redhat.com> PR python/17355 diff --git a/gdb/testsuite/gdb.base/attach.exp b/gdb/testsuite/gdb.base/attach.exp index 9714c29..a20c51a 100644 --- a/gdb/testsuite/gdb.base/attach.exp +++ b/gdb/testsuite/gdb.base/attach.exp @@ -70,13 +70,7 @@ proc do_attach_tests {} { # Start the program running and then wait for a bit, to be sure # that it can be attached to. - set testpid [eval exec $binfile &] - exec sleep 2 - if { [istarget "*-*-cygwin*"] } { - # testpid is the Cygwin PID, GDB uses the Windows PID, which might be - # different due to the way fork/exec works. - set testpid [ exec ps -e | gawk "{ if (\$1 == $testpid) print \$4; }" ] - } + set testpid [spawn_wait_for_attach $binfile] # Verify that we cannot attach to nonsense. @@ -279,16 +273,7 @@ proc do_attach_tests {} { remote_exec build "kill -9 ${testpid}" - # Start the program running and then wait for a bit, to be sure - # that it can be attached to. - - set testpid [eval exec $binfile &] - exec sleep 2 - if { [istarget "*-*-cygwin*"] } { - # testpid is the Cygwin PID, GDB uses the Windows PID, which might be - # different due to the way fork/exec works. - set testpid [ exec ps -e | gawk "{ if (\$1 == $testpid) print \$4; }" ] - } + set testpid [spawn_wait_for_attach $binfile] # Verify that we can attach to the process, and find its a.out # when we're cd'd to some directory that doesn't contain the @@ -335,16 +320,7 @@ proc do_call_attach_tests {} { global gdb_prompt global binfile2 - # Start the program running and then wait for a bit, to be sure - # that it can be attached to. - - set testpid [eval exec $binfile2 &] - exec sleep 2 - if { [istarget "*-*-cygwin*"] } { - # testpid is the Cygwin PID, GDB uses the Windows PID, which might be - # different due to the way fork/exec works. - set testpid [ exec ps -e | gawk "{ if (\$1 == $testpid) print \$4; }" ] - } + set testpid [spawn_wait_for_attach $binfile2] # Attach @@ -397,16 +373,7 @@ proc do_command_attach_tests {} { return 0 } - # Start the program running and then wait for a bit, to be sure - # that it can be attached to. - - set testpid [eval exec $binfile &] - exec sleep 2 - if { [istarget "*-*-cygwin*"] } { - # testpid is the Cygwin PID, GDB uses the Windows PID, which might be - # different due to the way fork/exec works. - set testpid [ exec ps -e | gawk "{ if (\$1 == $testpid) print \$4; }" ] - } + set testpid [spawn_wait_for_attach $binfile] gdb_exit if $verbose>1 then { diff --git a/gdb/testsuite/gdb.base/solib-overlap.exp b/gdb/testsuite/gdb.base/solib-overlap.exp index 68731be..7486b07 100644 --- a/gdb/testsuite/gdb.base/solib-overlap.exp +++ b/gdb/testsuite/gdb.base/solib-overlap.exp @@ -82,16 +82,7 @@ foreach prelink_lib1 {0x40000000 0x50000000} { with_test_prefix "$prelink_lib1" return -1 } - # Start the program running and then wait for a bit, to be sure - # that it can be attached to. - - set testpid [eval exec $binfile &] - sleep 2 - if { [istarget "*-*-cygwin*"] } { - # testpid is the Cygwin PID, GDB uses the Windows PID, which might be - # different due to the way fork/exec works. - set testpid [ exec ps -e | gawk "{ if (\$1 == $testpid) print \$4; }" ] - } + set testpid [spawn_wait_for_attach $binfile] remote_exec build "mv -f ${binfile_lib1} ${binfile_lib1}-running" remote_exec build "mv -f ${binfile_lib2} ${binfile_lib2}-running" diff --git a/gdb/testsuite/gdb.multi/multi-attach.exp b/gdb/testsuite/gdb.multi/multi-attach.exp index e933520..eaff2c9 100644 --- a/gdb/testsuite/gdb.multi/multi-attach.exp +++ b/gdb/testsuite/gdb.multi/multi-attach.exp @@ -30,15 +30,10 @@ if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug additiona # Start the programs running and then wait for a bit, to be sure that # they can be attached to. -set testpid1 [eval exec $binfile &] -set testpid2 [eval exec $binfile &] -exec sleep 2 -if { [istarget "*-*-cygwin*"] } { - # testpid{1,2} are the Cygwin PID, GDB uses the Windows PID, which might be - # different due to the way fork/exec works. - set testpid1 [ exec ps -e | gawk "{ if (\$1 == $testpid1) print \$4; }" ] - set testpid2 [ exec ps -e | gawk "{ if (\$1 == $testpid2) print \$4; }" ] -} + +set pid_list [spawn_wait_for_attach [list $binfile $binfile]] +set testpid1 [lindex $pid_list 0] +set testpid2 [lindex $pid_list 1] gdb_test "attach $testpid1" \ "Attaching to program: .*, process $testpid1.*(in|at).*" \ diff --git a/gdb/testsuite/gdb.python/py-prompt.exp b/gdb/testsuite/gdb.python/py-prompt.exp index ebe4cb6..1c53c03 100644 --- a/gdb/testsuite/gdb.python/py-prompt.exp +++ b/gdb/testsuite/gdb.python/py-prompt.exp @@ -80,15 +80,7 @@ gdb_test "python print (\"'\" + str(p\[0\]) + \"'\")" "'$gdb_prompt_fail '" \ "prompt_hook argument is default prompt. 2" gdb_exit -# Start the program running and then wait for a bit, to be sure -# that it can be attached to. -set testpid [eval exec $binfile &] -exec sleep 2 -if { [istarget "*-*-cygwin*"] } { - # testpid is the Cygwin PID, GDB uses the Windows PID, which might be - # different due to the way fork/exec works. - set testpid [ exec ps -e | gawk "{ if (\$1 == $testpid) print \$4; }" ] -} +set testpid [spawn_wait_for_attach $binfile] set GDBFLAGS [concat $tmp_gdbflags " -ex \"set pagination off\""] set GDBFLAGS [concat $GDBFLAGS " -ex \"set editing on\""] diff --git a/gdb/testsuite/gdb.python/py-sync-interp.exp b/gdb/testsuite/gdb.python/py-sync-interp.exp index b9b86bc..d62f966 100644 --- a/gdb/testsuite/gdb.python/py-sync-interp.exp +++ b/gdb/testsuite/gdb.python/py-sync-interp.exp @@ -41,15 +41,7 @@ if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile}] } { return -1 } -# Start the program running and then wait for a bit, to be sure -# that it can be attached to. -set testpid [eval exec $binfile &] -exec sleep 2 -if { [istarget "*-*-cygwin*"] } { - # testpid is the Cygwin PID, GDB uses the Windows PID, which might be - # different due to the way fork/exec works. - set testpid [ exec ps -e | gawk "{ if (\$1 == $testpid) print \$4; }" ] -} +set testpid [spawn_wait_for_attach $binfile] # Test command 'where' is executed when command 'attach' is done, otherwise # function 'sleep' may not show up in backtrace. diff --git a/gdb/testsuite/gdb.server/ext-attach.exp b/gdb/testsuite/gdb.server/ext-attach.exp index 5f7bac4..9baeeb7 100644 --- a/gdb/testsuite/gdb.server/ext-attach.exp +++ b/gdb/testsuite/gdb.server/ext-attach.exp @@ -44,15 +44,7 @@ gdbserver_start_extended gdb_test_no_output "set remote exec-file $target_exec" "set remote exec-file" -# Start the program running and then wait for a bit, to be sure -# that it can be attached to. -set testpid [eval exec $binfile &] -exec sleep 2 -if { [istarget "*-*-cygwin*"] } { - # testpid is the Cygwin PID, GDB uses the Windows PID, which might be - # different due to the way fork/exec works. - set testpid [ exec ps -e | gawk "{ if (\$1 == $testpid) print \$4; }" ] -} +set testpid [spawn_wait_for_attach $binfile] gdb_test "attach $testpid" \ "Attaching to program: .*, process $testpid.*(in|at).*" \ diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp index 7650d2a..e2ee110 100644 --- a/gdb/testsuite/lib/gdb.exp +++ b/gdb/testsuite/lib/gdb.exp @@ -3323,6 +3323,31 @@ proc gdb_exit { } { catch default_gdb_exit } +# Start a set of programs running and then wait for a bit, to be sure +# that they can be attached to. Return a list of the processes' PIDs. + +proc spawn_wait_for_attach { executable_list } { + set pid_list {} + + foreach {executable} $executable_list { + lappend pid_list [eval exec $executable &] + } + + sleep 2 + + if { [istarget "*-*-cygwin*"] } { + for {set i 0} {$i < [llength $pid_list]} {incr i} { + # testpid is the Cygwin PID, GDB uses the Windows PID, + # which might be different due to the way fork/exec works. + set testpid [lindex $pid_list $i] + set testpid [ exec ps -e | gawk "{ if (\$1 == $testpid) print \$4; }" ] + set pid_list [lreplace $pid_list $i $i $testpid] + } + } + + return $pid_list +} + # # gdb_load_cmd -- load a file into the debugger. # ARGS - additional args to load command. -- 1.9.3 ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: Regression: GDB stopped on run with attached process (PR 17347) [Re: [pushed+7.8] Re: [PATCH] Fix "attach" command vs user input race 2014-09-07 19:28 ` Jan Kratochvil 2014-09-08 16:19 ` [PATCH 1/2] testsuite: refactor spawn and wait for attach (was: Re: Regression: GDB stopped on run with attached process) Pedro Alves @ 2014-09-08 16:27 ` Pedro Alves 2014-09-09 18:25 ` Jan Kratochvil 1 sibling, 1 reply; 42+ messages in thread From: Pedro Alves @ 2014-09-08 16:27 UTC (permalink / raw) To: Jan Kratochvil; +Cc: Doug Evans, Mark Wielaard, gdb-patches On 09/07/2014 08:28 PM, Jan Kratochvil wrote: > Temporary breakpoint 1 at 0x8048481: file gdb/testsuite/gdb.base/attach.c, line 13.^M > Starting program: /unsafe/home/jkratoch/hammock/20140907fedorarel21-f21/fedora-2---Type <return> to continue, or q <return> to quit---ERROR: Window too small. > UNRESOLVED: gdb.base/attach.exp: cmdline attach run: run to main Good catch. I see that if one runs the test with a short enough terminal, do_command_attach_tests fails as well. I'm leaving that for a separate patch. >> +proc test_command_line_attach_run {} { >> + global gdb_prompt >> + global binfile > >> + global verbose >> + global GDB >> + global INTERNAL_GDBFLAGS > > These 3 lines are unused. Thanks, removed. >> + set test "run to prompt" >> + gdb_exit >> + set res [gdb_spawn_with_cmdline_opts "--pid=$testpid -ex \"start\""] > > Here see the attachment. Thanks for this. > Missing: > /* See top.h. */ > > Unless that rule from me has been abandoned. Added. Here's the updated patch. WDYT? ---------- 8< ---------- Subject: [PATCH 2/2] gdb/17347 - Regression: GDB stopped on run with attached process Doing: gdb --pid=PID -ex run Results in GDB getting a SIGTTIN, and thus ending stopped. That's usually indicative of a missing target_terminal_ours call. E.g., from the PR: $ sleep 1h & p=$!; sleep 0.1; gdb -batch sleep $p -ex run [1] 28263 [1] Killed sleep 1h [2]+ Stopped gdb -batch sleep $p -ex run The workaround is doing: gdb -ex "attach $PID" -ex "run" instead of gdb [-p] $PID -ex "run" With the former, gdb waits for the attach command to complete before moving on to the "run" command, because the interpreter is in sync mode at this point, within execute_command. But for the latter, attach_command is called directly from captured_main, and thus misses that waiting. IOW, "run" is running before the attach continuation has run, before the program stops and attach completes. The broken terminal settings are just one symptom of that. Any command that queries or requires input results in the same. The fix is to wait in catch_command_errors (which is specific to main.c nowadays), just like we wait in execute_command. gdb/ChangeLog: 2014-09-08 Pedro Alves <palves@redhat.com> PR gdb/17347 * main.c: Include "infrun.h". (catch_command_errors, catch_command_errors_const): Wait for the foreground command to complete. * top.c (maybe_wait_sync_command_done): New function, factored out from ... (maybe_wait_sync_command_done): ... here. * top.h (maybe_wait_sync_command_done): New declaration. gdb/testsuite/ChangeLog: 2014-09-08 Pedro Alves <palves@redhat.com> PR gdb/17347 * lib/gdb.exp (gdb_spawn_with_cmdline_opts): New procedure. * gdb.base/attach.exp (test_command_line_attach_run): New procedure. (top level): Call it. --- gdb/main.c | 9 ++++++++ gdb/testsuite/gdb.base/attach.exp | 45 +++++++++++++++++++++++++++++++++++++++ gdb/testsuite/lib/gdb.exp | 16 ++++++++++++++ gdb/top.c | 28 +++++++++++++++--------- gdb/top.h | 8 +++++++ 5 files changed, 96 insertions(+), 10 deletions(-) diff --git a/gdb/main.c b/gdb/main.c index 12f0146..756dd4f 100644 --- a/gdb/main.c +++ b/gdb/main.c @@ -45,6 +45,7 @@ #include "filestuff.h" #include <signal.h> #include "event-top.h" +#include "infrun.h" /* The selected interpreter. This will be used as a set command variable, so it should always be malloc'ed - since @@ -369,7 +370,11 @@ catch_command_errors (catch_command_errors_ftype *command, TRY_CATCH (e, mask) { + int was_sync = sync_execution; + command (arg, from_tty); + + maybe_wait_sync_command_done (was_sync); } return handle_command_errors (e); } @@ -388,7 +393,11 @@ catch_command_errors_const (catch_command_errors_const_ftype *command, TRY_CATCH (e, mask) { + int was_sync = sync_execution; + command (arg, from_tty); + + maybe_wait_sync_command_done (was_sync); } return handle_command_errors (e); } diff --git a/gdb/testsuite/gdb.base/attach.exp b/gdb/testsuite/gdb.base/attach.exp index a20c51a..6340496 100644 --- a/gdb/testsuite/gdb.base/attach.exp +++ b/gdb/testsuite/gdb.base/attach.exp @@ -396,6 +396,49 @@ proc do_command_attach_tests {} { remote_exec build "kill -9 ${testpid}" } +# Test ' gdb --pid PID -ex "run" '. GDB used to have a bug where +# "run" would run before the attach finished - PR17347. + +proc test_command_line_attach_run {} { + global gdb_prompt + global binfile + + if ![isnative] then { + unsupported "commandline attach run test" + return 0 + } + + with_test_prefix "cmdline attach run" { + set testpid [spawn_wait_for_attach $binfile] + + set test "run to prompt" + gdb_exit + + set res [gdb_spawn_with_cmdline_opts \ + "-iex \"set height 0\" -iex \"set width 0\" --pid=$testpid -ex \"start\""] + if { $res != 0} { + fail $test + return $res + } + gdb_test_multiple "" $test { + -re {Attaching to.*Start it from the beginning\? \(y or n\) } { + pass $test + } + } + + send_gdb "y\n" + + set test "run to main" + gdb_test_multiple "" $test { + -re "Temporary breakpoint .* main .*$gdb_prompt $" { + pass $test + } + } + + # Get rid of the process + remote_exec build "kill -9 ${testpid}" + } +} # Start with a fresh gdb @@ -420,4 +463,6 @@ do_call_attach_tests do_command_attach_tests +test_command_line_attach_run + return 0 diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp index ecf942e..c54ba4b 100644 --- a/gdb/testsuite/lib/gdb.exp +++ b/gdb/testsuite/lib/gdb.exp @@ -3334,6 +3334,22 @@ proc gdb_spawn { } { default_gdb_spawn } +# Spawn GDB with CMDLINE_FLAGS appended to the GDBFLAGS global. + +proc gdb_spawn_with_cmdline_opts { cmdline_flags } { + global GDBFLAGS + + set saved_gdbflags $GDBFLAGS + + append GDBFLAGS $cmdline_flags + + set res [gdb_spawn] + + set GDBFLAGS $saved_gdbflags + + return $res +} + # Start gdb running, wait for prompt, and disable the pagers. # Overridable function -- you can override this function in your diff --git a/gdb/top.c b/gdb/top.c index fc2b84d..d6696cd 100644 --- a/gdb/top.c +++ b/gdb/top.c @@ -373,6 +373,23 @@ check_frame_language_change (void) } } +/* See top.h. */ + +void +maybe_wait_sync_command_done (int was_sync) +{ + /* If the interpreter is in sync mode (we're running a user + command's list, running command hooks or similars), and we + just ran a synchronous command that started the target, wait + for that command to end. */ + if (!interpreter_async && !was_sync && sync_execution) + { + while (gdb_do_one_event () >= 0) + if (!sync_execution) + break; + } +} + /* Execute the line P as a command, in the current user context. Pass FROM_TTY as second argument to the defining function. */ @@ -459,16 +476,7 @@ execute_command (char *p, int from_tty) else cmd_func (c, arg, from_tty); - /* If the interpreter is in sync mode (we're running a user - command's list, running command hooks or similars), and we - just ran a synchronous command that started the target, wait - for that command to end. */ - if (!interpreter_async && !was_sync && sync_execution) - { - while (gdb_do_one_event () >= 0) - if (!sync_execution) - break; - } + maybe_wait_sync_command_done (was_sync); /* If this command has been post-hooked, run the hook last. */ execute_cmd_post_hook (c); diff --git a/gdb/top.h b/gdb/top.h index c322c33..94f6c48 100644 --- a/gdb/top.h +++ b/gdb/top.h @@ -42,6 +42,14 @@ extern void quit_command (char *, int); extern void quit_cover (void); extern void execute_command (char *, int); +/* If the interpreter is in sync mode (we're running a user command's + list, running command hooks or similars), and we just ran a + synchronous command that started the target, wait for that command + to end. WAS_SYNC indicates whether sync_execution was set before + the command was run. */ + +extern void maybe_wait_sync_command_done (int was_sync); + extern void check_frame_language_change (void); /* Prepare for execution of a command. -- 1.9.3 ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: Regression: GDB stopped on run with attached process (PR 17347) [Re: [pushed+7.8] Re: [PATCH] Fix "attach" command vs user input race 2014-09-08 16:27 ` Regression: GDB stopped on run with attached process (PR 17347) [Re: [pushed+7.8] Re: [PATCH] Fix "attach" command vs user input race Pedro Alves @ 2014-09-09 18:25 ` Jan Kratochvil 2014-09-11 12:36 ` Pedro Alves 0 siblings, 1 reply; 42+ messages in thread From: Jan Kratochvil @ 2014-09-09 18:25 UTC (permalink / raw) To: Pedro Alves; +Cc: Doug Evans, Mark Wielaard, gdb-patches On Mon, 08 Sep 2014 18:26:55 +0200, Pedro Alves wrote: > Here's the updated patch. WDYT? Yes, fine with that. Thanks, Jan ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: Regression: GDB stopped on run with attached process (PR 17347) [Re: [pushed+7.8] Re: [PATCH] Fix "attach" command vs user input race 2014-09-09 18:25 ` Jan Kratochvil @ 2014-09-11 12:36 ` Pedro Alves 2014-09-12 7:34 ` [testsuite patch] runaway attach processes [Re: Regression: GDB stopped on run with attached process (PR 17347)] Jan Kratochvil 0 siblings, 1 reply; 42+ messages in thread From: Pedro Alves @ 2014-09-11 12:36 UTC (permalink / raw) To: Jan Kratochvil; +Cc: Doug Evans, Mark Wielaard, gdb-patches On 09/09/2014 07:25 PM, Jan Kratochvil wrote: > On Mon, 08 Sep 2014 18:26:55 +0200, Pedro Alves wrote: >> Here's the updated patch. WDYT? > > Yes, fine with that. Thanks, pushed to both master and 7.8. Pedro Alves ^ permalink raw reply [flat|nested] 42+ messages in thread
* [testsuite patch] runaway attach processes [Re: Regression: GDB stopped on run with attached process (PR 17347)] 2014-09-11 12:36 ` Pedro Alves @ 2014-09-12 7:34 ` Jan Kratochvil 2014-09-12 10:14 ` Pedro Alves 0 siblings, 1 reply; 42+ messages in thread From: Jan Kratochvil @ 2014-09-12 7:34 UTC (permalink / raw) To: Pedro Alves; +Cc: Doug Evans, Mark Wielaard, gdb-patches [-- Attachment #1: Type: text/plain, Size: 497 bytes --] On Thu, 11 Sep 2014 14:35:53 +0200, Pedro Alves wrote: > Thanks, pushed to both master and 7.8. I have started seeing occasional runaway 'attach' processes these days. I cannot be certain it is really caused by this patch, for example grep 'FAIL.*cmdline attach run' does not show anything in my logs. But as I remember this 'attach' runaway process always happened in GDB (but I do not remember it in the past months) I think it would be most safe to just solve it forever by [attached]. Jan [-- Attachment #2: 1 --] [-- Type: text/plain, Size: 1823 bytes --] gdb/testsuite/ 2014-09-12 Jan Kratochvil <jan.kratochvil@redhat.com> * gdb.base/attach.c: Include unistd.h. (main): Call alarm. Add label postloop. * gdb.base/attach.exp (do_attach_tests): Use gdb_get_line_number, gdb_breakpoint, gdb_continue_to_breakpoint. (test_command_line_attach_run): Kill ${testpid} in one exit path. diff --git a/gdb/testsuite/gdb.base/attach.c b/gdb/testsuite/gdb.base/attach.c index 0041b47..91b180c 100644 --- a/gdb/testsuite/gdb.base/attach.c +++ b/gdb/testsuite/gdb.base/attach.c @@ -5,6 +5,7 @@ exit unless/until gdb sets the variable to non-zero.) */ #include <stdio.h> +#include <unistd.h> int should_exit = 0; @@ -12,9 +13,11 @@ int main () { int local_i = 0; + alarm (60); + while (! should_exit) { local_i++; } - return 0; + return 0; /* postloop */ } diff --git a/gdb/testsuite/gdb.base/attach.exp b/gdb/testsuite/gdb.base/attach.exp index 6340496..5fb5c53 100644 --- a/gdb/testsuite/gdb.base/attach.exp +++ b/gdb/testsuite/gdb.base/attach.exp @@ -256,11 +256,8 @@ proc do_attach_tests {} { # Verify that the modification really happened. - gdb_test "tbreak 19" "Temporary breakpoint .*at.*$srcfile, line 19.*" \ - "after attach2, set tbreak postloop" - - gdb_test "continue" "main.*at.*$srcfile:19.*" \ - "after attach2, reach tbreak postloop" + gdb_breakpoint [gdb_get_line_number "postloop"] temporary + gdb_continue_to_breakpoint "postloop" ".* postloop .*" # Allow the test process to exit, to cleanup after ourselves. @@ -418,6 +415,7 @@ proc test_command_line_attach_run {} { "-iex \"set height 0\" -iex \"set width 0\" --pid=$testpid -ex \"start\""] if { $res != 0} { fail $test + remote_exec build "kill -9 ${testpid}" return $res } gdb_test_multiple "" $test { ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [testsuite patch] runaway attach processes [Re: Regression: GDB stopped on run with attached process (PR 17347)] 2014-09-12 7:34 ` [testsuite patch] runaway attach processes [Re: Regression: GDB stopped on run with attached process (PR 17347)] Jan Kratochvil @ 2014-09-12 10:14 ` Pedro Alves 2014-09-12 11:40 ` [commit] " Jan Kratochvil 0 siblings, 1 reply; 42+ messages in thread From: Pedro Alves @ 2014-09-12 10:14 UTC (permalink / raw) To: Jan Kratochvil; +Cc: Doug Evans, Mark Wielaard, gdb-patches On 09/12/2014 08:34 AM, Jan Kratochvil wrote: > On Thu, 11 Sep 2014 14:35:53 +0200, Pedro Alves wrote: >> > Thanks, pushed to both master and 7.8. > I have started seeing occasional runaway 'attach' processes these days. > I cannot be certain it is really caused by this patch, for example > grep 'FAIL.*cmdline attach run' does not show anything in my logs. > > But as I remember this 'attach' runaway process always happened in GDB (but > I do not remember it in the past months) I think it would be most safe to just > solve it forever by [attached]. Agreed. OK. > gdb/testsuite/ > 2014-09-12 Jan Kratochvil <jan.kratochvil@redhat.com> > > * gdb.base/attach.c: Include unistd.h. > (main): Call alarm. Add label postloop. > * gdb.base/attach.exp (do_attach_tests): Use gdb_get_line_number, > gdb_breakpoint, gdb_continue_to_breakpoint. > (test_command_line_attach_run): Kill ${testpid} in one exit path. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 42+ messages in thread
* [commit] [testsuite patch] runaway attach processes [Re: Regression: GDB stopped on run with attached process (PR 17347)] 2014-09-12 10:14 ` Pedro Alves @ 2014-09-12 11:40 ` Jan Kratochvil 0 siblings, 0 replies; 42+ messages in thread From: Jan Kratochvil @ 2014-09-12 11:40 UTC (permalink / raw) To: Pedro Alves; +Cc: Doug Evans, Mark Wielaard, gdb-patches On Fri, 12 Sep 2014 12:14:18 +0200, Pedro Alves wrote: > On 09/12/2014 08:34 AM, Jan Kratochvil wrote: > > gdb/testsuite/ > > 2014-09-12 Jan Kratochvil <jan.kratochvil@redhat.com> > > > > * gdb.base/attach.c: Include unistd.h. > > (main): Call alarm. Add label postloop. > > * gdb.base/attach.exp (do_attach_tests): Use gdb_get_line_number, > > gdb_breakpoint, gdb_continue_to_breakpoint. > > (test_command_line_attach_run): Kill ${testpid} in one exit path. > > Thanks, Checked in: 1cf2f1b045e9e647f6dfd28829ff4592c588dcb7 Thanks, Jan ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] Fix "attach" command vs user input race [Re: Regression for attach from stdin [Re: [pushed] Re: [PATCH v6 0/2] enable target-async by default]] 2014-07-04 13:48 ` [PATCH] Fix "attach" command vs user input race [Re: Regression for attach from stdin [Re: [pushed] Re: [PATCH v6 0/2] enable target-async by default]] Pedro Alves 2014-07-04 21:13 ` Mark Wielaard 2014-07-07 16:39 ` Doug Evans @ 2014-07-07 17:02 ` Jan Kratochvil 2 siblings, 0 replies; 42+ messages in thread From: Jan Kratochvil @ 2014-07-07 17:02 UTC (permalink / raw) To: Pedro Alves; +Cc: Mark Wielaard, gdb-patches On Fri, 04 Jul 2014 15:48:40 +0200, Pedro Alves wrote: > Here's a fix. Let me know whether it fixes guality testing. Mark has already replied the same but I can also confirm it fixes the GCC testsuite. Thanks, Jan ^ permalink raw reply [flat|nested] 42+ messages in thread
* Crash regression for annota1.exp w/vDSO debuginfo [Re: [pushed] Re: [PATCH v6 0/2] enable target-async by default] 2014-05-29 13:44 ` [pushed] Re: [PATCH v6 0/2] enable target-async by default Pedro Alves 2014-07-01 16:28 ` Regression for attach from stdin [Re: [pushed] Re: [PATCH v6 0/2] enable target-async by default] Jan Kratochvil @ 2014-10-05 14:00 ` Jan Kratochvil 2014-10-05 14:14 ` Jan Kratochvil ` (2 more replies) 1 sibling, 3 replies; 42+ messages in thread From: Jan Kratochvil @ 2014-10-05 14:00 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches On Thu, 29 May 2014 15:44:02 +0200, Pedro Alves wrote: > I went ahead and pushed this all in, with Eli's comments addressed. 329ea57934a9d4b250a0b417af1ec47bc2d0ceb6 is the first bad commit commit 329ea57934a9d4b250a0b417af1ec47bc2d0ceb6 Author: Pedro Alves <palves@redhat.com> Date: Thu May 29 19:58:57 2014 +0100 enable target async by default; separate MI and target notions of async CFLAGS=-g ./configure --without-guile --with-separate-debug-dir=/usr/lib/debug;make;cd gdb/testsuite;make site.exp;i=0;while runtest gdb.base/annota1.exp;! grep 'called with no handler' gdb.log ;do i=$[$i+1];echo -----------$i;done Starting program: .../gdb/testsuite/gdb.base/annota1 ^M warning: section not found in /usr/lib/debug/lib/modules/3.16.3-200.fc20.x86_64/vdso/vdso64.so.debug^M [...] FAIL: gdb.base/annota1.exp: run until main breakpoint (timeout) [...] readline: readline_callback_read_char() called with no handler!^M ERROR: Process no longer exists kernel-debuginfo-3.16.3-200.fc20.x86_64 #0 in __GI_raise (sig=sig@entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56 #1 in __GI_abort () at abort.c:89 #2 in rl_callback_read_char () at callback.c:116 #3 in rl_callback_read_char_wrapper (client_data=0x0) at event-top.c:167 #4 in stdin_event_handler (error=0, client_data=0x0) at event-top.c:373 #5 in handle_file_event (data=...) at event-loop.c:763 #6 in process_event () at event-loop.c:340 #7 in gdb_do_one_event () at event-loop.c:361 #8 in start_event_loop () at event-loop.c:429 #9 in cli_command_loop (data=0x0) at event-top.c:182 #10 in current_interp_command_loop () at interps.c:318 #11 in captured_command_loop (data=0x0) at main.c:323 #12 in catch_errors (func=0x6133a6 <captured_command_loop>, func_args=0x0, errstring=0x902525 "", mask=RETURN_MASK_ALL) at exceptions.c:237 #13 in captured_main (data=0x7fff7eb43bd0) at main.c:1151 #14 in catch_errors (func=0x6137be <captured_main>, func_args=0x7fff7eb43bd0, errstring=0x902525 "", mask=RETURN_MASK_ALL) at exceptions.c:237 #15 in gdb_main (args=0x7fff7eb43bd0) at main.c:1159 #16 in main (argc=5, argv=0x7fff7eb43cd8) at gdb.c:32 That file /usr/lib/debug/lib/modules/3.16.3-200.fc20.x86_64/vdso/vdso64.so.debug is required for the crash reproducibility. That message warning: section not found in /usr/lib/debug/lib/modules/3.16.3-200.fc20.x86_64/vdso/vdso64.so.debug^M is does not seem to be right - the file looks OK to me - but that seems unrelated to this regression. Going to check next what vDSO separate debug info problem it is. Jan ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: Crash regression for annota1.exp w/vDSO debuginfo [Re: [pushed] Re: [PATCH v6 0/2] enable target-async by default] 2014-10-05 14:00 ` Crash regression for annota1.exp w/vDSO debuginfo [Re: [pushed] Re: [PATCH v6 0/2] enable target-async by default] Jan Kratochvil @ 2014-10-05 14:14 ` Jan Kratochvil 2014-10-05 16:43 ` Jan Kratochvil 2014-10-09 15:48 ` Pedro Alves 2 siblings, 0 replies; 42+ messages in thread From: Jan Kratochvil @ 2014-10-05 14:14 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches On Sun, 05 Oct 2014 16:00:39 +0200, Jan Kratochvil wrote: > On Thu, 29 May 2014 15:44:02 +0200, Pedro Alves wrote: > > I went ahead and pushed this all in, with Eli's comments addressed. > > 329ea57934a9d4b250a0b417af1ec47bc2d0ceb6 is the first bad commit > commit 329ea57934a9d4b250a0b417af1ec47bc2d0ceb6 > Author: Pedro Alves <palves@redhat.com> > Date: Thu May 29 19:58:57 2014 +0100 > > enable target async by default; separate MI and target notions of async > > CFLAGS=-g ./configure --without-guile --with-separate-debug-dir=/usr/lib/debug;make;cd gdb/testsuite;make site.exp;i=0;while runtest gdb.base/annota1.exp;! grep 'called with no handler' gdb.log ;do i=$[$i+1];echo -----------$i;done Another reproducer can be found in: crash in non-stop mode with continue -a & (readline_callback_read_char() called with no handler!) https://sourceware.org/bugzilla/show_bug.cgi?id=17300 Jan ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: Crash regression for annota1.exp w/vDSO debuginfo [Re: [pushed] Re: [PATCH v6 0/2] enable target-async by default] 2014-10-05 14:00 ` Crash regression for annota1.exp w/vDSO debuginfo [Re: [pushed] Re: [PATCH v6 0/2] enable target-async by default] Jan Kratochvil 2014-10-05 14:14 ` Jan Kratochvil @ 2014-10-05 16:43 ` Jan Kratochvil 2014-10-09 15:48 ` Pedro Alves 2 siblings, 0 replies; 42+ messages in thread From: Jan Kratochvil @ 2014-10-05 16:43 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches On Sun, 05 Oct 2014 16:00:39 +0200, Jan Kratochvil wrote: > That message > warning: section not found in /usr/lib/debug/lib/modules/3.16.3-200.fc20.x86_64/vdso/vdso64.so.debug^M > is does not seem to be right - the file looks OK to me - but that seems > unrelated to this regression. Going to check next what vDSO separate debug > info problem it is. The message happens only with the separate debug info installed but in fact the bug can be seen even without the separate debug info installed. kernel-3.16.3-200.fc20.x86_64 (gdb) info files [...] 0x00007ffff7ffb120 - 0x00007ffff7ffb160 is in system-supplied DSO at 0x7ffff7ffb000 ^^ 0x00007ffff7ffb160 - 0x00007ffff7ffb268 is .dynsym in system-supplied DSO at 0x7ffff7ffb000 0x00007ffff7ffb268 - 0x00007ffff7ffb2c6 is .dynstr in system-supplied DSO at 0x7ffff7ffb000 [ 8] .fake_shstrtab STRTAB 0000000000000780 000780 000076 00 A 0 0 32 This is because sh_name==0 for the first section and .fake_shstrtab errorneously did not contain 0x00 as its very first byte (as required by the ELF standard) and therefore BFD considered the first section nameless. I have verified the problem no longer occurs on: kernel-3.17.0-0.rc7.git3.1.fc22.x86_64 (gdb) info files 0x00007ffff7ffd120 - 0x00007ffff7ffd160 is .hash in system-supplied DSO at 0x7ffff7ffd000 ^^^^^ 0x00007ffff7ffd160 - 0x00007ffff7ffd268 is .dynsym in system-supplied DSO at 0x7ffff7ffd000 0x00007ffff7ffd268 - 0x00007ffff7ffd2c6 is .dynstr in system-supplied DSO at 0x7ffff7ffd000 [15] .shstrtab STRTAB 0000000000000000 001308 0000a2 00 0 0 1 Expecting it is probably since: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=da861e18ecccb5c126b9eb95ff720ce082a46286 x86, vdso: Get rid of the fake section mechanism I have tested that this older kernel also does not have the problem: kernel-3.10.0-123.el7.x86_64 0x00007ffff7ffa120 - 0x00007ffff7ffa160 is .hash in system-supplied DSO at 0x7ffff7ffa000 0x00007ffff7ffa160 - 0x00007ffff7ffa268 is .dynsym in system-supplied DSO at 0x7ffff7ffa000 0x00007ffff7ffa268 - 0x00007ffff7ffa2c6 is .dynstr in system-supplied DSO at 0x7ffff7ffa000 [15] .shstrtab STRTAB 0000000000000000 00105a 0000a3 00 0 0 1 Therefore I do not think it is worth workarounding in GDB. Jan ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: Crash regression for annota1.exp w/vDSO debuginfo [Re: [pushed] Re: [PATCH v6 0/2] enable target-async by default] 2014-10-05 14:00 ` Crash regression for annota1.exp w/vDSO debuginfo [Re: [pushed] Re: [PATCH v6 0/2] enable target-async by default] Jan Kratochvil 2014-10-05 14:14 ` Jan Kratochvil 2014-10-05 16:43 ` Jan Kratochvil @ 2014-10-09 15:48 ` Pedro Alves 2 siblings, 0 replies; 42+ messages in thread From: Pedro Alves @ 2014-10-09 15:48 UTC (permalink / raw) To: Jan Kratochvil; +Cc: gdb-patches On 10/05/2014 03:00 PM, Jan Kratochvil wrote: > On Thu, 29 May 2014 15:44:02 +0200, Pedro Alves wrote: >> I went ahead and pushed this all in, with Eli's comments addressed. > > 329ea57934a9d4b250a0b417af1ec47bc2d0ceb6 is the first bad commit > commit 329ea57934a9d4b250a0b417af1ec47bc2d0ceb6 > Author: Pedro Alves <palves@redhat.com> > Date: Thu May 29 19:58:57 2014 +0100 > > enable target async by default; separate MI and target notions of async > > CFLAGS=-g ./configure --without-guile --with-separate-debug-dir=/usr/lib/debug;make;cd gdb/testsuite;make site.exp;i=0;while runtest gdb.base/annota1.exp;! grep 'called with no handler' gdb.log ;do i=$[$i+1];echo -----------$i;done > > Starting program: .../gdb/testsuite/gdb.base/annota1 ^M > warning: section not found in /usr/lib/debug/lib/modules/3.16.3-200.fc20.x86_64/vdso/vdso64.so.debug^M > [...] > FAIL: gdb.base/annota1.exp: run until main breakpoint (timeout) > [...] > readline: readline_callback_read_char() called with no handler!^M > ERROR: Process no longer exists Thanks for the reproducer. After investigation with a lot of head scratching staring at hacked in logs in select place, in the end this turned out to be real simple to trigger. All it takes is turning on annotations, do "c", and type something while the program is running... I've filed PR 17472 for this. I'll send a series fixing this and a couple other terminal/readline issues shortly. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 42+ messages in thread
end of thread, other threads:[~2014-10-09 15:48 UTC | newest] Thread overview: 42+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-05-23 20:59 [PATCH v6 0/2] enable target-async by default Pedro Alves 2014-05-23 20:59 ` [PATCH v6 1/2] Make display_gdb_prompt CLI-only Pedro Alves 2014-05-23 20:59 ` [PATCH v6 2/2] enable target async by default; separate MI and target notions of async Pedro Alves 2014-05-24 7:03 ` Eli Zaretskii 2014-05-29 13:49 ` Pedro Alves 2014-07-30 18:40 ` Doug Evans 2014-05-29 13:44 ` [pushed] Re: [PATCH v6 0/2] enable target-async by default Pedro Alves 2014-07-01 16:28 ` Regression for attach from stdin [Re: [pushed] Re: [PATCH v6 0/2] enable target-async by default] Jan Kratochvil 2014-07-02 8:59 ` Mark Wielaard 2014-07-02 9:16 ` Pedro Alves 2014-07-03 15:39 ` Pedro Alves 2014-07-04 13:48 ` [PATCH] Fix "attach" command vs user input race [Re: Regression for attach from stdin [Re: [pushed] Re: [PATCH v6 0/2] enable target-async by default]] Pedro Alves 2014-07-04 21:13 ` Mark Wielaard 2014-07-07 16:39 ` Doug Evans 2014-07-08 15:24 ` Pedro Alves 2014-07-09 16:37 ` Doug Evans 2014-07-09 17:09 ` [pushed+7.8] " Pedro Alves 2014-07-29 22:03 ` Doug Evans 2014-07-29 23:10 ` Doug Evans 2014-07-30 12:46 ` Pedro Alves 2014-07-30 12:38 ` Pedro Alves 2014-07-30 16:59 ` Doug Evans 2014-08-21 16:34 ` [PUSHED] infcmd.c: Remove stale TODO Pedro Alves 2014-09-03 7:59 ` Regression: GDB stopped on run with attached process (PR 17347) [Re: [pushed+7.8] Re: [PATCH] Fix "attach" command vs user input race [Re: Regression for attach from stdin [Re: [pushed] Re: [PATCH v6 0/2] enable target-async by default]]] Jan Kratochvil 2014-09-03 20:11 ` Regression: GDB stopped on run with attached process (PR 17347) [Re: [pushed+7.8] Re: [PATCH] Fix "attach" command vs user input race Pedro Alves 2014-09-07 19:28 ` Jan Kratochvil 2014-09-08 16:19 ` [PATCH 1/2] testsuite: refactor spawn and wait for attach (was: Re: Regression: GDB stopped on run with attached process) Pedro Alves 2014-09-09 17:29 ` Jan Kratochvil 2014-09-09 17:35 ` [PATCH 1/2] testsuite: refactor spawn and wait for attach Pedro Alves 2014-09-10 21:25 ` Pedro Alves 2014-09-11 12:34 ` Pedro Alves 2014-09-08 16:27 ` Regression: GDB stopped on run with attached process (PR 17347) [Re: [pushed+7.8] Re: [PATCH] Fix "attach" command vs user input race Pedro Alves 2014-09-09 18:25 ` Jan Kratochvil 2014-09-11 12:36 ` Pedro Alves 2014-09-12 7:34 ` [testsuite patch] runaway attach processes [Re: Regression: GDB stopped on run with attached process (PR 17347)] Jan Kratochvil 2014-09-12 10:14 ` Pedro Alves 2014-09-12 11:40 ` [commit] " Jan Kratochvil 2014-07-07 17:02 ` [PATCH] Fix "attach" command vs user input race [Re: Regression for attach from stdin [Re: [pushed] Re: [PATCH v6 0/2] enable target-async by default]] Jan Kratochvil 2014-10-05 14:00 ` Crash regression for annota1.exp w/vDSO debuginfo [Re: [pushed] Re: [PATCH v6 0/2] enable target-async by default] Jan Kratochvil 2014-10-05 14:14 ` Jan Kratochvil 2014-10-05 16:43 ` Jan Kratochvil 2014-10-09 15:48 ` Pedro Alves
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).