GDB looses pgrp setting in the terminal for debugged process after break.

GDB looses pgrp setting in the terminal for debugged process after break.

[Takashi Yano]

Hi GDB maintainer,

In GDB, debugged process cannot continue execution after break
if it reads stdin.

With the following steps, cat is terminated with error.

1) Install coreutils-debuginfo package.
2) Run "gdb cat" in console (command prompt), not in mintty.
3) Enter "start" in gdb.
4) Enter "cont" in gdb.

This results in:
/usr/bin/cat: -: Input/output error

Both gdb-9.2-1 and gdb-10.1-1(TEST) have this problem.

I looked into this problem and found the cause is that the pgid
setting for /usr/bin/cat is lost after break. The following patch
for GDB source resolves the issue. In the following section,
winpid is passed to getpgid() rather than cygwin pid. Also, winpid
is passed to other POSIX system calls such as kill() elsewhere. 

--- inflow.c.orig	2020-05-24 06:10:29.000000000 +0900
+++ inflow.c	2021-01-23 17:48:27.963609500 +0900
@@ -364,11 +364,11 @@
 #ifdef HAVE_TERMIOS_H
 	  /* If we can't tell the inferior's actual process group,
 	     then restore whatever was the foreground pgrp the last
 	     time the inferior was running.  See also comments
 	     describing terminal_state::process_group.  */
-#ifdef HAVE_GETPGID
+#if defined (HAVE_GETPGID) && !defined (__CYGWIN__)
 	  result = tcsetpgrp (0, getpgid (inf->pid));
 #else
 	  result = tcsetpgrp (0, tinfo->process_group);
 #endif
 	  if (result == -1)


I hope the GDB maintainer will check it out.

Addresses: https://cygwin.com/pipermail/cygwin-patches/2021q1/011018.html

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

Re: GDB looses pgrp setting in the terminal for debugged process after break.

[Takashi Yano]

On Tue, 26 Jan 2021 12:14:02 +0900
Takashi Yano wrote:
> Hi GDB maintainer,
> 
> In GDB, debugged process cannot continue execution after break
> if it reads stdin.
> 
> With the following steps, cat is terminated with error.
> 
> 1) Install coreutils-debuginfo package.
> 2) Run "gdb cat" in console (command prompt), not in mintty.
> 3) Enter "start" in gdb.
> 4) Enter "cont" in gdb.
> 
> This results in:
> /usr/bin/cat: -: Input/output error
> 
> Both gdb-9.2-1 and gdb-10.1-1(TEST) have this problem.
> 
> I looked into this problem and found the cause is that the pgid
> setting for /usr/bin/cat is lost after break. The following patch
> for GDB source resolves the issue. In the following section,
> winpid is passed to getpgid() rather than cygwin pid. Also, winpid
> is passed to other POSIX system calls such as kill() elsewhere. 
> 
> --- inflow.c.orig	2020-05-24 06:10:29.000000000 +0900
> +++ inflow.c	2021-01-23 17:48:27.963609500 +0900
> @@ -364,11 +364,11 @@
>  #ifdef HAVE_TERMIOS_H
>  	  /* If we can't tell the inferior's actual process group,
>  	     then restore whatever was the foreground pgrp the last
>  	     time the inferior was running.  See also comments
>  	     describing terminal_state::process_group.  */
> -#ifdef HAVE_GETPGID
> +#if defined (HAVE_GETPGID) && !defined (__CYGWIN__)
>  	  result = tcsetpgrp (0, getpgid (inf->pid));
>  #else
>  	  result = tcsetpgrp (0, tinfo->process_group);
>  #endif
>  	  if (result == -1)
> 
> 
> I hope the GDB maintainer will check it out.
> 
> Addresses: https://cygwin.com/pipermail/cygwin-patches/2021q1/011018.html

I have noticed that cygwin gdb essentially has the problem
regarding terminal process group.

