Linux MIPS Architecture development
 help / color / mirror / Atom feed
* a REALLY, REALLY nasty bug
@ 2000-10-26  3:03 Jun Sun
  2000-10-26 23:38 ` Ralf Baechle
  0 siblings, 1 reply; 3+ messages in thread
From: Jun Sun @ 2000-10-26  3:03 UTC (permalink / raw)
  To: linux-mips

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


Nasty degree - 3 days of tracking.

The symptom was pthread cannot be created.  In the end the caller will
get a BUS error.

What exactly happened has to do with how registers are saved.  Below
attached is the beginning part of sys_sigsuspend() function.  It is easy
to see that s0 is saved into stack frame AFTER its modified.  Next time
when process returns to userland, the s0 reg will be wrong!

So the bug is either 

1) that we need to save s0 register in SAVE_SOME and not save it in
save_static; or that

2) we fix compiler so that it does not use s0 register in that case (it
does the same thing for sys_rt_sigsuspend)

I am sure Ralf will have something to say about it.  :-)  In any case, I
attached a patch for 1) fix.

Jun

------------



sys_sigsuspend(struct pt_regs regs)
{
    8008e280:   27bdffc0        addiu   $sp,$sp,-64
    8008e284:   afb00030        sw      $s0,48($sp)
        sigset_t *uset, saveset, newset;

        save_static(&regs);
    8008e288:   27b00040        addiu   $s0,$sp,64
    8008e28c:   afbf003c        sw      $ra,60($sp)
    8008e290:   afb20038        sw      $s2,56($sp)
    8008e294:   afb10034        sw      $s1,52($sp)
    8008e298:   afa40040        sw      $a0,64($sp)
    8008e29c:   afa50044        sw      $a1,68($sp)
    8008e2a0:   afa60048        sw      $a2,72($sp)
    8008e2a4:   afa7004c        sw      $a3,76($sp)
    8008e2a8:   ae100058        sw      $s0,88($s0)
    8008e2ac:   ae11005c        sw      $s1,92($s0)
    .....

[-- Attachment #2: junk --]
[-- Type: text/plain, Size: 1032 bytes --]

diff -Nru linux/include/asm-mips/stackframe.h.orig linux/include/asm-mips/stackframe.h
--- linux/include/asm-mips/stackframe.h.orig    Wed Oct 25 19:45:50 2000
+++ linux/include/asm-mips/stackframe.h Wed Oct 25 19:47:54 2000
@@ -49,7 +49,6 @@
 
 #define save_static(frame)                               \
        __asm__ __volatile__(                            \
-               "sw\t$16,"__str(PT_R16)"(%0)\n\t"        \
                "sw\t$17,"__str(PT_R17)"(%0)\n\t"        \
                "sw\t$18,"__str(PT_R18)"(%0)\n\t"        \
                "sw\t$19,"__str(PT_R19)"(%0)\n\t"        \
@@ -90,6 +89,7 @@
                mfc0    v1, CP0_EPC;                     \
                sw      $7, PT_R7(sp);                   \
                sw      v1, PT_EPC(sp);                  \
+               sw      $16, PT_R16(sp);                 \
                sw      $25, PT_R25(sp);                 \
                sw      $28, PT_R28(sp);                 \
                sw      $31, PT_R31(sp);                 \


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: a REALLY, REALLY nasty bug
  2000-10-26  3:03 a REALLY, REALLY nasty bug Jun Sun
@ 2000-10-26 23:38 ` Ralf Baechle
  2000-10-27  1:57   ` Jun Sun
  0 siblings, 1 reply; 3+ messages in thread
From: Ralf Baechle @ 2000-10-26 23:38 UTC (permalink / raw)
  To: Jun Sun; +Cc: linux-mips

On Wed, Oct 25, 2000 at 08:03:20PM -0700, Jun Sun wrote:

> I am sure Ralf will have something to say about it.  :-)  In any case, I
> attached a patch for 1) fix.

A fix that is less easily affected by compiler overoptmizations is contained
in 2.2; I'll merge it forward into 2.4.  Dunno how that didn't make it
into 2.4.

Time for a brown paper bag.

  Ralf

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: a REALLY, REALLY nasty bug
  2000-10-26 23:38 ` Ralf Baechle
