[PATCH 3/3] Set stdin/stdout/stderr to binary mode in cygwin.

[PATCH 3/3] Set stdin/stdout/stderr to binary mode in cygwin.

[Yao Qi]

I see the following fail on a remote windows host,

info tracepoints^M
Num     Type           Disp Enb Address    What^M^M
1       tracepoint     keep y   0x0040143f in gdb_c_test at actions.c:74^M^M
        not installed on target^M^M
2       tracepoint     keep y   0x00401687 in gdb_asm_test at actions.c:121^M^M
        not installed on target^M^M
3       tracepoint     keep y   0x004013d2 in gdb_recursion_test at actions.c:61^M^M
        not installed on target^M^M
(gdb) FAIL: gdb.trace/deltrace.exp: 3.1a: set three tracepoints

this fail is caused by an extra '\r' at end of each line.

on Windows, when a file is opened in text mode, a "\n" is always
expanded to "\r\n", so gdb on Windows is outputting "\r\n", and when
that goes throught the PTY, the '\n' is being expanded to "\r\n",
hence "\r\r\n".

This patch is to force stdin/stdout/stderr to binary mode prevents that
expansion.

gdb:

2013-07-29  Pedro Alves  <pedro@codesourcery.com>
	    Daniel Jacobowitz  <dan@codesourcery.com>
	    Yao Qi  <yao@codesourcery.com>

	* main.c [__MINGW32__]: Include fcntl.h and windows.h.
	(captured_main) [__MINGW32__]: Set stdout, stdin and stderr
	to binary mode if GDB is in cygwin.
---
 gdb/main.c |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/gdb/main.c b/gdb/main.c
index 4fa211e..7a932ef 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -47,6 +47,11 @@
 #include "filenames.h"
 #include "filestuff.h"
 
+#ifdef __MINGW32__
+#include <fcntl.h>
+#include <windows.h>
+#endif
+
 /* The selected interpreter.  This will be used as a set command
    variable, so it should always be malloc'ed - since
    do_setshow_command will free it.  */
@@ -384,6 +389,14 @@ captured_main (void *data)
 	 is performed.  */
       setvbuf (stdout, NULL, _IONBF, BUFSIZ);
       setvbuf (stderr, NULL, _IONBF, BUFSIZ);
+
+      /* In textmode, a '\n' is automatically expanded into "\r\n".  This
+	 results in expect seeing "\r\r\n".  The tests aren't prepared
+	 currently for other forms of eol.  As a workaround, we force the
+	 output to binary mode.  */
+      setmode (fileno (stdin), O_BINARY);
+      setmode (fileno (stdout), O_BINARY);
+      setmode (fileno (stderr), O_BINARY);
     }
 #endif
 
-- 
1.7.7.6

[PATCH 1/3] Detect GDB is in cygwin

[Yao Qi]

Hello,
This patch is to detect whether GDB is in cygwin by means of new added
function 'is_in_cygwin'.  In general, the detection is inspired by
Corinna's example on detecting whether GDB is running in a cygwin pty.
I extended the logic a little to handle the case we ssh to a cygwin
machine without allocating a pty.  In this patch, we'll find the file
name of stdin, if its name is prefixed by "\cygwin-", we know GDB is
running in cygwin.

The C code is simple, but getting these simple c code compiled on
various mingw compilers is not simple, due to different header files
they shipped.  I change configure.ac to first check winternl.h, if it
is not found, check other needed headers.  Then invoke AC_TRY_COMPILE
to test whether I can use function NtQueryInformationFile.

GDB is compiled with mingw32 gcc (Fedora MinGW 4.6.1-3.fc16) and
x86_64-w64-mingw32-gcc (4.8.0 20121031).

gdb:

2013-07-29  Yao Qi  <yao@codesourcery.com>
	    Corinna Vinschen  <vinschen@redhat.com>

	* configure.ac: Invoke AC_CHECK_HEADERS to check winternl.h,
	ntdef.h, winbase.h and ddk/ntddk.h.
	* config.in: Re-generated.
	* configure: Re-generated.
	* defs.h [__MINGW32__] (is_in_cygwin_p): Declare.
	* mingw-hdep.c: Inlcude wchar.h.
	[HAVE_WINTERNL_H]: Include winternl.h.
	[!HAVE_WINTERNL_H] [HAVE_WINBASE_H]: Include ntdef.h.
	[!HAVE_WINTERNL_H] [HAVE_NTDEF_H]: winbase.h.
	[!HAVE_WINTERNL_H] [HAVE_DDK_NTDDK_H]ddk/ntddk.h.
	(pNtQueryInformationFile): Declare.
	[HAVE_NTQUERYINFORMATIONFILE] (get_filename_from_handle):
	New.
	(is_in_cygwin_p): New.
---
 gdb/config.in    |   15 +++++++++
 gdb/configure    |   82 +++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/configure.ac |   40 ++++++++++++++++++++++++
 gdb/defs.h       |    4 ++
 gdb/mingw-hdep.c |   90 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 231 insertions(+), 0 deletions(-)

diff --git a/gdb/config.in b/gdb/config.in
index 92c2789..4fb45b4 100644
--- a/gdb/config.in
+++ b/gdb/config.in
@@ -84,6 +84,9 @@
 /* Define to 1 if you have the <curses.h> header file. */
 #undef HAVE_CURSES_H
 
+/* Define to 1 if you have the <ddk/ntddk.h> header file. */
+#undef HAVE_DDK_NTDDK_H
+
 /* Define to 1 if you have the declaration of `ADDR_NO_RANDOMIZE', and to 0 if
    you don't. */
 #undef HAVE_DECL_ADDR_NO_RANDOMIZE
@@ -270,6 +273,12 @@
 /* Define to 1 if you have the <nlist.h> header file. */
 #undef HAVE_NLIST_H
 
+/* Define to 1 if you have the <ntdef.h> header file. */
+#undef HAVE_NTDEF_H
+
+/* Define if function NtQueryInformationFile can be used. */
+#undef HAVE_NTQUERYINFORMATIONFILE
+
 /* Define if you support the personality syscall. */
 #undef HAVE_PERSONALITY
 
@@ -572,6 +581,12 @@
 /* Define to 1 if you have the `wborder' function. */
 #undef HAVE_WBORDER
 
+/* Define to 1 if you have the <winbase.h> header file. */
+#undef HAVE_WINBASE_H
+
+/* Define to 1 if you have the <winternl.h> header file. */
+#undef HAVE_WINTERNL_H
+
 /* Define to 1 if `fork' works. */
 #undef HAVE_WORKING_FORK
 
diff --git a/gdb/configure b/gdb/configure
index 4833297..2eb3bb8 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -9030,6 +9030,38 @@ fi
 done
 
 
+# Check header winternl.h, if not found, check ntdef.h winbase.h and ddk/ntddk.h.
+case "${host}" in
+  *-*-mingw*) for ac_header in winternl.h
+do :
+  ac_fn_c_check_header_mongrel "$LINENO" "winternl.h" "ac_cv_header_winternl_h" "$ac_includes_default"
+if test "x$ac_cv_header_winternl_h" = x""yes; then :
+  cat >>confdefs.h <<_ACEOF
+#define HAVE_WINTERNL_H 1
+_ACEOF
+
+else
+  for ac_header in ntdef.h winbase.h ddk/ntddk.h
+do :
+  as_ac_Header=`$as_echo "ac_cv_header_$ac_header" | $as_tr_sh`
+ac_fn_c_check_header_mongrel "$LINENO" "$ac_header" "$as_ac_Header" "$ac_includes_default"
+eval as_val=\$$as_ac_Header
+   if test "x$as_val" = x""yes; then :
+  cat >>confdefs.h <<_ACEOF
+#define `$as_echo "HAVE_$ac_header" | $as_tr_cpp` 1
+_ACEOF
+
+fi
+
+done
+
+fi
+
+done
+
+		;;
+esac
+
 # ------------------------- #
 # Checks for declarations.  #
 # ------------------------- #
@@ -11962,6 +11994,56 @@ fi
 $as_echo "$found" >&6; }
 
 
+
+cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+#include <windows.h>
+int
+main ()
+{
+
+#ifdef HAVE_WINTERNL_H
+#include <winternl.h>
+#else
+
+#ifdef HAVE_WINBASE_H
+#include <winbase.h>
+#endif
+#ifdef HAVE_NTDEF_H
+#include <ntdef.h>
+#endif
+#ifdef HAVE_DDK_NTDDK_H
+#include <ddk/ntddk.h>
+#endif
+
+#endif
+HANDLE fh;
+IO_STATUS_BLOCK io;
+NTSTATUS status;
+long buf[66];
+
+NTSTATUS (NTAPI *pNtQueryInformationFile) (HANDLE, PIO_STATUS_BLOCK, PVOID,
+	 				   ULONG, FILE_INFORMATION_CLASS);
+pNtQueryInformationFile = (NTSTATUS (NTAPI *)(HANDLE, PIO_STATUS_BLOCK,
+					      PVOID, ULONG, FILE_INFORMATION_CLASS))
+	GetProcAddress (GetModuleHandle ("ntdll.dll"), "NtQueryInformationFile");
+pNtQueryInformationFile (fh, &io, pfni, sizeof buf, FileNameInformation);
+
+
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_c_try_compile "$LINENO"; then :
+
+else
+
+$as_echo "#define HAVE_NTQUERYINFORMATIONFILE 1" >>confdefs.h
+
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+
+
 if test ${build} = ${host} -a ${host} = ${target} ; then
    case ${host_os} in
    solaris*)
diff --git a/gdb/configure.ac b/gdb/configure.ac
index 48f37c8..63284f3 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -1121,6 +1121,13 @@ AC_CHECK_HEADERS(term.h, [], [],
 #endif
 ])
 
+# Check header winternl.h, if not found, check ntdef.h winbase.h and ddk/ntddk.h.
+case "${host}" in
+  *-*-mingw*) AC_CHECK_HEADERS(winternl.h, [],
+  	      	[AC_CHECK_HEADERS(ntdef.h winbase.h ddk/ntddk.h, [], [],[])], [])
+		;;
+esac
+
 # ------------------------- #
 # Checks for declarations.  #
 # ------------------------- #
@@ -1715,6 +1722,39 @@ fi
 AC_SUBST(RDYNAMIC)
 AC_MSG_RESULT($found)
 
+
+
+AC_TRY_COMPILE([#include <windows.h>], [
+#ifdef HAVE_WINTERNL_H
+#include <winternl.h>
+#else
+
+#ifdef HAVE_WINBASE_H
+#include <winbase.h>
+#endif
+#ifdef HAVE_NTDEF_H
+#include <ntdef.h>
+#endif
+#ifdef HAVE_DDK_NTDDK_H
+#include <ddk/ntddk.h>
+#endif
+
+#endif
+HANDLE fh;
+IO_STATUS_BLOCK io;
+NTSTATUS status;
+long buf[66];
+
+NTSTATUS (NTAPI *pNtQueryInformationFile) (HANDLE, PIO_STATUS_BLOCK, PVOID,
+	 				   ULONG, FILE_INFORMATION_CLASS);
+pNtQueryInformationFile = (NTSTATUS (NTAPI *)(HANDLE, PIO_STATUS_BLOCK,
+					      PVOID, ULONG, FILE_INFORMATION_CLASS))
+	GetProcAddress (GetModuleHandle ("ntdll.dll"), "NtQueryInformationFile");
+pNtQueryInformationFile (fh, &io, pfni, sizeof buf, FileNameInformation);
+
+], [],
+[AC_DEFINE(HAVE_NTQUERYINFORMATIONFILE, 1, [Define if function NtQueryInformationFile can be used.])])
+
 dnl For certain native configurations, we need to check whether thread
 dnl support can be built in or not.
 dnl
