Re: [Patch, toplevel/config] fix tls.m4 configure race condition (bootstrap/PR43170)

[Paolo Bonzini <bonzini at gnu dot org>]

On 06/08/2010 06:24 PM, IainS wrote:
> On 8 Jun 2010, at 17:16, Paolo Bonzini wrote:
>
> > On 06/06/2010 02:58 PM, IainS wrote:
> > > Although this is reported against darwin, it affects all emulated TLS
> > > targets.
> > > there is a race condition that causes sporadic incorrect determination
> > > of tls capability,
> > > it has been discussed in detail on the PR thread
> > > (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43170).
> >
> > - static int *a_in_other_thread;
> > + int *a_in_other_thread;
> > + int *a_in_main_thread;
> >
> > No reason to make this global.
> >
> > Patch okay if you just move the "a_in_main_thread = &a" assignment for
> > all branches. I can take care of syncing to src.
>
> if
> int *a_in_main_thread;
>
> is local to main () is not the compiler at liberty to move the first
> access to it to the first place it's used?
>
> i.e. it could order the call to __emutls_get_address () for
> a_in_main_thread after the call the pthread_create().

I guess it depends on when __emutls_get_address is lowered.  But then 
the compiler could be smart and "localize" the variable with 
-fwhole-program, so your change is only safe for current compilers.  I 
think the best solution is to make "a" volatile, like

 		__thread volatile int a;
-		static int volatile *volatile a_in_other_thread;

...

 		int volatile *volatile a_in_main_thread;
 		a_in_main_thread = &a;
                *a_in_main_thread;

This is more future-proof, I guess. Can you try this?

Paolo