This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: Regression: GDB stopped on run with attached process (PR 17347) [Re: [pushed+7.8] Re: [PATCH] Fix "attach" command vs user input race
- From: Pedro Alves <palves at redhat dot com>
- To: Jan Kratochvil <jan dot kratochvil at redhat dot com>
- Cc: Doug Evans <dje at google dot com>, Mark Wielaard <mjw at redhat dot com>, gdb-patches at sourceware dot org
- Date: Mon, 08 Sep 2014 17:26:55 +0100
- Subject: Re: Regression: GDB stopped on run with attached process (PR 17347) [Re: [pushed+7.8] Re: [PATCH] Fix "attach" command vs user input race
- Authentication-results: sourceware.org; auth=none
- References: <1404291574 dot 3766 dot 35 dot camel at bordewijk dot wildebeest dot org> <53B3CDCC dot 9050502 at redhat dot com> <53B57911 dot 10304 at redhat dot com> <53B6B0B8 dot 2050702 at redhat dot com> <21434 dot 52532 dot 737427 dot 778289 at ruffy dot mtv dot corp dot google dot com> <53BC0D0B dot 7040001 at redhat dot com> <21437 dot 28600 dot 751354 dot 629884 at ruffy dot mtv dot corp dot google dot com> <53BD7749 dot 5000800 at redhat dot com> <20140903075858 dot GA23492 at host2 dot jankratochvil dot net> <540775D7 dot 7040003 at redhat dot com> <20140907192818 dot GA17035 at host2 dot jankratochvil dot net>
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