Cygwin gdb uses CreateProcessW() to execute debugging process
rather than exec(). If the debugging process is a cygwin process,
cygwin pid is assigned, however, if the debugging process is a
non cygwin process, cygwin pid is not assigned. Therefore, there
is no appropriate process group ID to set.

I wonder what is the right thing under that situation. Any idea?

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

Re: GDB looses pgrp setting in the terminal for debugged process after break.

[Takashi Yano]

On Fri, 5 Feb 2021 19:34:57 +0900
Takashi Yano wrote:
> On Tue, 26 Jan 2021 12:14:02 +0900
> Takashi Yano wrote:
> > Hi GDB maintainer,
> > 
> > In GDB, debugged process cannot continue execution after break
> > if it reads stdin.
> > 
> > With the following steps, cat is terminated with error.
> > 
> > 1) Install coreutils-debuginfo package.
> > 2) Run "gdb cat" in console (command prompt), not in mintty.
> > 3) Enter "start" in gdb.
> > 4) Enter "cont" in gdb.
> > 
> > This results in:
> > /usr/bin/cat: -: Input/output error
> > 
> > Both gdb-9.2-1 and gdb-10.1-1(TEST) have this problem.
> > 
> > I looked into this problem and found the cause is that the pgid
> > setting for /usr/bin/cat is lost after break. The following patch
> > for GDB source resolves the issue. In the following section,
> > winpid is passed to getpgid() rather than cygwin pid. Also, winpid
> > is passed to other POSIX system calls such as kill() elsewhere. 
> > 
> > --- inflow.c.orig	2020-05-24 06:10:29.000000000 +0900
> > +++ inflow.c	2021-01-23 17:48:27.963609500 +0900
> > @@ -364,11 +364,11 @@
> >  #ifdef HAVE_TERMIOS_H
> >  	  /* If we can't tell the inferior's actual process group,
> >  	     then restore whatever was the foreground pgrp the last
> >  	     time the inferior was running.  See also comments
> >  	     describing terminal_state::process_group.  */
> > -#ifdef HAVE_GETPGID
> > +#if defined (HAVE_GETPGID) && !defined (__CYGWIN__)
> >  	  result = tcsetpgrp (0, getpgid (inf->pid));
> >  #else
> >  	  result = tcsetpgrp (0, tinfo->process_group);
> >  #endif
> >  	  if (result == -1)
> > 
> > 
> > I hope the GDB maintainer will check it out.
> > 
> > Addresses: https://cygwin.com/pipermail/cygwin-patches/2021q1/011018.html
> 
> I have noticed that cygwin gdb essentially has the problem
> regarding terminal process group.
> 
> Cygwin gdb uses CreateProcessW() to execute debugging process
> rather than exec(). If the debugging process is a cygwin process,
> cygwin pid is assigned, however, if the debugging process is a
> non cygwin process, cygwin pid is not assigned. Therefore, there
> is no appropriate process group ID to set.
> 
> I wonder what is the right thing under that situation. Any idea?

The patch below seems to be better than the previous one a bit.

diff --git a/inflow.c.orig b/inflow.c
index 00b2235..fa7b408 100644
--- a/inflow.c.orig
+++ b/inflow.c
@@ -40,6 +40,10 @@
 #include <sys/ioctl.h>
 #endif
 
+#ifdef __CYGWIN__
+#include <sys/cygwin.h>
+#endif
+
 #ifndef O_NOCTTY
 #define O_NOCTTY 0
 #endif
@@ -367,7 +371,13 @@ child_terminal_inferior (struct target_ops *self)
 	     time the inferior was running.  See also comments
 	     describing terminal_state::process_group.  */
 #ifdef HAVE_GETPGID
+#ifdef __CYGWIN__
+	  pid_t cygpid = cygwin_internal (CW_WINPID_TO_CYGWIN_PID, inf->pid);
+	  if (cygpid <= cygwin_internal (CW_MAX_CYGWIN_PID))
+	    result = tcsetpgrp (0, getpgid (cygpid));
+#else
 	  result = tcsetpgrp (0, getpgid (inf->pid));
