Re: [avr-gcc-list] Avr-gcc Produces Incorrect Code with -Os

[David Brown]

I think this is getting *way* off-topic for this list.  However, it 
seems there are a number of people on the list who are interested in the 
thread.  Perhaps it should move to avr-chat?

Scott and Roxanne Munns wrote:
> David,
>
> I agree that this is a "point of view" thing.  And I will admit that most of
> my embedded programming (Coldfire, not AVR) is spent doing C++ programming,
> so that may make my view more data-centric than code-centric.
>
> I know that you understand the "critical section" issue, and you are able to
> state its requirements much more eloquently than I can.  I would trust your
> ability to recognize critical section issues and solve them properly,
> whether you think about them in a data-centric way or in a code-centric way.
> However, I am amazed how many embedded programmers *don't* understand how to
> work with critical sections.  Some developers I have talked to seem to
> interpret critical sections as "if some *code* can be executed in two
> different contexts, it needs a critical section to handle it".

I too am sometimes amazed at the lack of thought and understanding some 
developers have - although I have more often seen too little thought 
given to critical sections and other protection, than too much.  A 
common example is code writing messages to a screen called from both 
main loops and interrupts in the same system.

I can't say I've seen code using critical sections just because the code 
section is called from two different places in the program, but that 
could just be my luck.  Perhaps you've come across people who are used 
to programming for the PIC and other small micros where compilers often 
generate non-reentrant code (local variables are statically allocated 
instead of on the stack), and the programmers are thus extra paranoid.

> Maybe if you're talking about two threads accessing a shared code path, and
> that code path is also changing the state of shared data objects, that is
> correct.  If the developer always protects any shared code paths, the code
> will probably be safe in its code sequences, just very slow.
>
> On AVR, at least without an OS installed, there are no threads.  There are
> only contexts: mainline code context and interrupt contexts.  Depending on
> how the developer writes the code, it is possible that no code may get
> shared between contexts, only data.  Ideally, the developer should use
> shared functions to modify shared data objects and keep them in a coherent
> state, but there is no guarantee they will do that.  They may just clone
> code between the interrupt function and the mainline code.  Now there is no
> "shared code path".
>
> In my opinion, that is the possible flaw of thinking in a code-centric way.
> The developer may think there is no risk (no overlapping code paths), when
> the data is actually at risk.

If the developer is thinking in those lines, then he does not understand 
the issues at hand.  As you say, there is no need to protect code 
sequences themselves (unless you are dealing with non-reentrant code 
that can be called from different contexts) - the purpose is ultimately 
to protect the data.

Exclusively using shared functions to modify shared data is *not* ideal 
- it is very far from ideal on small systems looking for optimal code, 
and it is seldom ideal on larger systems.  It certainly has advantages 
in some situations - in this case, you only have a small number of code 
sections that need locks or other types of protection, so it is hard to 
get wrong.  But it can often lead to larger or slower code, and it can 
lead to at least two fundamental misunderstandings.

First, if you think you can make your code safe for multi-threading (or 
other multiple context access) by protecting individual data objects, 
you will have problems when you need to protect sequences of access.  It 
can be useful having a "queue" class with atomic "push" and "pop" 
classes, but it's easy then to forget that "q1.push(q2.pop())" is not 
atomic - you still need locks around the code sequence.

Secondly, if you believe too strongly in your "protected data items" 
approach, you'll forget that all sorts of other resources are shared and 
need protecting.  It's common to see code that is very careful about 
protecting access to a shared queue - and then happily calls printf() or 
similar functions from multiple contexts.


> There is probably a corollary to this for a data-centric way of thinking.
> Perhaps thinking that the get/set operations need critical sections, and
> forgetting that the in-between code sequences that modify a data object are
> also a vital part of maintaining overall data object coherency.  In other
> words, don't "drop the lock" until you're done modifying the data object!

Correct - that's why it is the *code sequence* that has the lock, not 
the data object.

However, if you are using an OS with mutexes, semaphores, and other 
locks (rather than the global "disable interrupts" lock), it's important 
that you use the correct lock in the correct place.  I certainly don't 
mean to imply that you use a particular lock for a sequence of code 
accessing some data, and a different lock for a different sequence of 
code accessing the same data!

> Either way, it comes down to training a developer about critical sections
> and making sure they really understand the core issues.  I guess I've been
> disappointed on that front up to this point.
>
> I guess as I ponder this issue, I am struggling to see which critical
> section primitives should be provided as "base GCC+avr-libc" independent of
> any OS, and which should be provided by an OS on top of the basic toolset.
> Do you have any ideas for guidelines we can use to choose which ones belong
> in each category?

Personally, I think all that is really needed is a clear and fast way to 
ensure that a section of code is run with interrupts disabled, and with 
the interrupt state restored at the end.  It's up to the OS to provide 
higher level locks, probably implemented using this disable interrupt 
primitive.  For those that are not using an OS, or only using a very 
simple one, something like a "critical" or "monitor" function attribute 
is sufficient.

mvh.,

David