Hello
The interrupt manager for the MPC57xx S32DS SDK has the following code:
static int32_t g_interruptDisableCount = 0; /* Consider: static volatile int32_t g_interruptDisableCount; */
void INT_SYS_EnableIRQGlobal(void)
{
/* Check and update */
if (g_interruptDisableCount > 0)
{
g_interruptDisableCount--;
if (g_interruptDisableCount <= 0)
{
/* Enable the global interrupt*/
ENABLE_INTERRUPTS();
}
}
}
void INT_SYS_DisableIRQGlobal(void)
{
/* Disable the global interrupt */
DISABLE_INTERRUPTS();
/* Update counter*/
g_interruptDisableCount++;
}
According to the C/C++ standards, the compiler optimizer is allowed to reorder side-effect free expressions. I believe that incrementing g_interruptDisableCount in INT_SYS_DisableIRQGlobal() is a side-effect free expression in the current implementation, so the optimizer could move this expression before DISABLE_INTERRUPTS(). If that happens, it would lead to a potential race condition where two threads would read/modify/write this global variable.
The DISABLE_INTERRUPTS() macro is a volatile asm statement (presumably the compiler treats this as having a side-effect), and would guarantee that the generated code would maintain program order if g_interruptDisableCount is also volatile qualified. Please see the following page on this topic: https://en.cppreference.com/w/cpp/language/as_if
A better implementation of these functions would probably involve stdatomic.h functions.
Thank, you,
Dan
