[PATCH 0/6] Introduce $inferior_spawn_id, make interrupt.exp work with GDBserver

[PATCH 0/6] Introduce $inferior_spawn_id, make interrupt.exp work with GDBserver

[Pedro Alves]

After fixing a gdbserver syscall restart issue, I thought of adding a
new test to exercise syscall restarting.  Then I recalled that we
already have interrupt.exp for that.  However, that test is skipped on
gdbserver, because it relies on inferior i/o.

When testing with gdbserver, inferior are spawned by gdbserver, on
gdbserver's pty, and so gdb_test_multiple/gdb_expect, don't see the
inferior's i/o.

This series introduces a mechanism that allows tests to match inferior
i/o separately from gdb i/o, like:

  send_inferior "echo me\n"
  gdb_test_multiple "continue" "test msg" {
    -i "$inferior_spawn_id" -re "echo me\r\necho\r\n" {
      ...
    }
    -i "$gdb_spawn_id" -re "error.*$gdb_prompt $" {
      ...
    }
  }

and then adjusts interrupt.exp to use the new mechanism.

I took the idea from Don Breazeal's use of $server_spawn_id here
<https://sourceware.org/ml/gdb-patches/2015-01/msg00689.html>, and
generalized it.

A couple bugs had to be fixed along the way, and I took the change to
clean up interrupt.exp to use gdb_test_multiple instead of gdb_expect
too.  interrupt.exp now passes against gdbserver here, on x86_64
Fedora 20 (It fails with gdbserver/-m32 testing in the exact same way
it fails on native/-m32 testing, due to a kernel bug).

Pedro Alves (6):
  gdb.base/interrupt.exp: Fix race
  gdb.base/interrupt.exp: Use gdb_test_multiple instead of gdb_expect
  gdb_test_multiple: Fix user code argument processing
  testsuite: Don't use expect_background to reap gdbserver
  testsuite: Introduce $inferior_spawn_id
  gdb.base/interrupt.exp: Use send_inferior/$inferior_spawn_id

 gdb/testsuite/gdb.base/interrupt.exp    | 141 +++++++++++++++++++-------------
 gdb/testsuite/lib/gdb.exp               |  67 +++++++++++----
 gdb/testsuite/lib/gdbserver-support.exp |  68 ++++++++++++---
 3 files changed, 188 insertions(+), 88 deletions(-)

-- 
1.9.3

[PATCH 6/6] gdb.base/interrupt.exp: Use send_inferior/$inferior_spawn_id

[Pedro Alves]

The gdb.base/interrupt.exp test is important for testing system call
restarting, but because it depends on inferior I/O, it ends up skipped
against gdbserver.  This patch adjusts the test to use send_inferior
and $inferior_spawn_id so it works against GDBserver.

gdb/testsuite/ChangeLog:
2015-02-23  Pedro Alves  <palves@redhat.com>

	* gdb.base/interrupt.exp: Don't skip if $inferior_spawn_id !=
	$gdb_spawn_id.  Use send_inferior and $inferior_spawn_id to
	interact with inferior program.
---
 gdb/testsuite/gdb.base/interrupt.exp | 56 ++++++++++++++++++++++++------------
 1 file changed, 37 insertions(+), 19 deletions(-)

diff --git a/gdb/testsuite/gdb.base/interrupt.exp b/gdb/testsuite/gdb.base/interrupt.exp
index 55c8fca..d093ff5 100644
--- a/gdb/testsuite/gdb.base/interrupt.exp
+++ b/gdb/testsuite/gdb.base/interrupt.exp
@@ -18,12 +18,6 @@ if [target_info exists gdb,nointerrupts] {
     continue
 }
 
-if [target_info exists gdb,noinferiorio] {
-    verbose "Skipping interrupt.exp because of noinferiorio."
-    return
-}
-
-
 standard_testfile
 
 set options { debug }
