bug in procps-ng

bug in procps-ng

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 729 bytes --]

Hi Wayne,

I hope you're still with us.

There appears to be a bug in procps-ng:

$ procps -e
  PID TTY          TIME CMD
 1507 ?        00:00:00 tcsh
 1529 ?        00:00:00 cygrunsrv
 1506 ?        00:00:00 mintty
 1531 ?        00:00:00 bash
 1551 pty0     00:00:00 procps
 1488 pty0     00:00:00 sh
 1487 ?        00:00:01 mintty
 1530 ?        00:00:00 xterm

As you can see, the tty info seems to be broken.  In theory
procps-ng should fetch the tty info from /proc/<PID>/stat.
As far as I can see, the tty info in this file is correct
for other processes.  But procps only shows info for its
own tty for some reason.

Any idea why?


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: bug in procps-ng

[Brian Inglis]

On 2019-03-12 11:03, Corinna Vinschen wrote:
> I hope you're still with us.
> There appears to be a bug in procps-ng:
> $ procps -e
>   PID TTY          TIME CMD
>  1507 ?        00:00:00 tcsh
>  1529 ?        00:00:00 cygrunsrv
>  1506 ?        00:00:00 mintty
>  1531 ?        00:00:00 bash
>  1551 pty0     00:00:00 procps
>  1488 pty0     00:00:00 sh
>  1487 ?        00:00:01 mintty
>  1530 ?        00:00:00 xterm
> As you can see, the tty info seems to be broken.  In theory
> procps-ng should fetch the tty info from /proc/<PID>/stat.
> As far as I can see, the tty info in this file is correct
> for other processes.  But procps only shows info for its
> own tty for some reason.
> Any idea why?

Are /dev/con?? and /dev/pty? visible only in those processes?
Need persistent visible mappings to /dev/con?? and /dev/pty?
Inconsistencies in /dev/con?? handling: why do con* appear under major 5 with
different minors, and also as 3,0 consistent with /proc/PID/stat?

$ ls -l /dev/ | egrep con\|pty
crw-rw-rw-  1 Brian  Users    5, 255 Mar 12 22:23 conin
crw-rw-rw-  1 Brian  Users    5, 254 Mar 12 22:23 conout
crw-rw-rw-  1 Brian  Users    5,   1 Mar 12 22:23 console
crw--w----  1 Brian  Users  136,   0 Mar 12 22:23 pty0
$ ls -l /dev/{con,pty}*		# /proc/PID/stat maps to these major/minors
crw-rw-rw- 1 Brian Users   3, 0 Mar 12 22:23 /dev/conin
crw-rw-rw- 1 Brian Users   3, 0 Mar 12 22:23 /dev/conout
crw-rw-rw- 1 Brian Users   3, 0 Mar 12 22:23 /dev/console
crw--w---- 1 Brian Users 136, 0 Mar 12 22:23 /dev/pty0
$ fgrep /dev/ /proc/?????/ctty
/proc/63731/ctty:/dev/cons0
/proc/63835/ctty:/dev/cons0
/proc/63836/ctty:/dev/cons0
/proc/63865/ctty:/dev/pty0
$ ls -glo /proc/?????/fd/0	# /dev/con?? links appear stale (red on black)
lrwxrwxrwx 1 0 Mar 12 22:23 /proc/63731/fd/0 -> /dev/cons0
lrwxrwxrwx 1 0 Mar 12 22:23 /proc/63835/fd/0 -> /dev/cons0
lrwxrwxrwx 1 0 Mar 12 22:23 /proc/63836/fd/0 -> /var/log/xwin/XWin.0.log
lrwxrwxrwx 1 0 Mar 12 22:23 /proc/63839/fd/0 -> /dev/null
lrwxrwxrwx 1 0 Mar 12 22:23 /proc/63849/fd/0 -> /dev/null
lrwxrwxrwx 1 0 Mar 12 22:23 /proc/63850/fd/0 -> /dev/null
lrwxrwxrwx 1 0 Mar 12 22:23 /proc/63854/fd/0 -> /dev/null
lrwxrwxrwx 1 0 Mar 12 22:23 /proc/63862/fd/0 -> /dev/null
lrwxrwxrwx 1 0 Mar 12 22:23 /proc/63864/fd/0 -> /dev/null
lrwxrwxrwx 1 0 Mar 12 22:23 /proc/63865/fd/0 -> /dev/pty0
lrwxrwxrwx 1 0 Mar 12 22:23 /proc/63867/fd/0 -> /dev/null
lrwxrwxrwx 1 0 Mar 12 22:23 /proc/63883/fd/0 -> /dev/null