+#endif
 #else
 	  result = tcsetpgrp (0, tinfo->process_group);
 #endif

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

Re: GDB looses pgrp setting in the terminal for debugged process after break.

[Takashi Yano]

On Sat, 6 Feb 2021 00:26:54 +0900
Takashi Yano wrote:
> On Fri, 5 Feb 2021 19:34:57 +0900
> Takashi Yano wrote:
> > On Tue, 26 Jan 2021 12:14:02 +0900
> > Takashi Yano wrote:
> > > Hi GDB maintainer,
> > > 
> > > In GDB, debugged process cannot continue execution after break
> > > if it reads stdin.
> > > 
> > > With the following steps, cat is terminated with error.
> > > 
> > > 1) Install coreutils-debuginfo package.
> > > 2) Run "gdb cat" in console (command prompt), not in mintty.
> > > 3) Enter "start" in gdb.
> > > 4) Enter "cont" in gdb.
> > > 
> > > This results in:
> > > /usr/bin/cat: -: Input/output error
> > > 
> > > Both gdb-9.2-1 and gdb-10.1-1(TEST) have this problem.
> > > 
> > > I looked into this problem and found the cause is that the pgid
> > > setting for /usr/bin/cat is lost after break. The following patch
> > > for GDB source resolves the issue. In the following section,
> > > winpid is passed to getpgid() rather than cygwin pid. Also, winpid
> > > is passed to other POSIX system calls such as kill() elsewhere. 
> > > 
> > > --- inflow.c.orig	2020-05-24 06:10:29.000000000 +0900
> > > +++ inflow.c	2021-01-23 17:48:27.963609500 +0900
> > > @@ -364,11 +364,11 @@
> > >  #ifdef HAVE_TERMIOS_H
> > >  	  /* If we can't tell the inferior's actual process group,
> > >  	     then restore whatever was the foreground pgrp the last
> > >  	     time the inferior was running.  See also comments
> > >  	     describing terminal_state::process_group.  */
> > > -#ifdef HAVE_GETPGID
> > > +#if defined (HAVE_GETPGID) && !defined (__CYGWIN__)
> > >  	  result = tcsetpgrp (0, getpgid (inf->pid));
> > >  #else
> > >  	  result = tcsetpgrp (0, tinfo->process_group);
> > >  #endif
> > >  	  if (result == -1)
> > > 
> > > 
> > > I hope the GDB maintainer will check it out.
> > > 
> > > Addresses: https://cygwin.com/pipermail/cygwin-patches/2021q1/011018.html
> > 
> > I have noticed that cygwin gdb essentially has the problem
> > regarding terminal process group.
> > 
> > Cygwin gdb uses CreateProcessW() to execute debugging process
> > rather than exec(). If the debugging process is a cygwin process,
> > cygwin pid is assigned, however, if the debugging process is a
> > non cygwin process, cygwin pid is not assigned. Therefore, there
> > is no appropriate process group ID to set.
> > 
> > I wonder what is the right thing under that situation. Any idea?
> 
> The patch below seems to be better than the previous one a bit.
> 
> diff --git a/inflow.c.orig b/inflow.c
> index 00b2235..fa7b408 100644
> --- a/inflow.c.orig
> +++ b/inflow.c
> @@ -40,6 +40,10 @@
>  #include <sys/ioctl.h>
>  #endif
>  
> +#ifdef __CYGWIN__
> +#include <sys/cygwin.h>
> +#endif
> +
>  #ifndef O_NOCTTY
>  #define O_NOCTTY 0
>  #endif
> @@ -367,7 +371,13 @@ child_terminal_inferior (struct target_ops *self)
>  	     time the inferior was running.  See also comments
>  	     describing terminal_state::process_group.  */
>  #ifdef HAVE_GETPGID
> +#ifdef __CYGWIN__
> +	  pid_t cygpid = cygwin_internal (CW_WINPID_TO_CYGWIN_PID, inf->pid);
> +	  if (cygpid <= cygwin_internal (CW_MAX_CYGWIN_PID))
> +	    result = tcsetpgrp (0, getpgid (cygpid));
> +#else
>  	  result = tcsetpgrp (0, getpgid (inf->pid));
> +#endif
>  #else
>  	  result = tcsetpgrp (0, tinfo->process_group);
>  #endif

