[RFC/gdb/testsuite] Another way to avoid the windows native tty troubles

[RFC/gdb/testsuite] Another way to avoid the windows native tty troubles

[Pierre Muller]

  Sometime ago, I proposed a patch that
was trying to cope with the tty problem
on windows cygwin and mingw targets.
  

The patch was for an individual test, gdb.base/fileio.exp
http://sourceware.org/ml/gdb-patches/2007-12/msg00045.html

  Several answers concerned the reasons of the tty problems,
but I was mainly interested in getting 
more reliable testsuite results for windows targets.
  Lots of tests fail simply because the programs think that
they are not on a terminal and thus use full buffering, which is
incompatible with the testsuite logic.

But, as Pedro said, individual files are not the right location to 
fix this.

  I finally came up with a different idea, that seems to work
now quite well.

  The idea is simply to add an object that call setbuf(stdout,NULL)
in a function that is added to the ctor section.
  The source
gdb.arch/cygwin-ctor.c is simply:
$ cat ../gdb.arch/cygwin-ctor.c

#include <stdio.h>

void
disableoutbuffer ()
{
  setbuf (stdout, 0);
}

asm (".section .ctor");
asm(" .long     _disableoutbuffer");

<<< End of cygwin-ctor.c file >>>
This file can be compile with gcc as cygwin-ctor.o
or with gcc -mno-cygwin as mingw-ctor.o (allowing to 
use it for mingw tests, even if those still have lots of other problems).

  Adding this object at link time is done in 
gdb_compile proc in lib/gdb.exp (see below).

  This allows me to get about 80 failures less and more than 100 passes more
for cygwin testsuite.
  The tty troubles are still visible in the three remaining
failure in gdb.base/fileio.exp (instead of 38 failures usually)
concerning istty tests for stdin stdout and stderr. 

  The gdb_compile procedure change is rather simple,
but it could probably be made more universal by converting this into
a target info property that would be set in site.exp file for instance.

setcurrtarget_info always_link 'mingw-ctor.o'
  In that case site.exp could also contain the code needed to compile
this object from the source.

  I tought about putting the object generation inside the gdb/testsuite
Makefile,
but this would mean that people using 'runtest' directly
inside the testsuite directory would get into troubles...

  I would like to know if such a testsuite change could be 
considered for inclusion in CVS tree, and in which direction
I should go regarding these questions:
1) Should the patch stay cygwin/mingw specific or is there any interest in
using the more universal target info approach that I tried to explain above?
2) How should I cope with the object generation?
3) There is one compilation failure that seems to be specific to my
patch and is related to a compilation using the -nodefaultlibs option    
in gdb.base/prelink.so.
  This probably means that cygwin is not linked in.
Note that the compilation also fails without my patch, but for
some other reason (probably because some of the linked object do
require cygwin).

All comments most welcome,


Pierre Muller
Pascal language support maintainer for GDB




  
$ cvs diff -up gdb.exp
Index: gdb.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/lib/gdb.exp,v
retrieving revision 1.102
diff -u -p -r1.102 gdb.exp
--- gdb.exp     4 May 2008 04:04:11 -0000       1.102
+++ gdb.exp     13 Jun 2008 23:49:55 -0000
@@ -1541,6 +1541,7 @@ proc gdb_compile {source dest type optio
     global gdb_wrapper_file;
     global gdb_wrapper_flags;
     global gdb_wrapper_initialized;
+    global objdir;

     set outdir [file dirname $dest]

@@ -1626,7 +1627,18 @@ proc gdb_compile {source dest type optio
        }
        set options [lreplace $options $nowarnings $nowarnings $flag]
     }
-
+    if { ($type == "executable") &&
+       ([istarget "*-*-mingw*"]
+        || [istarget *-*-cygwin*]
+        || [istarget *-*-pe*])} {
+      verbose "Adding ctor to avoid buffer flushing problems" 2
+      if [istarget "*-*-mingw*"] {
+        lappend options "additional_flags=${objdir}/mingw-ctor.o"
+      } else {
+        lappend options "additional_flags=${objdir}/cygwin-ctor.o"
+      }
+    }
+
     set result [target_compile $source $dest $type $options];

     # Prune uninteresting compiler (and linker) output.

Re: [RFC/gdb/testsuite] Another way to avoid the windows native tty troubles

[Pedro Alves]

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