diff --git a/gdb/defs.h b/gdb/defs.h
index 014d7d4..b226157 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -790,6 +790,10 @@ enum block_enum
   FIRST_LOCAL_BLOCK = 2
 };
 
+#ifdef __MINGW32__
+int is_in_cygwin_p (void);
+#endif
+
 #include "utils.h"
 
 #endif /* #ifndef DEFS_H */
diff --git a/gdb/mingw-hdep.c b/gdb/mingw-hdep.c
index efc9848..2ba8017 100644
--- a/gdb/mingw-hdep.c
+++ b/gdb/mingw-hdep.c
@@ -28,6 +28,22 @@
 #include "readline/readline.h"
 
 #include <windows.h>
+#include <wchar.h>
+
+#ifdef HAVE_WINTERNL_H
+#include <winternl.h>
+#else
+#ifdef HAVE_WINBASE_H
+#include <winbase.h>
+#endif
+#ifdef HAVE_NTDEF_H
+#include <ntdef.h>
+#endif
+#ifdef HAVE_DDK_NTDDK_H
+#include <ddk/ntddk.h>
+#endif
+
+#endif /* HAVE_WINTERNL_H */
 
 /* This event is signalled whenever an asynchronous SIGINT handler
    needs to perform an action in the main thread.  */
@@ -265,6 +281,80 @@ gdb_call_async_signal_handler (struct async_signal_handler *handler,
   SetEvent (sigint_event);
 }
 
+#if HAVE_NTQUERYINFORMATIONFILE
+
+/* Return the file name of handle FH.  */
+
+static PWCHAR
+get_filename_from_handle (HANDLE fh)
+{
+  IO_STATUS_BLOCK io;
+  NTSTATUS status;
+  long buf[66];	/* NAME_MAX + 1 + sizeof ULONG */
+  PFILE_NAME_INFORMATION pfni = (PFILE_NAME_INFORMATION) buf;
+  static NTSTATUS (NTAPI *pNtQueryInformationFile) (HANDLE,
+						    PIO_STATUS_BLOCK,
+						    PVOID, ULONG,
+						    FILE_INFORMATION_CLASS);
+
+  /* Calling the native NT function NtQueryInformationFile is required to
+     support pre-Vista systems.  If that's of no concern, Vista introduced
+     the GetFileInformationByHandleEx call with the FileNameInfo info class,
+     which can be used instead. */
+  if (!pNtQueryInformationFile)
+    {
+      pNtQueryInformationFile = (NTSTATUS (NTAPI *)(HANDLE, PIO_STATUS_BLOCK,
+						    PVOID, ULONG, FILE_INFORMATION_CLASS))
+	GetProcAddress (GetModuleHandle ("ntdll.dll"),
+			"NtQueryInformationFile");
+      if (pNtQueryInformationFile == NULL)
+	return NULL;
+    }
+  if (!NT_SUCCESS (pNtQueryInformationFile (fh, &io, pfni, sizeof buf,
+					   FileNameInformation)))
+    return NULL;
+
+  /* The filename is not guaranteed to be NUL-terminated. */
+  pfni->FileName[pfni->FileNameLength / sizeof (WCHAR)] = L'\0';
+
+  return pfni->FileName;
+}
+
+#endif /* HAVE_NTQUERYINFORMATIONFILE */
+
+/* Return true if GDB is running in Cygwin with or without pseudo-tty.  */
+
+int
+is_in_cygwin_p (void)
+{
+  PWCHAR cp;
+  /* Now fetch the underlying HANDLE of stdin. */
+  HANDLE fh = (HANDLE) _get_osfhandle (fileno (stdin));
+
+  if (!fh || fh == INVALID_HANDLE_VALUE)
+    return 0;
+
+#ifdef HAVE_NTQUERYINFORMATIONFILE
+  cp = get_filename_from_handle (fh);
+#else
+  cp = NULL;
+#endif
+
+  /* Now check the name pattern.  With pseudo-tty, the filename of
+     handle of stdin looks like this:
+
+       \cygwin-c5e39b7a9d22bafb-{p,t}ty1-from-master
+
+     Without pseudo-tty, the filename of handle of stdin looks like this:
+
+       \cygwin-c5e39b7a9d22bafb-pipe-0x14C8-0x3
+
+     If the file name is prefixed with "\cygwin-", GDB is running in
+     cygwin.  */
+
+  return (cp != NULL && wcsncmp (cp, L"\\cygwin-", 8) == 0);
+}
+
 /* -Wmissing-prototypes */
 extern initialize_file_ftype _initialize_mingw_hdep;
 
-- 
1.7.7.6

[PATCH 0/3 V3] Test mingw32 GDB in cygwin

[Yao Qi]

This patch series try to fix the problems we've seen on running
mingw32 native for testing in cygwin.  Patch 2/3 unbuffer the
stdout and stderr, so that dejagnu/expect can match the output in
the right order.  Likewise, patch 3/3 sets stdin/stdout/stderr into
binary mode, so that dejagnu/expects can match the eol correctly
too.  In order to avoid the side effects of these changes to native
win32 platform, we need some bits to detect whether GDB is running
in cygwin.  This is what patch 1/3 tries to do, and most of
discussions are on it.

In V2, I proposed a new GDB option '--cygwin-tty' to tell GDB that
it is in cygwin.  People don't like it, and Corinna gave an example
that we can detect whether GDB is in cygwin or not.

Thanks to Corinna's example, we can know whether GDB is in cygwin
by checking the file name of handler of stdin (or stdout).  As a
result, a new option '--cygwin-tty' is avoided.  Patch 1/3 is
almost rewritten in V3.

The whole series are tested on native mingw32 GDB running in cygwin.
Test results are improved dramatically.

My plan next step would be to wrap isatty by gdb_isatty, which uses
the logic in Corinna's example to return the correct result on
cygwin pty.  I didn't include this change into this series, because
I get some exceptions when GDB starts up from time to time:

  (gdb) Exception condition detected on fd 0
  error detected on stdin

It takes time investigating and I decide to stop here.

V2 of this series can be found in
<http://sourceware.org/ml/gdb-patches/2013-07/msg00590.html>.

*** BLURB HERE ***

Yao Qi (3):
  Detect GDB is in cygwin
  Unbuffer stdout and stderr in cygwin
  Set stdin/stdout/stderr to binary mode in cygwin.

 gdb/config.in    |   15 +++++++++
 gdb/configure    |   82 +++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/configure.ac |   40 ++++++++++++++++++++++++
 gdb/defs.h       |    4 ++
 gdb/main.c       |   25 +++++++++++++++
 gdb/mingw-hdep.c |   90 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 256 insertions(+), 0 deletions(-)

-- 
1.7.7.6

[PATCH 2/3] Unbuffer stdout and stderr in cygwin

[Yao Qi]

Hi,
This patch is to disable the buffering on mingw32 host when it is running
in cygwin, because the error message and gdb prompt come out in
different orders, which causes a lot of test fails.

We call setvbuf this place, because it is a place "before any other
operation is performed".  See the doc below:

  "The setvbuf() function may be used after the stream pointed to by
stream is associated with an open file but before any other operation
(other than an unsuccessful call to setvbuf()) is performed on the
stream."

It is not the first time this patch show up here.  Daniel posted it
http://sourceware.org/ml/gdb-patches/2009-06/msg00433.html and Joel
preferred it as the exact same piece of code is in their tree as well
http://sourceware.org/ml/gdb-patches/2009-06/msg00434.html
Eli wanted to check this patch didn't interfere with Emacs 23 GDB
interface on Windows, which is probably the last question to this
patch.  The discussion stopped there.  I build native mingw32 gdb
with buffering disabled, and use it with Emacs 24.3 in Windows
cmd.exe.  Emacs+GDB behave correctly.

gdb:

2013-07-29  Joseph Myers  <joseph@codesourcery.com>
	    Yao Qi  <yao@codesourcery.com>

	* main.c (captured_main) [__MINGW32__]: Set stdout and stderr
	unbuffered if GDB is running in Cygwin.
---
 gdb/main.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/gdb/main.c b/gdb/main.c
index 440094e..4fa211e 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -375,6 +375,18 @@ captured_main (void *data)
   saved_command_line[0] = '\0';
   instream = stdin;
 
+#ifdef __MINGW32__
+  if (is_in_cygwin_p ())
+    {
+      /* A Cygwin session may not look like a terminal to the Windows
+	 runtime; ensure unbuffered output.  Note that setvbuf may be
+	 used after the file is opened but before any other operation
+	 is performed.  */
+      setvbuf (stdout, NULL, _IONBF, BUFSIZ);
+      setvbuf (stderr, NULL, _IONBF, BUFSIZ);
+    }
+#endif
+
   gdb_stdout = stdio_fileopen (stdout);
   gdb_stderr = stdio_fileopen (stderr);
   gdb_stdlog = gdb_stderr;	/* for moment */
-- 
1.7.7.6

RE: [PATCH 0/3 V3] Test mingw32 GDB in cygwin

[Pierre Muller]

Hi all,


> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Yao Qi
> Envoyé : lundi 29 juillet 2013 10:46
> À : gdb-patches@sourceware.org
> Objet : [PATCH 0/3 V3] Test mingw32 GDB in cygwin
> 
> This patch series try to fix the problems we've seen on running
> mingw32 native for testing in cygwin.  Patch 2/3 unbuffer the
> stdout and stderr, so that dejagnu/expect can match the output in
> the right order.  Likewise, patch 3/3 sets stdin/stdout/stderr into
> binary mode, so that dejagnu/expects can match the eol correctly
> too.  In order to avoid the side effects of these changes to native
> win32 platform, we need some bits to detect whether GDB is running
> in cygwin.  This is what patch 1/3 tries to do, and most of
> discussions are on it.
> 
> In V2, I proposed a new GDB option '--cygwin-tty' to tell GDB that
> it is in cygwin.  People don't like it, and Corinna gave an example
> that we can detect whether GDB is in cygwin or not.

  I didn't like it because I explained that the changes you propose are
useful
to run the testsuite on Windows OS, but not only if running
under a cygwin terminal.
  I am using a msys port of dejagnu expect,
and this needs the same changes (remove buffering and switch to binary
mode),
but with the new version of your patch, 
nothing would happen for me and the testsuite would still fail.


 
> Thanks to Corinna's example, we can know whether GDB is in cygwin
> by checking the file name of handler of stdin (or stdout).  As a
> result, a new option '--cygwin-tty' is avoided.  Patch 1/3 is
> almost rewritten in V3.

  Yes, that's true, but the down side is that you transform
the terminal settings only when you are on a cygwin terminal...
 I think this is not correct:
 I suspect that is_in_cygwin will return true on every type of cygwin
terminal,
in particular also when you are in interactive mode... 
  I thought that your patch should only change settings if the pipes
from expect redirection are detected, but there are no pipe checks
anymore...
Note that I am not sure of the above, as 'maybe' the pty/tty name doesn't
match in the interactive shell case.
  Secondly, your patch will never trigger the no buffer/binary mode changes
on non-cygwin terminals, while I argued before that it should also be usable
in those conditions.
  
  This is why I would rather like to have the settings modification
