Re: gdb run < file

Re: gdb run < file

[Christopher Faylor]

I'm sorry but I don't think that this is the correct way to deal with
this.  I think that gdb normally handles things like redirection and
globbing by starting inferior processes via the user's shell.  Then it
uses some kind of "follow fork" method to notice when the process is
finally started.

It just occurred to me that Windows has the capability (check out the
CreateProcess call) to do this so I think it makes sense to use gdb's
standard method for dealing with this problem.

I don't have any specific pointers since I'm not familiar with the code
in question, but I've Cc'ed the gdb mailing list in case someone can
offer advice.

I should point out that you'll need to have an assignment on file with
the FSF for your changes to be officially incorporated into gdb.

I've Cc'ed the person responsible for gdb assignments in this email.

cgf

On Wed, Jun 27, 2001 at 11:57:00PM -0700, Tak Ota wrote:
>Since this is the first time for me to look at gdb source it took a
>while to get to the right place to tackle.  Thank to Source Navigator,
>eventually I found win32-nat.c was the one I needed to fix.  The fix
>itself is quite straight forward.
>
>Please see the patch to win32-nat.c in gdb-20010428-1 at the end of
>this message.  I am not quite satisfied with the argument parsing part
>but at least it fulfills what I wanted.  Please improve it if you have
>better idea.
>
>However I have verified this works, my testing environment is fairly
>limited.  Please verify it in various different configuration.
>
>-Tak
>
>On Wed, 27 Jun 2001 02:50:36 -0400, Christopher Faylor <cgf@redhat.com> wrote:
>
>> On Tue, Jun 26, 2001 at 11:44:02PM -0700, Tak Ota wrote:
>> >I encountered a problem in gdb.
>> >
>> >(gdb) run < file
>> >
>> >does not change the process stdin to the file.
>> 
>> Yep.  That's a limitation in windows gdb.
>> 
>> >I just started looking into gdb source code for the first time in my
>> >left.  For now I found top.c has command loop but the size of the code
>> >is so overwhelming.  So I thought reporting the incident first may
>> >solve the problem way before I can find the cause and fix.
>> 
>> No one is working on this, currently.  If you can provide a fix it would
>> be appreciated.  I don't know of any easy way to get this functionality,
>> though.

Re: gdb run < file

[Tak Ota]

I'm sorry too, since what I have done is essentially the work which
normally is taken care by a shell.  And talking about CreateProcess,
the 9th argument of this function is STARTUPINFO, which does hold
initial hStdInput, hStdOutput and hStdError information.  I first
tried to use these before reaching the method I submitted, however
without a success.  After struggling with STARTUPINFO (trying to match
file descriptors and handles) I realized that "why don't I do what
shell normally does?".  As an early comment line in win32-nat.c states
"We assume we're being built with and will be used for cygwin", all
normal open, close and dup should work, and they did.

I don't know why the original implementer did not use a shell to fork
and exec the process instead of calling CreateProcess.  I can only
imagine that there must have been nontrivial reasons for that.

So I agree with you that this is not the right implementation from
normal Unix point of view.  However, until the *CORRECT*
implementation, if it ever exists, becomes available this does serve
for easing users' inconvenience.  Missing redirection certainly has
been very annoying lack of feature to me.

I'm glad you've routed the information to the right people.  I hope we
can eventually reach to the *CORRECT* implementation someday.

In the meantime, for whom seeking for practical solutions, I've
attached revised patch which further emulates shell redirection work,
taking care of [n]>, [n]<, [n]>| and [n]>>.  I know this approach will
never become the *CORRECT* solution no matter how hard I try to
emulate shell's work and I certainly do not intend to emulate perfect
shell.  That is too much for me as well as being a wasteful effort.

Best regards,

-Tak


On Thu, 28 Jun 2001 22:06:02 -0400, Christopher Faylor <cgf@redhat.com> wrote:

> I'm sorry but I don't think that this is the correct way to deal with
> this.  I think that gdb normally handles things like redirection and
> globbing by starting inferior processes via the user's shell.  Then it
> uses some kind of "follow fork" method to notice when the process is
> finally started.
> 
> It just occurred to me that Windows has the capability (check out the
> CreateProcess call) to do this so I think it makes sense to use gdb's
> standard method for dealing with this problem.
> 
> I don't have any specific pointers since I'm not familiar with the code
> in question, but I've Cc'ed the gdb mailing list in case someone can
> offer advice.
> 
> I should point out that you'll need to have an assignment on file with
> the FSF for your changes to be officially incorporated into gdb.
> 
> I've Cc'ed the person responsible for gdb assignments in this email.
> 
> cgf
> 
> On Wed, Jun 27, 2001 at 11:57:00PM -0700, Tak Ota wrote:
> >Since this is the first time for me to look at gdb source it took a
> >while to get to the right place to tackle.  Thank to Source Navigator,
> >eventually I found win32-nat.c was the one I needed to fix.  The fix
> >itself is quite straight forward.
> >
> >Please see the patch to win32-nat.c in gdb-20010428-1 at the end of
> >this message.  I am not quite satisfied with the argument parsing part
> >but at least it fulfills what I wanted.  Please improve it if you have
> >better idea.
> >
> >However I have verified this works, my testing environment is fairly
> >limited.  Please verify it in various different configuration.
> >
> >-Tak
> >
> >On Wed, 27 Jun 2001 02:50:36 -0400, Christopher Faylor <cgf@redhat.com> wrote:
> >
> >> On Tue, Jun 26, 2001 at 11:44:02PM -0700, Tak Ota wrote:
> >> >I encountered a problem in gdb.
> >> >
> >> >(gdb) run < file
> >> >
> >> >does not change the process stdin to the file.
> >> 
> >> Yep.  That's a limitation in windows gdb.
> >> 
> >> >I just started looking into gdb source code for the first time in my
> >> >left.  For now I found top.c has command loop but the size of the code
> >> >is so overwhelming.  So I thought reporting the incident first may
> >> >solve the problem way before I can find the cause and fix.
> >> 
> >> No one is working on this, currently.  If you can provide a fix it would
> >> be appreciated.  I don't know of any easy way to get this functionality,
> >> though.
> 
> --
> Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
> Bug reporting:         http://cygwin.com/bugs.html
> Documentation:         http://cygwin.com/docs.html
> FAQ:                   http://cygwin.com/faq/
\f
*** win32-nat.org.c	Wed Apr 18 13:27:11 2001
--- win32-nat.c	Thu Jun 28 20:47:45 2001
***************
*** 49,54 ****
--- 49,56 ----
  #include <sys/param.h>
  #include <unistd.h>
  
+ #include <ctype.h>
+ 
  /* The ui's event loop. */
  extern int (*ui_loop_hook) (int signo);
  
***************
*** 1094,1099 ****
--- 1096,1103 ----
    BOOL ret;
    DWORD flags;
    char *args;
+   char *redirection_path[3];
+   int noclobber[3], append[3];
  
    if (!exec_file)
      error ("No executable specified, use `target exec'.\n");
***************
*** 1116,1122 ****
    strcpy (args, real_path);
  
    strcat (args, " ");
!   strcat (args, allargs);
  
    /* Prepare the environment vars for CreateProcess.  */
    {
--- 1120,1199 ----
    strcpy (args, real_path);
  
    strcat (args, " ");
!   {
!     /* Perform minimum argument parsing and
!        extract I/O redirection parameters. */
!     char *p = &args[strlen(args)], *q = allargs;
!     int i;
!     for (i = 0; i < 3; i++)
!       {
!         redirection_path[i] = NULL;
!         noclobber[i] = 1; /* default is noclobber */
!         append[i] = 0;
!       }
!     while (1)
!       {
!         char delimiter, *r;
!         int fd;
!         switch (*q)
!           {
!           case '\0': /* end */
!             goto parsed_allargs;
!           case '\\': /* escape */
!             if(*(q + 1) == '\0')
!               goto parsed_allargs;
!             *p++ = *q++;
!             *p++ = *q++;
!             break;
!           case '\'': case '\"': case '`': /* quotes */
!             delimiter = *q;
!             *p++ = *q++;
!             while (*q && *q != delimiter)
!               *p++ = *q++;
!             if(*q)
!               *p++ = *q++;
!             break;
!           case '<': case '>': /* redirection */
!             if (*(p - 1) == '0' || *(p - 1) == '1' || *(p - 1) == '2')
!               fd = *--p - '0';
!             else
!               fd = *q == '<' ? 0 : 1;
!             q++;
!             if (*q == '|')
!               {
!                 noclobber[fd] = 0;
!                 q++;
!               }
!             else if (*q == '>' && fd != 0)
!               {
!                 append[fd] = 1;
!                 q++;
!               }
!             r = redirection_path[fd] = alloca (strlen (q) + 1);
!             do
!               q++;
!             while (isspace (*q));
!             switch (*q)
!               {
!               case '\'': case '\"': case '`': /* quotes */
!                 delimiter = *q++;
!                 break;
!               default:
!                 delimiter = '\0';
!               }
!             while (*q && (delimiter ? *q != delimiter : !isspace (*q)))
!               *r++ = *q++;
!             *r = '\0';
!             if (*q && *q == delimiter)
!               q++;
!             break;
!           default:
!             *p++ = *q++;
!           }
!       }
!   parsed_allargs:
!     *p = '\0';
!   }
  
    /* Prepare the environment vars for CreateProcess.  */
    {
***************
*** 1191,1206 ****
      *temp = 0;
    }
  
!   ret = CreateProcess (0,
! 		       args,	/* command line */
! 		       NULL,	/* Security */
! 		       NULL,	/* thread */
! 		       TRUE,	/* inherit handles */
! 		       flags,	/* start flags */
! 		       winenv,
! 		       NULL,	/* current directory */
! 		       &si,
! 		       &pi);
    if (!ret)
      error ("Error creating process %s, (error %d)\n", exec_file, GetLastError ());
  
--- 1268,1326 ----
      *temp = 0;
    }
  