A Saturday 14 June 2008 01:15:45, Pierre Muller wrote:

>   Several answers concerned the reasons of the tty problems,
> but I was mainly interested in getting
> more reliable testsuite results for windows targets.
>   Lots of tests fail simply because the programs think that
> they are not on a terminal and thus use full buffering, which is
> incompatible with the testsuite logic.
>
> But, as Pedro said, individual files are not the right location to
> fix this.
>
>   I finally came up with a different idea, that seems to work
> now quite well.
>
>   The idea is simply to add an object that call setbuf(stdout,NULL)
> in a function that is added to the ctor section.
>   The source
> gdb.arch/cygwin-ctor.c is simply:
> $ cat ../gdb.arch/cygwin-ctor.c
>
> #include <stdio.h>
>
> void
> disableoutbuffer ()
> {
>   setbuf (stdout, 0);
> }
>
> asm (".section .ctor");
> asm(" .long     _disableoutbuffer");
>
> <<< End of cygwin-ctor.c file >>>

Pierre, this is exactly how we have fixed this issue for our
internal mingw testing.  I apologize for not telling you about it
before, and not sparring you the trouble.

We have the attached patch installed in our tree for a few
months now, and never seen a problem with it -- and we use
the same tree for linux testing too.  It works OK in remote
host testing too, and avoids as much as possible interfering
with test flags, and also avoids compiling the magic
object more than once per testsuite run.

> This file can be compile with gcc as cygwin-ctor.o
> or with gcc -mno-cygwin as mingw-ctor.o (allowing to
> use it for mingw tests, even if those still have lots of other problems).
>

>   The gdb_compile procedure change is rather simple,
> but it could probably be made more universal by converting this into
> a target info property that would be set in site.exp file for instance.

That's the reason I didn't submit this patch right away.  On the one
hand, this is a problem with the test environment, e.g., using ssh
to test mingw.  I'm not sure if running tests under MSFT's
telnetd/cmd.exe would still have this problem or not.  OTOH, cygwin
inferiors in a cygwin superior don't recognize they are running
under a tty, because GDB invokes them with CreateProcess, so
this is a GDB/Cygwin problem, not a test environment problem.

All in all, my opinion is that this is hitting everyone that tests
on Windows, so deserves to go into common GDB testsuite code.

What do others think?

-- 
Pedro Alves

[-- Attachment #2: mingw_setbuf.diff --]
[-- Type: text/x-diff, Size: 4295 bytes --]

	gdb/testsuite/
	* lib/gdb.exp (gdb_saved_set_unbuffered_mode_obj): New global.
	(gdb_compile): If target is *-*-cygwin* or *-*-mingw*, and we're
	compiling an executable, link in an object that forces unbuffered
	output.
	* lib/set_unbuffered_mode.c: New file.

---
 gdb/testsuite/lib/gdb.exp               |   45 ++++++++++++++++++++++++++++++++
 gdb/testsuite/lib/set_unbuffered_mode.c |   28 +++++++++++++++++++
 2 files changed, 73 insertions(+)

Index: src/gdb/testsuite/lib/set_unbuffered_mode.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ src/gdb/testsuite/lib/set_unbuffered_mode.c	2008-06-16 03:55:22.000000000 +0100
@@ -0,0 +1,28 @@
+/* Copyright (C) 2008 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.  */
+
+#include <stdio.h>
+
+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);
+}
Index: src/gdb/testsuite/lib/gdb.exp
===================================================================
--- src.orig/gdb/testsuite/lib/gdb.exp	2008-05-05 17:27:45.000000000 +0100
+++ src/gdb/testsuite/lib/gdb.exp	2008-06-16 03:55:22.000000000 +0100
@@ -1536,11 +1536,18 @@ proc gdb_wrapper_init { args } {
     set gdb_wrapper_initialized 1
 }
 
