Re: [RFA: mi testsuite] Gdbserver support for the MI testsuite

[Daniel Jacobowitz <drow at mvista dot com>]

On Mon, Sep 08, 2003 at 03:53:38PM -0400, Elena Zannoni wrote:
> Daniel Jacobowitz writes:
>  > Updated and improved version of a patch I've posted several times over the
>  > last year and a half or so.  This adds support for gdbserver-style remote
>  > stubs to the MI testsuite; the differences between the MI support and non-MI
>  > support are mostly cosmetic at this point (i.e. it doesn't go out of its way
>  > to use -target-select).  Just handles the different prompt.
>  > 
>  > There's still an unnecessary restart of gdbserver every test; fixing that is
>  > going to take some serious work.
>  > 
>  > Results with the patch are OK: linux-proc tests fail, not surprisingly;
>  > gcore fails; there are some new thread failures since I last updated it
>  > which I will have to investigate.  But most of the MI tests pass instead of
>  > timing out.
>  > 
>  > Does this patch look OK for 6.0?
>  > 
> 
> I still have the same objection I had back then. I'd like to see the
> mi_* functions be moved to mi-support.exp.
> see: http://sources.redhat.com/ml/gdb-patches/2002-07/msg00227.html
> 
> I'd still like to have mi-support.exp be loaded only when the mi tests
> are being done. 
> 
> The flo is like this: an mi test file calls mi_gdb_load (indirectly,
> via mi_run_to_main), which in turn decides what different mechanism to
> use depenging on the target. This stuff you are adding is just another
> kind of target that should be handled from the same central core
> place.  If you really are that desperate to keep the mi_ versions in
> gdbserver.exp and monitor.exp, could those be called from
> mi-support.exp at least, so that the path of execution is consistent
> with the rest?

Hi Elena,

I'm a little dense but after a couple of tries I think I've got it :)
The attached patch:
  - breaks mi_gdb_file_cmd out of mi_gdb_load, as before.
  - Moves the new mi_gdb_target_cmd from monitor.exp to mi-support.exp
    (keeps it with all the other MI support, and it's really not
    monitor-related anyway).
  - Adds a case to mi_gdb_load for gdbserver.
  - Breaks gdb_load up into two functions, one for the actions of
    starting a server (common to both cases) and the other for the
    non-MI overriding of gdb_load.

Also included is a change local to gdbserver.exp which improves the way
we start gdbserver to be more reliable (and less prone to leaving
zombies!).  The trick of waiting for an output character doesn't appear
to work 100% (I believe gdbserver prints the message too early) but
with the five-second retry in mi_gdb_target_cmd everything works OK.

