https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89199
boger at gcc dot gnu.org changed:
What |Removed |Added
----------------------------------------------------------------------------
Priority|P3 |P2
Severity|normal |major
--- Comment #1 from boger at gcc dot gnu.org ---
I have raised this priority because this looks like a bug in the Go code due to
the use of __atomic_compare_exchange_n builtin and how it is implementing the
CompareAndSwapUintptr.
In atomic.c:
_Bool
CompareAndSwapUintptr (uintptr_t *val, uintptr_t old, uintptr_t new)
{
return __atomic_compare_exchange_n (val, &old, new, true, __ATOMIC_SEQ_CST,
__ATOMIC_RELAXED);
}
The true argument is indicating 'weak' which results in the following code.
Note that there is no guarantee in this code that the CompareAndSwap will do
the store and return true, if it can't get the reservation it will return false
and the store won't happen.
00000000010cc280 <sync..z2fatomic.CompareAndSwapUintptr>:
10cc280: ac 04 00 7c hwsync
10cc284: a8 18 20 7d ldarx r9,0,r3
10cc288: 00 20 29 7c cmpd r9,r4
10cc28c: 0c 00 82 40 bne 10cc298
<sync..z2fatomic.CompareAndSwapUintptr+0x18>
10cc290: ad 19 a0 7c stdcx. r5,0,r3
10cc294: 2c 01 00 4c isync
10cc298: 26 00 78 7c mfocrf r3,128
10cc29c: fe 1f 63 54 rlwinm r3,r3,3,31,31
10cc2a0: 20 00 80 4e blr
If I look in sync/cond.go check() function:
func (c *copyChecker) check() {
if uintptr(*c) != uintptr(unsafe.Pointer(c)) &&
!atomic.CompareAndSwapUintptr((*uintptr)(c), 0,
uintptr(unsafe.Pointer(c))) &&
uintptr(*c) != uintptr(unsafe.Pointer(c)) {
panic("sync.Cond is copied")
}
}
This if check assumes that the CompareAndSwapUinptr will do the store if needed
which is not true with the latest implementation of CompareAndSwapUintptr. It
seems that the CompareAndSwapUinptr should be passing the argument to indicate
'strong' and not 'weak'. I made that change and that fixes this problem. I have
not checked the other CompareAndSwaps.