Cygwin /proc/PID/stat[$7] appears to have major in upper half, minor in lower
half e.g. 8912896 -> 00880000 -> 136,0; 196608 -> 00030000 -> 3,0.
Linux man 5 proc defines:
(7) tty_nr  %d
	The controlling terminal of the process. (The minor device number is
	contained in the combination of bits 31 to 20 and 7 to 0; the major
	device number is in bits 15 to 8.)

-- 
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada

This email may be disturbing to some readers as it contains
too much technical detail. Reader discretion is advised.

Re: bug in procps-ng

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 3522 bytes --]

On Mar 12 22:55, Brian Inglis wrote:
> On 2019-03-12 11:03, Corinna Vinschen wrote:
> > I hope you're still with us.
> > There appears to be a bug in procps-ng:
> > $ procps -e
> >   PID TTY          TIME CMD
> >  1507 ?        00:00:00 tcsh
> >  1529 ?        00:00:00 cygrunsrv
> >  1506 ?        00:00:00 mintty
> >  1531 ?        00:00:00 bash
> >  1551 pty0     00:00:00 procps
> >  1488 pty0     00:00:00 sh
> >  1487 ?        00:00:01 mintty
> >  1530 ?        00:00:00 xterm
> > As you can see, the tty info seems to be broken.  In theory
> > procps-ng should fetch the tty info from /proc/<PID>/stat.
> > As far as I can see, the tty info in this file is correct
> > for other processes.  But procps only shows info for its
> > own tty for some reason.
> > Any idea why?
> 
> Are /dev/con?? and /dev/pty? visible only in those processes?

/dev/consX, yes.  the ptys are always visible.

> Need persistent visible mappings to /dev/con?? and /dev/pty?
> Inconsistencies in /dev/con?? handling: why do con* appear under major 5 with
> different minors, and also as 3,0 consistent with /proc/PID/stat?
> 
> $ ls -l /dev/ | egrep con\|pty
> crw-rw-rw-  1 Brian  Users    5, 255 Mar 12 22:23 conin
> crw-rw-rw-  1 Brian  Users    5, 254 Mar 12 22:23 conout
> crw-rw-rw-  1 Brian  Users    5,   1 Mar 12 22:23 console
> crw--w----  1 Brian  Users  136,   0 Mar 12 22:23 pty0
> $ ls -l /dev/{con,pty}*		# /proc/PID/stat maps to these major/minors
> crw-rw-rw- 1 Brian Users   3, 0 Mar 12 22:23 /dev/conin
> crw-rw-rw- 1 Brian Users   3, 0 Mar 12 22:23 /dev/conout
> crw-rw-rw- 1 Brian Users   3, 0 Mar 12 22:23 /dev/console
> crw--w---- 1 Brian Users 136, 0 Mar 12 22:23 /dev/pty0

Try the same from within a console:

