On 2018-04-07 14:07:05 -0400, Tom Lane wrote: > Andres Freund <and...@anarazel.de> writes: > > As Daniel pointed out in: > > https://postgr.es/m/fb948276-7b32-4b77-83e6-d00167f8e...@yesql.se the > > pg_atomic_flag fallback implementation is broken. That has gone > > unnoticed because the fallback implementation wasn't testable until now: > > ... > > The attached fixes the bug and removes the edge-cases by storing a value > > separate from the semaphore. I should have done that from the start. > > This is an ABI break, but given the fallback didn't work at all, I don't > > think that's a problem for backporting. > > > Fix attached. Comments? > > pademelon says it's wrong. > > 2018-04-07 13:39:34.982 EDT [1197:89] pg_regress/lock LOG: statement: SELECT > test_atomic_ops(); > TRAP: UnalignedPointer("(((uintptr_t) ((uintptr_t)(ptr)) + ((sizeof(*ptr)) - > 1)) & ~((uintptr_t) ((sizeof(*ptr)) - 1))) != (uintptr_t)(ptr)", File: > "../../../src/include/port/atomics.h", Line: 177)
Yea, I just saw that. Afaict it's "just" an over-eager / wrong assert. I can't for the heck of it think why I wrote (9.5 timeframe) AssertPointerAlignment(ptr, sizeof(*ptr)); where the bigger ones all have asserts alignment to their own size. I assume I did because some platforms want to do atomics bigger than a single int - but then I should've used sizeof(ptr->value). So far pademelon is the only animal affected afaict - let me think about it for a bit and come up with a patch, ok? Greetings, Andres Freund