[5/6] testsuite: Introduce $inferior_spawn_id

Message ID 54F04A2B.5@redhat.com
State New, archived
Headers

Commit Message

Pedro Alves Feb. 27, 2015, 10:42 a.m. UTC
  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).
>
  

Comments

Yao Qi Feb. 27, 2015, 12:08 p.m. UTC | #1
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.
  
Pedro Alves Feb. 27, 2015, 12:29 p.m. UTC | #2
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
  
Antoine Tremblay April 16, 2015, 4:55 p.m. UTC | #3
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
  
Pedro Alves April 16, 2015, 5:14 p.m. UTC | #4
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
  
Pedro Alves April 21, 2015, 6:25 p.m. UTC | #5
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
  
Antoine Tremblay April 21, 2015, 6:32 p.m. UTC | #6
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
  

Patch

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