libstdc++/5037: Multithreaded read access to strings not threadsafe on Solaris/Sparc

Loren James Rittle rittle@latour.rsch.comm.mot.com
Thu Dec 6 17:40:00 GMT 2001


[Thank you Craig for finally producing a self-contained bug report for
 this issue.  You deserve a medal.  How about this patch against the
 3.0 source tree instead?  You need only rebuild and install libstdc++
 after running ``make clean'' (you might need to also explicitly
 remove the staged atomicity.h file) not a full gcc bootstrap.  Since
 it is exposed through standard headers, I am afraid that this is an
 ABI change for all sparc ports.  Assume that others might prefer
 another final solution to fix this on the 3.0.X branch with a
 different performance tradeoff.]

>> I believe this file [libstdc++-v3/config/cpu/sparc/*/bits/atomicity.h]
>> is broken.  __exchange_and_add and __atomic_add use separate locks,
>> and so are not protected from each other.

> Yikes!  I see exactly what you are saying. [...]
> Any best solution (in terms of contention) requires a locking word
> related to the first argument.  Would a designer of the atomicity.h
> abstraction layer prefer to provide such a solution?

In the interest of getting a fix in the mainline tree ASAP, here is
the obvious solution that requires no change to the abstraction layer
and I think meets any contention-efficiency goals.  This version was
tested against 3.0.3 prerelease since that is what I had built for
sparc (see ABI change warning as to why this might only be good for
the mainline, if someone comes up with a better non-ABI changing
patch).  I rebuilt libstdc++-v3 in an already bootstrapped tree for
sparc-sun-solaris2.7.  With this patch, the test case from
libstdc++/5037 works on a sparc multiprocessor box.  I wonder if this
patch will fix all those random PRs without test cases for mysterious
multithreaded crashes on sparc... ;-)

Other atomicity.h files not checked yet.

	* config/cpu/sparc/sparc32/bits/atomicity.h (_Atomic_word): Relate
	a lock on a per-word basis.
	(__exchange_and_add): Update to use new _Atomic_word.
	(__atomic_add): Likewise.
	(__compare_and_swap): Removed.

Index: libstdc++-v3/config/cpu/sparc/sparc32/bits/atomicity.h
===================================================================
RCS file: /cvs/gcc/gcc/libstdc++-v3/config/cpu/sparc/sparc32/bits/atomicity.h,v
retrieving revision 1.2.6.1
diff -c -r1.2.6.1 atomicity.h
*** atomicity.h	2001/02/28 00:04:10	1.2.6.1
--- atomicity.h	2001/12/07 01:29:26
***************
*** 30,56 ****
  #ifndef _BITS_ATOMICITY_H
  #define _BITS_ATOMICITY_H	1
  
! typedef int _Atomic_word;
  
  static int
  __attribute__ ((__unused__))
  __exchange_and_add (volatile _Atomic_word* __mem, int __val)
  {
!   static unsigned char __lock;
!   _Atomic_word __result, __tmp;
  
    __asm__ __volatile__("1:	ldstub	[%1], %0\n\t"
  		       "	cmp	%0, 0\n\t"
  		       "	bne	1b\n\t"
  		       "	 nop"
  		       : "=&r" (__tmp)
! 		       : "r" (&__lock)
  		       : "memory");
!   __result = *__mem;
!   *__mem += __val;
    __asm__ __volatile__("stb	%%g0, [%0]"
  		       : /* no outputs */
! 		       : "r" (&__lock)
  		       : "memory");
    return __result;
  }
--- 30,62 ----
  #ifndef _BITS_ATOMICITY_H
  #define _BITS_ATOMICITY_H	1
  
! struct _Atomic_word
! {
!   unsigned char __lock;
!   int __word;
!   operator int() const { return __word; }
!   operator = (const int x) { return x == __word; }
!   _Atomic_word () : __lock (0), __word (0) {}
! };
  
  static int
  __attribute__ ((__unused__))
  __exchange_and_add (volatile _Atomic_word* __mem, int __val)
  {
!   int __result, __tmp;
  
    __asm__ __volatile__("1:	ldstub	[%1], %0\n\t"
  		       "	cmp	%0, 0\n\t"
  		       "	bne	1b\n\t"
  		       "	 nop"
  		       : "=&r" (__tmp)
! 		       : "r" (&__mem->__lock)
  		       : "memory");
!   __result = __mem->__word;
!   __mem->__word += __val;
    __asm__ __volatile__("stb	%%g0, [%0]"
  		       : /* no outputs */
! 		       : "r" (&__mem->__lock)
  		       : "memory");
    return __result;
  }
***************
*** 59,108 ****
  __attribute__ ((__unused__))
  __atomic_add (volatile _Atomic_word* __mem, int __val)
  {
!   static unsigned char __lock;
!   _Atomic_word __tmp;
  
    __asm__ __volatile__("1:	ldstub	[%1], %0\n\t"
  		       "	cmp	%0, 0\n\t"
  		       "	bne	1b\n\t"
  		       "	 nop"
  		       : "=&r" (__tmp)
! 		       : "r" (&__lock)
  		       : "memory");
!   *__mem += __val;
    __asm__ __volatile__("stb	%%g0, [%0]"
  		       : /* no outputs */
! 		       : "r" (&__lock)
  		       : "memory");
- }
- 
- static int
- __attribute__ ((__unused__))
- __compare_and_swap (volatile long *__p, long __oldval, long __newval)
- {
-   static unsigned char __lock;
-   long __ret, __tmp;
- 
-   __asm__ __volatile__("1:	ldstub	[%1], %0\n\t"
- 		       "	cmp	%0, 0\n\t"
- 		       "	bne	1b\n\t"
- 		       "	 nop"
- 		       : "=&r" (__tmp)
- 		       : "r" (&__lock)
- 		       : "memory");
-   if (*__p != __oldval)
-     __ret = 0;
-   else
-     {
-       *__p = __newval;
-       __ret = 1;
-     }
-   __asm__ __volatile__("stb	%%g0, [%0]"
- 		       : /* no outputs */
- 		       : "r" (&__lock)
- 		       : "memory");
- 
-   return __ret;
  }
  
  #endif /* atomicity.h */
--- 65,84 ----
  __attribute__ ((__unused__))
  __atomic_add (volatile _Atomic_word* __mem, int __val)
  {
!   int __tmp;
  
    __asm__ __volatile__("1:	ldstub	[%1], %0\n\t"
  		       "	cmp	%0, 0\n\t"
  		       "	bne	1b\n\t"
  		       "	 nop"
  		       : "=&r" (__tmp)
! 		       : "r" (&__mem->__lock)
  		       : "memory");
!   __mem->__word += __val;
    __asm__ __volatile__("stb	%%g0, [%0]"
  		       : /* no outputs */
! 		       : "r" (&__mem->__lock)
  		       : "memory");
  }
  
  #endif /* atomicity.h */



More information about the Libstdc++ mailing list