grouped into a function (let's call it setup_handles_for_testsuite)
which would be an empty function for all but __MINGW32__ code.
This function could then be called by
(gdb) set maint testsuite-mode on
command or automatically when a cygwin shell and pipes are detected.

  I am sorry to bother you again, but I would really like to
get a patch that can be used in more general framework and
that avoids to change the standard handles when not necessary.

  And, to finish, I would like to reiterate my support for this
patch, even though I am still criticizing it.

Sorry to bother you more,


Pierre Muller

Re: [PATCH 1/3] Detect GDB is in cygwin

[Eli Zaretskii]

> From: Yao Qi <yao@codesourcery.com>
> Date: Mon, 29 Jul 2013 16:45:44 +0800
> 
> The C code is simple, but getting these simple c code compiled on
> various mingw compilers is not simple, due to different header files
> they shipped.  I change configure.ac to first check winternl.h, if it
> is not found, check other needed headers.  Then invoke AC_TRY_COMPILE
> to test whether I can use function NtQueryInformationFile.

Thanks.

However, I'm not sure you need to probe NtQueryInformationFile at
configure time, as long as GDB probes for it at run time.  That is the
usual way of using APIs that might not be available on older Windows
systems.

Probing at compile time is worse than at run time, because it assumes
that GDB will run on the same machine where it was built, which is a
bad assumption, especially on Windows, where people like to install
precompiled binaries.

Otherwise, the patch looks good to me (although I only read it, didn't
try to build with it).

Re: [PATCH 2/3] Unbuffer stdout and stderr in cygwin

[Eli Zaretskii]

> From: Yao Qi <yao@codesourcery.com>
> Date: Mon, 29 Jul 2013 16:45:45 +0800
> 
> +#ifdef __MINGW32__
> +  if (is_in_cygwin_p ())

I would suggest to call the function using_cygwin_pty or some such.
"is_in_cygwin" is IMO too ambiguous.

> +      setvbuf (stdout, NULL, _IONBF, BUFSIZ);
> +      setvbuf (stderr, NULL, _IONBF, BUFSIZ);

How about using line buffering instead on both streams?  Or at least
leave stdout line-buffered?  Did you try that, and if so, did that
have the same problems that triggered these patches?

You see, the way your patch works, using GDB from a Cygwin shell
window will always do the above, even when not running the test
suite.  Users might be unhappy that their standard output suddenly
becomes much less efficient.

Re: [PATCH 3/3] Set stdin/stdout/stderr to binary mode in cygwin.

[Eli Zaretskii]

> From: Yao Qi <yao@codesourcery.com>
> Date: Mon, 29 Jul 2013 16:45:46 +0800
> 
> +      setmode (fileno (stdin), O_BINARY);
> +      setmode (fileno (stdout), O_BINARY);
> +      setmode (fileno (stderr), O_BINARY);

Do you really need all 3 of the standard handles in binary mode?  I
thought the problem was only with output?

Re: [PATCH 0/3 V3] Test mingw32 GDB in cygwin

[Tom Tromey]

>>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:

Yao> My plan next step would be to wrap isatty by gdb_isatty, which uses
Yao> the logic in Corinna's example to return the correct result on
Yao> cygwin pty.

I wonder if this could be fixed in the gnulib isatty module instead.

Tom

Re: [PATCH 0/3 V3] Test mingw32 GDB in cygwin

[Eli Zaretskii]

> From: Tom Tromey <tromey@redhat.com>
> Cc: <gdb-patches@sourceware.org>
> Date: Mon, 29 Jul 2013 12:03:09 -0600
> 
> >>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:
> 
> Yao> My plan next step would be to wrap isatty by gdb_isatty, which uses
> Yao> the logic in Corinna's example to return the correct result on
> Yao> cygwin pty.
> 
> I wonder if this could be fixed in the gnulib isatty module instead.

That could be better, but the procedure would be much longer (from my
experience), and we currently don't import that module anyway.

Btw, what does the proposed code produce when GDB runs from the MSYS
Bash window?  If it recognizes it as a Cygwin shell, then I'd very
much like GDB not to do that, as I frequently need to run GDB like
that, when some complicated shell command or script crashes, and
recreating a similar situation from the Windows shell is impractical.

Re: [PATCH 0/3 V3] Test mingw32 GDB in cygwin

[Yao Qi]

On 07/29/2013 10:03 PM, Pierre Muller wrote:
>    I didn't like it because I explained that the changes you propose are
> useful
> to run the testsuite on Windows OS, but not only if running
> under a cygwin terminal.

Pierre,
the intention of this patch series is to improve the testing mingw 
native gdb running on a cygwin through ssh.

>    I am using a msys port of dejagnu expect,
> and this needs the same changes (remove buffering and switch to binary
> mode),
> but with the new version of your patch,
> nothing would happen for me and the testsuite would still fail.
>

Your environment is different from ours.  We run dejagnu/expect on linux 
machine, and test mingw32 gdb in a cygwin on windows machine through 
remote host mechanism.

>
>
>> >Thanks to Corinna's example, we can know whether GDB is in cygwin
>> >by checking the file name of handler of stdin (or stdout).  As a
>> >result, a new option '--cygwin-tty' is avoided.  Patch 1/3 is
>> >almost rewritten in V3.
>    Yes, that's true, but the down side is that you transform
> the terminal settings only when you are on a cygwin terminal...
>   I think this is not correct:
>   I suspect that is_in_cygwin will return true on every type of cygwin
> terminal,
> in particular also when you are in interactive mode...

Yes, anything wrong here?

>    I thought that your patch should only change settings if the pipes
> from expect redirection are detected, but there are no pipe checks
> anymore...

expect redirection is not involved in our testing.  We are running 
testsuite in remote host mode, so dejagnu/expect is on a linux machine.

> Note that I am not sure of the above, as 'maybe' the pty/tty name doesn't
> match in the interactive shell case.
>    Secondly, your patch will never trigger the no buffer/binary mode changes
> on non-cygwin terminals, while I argued before that it should also be usable
> in those conditions.
>

It is expected.  We test GDB for different targets on linux host and 
windows host (in cygwin) respectively.  We don't have the environment 
you described, so I am unable to extend the patches to support your 
environment, sorry.

>    This is why I would rather like to have the settings modification
> grouped into a function (let's call it setup_handles_for_testsuite)
> which would be an empty function for all but __MINGW32__ code.
> This function could then be called by
> (gdb) set maint testsuite-mode on
> command or automatically when a cygwin shell and pipes are detected.

If you really need an option, feel free to add one, and adjust my patches.

>
>    I am sorry to bother you again, but I would really like to
> get a patch that can be used in more general framework and
> that avoids to change the standard handles when not necessary.
>
>    And, to finish, I would like to reiterate my support for this
> patch, even though I am still criticizing it.
>
> Sorry to bother you more,

Never mind.  I don't see much conflicts here.  I'd like to improve mingw 
gdb testing in cygwin via remote host, and you'd like to improve mingw 
gdb testing on windows machine.  We have something in common, such as 
unbuffer stdout/stderr, etc, but we have different criteria on when to 
turn on these changes.  In my side, they can be turned on when gdb is 
running in cygwin, while in your side, then can be turned on when a 
certain option is set.  I don't think my patches make troubles or 
regressions to what you want to achieve.  I am inclined to to get these 
patches committed (after the review), and you can do some changes for 
your purpose on top of it.

-- 
Yao (齐尧)

Re: [PATCH 1/3] Detect GDB is in cygwin

[Yao Qi]

On 07/29/2013 11:38 PM, Eli Zaretskii wrote:
> However, I'm not sure you need to probe NtQueryInformationFile at
> configure time, as long as GDB probes for it at run time.  That is the
> usual way of using APIs that might not be available on older Windows
> systems.
>
> Probing at compile time is worse than at run time, because it assumes
> that GDB will run on the same machine where it was built, which is a
> bad assumption, especially on Windows, where people like to install
> precompiled binaries.

I am using AC_TRY_COMPILE to compile the code which probes 
NtQueryInformationFile, to make sure the code can be compiled 
successfully.  However, the code probes NtQueryInformationFile doesn't 
run on configure time.

-- 
Yao (齐尧)

Re: [PATCH 1/3] Detect GDB is in cygwin

[Eli Zaretskii]

> Date: Tue, 30 Jul 2013 17:26:35 +0800
> From: Yao Qi <yao@codesourcery.com>
> CC: <gdb-patches@sourceware.org>
> 
> I am using AC_TRY_COMPILE to compile the code which probes 
> NtQueryInformationFile, to make sure the code can be compiled 
> successfully.

There's no need to probe that at configure time: this code can always
be compiled on any MS-Windows system.  The compiler doesn't care and
doesn't check at compile time whether there is in fact
NtQueryInformationFile function inside the DLL.

Re: [PATCH 1/3] Detect GDB is in cygwin

[Yao Qi]

On 07/30/2013 11:33 PM, Eli Zaretskii wrote:
> There's no need to probe that at configure time: this code can always
> be compiled on any MS-Windows system.  The compiler doesn't care and
> doesn't check at compile time whether there is in fact
> NtQueryInformationFile function inside the DLL.

OK.  If so, in configure.ac, we only need to check headers, and don't need
AC_TRY_COMPILE at all.  Here is the updated one, with several changes,

 - Remove AC_TRY_COMPILE,
 - Include fewer headers in mingw-htep.c, and don't have to check them
in configure.ac.
 - Rename is_in_cygwin to using_cygwin_pty,

-- 
Yao (齐尧)

gdb:

2013-08-01  Yao Qi  <yao@codesourcery.com>
	    Corinna Vinschen  <vinschen@redhat.com>

	* configure.ac: Invoke AC_CHECK_HEADERS to check winternl.h,
	and ddk/ntddk.h.
	* config.in: Re-generated.
	* configure: Re-generated.
	* defs.h [__MINGW32__] (using_cygwin_pty): Declare.
	* mingw-hdep.c: Inlcude wchar.h.
	[HAVE_WINTERNL_H]: Include winternl.h.  Define
	USE_NTQUERYINFORMATIONFILE.
	[!HAVE_WINTERNL_H] [HAVE_DDK_NTDDK_H]: Include ddk/ntddk.h.
	Define USE_NTQUERYINFORMATIONFILE..
	[USE_NTQUERYINFORMATIONFILE] (get_filename_from_handle): New.
	(using_cygwin_pty): New.
---
 gdb/config.in    |    6 ++++
 gdb/configure    |   30 ++++++++++++++++++
 gdb/configure.ac |    7 ++++
 gdb/defs.h       |    4 ++
 gdb/mingw-hdep.c |   88 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 135 insertions(+), 0 deletions(-)

diff --git a/gdb/config.in b/gdb/config.in
index 92c2789..010146c 100644
--- a/gdb/config.in
+++ b/gdb/config.in
@@ -84,6 +84,9 @@
 /* Define to 1 if you have the <curses.h> header file. */
 #undef HAVE_CURSES_H
 
+/* Define to 1 if you have the <ddk/ntddk.h> header file. */
+#undef HAVE_DDK_NTDDK_H
+
 /* Define to 1 if you have the declaration of `ADDR_NO_RANDOMIZE', and to 0 if
    you don't. */
 #undef HAVE_DECL_ADDR_NO_RANDOMIZE
@@ -572,6 +575,9 @@
 /* Define to 1 if you have the `wborder' function. */
 #undef HAVE_WBORDER
 
+/* Define to 1 if you have the <winternl.h> header file. */
+#undef HAVE_WINTERNL_H
+
 /* Define to 1 if `fork' works. */
 #undef HAVE_WORKING_FORK
 
diff --git a/gdb/configure b/gdb/configure
index 4833297..ecfa203 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -9030,6 +9030,36 @@ fi
 done
 
 
+# Check header winternl.h, if not found, check ddk/ntddk.h.
+case "${host}" in
+  *-*-mingw*) for ac_header in winternl.h
+do :
+  ac_fn_c_check_header_mongrel "$LINENO" "winternl.h" "ac_cv_header_winternl_h" "$ac_includes_default"
+if test "x$ac_cv_header_winternl_h" = x""yes; then :
+  cat >>confdefs.h <<_ACEOF
+#define HAVE_WINTERNL_H 1
+_ACEOF
+
+else
+  for ac_header in ddk/ntddk.h
+do :
+  ac_fn_c_check_header_mongrel "$LINENO" "ddk/ntddk.h" "ac_cv_header_ddk_ntddk_h" "$ac_includes_default"
+if test "x$ac_cv_header_ddk_ntddk_h" = x""yes; then :
+  cat >>confdefs.h <<_ACEOF
+#define HAVE_DDK_NTDDK_H 1
+_ACEOF
+
+fi
+
+done
+
+fi
+
+done
+
+		;;
+esac
+
 # ------------------------- #
 # Checks for declarations.  #
 # ------------------------- #
diff --git a/gdb/configure.ac b/gdb/configure.ac
index 48f37c8..c3df68e 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -1121,6 +1121,13 @@ AC_CHECK_HEADERS(term.h, [], [],
 #endif
 ])
 