I confirmed that gdb-10.2-1 (TEST) still has this problem.

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

Re: GDB looses pgrp setting in the terminal for debugged process after break.

[Jon Turney]

On 02/05/2021 17:16, Takashi Yano via Cygwin wrote:
> On Sat, 6 Feb 2021 00:26:54 +0900
> Takashi Yano wrote:
>> On Fri, 5 Feb 2021 19:34:57 +0900
>> Takashi Yano wrote:
>>> On Tue, 26 Jan 2021 12:14:02 +0900
>>> Takashi Yano wrote:
>>>> Hi GDB maintainer,

Sorry! I meant to go back and look at this, but I forgot.

Thanks for reminding me!

>>>> In GDB, debugged process cannot continue execution after break
>>>> if it reads stdin.
>>>>
>>>> With the following steps, cat is terminated with error.
>>>>
>>>> 1) Install coreutils-debuginfo package.
>>>> 2) Run "gdb cat" in console (command prompt), not in mintty.
>>>> 3) Enter "start" in gdb.
>>>> 4) Enter "cont" in gdb.
>>>>
>>>> This results in:
>>>> /usr/bin/cat: -: Input/output error
>>>>
>>>> Both gdb-9.2-1 and gdb-10.1-1(TEST) have this problem.
>>>>
>>>> I looked into this problem and found the cause is that the pgid
>>>> setting for /usr/bin/cat is lost after break. The following patch
>>>> for GDB source resolves the issue. In the following section,
>>>> winpid is passed to getpgid() rather than cygwin pid. Also, winpid
>>>> is passed to other POSIX system calls such as kill() elsewhere.
>>>>
[...]
>>>>
>>>> I hope the GDB maintainer will check it out.
>>>>
>>>> Addresses: https://cygwin.com/pipermail/cygwin-patches/2021q1/011018.html
>>>
>>> I have noticed that cygwin gdb essentially has the problem
>>> regarding terminal process group.
>>>
>>> Cygwin gdb uses CreateProcessW() to execute debugging process

Yes, being half-aware that it's using the Win32 API to do the debugging 
makes things awkward in gdb.

At one stage, I did start writing some changes to gdb to use cygwin's 
fork/exec to start the inferior, rather than CreateProcess, but that's 
also awkward.

>>> rather than exec(). If the debugging process is a cygwin process,

This process is usually called the 'debugee' or 'the inferior'.

>>> cygwin pid is assigned, however, if the debugging process is a
>>> non cygwin process, cygwin pid is not assigned. Therefore, there
>>> is no appropriate process group ID to set.

I'm not sure how this scenario relates to the initial report which seems 
to be about a process tree:

cmd -> gdb (cygwin) -> cat (cygwin)

?

>>> I wonder what is the right thing under that situation. Any idea?
>>
>> The patch below seems to be better than the previous one a bit.
>>
>> diff --git a/inflow.c.orig b/inflow.c
>> index 00b2235..fa7b408 100644
>> --- a/inflow.c.orig
>> +++ b/inflow.c
>> @@ -40,6 +40,10 @@
>>   #include <sys/ioctl.h>
>>   #endif
>>   
>> +#ifdef __CYGWIN__
>> +#include <sys/cygwin.h>
>> +#endif
>> +
>>   #ifndef O_NOCTTY
>>   #define O_NOCTTY 0
>>   #endif
>> @@ -367,7 +371,13 @@ child_terminal_inferior (struct target_ops *self)
>>   	     time the inferior was running.  See also comments
>>   	     describing terminal_state::process_group.  */
>>   #ifdef HAVE_GETPGID
>> +#ifdef __CYGWIN__
>> +	  pid_t cygpid = cygwin_internal (CW_WINPID_TO_CYGWIN_PID, inf->pid);
>> +	  if (cygpid <= cygwin_internal (CW_MAX_CYGWIN_PID))
>> +	    result = tcsetpgrp (0, getpgid (cygpid));
>> +#else
>>   	  result = tcsetpgrp (0, getpgid (inf->pid));
>> +#endif
>>   #else
>>   	  result = tcsetpgrp (0, tinfo->process_group);
>>   #endif
> 
> I confirmed that gdb-10.2-1 (TEST) still has this problem.

