This is the mail archive of the
cygwin-patches
mailing list for the Cygwin project.
[PATCH?] Separate pthread patches, #2 take 2.
- From: Dave Korn <dave dot korn dot cygwin at googlemail dot com>
- To: cygwin-patches at cygwin dot com
- Date: Thu, 04 Jun 2009 00:25:10 +0100
- Subject: [PATCH?] Separate pthread patches, #2 take 2.
The attached patch implements ilockexch and ilockcmpexch, using the inline
asm definition from __arch_compare_and_exchange_val_32_acq in
glibc-2.10.1/sysdeps/i386/i486/bits/atomic.h, trivially expanded inline rather
than in its original preprocessor macro form.
It generates incorrect code. The sequence discussed before,
126 do
127 node->next = head;
128 while (InterlockedCompareExchangePointer (&head, node, node->next) !=
node->next);
is now compiled down to this loop:
L186:
.loc 3 127 0
movl __ZN13pthread_mutex7mutexesE+8, %edx # mutexes.head, D.28599
.loc 2 58 0
movl %edx, %eax # D.28599, tmp68
/APP
# 58 "/gnu/winsup/src/winsup/cygwin/winbase.h" 1
lock cmpxchgl %ebx, __ZN13pthread_mutex7mutexesE+8 # this,
# 0 "" 2
/NO_APP
movl %eax, -12(%ebp) # tmp68, ret
.loc 2 59 0
movl -12(%ebp), %eax # ret, D.28596
.loc 3 126 0
cmpl %eax, %edx # D.28596, D.28599
jne L186 #,
movl %edx, 36(%ebx) # D.28599, <variable>.next
... which is in fact the equivalent of
126 do
127 ;
128 while (InterlockedCompareExchangePointer (&head, node, node->next) !=
node->next);
(126) node->next = head;
As it caches the values of head in %edx during the spin loop, and only
stores it to node->next after having overwritten *head with node, there is a
short window after the new node is linked to the front of the chain during
which its chain pointer is incorrect.
Not OK for head?
cheers,
DaveK
Index: winsup/cygwin/winbase.h
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/winbase.h,v
retrieving revision 1.14
diff -p -u -r1.14 winbase.h
--- winsup/cygwin/winbase.h 12 Jul 2008 18:09:17 -0000 1.14
+++ winsup/cygwin/winbase.h 3 Jun 2009 22:54:38 -0000
@@ -34,27 +34,31 @@ ilockdecr (volatile long *m)
": "=&r" (__res), "=m" (*m): "m" (*m): "cc");
return __res;
}
-
-extern __inline__ long
-ilockexch (volatile long *t, long v)
-{
- register int __res;
- __asm__ __volatile__ ("\n\
-1: lock cmpxchgl %3,(%1)\n\
- jne 1b\n\
- ": "=a" (__res), "=q" (t): "1" (t), "q" (v), "0" (*t): "cc");
- return __res;
-}
-
-extern __inline__ long
-ilockcmpexch (volatile long *t, long v, long c)
-{
- register int __res;
- __asm__ __volatile__ ("\n\
- lock cmpxchgl %3,(%1)\n\
- ": "=a" (__res), "=q" (t) : "1" (t), "q" (v), "0" (c): "cc");
- return __res;
-}
+
+extern __inline__ long
+ilockexch (volatile long *t, long v)
+{
+ return ({
+ __typeof (*t) ret;
+ __asm __volatile ("1: lock cmpxchgl %2, %1\n"
+ " jne 1b\n"
+ : "=a" (ret), "=m" (*t)
+ : "r" (v), "m" (*t), "0" (*t));
+ ret;
+ });
+}
+
+extern __inline__ long
+ilockcmpexch (volatile long *t, long v, long c)
+{
+ return ({
+ __typeof (*t) ret;
+ __asm __volatile ("lock cmpxchgl %2, %1"
+ : "=a" (ret), "=m" (*t)
+ : "r" (v), "m" (*t), "0" (c));
+ ret;
+ });
+}
#undef InterlockedIncrement
#define InterlockedIncrement ilockincr