+# Check header winternl.h, if not found, check ddk/ntddk.h.
+case "${host}" in
+  *-*-mingw*) AC_CHECK_HEADERS(winternl.h, [],
+  	      	[AC_CHECK_HEADERS(ddk/ntddk.h, [], [],[])], [])
+		;;
+esac
+
 # ------------------------- #
 # Checks for declarations.  #
 # ------------------------- #
diff --git a/gdb/defs.h b/gdb/defs.h
index 014d7d4..71c1c30 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -790,6 +790,10 @@ enum block_enum
   FIRST_LOCAL_BLOCK = 2
 };
 
+#ifdef __MINGW32__
+int using_cygwin_pty (void);
+#endif
+
 #include "utils.h"
 
 #endif /* #ifndef DEFS_H */
diff --git a/gdb/mingw-hdep.c b/gdb/mingw-hdep.c
index efc9848..0ca15ee 100644
--- a/gdb/mingw-hdep.c
+++ b/gdb/mingw-hdep.c
@@ -28,6 +28,19 @@
 #include "readline/readline.h"
 
 #include <windows.h>
+#include <wchar.h>
+
+#ifdef HAVE_WINTERNL_H
+#include <winternl.h>
+#define USE_NTQUERYINFORMATIONFILE 1
+#else
+
+#ifdef HAVE_DDK_NTDDK_H
+#include <ddk/ntddk.h>
+#define USE_NTQUERYINFORMATIONFILE 1
+#endif /*HAVE_DDK_NTDDK_H */
+
+#endif /* HAVE_WINTERNL_H */
 
 /* This event is signalled whenever an asynchronous SIGINT handler
    needs to perform an action in the main thread.  */
@@ -265,6 +278,81 @@ gdb_call_async_signal_handler (struct async_signal_handler *handler,
   SetEvent (sigint_event);
 }
 
+#ifdef USE_NTQUERYINFORMATIONFILE
+
+/* Return the file name of handle FH.  */
+
+static PWCHAR
+get_filename_from_handle (HANDLE fh)
+{
+  IO_STATUS_BLOCK io;
+  NTSTATUS status;
+  long buf[66];	/* NAME_MAX + 1 + sizeof ULONG */
+  PFILE_NAME_INFORMATION pfni = (PFILE_NAME_INFORMATION) buf;
+  static NTSTATUS (NTAPI *pNtQueryInformationFile) (HANDLE,
+						    PIO_STATUS_BLOCK,
+						    PVOID, ULONG,
+						    FILE_INFORMATION_CLASS);
+
+  /* Calling the native NT function NtQueryInformationFile is required to
+     support pre-Vista systems.  If that's of no concern, Vista introduced
+     the GetFileInformationByHandleEx call with the FileNameInfo info class,
+     which can be used instead. */
+  if (!pNtQueryInformationFile)
+    {
+      pNtQueryInformationFile = (NTSTATUS (NTAPI *)(HANDLE, PIO_STATUS_BLOCK,
+						    PVOID, ULONG, FILE_INFORMATION_CLASS))
+	GetProcAddress (GetModuleHandle ("ntdll.dll"),
+			"NtQueryInformationFile");
+      if (pNtQueryInformationFile == NULL)
+	return NULL;
+    }
+  if (!NT_SUCCESS (pNtQueryInformationFile (fh, &io, pfni, sizeof buf,
+					   FileNameInformation)))
+    return NULL;
+
+  /* The filename is not guaranteed to be NUL-terminated. */
+  pfni->FileName[pfni->FileNameLength / sizeof (WCHAR)] = L'\0';
+
+  return pfni->FileName;
+}
+
+#endif /* USE_NTQUERYINFORMATIONFILE */
+
+/* Return true if GDB is running in Cygwin pseudo-tty.  */
+
+int
+using_cygwin_pty (void)
+{
+  PWCHAR cp;
+  /* Now fetch the underlying HANDLE of stdin. */
+  HANDLE fh = (HANDLE) _get_osfhandle (fileno (stdin));
+
+  if (!fh || fh == INVALID_HANDLE_VALUE)
+    return 0;
+
+#ifdef USE_NTQUERYINFORMATIONFILE
+  cp = get_filename_from_handle (fh);
+#else
+  cp = NULL;
+#endif
+
+  /* Now check the name pattern.  With pseudo-tty allocated in ssh,
+     the filename of handle of stdin looks like this:
+
+       \cygwin-c5e39b7a9d22bafb-{p,t}ty1-from-master
+
+     Without pseudo-tty allocated in ssh, the filename of handle of
+     stdin looks like this:
+
+       \cygwin-c5e39b7a9d22bafb-pipe-0x14C8-0x3
+
+     If the file name is prefixed with "\cygwin-", GDB is running in
+     cygwin.  */
+
+  return (cp != NULL && wcsncmp (cp, L"\\cygwin-", 8) == 0);
+}
+
 /* -Wmissing-prototypes */
 extern initialize_file_ftype _initialize_mingw_hdep;
 
-- 
1.7.7.6

Re: [PATCH 2/3] Unbuffer stdout and stderr in cygwin

[Yao Qi]

On 07/29/2013 11:42 PM, Eli Zaretskii wrote:
> I would suggest to call the function using_cygwin_pty or some such.
> "is_in_cygwin" is IMO too ambiguous.
> 

OK, fixed in patch 1/3.

>> >+      setvbuf (stdout, NULL, _IONBF, BUFSIZ);
>> >+      setvbuf (stderr, NULL, _IONBF, BUFSIZ);
> How about using line buffering instead on both streams?  Or at least
> leave stdout line-buffered?  Did you try that, and if so, did that
> have the same problems that triggered these patches?

I tried line-buffering for stdout, but get some regressions in python
related testing,

FAIL: gdb.python/py-explore-cc.exp: explore int_ptr_ref (timeout)
FAIL: gdb.python/py-explore-cc.exp: explore b (timeout)
FAIL: gdb.python/py-explore-cc.exp: explore B (timeout)

FAIL: gdb.python/py-explore.exp: explore ss_ptr (timeout)
FAIL: gdb.python/py-explore.exp: explore darray_ref (timeout)
FAIL: gdb.python/py-explore.exp: explore su (timeout)
FAIL: gdb.python/py-explore.exp: explore cs (timeout)
FAIL: gdb.python/py-explore.exp: explore cu (timeout)

however, I don't triage these fails yet.
> 
> You see, the way your patch works, using GDB from a Cygwin shell
> window will always do the above, even when not running the test
> suite.  Users might be unhappy that their standard output suddenly
> becomes much less efficient.

Yes, I get your point.  GDB would be less efficient on output.  How
about this patch? still unbuffering stdout and stderr, due to these
regression?

-- 
Yao (齐尧)

gdb:

2013-08-01  Joseph Myers  <joseph@codesourcery.com>
	    Yao Qi  <yao@codesourcery.com>

	* main.c (captured_main) [__MINGW32__]: Set stdout
	and stderr unbuffered if GDB is using Cygwin pty.
---
 gdb/main.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/gdb/main.c b/gdb/main.c
index 677f587..3a2fee6 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -375,6 +375,18 @@ captured_main (void *data)
   saved_command_line[0] = '\0';
   instream = stdin;
 
+#ifdef __MINGW32__
+  if (using_cygwin_pty ())
+    {
+      /* A Cygwin session may not look like a terminal to the Windows
+	 runtime; ensure stdout and stderr is unbuffered.  Note that
+	 setvbuf may be used after the file is opened but before any
+	 other operation is performed.  */
+      setvbuf (stdout, NULL, _IONBF, BUFSIZ);
+      setvbuf (stderr, NULL, _IONBF, BUFSIZ);
+    }
+#endif
+
   gdb_stdout = stdio_fileopen (stdout);
   gdb_stderr = stdio_fileopen (stderr);
   gdb_stdlog = gdb_stderr;	/* for moment */
-- 
1.7.7.6

Re: [PATCH 3/3] Set stdin/stdout/stderr to binary mode in cygwin.

[Yao Qi]

On 07/29/2013 11:44 PM, Eli Zaretskii wrote:
>> +      setmode (fileno (stdin), O_BINARY);
>> >+      setmode (fileno (stdout), O_BINARY);
>> >+      setmode (fileno (stderr), O_BINARY);
> Do you really need all 3 of the standard handles in binary mode?  I
> thought the problem was only with output?

Setting stdin in binary mode is for another purpose, should go to a
separated patch.  Sorry for the confusion.  Patch below is to only set
stdout and stderr in binary mode.

-- 
Yao (齐尧)

gdb:

2013-08-01  Pedro Alves  <pedro@codesourcery.com>
	    Daniel Jacobowitz  <dan@codesourcery.com>
	    Yao Qi  <yao@codesourcery.com>

	* main.c [__MINGW32__]: Include fcntl.h and windows.h.
	(captured_main) [__MINGW32__]: Set stdout and stderr to
	binary mode if GDB is using cygwin pty.
---
 gdb/main.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/gdb/main.c b/gdb/main.c
index 3a2fee6..6a79df9 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -47,6 +47,11 @@
 #include "filenames.h"
 #include "filestuff.h"
 
+#ifdef __MINGW32__
+#include <fcntl.h>
+#include <windows.h>
+#endif
+
 /* The selected interpreter.  This will be used as a set command
    variable, so it should always be malloc'ed - since
    do_setshow_command will free it.  */
@@ -384,6 +389,13 @@ captured_main (void *data)
 	 other operation is performed.  */
       setvbuf (stdout, NULL, _IONBF, BUFSIZ);
       setvbuf (stderr, NULL, _IONBF, BUFSIZ);
