[pushed] Fix gdbserver/MI testing regression (was: Re: [PATCH v3 31/34] Add testing infrastruture bits for running with MI on a separate UI)

[Pedro Alves <palves at redhat dot com>]

On 06/29/2016 11:50 AM, Pedro Alves wrote:
> On 06/28/2016 09:19 PM, Simon Marchi wrote:
>> I added a few traces to understand what's happening, and it seems that inferior_spawn_id
>> is being unset at two places:

Indeed.

>> The easy way would be to add a "info exists" check before unsetting it, but I don't know if
>> that would only hide a real problem.

I went ahead and pushed the patch below, with takes an even easier route.

I think we'll need to do something better in at least a couple scenarios:

 - If/when gdbserver learns about "set inferior-tty", the separate
   inferior tty spawn id should be used instead of gdbserver's.  gdbserver
   doesn't support that, so in tests that use that (or MI's equivalent),
   inferior output indeed is sent to gdbserver's tty.

 - Tests that disconnect from gdbserver/restart gdb/reconnect to gdbserver
   should end up inferior_spawn_id set to gdbserver's spawn id,
   otherwise tests that rely on inferior I/O, after the reconnect won't work
   properly.  I think there's no such test currently, though, so I'm ignoring
   this for now.

>From 038d48680941f014349256aeb7bab14b3f01d58e Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Thu, 30 Jun 2016 11:55:22 +0100
Subject: [PATCH] Fix gdbserver/MI testing regression

Commit 51f77c3704a6 ("Add testing infrastruture bits for running with
MI on a separate UI") broke MI testing with native-gdbserver:

 $ make check RUNTESTFLAGS="--target_board=native-gdbserver mi-var-child.exp"
	 ...
 Running .../src/binutils-gdb/gdb/testsuite/gdb.mi/mi-var-child.exp ...
 can't unset "inferior_spawn_id": no such variable
     while executing
 "unset inferior_spawn_id"
     (procedure "close_gdbserver" line 20)
     invoked from within
 "close_gdbserver"
 ...

When testing with gdbserver, gdb_exit is overridden with a special
version that calls close_gdbserver, which clears inferior_spawn_id.
The problem is that the commit mentioned above made
gdb_exit/mi_gdb_exit clear inferior_spawn_id too, and clearing a
non-existing variable is a tcl error.

Since gdb_exit/mi_gdb_exit always clears inferior_spawn_id now, the
fix is simply to stop clearing it in close_gdbserver.

gdb/testsuite/
2016-06-30  Pedro Alves  <palves@redhat.com>

	* lib/gdbserver-support.exp (close_gdbserver, gdb_exit): Don't
	unset inferior_spawn_id.
---
 gdb/testsuite/ChangeLog                 | 5 +++++
 gdb/testsuite/lib/gdbserver-support.exp | 6 ++----
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index ab0d9e6..1362f09 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,5 +1,10 @@
 2016-06-30  Pedro Alves  <palves@redhat.com>
 
+	* lib/gdbserver-support.exp (close_gdbserver, gdb_exit): Don't
+	unset inferior_spawn_id.
+
+2016-06-30  Pedro Alves  <palves@redhat.com>
+
 	* lib/mi-support.exp (default_mi_gdb_start): Declare global
 	FORCE_SEPARATE_MI_TTY, not SEPARATE_MI_TTY.
 
diff --git a/gdb/testsuite/lib/gdbserver-support.exp b/gdb/testsuite/lib/gdbserver-support.exp
index 951afe5..b792b23 100644
--- a/gdb/testsuite/lib/gdbserver-support.exp
+++ b/gdb/testsuite/lib/gdbserver-support.exp
@@ -324,7 +324,7 @@ proc gdbserver_spawn { child_args } {
 # Close the GDBserver connection.
 
 proc close_gdbserver {} {
-    global server_spawn_id inferior_spawn_id
+    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
@@ -340,7 +340,6 @@ proc close_gdbserver {} {
     catch "close -i $server_spawn_id"
     catch "wait -i $server_spawn_id"
     unset server_spawn_id
-    unset inferior_spawn_id
 }
 
 # Hook into GDB exit, and close GDBserver.
@@ -349,7 +348,7 @@ if { [info procs gdbserver_gdb_exit] == "" } {
     rename gdb_exit gdbserver_orig_gdb_exit
 }
 proc gdb_exit {} {
-    global gdb_spawn_id server_spawn_id inferior_spawn_id
+    global gdb_spawn_id server_spawn_id
     global gdb_prompt
     global gdbserver_reconnect_p
 
@@ -376,7 +375,6 @@ proc gdb_exit {} {
 		-i "$server_spawn_id" eof {
 		    wait -i $expect_out(spawn_id)
 		    unset server_spawn_id
-		    unset inferior_spawn_id
 		}
 	    }
 	}
-- 
2.5.5