From 448d46bddf2c0d6669209bb2699f2f09b2034e6c Mon Sep 17 00:00:00 2001 From: Jan Kratochvil Date: Sun, 7 Sep 2014 21:29:27 +0200 Subject: [PATCH] Fix GDB SIGTT* Stopped when using the PID argument (BZ 1136704, Pedro Alves). --- gdb-async-stopped-on-pid-arg-1of2.patch | 350 ++++++++++++++++++++++++ gdb-async-stopped-on-pid-arg-2of2.patch | 217 +++++++++++++++ gdb.spec | 11 +- 3 files changed, 577 insertions(+), 1 deletion(-) create mode 100644 gdb-async-stopped-on-pid-arg-1of2.patch create mode 100644 gdb-async-stopped-on-pid-arg-2of2.patch diff --git a/gdb-async-stopped-on-pid-arg-1of2.patch b/gdb-async-stopped-on-pid-arg-1of2.patch new file mode 100644 index 0000000..5c312e3 --- /dev/null +++ b/gdb-async-stopped-on-pid-arg-1of2.patch @@ -0,0 +1,350 @@ +http://sourceware.org/ml/gdb-patches/2014-09/msg00102.html +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 + +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 + + 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 + + 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(-) + +Index: gdb-7.8/gdb/main.c +=================================================================== +--- gdb-7.8.orig/gdb/main.c 2014-09-07 19:12:45.066981588 +0200 ++++ gdb-7.8/gdb/main.c 2014-09-07 19:14:22.613095201 +0200 +@@ -47,6 +47,7 @@ + #include "filenames.h" + #include "filestuff.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 +@@ -350,7 +351,11 @@ catch_command_errors (catch_command_erro + + 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); + } +@@ -369,7 +374,11 @@ catch_command_errors_const (catch_comman + + 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); + } +Index: gdb-7.8/gdb/testsuite/gdb.base/attach.exp +=================================================================== +--- gdb-7.8.orig/gdb/testsuite/gdb.base/attach.exp 2014-09-07 19:12:45.067981589 +0200 ++++ gdb-7.8/gdb/testsuite/gdb.base/attach.exp 2014-09-07 19:12:48.601985706 +0200 +@@ -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 +Index: gdb-7.8/gdb/top.c +=================================================================== +--- gdb-7.8.orig/gdb/top.c 2014-09-07 19:12:45.067981589 +0200 ++++ gdb-7.8/gdb/top.c 2014-09-07 19:12:48.601985706 +0200 +@@ -375,6 +375,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. */ + +@@ -461,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); +Index: gdb-7.8/gdb/top.h +=================================================================== +--- gdb-7.8.orig/gdb/top.h 2014-09-07 19:12:45.068981590 +0200 ++++ gdb-7.8/gdb/top.h 2014-09-07 19:12:48.601985706 +0200 +@@ -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. diff --git a/gdb-async-stopped-on-pid-arg-2of2.patch b/gdb-async-stopped-on-pid-arg-2of2.patch new file mode 100644 index 0000000..6683326 --- /dev/null +++ b/gdb-async-stopped-on-pid-arg-2of2.patch @@ -0,0 +1,217 @@ +http://sourceware.org/ml/gdb-patches/2014-09/msg00174.html +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 + + +--a8Wt8u1KmwUX3Y2C +Content-Type: text/plain; charset=us-ascii +Content-Disposition: inline + +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 ^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 +.^M +Find the GDB manual and other documentation resources online at:^M +.^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 to continue, or q 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 + +--a8Wt8u1KmwUX3Y2C +Content-Type: text/plain; charset=us-ascii +Content-Disposition: inline; filename=1 + +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 + +--a8Wt8u1KmwUX3Y2C-- + diff --git a/gdb.spec b/gdb.spec index ebab169..850398d 100644 --- a/gdb.spec +++ b/gdb.spec @@ -26,7 +26,7 @@ Version: 7.8 # The release always contains a leading reserved number, start it at 1. # `upstream' is not a part of `name' to stay fully rpm dependencies compatible for the testing. -Release: 20%{?dist} +Release: 21%{?dist} License: GPLv3+ and GPLv3+ with exceptions and GPLv2+ and GPLv2+ with exceptions and GPL+ and LGPLv2+ and BSD and Public Domain and GFDL Group: Development/Debuggers @@ -535,6 +535,10 @@ Patch927: gdb-python-gil.patch # Fix crash on Python frame filters with unreadable arg (BZ 1126177). Patch929: python-framefilter-invalidarg.patch +# Fix GDB SIGTT* Stopped when using the PID argument (BZ 1136704, Pedro Alves). +Patch930: gdb-async-stopped-on-pid-arg-1of2.patch +Patch931: gdb-async-stopped-on-pid-arg-2of2.patch + %if 0%{!?rhel:1} || 0%{?rhel} > 6 # RL_STATE_FEDORA_GDB would not be found for: # Patch642: gdb-readline62-ask-more-rh.patch @@ -822,6 +826,8 @@ find -name "*.info*"|xargs rm -f %patch928 -p1 %patch927 -p1 %patch929 -p1 +%patch930 -p1 +%patch931 -p1 %patch848 -p1 %if 0%{!?el6:1} @@ -1317,6 +1323,9 @@ then fi %changelog +* Sun Sep 7 2014 Jan Kratochvil - 7.8-21.fc21 +- Fix GDB SIGTT* Stopped when using the PID argument (BZ 1136704, Pedro Alves). + * Wed Aug 20 2014 Jan Kratochvil - 7.8-20.fc21 - Fix babeltrace errors (Yao Qi). - Fix crash on Python frame filters with unreadable arg (BZ 1126177).