Patchwork [google/gcc-4_7] fix race in __cxa_guard_acquire

login
register
mail settings
Submitter Ollie Wild
Date Sept. 19, 2012, 7:59 p.m.
Message ID <CAFOgFcQSac8B=apoCUEzFWvTsJm_iCcHgp60Fe0x88CfhDQO8Q@mail.gmail.com>
Download mbox | patch
Permalink /patch/185207/
State New
Headers show

Comments

Ollie Wild - Sept. 19, 2012, 7:59 p.m.
This is a merge of r191125 and r191191 from gcc-4_7-branch.  It fixes
a race in __cxa_guard_acquire which can cause duplicate initialization
of static variables within functions.

Okay for google/gcc-4_7?

Thanks,
Ollie


Google ref b/7173106.

        * libsupc++/guard.cc (__cxa_guard_acquire): Exit the loop earlier if
        we detect that another thread has had success. Don't compare_exchange
        from a finished state back to a waiting state.  Fix up the last
        argument of the first __atomic_compare_exchange_n.  Comment.
commit 40ec687ace62b4d4c64f72be2bdf4321f5213107
Author: Ollie Wild <aaw@google.com>
Date:   Wed Sep 19 14:52:53 2012 -0500

    Merge r191125 and r191191 from gcc-4_7-branch.
    
    Google ref b/7173106.
    
    	* libsupc++/guard.cc (__cxa_guard_acquire): Exit the loop earlier if
    	we detect that another thread has had success. Don't compare_exchange
    	from a finished state back to a waiting state.  Fix up the last
    	argument of the first __atomic_compare_exchange_n.  Comment.
Diego Novillo - Sept. 19, 2012, 8:01 p.m.
On 2012-09-19 15:59 , Ollie Wild wrote:

>          * libsupc++/guard.cc (__cxa_guard_acquire): Exit the loop earlier if
>          we detect that another thread has had success. Don't compare_exchange
>          from a finished state back to a waiting state.  Fix up the last
>          argument of the first __atomic_compare_exchange_n.  Comment.
>

OK.


Diego.

Patch

diff --git a/libstdc++-v3/libsupc++/guard.cc b/libstdc++-v3/libsupc++/guard.cc
index adc9608..f8550c0 100644
--- a/libstdc++-v3/libsupc++/guard.cc
+++ b/libstdc++-v3/libsupc++/guard.cc
@@ -244,16 +244,16 @@  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))
+					    __ATOMIC_ACQUIRE))
 	      {
 		// This thread should do the initialization.
 		return 1;
@@ -264,13 +264,26 @@  namespace __cxxabiv1
 		// Already initialized.
 		return 0;	
 	      }
+
 	     if (expected == pending_bit)
 	       {
+		 // Use acquire here.
 		 int newv = expected | waiting_bit;
 		 if (!__atomic_compare_exchange_n(gi, &expected, newv, false,
 						  __ATOMIC_ACQ_REL, 
-						  __ATOMIC_RELAXED))
-		   continue;
+						  __ATOMIC_ACQUIRE))
+		   {
+		     if (expected == guard_bit)
+		       {
+			 // Make a thread that failed to set the
+			 // waiting bit exit the function earlier,
+			 // if it detects that another thread has
+			 // successfully finished initialising.
+			 return 0;
+		       }
+		     if (expected == 0)
+		       continue;
+		   }
 		 
 		 expected = newv;
 	       }