Subject: Re: tcsh

From: Thomas Bogendoerfer <[email-masked]>
Date: Fri, 3 Jul 1998 00:59:27 +0200

On Mon, Jun 22, 1998 at 11:01:39AM +0200, [email-masked] wrote:
> maybe anybody feels like debugging the tcsh rpm.  The lack of having
> a properly working csh keep several other packages from building
> without trickery.

ok, I'm pretty close. After debugging tcsh for more than three hours,
it looks like this is a kernel bug. I've traced it down to the following
code in sh.proc.c:

xprintf ("pp before sigpause %x\n",pp);
        /* (void) sigpause(sigblock((sigmask_t) 0) &~ sigmask(SIGCHLD)); */
        (void) sigpause(omask & ~sigmask(SIGCHLD));
xprintf ("pp after sigpause %x\n",pp);

pp gets clobbered by sigpause. I'll have a fast look at the kernel, maybe
it's easy to spot the bug.

Thomas.

-- 
See, you not only have to be a good coder to create a system like Linux,
you have to be a sneaky bastard too ;-)
                   [Linus Torvalds in <[email-masked]>]

---

From: [email-masked]
Date: Fri, 3 Jul 1998 16:58:55 +0200

> it's easy to spot the bug.

Same result here, I found that even minimal modifications make the
sympthoms go away.

Sigpause() is a libc routine in libc/sysdeps/posix/sigpause.c; it's either
using sigprocmask(2) or sigsuspend(2).

  Ralf

---

From: Thomas Bogendoerfer <[email-masked]>
Date: Sat, 4 Jul 1998 00:37:29 +0200

> using sigprocmask(2) or sigsuspend(2).

it's sigsuspend. And after looking at scall_o32.S and realizing that
calling do_signal() needs to have the static registers saved/restored,
the bug is obvious (I also had a look at the Alpha sys_sigsuspend). Below
is a patch, which fixes tcsh and other programs, which use sigsupend.
If everybody agrees with the patch, I'll check it in.

Thomas.

Index: scall_o32.S
===================================================================
RCS file: /var/mips/linus/cvs/linux/arch/mips/kernel/scall_o32.S,v
retrieving revision 1.3
diff -u -r1.3 scall_o32.S
--- scall_o32.S	1998/03/27 04:47:55	1.3
+++ scall_o32.S	1998/07/03 22:32:56
@@ -98,6 +98,18 @@
 	jal	schedule
 	b	o32_ret_from_sys_call
 
+EXPORT(sys_sigsuspend)
+	SAVE_STATIC
+	jal	do_sigsuspend
+	RESTORE_STATIC
+	b	o32_ret_from_sys_call
+
+EXPORT(sys_rt_sigsuspend)
+	SAVE_STATIC
+	jal	do_rt_sigsuspend
+	RESTORE_STATIC
+	b	o32_ret_from_sys_call
+
 /* ------------------------------------------------------------------------ */
 
 trace_a_syscall:
Index: signal.c
===================================================================
RCS file: /var/mips/linus/cvs/linux/arch/mips/kernel/signal.c,v
retrieving revision 1.12
diff -u -r1.12 signal.c
--- signal.c	1998/04/05 11:23:53	1.12
+++ signal.c	1998/07/03 22:31:58
@@ -38,8 +38,8 @@
 /*
  * Atomically swap in the new signal mask, and wait for a signal.
  */
-asmlinkage inline int
-sys_sigsuspend(struct pt_regs regs)
+int
+do_sigsuspend(struct pt_regs regs)
 {
 	sigset_t *uset, saveset, newset;
 
@@ -62,8 +62,8 @@
 	}
 }
 
-asmlinkage int
-sys_rt_sigsuspend(struct pt_regs regs)
+int
+do_rt_sigsuspend(struct pt_regs regs)
 {
 	sigset_t *uset, saveset, newset;

---

From: [email-masked]
Date: Sat, 4 Jul 1998 01:14:56 +0200

> If everybody agrees with the patch, I'll check it in.

I've checked in a slightly different patch.  I already had to deal with
the problem of saving these registers for several other routines, so there
is an inline function named save_static to do that job.  Also it saves us
some cycles and looks a bit more beautyful.  Patch appended below.

  Ralf

Index: arch/mips/kernel/signal.c
===================================================================
RCS file: /src/ftp/cvs/linux/arch/mips/kernel/signal.c,v
retrieving revision 1.12
diff -u -r1.12 signal.c
--- signal.c	1998/04/05 11:23:53	1.12
+++ signal.c	1998/07/03 23:04:42
@@ -43,6 +43,7 @@
 {
 	sigset_t *uset, saveset, newset;
 
+	save_static(&regs);
 	uset = (sigset_t *) regs.regs[4];
 	if (copy_from_user(&newset, uset, sizeof(sigset_t)))
 		return -EFAULT;
@@ -67,6 +68,7 @@
 {
 	sigset_t *uset, saveset, newset;
 
+	save_static(&regs);
 	uset = (sigset_t *) regs.regs[4];
 	if (copy_from_user(&newset, uset, sizeof(sigset_t)))
 		return -EFAULT;