Testsuite results, with NPTL disabled (gdbserver doesn't support NPTL
yet; I'll fix it sometime soon...):

                === gdb Summary ===

# of expected passes            9791
# of unexpected failures        89
# of expected failures          62
# of unknown successes          28
# of known failures             40
# of unresolved testcases       1
# of untested testcases         6
# of unsupported tests          6
/opt/src/binutils/x86-as/gdb/testsuite/../../gdb/gdb version 2003-11-23-cvs -nx

Not stellar, but it's definitely progress, and this patch is much
cleaner than the last.

OK?

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

2003-06-22  Daniel Jacobowitz  <drow@mvista.com>

	* config/gdbserver.exp (gdbserver_gdb_load): Rename from gdb_load.
	Remove downloading, guessing the host executable, the calls to
	gdb_file_cmd and gdb_target_cmd, and "load" support.
	(infer_host_exec): New function broken out from gdb_load.
	(gdb_load): New wrapper for gdbserver_gdb_load.
	* lib/mi-support.exp (mi_gdb_target_cmd): New function, based on
	gdb_target_cmd.
	(mi_gdb_file_cmd): New function, broken out from mi_gdb_load.
	Download binaries to the host.  Clear last_mi_remote_file when
	we load a new binary.
	(mi_gdb_load): Call mi_gdb_file_cmd.  If gdbserver.exp is loaded,
	call gdbserver_gdb_load and mi_gdb_target_cmd.

Index: config/gdbserver.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/config/gdbserver.exp,v
retrieving revision 1.5
diff -u -p -r1.5 gdbserver.exp
--- config/gdbserver.exp	19 Jul 2002 19:40:28 -0000	1.5
+++ config/gdbserver.exp	24 Nov 2003 03:43:08 -0000
@@ -2,7 +2,7 @@
 # ie. a debug agent running as a native process on the same or
 # a different host.
 
-#   Copyright 2000, 2002 Free Software Foundation, Inc.
+#   Copyright 2000, 2002, 2003 Free Software Foundation, Inc.
 
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -89,12 +89,8 @@ global server_exec;
 global portnum;
 set portnum "2345";
 
-proc gdb_load { arg } {
-    global host_exec;
-    global server_exec;
-    global portnum;
-    global verbose;
-    global gdb_prompt;
+proc gdbserver_gdb_load { server_exec } {
+    global portnum
 
     # Port id -- either specified in baseboard file, or managed here.
     if [target_info exists gdb,socketport] {
@@ -134,35 +130,6 @@ proc gdb_load { arg } {
     # Export the host:port pair.
     set gdbport $debughost$portnum;
 
-    # Remember new exec file.
-    if { $arg == "" } {
-	if { ! [info exists host_exec] } {
-	    send_gdb "info files\n";
-	    gdb_expect 30 {
-		-re "Symbols from \"(\[^\"\]+)\"" {
-		    set host_exec $expect_out(1,string);
-		    exp_continue;
-		}
-		-re "Local exec file:\[\r\n\]+\[ \t\]*`(\[^'\]+)'," {
-		    set host_exec $expect_out(1,string);
-		    exp_continue;
-		}
-		-re "$gdb_prompt $" { }
-	    }
-	}
-    } else {
-	set host_exec $arg
-	if [info exists server_exec] { unset server_exec }
-    }
-
-    if { ! [info exists server_exec] } {
-	if [is_remote target] {
-	    set server_exec [remote_download target $host_exec]
-	} else {
-	    set server_exec $host_exec
-	}
-    }
-
     # Fire off the debug agent
     if [target_info exists gdb_server_args] {
         # This flavour of gdbserver takes as arguments those specified
@@ -175,52 +142,77 @@ proc gdb_load { arg } {
         # and the name of the executable file to be debugged.
 	set server_spawn_id [remote_spawn target \
 	    "$gdbserver $sockethost$portnum $server_exec"]
-    } 
+    }
 
-    # We can't call close, because if gdbserver is local then that means
-    # that it will get a SIGHUP.
-    ## close -i $server_spawn_id
-    wait -nowait -i $server_spawn_id
+    # Wait for the server to produce at least one character of output.
+    expect {
+	-i $server_spawn_id
+	-notransfer
+	-re . { }
+    }
+
+    # 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
+	-re "." { }
+	eof {
+	    # The spawn ID is already closed now (but not yet waited for).
+	    wait -i $expect_out(spawn_id)
+	}
+    }
 
-    # Give it a little time to establish
-    sleep 1
+    return [list $protocol $gdbport];
+}
 
-    # tell gdb what file we are debugging
-    if { $arg != "" } {
-	if [gdb_file_cmd $arg] {
-	    return -1;
+proc infer_host_exec { } {
+    set host_exec ""
+
+    send_gdb "info files\n";
+    gdb_expect 30 {
+	-re "Symbols from \"(\[^\"\]+)\"" {
+	    set host_exec $expect_out(1,string);
+	    exp_continue;
+	}
+	-re "Local exec file:\[\r\n\]+\[ \t\]*`(\[^'\]+)'," {
+	    set host_exec $expect_out(1,string);
+	    exp_continue;
 	}
+	-re "$gdb_prompt $" { }
     }
 
-    # attach to the "serial port"
-    gdb_target_cmd $protocol $gdbport;
+    return $host_exec
+}
 
-    # do the real load if needed
-    if [target_info exists gdb_server_do_load] {
-        send_gdb "load\n"
-        set timeout 2400
-        verbose "Timeout is now $timeout seconds" 2
-        gdb_expect {
-            -re ".*$gdb_prompt $" {
-                if $verbose>1 then {
-                    send_user "Loaded $arg into $GDB\n"
-                }
-                set timeout 30
-                verbose "Timeout is now $timeout seconds" 2
-                return 1
-            }
-            -re "$gdb_prompt $"     {
-                if $verbose>1 then {
-                    perror "GDB couldn't load."
-                }
-            }
-            timeout {
-                if $verbose>1 then {
-                    perror "Timed out trying to load $arg."
-                }
-            }
-        }
+proc gdb_load { arg } {
+    global host_exec
+    global server_exec
+
+    if { $arg == "" && $host_exec == "" } {
+	set host_exec [infer_host_exec]
+    } elseif { $arg != "" } {
+	set host_exec $arg
+	if [info exists server_exec] { unset server_exec }
     }
 
-    return 0;
+    if { ! [info exists server_exec] } {
+	if [is_remote target] {
+	    set server_exec [remote_download target $host_exec]
+	} else {
+	    set server_exec $host_exec
+	}
+    }
+
+    set res [gdbserver_gdb_load $host_exec]
+    set protocol [lindex $res 0]
+    set gdbport [lindex $res 1]
+
+    if { $arg != "" } {
+	if [gdb_file_cmd $arg] {
+	    return -1
+	}
+    }
+    gdb_target_cmd $protocol $gdbport
 }
Index: lib/mi-support.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/lib/mi-support.exp,v
retrieving revision 1.23
diff -u -p -r1.23 mi-support.exp
--- lib/mi-support.exp	22 Jun 2003 00:04:27 -0000	1.23
+++ lib/mi-support.exp	24 Nov 2003 03:43:08 -0000
@@ -1,4 +1,4 @@
-# Copyright 1999, 2000, 2002 Free Software Foundation, Inc.
+# Copyright 1999, 2000, 2002, 2003 Free Software Foundation, Inc.
 
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -281,35 +281,95 @@ proc mi_gdb_reinitialize_dir { subdir } 
     }
 }
 
+# Send GDB the "target" command.
+# FIXME: Some of these patterns are not appropriate for MI.  Based on
+# config/monitor.exp:gdb_target_command.
+# FIXME: Should use -target-select.
+proc mi_gdb_target_cmd { targetname serialport } {
+    global mi_gdb_prompt
+
+    for {set i 1} {$i <= 3} {incr i} {
+	send_gdb "target $targetname $serialport\n"
+	gdb_expect 60 {
+	    -re "Couldn't establish connection to remote.*$mi_gdb_prompt$" {
+		verbose "Connection failed";
+	    }
+	    -re "Remote MIPS debugging.*$mi_gdb_prompt$" {
+		verbose "Set target to $targetname";
+		return 0;
+	    }
+	    -re "Remote debugging using .*$serialport.*$mi_gdb_prompt$" {
+		verbose "Set target to $targetname";
+		return 0;
+	    }
+	    -re "Remote target $targetname connected to.*$mi_gdb_prompt$" {
+		verbose "Set target to $targetname";
+		return 0;
+	    }
+	    -re "Connected to.*$mi_gdb_prompt$" { 
+		verbose "Set target to $targetname";
+		return 0;
+	    }
+	    -re "Ending remote.*$mi_gdb_prompt$" { }
+	    -re "Connection refused.*$mi_gdb_prompt$" {
+		verbose "Connection refused by remote target.  Pausing, and trying again."
+		sleep 5
+		continue
+	    }
+	    -re "Timeout reading from remote system.*$mi_gdb_prompt$" {
+		verbose "Got timeout error from gdb.";
+	    }
+	    -re "\\^done,.*$mi_gdb_prompt$" {
+		verbose "Set target to $targetname"
+		return 0
+	    }
+	    timeout {
+		send_gdb "";
+		break
+	    }
+	}
+    }
+    return 1
+}
+
 #
-# load a file into the debugger.
+# load a file into the debugger (file command only).
 # return a -1 if anything goes wrong.
 #
-proc mi_gdb_load { arg } {
+proc mi_gdb_file_cmd { arg } {
     global verbose
     global loadpath
     global loadfile
     global GDB
     global mi_gdb_prompt
     global last_mi_gdb_file
+    global last_mi_remote_file
     upvar timeout timeout
 
     if { $arg == "" } {
 	set arg $last_mi_gdb_file;
+    } else {
+	set last_mi_gdb_file $arg
+	if { [ info exists last_mi_remote_file ] } {
+	    unset last_mi_remote_file
+	}
     }
 
-    set last_mi_gdb_file $arg;
-
-    # ``gdb_unload''
+    if [is_remote host] {
+	set arg [remote_download host $arg];
+	if { $arg == "" } {
+	    error "download failed"
+	    return -1;
+	}
+    }
 
-    # ``gdb_file_cmd''
 # FIXME: Several of these patterns are only acceptable for console
 # output.  Queries are an error for mi.
     send_gdb "105-file-exec-and-symbols $arg\n"
     gdb_expect 120 {
         -re "Reading symbols from.*done.*$mi_gdb_prompt$" {
             verbose "\t\tLoaded $arg into the $GDB"
-            # All OK
+            return 0
         }
         -re "has no symbol-table.*$mi_gdb_prompt$" {
             perror "$arg wasn't compiled with \"-g\""
@@ -338,14 +398,15 @@ proc mi_gdb_load { arg } {
             return -1
         }
         -re "105-file-exec-and-symbols .*\r\n105\\\^done\r\n$mi_gdb_prompt$" {
-            # We are just giving the prompt back for now
-	    # All OK
-            }
+            # We (MI) are just giving the prompt back for now, instead of giving
+	    # some acknowledgement.
+	    return 0
+	}
         timeout {
             perror "couldn't load $arg into $GDB (timed out)."
             return -1
         }
-        eof {
+	eof {
             # This is an attempt to detect a core dump, but seems not to
             # work.  Perhaps we need to match .* followed by eof, in which
             # gdb_expect does not seem to have a way to do that.
@@ -353,9 +414,46 @@ proc mi_gdb_load { arg } {
             return -1
         }
     }
-    
+}
+
+#
+# load a file into the debugger.
+# return a -1 if anything goes wrong.
+#
+proc mi_gdb_load { arg } {
+    global verbose
+    global loadpath
+    global loadfile
+    global GDB
+    global mi_gdb_prompt
+    upvar timeout timeout
+
+    # ``gdb_unload''
+    if { $arg != "" } {
+	mi_gdb_file_cmd $arg
+    }
+
     # ``load''
-    if { [info procs send_target_sid] != "" } {
+    if { [info procs gdbserver_gdb_load] != "" } {
+	global last_mi_gdb_file
+	global last_mi_remote_file
+
+	if { ! [info exists last_mi_remote_file] } {
+	    if [is_remote target] {
+		set last_mi_remote_file [remote_download target $arg]
+	    } else {
+		set last_mi_remote_file $last_mi_gdb_file
+	    }
+	}
+
+	set res [gdbserver_gdb_load $last_mi_remote_file]
+	set protocol [lindex $res 0]
+	set gdbport [lindex $res 1]
+
+	if { [mi_gdb_target_cmd $protocol $gdbport] != 0 } {
+	    return -1
+	}
+    } elseif { [info procs send_target_sid] != "" } {
 	# For SID, things get complex
 	send_target_sid
 	gdb_expect 60 {