diff mbox

[libstdc++] Fix PR54172

Message ID 3027697.Y09XNY3VZT@tjmaciei-mobl2
State New
Headers show

Commit Message

Thiago Macieira Aug. 30, 2012, 10:29 a.m. UTC 
Hello

The attached patch fixes a race condition in __cxa_guard_acquire, a regression 
from 4.6. When the code was refactored to use the new atomic intrinsics, the 
fact that __atomic_compare_exchange_n updates the "expected" variable with the 
current value was missed.

That causes the loop to possibly overwrite a successful initialisation and re-
start the initialisation phase. The bug report has a gdb trace with a 
watchpoint showing the guard variable transitioning

	0 (uninit) -> 256 (pending) -> 1 (done) -> 256 (pending) -> 1 (done)

This situation can happen if both CAS fail in given thread: the first because 
the other thread started the initialisation and the second because the 
initialisation succeeded.

Comments

Jakub Jelinek Sept. 5, 2012, 4:33 p.m. UTC | #1 
On Thu, Aug 30, 2012 at 12:29:51PM +0200, Thiago Macieira wrote:
> 2012-08-30  Thiago Macieira <thiago.macieira@intel.com>
> 
> 	PR libstdc++/54172
>         * libsupc++/guard.cc (__cxa_guard_acquire): Don't compare_exchange
>         from a finished state back to a waiting state.

This is ok for trunk and 4.7 branch.
The other patches I'd prefer to leave to Richard or Benjamin.

> --- a/libstdc++-v3/libsupc++/guard.cc
> +++ b/libstdc++-v3/libsupc++/guard.cc
> @@ -244,13 +244,13 @@ namespace __cxxabiv1
>      if (__gthread_active_p ())
>        {
>  	int *gi = (int *) (void *) g;
> -	int expected(0);
>  	const int guard_bit = _GLIBCXX_GUARD_BIT;
>  	const int pending_bit = _GLIBCXX_GUARD_PENDING_BIT;
>  	const int waiting_bit = _GLIBCXX_GUARD_WAITING_BIT;
>  
>  	while (1)
>  	  {
> +	    int expected(0);
>  	    if (__atomic_compare_exchange_n(gi, &expected, pending_bit, false,
>  					    __ATOMIC_ACQ_REL,
>  					    __ATOMIC_RELAXED))

	Jakub
diff mbox

Patch

2012-08-30  Thiago Macieira <thiago.macieira@intel.com>

	PR libstdc++/54172
        * libsupc++/guard.cc (__cxa_guard_acquire): Don't compare_exchange
        from a finished state back to a waiting state.
---
 libstdc++-v3/libsupc++/guard.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libstdc++-v3/libsupc++/guard.cc b/libstdc++-v3/libsupc++/guard.cc
index adc9608..4da9035 100644
--- a/libstdc++-v3/libsupc++/guard.cc
+++ b/libstdc++-v3/libsupc++/guard.cc
@@ -244,13 +244,13 @@  namespace __cxxabiv1
     if (__gthread_active_p ())
       {
 	int *gi = (int *) (void *) g;
-	int expected(0);
 	const int guard_bit = _GLIBCXX_GUARD_BIT;
 	const int pending_bit = _GLIBCXX_GUARD_PENDING_BIT;
 	const int waiting_bit = _GLIBCXX_GUARD_WAITING_BIT;
 
 	while (1)
 	  {
+	    int expected(0);
 	    if (__atomic_compare_exchange_n(gi, &expected, pending_bit, false,
 					    __ATOMIC_ACQ_REL,
 					    __ATOMIC_RELAXED))
-- 
1.7.11.4