!   {
!     /* When specified, set up I/O redirection while creating
!        the inferior process. */
!     int fd_copy[3], i;
!     for (i = 0; i < 3; i++)
!       {
!         if (redirection_path[i])
!           {
!             int fd;
!             fd_copy[i] = dup (i);
!             fflush (i == 0 ? stdin : i == 2 ? stdout : stderr);
!             close (i);
!             if (i == 0)
!               fd = open (redirection_path[i], O_RDONLY);
!             else
!               fd = open (redirection_path[i], O_CREAT | O_WRONLY |
!                          (noclobber[i] && !append[i] ? O_EXCL : 0) | (append[i] ? O_APPEND : O_TRUNC),
!                          0666);
!             if (fd == -1)
!               {
!                 int j;
!                 for (j = 0; j <= i; j++)
!                   {
!                     close (j);
!                     dup (fd_copy[j]);
!                     close (fd_copy[j]);
!                   }
!                 error ("Cannot open file `%s'.\n", redirection_path[i]);
!               }
!           }
!         else
!           fd_copy[i] = -1;
!       }
!     ret = CreateProcess (0,
!                          args,	/* command line */
!                          NULL,	/* Security */
!                          NULL,	/* thread */
!                          TRUE,	/* inherit handles */
!                          flags,	/* start flags */
!                          winenv,
!                          NULL,	/* current directory */
!                          &si,
!                          &pi);
!     for (i = 0; i < 3; i++)
!       {
!         if (fd_copy[i] != -1)
!           {
!             close (i);
!             dup (fd_copy[i]);
!             close (fd_copy[i]);
!           }
!       }
!   }
    if (!ret)
      error ("Error creating process %s, (error %d)\n", exec_file, GetLastError ());
  

Re: gdb run < file

[Eli Zaretskii]

> Date: Thu, 28 Jun 2001 22:06:02 -0400
> From: Christopher Faylor <cgf@redhat.com>
> 
> I'm sorry but I don't think that this is the correct way to deal with
> this.  I think that gdb normally handles things like redirection and
> globbing by starting inferior processes via the user's shell.  Then it
> uses some kind of "follow fork" method to notice when the process is
> finally started.

I don't know anything about Windows debugging interface, but it might
be that putting a shell between the debugger and the debuggee breaks
something.

Going through the shell also has the limitation that you buy whatever
idiosyncrasies there are in that shell.  Unlike on Unix, where the
redirection behavior is quite similar between the shells, on Windows
there's a very wide disagreement.  (We are fed up with that nuisance
in the Emacs-land.)  So I think there's a lot of sense in having a
unified, predictable redirection support inside GDB that doesn't
depend on the queer shell the user happens to have.

FWIW, the DJGPP port of GDB does something very similar to the patch
we discuss here (except that it also has to flip redirections every
time the execution thread jumps from the debugger to the debuggee).
See go32-nat.c (the actual implementation of the redirection routines
is in the DJGPP debug support library).

Re: gdb run < file

[Christopher Faylor]

On Fri, Jun 29, 2001 at 10:11:54AM +0300, Eli Zaretskii wrote:
>> Date: Thu, 28 Jun 2001 22:06:02 -0400
>> From: Christopher Faylor <cgf@redhat.com>
>> 
>> I'm sorry but I don't think that this is the correct way to deal with
>> this.  I think that gdb normally handles things like redirection and
>> globbing by starting inferior processes via the user's shell.  Then it
>> uses some kind of "follow fork" method to notice when the process is
>> finally started.
>
>I don't know anything about Windows debugging interface, but it might
>be that putting a shell between the debugger and the debuggee breaks
>something.
>
>Going through the shell also has the limitation that you buy whatever
>idiosyncrasies there are in that shell.  Unlike on Unix, where the
>redirection behavior is quite similar between the shells, on Windows
>there's a very wide disagreement.  (We are fed up with that nuisance
>in the Emacs-land.)  So I think there's a lot of sense in having a
>unified, predictable redirection support inside GDB that doesn't
>depend on the queer shell the user happens to have.

