Re: Is gcc thread-unsafe?

Willy Tarreau
2007-10-26T05:04:12Z

On Fri, Oct 26, 2007 at 01:42:37AM +0200, Andi Kleen wrote:
> On Friday 26 October 2007 01:32:53 Linus Torvalds wrote:
> >
> > On Fri, 26 Oct 2007, Andi Kleen wrote:
> > >
> > > No it can't (at least not on x86) as I have explained in the rest of the mail
> > > you conveniently snipped.
> >
> > I "conveniently snipped it" because it was pointless.
> >
> > "adc" or "cmov" has nothing what-so-ever to do with it. If some routine
> > returns 0-vs-1 and gcc then turns "if (routine()) x++" into
> > "x+=routine()", what does that have to do with adc or cmov?
>
> That is not what gcc did in that case. I don't think it tracks sets of values
> over function calls (or even inside functions) at all.
>
> The generated code was
>
> cmpl $1, %eax ; test res
> movl acquires_count, %edx ; load
> adcl $0, %edx ; maybe add 1
> movl %edx, acquires_count ; store
>
> So it just added the result of a comparison into a variable
> by (ab)using carry for this.

While this is OK in mono-threaded code, it introduces a race condition in
multi-threaded code. The code above tried to acquire a lock, and eax was
set to 1 if it succeeded. And whatever the result, all threads still
happily modify the shared memory area (acquires_count). So the classical
case where two threads perform the same operation at the same time ends
up with a random value in acquires_count.

> In theory such things can be done with CMOV too by redirecting
> a store into a dummy variable to cancel it, but gcc doesn't
> do that on its own.

Even with a CMOV, it's the memory write which should not be performed
if the lock was not acquired.

(...)
> But for registers it's a fine optimization.

100% agree.

What would really be needed is an attribute around conditions to
indicate whether they *may* be optimized or not. Something similar
to the likely/unlikely we currently use, we could have something
like __attribute__((unsafe_cond(cond))). I think that it could still
optimize by default but let the user explicitly state that he is
playing with thread-unsafe code. As you pointed out, you did not
find any such mis-optimization in the kernel, which means that it
does not hit too often. That's the reason why I'd let the user be
careful.

Willy