It certainly seems reasonable that we need to do this cygwin/windows pid 
mapping somewhere.

Re: GDB looses pgrp setting in the terminal for debugged process after break.

[Takashi Yano]

On Thu, 6 May 2021 21:31:27 +0100
Jon Turney wrote:
> On 02/05/2021 17:16, Takashi Yano via Cygwin wrote:
> > On Sat, 6 Feb 2021 00:26:54 +0900
> > Takashi Yano wrote:
> >> On Fri, 5 Feb 2021 19:34:57 +0900
> >> Takashi Yano wrote:
> >>> On Tue, 26 Jan 2021 12:14:02 +0900
> >>> Takashi Yano wrote:
> >>>> Hi GDB maintainer,
> 
> Sorry! I meant to go back and look at this, but I forgot.
> 
> Thanks for reminding me!

Thanks for replying.

> >>>> In GDB, debugged process cannot continue execution after break
> >>>> if it reads stdin.
> >>>>
> >>>> With the following steps, cat is terminated with error.
> >>>>
> >>>> 1) Install coreutils-debuginfo package.
> >>>> 2) Run "gdb cat" in console (command prompt), not in mintty.
> >>>> 3) Enter "start" in gdb.
> >>>> 4) Enter "cont" in gdb.
> >>>>
> >>>> This results in:
> >>>> /usr/bin/cat: -: Input/output error
> >>>>
> >>>> Both gdb-9.2-1 and gdb-10.1-1(TEST) have this problem.
> >>>>
> >>>> I looked into this problem and found the cause is that the pgid
> >>>> setting for /usr/bin/cat is lost after break. The following patch
> >>>> for GDB source resolves the issue. In the following section,
> >>>> winpid is passed to getpgid() rather than cygwin pid. Also, winpid
> >>>> is passed to other POSIX system calls such as kill() elsewhere.
> >>>>
> [...]
> >>>>
> >>>> I hope the GDB maintainer will check it out.
> >>>>
> >>>> Addresses: https://cygwin.com/pipermail/cygwin-patches/2021q1/011018.html
> >>>
> >>> I have noticed that cygwin gdb essentially has the problem
> >>> regarding terminal process group.
> >>>
> >>> Cygwin gdb uses CreateProcessW() to execute debugging process
> 
> Yes, being half-aware that it's using the Win32 API to do the debugging 
> makes things awkward in gdb.
> 
> At one stage, I did start writing some changes to gdb to use cygwin's 
> fork/exec to start the inferior, rather than CreateProcess, but that's 
> also awkward.
> 
> >>> rather than exec(). If the debugging process is a cygwin process,
> 
> This process is usually called the 'debugee' or 'the inferior'.
> 
> >>> cygwin pid is assigned, however, if the debugging process is a
> >>> non cygwin process, cygwin pid is not assigned. Therefore, there
> >>> is no appropriate process group ID to set.
> 
> I'm not sure how this scenario relates to the initial report which seems 
> to be about a process tree:
> 
> cmd -> gdb (cygwin) -> cat (cygwin)
> 
> ?

No. This may be another (potential) problem.

I mean:
gdb (cygwin) -> program (non-cygwin)

Non-cygwin program means the program compiled with non-cygwin
compiler (e.g. mingw compiler).

