[avr-libc-dev] Re: bug #22163 (atomic not working as expected in C++)

[David Brown]

On 09/06/2010 10:23, Joerg Wunsch wrote:
> https://savannah.nongnu.org/bugs/?22163
>
> The bug looks serious enough to me so I'd like to include David's
> alternative C++ implementation.  Did anyone else test this?  Any
> objections?

It's been a while since I wrote that code!

Looking back at it, there are a couple of points regarding memory blocks 
and interrupt flags in the description I wrote.

One is that I wrote that the memory barrier must occur /before/ a "sei" 
instruction or SREG restore.  I'm not sure how specific memory barriers 
are, especially if they are part of a single instruction such as 
asm("sei" ::: "memory").  I think it's important that there is a barrier 
both before and after these instructions - you don't want any reordering 
to occur.  And in contrast to what I wrote earlier, this applies to both 
"sei" and "cli" instructions.  As far as I understand it, you will get 
the correct effect by including the memory clobber in the asm statement 
itself.

To see what can go wrong with a single memory barrier that is not tied 
to the instruction, consider the sequence:

nonAtomicInt1 = 1234;
cli();
asm volatile ("" ::: "memory");
atomicGlobalInt = 12345;
sei();
asm volatile ("" ::: "memory");
nonAtomicInt2 = 5678;


The compiler can re-order the stores across the cli() and sei() 
instructions, but not across the memory barrier:

cli();
nonAtomicInt1 = 1234;
asm volatile ("" ::: "memory");
sei();
atomicGlobalInt = 12345;
asm volatile ("" ::: "memory");
nonAtomicInt2 = 5678;

The first move of nonAtomicInt1 will not affect the correctness of the 
code, but will lead to interrupts being disabled for longer than 
desired.  The second one will make the storage of atomicGlobalInt 
non-atomic.

If the memory barriers are placed before cli() and sei(), you get a 
similar effect.

If the memory clobbers are added to the definitions of cli() and sei(), 
as well as an "sreg set" macro (or inline function), then these should 
be used in the code here instead of writing the inline assembly explicitly.


mvh.,

David