MS-Windows file input redirection, v2

Message ID 83mvhnqy2w.fsf@gnu.org
State New, archived
Headers

Commit Message

Eli Zaretskii Oct. 29, 2016, 9:55 a.m. UTC
  This is version 2 of the patch to support command-line redirection in
native MS-Windows debugging.  (I removed "CYGWIN" from the subject
because the code below only affects the MinGW build; I don't know
enough to do the same for Cygwin, if required, and to test that.)

Changes from previous version:

  . Pedro's comments adjudicated
  . NEWS entry added
  . Added a large commentary describing what is supported

OK to push to master?

commit 3a0b5a89b4b5c53d19f06eca700ed59fcd9c848d
Author:     Eli Zaretskii <eliz@gnu.org>
Date: Sat Oct 29 12:48:21 2016 +0300

    Support command-line redirection in native MS-Windows debugging
    
    gdb/ChangeLog
    2016-10-29  Eli Zaretskii  <eliz@gnu.org>
    
            * NEWS: Mention support for redirection on MS-Windows.
    
            * windows-nat.c (redir_open, redir_set_redirection)
            (redirect_inferior_handles) [!__CYGWIN__]: New functions.
            (windows_create_inferior) [!__CYGWIN__]: Use
            'redirect_inferior_handles' to redirect standard handles of the
            debuggee if the command line requests that.
  

Comments

Pedro Alves Oct. 29, 2016, 3:06 p.m. UTC | #1
On 10/29/2016 10:55 AM, Eli Zaretskii wrote:
> This is version 2 of the patch to support command-line redirection in
> native MS-Windows debugging.  (I removed "CYGWIN" from the subject
> because the code below only affects the MinGW build; I don't know
> enough to do the same for Cygwin, if required, and to test that.)
> 
> Changes from previous version:
> 
>   . Pedro's comments adjudicated
>   . NEWS entry added
>   . Added a large commentary describing what is supported
> 
> OK to push to master?

LGTM.  Thanks for the detailed comments!

Thanks,
Pedro Alves
  
Eli Zaretskii Oct. 29, 2016, 3:11 p.m. UTC | #2
> Cc: gdb-patches@sourceware.org, samsurfer117@gmail.com
> From: Pedro Alves <palves@redhat.com>
> Date: Sat, 29 Oct 2016 16:06:27 +0100
> 
> > Changes from previous version:
> > 
> >   . Pedro's comments adjudicated
> >   . NEWS entry added
> >   . Added a large commentary describing what is supported
> > 
> > OK to push to master?
> 
> LGTM.  Thanks for the detailed comments!

Thanks, pushed to master.
  

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 3376b1b..b59a749 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,13 @@ 
+2016-10-29  Eli Zaretskii  <eliz@gnu.org>
+
+	* NEWS: Mention support for redirection on MS-Windows.
+
+	* windows-nat.c (redir_open, redir_set_redirection)
+	(redirect_inferior_handles) [!__CYGWIN__]: New functions.
+	(windows_create_inferior) [!__CYGWIN__]: Use
+	'redirect_inferior_handles' to redirect standard handles of the
+	debuggee if the command line requests that.
+
 2016-10-28  Pedro Alves  <palves@redhat.com>
 
 	* Makefile.in (CXX_DIALECT): Get from configure.
diff --git a/gdb/NEWS b/gdb/NEWS
index 4a61438..5f3ca99 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -9,6 +9,14 @@ 
   compiler.  The --disable-build-with-cxx configure option has been
   removed.
 
+* Native debugging on MS-Windows supports command-line redirection
+
+  Command-line arguments used for starting programs on MS-Windows can
+  now include redirection symbols supported by native Windows shells,
+  such as '<', '>', '>>', '2>&1', etc.  This affects GDB commands such
+  as "run", "start", and "set args", as well as the corresponding MI
+  features.
+
 * Support for thread names on MS-Windows.
 
   GDB now catches and handles the special exception that programs
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index c6a809b..4b8adac 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -2116,6 +2116,301 @@  clear_win32_environment (char **env)
 }
 #endif
 