Since Cygwin is supposed to emulate Windows, there shouldn't be much
difference between the way UNIX does it and the way Cygwin does it.
All of the shells come from UNIX, anyway.

So, if this is an issue then it must be an issue for UNIX as well.

>FWIW, the DJGPP port of GDB does something very similar to the patch
>we discuss here (except that it also has to flip redirections every
>time the execution thread jumps from the debugger to the debuggee).
>See go32-nat.c (the actual implementation of the redirection routines
>is in the DJGPP debug support library).

win32-nat.c already has a lot of gratutitous differences with the rest
of the "Unix" code in gdb.  I want to be moving in the direction of
making the cygwin parts more unix-like.  So, I would rather that the
redirection follow UNIX rather than attempting to reimplement the
already implemented.

This is, of course, a problem for running a windows-only version of
gdb but I don't know of anyone currently working on that.  Maybe
we need a separate option for when a complete cygwin system isn't
installed.  However, that would be secondary to getting something
working with /bin/sh or whatever.

cgf

Re: gdb run < file

[Christopher Faylor]

On Fri, Jun 29, 2001 at 04:53:52PM -0400, Christopher Faylor wrote:
>Since Cygwin is supposed to emulate Windows, there shouldn't be much
				     UNIX

sigh.

cgf

Re: gdb run < file

[Eli Zaretskii]

> Date: Fri, 29 Jun 2001 16:53:52 -0400
> From: Christopher Faylor <cgf@redhat.com>
> 
> Since Cygwin is supposed to emulate Windows, there shouldn't be much
> difference between the way UNIX does it and the way Cygwin does it.

If you can do that without losing features, then it's of course okay.
But losing features in the name of greater Unix compatibility is IMHO
a grave mistake.  It will prevent many Windows users from using Cygwin
tools as a development platform for Windows programs.  Likewise,
waiting indefinitely for the ``right'' solution to surface and in the
meantime rejecting a less ``right'' but nevertheless clean solution,
is also a mistake.

However, this is not something to be discussed on the GDB list.

Re: gdb run < file

[Christopher Faylor]

On Sat, Jun 30, 2001 at 09:33:31AM +0300, Eli Zaretskii wrote:
>> Date: Fri, 29 Jun 2001 16:53:52 -0400
>> From: Christopher Faylor <cgf@redhat.com>
>> 
>> Since Cygwin is supposed to emulate UNIX, there shouldn't be much
>> difference between the way UNIX does it and the way Cygwin does it.
>
>If you can do that without losing features, then it's of course okay.
>But losing features in the name of greater Unix compatibility is IMHO
>a grave mistake.

Who's talking about "losing features"?

>It will prevent many Windows users from using Cygwin tools as a
>development platform for Windows programs.  Likewise, waiting
>indefinitely for the ``right'' solution to surface and in the meantime
>rejecting a less ``right'' but nevertheless clean solution, is also a
>mistake.

Perhaps.  However, implementing this the UNIX way should be relatively
trivial.  It at least deserves study before we throw in the towel.  This
is consistent with the way gdb is managed.  We don't just accept patches
from someone because they aren't sure how to do it the accepted way and
say "Oh well, this is as good as it is going to get."

It's my job as a maintainer to ensure that the code that is added to
win32-nat.c is...  maintainable.  IMO, it will be a lot more
maintainable if new code is not inventing new ways of doing things,
ignoring established gdb practices.

I really regret not suggesting that the original poster explore the
"follow fork" solution.  I also forgot to preload the expectation that
an assignment form was necessary.  That was probably discouraging.

>However, this is not something to be discussed on the GDB list.

I don't see why not.  We're talking about gdb.

cgf

Re: gdb run < file

[Tak Ota]

On Sat, 30 Jun 2001 13:25:57 -0400, Christopher Faylor <cgf@redhat.com> wrote:

> Perhaps.  However, implementing this the UNIX way should be relatively
> trivial.  It at least deserves study before we throw in the towel.  This

You are right.

> I really regret not suggesting that the original poster explore the
> "follow fork" solution.  I also forgot to preload the expectation that
> an assignment form was necessary.  That was probably discouraging.

