Subject: Re: Illegal instruction - a workaround or fix ?

From: Pete Popov <[email-masked]>
Date: Fri, 20 Apr 2001 12:05:03 -0700

Florian Lohoff wrote:
> 
> On Mon, Mar 12, 2001 at 12:21:34PM +0100, Ralf Baechle wrote:
> > Thanks, that was the hint I needed.  o32_ret_from_sys_call expects the
> > content of s-registers to be unchanged from userspace but sys_sysmips
> > clobbers them.
> >
> > Below a patch from the famous ``Smoke This, It's Good For You (TM)''
> > series.  Lemme know if it helps.
> 
> As mentioned on IRC - This "Oopses" for me ...

I'm bringing this up again because none of the related patches on this
topic have been applied to the latest cvs kernel.  The patch Florian
refers to above oopses for me as well.  This patch below, from Florian,
but updated against the latest cvs kernel, works (at least the few
simple tests I've run do work now).  

--- arch/mips/kernel/sysmips.c.old      Fri Apr 20 11:58:38 2001
+++ arch/mips/kernel/sysmips.c  Fri Apr 20 11:59:59 2001
@@ -99,7 +99,7 @@
                        ".word\t1b, 3b\n\t"
                        ".word\t2b, 3b\n\t"
                        ".previous\n\t"
-                       : "=&r" (tmp), "=o" (* (u32 *) p), "=r" (errno)
+                       : "=&r" (retval), "=o" (* (u32 *) p), "=r"
(errno)
                        : "r" (arg2), "o" (* (u32 *) p), "2" (errno)
                        : "$1");
 
@@ -110,14 +110,7 @@
                if (current->ptrace & PT_TRACESYS)
                        syscall_trace();
 
-               ((struct pt_regs *)&cmd)->regs[2] = tmp;
-               ((struct pt_regs *)&cmd)->regs[7] = 0;
-
-               __asm__ __volatile__(
-                       "move\t$29, %0\n\t"
-                       "j\to32_ret_from_sys_call"
-                       : /* No outputs */
-                       : "r" (&cmd));
+               goto out;
                /* Unreached */
 #else
        printk("sys_sysmips(MIPS_ATOMIC_SET, ...) not ready for
!CONFIG_CPU_HAS_LLSC\n");

---

From: Ralf Baechle <[email-masked]>
Date: Fri, 20 Apr 2001 16:18:20 -0300

> simple tests I've run do work now).  

This patch is broken for small negative numbers in the memory.

Florian now has a better patch that should be correct.

  Ralf

---

From: Pete Popov <[email-masked]>
Date: Fri, 20 Apr 2001 12:19:00 -0700

> Florian now has a better patch that should be correct.

Are you talking about the fast_sysmips patch Florian posted? 

Florian, if you have a newer patch than what you last posted on the
mailing list, would you mind sending it?

Thanks,

Pete

---

From: Florian Lohoff <[email-masked]>
Date: Fri, 20 Apr 2001 21:58:41 +0200

> simple tests I've run do work now).  

The patch is definitly only a workaround and not a fix - The problem
is that the sysmips saves registers on the stack in the function
prolog. When taking the shortcut exit which is needed to not garble
the return code which can be "unsigned int" and not interpret it
as an -ESOMETHING. When doing this you need to skip a couple of instructions in
scall_o32.S which makes the shortcut necessary. On the shortcut
the compiler does not generate a correct epilgue (How should it know)
so the registers get garbled in the syscall which lets your programs
die.

The asm variant tries to only touch registers already saved in
scall_o32.S (and also restored on exit) which are not anymore
used or register we are allowed to change anyway (caller saved).

A different solution would be to take the usual exit in sysmips via
the return at the end (for which the compiler generated a correct
epilogue) and modify the return address - This is an very ugly hack
and you cant tell where the compiler stores the ra on the stack.

Flo
-- 
Florian Lohoff                  [email-masked]             +49-5201-669912
     Why is it called "common sense" when nobody seems to have any?

---

From: Florian Lohoff <[email-masked]>
Date: Fri, 20 Apr 2001 21:48:32 +0200

> mailing list, would you mind sending it?

I havent got anything newer - I am using that patch now on 2 machines
running continuesly as a build machine open in the net. Nobody
yet has reported any problems (Which isnt a good indicator)

I am not shure if it is really correct so someone with deep knowledge
of the inners of sysmips should check if its really ok.

Nevertheless - It works for me ...

Flo

---

From: "Maciej W. Rozycki" <[email-masked]>
Date: Mon, 30 Apr 2001 11:34:32 +0200 (MET DST)

> and you cant tell where the compiler stores the ra on the stack.

 It could be doable with __builtin_frame_address().  Haven't investigated
it further, though. 

-- 
+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: [email-masked], PGP key available        +

---

From: Ralf Baechle <[email-masked]>
Date: Mon, 30 Apr 2001 17:24:19 -0300

>  It could be doable with __builtin_frame_address().  Haven't investigated

MIPS ABI doesn't define that ra gets stored at a constant offset in
the stackframe, so that won't work.

  Ralf