+#ifndef __CYGWIN__
+
+/* Redirection of inferior I/O streams for native MS-Windows programs.
+   Unlike on Unix, where this is handled by invoking the inferior via
+   the shell, on MS-Windows we need to emulate the cmd.exe shell.
+
+   The official documentation of the cmd.exe redirection features is here:
+
+     http://www.microsoft.com/resources/documentation/windows/xp/all/proddocs/en-us/redirection.mspx
+
+   (That page talks about Windows XP, but there's no newer
+   documentation, so we assume later versions of cmd.exe didn't change
+   anything.)
+
+   Caveat: the documentation on that page seems to include a few lies.
+   For example, it describes strange constructs 1<&2 and 2<&1, which
+   seem to work only when 1>&2 resp. 2>&1 would make sense, and so I
+   think the cmd.exe parser of the redirection symbols simply doesn't
+   care about the < vs > distinction in these cases.  Therefore, the
+   supported features are explicitly documented below.
+
+   The emulation below aims at supporting all the valid use cases
+   supported by cmd.exe, which include:
+
+     < FILE    redirect standard input from FILE
+     0< FILE   redirect standard input from FILE
+     <&N       redirect standard input from file descriptor N
+     0<&N      redirect standard input from file descriptor N
+     > FILE    redirect standard output to FILE
+     >> FILE   append standard output to FILE
+     1>> FILE  append standard output to FILE
+     >&N       redirect standard output to file descriptor N
+     1>&N      redirect standard output to file descriptor N
+     >>&N      append standard output to file descriptor N
+     1>>&N     append standard output to file descriptor N
+     2> FILE   redirect standard error to FILE
+     2>> FILE  append standard error to FILE
+     2>&N      redirect standard error to file descriptor N
+     2>>&N     append standard error to file descriptor N
+
+     Note that using N > 2 in the above construct is supported, but
+     requires that the corresponding file descriptor be open by some
+     means elsewhere or outside GDB.  Also note that using ">&0" or
+     "<&2" will generally fail, because the file descriptor redirected
+     from is normally open in an incompatible mode (e.g., FD 0 is open
+     for reading only).  IOW, use of such tricks is not recommended;
+     you are on your own.
+
+     We do NOT support redirection of file descriptors above 2, as in
+     "3>SOME-FILE", because MinGW compiled programs don't (supporting
+     that needs special handling in the startup code that MinGW
+     doesn't have).  Pipes are also not supported.
+
+     As for invalid use cases, where the redirection contains some
+     error, the emulation below will detect that and produce some
+     error and/or failure.  But the behavior in those cases is not
+     bug-for-bug compatible with what cmd.exe does in those cases.
+     That's because what cmd.exe does then is not well defined, and
+     seems to be a side effect of the cmd.exe parsing of the command
+     line more than anything else.  For example, try redirecting to an
+     invalid file name, as in "> foo:bar".
+
+     There are also minor syntactic deviations from what cmd.exe does
+     in some corner cases.  For example, it doesn't support the likes
+     of "> &foo" to mean redirect to file named literally "&foo"; we
+     do support that here, because that, too, sounds like some issue
+     with the cmd.exe parser.  Another nicety is that we support
+     redirection targets that use file names with forward slashes,
+     something cmd.exe doesn't -- this comes in handy since GDB
+     file-name completion can be used when typing the command line for
+     the inferior.  */
+
+/* Support routines for redirecting standard handles of the inferior.  */
+
+/* Parse a single redirection spec, open/duplicate the specified
+   file/fd, and assign the appropriate value to one of the 3 standard
+   file descriptors. */
+static int
+redir_open (const char *redir_string, int *inp, int *out, int *err)
+{
+  int *fd, ref_fd = -2;
+  int mode;
+  const char *fname = redir_string + 1;
+  int rc = *redir_string;
+
+  switch (rc)
+    {
+    case '0':
+      fname++;
+      /* FALLTHROUGH */
+    case '<':
+      fd = inp;
+      mode = O_RDONLY;
+      break;
+    case '1': case '2':
+      fname++;
+      /* FALLTHROUGH */
+    case '>':
+      fd = (rc == '2') ? err : out;
+      mode = O_WRONLY | O_CREAT;
+      if (*fname == '>')
+	{
+	  fname++;
+	  mode |= O_APPEND;
+	}
+      else
+	mode |= O_TRUNC;
+      break;
+    default:
+      return -1;
+    }
+
+  if (*fname == '&' && '0' <= fname[1] && fname[1] <= '9')
+    {
+      /* A reference to a file descriptor.  */
+      char *fdtail;
+      ref_fd = (int) strtol (fname + 1, &fdtail, 10);
+      if (fdtail > fname + 1 && *fdtail == '\0')
+	{
+	  /* Don't allow redirection when open modes are incompatible.  */
+	  if ((ref_fd == 0 && (fd == out || fd == err))
+	      || ((ref_fd == 1 || ref_fd == 2) && fd == inp))
+	    {
+	      errno = EPERM;
+	      return -1;
+	    }
+	  if (ref_fd == 0)
+	    ref_fd = *inp;
+	  else if (ref_fd == 1)
+	    ref_fd = *out;
+	  else if (ref_fd == 2)
+	    ref_fd = *err;
+	}
+      else
+	{
+	  errno = EBADF;
+	  return -1;
+	}
+    }
+  else
+    fname++;	/* skip the separator space */
+  /* If the descriptor is already open, close it.  This allows
+     multiple specs of redirections for the same stream, which is
+     somewhat nonsensical, but still valid and supported by cmd.exe.
+     (But cmd.exe only opens a single file in this case, the one
+     specified by the last redirection spec on the command line.)  */
+  if (*fd >= 0)
+    _close (*fd);
+  if (ref_fd == -2)
+    {
+      *fd = _open (fname, mode, _S_IREAD | _S_IWRITE);
+      if (*fd < 0)
+	return -1;
+    }
+  else if (ref_fd == -1)
+    *fd = -1;	/* reset to default destination */
+  else
+    {
+      *fd = _dup (ref_fd);
+      if (*fd < 0)
+	return -1;
+    }
+  /* _open just sets a flag for O_APPEND, which won't be passed to the
+     inferior, so we need to actually move the file pointer.  */
+  if ((mode & O_APPEND) != 0)
+    _lseek (*fd, 0L, SEEK_END);
+  return 0;
+}
+
+/* Canonicalize a single redirection spec and set up the corresponding
+   file descriptor as specified.  */
+static int
+redir_set_redirection (const char *s, int *inp, int *out, int *err)
+{
+  char buf[__PMAX + 2 + 5]; /* extra space for quotes & redirection string */
+  char *d = buf;
+  const char *start = s;
+  int quote = 0;
+
+  *d++ = *s++;	/* copy the 1st character, < or > or a digit */
+  if ((*start == '>' || *start == '1' || *start == '2')
+      && *s == '>')
+    {
+      *d++ = *s++;
+      if (*s == '>' && *start != '>')
+	*d++ = *s++;
+    }
+  else if (*start == '0' && *s == '<')
+    *d++ = *s++;
+  /* cmd.exe recognizes "&N" only immediately after the redirection symbol.  */
+  if (*s != '&')
+    {
+      while (isspace (*s))  /* skip whitespace before file name */
+	s++;
+      *d++ = ' ';	    /* separate file name with a single space */
+    }
+
+  /* Copy the file name.  */
+  while (*s)
+    {
+      /* Remove quoting characters from the file name in buf[].  */
+      if (*s == '"')	/* could support '..' quoting here */
+	{
+	  if (!quote)
+	    quote = *s++;
+	  else if (*s == quote)
+	    {
+	      quote = 0;
+	      s++;
+	    }
+	  else
+	    *d++ = *s++;
+	}
+      else if (*s == '\\')
+	{
+	  if (s[1] == '"')	/* could support '..' here */
+	    s++;
+	  *d++ = *s++;
+	}
+      else if (isspace (*s) && !quote)
+	break;
+      else
+	*d++ = *s++;
+      if (d - buf >= sizeof (buf) - 1)
+	{
+	  errno = ENAMETOOLONG;
+	  return 0;
+	}
+    }
+  *d = '\0';
+
+  /* Windows doesn't allow redirection characters in file names, so we
+     can bail out early if they use them, or if there's no target file
+     name after the redirection symbol.  */
+  if (d[-1] == '>' || d[-1] == '<')
+    {
+      errno = ENOENT;
+      return 0;
+    }
+  if (redir_open (buf, inp, out, err) == 0)
+    return s - start;
+  return 0;
+}
+
+/* Parse the command line for redirection specs and prepare the file
+   descriptors for the 3 standard streams accordingly.  */
+static bool
+redirect_inferior_handles (const char *cmd_orig, char *cmd,
+			   int *inp, int *out, int *err)
+{
+  const char *s = cmd_orig;
+  char *d = cmd;
+  int quote = 0;
+  bool retval = false;
+
+  while (isspace (*s))
+    *d++ = *s++;
+
+  while (*s)
+    {
+      if (*s == '"')	/* could also support '..' quoting here */
+	{
+	  if (!quote)
+	    quote = *s;
+	  else if (*s == quote)
+	    quote = 0;
+	}
+      else if (*s == '\\')
+	{
+	  if (s[1] == '"')	/* escaped quote char */
+	    s++;
+	}
+      else if (!quote)
+	{
+	  /* Process a single redirection candidate.  */
+	  if (*s == '<' || *s == '>'
+	      || ((*s == '1' || *s == '2') && s[1] == '>')
+	      || (*s == '0' && s[1] == '<'))
+	    {
+	      int skip = redir_set_redirection (s, inp, out, err);
+
+	      if (skip <= 0)
+		return false;
+	      retval = true;
+	      s += skip;
+	    }
+	}
+      if (*s)
+	*d++ = *s++;
+    }
+  *d = '\0';
+  return retval;
+}
+#endif	/* !__CYGWIN__ */
+
 /* Start an inferior windows child process and sets inferior_ptid to its pid.
    EXEC_FILE is the file to run.
    ALLARGS is a string containing the arguments to the program.
@@ -2138,20 +2433,25 @@  windows_create_inferior (struct target_ops *ops, char *exec_file,
   size_t len;
   int tty;
   int ostdin, ostdout, ostderr;
-#else
+#else  /* !__CYGWIN__ */
   char real_path[__PMAX];
   char shell[__PMAX]; /* Path to shell */
   char *toexec;