@@ -49,9 +43,16 @@ if ![file exists $binfile] then {
     gdb_test "shell stty intr '^C'" ".*" \
 	"set interrupt character in interrupt.exp"
     if [runto_main] then {
+	global inferior_spawn_id gdb_spawn_id
+
+	if {[target_info exists gdb,noinferiorio] && $inferior_spawn_id == $gdb_spawn_id} {
+	    verbose "Skipping interrupt.exp because of noinferiorio."
+	    return
+	}
+
 	set msg "process is alive"
 	gdb_test_multiple "continue" $msg {
-	    -re "\r\ntalk to me baby\r\n" {
+	    -i "$inferior_spawn_id" -re "talk to me baby\r\n" {
 		pass $msg
 	    }
 	}
@@ -60,9 +61,9 @@ if ![file exists $binfile] then {
 	# program's output.
 
 	set msg "child process ate our char"
-	send_gdb "a\n"
+	send_inferior "a\n"
 	gdb_test_multiple "" $msg {
-	    -re "^a\r\na\r\n$" {
+	    -i "$inferior_spawn_id" -re "^a\r\na\r\n$" {
 		pass $msg
 	    }
 	}
@@ -120,8 +121,8 @@ if ![file exists $binfile] then {
 		setup_xfail "*-*-hpux*"
 		setup_xfail "*-*-*lynx*"
 		fail "$msg (stays asleep)"
-		# Send_Gdb a newline to wake it up
-		send_gdb "\n"
+		# Send the inferior a newline to wake it up.
+		send_inferior "\n"
 		gdb_test "" " = 4" "call function after waking it"
 	    }
 	}
@@ -148,7 +149,7 @@ if ![file exists $binfile] then {
 	    }
 	}
 
-	send_gdb "data\n"
+	send_inferior "data\n"
 	# The optional leading \r\n is in case we sent a newline above
 	# to wake the program, in which case the program now sends it
 	# back.
@@ -158,10 +159,10 @@ if ![file exists $binfile] then {
 
 	set msg "echo data"
 	gdb_test_multiple "" $msg {
-	    -re "^(\r\n|)data\r\ndata\r\n$" {
+	    -i "$inferior_spawn_id" -re "^(\r\n|)data\r\ndata\r\n$" {
 		pass $msg
 	    }
-	    -re "Undefined command.*$gdb_prompt " {
+	    -i "$gdb_spawn_id" -re "Undefined command.*$gdb_prompt " {
 		fail $msg
 	    }
 	}
@@ -189,23 +190,40 @@ if ![file exists $binfile] then {
 	    }
 
 	    # We should be back in the loop.
-	    send_gdb "more data\n"
+	    send_inferior "more data\n"
 
 	    set msg "echo more data"
 	    gdb_test_multiple "" $msg {
-		-re "^(\r\n|)more data\r\nmore data\r\n$" {
+		-i "$inferior_spawn_id" -re "^(\r\n|)more data\r\nmore data\r\n$" {
 		    pass $msg
 		}
 	    }
 	}
 
+	set saw_eof 0
+	set saw_inferior_exit 0
+
 	set msg "send end of file"
-	send_gdb "\004"
+	send_inferior "\004"
 	gdb_test_multiple "" $msg {
-	    -re "end of file.*$inferior_exited_re normally.*$gdb_prompt $" {
-		pass $msg
+	    -i "$inferior_spawn_id" -re "end of file" {
+		verbose -log "saw eof: $saw_eof"
+		set saw_eof 1
+		verbose -log "saw eof"
+		if {!$saw_inferior_exit} {
+		    exp_continue
+		}
+	    }
+	    -i "$gdb_spawn_id" -re "$inferior_exited_re normally.*$gdb_prompt " {
+		set saw_inferior_exit 1
+		verbose -log "saw inferior exit"
+		if {!$saw_eof} {
+		    exp_continue
+		}
 	    }
 	}
+
+	gdb_assert { $saw_eof && $saw_inferior_exit } $msg
     }
 }
 return 0
-- 
1.9.3

[PATCH 2/6] gdb.base/interrupt.exp: Use gdb_test_multiple instead of gdb_expect

[Pedro Alves]

gdb/testsuite/ChangeLog:
2015-02-23  Pedro Alves  <palves@redhat.com>

	* gdb.base/interrupt.exp: Use gdb_test_multiple instead of
	gdb_expect.
---
 gdb/testsuite/gdb.base/interrupt.exp | 89 ++++++++++++++++++++----------------
 1 file changed, 49 insertions(+), 40 deletions(-)

diff --git a/gdb/testsuite/gdb.base/interrupt.exp b/gdb/testsuite/gdb.base/interrupt.exp
index 82137e6..55c8fca 100644
--- a/gdb/testsuite/gdb.base/interrupt.exp
+++ b/gdb/testsuite/gdb.base/interrupt.exp
@@ -49,24 +49,24 @@ if ![file exists $binfile] then {
     gdb_test "shell stty intr '^C'" ".*" \
 	"set interrupt character in interrupt.exp"
     if [runto_main] then {
-	send_gdb "continue\n"
-	gdb_expect {
-	    -re "\r\ntalk to me baby\r\n$" {
-		pass "child process is alive"
+	set msg "process is alive"
+	gdb_test_multiple "continue" $msg {
+	    -re "\r\ntalk to me baby\r\n" {
+		pass $msg
 	    }
-	    timeout { fail "run (timeout)" }
-	    eof { fail "run (eof)" }
 	}
+
 	# This should appear twice, once for the echo and once for the
 	# program's output.
+
+	set msg "child process ate our char"
 	send_gdb "a\n"
-	gdb_expect {
+	gdb_test_multiple "" $msg {
 	    -re "^a\r\na\r\n$" {
-		pass "child process ate our char"
+		pass $msg
 	    }
-	    timeout { fail "echo a (timeout)" }
-	    eof { fail "echo a (eof)" }
 	}
+
 	# Wait until the program is in the read system call again.
 	sleep 2
 
@@ -79,18 +79,19 @@ if ![file exists $binfile] then {
 	# the read and delivering the cntrl-c.
 
 	send_gdb "\003"
-	gdb_expect {
+	set msg "send_gdb control C"
+	gdb_test_multiple "" $msg {
 	    -re "Program received signal SIGINT.*$gdb_prompt $" {
-		pass "send_gdb control C"
+		pass $msg
 	    }
-	    -re ".*$gdb_prompt $" { fail "send_gdb control C" }
-	    timeout { fail "send_gdb control C (timeout)" }
-	    eof { fail "send_gdb control C (eof)" }
 	}
 
+	set msg "call function when asleep"
 	send_gdb "p func1 ()\n"
-	gdb_expect {
-	    -re " = 4.*$gdb_prompt $" { pass "call function when asleep" }
+	gdb_test_multiple "" $msg {
+	    -re " = 4.*$gdb_prompt $" {
+		pass $msg
+	    }
 	    -re ".*Program received signal SIG(SEGV|ILL).*$gdb_prompt $" {
 		setup_xfail "i*86-pc-linux*-gnu*"
 		fail "child died when we called func1, skipped rest of tests"
@@ -118,12 +119,11 @@ if ![file exists $binfile] then {
 		setup_xfail "*-*-*bsd*"
 		setup_xfail "*-*-hpux*"
 		setup_xfail "*-*-*lynx*"
-		fail "call function when asleep (stays asleep)"
+		fail "$msg (stays asleep)"
 		# Send_Gdb a newline to wake it up
 		send_gdb "\n"
 		gdb_test "" " = 4" "call function after waking it"
 	    }
-#	    eof { fail "call function when asleep (eof)" }
 	}
 
 	# Now try calling the function again.
@@ -137,12 +137,15 @@ if ![file exists $binfile] then {
 	send_gdb "continue\n"
 	# For some reason, i386-*-sysv4 gdb fails to issue the Continuing
 	# message, but otherwise appears normal (FIXME).
-	gdb_expect {
-	    -re "^continue\r\nContinuing.\r\n(\r\n|)$" { pass "continue" }
-	    -re "^continue\r\n\r\n" { fail "continue (missing Continuing.)" }
-	    -re "$gdb_prompt $" { fail "continue" }
-	    timeout { fail "continue (timeout)" }
-	    eof { fail "continue (eof)" }
+
+	set msg "continue"
+	gdb_test_multiple "" "$msg" {
+	    -re "^continue\r\nContinuing.\r\n(\r\n|)$" {
+		pass $msg
+	    }
+	    -re "^continue\r\n\r\n" {
+		fail "$msg (missing Continuing.)"
+	    }
 	}
 
 	send_gdb "data\n"
@@ -152,11 +155,15 @@ if ![file exists $binfile] then {
         # FIXME: The pattern below leads to an expected success on HPUX-11.0
         # but the success is spurious. Need to provide the right reg.expr.
         # here.
-	gdb_expect {
-	    -re "^(\r\n|)data\r\ndata\r\n$" { pass "echo data" }
-	    -re "Undefined command.*$gdb_prompt " { fail "echo data" }
-	    timeout { fail "echo data (timeout)" }
-	    eof { fail "echo data (eof)" }
+
+	set msg "echo data"
+	gdb_test_multiple "" $msg {
+	    -re "^(\r\n|)data\r\ndata\r\n$" {
+		pass $msg
+	    }
+	    -re "Undefined command.*$gdb_prompt " {
+		fail $msg
+	    }
 	}
 
  	if { ! [target_info exists gdb,nosignals] } {
@@ -176,26 +183,28 @@ if ![file exists $binfile] then {
 	    # return to the loop.
 	    set msg "signal SIGINT"
 	    gdb_test_multiple "signal SIGINT" "$msg" {
-		-re "^signal SIGINT\r\nContinuing with signal SIGINT.\r\n(\r\n|)$" { pass "$msg" }
+		-re "^signal SIGINT\r\nContinuing with signal SIGINT.\r\n(\r\n|)$" {
+		    pass "$msg"
+		}
 	    }
 
 	    # We should be back in the loop.
 	    send_gdb "more data\n"
-	    gdb_expect {
-		-re "^(\r\n|)more data\r\nmore data\r\n$" { pass "echo more data" }
-		timeout { fail "echo more data (timeout)" }
-		eof { fail "echo more data (eof)" }
+
+	    set msg "echo more data"
+	    gdb_test_multiple "" $msg {
+		-re "^(\r\n|)more data\r\nmore data\r\n$" {
+		    pass $msg
+		}
 	    }
 	}
 
+	set msg "send end of file"
 	send_gdb "\004"
-	gdb_expect {
+	gdb_test_multiple "" $msg {
 	    -re "end of file.*$inferior_exited_re normally.*$gdb_prompt $" {
-		pass "send end of file"
+		pass $msg
 	    }
-	    -re "$gdb_prompt $" { fail "send end of file" }
-	    timeout { fail "send end of file (timeout)" }
-	    eof { fail "send end of file (eof)" }
 	}
     }
 }
-- 
1.9.3

[PATCH 1/6] gdb.base/interrupt.exp: Fix race

[Pedro Alves]

Working on splitting gdb and inferior output handling in this test, I
noticed a race that happens to be masked out today.

The test sends "a\n" to the inferior, and then inferior echoes back
"a\n".

If expect manages to read only the first "a\r\n" into its buffer, then
this matches:

    -re "^a\r\n(|a\r\n)$" {

and leaves the second "a\r\n" in output.

Then the next test that processes inferior I/O sends "data\n", and expects:

    -re "^(\r\n|)data\r\n(|data\r\n)$"

which fails given the anchor and given "a\r\n" is still in the buffer.

This is masked today because the test relies on inferior I/O being
done on GDB's terminal, and there are tested GDB commands in between,
which consume the "a\r\n" that was left in the output.

We don't support SunOS4 anymore, so just remove the workaround.

gdb/testsuite/ChangeLog
2015-02-23  Pedro Alves  <palves@redhat.com>

	* gdb.base/interrupt.exp: Don't handle the case of the inferior
	output appearing once only.
---
 gdb/testsuite/gdb.base/interrupt.exp | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/gdb/testsuite/gdb.base/interrupt.exp b/gdb/testsuite/gdb.base/interrupt.exp
index 4c0fc77..82137e6 100644
--- a/gdb/testsuite/gdb.base/interrupt.exp
+++ b/gdb/testsuite/gdb.base/interrupt.exp
@@ -58,12 +58,10 @@ if ![file exists $binfile] then {
 	    eof { fail "run (eof)" }
 	}
 	# This should appear twice, once for the echo and once for the
-	# program's output.  Under dejagnu (but not interactively) for
-	# SunOS4, it only appears once.  Don't worry about it, I imagine
-	# dejagnu has just done something to the tty modes.
+	# program's output.
 	send_gdb "a\n"
 	gdb_expect {
-	    -re "^a\r\n(|a\r\n)$" {
+	    -re "^a\r\na\r\n$" {
 		pass "child process ate our char"
 	    }
 	    timeout { fail "echo a (timeout)" }
@@ -155,7 +153,7 @@ if ![file exists $binfile] then {
         # but the success is spurious. Need to provide the right reg.expr.
         # here.
 	gdb_expect {
-	    -re "^(\r\n|)data\r\n(|data\r\n)$" { pass "echo data" }
+	    -re "^(\r\n|)data\r\ndata\r\n$" { pass "echo data" }
 	    -re "Undefined command.*$gdb_prompt " { fail "echo data" }
 	    timeout { fail "echo data (timeout)" }
 	    eof { fail "echo data (eof)" }
@@ -184,7 +182,7 @@ if ![file exists $binfile] then {
 	    # We should be back in the loop.
 	    send_gdb "more data\n"
 	    gdb_expect {
-		-re "^(\r\n|)more data\r\n(|more data\r\n)$" { pass "echo more data" }
+		-re "^(\r\n|)more data\r\nmore data\r\n$" { pass "echo more data" }
 		timeout { fail "echo more data (timeout)" }
 		eof { fail "echo more data (eof)" }
 	    }
-- 
1.9.3

[PATCH 3/6] gdb_test_multiple: Fix user code argument processing

[Pedro Alves]

While teaching gdb_test_multiple to forward "-i" to gdb_expect, I
found that with:

      gdb_test_multiple (...) {
        -i $some_variable -re "..." {}
      }

$some_variable was not getting expanded in the gdb_test_multiple
caller's scope.  This is a bug inside gdb_test_multiple.  When
processing an argument in passed in user code, it was appending the
original argument literally, instead of appending the uplist'ed
argument.

gdb/testsuite/ChangeLog:
2015-02-23  Pedro Alves  <palves@redhat.com>

        * lib/gdb.exp (gdb_test_multiple): When processing an argument,
	append the substituted item, not the original item.
---
 gdb/testsuite/lib/gdb.exp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index bbc657c..8e12ea4 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -720,7 +720,7 @@ proc gdb_test_multiple { command message user_code } {
 	}
 	if { $expecting_arg } {
 	    set expecting_arg 0
-	    lappend processed_code $item
+	    lappend processed_code $subst_item
 	    continue
 	}
 	if { $expecting_action } {
-- 
1.9.3

[PATCH 4/6] testsuite: Don't use expect_background to reap gdbserver

[Pedro Alves]

I adjusted a test to do 'expect -i $server_spawn_id -re ...', and saw
really strange behavior.  Whether that expect would work, depended on
whether GDB would also send output and the same expect matched it too
(on $gdb_spawn_id).  I was perplexed until I noticed that
gdbserver_spawn spawns gdbserver and then uses expect_background to
reap gdbserver.  That expect_background conflicts/races with any
"expect -i $server_spawn_id" done anywhere else in parallel...

In order to make it possible for tests to read inferior I/O out of
$server_spawn_id, we to get rid of that expect_background.  This patch
makes us instead reap gdbserver's spawn id when GDB exits.  If GDB is
still around, this gives a chance for gdbserver to exit cleanly.  The
current code in gdb_finish uses "kill", but that doesn't work with
extended-remote (gdbserver doesn't exit).  We now use "monitor exit"
instead which works in both remote and extended-remote modes.

gdb/testsuite/ChangeLog:
2015-02-23  Pedro Alves  <palves@redhat.com>

	* lib/gdb.exp (gdb_finish): Delete persistent gdbserver handling.
	* lib/gdbserver-support.exp (gdbserver_start): Make
	$server_spawn_id global.
	(gdbserver_start): Don't wait for gdbserver's spawn id with
	expect_background.
	(close_gdbserver): New procedure.
	(gdb_exit): Rename the default version and reimplement.
---
 gdb/testsuite/lib/gdb.exp               | 14 --------
 gdb/testsuite/lib/gdbserver-support.exp | 61 ++++++++++++++++++++++++++-------
 2 files changed, 48 insertions(+), 27 deletions(-)

diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 8e12ea4..3e8e574 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -3986,20 +3986,6 @@ proc gdb_finish { } {
     global gdb_prompt
     global cleanfiles
 
-    # Give persistent gdbserver a chance to terminate before GDB is killed.
-    if {[info exists gdbserver_reconnect_p] && $gdbserver_reconnect_p
-	&& [info exists gdb_spawn_id]} {
-	send_gdb "kill\n";
-	gdb_expect 10 {
-	    -re "y or n" {
-		send_gdb "y\n";
-		exp_continue;
-	    }
-	    -re "$gdb_prompt $" {
-	    }
-	}
-    }
-
     # Exit first, so that the files are no longer in use.
     gdb_exit
 
diff --git a/gdb/testsuite/lib/gdbserver-support.exp b/gdb/testsuite/lib/gdbserver-support.exp
index 4a59154..f19b796 100644
--- a/gdb/testsuite/lib/gdbserver-support.exp
+++ b/gdb/testsuite/lib/gdbserver-support.exp
@@ -270,6 +270,7 @@ proc gdbserver_start { options arguments } {
 	    append gdbserver_command " $arguments"
 	}
 
+	global server_spawn_id
 	set server_spawn_id [remote_spawn target $gdbserver_command]
 
 	# Wait for the server to open its TCP socket, so that GDB can connect.
@@ -294,19 +295,6 @@ proc gdbserver_start { options arguments } {
 	break
     }
 
-    # 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
-	full_buffer { }
-	eof {
-	    # The spawn ID is already closed now (but not yet waited for).
-	    wait -i $expect_out(spawn_id)
-	}
-    }
-
     return [list $protocol [$get_remote_address $debughost $portnum]]
 }
 
@@ -328,6 +316,53 @@ proc gdbserver_spawn { child_args } {
     return [gdbserver_start "" $arguments]
 }
 
+# Close the GDBserver connection.
+
+proc close_gdbserver {} {
+    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
+    # get at the inferior's input or output if necessary, and means that we
+    # don't need to redirect output.
+
+    if {![info exists server_spawn_id]} {
+	return
+    }
+
+    verbose "Quitting GDBserver"
+
+    catch "close -i $server_spawn_id"
+    catch "wait -i $server_spawn_id"
+    unset server_spawn_id
+}
+
+# Hook into GDB exit, and close GDBserver.
+
+if { [info procs gdbserver_gdb_exit] == "" } {
+    rename gdb_exit gdbserver_orig_gdb_exit
+}
+proc gdb_exit {} {
+    global gdb_spawn_id server_spawn_id
+    global gdb_prompt
+
+    if {[info exists gdb_spawn_id] && [info exists server_spawn_id]} {
+	send_gdb "monitor exit\n";
+	gdb_expect {
+	    -re "$gdb_prompt $" {
+		exp_continue
+	    }
+	    -i "$server_spawn_id" eof {
+		wait -i $expect_out(spawn_id)
+		unset server_spawn_id
+	    }
+	}
+    }
+    close_gdbserver
+
+    gdbserver_orig_gdb_exit
+}
+
 # Start a gdbserver process running HOST_EXEC and pass CHILD_ARGS
 # to it.  Return 0 on success, or non-zero on failure: 2 if gdbserver
 # failed to start or 1 if we couldn't connect to it.
-- 
1.9.3

[PATCH 5/6] testsuite: Introduce $inferior_spawn_id

[Pedro Alves]

Some important tests, like gdb.base/interrupt.exp end up skipped
against gdbserver, because they depend on inferior I/O, which
gdbserver doesn't do.

This patch adds a mechanism that makes it possible to make them work.
It adds a new "inferior_spawn_id" global that is the spawn ID used for
I/O interaction with the inferior.  By default, for native targets, or
remote targets that can do I/O through GDB (semi-hosting) this will be
the same as the gdb/host spawn ID.  Otherwise, the board may set this
to some other spawn ID.  When debugging with GDBserver, this will be
set to GDBserver's spawn ID.

Then tests can use send_inferior instead of send_gdb to send input to
the inferior, and use expect's "-i" switch to select which spawn ID to
use for matching input/output.  That is, something like this will now
work:

  send_inferior "echo me\n"
  gdb_test_multiple "continue" "test msg" {
    -i "$inferior_spawn_id" -re "echo me\r\necho\r\n" {
      ...
    }
  }

Or even:

  gdb_test_multiple "continue" "test msg" {
    -i "$inferior_spawn_id" -re "hello world" {
      ...
    }
    -i "$gdb_spawn_id" -re "error.*$gdb_prompt $" {
      ...
    }
  }

Of course, by default, gdb_test_multiple still matches with
$gdb_spawn_id.

gdb/testsuite/ChangeLog:
2015-02-23  Pedro Alves  <palves@redhat.com>

	* lib/gdb.exp (inferior_spawn_id): New global.
	(gdb_test_multiple): Handle "-i".  Reset the spawn id to GDB's
	spawn id after processing the user code.
	(default_gdb_start): Set inferior_spawn_id.
	(send_inferior): New procedure.
	* lib/gdbserver-support.exp (gdbserver_start): Set
	inferior_spawn_id.
	(close_gdbserver, gdb_exit): Unset inferior_spawn_id.
---
 gdb/testsuite/lib/gdb.exp               | 51 +++++++++++++++++++++++++++++++--
 gdb/testsuite/lib/gdbserver-support.exp | 11 +++++--
 2 files changed, 58 insertions(+), 4 deletions(-)

diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 3e8e574..e4722d2 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -31,6 +31,14 @@ load_lib gdb-utils.exp
 
 global GDB
 
+# The spawn ID used for I/O interaction with the inferior.  For native
+# targets, or remote targets that can do I/O through GDB
+# (semi-hosting) this will be the same as the host/GDB's spawn ID.
+# Otherwise, the board may set this to some other spawn ID.  E.g.,
+# when debugging with GDBserver, this is set to GDBserver's spawn ID,
+# so input/output is done on gdbserver's tty.
+global inferior_spawn_id
+
 if [info exists TOOL_EXECUTABLE] {
     set GDB $TOOL_EXECUTABLE
 }
@@ -646,13 +654,31 @@ proc gdb_internal_error_resync {} {
 #    }
 # }
 #
+# Like with "expect", you can also specify the spawn id to match with
+# -i "$id".  Interesting spawn ids are $inferior_spawn_id and
+# $gdb_spawn_id.  The former matches inferior I/O, while the latter
+# matches GDB I/O.  E.g.:
+#
+# send_inferior "hello\n"
+# gdb_test_multiple "continue" "test echo" {
+#    -i "$inferior_spawn_id" -re "^hello\r\nhello\r\n$" {
+#        pass "got echo"
+#    }
+#    -i "$gdb_spawn_id" -re "Breakpoint.*$gdb_prompt $" {
+#        fail "hit breakpoint"
+#    }
+# }
+#
 # The standard patterns, such as "Inferior exited..." and "A problem
-# ...", all being implicitly appended to that list.
+# ...", all being implicitly appended to that list.  These are always
+# expected from $gdb_spawn_id.  IOW, callers do not need to worry
+# about resetting "-i" back to $gdb_spawn_id explicitly.
 #
 proc gdb_test_multiple { command message user_code } {
     global verbose use_gdb_stub
     global gdb_prompt pagination_prompt
     global GDB
+    global gdb_spawn_id
     global inferior_exited_re
     upvar timeout timeout
     upvar expect_out expect_out
@@ -713,7 +739,7 @@ proc gdb_test_multiple { command message user_code } {
 	    lappend processed_code $item
 	    continue
 	}
-	if { $item == "-timeout" } {
+	if { $item == "-timeout" || $item == "-i" } {
 	    set expecting_arg 1
 	    lappend processed_code $item
 	    continue
@@ -809,6 +835,9 @@ proc gdb_test_multiple { command message user_code } {
     }
     append code $processed_code
     append code {
+	# Reset the spawn id, in case the processed code used -i.
+	-i "$gdb_spawn_id"
+
 	-re "Ending remote debugging.*$gdb_prompt $" {
 	    if ![isnative] then {
 		warning "Can`t communicate to remote target."
@@ -1454,6 +1483,7 @@ proc default_gdb_spawn { } {
 proc default_gdb_start { } {
     global gdb_prompt pagination_prompt
     global gdb_spawn_id
+    global inferior_spawn_id
 
     if [info exists gdb_spawn_id] {
 	return 0
@@ -1464,6 +1494,11 @@ proc default_gdb_start { } {
 	return $res
     }
 
+    # Default to assuming inferior I/O is done on GDB's terminal.
+    if {![info exists inferior_spawn_id]} {
+	set inferior_spawn_id $gdb_spawn_id
+    }
+
     # When running over NFS, particularly if running many simultaneous
     # tests on different hosts all using the same server, things can
     # get really slow.  Give gdb at least 3 minutes to start up.
@@ -3195,6 +3230,18 @@ proc send_gdb { string } {
     return [remote_send host "$string"]
 }
 
+# Send STRING to the inferior's terminal.
+
+proc send_inferior { string } {
+    global inferior_spawn_id
+
+    if {[catch "send -i $inferior_spawn_id -- \$string" errorInfo]} {
+	return "$errorInfo"
+    } else {
+	return ""
+    }
+}
+
 #
 #
 
diff --git a/gdb/testsuite/lib/gdbserver-support.exp b/gdb/testsuite/lib/gdbserver-support.exp
index f19b796..53843b8 100644
--- a/gdb/testsuite/lib/gdbserver-support.exp
+++ b/gdb/testsuite/lib/gdbserver-support.exp
@@ -273,6 +273,11 @@ proc gdbserver_start { options arguments } {
 	global server_spawn_id
 	set server_spawn_id [remote_spawn target $gdbserver_command]
 
+	# 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
+
 	# Wait for the server to open its TCP socket, so that GDB can connect.
 	expect {
 	    -i $server_spawn_id
@@ -319,7 +324,7 @@ proc gdbserver_spawn { child_args } {
 # Close the GDBserver connection.
 
 proc close_gdbserver {} {
-    global server_spawn_id
+    global server_spawn_id inferior_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
@@ -335,6 +340,7 @@ 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.
@@ -343,7 +349,7 @@ if { [info procs gdbserver_gdb_exit] == "" } {
     rename gdb_exit gdbserver_orig_gdb_exit
 }
 proc gdb_exit {} {
-    global gdb_spawn_id server_spawn_id
+    global gdb_spawn_id server_spawn_id inferior_spawn_id
     global gdb_prompt
 
     if {[info exists gdb_spawn_id] && [info exists server_spawn_id]} {
@@ -355,6 +361,7 @@ proc gdb_exit {} {
 	    -i "$server_spawn_id" eof {
 		wait -i $expect_out(spawn_id)
 		unset server_spawn_id
+		unset inferior_spawn_id
 	    }
 	}
     }
-- 
1.9.3

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

[Yao Qi]

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)

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.

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

-- 
Yao (齐尧)

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

[Pedro Alves]

[-- Attachment #1: Type: text/plain, Size: 2663 bytes --]

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).
> 



[-- Attachment #2: 0001-Add-a-way-for-boards-to-request-usage-of-the-force-u.patch --]
[-- Type: text/x-patch, Size: 2298 bytes --]

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

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

[Pedro Alves]

On 02/27/2015 10:42 AM, Pedro Alves wrote:

> 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...

Something like this.  I think this is the approach I'm liking best.

----- 
From d68fb948b275b85b7f2fa95ac0626c5ba7037114 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Fri, 27 Feb 2015 10:44:15 +0000
Subject: [PATCH] Add "../lib/unbuffer_output.c" and use it in interrupt.exp

---
 gdb/testsuite/gdb.base/interrupt.c  |  5 +++++
 gdb/testsuite/lib/unbuffer_output.c | 37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+)
 create mode 100644 gdb/testsuite/lib/unbuffer_output.c

diff --git a/gdb/testsuite/gdb.base/interrupt.c b/gdb/testsuite/gdb.base/interrupt.c
index d7bb271..6426015 100644
--- a/gdb/testsuite/gdb.base/interrupt.c
+++ b/gdb/testsuite/gdb.base/interrupt.c
@@ -3,6 +3,8 @@
 #include <unistd.h>
 #include <stdlib.h>
 
+#include "../lib/unbuffer_output.c"
+
 #ifdef SIGNALS
 #include <signal.h>
 
@@ -17,6 +19,9 @@ main ()
 {
   char x;
   int nbytes;
+
+  gdb_unbuffer_output ();
+
 #ifdef SIGNALS
   signal (SIGINT, sigint_handler);
 #endif
diff --git a/gdb/testsuite/lib/unbuffer_output.c b/gdb/testsuite/lib/unbuffer_output.c
new file mode 100644
index 0000000..654d01c
--- /dev/null
+++ b/gdb/testsuite/lib/unbuffer_output.c
@@ -0,0 +1,37 @@
+/* Copyright (C) 2008-2015 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* Force outputs to unbuffered mode if not connected to a
+   terminal.  */
+
+#include <stdio.h>
+
+static int
+gdb_unbuffer_output (void)
+{
+  /* Always force this for Windows testing.  To a native Windows
+     program running under under a Cygwin shell/ssh, stdin is really a
+     Windows pipe, thus not a tty and its outputs ends up fully
+     buffered.  */
+#ifndef __MINGW32__
+  if (!isatty (fileno (stdin)))
+#endif
+    {
+      setvbuf (stdout, NULL, _IONBF, BUFSIZ);
+      setvbuf (stderr, NULL, _IONBF, BUFSIZ);
+    }
+}
-- 
1.9.3

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

[Pedro Alves]

On 02/27/2015 10:59 AM, Pedro Alves wrote:
> On 02/27/2015 10:42 AM, Pedro Alves wrote:
> 
>> 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...
> 
> Something like this.  I think this is the approach I'm liking best.

TBC, I tried both ssh with -t and without -t, and confirmed
the wanted isatty path is take accordingly.  The mingw32 bit
I did not test, but I'm sure we need it.

> 
> ----- 
> From d68fb948b275b85b7f2fa95ac0626c5ba7037114 Mon Sep 17 00:00:00 2001
> From: Pedro Alves <palves@redhat.com>
> Date: Fri, 27 Feb 2015 10:44:15 +0000
> Subject: [PATCH] Add "../lib/unbuffer_output.c" and use it in interrupt.exp
> 
> ---
>  gdb/testsuite/gdb.base/interrupt.c  |  5 +++++
>  gdb/testsuite/lib/unbuffer_output.c | 37 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 42 insertions(+)
>  create mode 100644 gdb/testsuite/lib/unbuffer_output.c
> 
> diff --git a/gdb/testsuite/gdb.base/interrupt.c b/gdb/testsuite/gdb.base/interrupt.c
> index d7bb271..6426015 100644
> --- a/gdb/testsuite/gdb.base/interrupt.c
> +++ b/gdb/testsuite/gdb.base/interrupt.c
> @@ -3,6 +3,8 @@
>  #include <unistd.h>
>  #include <stdlib.h>
>  
> +#include "../lib/unbuffer_output.c"
> +
>  #ifdef SIGNALS
>  #include <signal.h>
>  
> @@ -17,6 +19,9 @@ main ()
>  {
>    char x;
>    int nbytes;
> +
> +  gdb_unbuffer_output ();
> +
>  #ifdef SIGNALS
>    signal (SIGINT, sigint_handler);
>  #endif
> diff --git a/gdb/testsuite/lib/unbuffer_output.c b/gdb/testsuite/lib/unbuffer_output.c
> new file mode 100644
> index 0000000..654d01c
> --- /dev/null
> +++ b/gdb/testsuite/lib/unbuffer_output.c
> @@ -0,0 +1,37 @@
> +/* Copyright (C) 2008-2015 Free Software Foundation, Inc.
> +
> +   This file is part of GDB.
> +
> +   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
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +/* Force outputs to unbuffered mode if not connected to a
> +   terminal.  */
> +
> +#include <stdio.h>
> +
> +static int
> +gdb_unbuffer_output (void)
> +{
> +  /* Always force this for Windows testing.  To a native Windows
> +     program running under under a Cygwin shell/ssh, stdin is really a
> +     Windows pipe, thus not a tty and its outputs ends up fully
> +     buffered.  */
> +#ifndef __MINGW32__
> +  if (!isatty (fileno (stdin)))
> +#endif
> +    {
> +      setvbuf (stdout, NULL, _IONBF, BUFSIZ);
> +      setvbuf (stderr, NULL, _IONBF, BUFSIZ);
> +    }
> +}
> 


Thanks,
Pedro Alves

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

[Yao Qi]

On 27/02/15 10:42, Pedro Alves wrote:
> 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

I think you meant "fully buffered mode", don't you?

> printf calls don't flush...
>
> Adding:
>    setvbuf (stdout, NULL, _IONBF, BUFSIZ);
>
> to interrupt.c fixes it.
>


>
> 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"

I prefer this one.

-- 
Yao (齐尧)

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

[Yao Qi]

Hi Pedro,
This patch is fine by me.

On 27/02/15 10:59, Pedro Alves wrote:
> +  /* Always force this for Windows testing.  To a native Windows
> +     program running under under a Cygwin shell/ssh, stdin is really a

Double "under".

> +     Windows pipe, thus not a tty and its outputs ends up fully
> +     buffered.  */
> +#ifndef __MINGW32__
> +  if (!isatty (fileno (stdin)))
> +#endif

Include unistd.h for isatty?

-- 
Yao (齐尧)

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

[Pedro Alves]

On 02/27/2015 12:08 PM, Yao Qi wrote:
> On 27/02/15 10:42, Pedro Alves wrote:
>> 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
> 
> I think you meant "fully buffered mode", don't you?

Blah, indeed.  Sorry about the confusion.  :-/

Thanks,
Pedro Alves

[pushed] Add "../lib/unbuffer_output.c" and use it in gdb.base/interrupt.c (Re: [PATCH 5/6] testsuite: Introduce $inferior_spawn_id)

[Pedro Alves]

On 02/27/2015 12:12 PM, Yao Qi wrote:
> Hi Pedro,
> This patch is fine by me.

Awesome, patch below is pushed...

> On 27/02/15 10:59, Pedro Alves wrote:
>> +  /* Always force this for Windows testing.  To a native Windows
>> +     program running under under a Cygwin shell/ssh, stdin is really a
> 
> Double "under".
> 
>> +     Windows pipe, thus not a tty and its outputs ends up fully
>> +     buffered.  */
>> +#ifndef __MINGW32__
>> +  if (!isatty (fileno (stdin)))
>> +#endif
> 
> Include unistd.h for isatty?
> 

... with these fixed.

Do you want to comment on the rest of the series, or shall I push it?

----
From 6f98576f29a70ed947f102015df0388bccc6aa1a Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Fri, 27 Feb 2015 13:54:22 +0000
Subject: [PATCH] Add "../lib/unbuffer_output.c" and use it in
 gdb.base/interrupt.c

In some scenarios, GDB or GDBserver can be spawned with input _not_
connected to a tty, and then tests that rely on stdio fail with
timeouts, because the inferior's stdout and stderr streams end up
fully buffered.

See discussion here:
  https://sourceware.org/ml/gdb-patches/2015-02/msg00809.html

We have a hack in place that works around this for Windows testing,
that forces every test program to link with an .o file that does
(lib/set_unbuffered_mode.c):

 static int __gdb_set_unbuffered_output (void) __attribute__ ((constructor));
 static int
 __gdb_set_unbuffered_output (void)
 {
   setvbuf (stdout, NULL, _IONBF, BUFSIZ);
   setvbuf (stderr, NULL, _IONBF, BUFSIZ);
 }

That's a bit hacky; it ends up done for _all_ tests.

This patch adds a way to do this unbuffering explicitly from the test
code itself, so it is done only when necessary, and for all
targets/hosts.  For starters, it adjusts gdb.base/interrupt.c to use
it.

Tested on x86_64 Fedora 20, native, and against a remote gdbserver
board file that connects to the target with ssh, with and without -t
(create pty).

gdb/testsuite/
2015-02-27  Pedro Alves  <palves@redhat.com>

	* lib/unbuffer_output.c: New file.
	* gdb.base/interrupt.c: Include "../lib/unbuffer_output.c".
	(main): Call gdb_unbuffer_output.
---
 gdb/testsuite/ChangeLog             |  6 ++++++
 gdb/testsuite/gdb.base/interrupt.c  |  5 +++++
 gdb/testsuite/lib/unbuffer_output.c | 39 +++++++++++++++++++++++++++++++++++++
 3 files changed, 50 insertions(+)
 create mode 100644 gdb/testsuite/lib/unbuffer_output.c

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index a5896be..efc74f6 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2015-02-27  Pedro Alves  <palves@redhat.com>
+
+	* lib/unbuffer_output.c: New file.
+	* gdb.base/interrupt.c: Include "../lib/unbuffer_output.c".
+	(main): Call gdb_unbuffer_output.
+
 2015-02-27  Yao Qi  <yao.qi@linaro.org>
 
 	* gdb.base/catch-syscall.exp: Don't skip it on hppa*-hp-hpux*
diff --git a/gdb/testsuite/gdb.base/interrupt.c b/gdb/testsuite/gdb.base/interrupt.c
index d7bb271..6426015 100644
--- a/gdb/testsuite/gdb.base/interrupt.c
+++ b/gdb/testsuite/gdb.base/interrupt.c
@@ -3,6 +3,8 @@
 #include <unistd.h>
 #include <stdlib.h>
 
+#include "../lib/unbuffer_output.c"
+
 #ifdef SIGNALS
 #include <signal.h>
 
@@ -17,6 +19,9 @@ main ()
 {
   char x;
   int nbytes;
+
+  gdb_unbuffer_output ();
+
 #ifdef SIGNALS
   signal (SIGINT, sigint_handler);
 #endif
diff --git a/gdb/testsuite/lib/unbuffer_output.c b/gdb/testsuite/lib/unbuffer_output.c
new file mode 100644
index 0000000..5093299
--- /dev/null
+++ b/gdb/testsuite/lib/unbuffer_output.c
@@ -0,0 +1,39 @@
+/* Copyright (C) 2008-2015 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* Force output to unbuffered mode if not connected to a terminal.  */
+
+#include <stdio.h>
+#ifndef __MINGW32__
+#include <unistd.h>
+#endif
+
+static int
+gdb_unbuffer_output (void)
+{
+  /* Always force this for Windows testing.  To a native Windows
+     program running under a Cygwin shell/ssh, stdin is really a
+     Windows pipe, thus not a tty and its outputs ends up fully
+     buffered.  */
+#ifndef __MINGW32__
+  if (!isatty (fileno (stdin)))
+#endif
+    {
+      setvbuf (stdout, NULL, _IONBF, BUFSIZ);
+      setvbuf (stderr, NULL, _IONBF, BUFSIZ);
+    }
+}
-- 
1.9.3

Re: [pushed] Add "../lib/unbuffer_output.c" and use it in gdb.base/interrupt.c (Re: [PATCH 5/6] testsuite: Introduce $inferior_spawn_id)

[Yao Qi]

Pedro Alves <palves@redhat.com> writes:

> Do you want to comment on the rest of the series, or shall I push it?

I've gone through the whole series, and only comment/question is about
this non-tty-buffered-output problem.  These patches look good to me,
but let us see if other people have comments on them in a couple of
days.

-- 
Yao (齐尧)

Re: [pushed] Add "../lib/unbuffer_output.c" and use it in gdb.base/interrupt.c (Re: [PATCH 5/6] testsuite: Introduce $inferior_spawn_id)

[Eli Zaretskii]

> Date: Fri, 27 Feb 2015 13:59:05 +0000
> From: Pedro Alves <palves@redhat.com>
> CC: gdb-patches@sourceware.org
> 
> >> +     Windows pipe, thus not a tty and its outputs ends up fully
> >> +     buffered.  */
> >> +#ifndef __MINGW32__
> >> +  if (!isatty (fileno (stdin)))
> >> +#endif
> > 
> > Include unistd.h for isatty?
> > 
> 
> ... with these fixed.
> 
> Do you want to comment on the rest of the series, or shall I push it?

If that's isatty from the MS runtime, it will return non-zero for
every character device, including the null device.  Is that what we
want here?

Re: [pushed] Add "../lib/unbuffer_output.c" and use it in gdb.base/interrupt.c (Re: [PATCH 5/6] testsuite: Introduce $inferior_spawn_id)

[Pedro Alves]

On 02/27/2015 02:43 PM, Eli Zaretskii wrote:
>> Date: Fri, 27 Feb 2015 13:59:05 +0000
>> From: Pedro Alves <palves@redhat.com>
>> CC: gdb-patches@sourceware.org
>>
>>>> +     Windows pipe, thus not a tty and its outputs ends up fully
>>>> +     buffered.  */
>>>> +#ifndef __MINGW32__
>>>> +  if (!isatty (fileno (stdin)))
>>>> +#endif
>>>
>>> Include unistd.h for isatty?
>>>
>>
>> ... with these fixed.
>>
>> Do you want to comment on the rest of the series, or shall I push it?
> 
> If that's isatty from the MS runtime, it will return non-zero for
> every character device, including the null device.  Is that what we
> want here?

No, and so we skip the isatty check on Windows.  Note the #ifndef.

Thanks,
Pedro Alves

Re: [PATCH 0/6] Introduce $inferior_spawn_id, make interrupt.exp work with GDBserver

[Pedro Alves]

On 02/23/2015 01:54 PM, Pedro Alves wrote:
> After fixing a gdbserver syscall restart issue, I thought of adding a
> new test to exercise syscall restarting.  Then I recalled that we
> already have interrupt.exp for that.  However, that test is skipped on
> gdbserver, because it relies on inferior i/o.
> 
> When testing with gdbserver, inferior are spawned by gdbserver, on
> gdbserver's pty, and so gdb_test_multiple/gdb_expect, don't see the
> inferior's i/o.
> 
> This series introduces a mechanism that allows tests to match inferior
> i/o separately from gdb i/o, like:
> 
>   send_inferior "echo me\n"
>   gdb_test_multiple "continue" "test msg" {
>     -i "$inferior_spawn_id" -re "echo me\r\necho\r\n" {
>       ...
>     }
>     -i "$gdb_spawn_id" -re "error.*$gdb_prompt $" {
>       ...
>     }
>   }
> 
> and then adjusts interrupt.exp to use the new mechanism.
> 
> I took the idea from Don Breazeal's use of $server_spawn_id here
> <https://sourceware.org/ml/gdb-patches/2015-01/msg00689.html>, and
> generalized it.
> 
> A couple bugs had to be fixed along the way, and I took the change to
> clean up interrupt.exp to use gdb_test_multiple instead of gdb_expect
> too.  interrupt.exp now passes against gdbserver here, on x86_64
> Fedora 20 (It fails with gdbserver/-m32 testing in the exact same way
> it fails on native/-m32 testing, due to a kernel bug).
> 

I've now pushed this series in.

Thanks,
Pedro Alves

Re: [PATCH 4/6] testsuite: Don't use expect_background to reap gdbserver

[Yao Qi]

Pedro Alves <palves@redhat.com> writes:

> +proc gdb_exit {} {
> +    global gdb_spawn_id server_spawn_id
> +    global gdb_prompt
> +
> +    if {[info exists gdb_spawn_id] && [info exists server_spawn_id]} {
> +	send_gdb "monitor exit\n";
> +	gdb_expect {
> +	    -re "$gdb_prompt $" {
> +		exp_continue
> +	    }
> +	    -i "$server_spawn_id" eof {
> +		wait -i $expect_out(spawn_id)
> +		unset server_spawn_id
> +	    }
> +	}
> +    }

Do we need to catch exception here?

I see the error when I run gdb-sigterm.exp with native-gdbserver
on x86_64-linux.

infrun: prepare_to_wait^M
Cannot execute this command while the target is running.^M
Use the "interrupt" command to stop the target^M
and then try again.^M
gdb.base/gdb-sigterm.exp: expect eof #0: got eof
gdb.base/gdb-sigterm.exp: expect eof #0: stepped 12 times
ERROR OCCURED: : spawn id exp8 not open
    while executing
"expect {
-i exp8 -timeout 10
            -re "$gdb_prompt $" {
                exp_continue
            }
            -i "$server_spawn_id" eof {
                wait -i $expect_out(spawn_id)
                unse..."
    ("uplevel" body line 1)
    invoked from within

In gdb-sigterm.exp, SIGTERM is sent to GDB and it exits.  However,
Dejagnu or tcl doesn't know this.


> +    close_gdbserver
> +
> +    gdbserver_orig_gdb_exit
> +}

This error terminates the whole testing, so the following tests are
not run.

I wrap the send_gdb and gdb_expect statement above by "catch",
testing looks fine, although error messages are still shown in the
console and gdb.log.

-- 
Yao (齐尧)

From: Yao Qi <yao.qi@linaro.org>
Date: Mon, 13 Apr 2015 12:36:56 +0100
Subject: [PATCH] Catch exception in lib/gdbserver-support.exp:gdb_exit

I see the error when I run gdb-sigterm.exp with native-gdbserver
on x86_64-linux.

infrun: prepare_to_wait^M
Cannot execute this command while the target is running.^M
Use the "interrupt" command to stop the target^M
and then try again.^M
gdb.base/gdb-sigterm.exp: expect eof #0: got eof
gdb.base/gdb-sigterm.exp: expect eof #0: stepped 12 times
ERROR OCCURED: : spawn id exp8 not open
    while executing
"expect {
-i exp8 -timeout 10
            -re "$gdb_prompt $" {
                exp_continue
            }
            -i "$server_spawn_id" eof {
                wait -i $expect_out(spawn_id)
                unse..."
    ("uplevel" body line 1)
    invoked from within

In gdb-sigterm.exp, SIGTERM is sent to GDB and it exits.  However,
Dejagnu or tcl doesn't know this.

This patch is to catch the exception, but error messages are still
shown in the console and gdb.log.

gdb/testsuite:

2015-04-13  Yao Qi  <yao.qi@linaro.org>

	* lib/gdbserver-support.exp (gdb_exit): Catch exception.

diff --git a/gdb/testsuite/lib/gdbserver-support.exp b/gdb/testsuite/lib/gdbserver-support.exp
index 53843b8..8d4858a 100644
--- a/gdb/testsuite/lib/gdbserver-support.exp
+++ b/gdb/testsuite/lib/gdbserver-support.exp
@@ -353,15 +353,20 @@ proc gdb_exit {} {
     global gdb_prompt
 
     if {[info exists gdb_spawn_id] && [info exists server_spawn_id]} {
-	send_gdb "monitor exit\n";
-	gdb_expect {
-	    -re "$gdb_prompt $" {
-		exp_continue
-	    }
-	    -i "$server_spawn_id" eof {
-		wait -i $expect_out(spawn_id)
-		unset server_spawn_id
-		unset inferior_spawn_id
+	# GDB may be terminated in an expected way or an unexpected way,
+	# but DejaGNU doesn't know that, so gdb_spawn_id isn't unset.
+	# Catch the exceptions.
+	catch {
+	    send_gdb "monitor exit\n";
+	    gdb_expect {
+		-re "$gdb_prompt $" {
+		    exp_continue
+		}
+		-i "$server_spawn_id" eof {
+		    wait -i $expect_out(spawn_id)
+		    unset server_spawn_id
+		    unset inferior_spawn_id
+		}
 	    }
 	}
     }

Re: [PATCH 4/6] testsuite: Don't use expect_background to reap gdbserver

[Pedro Alves]

On 04/13/2015 12:42 PM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
> 
>> +proc gdb_exit {} {
>> +    global gdb_spawn_id server_spawn_id
>> +    global gdb_prompt
>> +
>> +    if {[info exists gdb_spawn_id] && [info exists server_spawn_id]} {
>> +	send_gdb "monitor exit\n";
>> +	gdb_expect {
>> +	    -re "$gdb_prompt $" {
>> +		exp_continue
>> +	    }
>> +	    -i "$server_spawn_id" eof {
>> +		wait -i $expect_out(spawn_id)
>> +		unset server_spawn_id
>> +	    }
>> +	}
>> +    }
> 
> Do we need to catch exception here?

Whoops, yes, looks like it.

> I wrap the send_gdb and gdb_expect statement above by "catch",
> testing looks fine, although error messages are still shown in the
> console and gdb.log.

Why not suppress the error message?  I think you just need to pass
a var name as second parameter to "catch".

Thanks,
Pedro Alves

Re: [PATCH 4/6] testsuite: Don't use expect_background to reap gdbserver

[Yao Qi]

On 13/04/15 13:09, Pedro Alves wrote:
>> I wrap the send_gdb and gdb_expect statement above by "catch",
>> >testing looks fine, although error messages are still shown in the
>> >console and gdb.log.
> Why not suppress the error message?  I think you just need to pass
> a var name as second parameter to "catch".

I did that, but it is useless.  These messages prefixed with
"ERROR OCCURED:" are printed by DejaGNU, lib/remote.exp:remote_expect,

     if {$code == 1} {
         if {[info exists string]} {send_user "ERROR OCCURED: $errorInfo 
$errorCode $string"}

looks we can't prevent DejaGNU invoking send_user.  If this error is
annoying, we can unset gdb_spawn_id at the end of proc do_test in
gdb-sigterm.exp.

-- 
Yao (齐尧)

Re: [PATCH 4/6] testsuite: Don't use expect_background to reap gdbserver

[Pedro Alves]

On 04/13/2015 02:25 PM, Yao Qi wrote:
> On 13/04/15 13:09, Pedro Alves wrote:
>>> I wrap the send_gdb and gdb_expect statement above by "catch",
>>>> testing looks fine, although error messages are still shown in the
>>>> console and gdb.log.
>> Why not suppress the error message?  I think you just need to pass
>> a var name as second parameter to "catch".
> 
> I did that, but it is useless.  These messages prefixed with
> "ERROR OCCURED:" are printed by DejaGNU, lib/remote.exp:remote_expect,
> 
>      if {$code == 1} {
>          if {[info exists string]} {send_user "ERROR OCCURED: $errorInfo 
> $errorCode $string"}
> 
> looks we can't prevent DejaGNU invoking send_user.

I think we should just call raw "expect" instead then.

> If this error is
> annoying, we can unset gdb_spawn_id at the end of proc do_test in
> gdb-sigterm.exp.

I think also need to call wait too?  There are other eof calls in other
tests too and also under lib/  We'd need to do the same to all of those,
and then, at least, we'd need to make default_gdb_exit not skip the
inotify_log_file code too.  I'm not sure I like that direction.

(BTW, that remote host code in default_gdb_exit looks like should be
given the same treatment.)

Thanks,
Pedro Alves

Re: [PATCH 4/6] testsuite: Don't use expect_background to reap gdbserver

[Yao Qi]

Pedro Alves <palves@redhat.com> writes:

>> looks we can't prevent DejaGNU invoking send_user.
>
> I think we should just call raw "expect" instead then.

Using "expect" is OK to me, how about the patch below?

-- 
Yao (齐尧)

From: Yao Qi <yao.qi@linaro.org>
Date: Mon, 13 Apr 2015 12:36:56 +0100
Subject: [PATCH] Catch exception in lib/gdbserver-support.exp:gdb_exit

I see the error when I run gdb-sigterm.exp with native-gdbserver
on x86_64-linux.

infrun: prepare_to_wait^M
Cannot execute this command while the target is running.^M
Use the "interrupt" command to stop the target^M
and then try again.^M
gdb.base/gdb-sigterm.exp: expect eof #0: got eof
gdb.base/gdb-sigterm.exp: expect eof #0: stepped 12 times
ERROR OCCURED: : spawn id exp8 not open
    while executing
"expect {
-i exp8 -timeout 10
            -re "$gdb_prompt $" {
                exp_continue
            }
            -i "$server_spawn_id" eof {
                wait -i $expect_out(spawn_id)
                unse..."
    ("uplevel" body line 1)
    invoked from within

In gdb-sigterm.exp, SIGTERM is sent to GDB and it exits.  However,
Dejagnu or tcl doesn't know this.

This patch is to catch the exception, but error messages are still
shown in the console and gdb.log.  In order to avoid this, we also
replace gdb_expect with expect.

gdb/testsuite:

2015-04-13  Yao Qi  <yao.qi@linaro.org>

	* lib/gdbserver-support.exp (gdb_exit): Catch exception
	and use expect instead of gdb_expect.

diff --git a/gdb/testsuite/lib/gdbserver-support.exp b/gdb/testsuite/lib/gdbserver-support.exp
index 53843b8..b3140c2 100644
--- a/gdb/testsuite/lib/gdbserver-support.exp
+++ b/gdb/testsuite/lib/gdbserver-support.exp
@@ -353,15 +353,23 @@ proc gdb_exit {} {
     global gdb_prompt
 
     if {[info exists gdb_spawn_id] && [info exists server_spawn_id]} {
-	send_gdb "monitor exit\n";
-	gdb_expect {
-	    -re "$gdb_prompt $" {
-		exp_continue
-	    }
-	    -i "$server_spawn_id" eof {
-		wait -i $expect_out(spawn_id)
-		unset server_spawn_id
-		unset inferior_spawn_id
+	# GDB may be terminated in an expected way or an unexpected way,
+	# but DejaGNU doesn't know that, so gdb_spawn_id isn't unset.
+	# Catch the exceptions.
+	catch {
+	    send_gdb "monitor exit\n";
+	    # We use expect rather than gdb_expect because
+	    # we want to suppress printing exception messages, otherwise,
+	    # remote_expect, invoked by gdb_expect, prints the exceptions.
+	    expect {
+		-i "$gdb_spawn_id" -re "$gdb_prompt $" {
+		    exp_continue
+		}
+		-i "$server_spawn_id" eof {
+		    wait -i $expect_out(spawn_id)
+		    unset server_spawn_id
+		    unset inferior_spawn_id
+		}
 	    }
 	}
     }

Re: [PATCH 4/6] testsuite: Don't use expect_background to reap gdbserver

[Pedro Alves]

On 04/13/2015 03:20 PM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
> 
>>> looks we can't prevent DejaGNU invoking send_user.
>>
>> I think we should just call raw "expect" instead then.
> 
> Using "expect" is OK to me, how about the patch below?
> 

Looks good, thanks.

-- 
Pedro Alves

Re: [PATCH 4/6] testsuite: Don't use expect_background to reap gdbserver

[Yao Qi]

On 13/04/15 15:22, Pedro Alves wrote:
> Looks good, thanks.

Thanks, patch is pushed in.

-- 
Yao (齐尧)

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

[Antoine Tremblay]

I have a question regarding noinferiorio and it's future usage...

As the condition with noinferiorio in interrupt.exp is now  :

if {[target_info exists gdb,noinferiorio] && $inferior_spawn_id == 
$gdb_spawn_id}

noinferiorio is effectively bypassed when we are using gdbserver, even 
as noinferirorio is true.

But what if the board or simulator really can't handle io at all, and 
that setvbuf would not work.. then there is no option to disable io 
tests in that case ?

Is the intention to remove noinferiorio from gdbserver-base.exp and keep 
the tests with the if {[target_info exists gdb,noinferiorio]} check ?

Regards,

Antoine

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

[Pedro Alves]

On 04/16/2015 05:55 PM, Antoine Tremblay wrote:
> I have a question regarding noinferiorio and it's future usage...
> 
> As the condition with noinferiorio in interrupt.exp is now  :
> 
> if {[target_info exists gdb,noinferiorio] && $inferior_spawn_id == 
> $gdb_spawn_id}
> 
> noinferiorio is effectively bypassed when we are using gdbserver, even 
> as noinferirorio is true.
> 
> But what if the board or simulator really can't handle io at all, and 
> that setvbuf would not work.. then there is no option to disable io 
> tests in that case ?

Hmm, but why would such a board be using gdbserver-support.exp?  Can you
expand a little?

> 
> Is the intention to remove noinferiorio from gdbserver-base.exp and keep 
> the tests with the if {[target_info exists gdb,noinferiorio]} check ?

The idea was make all tests that rely on inferior io make use
of $inferior_spawn_id, like interrupt.exp.  My thought was that once that
is done, we can either remove noinferiorio from gdbserver-base.exp or
leave it, it wouldn't matter, as for gdbserver testing, would be
always $inferior_spawn_id != $gdb_spawn_id.  But it sounds like you have
some environment that may make that troublesome.  I'll need to know more
about it though.

Thanks,
Pedro Alves

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

[Pedro Alves]

On 04/16/2015 06:14 PM, Pedro Alves wrote:
> On 04/16/2015 05:55 PM, Antoine Tremblay wrote:
>> I have a question regarding noinferiorio and it's future usage...
>>
>> As the condition with noinferiorio in interrupt.exp is now  :
>>
>> if {[target_info exists gdb,noinferiorio] && $inferior_spawn_id == 
>> $gdb_spawn_id}
>>
>> noinferiorio is effectively bypassed when we are using gdbserver, even 
>> as noinferirorio is true.
>>
>> But what if the board or simulator really can't handle io at all, and 
>> that setvbuf would not work.. then there is no option to disable io 
>> tests in that case ?
> 
> Hmm, but why would such a board be using gdbserver-support.exp?  Can you
> expand a little?
> 
>>
>> Is the intention to remove noinferiorio from gdbserver-base.exp and keep 
>> the tests with the if {[target_info exists gdb,noinferiorio]} check ?
> 
> The idea was make all tests that rely on inferior io make use
> of $inferior_spawn_id, like interrupt.exp.  My thought was that once that
> is done, we can either remove noinferiorio from gdbserver-base.exp or
> leave it, it wouldn't matter, as for gdbserver testing, would be
> always $inferior_spawn_id != $gdb_spawn_id.  But it sounds like you have
> some environment that may make that troublesome.  I'll need to know more
> about it though.
> 

FYI, I now finished the transition to inferior_spawn_id in the
whole testsuite, and posted it as a series here:

  https://sourceware.org/ml/gdb-patches/2015-04/msg00776.html

the last patch removes noinferiorio from gdbserver-base.exp.

Thanks,
Pedro Alves

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

[Antoine Tremblay]

On 04/21/2015 02:25 PM, Pedro Alves wrote:
> On 04/16/2015 06:14 PM, Pedro Alves wrote:
>> On 04/16/2015 05:55 PM, Antoine Tremblay wrote:
>>> I have a question regarding noinferiorio and it's future usage...
>>>
>>> As the condition with noinferiorio in interrupt.exp is now  :
>>>
>>> if {[target_info exists gdb,noinferiorio] && $inferior_spawn_id ==
>>> $gdb_spawn_id}
>>>
>>> noinferiorio is effectively bypassed when we are using gdbserver, even
>>> as noinferirorio is true.
>>>
>>> But what if the board or simulator really can't handle io at all, and
>>> that setvbuf would not work.. then there is no option to disable io
>>> tests in that case ?
>>
>> Hmm, but why would such a board be using gdbserver-support.exp?  Can you
>> expand a little?
>>
>>>
>>> Is the intention to remove noinferiorio from gdbserver-base.exp and keep
>>> the tests with the if {[target_info exists gdb,noinferiorio]} check ?
>>
>> The idea was make all tests that rely on inferior io make use
>> of $inferior_spawn_id, like interrupt.exp.  My thought was that once that
>> is done, we can either remove noinferiorio from gdbserver-base.exp or
>> leave it, it wouldn't matter, as for gdbserver testing, would be
>> always $inferior_spawn_id != $gdb_spawn_id.  But it sounds like you have
>> some environment that may make that troublesome.  I'll need to know more
>> about it though.
>>
>
> FYI, I now finished the transition to inferior_spawn_id in the
> whole testsuite, and posted it as a series here:
>
>    https://sourceware.org/ml/gdb-patches/2015-04/msg00776.html
>
> the last patch removes noinferiorio from gdbserver-base.exp.
>

Thanks! I'll check it out :)

Antoine