@ 2000-10-27  1:57   ` Jun Sun
  0 siblings, 0 replies; 3+ messages in thread
From: Jun Sun @ 2000-10-27  1:57 UTC (permalink / raw)
  To: Ralf Baechle; +Cc: linux-mips

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

Ralf Baechle wrote:
> 
> On Wed, Oct 25, 2000 at 08:03:20PM -0700, Jun Sun wrote:
> 
> > I am sure Ralf will have something to say about it.  :-)  In any case, I
> > attached a patch for 1) fix.
> 
> A fix that is less easily affected by compiler overoptmizations is contained
> in 2.2; I'll merge it forward into 2.4.  Dunno how that didn't make it
> into 2.4.
> 

I took a look of what v2.2 does.  It does look better than my hack. 
Just verified that it worked on my machine.  I attached the patch - if
you want to save some time.
 
> Time for a brown paper bag.
>

You mean time for beer and pork feet?
 
Jun

[-- Attachment #2: junk --]
[-- Type: text/plain, Size: 4609 bytes --]

diff -Nru linux/include/asm-mips/stackframe.h.orig linux/include/asm-mips/stackframe.h
--- linux/include/asm-mips/stackframe.h.orig	Wed Oct 25 19:45:50 2000
+++ linux/include/asm-mips/stackframe.h	Thu Oct 26 18:34:50 2000
@@ -47,19 +47,28 @@
 #define __str2(x) #x
 #define __str(x) __str2(x)
 
-#define save_static(frame)                               \
-	__asm__ __volatile__(                            \
-		"sw\t$16,"__str(PT_R16)"(%0)\n\t"        \
-		"sw\t$17,"__str(PT_R17)"(%0)\n\t"        \
-		"sw\t$18,"__str(PT_R18)"(%0)\n\t"        \
-		"sw\t$19,"__str(PT_R19)"(%0)\n\t"        \
-		"sw\t$20,"__str(PT_R20)"(%0)\n\t"        \
-		"sw\t$21,"__str(PT_R21)"(%0)\n\t"        \
-		"sw\t$22,"__str(PT_R22)"(%0)\n\t"        \
-		"sw\t$23,"__str(PT_R23)"(%0)\n\t"        \
-		"sw\t$30,"__str(PT_R30)"(%0)\n\t"        \
-		: /* No outputs */                       \
-		: "r" (frame))
+#define save_static_function(symbol)                                    \
+__asm__ (                                                               \
+        ".globl\t" #symbol "\n\t"                                       \
+        ".align\t2\n\t"                                                 \
+        ".type\t" #symbol ", @function\n\t"                             \
+        ".ent\t" #symbol ", 0\n"                                        \
+        #symbol":\n\t"                                                  \
+        ".frame\t$29, 0, $31\n\t"                                       \
+        "sw\t$16,"__str(PT_R16)"($29)\t\t\t# save_static_function\n\t"  \
+        "sw\t$17,"__str(PT_R17)"($29)\n\t"                              \
+        "sw\t$18,"__str(PT_R18)"($29)\n\t"                              \
+        "sw\t$19,"__str(PT_R19)"($29)\n\t"                              \
+        "sw\t$20,"__str(PT_R20)"($29)\n\t"                              \
+        "sw\t$21,"__str(PT_R21)"($29)\n\t"                              \
+        "sw\t$22,"__str(PT_R22)"($29)\n\t"                              \
+        "sw\t$23,"__str(PT_R23)"($29)\n\t"                              \
+        "sw\t$30,"__str(PT_R30)"($29)\n\t"                              \
+        ".end\t" #symbol "\n\t"                                         \
+        ".size\t" #symbol",. - " #symbol)
+
+/* Used in declaration of save_static functions.  */
+#define unused __attribute__((unused))
 
 #define SAVE_SOME                                        \
 		.set	push;                            \
@@ -90,6 +99,7 @@
 		mfc0	v1, CP0_EPC;                     \
 		sw	$7, PT_R7(sp);                   \
 		sw	v1, PT_EPC(sp);                  \
+		sw	$16, PT_R16(sp);                 \
 		sw	$25, PT_R25(sp);                 \
 		sw	$28, PT_R28(sp);                 \
 		sw	$31, PT_R31(sp);                 \
diff -Nru linux/arch/mips/kernel/signal.c.orig linux/arch/mips/kernel/signal.c
--- linux/arch/mips/kernel/signal.c.orig	Thu Oct 26 18:36:54 2000
+++ linux/arch/mips/kernel/signal.c	Thu Oct 26 18:42:01 2000
@@ -76,12 +76,12 @@
 /*
  * Atomically swap in the new signal mask, and wait for a signal.
  */
-asmlinkage inline int
-sys_sigsuspend(struct pt_regs regs)
+save_static_function(sys_sigsuspend);
+static unused int
+_sys_sigsuspend(struct pt_regs regs)
 {
 	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;
@@ -103,13 +103,13 @@
 	}
 }
 
-asmlinkage int
-sys_rt_sigsuspend(struct pt_regs regs)
+
+save_static_function(sys_rt_sigsuspend);
+static unused int
+_sys_rt_sigsuspend(struct pt_regs regs)
 {
 	sigset_t *unewset, saveset, newset;
         size_t sigsetsize;
-
-	save_static(&regs);
 
 	/* XXX Don't preclude handling different sized sigset_t's.  */
 	sigsetsize = regs.regs[5];
diff -Nru linux/arch/mips/kernel/syscall.c.orig linux/arch/mips/kernel/syscall.c
--- linux/arch/mips/kernel/syscall.c.orig	Thu Oct 26 18:43:10 2000
+++ linux/arch/mips/kernel/syscall.c	Thu Oct 26 18:47:00 2000
@@ -92,22 +92,22 @@
 	return do_mmap2(addr, len, prot, flags, fd, pgoff);
 }
 
-asmlinkage int sys_fork(struct pt_regs regs)
+save_static_function(sys_fork);
+static unused int _sys_fork(struct pt_regs regs)
 {
 	int res;
 
-	save_static(&regs);
 	res = do_fork(SIGCHLD, regs.regs[29], &regs);
 	return res;
 }
 
-asmlinkage int sys_clone(struct pt_regs regs)
+save_static_function(sys_clone);
+static unused int _sys_clone(struct pt_regs regs)
 {
 	unsigned long clone_flags;
 	unsigned long newsp;
 	int res;
 
-	save_static(&regs);
 	clone_flags = regs.regs[4];
 	newsp = regs.regs[5];
 	if (!newsp)

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2000-10-27  1:56 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2000-10-26  3:03 a REALLY, REALLY nasty bug Jun Sun
2000-10-26 23:38 ` Ralf Baechle
2000-10-27  1:57   ` Jun Sun

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox