[En-Nut-Discussion] Fwd: Not fixed!!!: Re: confirmed!!! Re: NutOS 4.4.0 on ARM7: possibly bug in NutEnterCritical / NutExitCritical

[duane ellis]

Alain M. wrote:
>
> Duane Ellis escreveu:
>> Nathan Moore wrote:
>>> The reason you use "memory" in clobber as I understand it is if you 
>>> were
>>> to alter something through a pointer within an asm operation.
>>>  
>> Yes, and that is effectively what *can* *appear* to happen when IRQs are
>> re-enabled.
> Can you explain this a bit, please? I am new to ARM and relatively new 
> to GCC, so I missed something and I cannot see what :)
"memory" tells the compiler that "memory is being clobbered"
Or put another way - "this is a memory barrier"

Picture Gandolf the wizard screaming at the Optimizer Beast  "THOU SHALT 
NOT PASS"
Less dramatically put: It is an optimization fence post or barrier.

Compile the following code snippet with  "arm-elf-gcc -O2 -S foo.c", 
look at the result "foo.s" file.
GCC in the UNSAFE condition optimizes the two additions into a single +2 
addition.

The difference is "memory" or "not".

In this case, I am not using "volatile variables" - or am I accessing a 
through a pointer.
A naive developer would think the code is safe.

The below only fails in *ARM*. It will not fail in *THUMB*.

Why? The "mrs" instruction is only ARM.
One could - construct a fancier inline asm for THUMB...
that (1) does the arm/thumb dance, (2) execute mrs, and (2) dance to thumb.
Maybe - in that case this bug would remain, I do not know.

Most often, one would instead create a small "arm/thumb" interwork 
function to do the MRS instruction.
Because it would be *not*inline*asm* - the function call it self acts 
like a memory barrier.
Hence, the bug report would be "works in thumb" and "fails in arm"

-Duane.

===SNIP===

#define LINUX_raw_local_irq_enable()                             \
        ({                                                      \
                unsigned long temp;                             \
        __asm__ __volatile__(                                   \
        "mrs    %0, cpsr                @ local_irq_enable\n"   \
"       bic     %0, %0, #128\n"                                 \
"       msr     cpsr_c, %0"                                     \
        : "=r" (temp)                                           \
        :                                                       \
        : "memory", "cc");                                      \
        })


#define UNSAFE_raw_local_irq_enable()                             \
        ({                                                      \
                unsigned long temp;                             \
        __asm__ __volatile__(                                   \
        "mrs    %0, cpsr                @ local_irq_enable\n"   \
"       bic     %0, %0, #128\n"                                 \
"       msr     cpsr_c, %0"                                     \
        : "=r" (temp)                                           \
        : : "cc"                            \
         );                                      \
        })

int x;
int y;

void
safe_function(void)
{
  x++;
  LINUX_raw_local_irq_enable();
  x++;
}

void
unsafe_function(void)
{
  y++;
  UNSAFE_raw_local_irq_enable();
  y++;
}


=== SNIP ====