+
+      /* In textmode, a '\n' is automatically expanded into "\r\n".  This
+	 results in expect seeing "\r\r\n".  The tests aren't prepared
+	 currently for other forms of eol.  As a workaround, we force the
+	 output to binary mode.  */
+      setmode (fileno (stdout), O_BINARY);
+      setmode (fileno (stderr), O_BINARY);
     }
 #endif
 
-- 
1.7.7.6

Re: [PATCH 1/3] Detect GDB is in cygwin

[Eli Zaretskii]

> Date: Thu, 1 Aug 2013 15:51:23 +0800
> From: Yao Qi <yao@codesourcery.com>
> CC: <gdb-patches@sourceware.org>
> 
> On 07/30/2013 11:33 PM, Eli Zaretskii wrote:
> > There's no need to probe that at configure time: this code can always
> > be compiled on any MS-Windows system.  The compiler doesn't care and
> > doesn't check at compile time whether there is in fact
> > NtQueryInformationFile function inside the DLL.
> 
> OK.  If so, in configure.ac, we only need to check headers, and don't need
> AC_TRY_COMPILE at all.  Here is the updated one, with several changes,
> 
>  - Remove AC_TRY_COMPILE,
>  - Include fewer headers in mingw-htep.c, and don't have to check them
> in configure.ac.
>  - Rename is_in_cygwin to using_cygwin_pty,

Thanks.  I have just one more request: can you verify that GDB will
not think it runs in a Cygwin pty when it is run from an MSYS Bash
window?  (Since MSYS is a fork of an old version of Cygwin, it's
possible that it uses the same template for however it sets up the
shell window.)

Re: [PATCH 2/3] Unbuffer stdout and stderr in cygwin

[Eli Zaretskii]

> Date: Thu, 1 Aug 2013 16:05:26 +0800
> From: Yao Qi <yao@codesourcery.com>
> CC: <gdb-patches@sourceware.org>
> 
> On 07/29/2013 11:42 PM, Eli Zaretskii wrote:
> > I would suggest to call the function using_cygwin_pty or some such.
> > "is_in_cygwin" is IMO too ambiguous.
> > 
> 
> OK, fixed in patch 1/3.

Thanks.

> Yes, I get your point.  GDB would be less efficient on output.  How
> about this patch? still unbuffering stdout and stderr, due to these
> regression?

Apart of the change in the function's name, nothing else changed,
right?

Re: [PATCH 3/3] Set stdin/stdout/stderr to binary mode in cygwin.

[Eli Zaretskii]

> Date: Thu, 1 Aug 2013 16:09:39 +0800
> From: Yao Qi <yao@codesourcery.com>
> CC: <gdb-patches@sourceware.org>
> 
> On 07/29/2013 11:44 PM, Eli Zaretskii wrote:
> >> +      setmode (fileno (stdin), O_BINARY);
> >> >+      setmode (fileno (stdout), O_BINARY);
> >> >+      setmode (fileno (stderr), O_BINARY);
> > Do you really need all 3 of the standard handles in binary mode?  I
> > thought the problem was only with output?
> 
> Setting stdin in binary mode is for another purpose, should go to a
> separated patch.  Sorry for the confusion.  Patch below is to only set
> stdout and stderr in binary mode.

Thanks.

Re: [PATCH 2/3] Unbuffer stdout and stderr in cygwin

[Yao Qi]

On 08/02/2013 12:36 AM, Eli Zaretskii wrote:
> Apart of the change in the function's name, nothing else changed,
> right?

Right.

-- 
Yao (齐尧)

Re: [PATCH 1/3] Detect GDB is in cygwin

[Yao Qi]

On 08/02/2013 12:33 AM, Eli Zaretskii wrote:
> Thanks.  I have just one more request: can you verify that GDB will
> not think it runs in a Cygwin pty when it is run from an MSYS Bash
> window?  (Since MSYS is a fork of an old version of Cygwin, it's
> possible that it uses the same template for however it sets up the
> shell window.)

I run GDB in MSYS bash, and GDB thinks it runs in a Cygwin pty
unfortunately.  As you said, the file name template is the same.  I
add the following changes in using_cygwin_pty to differentiate MSYS
and Cygwin.

  const char *msystem = getenv ("MSYSTEM");

  /* Return false if environment variable "MSYSTEM" is set, because
     the code below can't tell GDB runs from MSYS or Cygwin.  GDB
     shouldn't think it runs in a Cygwin pty when it actually runs
     from MSYS bash.  */
  if (msystem != NULL)
    return 0;

the rest of the patch is unchanged.

-- 
Yao (齐尧)

gdb:

2013-08-02  Yao Qi  <yao@codesourcery.com>
	    Corinna Vinschen  <vinschen@redhat.com>

	* configure.ac: Invoke AC_CHECK_HEADERS to check winternl.h,
	and ddk/ntddk.h.
	* config.in: Re-generated.
	* configure: Re-generated.
	* defs.h [__MINGW32__] (using_cygwin_pty): Declare.
	* mingw-hdep.c: Inlcude wchar.h.
	[HAVE_WINTERNL_H]: Include winternl.h.  Define
	USE_NTQUERYINFORMATIONFILE.
	[!HAVE_WINTERNL_H] [HAVE_DDK_NTDDK_H]: Include ddk/ntddk.h.
	Define USE_NTQUERYINFORMATIONFILE..
	[USE_NTQUERYINFORMATIONFILE] (get_filename_from_handle): New.
	(using_cygwin_pty): New.
---
 gdb/config.in    |    6 +++
 gdb/configure    |   30 +++++++++++++++++
 gdb/configure.ac |    7 ++++
 gdb/defs.h       |    4 ++
 gdb/mingw-hdep.c |   96 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 143 insertions(+), 0 deletions(-)

diff --git a/gdb/config.in b/gdb/config.in
index 92c2789..010146c 100644
--- a/gdb/config.in
+++ b/gdb/config.in
@@ -84,6 +84,9 @@
 /* Define to 1 if you have the <curses.h> header file. */
 #undef HAVE_CURSES_H
 
+/* Define to 1 if you have the <ddk/ntddk.h> header file. */
+#undef HAVE_DDK_NTDDK_H
+
 /* Define to 1 if you have the declaration of `ADDR_NO_RANDOMIZE', and to 0 if
    you don't. */
 #undef HAVE_DECL_ADDR_NO_RANDOMIZE
@@ -572,6 +575,9 @@
 /* Define to 1 if you have the `wborder' function. */
 #undef HAVE_WBORDER
 
+/* Define to 1 if you have the <winternl.h> header file. */
+#undef HAVE_WINTERNL_H
+
 /* Define to 1 if `fork' works. */
 #undef HAVE_WORKING_FORK
 
diff --git a/gdb/configure b/gdb/configure
index 4833297..ecfa203 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -9030,6 +9030,36 @@ fi
 done
 
 
+# Check header winternl.h, if not found, check ddk/ntddk.h.
+case "${host}" in
+  *-*-mingw*) for ac_header in winternl.h
+do :
+  ac_fn_c_check_header_mongrel "$LINENO" "winternl.h" "ac_cv_header_winternl_h" "$ac_includes_default"
+if test "x$ac_cv_header_winternl_h" = x""yes; then :
+  cat >>confdefs.h <<_ACEOF
+#define HAVE_WINTERNL_H 1
+_ACEOF
+
+else
+  for ac_header in ddk/ntddk.h
+do :
+  ac_fn_c_check_header_mongrel "$LINENO" "ddk/ntddk.h" "ac_cv_header_ddk_ntddk_h" "$ac_includes_default"
+if test "x$ac_cv_header_ddk_ntddk_h" = x""yes; then :
+  cat >>confdefs.h <<_ACEOF
+#define HAVE_DDK_NTDDK_H 1
+_ACEOF
+
+fi
+
+done
+
+fi
+
+done
+
+		;;
+esac
+
 # ------------------------- #
 # Checks for declarations.  #
 # ------------------------- #
diff --git a/gdb/configure.ac b/gdb/configure.ac
index 48f37c8..c3df68e 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -1121,6 +1121,13 @@ AC_CHECK_HEADERS(term.h, [], [],
 #endif
 ])
 
+# Check header winternl.h, if not found, check ddk/ntddk.h.
+case "${host}" in
+  *-*-mingw*) AC_CHECK_HEADERS(winternl.h, [],
+  	      	[AC_CHECK_HEADERS(ddk/ntddk.h, [], [],[])], [])
+		;;
+esac
+
 # ------------------------- #
 # Checks for declarations.  #
 # ------------------------- #
diff --git a/gdb/defs.h b/gdb/defs.h
index 014d7d4..71c1c30 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -790,6 +790,10 @@ enum block_enum
   FIRST_LOCAL_BLOCK = 2
 };
 
+#ifdef __MINGW32__
+int using_cygwin_pty (void);
+#endif
+
 #include "utils.h"
 
 #endif /* #ifndef DEFS_H */
diff --git a/gdb/mingw-hdep.c b/gdb/mingw-hdep.c
index efc9848..965bda2 100644
--- a/gdb/mingw-hdep.c
+++ b/gdb/mingw-hdep.c
@@ -28,6 +28,19 @@
 #include "readline/readline.h"
 
 #include <windows.h>
+#include <wchar.h>
+
+#ifdef HAVE_WINTERNL_H
+#include <winternl.h>
+#define USE_NTQUERYINFORMATIONFILE 1
+#else
+
+#ifdef HAVE_DDK_NTDDK_H
+#include <ddk/ntddk.h>
+#define USE_NTQUERYINFORMATIONFILE 1
+#endif /*HAVE_DDK_NTDDK_H */
+
+#endif /* HAVE_WINTERNL_H */
 
 /* This event is signalled whenever an asynchronous SIGINT handler
    needs to perform an action in the main thread.  */
@@ -265,6 +278,89 @@ gdb_call_async_signal_handler (struct async_signal_handler *handler,
   SetEvent (sigint_event);
 }
 
