From 6423214fcb912a5d402ffcb84d6f61f2b900a900 Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Tue, 7 Apr 2015 18:19:30 +0100 Subject: [PATCH] testsuite: Don't use expect_background to reap gdbserver I adjusted a test to do 'expect -i $server_spawn_id -re ...', and saw really strange behavior. Whether that expect would work, depended on whether GDB would also send output and the same expect matched it too (on $gdb_spawn_id). I was perplexed until I noticed that gdbserver_spawn spawns gdbserver and then uses expect_background to reap gdbserver. That expect_background conflicts/races with any "expect -i $server_spawn_id" done anywhere else in parallel... In order to make it possible for tests to read inferior I/O out of $server_spawn_id, we to get rid of that expect_background. This patch makes us instead reap gdbserver's spawn id when GDB exits. If GDB is still around, this gives a chance for gdbserver to exit cleanly. The current code in gdb_finish uses "kill", but that doesn't work with extended-remote (gdbserver doesn't exit). We now use "monitor exit" instead which works in both remote and extended-remote modes. gdb/testsuite/ChangeLog: 2015-04-07 Pedro Alves * lib/gdb.exp (gdb_finish): Delete persistent gdbserver handling. * lib/gdbserver-support.exp (gdbserver_start): Make $server_spawn_id global. (gdbserver_start): Don't wait for gdbserver's spawn id with expect_background. (close_gdbserver): New procedure. (gdb_exit): Rename the default version and reimplement. --- gdb/testsuite/ChangeLog | 10 ++++ gdb/testsuite/lib/gdb.exp | 14 ------ gdb/testsuite/lib/gdbserver-support.exp | 61 +++++++++++++++++++------ 3 files changed, 58 insertions(+), 27 deletions(-) diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 694b1c80f3..eb084d2e5a 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,13 @@ +2015-04-07 Pedro Alves + + * lib/gdb.exp (gdb_finish): Delete persistent gdbserver handling. + * lib/gdbserver-support.exp (gdbserver_start): Make + $server_spawn_id global. + (gdbserver_start): Don't wait for gdbserver's spawn id with + expect_background. + (close_gdbserver): New procedure. + (gdb_exit): Rename the default version and reimplement. + 2015-04-07 Pedro Alves * lib/gdb.exp (gdb_test_multiple): When processing an argument, diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp index 203b8cf080..1669f45661 100644 --- a/gdb/testsuite/lib/gdb.exp +++ b/gdb/testsuite/lib/gdb.exp @@ -4000,20 +4000,6 @@ proc gdb_finish { } { global gdb_prompt global cleanfiles - # Give persistent gdbserver a chance to terminate before GDB is killed. - if {[info exists gdbserver_reconnect_p] && $gdbserver_reconnect_p - && [info exists gdb_spawn_id]} { - send_gdb "kill\n"; - gdb_expect 10 { - -re "y or n" { - send_gdb "y\n"; - exp_continue; - } - -re "$gdb_prompt $" { - } - } - } - # Exit first, so that the files are no longer in use. gdb_exit diff --git a/gdb/testsuite/lib/gdbserver-support.exp b/gdb/testsuite/lib/gdbserver-support.exp index 4a591541cd..f19b796168 100644 --- a/gdb/testsuite/lib/gdbserver-support.exp +++ b/gdb/testsuite/lib/gdbserver-support.exp @@ -270,6 +270,7 @@ proc gdbserver_start { options arguments } { append gdbserver_command " $arguments" } + global server_spawn_id set server_spawn_id [remote_spawn target $gdbserver_command] # Wait for the server to open its TCP socket, so that GDB can connect. @@ -294,19 +295,6 @@ proc gdbserver_start { options arguments } { break } - # We can't just call close, because if gdbserver is local then that means - # that it will get a SIGHUP. Doing it this way could also allow us to - # get at the inferior's input or output if necessary, and means that we - # don't need to redirect output. - expect_background { - -i $server_spawn_id - full_buffer { } - eof { - # The spawn ID is already closed now (but not yet waited for). - wait -i $expect_out(spawn_id) - } - } - return [list $protocol [$get_remote_address $debughost $portnum]] } @@ -328,6 +316,53 @@ proc gdbserver_spawn { child_args } { return [gdbserver_start "" $arguments] } +# Close the GDBserver connection. + +proc close_gdbserver {} { + global server_spawn_id + + # We can't just call close, because if gdbserver is local then that means + # that it will get a SIGHUP. Doing it this way could also allow us to + # get at the inferior's input or output if necessary, and means that we + # don't need to redirect output. + + if {![info exists server_spawn_id]} { + return + } + + verbose "Quitting GDBserver" + + catch "close -i $server_spawn_id" + catch "wait -i $server_spawn_id" + unset server_spawn_id +} + +# Hook into GDB exit, and close GDBserver. + +if { [info procs gdbserver_gdb_exit] == "" } { + rename gdb_exit gdbserver_orig_gdb_exit +} +proc gdb_exit {} { + global gdb_spawn_id server_spawn_id + global gdb_prompt + + if {[info exists gdb_spawn_id] && [info exists server_spawn_id]} { + send_gdb "monitor exit\n"; + gdb_expect { + -re "$gdb_prompt $" { + exp_continue + } + -i "$server_spawn_id" eof { + wait -i $expect_out(spawn_id) + unset server_spawn_id + } + } + } + close_gdbserver + + gdbserver_orig_gdb_exit +} + # Start a gdbserver process running HOST_EXEC and pass CHILD_ARGS # to it. Return 0 on success, or non-zero on failure: 2 if gdbserver # failed to start or 1 if we couldn't connect to it.