Re: [PATCH 5/6] testsuite: Introduce $inferior_spawn_id

[Pedro Alves <palves at redhat dot com>]

On 02/24/2015 04:31 PM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
> 
> Hi Pedro,
> 
>> +	# GDBserver doesn't do inferior I/O through GDB.  But we can
>> +	# talk to the program using GDBserver's tty instead.
>> +	global inferior_spawn_id
>> +	set inferior_spawn_id $server_spawn_id
> 
> Does it still work well if GDBserver is started without tty?  In my
> remote testing, gdbserver is started without tty,
> 
> spawn /usr/bin/ssh -l yao junor1 /gdbserver/aarch64/gdbserver --once :2346 aarch64-linux-gnu/gdb/testsuite/gdb.base/interrupt
> 
> I see the following timeouts:
> 
> Continuing.^M
> Remote debugging from host 10.2.206.34^M
> FAIL: gdb.base/interrupt.exp: process is alive (timeout)
> a^M
> a^M
> FAIL: gdb.base/interrupt.exp: child process ate our char (timeout)

I tried it now, and the problem is that when the gdbserver/inferior is
started without a tty, then stdout is put in unbuffered mode, so the
printf calls don't flush...

Adding:
  setvbuf (stdout, NULL, _IONBF, BUFSIZ);

to interrupt.c fixes it.

This is the same problem we see when testing with Windows (native or
remote) from Cygwin, given in that case stdin/stdout is connected
to a pipe.

Long ago I added the set_unbuffered_mode.c hack to for this.  So enabling
that hack fixes this too then.  See patch below. (I also hacked
native-gdbserver.exp locally to use ssh with no -t to test it).  That would
require boards to set that  gdb,force_unbuffered_mode flag if they need it.

But I'm not sure we want to expose that to boards.  We could also
always enable the hack for gdbserver in gdbserver-support.exp.

Or we could fix the tests themselves to explicitly call setvbuf
if needed and not bother boards at all.  I count only around 20
tests that check gdb,noinferiorio, or use gdb_skip_stdio_test, and
we could fix them incrementally, as they're converted to
use $inferior_spawn_id.  Maybe that's the cleanest.  We can
e.g., add:

 #include "lib/set_unbuffered_mode.c"

at the top of such files, which avoids an explicit call in
"main".   That relies on __attribute__ ((constructor)), but
we could also call an helper shared function that does the
setvbuf from the tests' "main" if we don't want to rely on
that attribute.

Options, options...

> 
> We need to override ${board}_spawn and pass "-t" to ssh.  After this change,
> all interrupt.exp tests pass.  Since the test harness assumes GDBserver
> has tty, probably we should document such requirement somewhere.

That's an option too, but it makes me a bit nervous.  I'm not sure
if we can assume that.

> 
> However, I don't run the whole testsuite with the updated board file
> (with -t option to ssh).
> 


>From c4f1429d5274f9729cd3254813bacfda48f7ef95 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Thu, 26 Feb 2015 23:18:12 +0000
Subject: [PATCH] Add a way for boards to request usage of the
 force-unbuffered-mode trick

---
 gdb/testsuite/boards/native-gdbserver.exp |  2 ++
 gdb/testsuite/lib/gdb.exp                 | 16 ++++++++++++----
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/gdb/testsuite/boards/native-gdbserver.exp b/gdb/testsuite/boards/native-gdbserver.exp
index f00ef60..b3bc889 100644
--- a/gdb/testsuite/boards/native-gdbserver.exp
+++ b/gdb/testsuite/boards/native-gdbserver.exp
@@ -26,6 +26,8 @@ load_board_description "gdbserver-base"
 # This gdbserver can only run a process once per session.
 set_board_info gdb,do_reload_on_run 1
 
+set_board_info gdb,force_unbuffered_mode 1
+
 # There's no support for argument-passing (yet).
 set_board_info noargs 1
 
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index e4722d2..e7d5e48 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -2850,6 +2850,13 @@ proc gdb_wrapper_init { args } {
 global gdb_saved_set_unbuffered_mode_obj
 set gdb_saved_set_unbuffered_mode_obj ""
 
+proc force_unbuffered_mode_p {} {
+    if {[target_info exists gdb,force_unbuffered_mode] && [target_info gdb,force_unbuffered_mode]} {
+	return 1
+    }
+    return 0
+}
+
 proc gdb_compile {source dest type options} {
     global GDB_TESTCASE_OPTIONS
     global gdb_wrapper_file
@@ -2947,13 +2954,14 @@ proc gdb_compile {source dest type options} {
     }
 
     if { $type == "executable" } {
-	if { ([istarget "*-*-mingw*"]
-	      || [istarget "*-*-*djgpp"]
-	      || [istarget "*-*-cygwin*"])} {
+	if { [force_unbuffered_mode_p]
+	     || [istarget "*-*-mingw*"]
+	     || [istarget "*-*-*djgpp"]
+	     || [istarget "*-*-cygwin*"]} {
 	    # Force output to unbuffered mode, by linking in an object file
 	    # with a global contructor that calls setvbuf.
 	    #
-	    # Compile the special object seperatelly for two reasons:
+	    # Compile the special object seperately for two reasons:
 	    #  1) Insulate it from $options.
 	    #  2) Avoid compiling it for every gdb_compile invocation,
 	    #  which is time consuming, especially if we're remote
-- 
1.9.3