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

Pierre Muller
2013-08-08T07:28:27Z

> -----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);
+
}