public inbox for glibc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug nptl/16882] New: Incorrect implementation of pthread_spin_lock for sparc, no ldstub after spinning
@ 2014-04-29 18:39 a5b at mail dot ru
  2014-05-23  4:33 ` [Bug nptl/16882] " culu.gyx at gmail dot com
                   ` (13 more replies)
  0 siblings, 14 replies; 16+ messages in thread
From: a5b at mail dot ru @ 2014-04-29 18:39 UTC (permalink / raw)
  To: glibc-bugs

https://sourceware.org/bugzilla/show_bug.cgi?id=16882

            Bug ID: 16882
           Summary: Incorrect implementation of pthread_spin_lock for
                    sparc, no ldstub after spinning
           Product: glibc
           Version: 2.16
            Status: NEW
          Severity: normal
          Priority: P2
         Component: nptl
          Assignee: unassigned at sourceware dot org
          Reporter: a5b at mail dot ru
                CC: drepper.fsp at gmail dot com

I think there is sparc-specific bug introduced by commit 
'Avoid "anonymous" code in pthread_spin_lock' from 3 May 2012
https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=e2dbf201abdfa13fc4035a1a8888ecec91bef44c

The bug exists since 2.16 version of glibc.

New version of pthread_spin_lock for sparc64 and sparcv8+ doesn't do "ldstub"
to lock the spinlock, if there were any spins on spinlock, locked by another
thread.

Original implementation of pthread_spin_lock: 

 Try to lock spinlock and return
 if spinlock was locked, go to 2: and spin, waiting spinlock to unlock
 then go to 1: to try lock it


    ("1: ldstub [%0], %%g2\n"
     "   orcc   %%g2, 0x0, %%g0\n"
     "   bne,a  2f\n"
     "   ldub   [%0], %%g2\n"
     ".subsection 2\n"
     "2: orcc   %%g2, 0x0, %%g0\n"
     "   bne,a  2b\n"
     "   ldub   [%0], %%g2\n"
     "   b,a    1b\n"
     ".previous"
     : /* no outputs */
     : "r" (lock)
     : "g2", "memory", "cc");

Code after patch:

 Try to lock spinlock and return
 if spinlock was locked, go to 2: and spin, waiting spinlock to unlock
 then return  

ENTRY(pthread_spin_lock)
       ldstub          [%o0], %g1
       brnz,pn         %g1, 2f
        membar         #StoreLoad | #StoreStore
1:     retl
        mov            0, %o0
2:     ldub            [%o0], %g1
       brnz,pt         %g1, 2b
        membar         #LoadLoad
       ba,a,pt         %xcc, 1b
END(pthread_spin_lock)


With newer code, pthread_spin_lock may return without actually locking the
spinlock. This is incorrect according to The Open Group Base Specifications
Issue 6
http://pubs.opengroup.org/onlinepubs/009695299/functions/pthread_spin_lock.html

> The pthread_spin_lock() function shall lock the spin lock referenced by lock.

I have short test:

$ cat pthread_spin_test.c
#include <pthread.h>
#include <stdio.h>
#include <unistd.h>

void* thread(void *arg)
{
    pthread_spinlock_t *lock = (pthread_spinlock_t*) arg;
    printf("Thread started, lock locked by main, value is %08x\n",
*(int*)lock);
    pthread_spin_lock(lock);
    printf("Lock now locked by thread, value is %08x\n", *(int*)lock);
    pthread_spin_unlock(lock);
    printf("Lock now unlocked by thread, value is %08x\n", *(int*)lock);
    return NULL;
}

int main()
{
    pthread_t thr1;
    pthread_spinlock_t lock;
    pthread_spin_init(&lock, PTHREAD_PROCESS_PRIVATE);
    printf("Lock created, value is %08x after init\n", *(int*)&lock);
    pthread_spin_lock(&lock);
    printf("Lock locked by main, value is %08x\n", *(int*)&lock);
    pthread_create(&thr1, NULL, thread, (void*)&lock);
    sleep(1);
    pthread_spin_unlock(&lock);
    pthread_join(thr1, NULL);
    return 0;
}

With glibc-2.7 it always outputs 0xFF as value of locked spinlock:

Lock created, value is 00000000 after init
Lock locked by main, value is ff000000
Thread started, lock locked by main, value is ff000000
Lock now locked by thread, value is ff000000
Lock now unlocked by thread, value is 00000000


And with glibc-2.16, spinlock locked by thread has incorrect value 0
(unlocked):

Lock created, value is 00000000 after init
Lock locked by main, value is ff000000
Thread started, lock locked by main, value is ff000000
Lock now locked by thread, value is 00000000       <<<<
Lock now unlocked by thread, value is 00000000


To fix the bug, I think it can be enough to move "1:" label before ldstub in
files
nptl/sysdeps/sparc/sparc64/pthread_spin_lock.S
nptl/sysdeps/sparc/sparc32/pthread_spin_lock.S

-- 
You are receiving this mail because:
You are on the CC list for the bug.


^ permalink raw reply	[flat|nested] 16+ messages in thread
end of thread, other threads:[~2014-06-04  1:22 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-29 18:39 [Bug nptl/16882] New: Incorrect implementation of pthread_spin_lock for sparc, no ldstub after spinning a5b at mail dot ru
2014-05-23  4:33 ` [Bug nptl/16882] " culu.gyx at gmail dot com
2014-05-23  4:33 ` culu.gyx at gmail dot com
2014-05-23  4:36 ` culu.gyx at gmail dot com
2014-05-23 14:26 ` culu.gyx at gmail dot com
2014-05-23 14:34   ` Ondřej Bílka
2014-05-23 14:35 ` neleai at seznam dot cz
2014-05-23 15:09 ` culu.gyx at gmail dot com
2014-05-25  2:12 ` culu.gyx at gmail dot com
2014-05-30  4:38 ` davem at davemloft dot net
2014-06-03 23:11 ` cvs-commit at gcc dot gnu.org
2014-06-03 23:42 ` cvs-commit at gcc dot gnu.org
2014-06-04  0:19 ` cvs-commit at gcc dot gnu.org
2014-06-04  0:58 ` cvs-commit at gcc dot gnu.org
2014-06-04  1:19 ` cvs-commit at gcc dot gnu.org
2014-06-04  1:22 ` davem at davemloft dot net

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).