taskCHECK_FOR_STACK_OVERFLOW (StackMacros.h) - undefined behaviour

Kernel
1

philpem wrote on Friday, January 27, 2017:

Here’s one picked up by IAR and confirmed by PC-LINT.

When FreeRTOS 9.0.0 is built with IAR EWARM, a warning is reported every time the taskCHECK_FOR_STACK_OVERFLOW macro is used. IAR doesn’t elaborate other than to say:

Warning[Pa082]: undefined behavior: the order of volatile accesses is undefined in this statement

Thankfully, PC-LINT also detects this error and can expand into the macro. The issue is the call to vApplicationStackOverflowHook on lines 97, 132 and 156 of StackMacros.h. The issue is that pxCurrentTCB is defined as volatile in tasks.c, but it is being used twice in the call of the stack overflow hook: once as the task handle (after being converted), and again to retrieve the task name.

While the stack checks are taking place during task switches and pxCurrentTCB shouldn’t be changing, the compiler isn’t to know this, so we get a warning that “pxCurrentTCB might change betweent the two reads and things might be inconsistent”.

Practically I can see two ways to resolve this –

  • Take a copy of pxCurrentTCB as part of the stack overflow check and work on this instead of pxCurrentTCB. This already sort-of happens with pulStack, although I suspect this is more for reasons of brevity than an actual bug.
  • Suppress the warning as part of the macro (this would be compiler specific).

What are everyone’s thoughts on this one? Seems like this is a fairly minor, easy to fix thing which wouldn’t cause an actual “bug”, just irritates developers with spurious compiler warnings.

Cheers,
Phil.

2

rtel wrote on Friday, January 27, 2017:

This is one of those cases where the IAR compiler picks up on things that many others don’t. If you have that warning turned on then I’m surprised it doesn’t generate more of them. All the places where IAR generates the warning have been inspected and determined to be safe, so I thought it had been turned off in the source file (header file). The macro you identify is called as part of the context switch, so we don’t want to add code that would use more stack or take longer to execute. Your first suggestion might not generate more code when using the IAR compler (in fact it might actually reduce the amount of code), but the same would not be true for all compilers.

3

philpem wrote on Tuesday, January 31, 2017:

I suspected as much. I’ll see if I can find a warning suppression pragma for it.
Thanks.