+#ifdef USE_NTQUERYINFORMATIONFILE
+
+/* Return the file name of handle FH.  */
+
+static PWCHAR
+get_filename_from_handle (HANDLE fh)
+{
+  IO_STATUS_BLOCK io;
+  NTSTATUS status;
+  long buf[66];	/* NAME_MAX + 1 + sizeof ULONG */
+  PFILE_NAME_INFORMATION pfni = (PFILE_NAME_INFORMATION) buf;
+  static NTSTATUS (NTAPI *pNtQueryInformationFile) (HANDLE,
+						    PIO_STATUS_BLOCK,
+						    PVOID, ULONG,
+						    FILE_INFORMATION_CLASS);
+
+  /* Calling the native NT function NtQueryInformationFile is required to
+     support pre-Vista systems.  If that's of no concern, Vista introduced
+     the GetFileInformationByHandleEx call with the FileNameInfo info class,
+     which can be used instead. */
+  if (!pNtQueryInformationFile)
+    {
+      pNtQueryInformationFile = (NTSTATUS (NTAPI *)(HANDLE, PIO_STATUS_BLOCK,
+						    PVOID, ULONG, FILE_INFORMATION_CLASS))
+	GetProcAddress (GetModuleHandle ("ntdll.dll"),
+			"NtQueryInformationFile");
+      if (pNtQueryInformationFile == NULL)
+	return NULL;
+    }
+  if (!NT_SUCCESS (pNtQueryInformationFile (fh, &io, pfni, sizeof buf,
+					   FileNameInformation)))
+    return NULL;
+
+  /* The filename is not guaranteed to be NUL-terminated. */
+  pfni->FileName[pfni->FileNameLength / sizeof (WCHAR)] = L'\0';
+
+  return pfni->FileName;
+}
+
+#endif /* USE_NTQUERYINFORMATIONFILE */
+
+/* Return true if GDB is running in Cygwin pseudo-tty.  */
+
+int
+using_cygwin_pty (void)
+{
+  PWCHAR cp;
+  /* Now fetch the underlying HANDLE of stdin. */
+  HANDLE fh = (HANDLE) _get_osfhandle (fileno (stdin));
+  const char *msystem = getenv ("MSYSTEM");
+
+  if (!fh || fh == INVALID_HANDLE_VALUE)
+    return 0;
+
+  /* Return false if environment variable "MSYSTEM" is set, because
+     the code below can't tell GDB runs from MSYS or Cygwin.  GDB
+     shouldn't think it runs in a Cygwin pty when it actually runs
+     from MSYS bash.  */
+  if (msystem != NULL)
+    return 0;
+
+#ifdef USE_NTQUERYINFORMATIONFILE
+  cp = get_filename_from_handle (fh);
+#else
+  cp = NULL;
+#endif
+
+  /* Now check the name pattern.  With pseudo-tty allocated in ssh,
+     the filename of handle of stdin looks like this:
+
+       \cygwin-c5e39b7a9d22bafb-{p,t}ty1-from-master
+
+     Without pseudo-tty allocated in ssh, the filename of handle of
+     stdin looks like this:
+
+       \cygwin-c5e39b7a9d22bafb-pipe-0x14C8-0x3
+
+     If the file name is prefixed with "\cygwin-", GDB is running in
+     cygwin.  */
+
+  return (cp != NULL && wcsncmp (cp, L"\\cygwin-", 8) == 0);
+}
+
 /* -Wmissing-prototypes */
 extern initialize_file_ftype _initialize_mingw_hdep;
 
-- 
1.7.7.6

Re: [PATCH 1/3] Detect GDB is in cygwin

[Eli Zaretskii]

> Date: Fri, 2 Aug 2013 10:51:00 +0800
> From: Yao Qi <yao@codesourcery.com>
> CC: <gdb-patches@sourceware.org>
> 
> On 08/02/2013 12:33 AM, Eli Zaretskii wrote:
> > Thanks.  I have just one more request: can you verify that GDB will
> > not think it runs in a Cygwin pty when it is run from an MSYS Bash
> > window?  (Since MSYS is a fork of an old version of Cygwin, it's
> > possible that it uses the same template for however it sets up the
> > shell window.)
> 
> I run GDB in MSYS bash, and GDB thinks it runs in a Cygwin pty
> unfortunately.  As you said, the file name template is the same.  I
> add the following changes in using_cygwin_pty to differentiate MSYS
> and Cygwin.
> 
>   const char *msystem = getenv ("MSYSTEM");

Thanks.  I have no further comments to this patch.

Re: [PATCH 1/3] Detect GDB is in cygwin

[Christopher Faylor]

On Thu, Aug 01, 2013 at 03:51:23PM +0800, Yao Qi wrote:
>On 07/30/2013 11:33 PM, Eli Zaretskii wrote:
>> There's no need to probe that at configure time: this code can always
>> be compiled on any MS-Windows system.  The compiler doesn't care and
>> doesn't check at compile time whether there is in fact
>> NtQueryInformationFile function inside the DLL.
>
>OK.  If so, in configure.ac, we only need to check headers, and don't need
>AC_TRY_COMPILE at all.  Here is the updated one, with several changes,
>
> - Remove AC_TRY_COMPILE,
> - Include fewer headers in mingw-htep.c, and don't have to check them
>in configure.ac.
> - Rename is_in_cygwin to using_cygwin_pty,

This patch relies on low-level, unadvertised properties of the Cygwin DLL.
Are you sure it doesn't detect a cygwin pipe as a pty?

cgf

Re: [PATCH 1/3] Detect GDB is in cygwin

[Yao Qi]

On 08/03/2013 12:54 PM, Christopher Faylor wrote:
> This patch relies on low-level, unadvertised properties of the Cygwin DLL.
> Are you sure it doesn't detect a cygwin pipe as a pty?

Hi,
I am not sure, but we never detect a cygwin pipe here.  In
using_cygwin_pty, we only check the file name of handle of stdin, to
see its name match the template or not.  We don't have to worry about
it.

-- 
Yao (齐尧)

Re: [PATCH 1/3] Detect GDB is in cygwin

[Christopher Faylor]

On Sun, Aug 04, 2013 at 04:45:07PM +0800, Yao Qi wrote:
>On 08/03/2013 12:54 PM, Christopher Faylor wrote:
>> This patch relies on low-level, unadvertised properties of the Cygwin DLL.
>> Are you sure it doesn't detect a cygwin pipe as a pty?
>
>Hi,
>I am not sure, but we never detect a cygwin pipe here.  In
>using_cygwin_pty, we only check the file name of handle of stdin, to
>see its name match the template or not.  We don't have to worry about
>it.

Why can't the handle be a pipe?  They also begin with "cygwin-".

Re: [PATCH 1/3] Detect GDB is in cygwin

[Yao Qi]

On 08/05/2013 12:41 PM, Christopher Faylor wrote:
>> I am not sure, but we never detect a cygwin pipe here.  In
>> >using_cygwin_pty, we only check the file name of handle of stdin, to
>> >see its name match the template or not.  We don't have to worry about
>> >it.
> Why can't the handle be a pipe?  They also begin with "cygwin-".

I don't understand your problem.  Can you elaborate? or point out the 
problem in the patch explicitly.

-- 
Yao (齐尧)

Re: [PATCH 1/3] Detect GDB is in cygwin

[Christopher Faylor]

On Mon, Aug 05, 2013 at 02:21:55PM +0800, Yao Qi wrote:
>On 08/05/2013 12:41 PM, Christopher Faylor wrote:
>>> I am not sure, but we never detect a cygwin pipe here.  In
>>> >using_cygwin_pty, we only check the file name of handle of stdin, to
>>> >see its name match the template or not.  We don't have to worry about
>>> >it.
>> Why can't the handle be a pipe?  They also begin with "cygwin-".
>
>I don't understand your problem.  Can you elaborate? or point out the 
>problem in the patch explicitly.

I'm saying that it looks like your code will detect "echo yes | gdb"
as running on a cygwin pty.

Re: [PATCH 1/3] Detect GDB is in cygwin

[Yao Qi]

On 08/06/2013 10:08 AM, Christopher Faylor wrote:
> I'm saying that it looks like your code will detect "echo yes | gdb"
> as running on a cygwin pty.

In this case, the file name of handle looks like 
"\cygwin-c5e39b7a9d22bafb-pipe-0xBC0-0x1".  It is expected to return 
true in this case too, that is to say, we need to set stdout/stderr 
unbuffered in this case too.

In my test configuration, I use two "methods" to access remote Cygwin 
respectively, one is "ssh" and the other is "ssh -t".  We need these 
changes on stdout/stderr for these two "methods".

See the code in my patch ...

> +  /* Now check the name pattern.  With pseudo-tty allocated in ssh,
> +     the filename of handle of stdin looks like this:
> +
> +       \cygwin-c5e39b7a9d22bafb-{p,t}ty1-from-master

This is the name pattern I get if I "ssh -t" to remote Cygwin.

> +
> +     Without pseudo-tty allocated in ssh, the filename of handle of
> +     stdin looks like this:
> +
> +       \cygwin-c5e39b7a9d22bafb-pipe-0x14C8-0x3

This is the name patter I get if I "ssh" to remtoe Cygwin.

> +
> +     If the file name is prefixed with "\cygwin-", GDB is running in
> +     cygwin.  */
> +
> +  return (cp != NULL && wcsncmp (cp, L"\\cygwin-", 8) == 0);

We really need to return true for these two cases.  I find it is 
confusing to say "using pty" or "not using pty" in the comments, so I 
choose "with pty allocated" or "without pty allocated", which is much 
clear to me.

> +/* Return true if GDB is running in Cygwin pseudo-tty.  */
> +
> +int
> +using_cygwin_pty (void)

Probably you are confused by the function name and the comments.  Does 
it below help?

/* Return true if GDB is running in Cygwin.  */

int
using_cygwin (void)

-- 
Yao (齐尧)

Re: [PATCH 1/3] Detect GDB is in cygwin

[Christopher Faylor]

On Tue, Aug 06, 2013 at 11:04:43AM +0800, Yao Qi wrote:
>On 08/06/2013 10:08 AM, Christopher Faylor wrote:
>> I'm saying that it looks like your code will detect "echo yes | gdb"
>> as running on a cygwin pty.
>
>In this case, the file name of handle looks like 
>"\cygwin-c5e39b7a9d22bafb-pipe-0xBC0-0x1".  It is expected to return 
>true in this case too, that is to say, we need to set stdout/stderr 
>unbuffered in this case too.

If you're just going to always set to unbuffered when something
is a pipe, why not just check for a pipe using GetFileType?  Then
you don't have to try to use an undocumented Cygwin behaviour.

cgf

Re: [PATCH 1/3] Detect GDB is in cygwin

[Yao Qi]

On 08/08/2013 01:11 PM, Christopher Faylor wrote:
> If you're just going to always set to unbuffered when something
> is a pipe, why not just check for a pipe using GetFileType?  Then
> you don't have to try to use an undocumented Cygwin behaviour.

What I am going to do is to set stdout/stderr unbuffered if we can
detect that GDB is running in cygwin, with tty allocated or without tty
allocated.  We'd like to restrict this behaviour change only when
mingw gdb is running in cygwin.  We don't want to change the behaviour
on native windows, so we have to rely on this Cygwin behaviour.

-- 
Yao (齐尧)

RE: [PATCH 1/3] Detect GDB is in cygwin

[Pierre Muller]

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Christopher Faylor
> Envoyé : jeudi 8 août 2013 07:11
> À : gdb-patches@sourceware.org; Yao Qi
> Objet : Re: [PATCH 1/3] Detect GDB is in cygwin
> 
> On Tue, Aug 06, 2013 at 11:04:43AM +0800, Yao Qi wrote:
> >On 08/06/2013 10:08 AM, Christopher Faylor wrote:
> >> I'm saying that it looks like your code will detect "echo yes | gdb"
> >> as running on a cygwin pty.
> >
> >In this case, the file name of handle looks like
> >"\cygwin-c5e39b7a9d22bafb-pipe-0xBC0-0x1".  It is expected to return
> >true in this case too, that is to say, we need to set stdout/stderr
> >unbuffered in this case too.
> 
> If you're just going to always set to unbuffered when something
> is a pipe, why not just check for a pipe using GetFileType?  Then
> you don't have to try to use an undocumented Cygwin behaviour.

  But this would mean that this patch will also modify
interactive GDB runs, because with mintty terminal, you get a
pipe for the pty even when running gdb interactively
(I added some code to display the file type of stdin 
and the name of the pipe if the call to NtQueryFileInformation succeeds for
that).

  Without mintty (running Cygwin bash directly), I do get a FILE_TYPE_CHAR,
and a second check by calling GetConsoleMode with the same handle
allows to verify that it is indeed a Windows OS Console handle.

  I think that there is indeed no good way to know if we are using
a non-interactive pipe...
  As the primary purpose of the patch was to allow better results for the
testsuite
for mingw builds, I think that the idea
of adding a
 "maint set testsuite-mode on/off"
