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

[PATCH 3/3] Set 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 stdout/stderr to binary mode prevents that
expansion.

gdb:

2013-08-13  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 0174992..76be6ce 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

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

[Yao Qi]

Hi,
This patch is to set the buffering of stdout and stderr properly on
mingw32 host when it is running in cygwin pty, 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-08-13  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 1c240e4..0174992 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

[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:

2013-08-13  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 76abd04..3d4f1cc 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
@@ -575,6 +578,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 8067825..e2898d4 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 667821f..21105a4 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

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

[Yao Qi]

Hi,
Here is the V4 of this patch series, which fix problems we've seen on
running mingw32 native for testing cygwin.  V3 was reviewed in this thread 
http://sourceware.org/ml/gdb-patches/2013-07/msg00691.html by Eli mostly.
All the questions/comments were addressed, AFAIK.  However, no one
approved it explicitly.  I post them again, and call it V4, which is
identical to patches I finally posted in V3 thread.  It is sort of a
patch ping.

These patches was discussed some times (since 2009), and people think
they are useful, but unfortunately, they were not checked in since then.
There are some issues in GDB on Windows (including mingw32 and cygwin),
and Pierre has some other fixes too on top of mine.  The patch series
can be a good starting point.

Here is the description in V3, for people who don't have much context
of this series....

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

*** BLURB HERE ***

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

 gdb/config.in    |    6 +++
 gdb/configure    |   30 +++++++++++++++++
 gdb/configure.ac |    7 ++++
 gdb/defs.h       |    4 ++
 gdb/main.c       |   24 +++++++++++++
 gdb/mingw-hdep.c |   96 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 167 insertions(+), 0 deletions(-)

-- 
1.7.7.6

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

[Eli Zaretskii]

> From: Yao Qi <yao@codesourcery.com>
> Date: Tue, 13 Aug 2013 17:34:25 +0800
> 
> Here is the V4 of this patch series, which fix problems we've seen on
> running mingw32 native for testing cygwin.  V3 was reviewed in this thread 
> http://sourceware.org/ml/gdb-patches/2013-07/msg00691.html by Eli mostly.
> All the questions/comments were addressed, AFAIK.  However, no one
> approved it explicitly.  I post them again, and call it V4, which is
> identical to patches I finally posted in V3 thread.  It is sort of a
> patch ping.

I suggest to commit this, based on my review, if no one objects in a
few days.

Thanks.

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

[Christopher Faylor]

On Tue, Aug 13, 2013 at 07:00:32PM +0300, Eli Zaretskii wrote:
>> From: Yao Qi <yao@codesourcery.com>
>> Date: Tue, 13 Aug 2013 17:34:25 +0800
>> 
>> Here is the V4 of this patch series, which fix problems we've seen on
>> running mingw32 native for testing cygwin.  V3 was reviewed in this thread 
>> http://sourceware.org/ml/gdb-patches/2013-07/msg00691.html by Eli mostly.
>> All the questions/comments were addressed, AFAIK.  However, no one
>> approved it explicitly.  I post them again, and call it V4, which is
>> identical to patches I finally posted in V3 thread.  It is sort of a
>> patch ping.
>
>I suggest to commit this, based on my review, if no one objects in a
>few days.

I'll just register my objection to gdb using undocumented cygwin
internal data representations one more time.

I think that just making pipes unbuffered or using an environment
variable or command-line option is better way to handle this than
assuming that we won't be changing something in Cygwin.

cgf