Re: [patch/testsuite] MI inferior's output in remote target

[Yao Qi <yao at codesourcery dot com>]

On 08/29/2011 09:37 PM, Pedro Alves wrote:
> On Friday 19 August 2011 14:16:36, Yao Qi wrote:
> 

[ Sorry for the delayed reply.  The board is moving to a new place, and 
it takes a lot of time to set it up. ]

>> I noticed one test fail when testing gdb with my remote stub,
>>
>>   FAIL: gdb.mi/mi-console.exp: Testing console output inferior output
>> (time out)
>>
>> The remote stub is able to "transfer" remote IO to host via `remote
>> fileio' packets, and output is printed to gdb_stdtarg or gdb_stdtargerr
>> (as indicated in remote-fileio.c:remote_fileio_func_write).  Here are
>> two problems,
>>  1.  In mi-console.exp, it is expected to read inferior's output from
>> another tty (a separate tty from gdb's), but the output is printed in
>> the same tty as gdb's.
>>  2.  In MI, gdb_stdtarg and gdb_stdtargerr is initialized by
>> mi_console_file_new (raw_stdout, "@", '"').  So, the output printed to
>> them is prefixed with "@".
>>
>> I tried to fix these two problems [1], but it doesn't work.  
> 
>> [1] I tried to fix these two problems by opening a new ui_file when
>> inferior-tty is changed, and update gdb_stdtarg/gdb_stdtargerr to it.
> 
> Let's discuss what _should_ be happening, and if indeed this is
> a gdb bug to fix.  I think it does make sense that if the frontend
> wants inferior output in a separate channel, then we shouldn't
> "@"-prefix the inferior output, as the prefix's whole purpose is
> to be able to untangle the inferior's output from regular MI output.
> Does anyone know of any frontend out where such a change would break
> things?

Your description on "what should be happening" above is reasonable and 
correct to me.  I think of this problem again, and it looks 
mi-support.exp:proc mi_gdb_test doesn't handle remote testing when 
checking inferior's output.

At the end of mi_gdb_test, it expects the output from 
mi_inferior_spawn_id.  It is wrong in remote testing, because in 
remote testing, IO is transfered back to gdb.  So in remote testing, 
we should use gdb_expect instead of `expect -i mi_inferior_spawn_id' 
to expect the inferior's output from gdb.  This doesn't fix this 
problem, but make some progress, because test case can get inferior's 
output, but still doesn't match.

> 
> Why didn't your original attempt work?
> 

I look at my work again, and think my original attempt works, but 
expect is unable to get these output.  During test, inferior-tty is 
set to `/dev/pts/55', for example, and update 
gdb_stdtarg/gdb_stdtargerr to a new ui-file created for `/dev/pts/55'.
At that moment, expect is getting gdb's output, and unable to get 
output from another pty in the same process.  That is the limitation
of expect, if I read "Exploring  Exect" book correctly.  If my
understand is correct here, the problem #2 in my first post is unable
to fix.  So I change test case mi-console.exp to match @-prefixed output 
from gdb in remote testing.

>> I suggest that we check the inferior's
>> output if gdb,noinferiorio is false and is not a remote target.
> 
> A quick grep tells me this is probably happening on the sims too.
> 

I don't understand your bit here, could you elaborate a little please?

-- 
Yao (éå) 

2011-09-28  Yao Qi  <yao@codesourcery.com>

        * gdb.mi/mi-console.exp: Set a different pattern to match output in
	remote test.
        * lib/mi-support.exp (default_mi_gdb_start): Set separate_inferior_pty
	when arg is separate-inferior-tty and test is not running for remote
	target.
	(mi_gdb_test): Check inferior's output in remote test.

---
 gdb/testsuite/gdb.mi/mi-console.exp |   13 ++++++++++++-
 gdb/testsuite/lib/mi-support.exp    |   34 ++++++++++++++++++++++++++--------
 2 files changed, 38 insertions(+), 9 deletions(-)

diff --git a/gdb/testsuite/gdb.mi/mi-console.exp b/gdb/testsuite/gdb.mi/mi-console.exp
index d62ff59..c09d575 100644
--- a/gdb/testsuite/gdb.mi/mi-console.exp
+++ b/gdb/testsuite/gdb.mi/mi-console.exp
@@ -47,11 +47,22 @@ if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {deb
 
 mi_run_to_main
 
+set mi_inferior_output "Hello \\\\\"!\[\r\n\]+"
+if { [is_remote target] } {
+    # In remote testing, inferior's output is transfered back to gdb side
+    # by remote fileio packets.  In MI, inferior's output is prefixed with
+    # '@' in gdb, and we can't put inferior's output in a separate pty,
+    # because expect is unable to receive message from it.  As a workaround,
+    # we check the @-prefixed output in remote testing.
+    set mi_inferior_output \
+	".*@\"H\"\[\r\n\]+@\"e\"\[\r\n\]+@\"l\"\[\r\n\]+@\"l\"\[\r\n\]+.*"
+}
+
 # Next over the hello() call which will produce lots of output
 mi_gdb_test "220-exec-next" \
 	    "220\\^running(\r\n\\*running,thread-id=\"all\")?" \
 	    "Testing console output" \
-	    "Hello \\\\\"!\[\r\n\]+"
+	    "$mi_inferior_output"
 
 mi_expect_stop "end-stepping-range" "main" "" ".*mi-console.c" "14" "" \
     "finished step over hello"
diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp
index 7849411..92eacb0 100644
--- a/gdb/testsuite/lib/mi-support.exp
+++ b/gdb/testsuite/lib/mi-support.exp
@@ -112,7 +112,8 @@ proc default_mi_gdb_start { args } {
 	set inferior_pty [lindex $args 0]
     }
 
-    set separate_inferior_pty [string match $inferior_pty separate-inferior-tty]
+    set separate_inferior_pty [ expr [string match $inferior_pty separate-inferior-tty] \
+				    && ![is_remote target]]
 
     # Start SID.
     if { [info procs sid_start] != "" } {
@@ -802,14 +803,31 @@ proc mi_gdb_test { args } {
     if { $result == 0 } {
 	if [ info exists ipattern ] {
 	    if { ![target_info exists gdb,noinferiorio] } {
-		global mi_inferior_spawn_id
-		expect {
-		    -i $mi_inferior_spawn_id -re "$ipattern" {
-			pass "$message inferior output"
+
+		if { [is_remote target] } {
+		    # If it is a remote target and support inferior io, that
+		    # means remote target has the capability to transfer io
+		    # via remote fileio packets.
+		    gdb_expect {
+			-re "$ipattern" {
+			    pass "$message inferior output"
+			}
+			timeout {
+			    fail "$message inferior output (timeout)"
+			    set result 1
+			}
 		    }
-		    timeout {
-			fail "$message inferior output (timeout)"
-			set result 1
+		} else {
+		    global mi_inferior_spawn_id
+
+		    expect {
+			-i $mi_inferior_spawn_id -re "$ipattern" {
+			    pass "$message inferior output"
+			}
+			timeout {
+			    fail "$message inferior output (timeout)"
+			    set result 1
+			}
 		    }
 		}
 	    } else {
-- 
1.7.0.4