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

[duane ellis]

Harald Kipp wrote:
> duane ellis schrieb:
>   
>> Look at how Linux does this.
>>
>> http://www.promethos.org/lxr/http/ident?v=2.6.14;i=local_irq_save
>>
>> Those guys are smart. Learn from them.
>>     
>
>
> Well, I tried. But there are things I do not understand. For example, 
> why do they use the "memory" clobber? This forces the compiler to forget 
> about all values currently stored in registers.
>   
I believe (90% sure) it has to do with volatility and optimization of
variables. For example, if reg0 has the value of global X, and you call
some function, upon return, the global variable X may have changed.
Likewise, when you re-enable interrupts using inline assembly, the odds
are, you might have an IRQ pending which will immediately occur, and
thus, the complier must forget what it knows about optimized values.
  From an optimization point of view - the inline code must act like the
inline assembly was a function call.

> I've done some research on the arm-elf-gcc. It's not finished, but I 
> hope I can publish the results in a related document. What I can say so far:
>
> Storing the state of the IRQ flags on the stack is dangerous, or in 
> other words, very bad to do. Matthias raised the question, whether 
> resulting stack corruption is based on an academic question or on 
> reality. Obviously it happens quite rarely and after this long 
> discussion thread I assume that everyone agrees to: There is no 
> guarantee that it will _not_ happen.
>
> Now, how to solve this?
>   
I would state the problem differently
       Storing the state in a variable known to the compiler - is
	perfectly fine.
       That variable may or may not be on the CPU stack

And
      Storing the state 'behind the compilers back' -
      manipulating the machine (ie: stack) is a bad
      thing to do.

> Some posters do not understand, why we do not simply take the approach 
> of other operating systems (not just Linux btw.), passing a local 
> variable to the Enter/ExitCritical macros.
> (Maintainer hat on)
>  From the view of the developer this might not hurt much. From my view, 
> this will cause a lot of trouble, because it will break almost all 
> existing applications, which make use of critical sections. I definitely 
> know, that many organizations and companies out there are using Nut/OS 
> based applications, while the original developers are no longer 
> available. I'd really hate to tell them, that there is a bug fix for 
> their strange crashes, but they need to rewrite some device driver code.
> (Maintainer hat off)
>   
If your goal is to keep backward compatibility - then there is an easy
way of solving it.

(1) Create NEW funcs/macros that do it the right way.
(2) Change OLD funcs/macros to use the workaround [see below]
(3) Deprecate OLD, give X time frame warning.
(4) OLD users have 2 choices, stay with the old version, or move to the new.

>> Why don't we simply use a global counter 
>> variable to keep track of the enter/exit level?


  >> My suggestion is, to fix the existing NutEnter/ExitCritical using a
  >>global counter variable and, ....

if you *NEVER* want to support nested piority based IRQs, a counter will
work. ie: On RESTORE, if counter==0, enable_irqs().

example: In an IRQ - re-enable interrupts and let the IRQ hardware
priority work its magic.

If you want nested priorities - then - you must have a stack based
solution. Or a "stack like solution".

Both are doable by manipulating a *VOLATILE* counter.

For the nested IRQ case.

(1) save the existing counter value [ie: in a local variable]
(2) set counter value to 1
(3) Invoke Exit Critical
     (which should subtract 1 from the counter,
      detect 0, and thus enable irqs)

Later, to un-nest, one must do something special.

1) disable IRQs
2) Restore the old counter from the stack.
3) Return from the IRQ.

Nasty, tricky and confusing code to the un-initiated.


-Duane.