Don't regret.  I think I am doing the right thing.  As I've said
before what I tried first was admittedly a kludge solution.  I am done
with that and now exploring the correct unix way.  This is
experimental (shell is hard coded as "bash") however the mechanism
seems to be working.  I only need to add some more code to figure out
what the user's shell is.

-Tak


\f
*** win32-nat.org.c	Wed Apr 18 13:27:11 2001
--- win32-nat.c	Sat Jun 30 16:51:52 2001
***************
*** 1094,1099 ****
--- 1094,1100 ----
    BOOL ret;
    DWORD flags;
    char *args;
+   char *shell = "bash -c 'exec %s %s'";
  
    if (!exec_file)
      error ("No executable specified, use `target exec'.\n");
***************
*** 1101,1107 ****
    memset (&si, 0, sizeof (si));
    si.cb = sizeof (si);
  
!   cygwin_conv_to_win32_path (exec_file, real_path);
  
    flags = DEBUG_ONLY_THIS_PROCESS;
  
--- 1102,1108 ----
    memset (&si, 0, sizeof (si));
    si.cb = sizeof (si);
  
!   /*cygwin_conv_to_win32_path (exec_file, real_path);*/
  
    flags = DEBUG_ONLY_THIS_PROCESS;
  
***************
*** 1111,1122 ****
    if (new_console)
      flags |= CREATE_NEW_CONSOLE;
  
!   args = alloca (strlen (real_path) + strlen (allargs) + 2);
  
!   strcpy (args, real_path);
! 
!   strcat (args, " ");
!   strcat (args, allargs);
  
    /* Prepare the environment vars for CreateProcess.  */
    {
--- 1112,1120 ----
    if (new_console)
      flags |= CREATE_NEW_CONSOLE;
  
!   args = alloca (strlen(shell) + strlen (exec_file) + strlen (allargs) + 2);
  
!   sprintf(args, shell, exec_file, allargs);
  
    /* Prepare the environment vars for CreateProcess.  */
    {

Re: gdb run < file

[Tak Ota]

This is much simpler and cleaner than the previous attempt.  And it is
the *CORRECT* implementation from UNIX point of view.  The existence
of "SHELL" environment variable does not necessarily guaranty that it
is usable for this purpose.  This needs to be improved.

-Tak

\f
*** win32-nat.org.c	Wed Apr 18 13:27:11 2001
--- win32-nat.c	Sat Jun 30 21:59:29 2001
***************
*** 1084,1090 ****
  static void
  child_create_inferior (char *exec_file, char *allargs, char **env)
  {
!   char real_path[MAXPATHLEN];
    char *winenv;
    char *temp;
    int envlen;
--- 1084,1090 ----
  static void
  child_create_inferior (char *exec_file, char *allargs, char **env)
  {
!   char *shell;
    char *winenv;
    char *temp;
    int envlen;
***************
*** 1101,1108 ****
    memset (&si, 0, sizeof (si));
    si.cb = sizeof (si);
  
-   cygwin_conv_to_win32_path (exec_file, real_path);
- 
    flags = DEBUG_ONLY_THIS_PROCESS;
  
    if (new_group)
--- 1101,1106 ----
***************
*** 1111,1122 ****
    if (new_console)
      flags |= CREATE_NEW_CONSOLE;
  
!   args = alloca (strlen (real_path) + strlen (allargs) + 2);
! 
!   strcpy (args, real_path);
! 
!   strcat (args, " ");
!   strcat (args, allargs);
  
    /* Prepare the environment vars for CreateProcess.  */
    {
--- 1109,1132 ----
    if (new_console)
      flags |= CREATE_NEW_CONSOLE;
  
!   shell = getenv ("SHELL");
!   if (shell)
!     {
!       char *exec_format = "%s -c 'exec %s %s'";
! 
!       args = alloca (strlen (exec_format) + strlen (shell) + strlen (exec_file) + strlen (allargs));
!       sprintf(args, exec_format, shell, exec_file, allargs);
!     }
!   else
!     {
!       char real_path[MAXPATHLEN];
! 
!       cygwin_conv_to_win32_path (exec_file, real_path);
!       args = alloca (strlen (real_path) + strlen (allargs) + 2);
!       strcpy (args, real_path);
!       strcat (args, " ");
!       strcat (args, allargs);
!     }
  
    /* Prepare the environment vars for CreateProcess.  */
    {

Re: gdb run < file

[Eli Zaretskii]

On Sat, 30 Jun 2001, Tak Ota wrote:

> +   char *shell = "bash -c 'exec %s %s'";

I think this will fail if the command includes the apostrophe character.

And, of course, hard-wiring the shell name is not really a good idea.