command that could be
automatically added to 
INTERNAL_GDBFLAGS as "-iex {maint set testsuite-mode on}"
would be a simpler approach.
  It would guaranty that we do not change existing behavior for
interactive GDB use and should solve the problem about ^M^M patterns that
lead to lots of failures when testing mingw builds.
  As I explained earlier, this changes are also required if you want
to run the testsuite on msys terminal.

  Should I prepare a RFC? 

  One question about this new command that I had in mind
was about the scope of this command.
  Should I put it inside mingw-hdep.c code and restrict it to
mingw builds, or should I introduce that command globally,
the posix-tdep.c version would be an empty function for now,
but could be useful (?) later.
  The advantage of the second approach is that we could add
the "-iex {maint set testsuite-mode on}" unconditionally in gdb.exp,
but that might seem like a weak argument...



Pierre Muller

If someone wants to test the code, here is a diff,
based on Yao's patch, plus file type information printing (this would be
completely removed
in my RFC), this part using printf directly because uifile are not yet
set up when using_cygwin_pty is called 
 
and the 'maint show/set testsuite-mode command', here restricted to the
mingw builds.


$ cvs diff -u -p mingw-hdep.c
Index: mingw-hdep.c
===================================================================
RCS file: /cvs/src/src/gdb/mingw-hdep.c,v
retrieving revision 1.16
diff -u -p -r1.16 mingw-hdep.c
--- mingw-hdep.c        6 Apr 2013 06:52:06 -0000       1.16
+++ mingw-hdep.c        8 Aug 2013 07:22:04 -0000
@@ -21,6 +21,8 @@
 #include "main.h"
 #include "serial.h"
 #include "event-loop.h"
+#include "command.h"
+#include "gdbcmd.h"

 #include "gdb_assert.h"
 #include "gdb_select.h"
@@ -28,6 +30,21 @@
 #include "readline/readline.h"

 #include <windows.h>
+#include <wchar.h>
+
+#ifdef HAVE_WINTERNL_H
+#include <winternl.h>
+#include <ntstatus.h>
+#define USE_NTQUERYINFORMATIONFILE 1
+#else
+
+#ifdef HAVE_DDK_NTDDK_H
+#include <ddk/ntddk.h>
+#include <ddk/ntstatus.h>
+#define USE_NTQUERYINFORMATIONFILE 1
+#endif /*HAVE_DDK_NTDDK_H */
+
+#endif /* HAVE_WINTERNL_H */

 /* This event is signalled whenever an asynchronous SIGINT handler
    needs to perform an action in the main thread.  */
@@ -265,6 +282,182 @@ gdb_call_async_signal_handler (struct as
   SetEvent (sigint_event);
 }

+#ifdef USE_NTQUERYINFORMATIONFILE
+
+/* Return the file name of handle FH.  */
+
+static PWCHAR
+get_filename_from_handle (HANDLE fh, int *len)
+{
+  IO_STATUS_BLOCK io;
+  NTSTATUS status;
+  char *st;
+  DWORD filetype;
+  long buf[66];        /* NAME_MAX + 1 + sizeof ULONG */
+  PFILE_NAME_INFORMATION pfni = (PFILE_NAME_INFORMATION) buf;
+  static NTSTATUS (NTAPI *pNtQueryInformationFile) (HANDLE,
+                                                   PIO_STATUS_BLOCK,
+                                                   PVOID, ULONG,
+                                                   FILE_INFORMATION_CLASS);
+
+  /* Calling the native NT function NtQueryInformationFile is required to
+     support pre-Vista systems.  If that's of no concern, Vista introduced
+     the GetFileInformationByHandleEx call with the FileNameInfo info
class,
+     which can be used instead. */
+  if (!pNtQueryInformationFile)
+    {
+      pNtQueryInformationFile = (NTSTATUS (NTAPI *)(HANDLE,
PIO_STATUS_BLOCK,
+                                                   PVOID, ULONG,
FILE_INFORMATION_CLASS))
+       GetProcAddress (GetModuleHandle ("ntdll.dll"),
+                       "NtQueryInformationFile");
+      if (pNtQueryInformationFile == NULL)
+       return NULL;
+    }
+  filetype = GetFileType (fh);
+  if (filetype == FILE_TYPE_CHAR)
+    {
+      DWORD info;
+      if (GetConsoleMode (fh, &info))
+       st = "Console";
+      else
+       st = "FILE_TYPE_CHAR";
+    }
+  else if (filetype == FILE_TYPE_PIPE)
+    st = "FILE_TYPE_PIPE";
+  else if (filetype == FILE_TYPE_DISK)
+    st = "FILE_TYPE_DISK";
+  else
+    st = "Unknown file type";
+
+  fprintf (stdout, "File type is %s\n", st);
+
+  status = pNtQueryInformationFile (fh, &io, pfni, sizeof buf,
+                                   FileNameInformation);
+  if (!NT_SUCCESS (status))
+    {
+      if (status ==  STATUS_OBJECT_TYPE_MISMATCH)
+       fprintf(stdout, "NtQueryInformationFile returned "
+                        "STATUS_OBJECT_TYPE_MISMATCH error\n");
+      else if (status == STATUS_INVALID_HANDLE)
+       fprintf(stdout, "NtQueryInformationFile returned "
+                        "STATUS_INVALID_HANDLE error\n");
+      else
+        fprintf (stdout, "NtQueryInformationFile returned 0x%lx\n",
+                        (unsigned long int) status);
+      return NULL;
+    }
+
+  /* The filename is not guaranteed to be NUL-terminated. */
+  pfni->FileName[pfni->FileNameLength / sizeof (WCHAR)] = L'\0';
+  *len = pfni->FileNameLength;
+  return pfni->FileName;
+}
+
+#endif /* USE_NTQUERYINFORMATIONFILE */
+
+/* Return true if GDB is running in Cygwin pseudo-tty.  */
+
+int
+using_cygwin_pty (void)
+{
+  int len, res;
+  PWCHAR cp;
+  static char buf[(2 * PATH_MAX) + 2];
+  /* Now fetch the underlying HANDLE of stdin. */
+  HANDLE fh = (HANDLE) _get_osfhandle (fileno (stdin));
+  const char *msystem = getenv ("MSYSTEM");
+
+  if (!fh || fh == INVALID_HANDLE_VALUE)
+    return 0;
+
+  /* Return false if environment variable "MSYSTEM" is set, because
+     the code below can't tell GDB runs from MSYS or Cygwin.  GDB
+     shouldn't think it runs in a Cygwin pty when it actually runs
+     from MSYS bash.  */
+//  if (msystem != NULL)
+//    return 0;
+
+#ifdef USE_NTQUERYINFORMATIONFILE
+  cp = get_filename_from_handle (fh, &len);
+#else
+  cp = NULL;
+  len = 0;
+#endif
+
+  /* Now check the name pattern.  With pseudo-tty allocated in ssh,
+     the filename of handle of stdin looks like this:
+
+       \cygwin-c5e39b7a9d22bafb-{p,t}ty1-from-master
+
+     Without pseudo-tty allocated in ssh, the filename of handle of
+     stdin looks like this:
+
+       \cygwin-c5e39b7a9d22bafb-pipe-0x14C8-0x3
+
+     If the file name is prefixed with "\cygwin-", GDB is running in
+     cygwin.  */
+  WideCharToMultiByte (CP_ACP, 0, cp, len, buf, sizeof buf,
+                      0, 0);
+  fprintf (stdout, "File name returned \"%s\"\n", buf);
+
+  res = (cp != NULL && wcsncmp (cp, L"\\cygwin-", 8) == 0);
+  return res;
+}
+
+/* Set stdout and stderr handles to binary unbuffered mode.  */
+
+void
+set_output_binary_unbuffered (void)
+{
+  /* A Cygwin session may not look like a terminal to the Windows
+     runtime; ensure stdout and stderr is unbuffered.  Note that
+     setvbuf may be used after the file is opened but before any
+     other operation is performed.  */
+  setvbuf (stdout, NULL, _IONBF, BUFSIZ);
+  setvbuf (stderr, NULL, _IONBF, BUFSIZ);
+
+  /* In textmode, a '\n' is automatically expanded into "\r\n".  This
+     results in expect seeing "\r\r\n".  The tests aren't prepared
+     currently for other forms of eol.  As a workaround, we force the
+     output to binary mode.  */
+  setmode (fileno (stdout), O_BINARY);
+  setmode (fileno (stderr), O_BINARY);
+}
+
+/* Restore stdout and stderr handles to "normal" mode.  */
+
+static void
+set_output_normal_mode (void)
+{
+  setvbuf (stdout, NULL, _IOLBF, BUFSIZ);
+  setvbuf (stderr, NULL, _IOLBF, BUFSIZ);
+
+  setmode (fileno (stdout), O_TEXT);
+  setmode (fileno (stderr), O_TEXT);
+}
+
+static int maint_testsuite_mode = 0;
+
+/* Sets the maintenance testsuite mode using the static global
+   testuite_mode.  */
+
+static void
+set_maint_testsuite_mode (char *args, int from_tty,
+                              struct cmd_list_element *c)
+{
+  if (maint_testsuite_mode)
+    set_output_binary_unbuffered ();
+  else
+    set_output_normal_mode ();
+}
+
+static void
+show_maint_testsuite_mode (struct ui_file *file, int from_tty,
+               struct cmd_list_element *c, const char *value)
+{
+  fprintf_filtered (file, _("Testsuite mode is %s.\n"), value);
+}
+
 /* -Wmissing-prototypes */
 extern initialize_file_ftype _initialize_mingw_hdep;

@@ -272,4 +465,16 @@ void
 _initialize_mingw_hdep (void)
 {
   sigint_event = CreateEvent (0, FALSE, FALSE, 0);
+  add_setshow_boolean_cmd ("testsuite-mode", class_maintenance,
+                          &maint_testsuite_mode, _("\
+Set to adapt to dejagnu testsuite runs."), _("\
+Show whether GDB is adapted to run testsuite."), _("\
+Use \"on\" to enable, \"off\" to disable.\n\
+If enabled, stdout and stderr are set to binary mode,\n\
+and their buffering is completely disabled to allow better testing."),
+                          set_maint_testsuite_mode,
+                          show_maint_testsuite_mode,
+                          &maintenance_set_cmdlist,
+                          &maintenance_show_cmdlist);
+
 } 

Re: [PATCH 1/3] Detect GDB is in cygwin

[Yao Qi]

On 08/08/2013 03:28 PM, Pierre Muller wrote:
>    Without mintty (running Cygwin bash directly), I do get a FILE_TYPE_CHAR,
> and a second check by calling GetConsoleMode with the same handle
> allows to verify that it is indeed a Windows OS Console handle.
>
>    I think that there is indeed no good way to know if we are using
> a non-interactive pipe...
>    As the primary purpose of the patch was to allow better results for the
> testsuite
> for mingw builds, I think that the idea
> of adding a
>   "maint set testsuite-mode on/off"
> command that could be
> automatically added to
> INTERNAL_GDBFLAGS as "-iex {maint set testsuite-mode on}"
> would be a simpler approach.
>    It would guaranty that we do not change existing behavior for
> interactive GDB use and should solve the problem about ^M^M patterns that
> lead to lots of failures when testing mingw builds.
>    As I explained earlier, this changes are also required if you want
> to run the testsuite on msys terminal.
>
>    Should I prepare a RFC?
>

Pierre,
Yes, I think so, but before you post it, we have to coordinate on this. 
  Two people (you and I) post the similar-but-different patches may 
