For ARM EABI the required stack alignment is 8 bytes. This has recently been addressed in V 6.0.0 and onwards by providing a check on any malloced stack pointer. And while this alignment works fine (on most platforms >= 32bit, check previous post: “EABI Stack Alignment on ARM7″), the problem is what happens next. In task.c line i have the following code:

pxTopOfStack = pxNewTCB->pxStack + ( usStackDepth - ( unsigned short ) 1 );
pxTopOfStack = ( portSTACK_TYPE * ) ( ( ( uintptr_t ) pxTopOfStack ) & ( ( uintptr_t ) ~portBYTE_ALIGNMENT_MASK  ) );
/* Check the alignment of the calculated top of stack is correct. */
configASSERT( ( ( ( uintptr_t ) pxTopOfStack & ( uintptr_t ) portBYTE_ALIGNMENT_MASK ) == 0UL ) );

(Notice that I replaced unsigned long with uintptr_t in order to make compiling on 8-bit AVR working. To fix previously said bug). The assertion here does not fail. This ensures that the top of stack pointer is now aligned. Then there is a call:

pxNewTCB->pxTopOfStack = pxPortInitialiseStack( pxTopOfStack, pxTaskCode, pvParameters );

And now there is a second alignment assertion, which fails:

/* Check the alignment of the initialised stack. */
configASSERT( ( ( ( uintptr_t ) pxNewTCB->pxTopOfStack & ( uintptr_t ) portBYTE_ALIGNMENT_MASK ) == 0UL ) );





assertion "( ( ( uintptr_t ) pxNewTCB->pxTopOfStack & ( uintptr_t ) portBYTE_ALIGNMENT_MASK ) == 0UL )" failed: file "../src/freertos/tasks.c", line 508, function: xTaskGenericCreate
Exit 1

This is because the ARM7tdmi based port stacks an uneven number of elements (17) to the task stack. This does not mean that the task stack itself will be unaligned because the original top of stack was stored in the task context itself. So the second assertion does nothing to ensure that the stack of the task is aligned. Rather it checks if the size of the stored context is even or odd… I added a pxTopOfStack-; to the beginning of initializeStack to make it stack 18 elements, and now the assertion does not fail, and the code also works… Is this going to be a future requirements that all ports must initialize the task context with a number of elements which corresponds to the stack alignment?

What is the point of the line:

configASSERT( ( ( ( unsigned long ) pxNewTCB->pxTopOfStack & ( unsigned long ) portBYTE_ALIGNMENT_MASK ) == 0UL ) );

If the top of the stack has the right alignment, then you populate the stack will variables and the newly populated stack has bad alignment, it will be corrected when the task starts for the first time. When the task starts its context will be popped off, leaving the stack pointer at the top of the stack, which has already been checked for correct alignment so there is not a problem. It might be though, that the check is there for when the task is swapped out next. If it has 8 byte aligned stack entering the SWI interrupt, and has its context pushed to the stack, and that push leaves it only 4 byte aligned, then it will have 4 byte alignment  in the interrupt routine. However, interrupts don’t use the task stack. Experimenting I see that when a task starts its stack is aligned to 8, as I expected, even though the assert fails. Entering vPortYieldProcessor it also has 8 alignment, but that depends on the start up code as the SWI stack is allocated there. vPortYieldProcess then saves the task context to the task stack, and that does not effect the SWI stack, so the SWI stack remains 8 byte aligned. The same it true for the tick interrupt. So, my take is that everything is working perfectly except the assertion quoted above, which might be right for some ports, but is creating a false assert for ARM7. Maybe it is only relevant for when a stack grows the other way?

Thank you Dave. I didn’t want to jump to too many conclusions in my original post, but in fact, we have come to the exact same conclusion as you have. We also considered the stack alignment of ISR’s, but as you say, they run on their own stack. We conducted a short test with stack alignment both in task and in an ISR and found that removing the second alignment assertion is safe on ARM7. However, because we don’t have richard’s words for it yet, we didn’t remove it completely. We adjusted the size of the initial stacked context to 18 instead of 17 in order to “trick” the assertion. BTW. kudo’s to the FreeRTOS developers for a nice and much more clean V7.0.1. I like the use of taskENTER_CRITICAL instead of portENTER_CRITICAL, and the use of assertions is very good. I’d like to see even more of them implemented in the future. :-)

“I agree with Dave”.  The assertion is not correct for the ARM7 as it is.  The question is what is best to do about it.  It was put in to catch errors introduced during maintenance work, rather than when ports are first created.  One option is just to remove it, another would be to have a directive in portmacro.h, which then makes it on a port by port basis. Regards.

I want the assertion to stay in tasks.c because, while in the ARM7 case it is superfluous, in other ports it is not.  Therefore it is valuable when a port is being created.  I have therefore instead, gone through all the ARM7 ports and, in pxPortInitialiseStack(), after the line:

pxOriginalTOS = pxTopOfStack;

I have added the lines:

    /* To ensure asserts in tasks.c don't fail, although in this case the assert
    is not really required. */
    pxTopOfStack--;

This costs 4 bytes of stack per task, which is a pity, but not a killer.  That now seems to fix the incorrect assertion problem. Please let me know if you agree with this, and if indeed it is the same fix you described yourself. Thanks – feedback is always appreciated. Regards.

Indeed it is the same fix, here is the start of our initialize function as it looks right now:

portSTACK_TYPE *pxPortInitialiseStack( portSTACK_TYPE *pxTopOfStack, pdTASK_CODE pxCode, void *pvParameters )
{
portSTACK_TYPE *pxOriginalTOS;
    pxOriginalTOS = pxTopOfStack;
    /* Use this to align context to 8 bytes */
    pxTopOfStack--;

Thank you for evaluating the issue and making a decision. Some times that is last part the hardest :-) Regards,
Johan. P.s. May is remind you that the issue about using (unsigned long) in the stack alignment code still remains. The use of (uintptr_t) has been suggested and seems to be exactly what we needed (a portPOINTER_TYPE was suggested, but too comprehensive to implement in all ports). The only disadvantage to (uintptr_t) seems to be that it might not be present in all compilers, although mandated in C99 <stdint.h>. This is discussed in this thread: https://sourceforge.net/projects/freertos/forums/forum/382005/topic/3917293

P.s. May is remind you that the issue about using (unsigned long) in the stack alignment code still remains. The use of (uintptr_t) has been suggested and seems to be exactly what we needed (a portPOINTER_TYPE was suggested, but too comprehensive to implement in all ports). The only disadvantage to (uintptr_t) seems to be that it might not be present in all compilers, although mandated in C99 <stdint.h>.

That should get mopped up in a larger re-work that is planned – timescale as yet unknown due to urgent work load. Regards.