Re: Bug#755295: Hurd term server (was: Hurd GCC ping)

[Thomas Schwinge]

Hi!

Sorry for the long silence on this.  I'm now writing this while
struggling with a flu, so let's hope that I'm not speaking in "feverish
delirium".

On Sun, 2 Nov 2014 18:13:11 +0100, Samuel Thibault <samuel.thibault@gnu.org> 
wrote:
> Thomas Schwinge, le Thu 09 Oct 2014 14:02:39 +0200, a écrit :
> >     #!/usr/bin/expect -f
> >     
> >     # Doesn't seem to matter.
> >     #stty -cooked
> >     stty cooked
> >     
> >     #spawn sh -c 
> > "/media/erich/home/thomas/tmp/gcc/755295.build/gcc/xgcc.real 
> > -B/media/erich/home/thomas/tmp/gcc/755295.build/gcc/ 
> > /media/erich/home/thomas/tmp/gcc/755295/gcc/testsuite/gcc.dg/cpp/pr33466.c  
> > -fno-diagnostics-show-caret -fdiagnostics-color=never -std=gnu99 -S -o 
> > pr33466.s 2> /tmp/e; cat < /tmp/e"
> >     spawn sh -c "cat < /tmp/e"
> >     #spawn sh -c "for i in \$(seq 1 99); do echo \$i \$(seq 0 50); done > 
> > /tmp/d; cat < /tmp/d"
> >     #spawn sh -c "for i in \$(seq 1 99); do echo \$i 
> > --------------------------------------------------------------------------; 
> > done > /tmp/d; cat < /tmp/d"
> >     #spawn printf "%4095d\r\nabc" 1
> >     #spawn printf "%4096d%4096d\r\nabc" 1 2
> >     interact
> 
> I don't manage to make this behave differently on Linux and on GNU/Hurd.
> Which testcase should I be enabling above?  What is supposed to be the
> content of /tmp/e?

Sorry for not attaching that.  It's the file as generated by the GCC
invocation that is commented out -- which you obviously can't easily
reproduce.  (Though, its content also doesn't matter much.)  The file is
now attached.  Use the test case that is currently enabled.  (Can you now
reproduce the issue?)

> It really seems to me the change I propose is the
> right way, but I haven't yet managed to trigger a failure without it.

Even with your patch applied (which became commit
fa108a1e6f15946aa72cd8edd09f3c08255b76fd), I'm still seeing the
truncation in the expect script's output.

As my current attention span roughly equals that of a mole (nothing
against those animals, of course), I resorted to "patching by the
symptoms" rather than trying to understand Unix terminals.

My steps in some detail, in case that's helpful for anyone.  (Also,
another nice example of how you can debug "core system components" from
user space.)

Build and install a new /hurd/term.  Instead of rebooting, or restarting
them selectively, I opted to "remove" (that is, orphan -- easier than
terminating) all active PTY servers but the one of the current (login)
shell:

    $ (set -x && cur=$(tty) && for pty in /dev/ttyp*; do [ "$pty" = "$cur" ] && 
continue; sudo settrans -ao "$pty"; sudo settrans -ao "${pty/tty/pty}"; done)

Attached as term-debug.patch is the patch I used to gather some internal
state.  (I didn't use GDB, because I a) wanted to get a non-invasive
trace, and b) wanted to occupy as few PTYs as possible with the debugging
setup itself.)

This shows, for the expect script:

    2946: /dev/ptyp1: starting PTY
    2946: /dev/ptyp1: tf 2 qs(oq) 7461
    2946: /dev/ptyp1: pm 0 uim 0 cb 0
    2946: /dev/ptyp1: tf 2 qs(oq) 3365
    2946: /dev/ptyp1: pm 0 uim 0 cb 0
    2946: /dev/ptyp1: tf 0 qs(oq) 0
    2946: /dev/ptyp1: pm 0 uim 0 cb 0

..., and for a screen window exit:

    2970: /dev/ptyp2: tf 2 qs(oq) 6
    2970: /dev/ptyp2: pm 1 uim 0 cb 0
    2970: /dev/ptyp2: tf 0 qs(oq) 0
    2970: /dev/ptyp2: pm 1 uim 0 cb 1

In both cases, the last iteration (starting with "tf") is the problematic
one.  The size of the output queue is zero in both cases, which is why
Samuel's patch doesn't make a difference.  The expect script case is
different from the screen window exit one in that the latter is running
in packet mode.  So here is my patch -- with no rationale by citing any
standards, or similar.  Any comments on that already?  (I don't feel like
committing such a change without understanding it.)

--- term/ptyio.c
+++ term/ptyio.c
@@ -331,7 +331,7 @@ pty_io_read (struct trivfs_protid *cred,
        }
     }
 
