[PATCH, RFC] fix UP futex code to not generate invalid streqt instruction

[Mikael Pettersson]

Compiling kernel 3.0 for UP ARM (ARMv5) I see:

kernel/futex.c: In function 'fixup_pi_state_owner':
kernel/futex.c:1549: warning: 'curval' may be used uninitialized in this function
kernel/futex.c: In function 'handle_futex_death':
kernel/futex.c:2454: warning: 'nval' may be used uninitialized in this function
/tmp/ccVDjh0q.s: Assembler messages:
/tmp/ccVDjh0q.s:113: Warning: source register same as write-back base

The corresponding instruction is:

2:	streqt  r3, [r3]

(same as strteq) which the ARMv5 ARM ARM describes as being UNPREDICTABLE.

This code originates from futex_atomic_cmpxchg_inatomic():

static inline int
futex_atomic_cmpxchg_inatomic(u32 *uval, u32 __user *uaddr,
                              u32 oldval, u32 newval)
{
        int ret = 0;
        u32 val;

        if (!access_ok(VERIFY_WRITE, uaddr, sizeof(u32)))
                return -EFAULT;

        __asm__ __volatile__("@futex_atomic_cmpxchg_inatomic\n"
        "1:     " T(ldr) "      %1, [%4]\n"
        "       teq     %1, %2\n"
        "       it      eq      @ explicit IT needed for the 2b label\n"
        "2:     " T(streq) "    %3, [%4]\n"
        __futex_atomic_ex_table("%5")
        : "+r" (ret), "=&r" (val)
        : "r" (oldval), "r" (newval), "r" (uaddr), "Ir" (-EFAULT)
        : "cc", "memory");

        *uval = val;
        return ret;
}

The problem is that in the T(streq) insn, %3 and %4 MUST be different registers,
but nothing in the asm() constrains them to be different.  kernel/futex.c:init_futex()
calls this with uaddr, oldval, and newval all zero, and the compiler may allocate
all three to the same register.  That's exactly what happens for me with a gcc-4.4.6
based compiler and the 2.6.39 and 3.0 kernels. (It didn't happen with 2.6.38.)

One way of fixing this is to make uaddr an input/output register, since that prevents
it from overlapping any other input or output.  The patch below does exactly that;
on the problematic fragment in futex.c it causes the following change:

@@ -104,13 +104,14 @@ futex_init:
        mvnne   r3, #13
        bne     .L6
        mvn     r2, #13
+       mov     r0, r3
 #APP
 @ 97 "/tmp/linux-3.0/arch/arm/include/asm/futex.h" 1
        @futex_atomic_cmpxchg_inatomic
-1:     ldrt    r1, [r3]
+1:     ldrt    r1, [r0]
        teq     r1, r3
        it      eq      @ explicit IT needed for the 2b label
-2:     streqt  r3, [r3]
+2:     streqt  r3, [r0]
 3:
        .pushsection __ex_table,"a"
        .align  3

which is pretty much what we want.

Lightly tested on an n2100 (UP, ARMv5).

Does this seem like a reasonable solution, or is there a better way to force
distinct input registers to an asm()?

/Mikael

--- linux-3.0/arch/arm/include/asm/futex.h.~1~  2011-07-22 12:01:07.000000000 +0200
+++ linux-3.0/arch/arm/include/asm/futex.h      2011-07-22 18:31:08.000000000 +0200
@@ -95,13 +95,13 @@ futex_atomic_cmpxchg_inatomic(u32 *uval,
                return -EFAULT;
 
        __asm__ __volatile__("@futex_atomic_cmpxchg_inatomic\n"
-       "1:     " T(ldr) "      %1, [%4]\n"
-       "       teq     %1, %2\n"
+       "1:     " T(ldr) "      %1, [%2]\n"
+       "       teq     %1, %3\n"
        "       it      eq      @ explicit IT needed for the 2b label\n"
-       "2:     " T(streq) "    %3, [%4]\n"
+       "2:     " T(streq) "    %4, [%2]\n"
        __futex_atomic_ex_table("%5")
-       : "+r" (ret), "=&r" (val)
-       : "r" (oldval), "r" (newval), "r" (uaddr), "Ir" (-EFAULT)
+       : "+r" (ret), "=&r" (val), "+r" (uaddr)
+       : "r" (oldval), "r" (newval), "Ir" (-EFAULT)
        : "cc", "memory");
 
        *uval = val;