[binutils-gdb] [gdb/testsuite] Fix silent timeout in gdb.multi/multi-target.exp

[binutils-gdb] [gdb/testsuite] Fix silent timeout in gdb.multi/multi-target.exp

[Tom de Vries]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=f32682eacae76881752bae2a72c485b98badb2c3

commit f32682eacae76881752bae2a72c485b98badb2c3
Author: Tom de Vries <tdevries@suse.de>
Date:   Thu Apr 2 09:46:00 2020 +0200

    [gdb/testsuite] Fix silent timeout in gdb.multi/multi-target.exp
    
    While running test-case gdb.multi/multi-target.exp, I observed a silent
    timeout related to "monitor exit".
    
    By making the timeout explicit in an expect clause in gdbserver_gdb_exit:
    ...
    +  timeout {
    +    warning "Timed out waiting for EOF in server after $monitor_exit"
    +  }
    ...
    we get in the log:
    ...
    monitor exit^M
    "monitor" command not supported by this target.^M
    (gdb) WARNING: Timed out waiting for EOF in server after monitor exit
    ...
    
    What happens is the following:
    - the inferior 5 is selected
    - a breakpoint is set in inferior 1
    - the breakpoint triggers and we switch to inferior 1
    - setup is called by test_continue, which calls clean_restart, which calls
      gdbserver_gdb_exit (due to load_lib gdbserver-support.exp)
    - gdbserver_gdb_exit issues "monitor exit"
    - gdb responds with "not supported by this target" because inferior 1 is
      native
    
    Fix this by keeping a list of server_spawn_id, and cleaning those up before
    calling gdbserver_gdb_exit.
    
    This reduces testing time from 1m22s to 32s.
    
    gdb/testsuite/ChangeLog:
    
    2020-04-02  Tom de Vries  <tdevries@suse.de>
    
            * lib/gdbserver-support.exp (gdbserver_exit): Factor out of ...
            (gdbserver_gdb_exit): ... here.  Add timeout warning.
            * gdb.multi/multi-target.exp (server_spawn_ids): New global var.
            (connect_target_extended_remote): Append new server_spawn_id to
            server_spawn_ids.
            (cleanup): New proc.
            (setup, <toplevel>): Call cleanup.

Diff:
---
 gdb/testsuite/ChangeLog                  | 10 ++++++++
 gdb/testsuite/gdb.multi/multi-target.exp | 24 +++++++++++++++++--
 gdb/testsuite/lib/gdbserver-support.exp  | 40 +++++++++++++++++++++-----------
 3 files changed, 59 insertions(+), 15 deletions(-)

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 5dcb844bfde..99813054d5d 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,13 @@
+2020-04-02  Tom de Vries  <tdevries@suse.de>
+
+	* lib/gdbserver-support.exp (gdbserver_exit): Factor out of ...
+	(gdbserver_gdb_exit): ... here.  Add timeout warning.
+	* gdb.multi/multi-target.exp (server_spawn_ids): New global var.
+	(connect_target_extended_remote): Append new server_spawn_id to
+	server_spawn_ids.
+	(cleanup): New proc.
+	(setup, <toplevel>): Call cleanup.
+
 2020-04-02  Tom de Vries  <tdevries@suse.de>
 
 	* gdb.base/main-psymtab.exp: New file.
diff --git a/gdb/testsuite/gdb.multi/multi-target.exp b/gdb/testsuite/gdb.multi/multi-target.exp
index 6c727b5e3bd..b519eda4e8d 100644
--- a/gdb/testsuite/gdb.multi/multi-target.exp
+++ b/gdb/testsuite/gdb.multi/multi-target.exp
@@ -33,8 +33,13 @@ if { [prepare_for_testing "failed to prepare" ${binfile} "${srcfile}" \
     return
 }
 
-proc connect_target_extended_remote {binfile} {
+# Keep a list of (inferior ID, spawn ID).
+set server_spawn_ids [list]
+
+proc connect_target_extended_remote {binfile num} {
     set res [gdbserver_start "--multi" ""]
+    global server_spawn_ids server_spawn_id
+    lappend server_spawn_ids $num $server_spawn_id
     set gdbserver_gdbport [lindex $res 1]
     return [gdb_target_cmd "extended-remote" $gdbserver_gdbport]
 }
@@ -58,7 +63,7 @@ proc add_inferior {num target binfile {gcorefile ""}} {
     }
 
     if {$target == "extended-remote"} {
-	if {[connect_target_extended_remote $binfile]} {
+	if {[connect_target_extended_remote $binfile $num]} {
 	    return 0
 	}
     }
@@ -99,12 +104,25 @@ proc next_live_inferior {inf} {
     return $inf
 }
 
+# Clean up the server_spawn_ids.
+proc cleanup_gdbservers { } {
+    global server_spawn_id
+    global server_spawn_ids
+    foreach { inferior_id spawn_id } $server_spawn_ids {
+	set server_spawn_id $spawn_id
+	gdb_test "inferior $inferior_id"
+	gdbserver_exit 0
+    }
+    set server_spawn_ids [list]
+}
+
 # Return true on success, false otherwise.
 
 proc setup {non-stop} {
     global gcorefile gcore_created
     global binfile
 
+    cleanup_gdbservers
     clean_restart ${binfile}
 
     # multi-target depends on target running in non-stop mode.  Force
@@ -448,3 +466,5 @@ with_test_prefix "info-inferiors" {
 	test_info_inferiors $multi_process
     }
 }
+
+cleanup_gdbservers
diff --git a/gdb/testsuite/lib/gdbserver-support.exp b/gdb/testsuite/lib/gdbserver-support.exp
index 706bbeb9df4..a2cc80f28d7 100644
--- a/gdb/testsuite/lib/gdbserver-support.exp
+++ b/gdb/testsuite/lib/gdbserver-support.exp
@@ -431,21 +431,11 @@ if { [info procs gdbserver_orig_gdb_exit] == "" } {
     rename mi_gdb_exit gdbserver_orig_mi_gdb_exit
 }
 
-proc gdbserver_gdb_exit { is_mi } {
+# Cleanup gdbserver $server_spawn_id
+
+proc gdbserver_exit { is_mi } {
     global gdb_spawn_id server_spawn_id
     global gdb_prompt
-    global gdbserver_reconnect_p
-
-    # Leave GDBserver running if we're exiting GDB in order to
-    # reconnect to the same instance of GDBserver again.
-    if {[info exists gdbserver_reconnect_p] && $gdbserver_reconnect_p} {
-	if { $is_mi } {
-	    gdbserver_orig_mi_gdb_exit
-	} else {
-	    gdbserver_orig_gdb_exit
-	}
-	return
-    }
 
     if {[info exists gdb_spawn_id] && [info exists server_spawn_id]} {
 	# GDB may be terminated in an expected way or an unexpected way,
@@ -469,10 +459,34 @@ proc gdbserver_gdb_exit { is_mi } {
 		    wait -i $expect_out(spawn_id)
 		    unset server_spawn_id
 		}
+               timeout {
+                   warning "Timed out waiting for EOF in server after $monitor_exit"
+               }
 	    }
 	}
     }
     close_gdbserver
+}
+
+# Local version of gdb_exit that also cleans up gdbserver $server_spawn_id.
+
+proc gdbserver_gdb_exit { is_mi } {
+    global gdb_spawn_id server_spawn_id
+    global gdb_prompt
+    global gdbserver_reconnect_p
+
+    # Leave GDBserver running if we're exiting GDB in order to
+    # reconnect to the same instance of GDBserver again.
+    if {[info exists gdbserver_reconnect_p] && $gdbserver_reconnect_p} {
+	if { $is_mi } {
+	    gdbserver_orig_mi_gdb_exit
+	} else {
+	    gdbserver_orig_gdb_exit
+	}
+	return
+    }
+
+    gdbserver_exit $is_mi
 
     if { $is_mi } {
 	gdbserver_orig_mi_gdb_exit