port-arm/48635: ARM atomic_cas_64() probably not working

[dennis.c.ferguson%gmail.com@localhost]

>Number:         48635
>Category:       port-arm
>Synopsis:       ARM atomic_cas_64() probably not working
>Confidential:   no
>Severity:       serious
>Priority:       low
>Responsible:    port-arm-maintainer
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Mar 04 03:30:00 +0000 2014
>Originator:     Dennis Ferguson
>Release:        6.99.33
>Organization:
none at all
>Environment:
NetBSD beagle10.to.akit-ferguson.com 6.99.31 NetBSD 6.99.31 (BEAGLEBONE) #1: 
Sun Feb 16 03:15:09 EST 2014  
root%b1.to.mistimed.ca@localhost:/build/beagle/obj/sys/arch/evbarm/compile/BEAGLEBONE
 evbarm

>Description:
In common/lib/libc/arch/arm/atomic/atomic_cas_64.S, the start
of the function pushes 3 registers onto the stack and then
loads the third function argument from the stack into 2 of
those registers, i.e.

ENTRY_NP(_atomic_cas_64)
        push    {r4, r5, r6}            /* save temporaries */
        mov     ip, r0                  /* we need r0 for return value */
#ifdef __ARM_EABI__
        ldrd    r4, r5, [sp]            /* fetch new value */
#else
        ldr     r4, [sp, #0]            /* fetch new value */
        ldr     r5, [sp, #4]            /* fetch new value */

The problem is that while "new value" was at the top (bottom?)
of the stack when the function was entered it is 12 bytes offset
from there by the time the code loads it.

I also noticed that every other function in source files
in this directory pushes an even number of registers on the stack,
even at the expense of unnecessarily saving an extra register,
but this one only pushes 3 registers.  I don't know if that's
a problem or not; the patch below assumes it isn't.

>How-To-Repeat:
Noticed by inspection.
>Fix:
Index: common/lib/libc/arch/arm/atomic/atomic_cas_64.S
===================================================================
RCS file: /cvsroot/src/common/lib/libc/arch/arm/atomic/atomic_cas_64.S,v
retrieving revision 1.5
diff -u -r1.5 atomic_cas_64.S
--- common/lib/libc/arch/arm/atomic/atomic_cas_64.S     30 Nov 2013 21:09:11 
-0000      1.5
+++ common/lib/libc/arch/arm/atomic/atomic_cas_64.S     4 Mar 2014 03:15:40 
-0000
@@ -39,10 +39,10 @@
        push    {r4, r5, r6}            /* save temporaries */
        mov     ip, r0                  /* we need r0 for return value */
 #ifdef __ARM_EABI__
-       ldrd    r4, r5, [sp]            /* fetch new value */
+       ldrd    r4, r5, [sp, #12]       /* fetch new value */
 #else
-       ldr     r4, [sp, #0]            /* fetch new value */
-       ldr     r5, [sp, #4]            /* fetch new value */
+       ldr     r4, [sp, #12]           /* fetch new value */
+       ldr     r5, [sp, #16]           /* fetch new value */
        mov     r3, r2                  /* r2 will be overwritten by r1 which 
... */
        mov     r2, r1                  /* r1 will be overwritten by ldrexd */
 #endif