$ ls -l /dev/ | egrep con\|pty
crw-rw-rw-  4 corinna vinschen   3,   0 Mar 13 10:37 conin
crw-rw-rw-  4 corinna vinschen   3,   0 Mar 13 10:37 conout
crw-rw-rw-  4 corinna vinschen   3,   0 Mar 13 10:37 cons0
crw-rw-rw-  4 corinna vinschen   3,   0 Mar 13 10:37 console
crw--w----  1 corinna vinschen 136,   0 Mar 13 10:37 pty0
[~](64)$ ls -l /dev/{con,pty}*
crw-rw-rw- 4 corinna vinschen   3, 0 Mar 13 10:37 /dev/conin
crw-rw-rw- 4 corinna vinschen   3, 0 Mar 13 10:37 /dev/conout
crw-rw-rw- 4 corinna vinschen   3, 0 Mar 13 10:37 /dev/cons0
crw-rw-rw- 4 corinna vinschen   3, 0 Mar 13 10:37 /dev/console
crw--w---- 1 corinna vinschen 136, 0 Mar 13 10:37 /dev/pty0

The device numbers for conin/out/sole are different from a pty
because the 5,X device numbers are the offical device numbers
of these devices.  But these files have no further meaning from
inside a pty.

As soon as you are inside a console, the conin/out/sole devices
are redirected to your actual consX, which has a device number
3,X and all of them are connected now to a real device.

> [...]
> Cygwin /proc/PID/stat[$7] appears to have major in upper half, minor in lower
> half e.g. 8912896 -> 00880000 -> 136,0; 196608 -> 00030000 -> 3,0.
> Linux man 5 proc defines:
> (7) tty_nr  %d
> 	The controlling terminal of the process. (The minor device number is
> 	contained in the combination of bits 31 to 20 and 7 to 0; the major
> 	device number is in bits 15 to 8.)

Thanks.  I changed the output of /proc/<PID>/stat accordingly, but
now `procps' always shows a question mark rather than a pty number,
so that's not the problem:

https://paste.fedoraproject.org/paste/BqENX~bhFzNkYQpa-lwWMw/raw


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: bug in procps-ng

[Achim Gratz]

Corinna Vinschen writes:
>> [...]
>> Cygwin /proc/PID/stat[$7] appears to have major in upper half, minor in lower
>> half e.g. 8912896 -> 00880000 -> 136,0; 196608 -> 00030000 -> 3,0.
>> Linux man 5 proc defines:
>> (7) tty_nr  %d
>> 	The controlling terminal of the process. (The minor device number is
>> 	contained in the combination of bits 31 to 20 and 7 to 0; the major
>> 	device number is in bits 15 to 8.)
>
> Thanks.  I changed the output of /proc/<PID>/stat accordingly, but
> now `procps' always shows a question mark rather than a pty number,
> so that's not the problem:
>
> https://paste.fedoraproject.org/paste/BqENX~bhFzNkYQpa-lwWMw/raw

That was an exercise in futility.  Procps uses system macros to
dissemble the major/minor devices, so of course it's doing the right
thing on Cygwin while using Cygwin macros (I think these are actually
inline functions now).  But your assertion that it should use stat for
finding the tty is probably wrong, there's Cygwin specific code that
looks at /dev/ctty:

https://gitlab.com/procps-ng/procps/blob/master/proc/devname.c#L303

That's used in addition to code that would correctly determine the
devices fromt the stat info, so at the moment I have no idea where the
wheels fall off that wagon.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Factory and User Sound Singles for Waldorf rackAttack:
http://Synth.Stromeko.net/Downloads.html#WaldorfSounds

Re: bug in procps-ng

[Achim Gratz]

Achim Gratz writes:
> That was an exercise in futility.  Procps uses system macros to
> dissemble the major/minor devices, so of course it's doing the right
> thing on Cygwin while using Cygwin macros (I think these are actually
> inline functions now).

Actually, it didn't -- but I patched it to do so now.

We should think a bit about whether or not Cygwin should conform to the
/proc/…/stat definition of Linux with it's peculiar bit allotment for
the major/minor device numbers, in which case I'll have to change to a
versioned #ifdef (I don't particularly want to use a runtime version
check).

