* [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 ` [PATCH v6 2/2] enable target async by default; separate MI and target notions of async Pedro Alves
@ 2014-05-23 20:59 ` 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 ` Pedro Alves
2014-05-24 7:03 ` Eli Zaretskii
2014-05-23 20:59 ` [PATCH v6 1/2] Make display_gdb_prompt CLI-only Pedro Alves
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
* [PATCH v6 0/2] enable target-async by default
@ 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
` (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
* 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
* [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 2/2] enable target async by default; separate MI and target notions of async Pedro Alves
2014-05-23 20:59 ` [PATCH v6 1/2] Make display_gdb_prompt CLI-only 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
* 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
* 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-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
* 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 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-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-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
* 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] 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: 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: [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: 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: [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-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
* 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 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-23 20:59 ` [PATCH v6 1/2] Make display_gdb_prompt CLI-only Pedro Alves
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).