[cairo-bugs] [Bug 103037] Segmentation fault in _cairo_traps_compositor_glyphs

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=103037

--- Comment #21 from Bill Spitzak <spitzak at gmail.com> ---
>   Thread 2:
>   // do not read x here
>   while (y != 1) { // non-atomic read!
>     // code that does other atomic operations
>   }
>   assert( x == foo ); // this works

>That is relying on undefined behavior. Future versions of the compiler may >decided to optimize away second and subsequent reads. 

That is not exactly the problem. If there is a release fence inside the loop it
MUST re-read y on the next iteration. The actual problem is that it could
pre-load x. It must re-load x if the loop is executed, but if the loop is not
run (because y==1 initially) then it does not have to reload it, and therefore
it can get an incorrect version of x.

However I have run some more tests and it now looks like gcc (4.8.2) produces
identical code for "if (y)" and "if (__atomic_load_n(&y,__ATOMIC_X))" for all
valid values of X, even SEQ_CST (!). I was basing my comments on attempts to
use the __sync operations. Absolutely I would recommend using __atomic in all
code from now on (if in fact gcc is correct, I am surprised there is no sync or
other added instruction).

It seems correct to use the acquire/release types for this init flag, but since
it produces identical code on x86 there could be a worry that this will be
insufficiently tested.

This is my test code (compiled with -S -O7 -lthread) which seems to show no
difference (change the if statement to get other versions):

int y;
int x1;
int x2;
int foo() {
    if (__atomic_load_n (&y, __ATOMIC_SEQ_CST))
        return x1;
    else
        return x2;
}

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.cairographics.org/archives/cairo-bugs/attachments/20171010/5036ed9f/attachment.html>