> But your assertion that it should use stat for
> finding the tty is probably wrong, there's Cygwin specific code that
> looks at /dev/ctty:
>
> https://gitlab.com/procps-ng/procps/blob/master/proc/devname.c#L303
>
> That's used in addition to code that would correctly determine the
> devices fromt the stat info, so at the moment I have no idea where the
> wheels fall off that wagon.

I still don't know where the bug is in the current package, as just
building the current version (without my patch that removes the function
that uses /proc/ctty) also works correctly.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Wavetables for the Terratec KOMPLEXER:
http://Synth.Stromeko.net/Downloads.html#KomplexerWaves

Re: bug in procps-ng

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 1791 bytes --]

On Mar 14 21:25, Achim Gratz wrote:
> Achim Gratz writes:
> > That was an exercise in futility.  Procps uses system macros to
> > dissemble the major/minor devices, so of course it's doing the right
> > thing on Cygwin while using Cygwin macros (I think these are actually
> > inline functions now).
> 
> Actually, it didn't -- but I patched it to do so now.
> 
> We should think a bit about whether or not Cygwin should conform to the
> /proc/…/stat definition of Linux with it's peculiar bit allotment for
> the major/minor device numbers, in which case I'll have to change to a
> versioned #ifdef (I don't particularly want to use a runtime version
> check).

Here's my stance:

- We can keep the code as is, or change that to Linux-compat, whatever
  you think is the right thing for procps-ng.

- Whatever you decide, the result will go into the next Cygwin version.

- When I release the next Cygwin version, you update procps-ng and
  there won't be any reason to be backward compat.

Ideally: Decide today, and Cygwin 3.0.4 will be ready and released this
weekend.

> > But your assertion that it should use stat for
> > finding the tty is probably wrong, there's Cygwin specific code that
> > looks at /dev/ctty:
> >
> > https://gitlab.com/procps-ng/procps/blob/master/proc/devname.c#L303
> >
> > That's used in addition to code that would correctly determine the
> > devices fromt the stat info, so at the moment I have no idea where the
> > wheels fall off that wagon.
> 
> I still don't know where the bug is in the current package, as just
> building the current version (without my patch that removes the function
> that uses /proc/ctty) also works correctly.

¯\_(ツ)_/¯


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: bug in procps-ng

[Achim Gratz]

Corinna Vinschen writes:
> - We can keep the code as is, or change that to Linux-compat, whatever
>   you think is the right thing for procps-ng.

Either way is fine for procps-ng, since in both cases there will need to
be some Cygwin specific parts.

> - Whatever you decide, the result will go into the next Cygwin version.

OK, then I propose to leave things as they are.  If anybody finds some
other application that presents a compelling reason for following Linux
more closely we can still change it then.

> - When I release the next Cygwin version, you update procps-ng and
>   there won't be any reason to be backward compat.

No change, no requirement for a synchronized release.  :-)

> Ideally: Decide today, and Cygwin 3.0.4 will be ready and released this
> weekend.

Great!  I'll need to check if I can move my patches into the the
configury where they properly belong quickly enough, otherwise I'll
upload as soon I get the upload rights.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

SD adaptations for KORG EX-800 and Poly-800MkII V0.9:
http://Synth.Stromeko.net/Downloads.html#KorgSDada

Re: bug in procps-ng

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 649 bytes --]

On Mar 15 17:05, Achim Gratz wrote:
> Corinna Vinschen writes:
> > - We can keep the code as is, or change that to Linux-compat, whatever
> >   you think is the right thing for procps-ng.
> 
> Either way is fine for procps-ng, since in both cases there will need to
> be some Cygwin specific parts.
> 
> > - Whatever you decide, the result will go into the next Cygwin version.
> 
> OK, then I propose to leave things as they are.  If anybody finds some
> other application that presents a compelling reason for following Linux
> more closely we can still change it then.

ACK


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]