Weird FIXME

[lecopzer]

Most of pendsv_handler() have the comment above it:

/* FIXME: Without naked attribute, GCC will corrupt r7 which is used for stack
 * pointer. If so, after restoring the tasks' context, we will get wrong stack
 * pointer.
 */

Issue of r7 is due to calling convention of ARM. So why we use naked attribute to make code work should be FIXED?

I think it not a bug or something needs to be fixed, it just a kind of solution.

[jserv]

While developing mini-arm-os, I encountered a bug triggered by GCC optimizations (version 5.x).

You can check again for the generated instructions by recent GNU toolchain. Such workaround should be eliminated.

[jserv]

@lecopzer, any further evidence?

[lecopzer]

Sorry for suspending this issue.

PendSV is not only an interrupt handler but where the context switch happends r7 is usually used to preserve system call number (isr number).

Without __attribute__, it looks like:

80005c8: b480        push  {r7} 
 80005ca: af00        add r7, sp, #0
  /* Save the old task's context */
  asm volatile("mrs   r0, psp\n"
 80005cc: f3ef 8009   mrs r0, PSP

But our pendsv_handler has context switch code at end of

asm volatile("mov r0, %0\n" : : "r" (tasks[lastTask].stack));
   40       /* Restore the new task's context and jump to the task */
   41       asm volatile("ldmia r0!, {r4-r11, lr}\n"
   42                    "msr psp, r0\n"
   43                    "bx lr\n");

The core register was changed here and we even have our own return code (line 41 to 43) that the compiler will never know this.

Compiler don't know we handle core register and stack in C function in our own and will never assume user "corrupt" calling convention. Compiler is doing its jobs performing regular push r7 and pop r7 Even in 7.2.1 arm-eabi compiler, there is still push r7 in pendsv_handler

So it's reasonable to have naked attribute where we do the context switch or have our own register/stack handling in C function and should not be a FIXME.

[jserv]

I agree. Please send another pull request to revise the existing C comment to reflect its necessity. It is no longer "FIXME". Instead, it appears with "Caution".