Data overriden when branch to PendSV

Question

I have a little "os" for an arm cortex m4. I implemented a wait function. But since then somehow, the context switch is corrupted. When stepping through the instructions i noticed, that for whatever reason the current_task variable gets overriden at entering the PendSV interrupt.

These are global variables

volatile struct OS_task * current_task;
volatile struct OS_task * next_task;

of the following type:

struct OS_task{
    volatile unsigned int *sp;
    void (*handler)(void * params);
    void * params;
    volatile enum task_state state;
    volatile unsigned char number;
    volatile unsigned int delay;
};

This is the scheduler function. It is called from the Systick interrupt, too.

void OS_Scheduler(void)
{
    current_task = &OS_tasktable.task_list[OS_tasktable.current_task];
    current_task->state = OS_TASK_STATE_IDLE;

    int next = OS_GetNextTask(OS_tasktable.current_task);
    while (1)
    {
        if (OS_tasktable.task_list[next].delay == 0)
            break;
        OS_tasktable.task_list[next].delay--;
        next = OS_GetNextTask(next);
    }
    OS_tasktable.current_task = next;

    next_task = &OS_tasktable.task_list[OS_tasktable.current_task];
    next_task->state = OS_TASK_STATE_ACTIVE;
    S32_SCB->ICSR |= S32_SCB_ICSR_PENDSVSET_MASK;
}

This is the PendSV handler. Although i'm working on a Cortex-M4F i do not save the FPU registers simply because i don't need floating point arithmetics.

.syntax unified

.thumb

.global PendSV_Handler
.type PendSV_Handler, %function
PendSV_Handler:
    /* Disable interrupts: */
    cpsid   i

    /* Save registers R4-R11 (32 bytes) onto current PSP (process stack
       pointer) and make the PSP point to the last stacked register (R8).*/
    mrs r0, psp
    subs    r0, #16
    stmia   r0!,{r4-r7}
    mov r4, r8
    mov r5, r9
    mov r6, r10
    mov r7, r11
    subs    r0, #32
    stmia   r0!,{r4-r7}
    subs    r0, #16

    /* Save current task's SP: */
    ldr r2, =current_task
    ldr r1, [r2]
    str r0, [r1]

    /* Load next task's SP: */
    ldr r2, =next_task
    ldr r1, [r2]
    ldr r0, [r1]

    /* Load registers R4-R11 (32 bytes) from the new PSP and make the PSP
       point to the end of the exception stack frame. */
    ldmia   r0!,{r4-r7}
    mov r8, r4
    mov r9, r5
    mov r10, r6
    mov r11, r7
    ldmia   r0!,{r4-r7}
    msr psp, r0

    /* EXC_RETURN - Thread mode with PSP: */
    ldr r0, =0xFFFFFFFD

    /* Enable interrupts: */
    cpsie   i

    bx  r0

.size PendSV_Handler, .-PendSV_Handler

This is the wait function. It is called through a SVC interrupt. After this has executed, the current_task and next_task variables are set correctly. Only upon entering the following PendSV interrupt, somehow current_task gets overriden. Which leads to both tasks getting set to the same stack -> not good.

void __os_wait_ms(unsigned int ms)
{
    struct OS_task * current;
    current = &OS_tasktable.task_list[OS_tasktable.current_task];
    current->delay = ms * OS_tasktable.delay_factor;
    OS_Scheduler();
    return;
}

If it helps: I use the S32K146EVB from NXP to be precise.

EDIT: I disable interrupts during the wait function executes, to avoid the Systick calling the Scheduler and messing things up.

cooperised cooperised · Accepted Answer · 2019-03-10T21:08:21

There may be a number of reasons for the behaviour you're observing. One is that it's not really happening; debugging across interrupt boundaries is tricky because pausing and stepping disables interrupts, so it can be difficult to keep track of the order in which things are happening. Given that the variable in question is statically allocated it's unlikely to be stack corruption, which narrows it down. But from the code you've provided, the cause is not obvious.

If I may I'd like to devote most of this answer to addressing some oddities in your context switch and scheduler. Perhaps in looking at these you'll find the answer to your original problem!

I would suggest that you call the scheduler from the context switch. That has several benefits: it simplifies the invocation of the context switch (you just have to set the PENDSV bit, no need to call the scheduler first; you no longer need a global next_task variable because the scheduler returns a pointer to the next task directly to the context switch; you no longer need to disable interrupts during the context switch (the worst that can happen if an ISR triggers a context switch while another is underway is that you just get two in succession); and you no longer need to invoke the scheduler when a task enters the waiting state (though you will need to set the PENDSV bit, the easiest way being to use an SVC handler).

Something like this in the context switch:
```
LDR r0, =OS_Scheduler
BLX r0
/* Pointer to next task is now in r0 */
```
Obviously you'd also have to rewrite your scheduler so it returns a pointer to a task structure.
Your saving and loading of r4-r11 is a bit odd. Why use STMIA and the pointer arithmetic, rather than STMDB or STMFD or PUSH (all synonyms)? Any why push the registers four at a time? Having done for example
```
MRS r1, PSP
```
you could then simply write
```
STMFD r1!, {r4-r11}
```
to push the whole lot, and the same with the pop (using LDMFD or POP).
It's not clear to me why you're creating and using a specific exception return code. The correct code should already be loaded into LR on entry to the PendSV handler. A simple
```
BX lr
```
is all you need.

Data overriden when branch to PendSV

1 Answers