confuse maintainers and doesn't help on review and approval on any
of them.  There are some issues related to mingw and cygwin, and we
(you and I) are working on some of them, with different focus.  If you
don't mind, I'll post my patches again, a fresh version, V4, and
hopefully maintainers can approve them this time.  In the review of V3,
all the questions were addressed, AFAIK, but no one approved it
explicitly.

Then, you can prepare your RFC, and I am happy to help (review and 
test).  Is it OK to you?

>    One question about this new command that I had in mind
> was about the scope of this command.
>    Should I put it inside mingw-hdep.c code and restrict it to
> mingw builds, or should I introduce that command globally,
> the posix-tdep.c version would be an empty function for now,
> but could be useful (?) later.

I prefer the former one.

>    The advantage of the second approach is that we could add
> the "-iex {maint set testsuite-mode on}" unconditionally in gdb.exp,
> but that might seem like a weak argument...

You can modify your board file or build_dir/gdb/testsuite/site.exp to 
append the option to GDBFLAGS.

-- 
Yao (齐尧)

RE: [PATCH 1/3] Detect GDB is in cygwin

[Pierre Muller]

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Yao Qi
> Envoyé : mardi 13 août 2013 10:11
> À : Pierre Muller
> Cc : gdb-patches@sourceware.org
> Objet : Re: [PATCH 1/3] Detect GDB is in cygwin
> 
> On 08/08/2013 03:28 PM, Pierre Muller wrote:
> >    Without mintty (running Cygwin bash directly), I do get a
> FILE_TYPE_CHAR,
> > and a second check by calling GetConsoleMode with the same handle
> > allows to verify that it is indeed a Windows OS Console handle.
> >
> >    I think that there is indeed no good way to know if we are using
> > a non-interactive pipe...
> >    As the primary purpose of the patch was to allow better results for the
> > testsuite
> > for mingw builds, I think that the idea
> > of adding a
> >   "maint set testsuite-mode on/off"
> > command that could be
> > automatically added to
> > INTERNAL_GDBFLAGS as "-iex {maint set testsuite-mode on}"
> > would be a simpler approach.
> >    It would guaranty that we do not change existing behavior for
> > interactive GDB use and should solve the problem about ^M^M patterns that
> > lead to lots of failures when testing mingw builds.
> >    As I explained earlier, this changes are also required if you want
> > to run the testsuite on msys terminal.
> >
> >    Should I prepare a RFC?
> >
> 
> Pierre,
> Yes, I think so, but before you post it, we have to coordinate on this.
>   Two people (you and I) post the similar-but-different patches may
> confuse maintainers and doesn't help on review and approval on any
> of them.  There are some issues related to mingw and cygwin, and we
> (you and I) are working on some of them, with different focus.  If you
> don't mind, I'll post my patches again, a fresh version, V4, and
> hopefully maintainers can approve them this time.  In the review of V3,
> all the questions were addressed, AFAIK, but no one approved it
> explicitly.
>
> Then, you can prepare your RFC, and I am happy to help (review and
> test).  Is it OK to you?

  OK, this is fine with me. 
> >    One question about this new command that I had in mind
> > was about the scope of this command.
> >    Should I put it inside mingw-hdep.c code and restrict it to
> > mingw builds, or should I introduce that command globally,
> > the posix-tdep.c version would be an empty function for now,
> > but could be useful (?) later.
> 
> I prefer the former one.

  
 
> >    The advantage of the second approach is that we could add
> > the "-iex {maint set testsuite-mode on}" unconditionally in gdb.exp,
> > but that might seem like a weak argument...
> 
> You can modify your board file or build_dir/gdb/testsuite/site.exp to
> append the option to GDBFLAGS.

   This is a tricky area for a newcomer to understand,
thus I would really like to get something that would
automatically use that new command.
  In the case we decide to restrict this command to GDB
executables built to run as mingw programs, I would like gdb.exp to detect the
host settings correctly and add "-iex {maint set testsuite-mode on}"
to INTERNAL_GDBFLAGS.

  But let's first finalize your patch.
Please send your V4, I will wait.

Pierre Muller

Re: [PATCH 1/3] Detect GDB is in cygwin

[Christopher Faylor]

On Thu, Aug 08, 2013 at 03:23:39PM +0800, Yao Qi wrote:
>On 08/08/2013 01:11 PM, Christopher Faylor wrote:
>> If you're just going to always set to unbuffered when something
>> is a pipe, why not just check for a pipe using GetFileType?  Then
>> you don't have to try to use an undocumented Cygwin behaviour.
>
>What I am going to do is to set stdout/stderr unbuffered if we can
>detect that GDB is running in cygwin, with tty allocated or without tty
>allocated.  We'd like to restrict this behaviour change only when
>mingw gdb is running in cygwin.  We don't want to change the behaviour
>on native windows, so we have to rely on this Cygwin behaviour.

You've already acknowledged that your code will decide to become
unbuffered whether you are running on a cygwin pipe or cygwin pty.  What
is special about cygwin pipes that makes you want to make them
unbuffered while ignoring normal Windows pipes?

Can you explain *why* you don't want to change the behavior on native
windows?

I really don't like having gdb rely on undocumented Cygwin behavior.
You're introducing a tenuous dependency between the way Cygwin creates
ptys and pipes which could easily break if we decide to change something
in Cygwin.

cgf

Re: [PATCH 1/3] Detect GDB is in cygwin

[Tom Tromey]

cgf> I really don't like having gdb rely on undocumented Cygwin behavior.
cgf> You're introducing a tenuous dependency between the way Cygwin creates
cgf> ptys and pipes which could easily break if we decide to change something
cgf> in Cygwin.

One other idea that comes to mind is implementing the buffering and even
line-ending transformations in gdb.  I think most output like this in
gdb winds up in stdio_file_write and stdio_file_fputs.

The idea is something like, for Windows hosts, put stdout and stderr
into "binary" mode.  Then, have those two functions implement line
buffering internally (again just for Windows hosts).  Finally, have them
also transform \n -> \r\n on output.

Would this work?

Tom

Re: [PATCH 1/3] Detect GDB is in cygwin

[Eli Zaretskii]

> From: Tom Tromey <tromey@redhat.com>
> Cc: Yao Qi <yao@codesourcery.com>
> Date: Thu, 15 Aug 2013 12:58:05 -0600
> 
> The idea is something like, for Windows hosts, put stdout and stderr
> into "binary" mode.  Then, have those two functions implement line
> buffering internally (again just for Windows hosts).  Finally, have them
> also transform \n -> \r\n on output.
> 
> Would this work?

How does this solve Yao's problem?  I thought the problem was that
\r\n got converted into \r\r\n by something outside of GDB, no?

Re: [PATCH 1/3] Detect GDB is in cygwin

[Yao Qi]

On 08/16/2013 03:15 AM, Eli Zaretskii wrote:
> How does this solve Yao's problem?  I thought the problem was that
> \r\n got converted into \r\r\n by something outside of GDB, no?

Right, that is the problem I want to solve.

-- 
Yao (齐尧)

Re: [PATCH 1/3] Detect GDB is in cygwin

[Yao Qi]

On 08/16/2013 01:40 AM, Christopher Faylor wrote:
> You've already acknowledged that your code will decide to become
> unbuffered whether you are running on a cygwin pipe or cygwin pty.  What
> is special about cygwin pipes that makes you want to make them
> unbuffered while ignoring normal Windows pipes?

In a cygwin session, I start gdb "./gdb", and stdin is a cygwin pty.  If 
I start gdb "echo yes | ./gdb", stdin is a cygwin pipe, right?

In a windows console (cmd.exe), if I start gdb "echo yes | ./gdb", stdin 
is a windows pipe.  Is it correct?

>
> Can you explain*why*  you don't want to change the behavior on native
> windows?

because the problem this patch series want to address is not related to
native windows, so we don't want to change its behaviour.  We think 
"buffered/unbuffered" are user visible behaviour, and we should be 
careful on changing them.

The original problem this patch series want to address is about testing 
mingw32 native gdb in cygwin.  This patch series can make 
dejagnu/testsuite happy, and improve the test result dramatically 
(without this patch series, the result is unusable at all).

If Pierre or some one else thinks we need changes here on native 
windows, that is fine, let us do it.  There are many issues for mingw, 
cygwin and windows, and it is impossible to fix all of them in one patch 
series.  This patch series make some progresses and Pierre's incoming 
RCF will make some progresses too, isn't it what we want?

>
> I really don't like having gdb rely on undocumented Cygwin behavior.
> You're introducing a tenuous dependency between the way Cygwin creates
> ptys and pipes which could easily break if we decide to change something
> in Cygwin.

GDB has been relying on undocumented behaviours here and there.
'grep "no.*document" *.c" may find some.  If the file name pattern of
ptys and pipes are changed some day, we can update the code here to
match, which is not difficult, IMO.

On the other hand, do you think it is hard to get this documented in
Cygwin? in order to improve the interoperability between Cygwin and GDB.

-- 
Yao (齐尧)

Re: [PATCH 1/3] Detect GDB is in cygwin

[Tom Tromey]

>>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:

Yao> On 08/16/2013 03:15 AM, Eli Zaretskii wrote:
>> How does this solve Yao's problem?  I thought the problem was that
>> \r\n got converted into \r\r\n by something outside of GDB, no?

Yao> Right, that is the problem I want to solve.

Ok, sorry about that.
I must have drastically misunderstood.

Tom

Re: [PATCH 1/3] Detect GDB is in cygwin

[Christopher Faylor]

On Fri, Aug 16, 2013 at 09:06:04AM +0800, Yao Qi wrote:
>On 08/16/2013 01:40 AM, Christopher Faylor wrote:
>> You've already acknowledged that your code will decide to become
>> unbuffered whether you are running on a cygwin pipe or cygwin pty.  What
>> is special about cygwin pipes that makes you want to make them
>> unbuffered while ignoring normal Windows pipes?
>
>In a cygwin session, I start gdb "./gdb", and stdin is a cygwin pty.  If 
>I start gdb "echo yes | ./gdb", stdin is a cygwin pipe, right?
>
>In a windows console (cmd.exe), if I start gdb "echo yes | ./gdb", stdin 
>is a windows pipe.  Is it correct?
>
>>
>> Can you explain*why*  you don't want to change the behavior on native
>> windows?
>
>because the problem this patch series want to address is not related to
>native windows, so we don't want to change its behaviour.  We think 
>"buffered/unbuffered" are user visible behaviour, and we should be 
>careful on changing them.

The "user visible behaviour" in this case would be running gdb with a
pipe for stdin/stdout.  It still isn't clear to me why it's acceptable
to change this behavior for Cygwin but not for other platforms.

The only reason I can see is that you are trying to run a testsuite over
ssh without using the -t option to ssh.  That causes ssh to use pipes.
There is nothing Cygwin-specific about this behaviour.  So, why wouldn't
a MinGW gdb need something similar for "putty"?  Or, why wouldn't a linux
version need this behavior in similar scenarios?

>On the other hand, do you think it is hard to get this documented in
>Cygwin? in order to improve the interoperability between Cygwin and GDB.

No, I'm not going to document this.  This isn't to improve
interoperability between Cygwin and GDB.  It is to to make running MinGW
gdb under cygwin's ptys and pipes work better.  The fact that you've
uncovered how Cygwin works under the hood doesn't mean that it is
something that I want to officially support.

cgf