Question About Sequence Points and Interrupt/Thread Safety

//--------------------------------------------------------------------------------

Several comments.

One: I assume the timers (the counters in the 'in_arg' array) might be read and/or modified during an interrupt routine, which is why it's necessary to delimit the access with DI()/EI(), plus whatever else may be necessary.

Two: The comment to the function talks about decrementing an array of timers, but the function body shown only ever deals with the first element of such an array. So something is amiss there.

Three: If the assumption about interrupt routines accessing the counters holds, this case certainly is one where 'volatile' should be used inside the DI()/EI() region to access the counters.

Four: As some other people have noted, if 'in_arg' is changed to be a pointer to volatile (so, 'volatile UINT8 *in_arg'), then the 'if(*in_arg) (*in_arg)--;' can access a counter three times (two reads and a write) instead of two. Whether this actually happens is either implementation-defined behavior or an ambiguity in the Standard (take your pick). One way around this potential performance hit is to use a temporary:

{ UINT8 t = *in_arg; if(t) *in_arg = t-1; }

Five: Another way around the problem is to use 'volatile' selectively rather than changing the declaration for 'in_arg':

if( *(volatile UINT8*)in_arg ) *(volatile UINT8*)in_arg = *in_arg - 1;

Of course, there is no guarantee any particular compiler will be smart enough to optimize away the second read, so declaring 'volatile UINT8 *in_arg' and using a temporary is probably safer.

Six: Although the Standard requires the write to the "object" to complete in the implementation machine, whether that write has actually made it out through the memory pipeline depends on the memory subsystem and on the particulars of the interruption mechanism. Some systems require a call to insert a write barrier into the store queue so that subsequent loads are guaranteed to do the right thing. Different kinds of write barriers can be required depending on what accesses are necessary to guarantee (single core, multi-core, multiprocessor).

Seven: For a single threaded, single core, single processor system (which sounds like what you're describing), just getting the store into the store queue (which using volatile should do) will be enough, provided the interrupt service routines do enough to flush the store queue before they start accessing any counters. Check what guarantees are made regarding pending writes by the hardware interrupt mechanism and by the interrupt service routines themselves; the two together /can/ guarantee that all pending writes have completed, the question is are the ISR's written in such a way that they /do/ guarantee it.

Eight: There is a corresponding question going the other direction, i.e., from the ISR back to the interrupted process.

Nine: The question of what the basic interrupt mechanism does can be rendered moot by putting in suitable write barriers around the code in ISR's that accesses the clock timers in question. Accesses of clock timers in the interrupt routines should also use volatile; probably not necessary in a practical sense, but the smart choice unless the performance consequences are too costly.

Ten: Unless the compilers are known to be rock solid, it's a good idea to check the generated assembly code in any case.

Summary:

use volatile, with explicit temporary; verify particulars of memory and hardware interrupt system; verify guarantee of pending writes, either (a) for general ISR invocation and return, or (b) specifically surrounding counter-access region in ISR's; also use volatile for counter access in ISR's; sanity check of generated assembly, just in case.

You've had quite a number of comments from others - rather than replying to everyone, I'll add a few points here.

Someone suggested making DI() and EI() external functions. If the compiler is calling these as external functions which it knows nothing about, then it must assume that they could read or write any globally accessible data, and thus it follows sequence points strictly ("x" is not read until after "DI();", and written back before "EI();"). But that's a big *if* - if information is available about the function (maybe it's defined in a header, or you have some sort of intern-module optimisation), all bets are off. It might be good enough for your tools now, but not for future compilers.

Using "volatile" accesses can certainly help with sequencing. In particular, the compiler will not re-organise volatile accesses with respect to each other. However, it is perfectly free to re-organise other accesses before, after, and around those volatile accesses. If your compiler does not consider your DI() and EI() to act as volatile accesses to memory, it can re-arrange them around accesses to "x" even if "x" is declared volatile.

One point to remember about "volatile", especially when trying to write optimal code - it is *accesses* that are volatile, not the data itself. It is perfectly possible to have "x" as a normal "UINT8", and force volatile accesses as and when needed by using *((volatile UINT8*) &x). It is also possible to cast away the volatile access on a variable that is declared volatile, but that should give you compiler warnings, and is probably better achieved by explicitly caching to a local variable. If you use gcc, this macro can be handy:

#define volatileAccess(v) *((volatile typeof((v)) *) &(v))

Once you have made "x" volatile (or added volatile access casts), the next issue is to make sure that DI() and EI() are considered volatile accesses to memory. This is highly dependent on the compiler.

For many compilers, inline assembly and assembly functions are *always* considered volatile memory accesses. Some compilers, however, are smarter than that, and can do a substantial amount of optimising of inline assembly. gcc, for example, will move an "asm ("DI")" instruction back and forth - sometimes even if it is declared "asm volatile ("DI")" (the "volatile" will ensure that things like loop unrolling don't duplicate the assembly). The trick is to tell gcc that it "clobbers" memory: asm ("DI" ::: "memory").

Of course, the *real* fun comes when you are using a processor that re-orders accesses in hardware which the compiler can't directly control...

Here's a links on the subject:

That wasn't my reaction at all. For developers, it had this set of suggestions:

We recommend that the developers of mission-critical or safety-critical embedded software that is written in C and that relies on volatile take one or more of the following steps:

• Manually validate the compiler's output for functions that rely importantly on proper compilation of accesses to volatile variables.

• Develop specific tests to ensure that the compiler generates proper code for the kinds of volatile accesses found in the software of interest.

• Factor accesses to volatiles into small helper functions, if the overhead of this can be tolerated.

• Compile functions that critically rely on volatile with optimizations turned off, if the overhead of this can be tolerated. Our belief (based on observations, but unsupported by systematic data) is that code emitted by compilers for accessing volatiles is generally correct when optimizations are turned off. It is folklore among embedded software researchers that in many cases, safety-critical embedded software for applications such as commercial avionics is always compiled without optimizations.

On top of that there was lots of other stuff about why the problems may tend to come up, and how to go about dealing with them (meaning, at a compiler level). The main result is that it identifies an area of C compilers that is likely to be weak, which isn't that surprising since volatile tends not to be used - if more attention is put on it, the results should get better relatively easily.

JL> #3 shows that you are not under 30 years old.

Hardly; my first real programming job, part time while I was a student, was as a maintenance programmer, and the person before me thought nothing mattered except whether the program compiled and did mostly what it was supposed to.

That taught me the value of clear code by the time I was 20 years old.

Charlton

Yes, yes, yes. For anything more complex then "Hello world", switching the compiler version can be painful. For that matter, keep the complete backup of the virtual machine with the older toolchain.

Well, messy code is a job security for some.

Vladimir Vassilevsky DSP and Mixed Signal Design Consultant

I'm feeling a bit dense currently. I'm having issues with the following: """

1. WHAT DOES VOLATILE MEAN? ... A compiler may not move accesses to volatile variables across sequence points.[1] ... [1. According to Section 3.8 of the C FAQ [18], "A sequence point is a point in time at which the dust has settled and all side effects which have been seen so far are guaranteed to be complete. The sequence points listed in the C standard are at the end of the evaluation of a full expression (a full expression is an expression statement, or any other expression which is not a subexpression within any larger expression); at the ||, &&, ?:, and comma operators; and at a function call (after the evaluation of all the arguments, and just before the actual call)." ] ... No guarantees are made about the atomicity of any given volatile access, about the ordering of multiple volatile accesses between two consecutive sequence points, or about the ordering of volatile and non-volatile accesses. """

That final clause seems to contradict the initial footnoted clause, on the presumption that the non-volatile accesses begin with, end with, or contain, a sequence point.

""" For example, the following code illustrates a common mistake in which a volatile variable is used to signal a condition about a non-volatile data struc- ture, perhaps to another thread:

volatile int buffer_ready; char buffer[BUF_SIZE]; void buffer_init() { int i; for (i=0; i

But it may move all other accesses however it wants.

I.e. if two volatile accesses happen in the same evaluation phase, then their order relative to each other is not well-defined, but if they are separated by a sequence point, their order /is/ well defined.

But other accesses do not constitute side effects, and so can be eliminated or reordered arbitrarily.

All accesses are contained in some evaluation phase that is delimited by sequence points.

The final clause is about accesses that are delimited in the same evaluation phase, whereas the footnoted sentence is about accesses that have a sequence point between them, and are thus in separate evaluation phases.

So there is no contradiction; the texts are about different situations.

But sequence points do not constrain accesses.

No it isn't, because the volatile store is not being reordered with respect to other volatile stores.

Not really.

You can regard the division into sequence points as being a fixed medium, in which accesses can move around. When accesses are relocated from one evaluation phase (space between sequence points) to another, the medium itself does not move; these moves do not drag the sequence points along.

Volatile accesses must stay in their evaluation phases, but non-volatile accesses can be moved, coalesced or eliminated.

=A0As

fe

at

Hopefully, the regulars will confirm or discount this, but I believe you can force the ordering by making wrappers for your two functions at take x as a parameter. I think this make the compiler think they are dependent on x's value, and not reorder the code. Something like:

DIx(x); if (x) x--; EIx(x);

REH

Setting aside the interrupt disable, 'x' should to be qualified volatile for exactly the same reasons that made you want to disable interrupts around that fragment.

Whether or not it's strictly necessary depends on the semantics of those non-standard calls DI()/EI(). If the compiler can be sure that EI() doesn't reference x (e.g. because it's a tightly described bit of inline asm, or a compiler intrinsic), it's allowed to delay storing 'x' in memory unless you qualify it as volatile.

If you don't tell the compiler about it needing protection by qualifying it as 'volatile', there is.

I would not rely on that. x can be passed (to EI) by copying the register from the OP's example and the change to x could still occur later. You might have move luck with &x because in that case the compiler would have to do extra work to avoid the obvious method of updating x "on time".

Others have already pointer out that there can be inter-function and even inter-translation-unit (link time) optimisation which will allow the compiler to break the code.

Using non-standard (possibly OS provided) routines is probably the best way to go in my opinion.

The former quote says that compilers can't move volatile accesses _across_ a sequence point; the latter quote says that the order of volatile accesses _between_ two sequence points is undefined. The two do not contradict each other.

For instance, consider the following:

volatile int a, b, c; /* other code */ foo(); a += b + c; bar();

The compiler cannot move the volatile accesses before foo() or after bar(), but it _can_ choose in what order to read a, b, and c. However, it can also choose to move other (non-volatile) code between foo() and bar(), or move code before foo() to after bar() or vice versa, if it could prove that doing so wouldn't violate the as-if rule.

Ehm, no. You're overlooking that by moving code down from before foo() to after bar(), you're also moving your volatile accesses up, across the sequence point(s) in that code, which would violate the first statement above.

It cuts both ways --- whenever you talking about "moving" code, what actually happens is that operations are swapped, and that means you move each across the other.

You are correct. Actually, I hadn't unit-tested the function, and I would have found that.

Want to be my code reviewer?

That is embarrassing. I don't do that often.

I don't see that corresponding question, at least not on my hardware. By the time the ISR returns, the data will have been written to memory.

The STM8 is one step above toaster/dishwasher grade ...

If you're seeing a problem going the other way, please explain it ...

The Lizard

Hmm. I think that you're right in theory, but is it possible that a conforming program could tell the difference? Doesn't the as-if rule still apply to the non-volatile parts of the code? Or does a single volatile access break the as-if rule even for non-volatile operations?

I've seen compilers do what I described (from studying the asm output), and nobody seems to complain about it. Mixing volatile and non-volatile operations is pretty unpredictable in practice, so I have always avoided that entirely rather than try to figure out exactly what the Standard guarantees (which doesn't appear to be much when it comes to volatile variables).

I'm happy with that. A nice clear absolute. No wondering about 'as if's.

Perfectly happy with that too, wouldn't want it any other way.

What's so special about function calls? Why wouldn't a for loop, or even simple assignments, contain sequence points that the volatile accesses couldn't be moved past? What property do foo() and bar() have that those other things with sequence points don't have?

Phil

They're in the example, so they can be referred to.

I know it isn't strictly true, but my mental shortcut for all the rules about volatile variables/operations is that volatile voids the as-if rule. The problems only come when you mix volatile and non-volatile operations together.

Any sequence point suffices; the only magical property of function calls in my example is that they're easy to refer to by name. It's hard to refer to particular semicolons because they all look the same...

So you think the document's wrong?

Phil