PIC32MX port - adding support for stack overflow checking

I’ve been trying resolve some instability in an application I’m developing and have added stack overflow checking to the PIC32MX port. Before I raise a pull request in the FreeRTOS-Kernel repository I’d welcome some feedback on whether my implementation is sensible !

#if ( portHAS_STACK_OVERFLOW_CHECKING == 1 )
StackType_t *pxPortInitialiseStack( StackType_t *pxTopOfStack, StackType_t *pxEndOfStack, TaskFunction_t pxCode, void *pvParameters )
#else
StackType_t *pxPortInitialiseStack( StackType_t *pxTopOfStack, TaskFunction_t pxCode, void *pvParameters )
#endif
{
	/* Ensure 8 byte alignment is maintained when the context is popped from
	 * stack. The size of the context is 33 words (132 bytes). */
	pxTopOfStack--;
	pxTopOfStack--;

	*pxTopOfStack = (StackType_t) 0xDEADBEEF;
	pxTopOfStack--;

	#if ( portHAS_STACK_OVERFLOW_CHECKING == 1 )
		/* Store the stack limits. */
		*pxTopOfStack = (StackType_t) (pxTopOfStack + 3);
		pxTopOfStack--;
		*pxTopOfStack = (StackType_t) pxEndOfStack;
		pxTopOfStack--;
	#endif
        
    *pxTopOfStack = (StackType_t) 0x12345678;	/* Word to which the stack pointer will be left pointing after context restore. */
	pxTopOfStack--;

       ....

Hi - I see you have stored the stack boundaries on the stack itself. Can you then use these functions to check for a stack overflow? Are the added values just there to for viewing in the debugger?

Hi Richard. Good question. For a downward growing stack, I believe stack checking is performed by checking that the current stack pointer hasn’t reached the empty 0xa5a5a5a5 check value. The top of stack and end of stack pointers are just pushed for debugging and not used as far as I am aware. It was done like this in the MicroBlazeV9 port so I thought I’d be consistent. The only other ports that seems to support stack overflow checking are the Xtensa and TI C66x DSP but they just update the prototype and don’t use the pxEndOfStack parameter.

All ports implement stack overflow checking in software. This only gives a high probability of catching an overflow after it has occured.

There are some architectures that check for stack overflows in hardware. For example, the port for the Cortex-M33 family uses portHAS_STACK_OVERFLOW_CHECKING to know whether to program the stack limit registers on each context switch. The constant is required by not all implementations of the M33 provide the registers.

I see the Microblaze port is doing the same thing as your suggestion for the PIC32 here: https://github.com/FreeRTOS/FreeRTOS-Kernel/blob/main/portable/GCC/MicroBlazeV9/port.c#L130, but then it is not clear that the kernel uses those additional values anywhere. The Microblaze is a configurable core, and from this line https://github.com/FreeRTOS/FreeRTOS-Kernel/blob/main/portable/GCC/MicroBlazeV9/portmacro.h#L160 it looks like XPAR_MICROBLAZE_USE_STACK_PROTECTION may be defined when the core is configured for hardware stack protection. Either that, or Xilinx’s tools know where to look for the stack limits in the stack.

I’m not aware of the PIC32 having hardware stack protection - but the work “stack” appears in the datasheet to know for sure. I’m not sure of the value of saving the additional registers if it doesn’t provide hardware stack protection as you can probably get the same information by inspecting the pxCurrentTCB variable in the debugger.

Hi Richard,

Thank you for providing the detailed explanation above. It seems that I’d mis-interpretted the purpose of portHAS_STACK_OVERFLOW_CHECKING and my change adds no value so I’ll cancel the pull-request.