+# Some targets need to always link a special object in.  Save its path here.
+global gdb_saved_set_unbuffered_mode_obj
+set gdb_saved_set_unbuffered_mode_obj ""
+
 proc gdb_compile {source dest type options} {
     global GDB_TESTCASE_OPTIONS;
     global gdb_wrapper_file;
     global gdb_wrapper_flags;
     global gdb_wrapper_initialized;
+    global srcdir
+    global objdir
+    global gdb_saved_set_unbuffered_mode_obj
 
     set outdir [file dirname $dest]
 
@@ -1627,6 +1634,44 @@ proc gdb_compile {source dest type optio
 	set options [lreplace $options $nowarnings $nowarnings $flag]
     }
 
+    if { $type == "executable" } {
+	if { ([istarget "*-*-mingw*"]
+	      || [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:
+	    #  1) Insulate it from $options.
+	    #  2) Avoid compiling it for every gdb_compile invocation,
+	    #  which is time consuming, especially if we're remote
+	    #  host testing.
+	    #
+	    if { $gdb_saved_set_unbuffered_mode_obj == "" } {
+		verbose "compiling gdb_saved_set_unbuffered_obj"
+		set unbuf_src ${srcdir}/lib/set_unbuffered_mode.c
+		set unbuf_obj ${objdir}/set_unbuffered_mode.o
+
+		set result [gdb_compile "${unbuf_src}" "${unbuf_obj}" object {nowarnings}]
+		if { $result != "" } {
+		    return $result
+		}
+
+		set gdb_saved_set_unbuffered_mode_obj ${objdir}/set_unbuffered_mode_saved.o
+		# Link a copy of the output object, because the
+		# original may be automatically deleted.
+		remote_exec host "cp -f $unbuf_obj $gdb_saved_set_unbuffered_mode_obj"
+	    } else {
+		verbose "gdb_saved_set_unbuffered_obj already compiled"
+	    }
+
+	    # Rely on the internal knowledge that the global ctors are ran in
+	    # reverse link order.  In that case, we can use ldflags to
+	    # avoid copying the object file to the host multiple
+	    # times.
+	    lappend options "ldflags=$gdb_saved_set_unbuffered_mode_obj"
+	}
+    }
+
     set result [target_compile $source $dest $type $options];
 
     # Prune uninteresting compiler (and linker) output.

RE: [RFC/gdb/testsuite] Another way to avoid the windows native tty troubles

[Pierre Muller]

 Hi Pedro,

  of course your patch is far better than mine and I second the integration
into CVS of it.

  Just one thing, did you think about the -nostdlibs issue?
>3) There is one compilation failure that seems to be specific to my
>patch and is related to a compilation using the -nodefaultlibs option    
>in gdb.base/prelink.so.
>  This probably means that cygwin is not linked in.
>Note that the compilation also fails without my patch, 
>but for some other reason (probably because some of 
>the linked object do require cygwin).

  Anyhow, I support this patch!

Pierre Muller
Pascal language support maintainer for GDB




-----Message d'origine-----
De : gdb-patches-owner@sourceware.org
[mailto:gdb-patches-owner@sourceware.org] De la part de Pedro Alves
Envoyé : Monday, June 16, 2008 5:23 AM
À : gdb-patches@sourceware.org
Cc : Pierre Muller
Objet : Re: [RFC/gdb/testsuite] Another way to avoid the windows native tty
troubles

A Saturday 14 June 2008 01:15:45, Pierre Muller wrote:

>   Several answers concerned the reasons of the tty problems, but I was 
> mainly interested in getting more reliable testsuite results for 
> windows targets.
>   Lots of tests fail simply because the programs think that they are 
> not on a terminal and thus use full buffering, which is incompatible 
> with the testsuite logic.
>
> But, as Pedro said, individual files are not the right location to fix 
> this.
>
>   I finally came up with a different idea, that seems to work now 
> quite well.
>
>   The idea is simply to add an object that call setbuf(stdout,NULL) in 
> a function that is added to the ctor section.
>   The source
> gdb.arch/cygwin-ctor.c is simply:
> $ cat ../gdb.arch/cygwin-ctor.c
>
> #include <stdio.h>
>
> void
> disableoutbuffer ()
> {
>   setbuf (stdout, 0);
> }
>
> asm (".section .ctor");
> asm(" .long     _disableoutbuffer");
>
> <<< End of cygwin-ctor.c file >>>

Pierre, this is exactly how we have fixed this issue for our internal mingw
testing.  I apologize for not telling you about it before, and not sparring
you the trouble.

We have the attached patch installed in our tree for a few months now, and
never seen a problem with it -- and we use the same tree for linux testing
too.  It works OK in remote host testing too, and avoids as much as possible
interfering with test flags, and also avoids compiling the magic object more
than once per testsuite run.

> This file can be compile with gcc as cygwin-ctor.o or with gcc 
> -mno-cygwin as mingw-ctor.o (allowing to use it for mingw tests, even 
> if those still have lots of other problems).
>

>   The gdb_compile procedure change is rather simple, but it could 
> probably be made more universal by converting this into a target info 
> property that would be set in site.exp file for instance.

That's the reason I didn't submit this patch right away.  On the one hand,
this is a problem with the test environment, e.g., using ssh to test mingw.
I'm not sure if running tests under MSFT's telnetd/cmd.exe would still have
this problem or not.  OTOH, cygwin inferiors in a cygwin superior don't
recognize they are running under a tty, because GDB invokes them with
CreateProcess, so this is a GDB/Cygwin problem, not a test environment
problem.

All in all, my opinion is that this is hitting everyone that tests on
Windows, so deserves to go into common GDB testsuite code.

What do others think?

--
Pedro Alves

Re: [RFC/gdb/testsuite] Another way to avoid the windows native tty troubles

[Daniel Jacobowitz]

On Mon, Jun 16, 2008 at 09:21:42AM +0200, Pierre Muller wrote:
> >3) There is one compilation failure that seems to be specific to my
> >patch and is related to a compilation using the -nodefaultlibs option    
> >in gdb.base/prelink.so.
> >  This probably means that cygwin is not linked in.
> >Note that the compilation also fails without my patch, 
> >but for some other reason (probably because some of 
> >the linked object do require cygwin).

It doesn't matter whether the test fails to compile on Windows; it
should be restricted to Linux, because it is testing gdb interaction
with the 'prelink' program - which is GNU/Linux specific.

We have a few more Windows patches in our tree.  I promised to post
them a while ago; I haven't had time :-( But I haven't forgotten,
either!

-- 
Daniel Jacobowitz
CodeSourcery

RE: [RFC/gdb/testsuite] Another way to avoid the windows native tty troubles

[Pierre Muller]

In
http://sourceware.org/ml/gdb-patches/2008-06/msg00301.html

  Pedro sent a patch to fix the tty related problems
for cygwin/mingw32 testsuite.

  This patch is better than the one I proposed before
http://sourceware.org/ml/gdb-patches/2008-06/msg00252.html

  Am I really the only one interested in that patch?

  Does anyone have some reservations?
  I had that comment about the -nostdlibs,
but as it seems that the only test using this option
is Linux specific anyhow...
(see http://sourceware.org/ml/gdb-patches/2008-06/msg00308.html)


  Who could accept that patch?
  

Pierre Muller
Pascal language support maintainer for GDB

Re: [RFC/gdb/testsuite] Another way to avoid the windows native tty troubles

[Daniel Jacobowitz]

On Mon, Jun 23, 2008 at 09:40:00AM +0200, Pierre Muller wrote:
>   Am I really the only one interested in that patch?

As you know, it takes a minimum of a week before I get around to
reviewing patches - often a month.  I'm always open to help from
others; even if they can't approve the patch themselves, a comment
that someone else has looked at it and Seen That It Is Good is
helpful.

I'll get to it the next time I have a chance for general patch review.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFC/gdb/testsuite] Another way to avoid the windows native tty troubles

[Daniel Jacobowitz]

On Mon, Jun 16, 2008 at 04:23:20AM +0100, Pedro Alves wrote:
> 	gdb/testsuite/
> 	* lib/gdb.exp (gdb_saved_set_unbuffered_mode_obj): New global.
> 	(gdb_compile): If target is *-*-cygwin* or *-*-mingw*, and we're
> 	compiling an executable, link in an object that forces unbuffered
> 	output.
> 	* lib/set_unbuffered_mode.c: New file.

This is OK.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFC/gdb/testsuite] Another way to avoid the windows native tty troubles

[Pedro Alves]

A Thursday 26 June 2008 14:36:44, Daniel Jacobowitz wrote:
> On Mon, Jun 16, 2008 at 04:23:20AM +0100, Pedro Alves wrote:
> > 	gdb/testsuite/
> > 	* lib/gdb.exp (gdb_saved_set_unbuffered_mode_obj): New global.
> > 	(gdb_compile): If target is *-*-cygwin* or *-*-mingw*, and we're
> > 	compiling an executable, link in an object that forces unbuffered
> > 	output.
> > 	* lib/set_unbuffered_mode.c: New file.
>
> This is OK.

Checked in.  Thanks.

-- 
Pedro Alves