PIC port stack corruption/overflow

nobody wrote on Friday, January 27, 2006:

I have just traced a bug in the PIC port. See my comments below.

Ben Tucker
MPC Data Limited.

* I have added the pxStackStart parameter that needs to be set to the start
* address of the stack to be used. This allows us to define a location for the
* stack in the link script. This is needed to overcome a fundamental problem
* with the PIC port of FreeRTOS.
* Function call and return code loads the new stack pointer (FSR1) in
* two operations: load FSR1L and then load FSR1H. The problem is that if the
* stack crosses a page boundary, and a scheduler interrupt comes in-between
* the update of the two halves of the stack pointer, then it will be corrupted
* when running the task switch code.
* Ben Tucker
*
*/
signed portBASE_TYPE xTaskCreate(
        pdTASK_CODE pvTaskCode,
        const signed portCHAR * const pcName,
        portSTACK_TYPE * pxStackStart,
        unsigned portSHORT usStackDepth,
        void *pvParameters,
        unsigned portBASE_TYPE uxPriority,
        xTaskHandle *pxCreatedTask );

rtel wrote on Friday, January 27, 2006:

Hi Ben,

Thanks for the info.  It’s been a while since I looked at the PIC port - I would be grateful if you could send me the updated file so I can look in more detail.  Can you send it to the email address on the Contacts page of the FreeRTOS site (r dot barry at freertos dot ___org___).

Regards.

nobody wrote on Friday, July 21, 2006:

Hello Ben,

I tried to understand your report of 2006-01-27, after being notified of it by Richard.

I assume you use the microchip C18 compiler, since it isn´t specified in the post.

I fail to see the supposed bug, since FSR1 is _never_ changed in 2 operations during normal function calling.
The compiler uses POSTINC1 and POSTDEC1 registers, which properly handle overflow of the lower byte of FSR1 into the high byte. The PIC18C reference manual says: "Incrementing or decrementing an FSR affects all 12 bits. That is, when FSRnL overflows from an increment, FSRnH will be incremented automatically."

FSR1 is the stack pointer, points to first UNused location
FSR1L: adres 0xFE1
FSR1H: adres 0xFE2
PREINC: adres 0xFE4
POSTDEC: adres 0xFE5
POSTINC: adres 0xFE6

FSR2 is the frame pointer
FSR2H: adres 0xFDA
FSR2L: adres 0xFD9

Function call:
140:                   vStartIntegerMathTasks( tskIDLE_PRIORITY );
  3DA8    6AE6     CLRF 0xfe6, ACCESS        // Push parameter (with value: 0) on the stack through postinc1
  3DAA    EC88     CALL 0x3b10, 0        // Call function
  3DAC    F01D     NOP
  3DAE    52E5     MOVF 0xfe5, F, ACCESS    // Discard parameter from stack hrough postdec1.
                        // FSR1 is restored to value before calling the functions.

Overhead inside a function:

On entering function:
Push old frame pointer FSR2 on stack and set FSR2 to FSR1
  434C    CFD9     MOVFF 0xfd9, 0xfe6        // Push FSR2L on stack and increase FSR1
  434E    FFE6     NOP                // through indirect adressing through postinc1
  4350    CFDA     MOVFF 0xfda, 0xfe6        // Push FSR2H on stack and increase FSR1
  4352    FFE6     NOP                // though indirect adressing through postinc1
  4354    CFE1     MOVFF 0xfe1, 0xfd9        // Copy FSR1L to FRS2L, FSR1 not modified
  4356    FFD9     NOP
  4358    CFE2     MOVFF 0xfe2, 0xfda        // Copy FSR1H to FRS2H, FSR1 not modified
  435A    FFDA     NOP

On exit function:
Restore old frame pointer from stack
  3F80    52E5     MOVF 0xfe5, F, ACCESS    // Discard unused top of stack through postdec1
  3F82    CFE5     MOVFF 0xfe5, 0xfda        // Restore FSR2H through postdec1
  3F84    FFDA     NOP
  3F86    CFE7     MOVFF 0xfe7, 0xfd9        // Restore FSR2L from IND1, do not decrement pointer!
  3F88    FFD9     NOP
  3F8A    0012     RETURN 0

FSR1 (FSR1L + FSR1H) are always consistent during normal function calling.

If there would be a problem it would be in FSR2, since FSR2 IS saved and restored from the stack in 2 steps.
However, an inconsistent FSR2 (through an interrupt in between the 2 assignments) is never used in the interrupt context, since the interrupt function immediately sets a new frame pointer. Storage of an inconsistent FSR2 on the stack is no problem. On leaving the interrupt routine, the inconsistent FSR2L and FSR2H pair have been restored and the next instruction (remember where we were?) restores FSR2 to a consistent state.

Maybe what you are referring to is the FSR1 save and restore by the FreeRTOS framework? since this does take 2 instructions.
This is also safe since through a taskswitch, interrupts are always off:
portSAVE_CONTEXT clears the global interrupt pretty early, and interrupts aren´t enabled again until exiting portRESTORE_CONTEXT()

I don´t think there is any need to change anything.

Paul van der Hulst