-  char *args;
-  size_t args_len;
-  HANDLE tty;
+  char *args, *allargs_copy;
+  size_t args_len, allargs_len;
+  int fd_inp = -1, fd_out = -1, fd_err = -1;
+  HANDLE tty = INVALID_HANDLE_VALUE;
+  HANDLE inf_stdin = INVALID_HANDLE_VALUE;
+  HANDLE inf_stdout = INVALID_HANDLE_VALUE;
+  HANDLE inf_stderr = INVALID_HANDLE_VALUE;
+  bool redirected = false;
   char *w32env;
   char *temp;
   size_t envlen;
   int i;
   size_t envsize;
   char **env;
-#endif
+#endif	/* !__CYGWIN__ */
   PROCESS_INFORMATION pi;
   BOOL ret;
   DWORD flags = 0;
@@ -2183,7 +2483,7 @@  windows_create_inferior (struct target_ops *ops, char *exec_file,
 	error (_("Error starting executable: %d"), errno);
       cygallargs = (wchar_t *) alloca (len * sizeof (wchar_t));
       mbstowcs (cygallargs, allargs, len);
-#else
+#else  /* !__USEWIDE */
       cygallargs = allargs;
 #endif
     }
@@ -2199,12 +2499,12 @@  windows_create_inferior (struct target_ops *ops, char *exec_file,
 	    + mbstowcs (NULL, allargs, 0) + 2;
       cygallargs = (wchar_t *) alloca (len * sizeof (wchar_t));
       swprintf (cygallargs, len, L" -c 'exec %s %s'", exec_file, allargs);
-#else
+#else  /* !__USEWIDE */
       len = (sizeof (" -c 'exec  '") + strlen (exec_file)
 	     + strlen (allargs) + 2);
       cygallargs = (char *) alloca (len);
       xsnprintf (cygallargs, len, " -c 'exec %s %s'", exec_file, allargs);
-#endif
+#endif	/* __USEWIDE */
       toexec = shell;
       flags |= DEBUG_PROCESS;
     }