IIUC, gdb sets terminal pgrp to the debugee while the debugee is
running and set terminal pgrp back to gdb itself when the debugee
breaks. In the case obove, gdb sets terminal pgrp to pgid of cat
when 'start' command is issued. When cat breaks at main(), gdb set
terminal pgrp to pgid of gdb itself. Then, when 'cont' command is
issued, gdb sets terminal pgrp to pgid of cat again.

If the debugee is a non-cygwin program (rather than cat), there
is no cygwin pgid for the debugee. Therefore, gdb cannot set
terminal pgrp to appropriate value while the debugee is running.

In that case, it may be the right thing not to set terminal pgrp
to the debugee. The code below does that. What do you think?

> >>> I wonder what is the right thing under that situation. Any idea?
> >>
> >> The patch below seems to be better than the previous one a bit.
> >>
> >> diff --git a/inflow.c.orig b/inflow.c
> >> index 00b2235..fa7b408 100644
> >> --- a/inflow.c.orig
> >> +++ b/inflow.c
> >> @@ -40,6 +40,10 @@
> >>   #include <sys/ioctl.h>
> >>   #endif
> >>   
> >> +#ifdef __CYGWIN__
> >> +#include <sys/cygwin.h>
> >> +#endif
> >> +
> >>   #ifndef O_NOCTTY
> >>   #define O_NOCTTY 0
> >>   #endif
> >> @@ -367,7 +371,13 @@ child_terminal_inferior (struct target_ops *self)
> >>   	     time the inferior was running.  See also comments
> >>   	     describing terminal_state::process_group.  */
> >>   #ifdef HAVE_GETPGID
> >> +#ifdef __CYGWIN__
> >> +	  pid_t cygpid = cygwin_internal (CW_WINPID_TO_CYGWIN_PID, inf->pid);
> >> +	  if (cygpid <= cygwin_internal (CW_MAX_CYGWIN_PID))
> >> +	    result = tcsetpgrp (0, getpgid (cygpid));
> >> +#else
> >>   	  result = tcsetpgrp (0, getpgid (inf->pid));
> >> +#endif
> >>   #else
> >>   	  result = tcsetpgrp (0, tinfo->process_group);
> >>   #endif
> > 
> > I confirmed that gdb-10.2-1 (TEST) still has this problem.
> 
> It certainly seems reasonable that we need to do this cygwin/windows pid 
> mapping somewhere.

I would be happy if you could consider how to fix this issue.

Thanks!

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

Re: GDB looses pgrp setting in the terminal for debugged process after break.

[Jon Turney]

On 07/05/2021 09:33, Takashi Yano via Cygwin wrote:
> On Thu, 6 May 2021 21:31:27 +0100
> Jon Turney wrote:
>> On 02/05/2021 17:16, Takashi Yano via Cygwin wrote:
>>> On Sat, 6 Feb 2021 00:26:54 +0900
>>> Takashi Yano wrote:
>>>> On Fri, 5 Feb 2021 19:34:57 +0900
>>>> Takashi Yano wrote:
>>>>> On Tue, 26 Jan 2021 12:14:02 +0900
>>>>> Takashi Yano wrote:
>>>>>> Hi GDB maintainer,
>>
>> Sorry! I meant to go back and look at this, but I forgot.
>>
>> Thanks for reminding me!
> 
> Thanks for replying.

I've included this patch in the gdb-11.22-1 test package.
I don't think this patch is upstreamable as is.

I guess I'd like something which stores both winpid and cygwin pid,
where the winpid is only used inside windows-nat with Win32 API calls, 
and the cygwin pid is the pid used by core code in POSIX API calls.

Unfortunately, writing that seems complex, as the 
CW_WINPID_TO_CYGWIN_PID mapping can't be done immediately after 
CreateProcess(), as some code in the inferior needs to run first (since 
to perform the mapping, it uses links created by 
pinfo::create_winpid_symlink(), called in crt0)