-  if (!(termflags & TTY_OPEN) && !qsize (outputq))
+  if (!(termflags & TTY_OPEN) && packet_mode)
     {
       pthread_mutex_unlock (&global_lock);
       return EIO;


Grüße,
 Thomas


/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:8:18: 
error: invalid suffix "rh" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:9:18: 
error: invalid suffix "rl" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:10:19:
 error: invalid suffix "rll" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:11:18:
 error: invalid suffix "kh" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:12:18:
 error: invalid suffix "kl" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:13:19:
 error: invalid suffix "kll" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:14:18:
 error: invalid suffix "ru" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:15:19:
 error: invalid suffix "urh" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:16:19:
 error: invalid suffix "hur" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:17:19:
 error: invalid suffix "hru" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:18:19:
 error: invalid suffix "ruh" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:19:19:
 error: invalid suffix "rhu" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:20:19:
 error: invalid suffix "url" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:21:19:
 error: invalid suffix "lur" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:22:19:
 error: invalid suffix "lru" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:23:19:
 error: invalid suffix "rul" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:24:19:
 error: invalid suffix "rlu" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:25:20:
 error: invalid suffix "urll" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:26:20:
 error: invalid suffix "llur" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:27:20:
 error: invalid suffix "llru" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:28:20:
 error: invalid suffix "rull" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:29:20:
 error: invalid suffix "rllu" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:30:18:
 error: invalid suffix "ku" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:31:19:
 error: invalid suffix "ukh" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:32:19:
 error: invalid suffix "huk" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:33:19:
 error: invalid suffix "hku" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:34:19:
 error: invalid suffix "kuh" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:35:19:
 error: invalid suffix "khu" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:36:19:
 error: invalid suffix "ukl" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:37:19:
 error: invalid suffix "luk" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:38:19:
 error: invalid suffix "lku" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:39:19:
 error: invalid suffix "kul" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:40:19:
 error: invalid suffix "klu" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:41:20:
 error: invalid suffix "ukll" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:42:20:
 error: invalid suffix "lluk" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:43:20:
 error: invalid suffix "llku" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:44:20:
 error: invalid suffix "kull" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:45:20:
 error: invalid suffix "kllu" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:46:18:
 error: invalid suffix "ld" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:47:18:
 error: invalid suffix "fd" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:48:18:
 error: invalid suffix "dk" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:49:18:
 error: invalid suffix "dr" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:50:19:
 error: invalid suffix "ddw" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:51:19:
 error: invalid suffix "ddq" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:52:19:
 error: invalid suffix "ddl" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:53:19:
 error: invalid suffix "ddf" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:54:19:
 error: invalid suffix "ddd" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:55:18:
 error: invalid suffix "dw" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:56:18:
 error: invalid suffix "dq" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:57:18:
 error: invalid suffix "wd" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:58:18:
 error: invalid suffix "qd" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:59:19:
 error: invalid suffix "wdd" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:60:19:
 error: invalid suffix "qdd" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:61:19:
 error: invalid suffix "ldd" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:62:19:
 error: invalid suffix "fdd" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:63:19:
 error: invalid suffix "ddi" on floating constant
/media/erich/home/thomas/tmp/gcc/trunk/gcc/testsuite/gcc.dg/cpp/pr33466.c:64:19:
 error: invalid suffix "idd" on floating constant

diff --git term/Makefile term/Makefile
index 5006c0d..f75f34b 100644
--- term/Makefile
+++ term/Makefile
@@ -27,7 +27,7 @@ SRCS = devio.c munge.c users.c main.c ptyio.c hurdio.c xinl.c
 
 HURDLIBS = trivfs fshelp iohelp ports ihash shouldbeinlibc
 OTHERLIBS = -lpthread
-OBJS = $(subst .c,.o,$(SRCS)) termServer.o device_replyServer.o tioctlServer.o 
ourmsgUser.o
+OBJS = $(subst .c,.o,$(SRCS)) termServer.o device_replyServer.o tioctlServer.o 
ourmsgUser.o mach_print.o
 
 include ../Makeconf
 
diff --git term/mach_print.S term/mach_print.S
new file mode 100644
index 0000000..606a66e
--- /dev/null
+++ term/mach_print.S
@@ -0,0 +1,3 @@
+#include <mach/machine/syscall_sw.h>
+
+kernel_trap(mach_print,-30,1)
diff --git term/main.c term/main.c
index 9cc32d4..0d86bad 100644
--- term/main.c
+++ term/main.c
@@ -32,6 +32,23 @@
 
 #include <version.h>
 
+extern void mach_print(const char *);
+
+void mach_print_(const char *msg)
+{
+  static char buf[1024];
+  if (buf[0] == '\0')
+    {
+      extern int getpid(void);
+      extern char *tty_name;
+      snprintf(buf, sizeof buf, "%u: %s: ",
+              getpid (), tty_name);
+      buf[sizeof buf - 1] = '\0';
+    }
+  mach_print(buf);
+  mach_print(msg);
+}
+
 const char *argp_program_version = STANDARD_HURD_VERSION (term);
 
 int trivfs_fstype = FSTYPE_TERM;
@@ -368,6 +385,9 @@ main (int argc, char **argv)
       return 1;
     }
 
+  if (peerclass)
+    mach_print_("starting PTY\n");
+
   task_get_bootstrap_port (mach_task_self (), &bootstrap);
 
   if (bootstrap == MACH_PORT_NULL)
diff --git term/ptyio.c term/ptyio.c
index 172845d..8eb90fc 100644
--- term/ptyio.c
+++ term/ptyio.c
@@ -26,6 +26,9 @@
 #include "term.h"
 #include "tioctl_S.h"
 
+#include <stdio.h>
+extern void mach_print_(const char *);
+
 /* Set if we need a wakeup when tty output has been done */
 static int pty_read_blocked = 0;
 
@@ -308,6 +311,20 @@ pty_io_read (struct trivfs_protid *cred,
 
   pthread_mutex_lock (&global_lock);
 
+  {
+    char buf[1024];
+
+    snprintf(buf, sizeof buf, "tf %lx qs(oq) %d\n",
+            termflags, qsize(outputq));
+    buf[sizeof buf - 1] = '\0';
+    mach_print_(buf);
+
+    snprintf(buf, sizeof buf, "pm %x uim %x cb %x\n",
+            packet_mode, user_ioctl_mode, control_byte);
+    buf[sizeof buf - 1] = '\0';
+    mach_print_(buf);
+  }
+
   if ((cred->po->openmodes & O_READ) == 0)
     {
       pthread_mutex_unlock (&global_lock);

signature.asc
Description: PGP signature