@@ -2215,12 +2515,12 @@  windows_create_inferior (struct target_ops *ops, char *exec_file,
   wcscpy (args, toexec);
   wcscat (args, L" ");
   wcscat (args, cygallargs);
-#else
+#else  /* !__USEWIDE */
   args = (cygwin_buf_t *) alloca (strlen (toexec) + strlen (cygallargs) + 2);
   strcpy (args, toexec);
   strcat (args, " ");
   strcat (args, cygallargs);
-#endif
+#endif	/* !__USEWIDE */
 
 #ifdef CW_CVT_ENV_TO_WINENV
   /* First try to create a direct Win32 copy of the POSIX environment. */
@@ -2229,7 +2529,7 @@  windows_create_inferior (struct target_ops *ops, char *exec_file,
     flags |= CREATE_UNICODE_ENVIRONMENT;
   else
     /* If that fails, fall back to old method tweaking GDB's environment. */
-#endif
+#endif	/* CW_CVT_ENV_TO_WINENV */
     {
       /* Reset all Win32 environment variables to avoid leftover on next run. */
       clear_win32_environment (environ);
@@ -2294,21 +2594,26 @@  windows_create_inferior (struct target_ops *ops, char *exec_file,
       close (ostdout);
       close (ostderr);
     }
-#else
-  toexec = exec_file;
-  /* Build the command line, a space-separated list of tokens where
-     the first token is the name of the module to be executed.
-     To avoid ambiguities introduced by spaces in the module name,
-     we quote it.  */
-  args_len = strlen (toexec) + 2 /* quotes */ + strlen (allargs) + 2;
-  args = (char *) alloca (args_len);
-  xsnprintf (args, args_len, "\"%s\" %s", toexec, allargs);
-
-  flags |= DEBUG_ONLY_THIS_PROCESS;
-
-  if (!inferior_io_terminal)
-    tty = INVALID_HANDLE_VALUE;
-  else
+#else  /* !__CYGWIN__ */
+  allargs_len = strlen (allargs);
+  allargs_copy = strcpy ((char *) alloca (allargs_len + 1), allargs);
+  if (strpbrk (allargs_copy, "<>") != NULL)
+    {
+      int e = errno;
+      errno = 0;
+      redirected =
+	redirect_inferior_handles (allargs, allargs_copy,
+				   &fd_inp, &fd_out, &fd_err);
+      if (errno)
+	warning (_("Error in redirection: %s."), strerror (errno));
+      else
+	errno = e;
+      allargs_len = strlen (allargs_copy);
+    }
+  /* If not all the standard streams are redirected by the command
+     line, use inferior_io_terminal for those which aren't.  */
+  if (inferior_io_terminal
+      && !(fd_inp >= 0 && fd_out >= 0 && fd_err >= 0))
     {
       SECURITY_ATTRIBUTES sa;
       sa.nLength = sizeof(sa);
@@ -2319,15 +2624,41 @@  windows_create_inferior (struct target_ops *ops, char *exec_file,
       if (tty == INVALID_HANDLE_VALUE)
 	warning (_("Warning: Failed to open TTY %s, error %#x."),
 		 inferior_io_terminal, (unsigned) GetLastError ());
+    }
+  if (redirected || tty != INVALID_HANDLE_VALUE)
+    {
+      if (fd_inp >= 0)
+	si.hStdInput = (HANDLE) _get_osfhandle (fd_inp);
+      else if (tty != INVALID_HANDLE_VALUE)
+	si.hStdInput = tty;
       else
-	{
-	  si.hStdInput = tty;
-	  si.hStdOutput = tty;
-	  si.hStdError = tty;
-	  si.dwFlags |= STARTF_USESTDHANDLES;
-	}
+	si.hStdInput = GetStdHandle (STD_INPUT_HANDLE);
+      if (fd_out >= 0)
+	si.hStdOutput = (HANDLE) _get_osfhandle (fd_out);
+      else if (tty != INVALID_HANDLE_VALUE)
+	si.hStdOutput = tty;
+      else
+	si.hStdOutput = GetStdHandle (STD_OUTPUT_HANDLE);
+      if (fd_err >= 0)
+	si.hStdError = (HANDLE) _get_osfhandle (fd_err);
+      else if (tty != INVALID_HANDLE_VALUE)
+	si.hStdError = tty;
+      else
+	si.hStdError = GetStdHandle (STD_ERROR_HANDLE);
+      si.dwFlags |= STARTF_USESTDHANDLES;
     }
 
+  toexec = exec_file;
+  /* Build the command line, a space-separated list of tokens where
+     the first token is the name of the module to be executed.
+     To avoid ambiguities introduced by spaces in the module name,
+     we quote it.  */
+  args_len = strlen (toexec) + 2 /* quotes */ + allargs_len + 2;
+  args = (char *) alloca (args_len);
+  xsnprintf (args, args_len, "\"%s\" %s", toexec, allargs_copy);
+
+  flags |= DEBUG_ONLY_THIS_PROCESS;
+
   /* CreateProcess takes the environment list as a null terminated set of
      strings (i.e. two nulls terminate the list).  */
 
@@ -2366,7 +2697,13 @@  windows_create_inferior (struct target_ops *ops, char *exec_file,
 			&pi);
   if (tty != INVALID_HANDLE_VALUE)
     CloseHandle (tty);
-#endif
+  if (fd_inp >= 0)
+    _close (fd_inp);
+  if (fd_out >= 0)
+    _close (fd_out);
+  if (fd_err >= 0)
+    _close (fd_err);
+#endif	/* !__CYGWIN__ */
 
   if (!ret)
     error (_("Error creating process %s, (error %u)."),