GCC bug or FreeRTOS bug?

[After Forever]

Posted on September 21, 2017 at 13:19

I spent 3 hours debugging this. I'm using STM32L051 with FreeRTOS (initial code generated by STM32CubeMX v4.1). GCC version is the one that's coming with Ubuntu 04 by default:

~$ arm-none-eabi-gcc --version

arm-none-eabi-gcc (15:4.9.3+svn231177-1) 4.9.3 20150529 (prerelease) Copyright (C) 2014 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

I'm using the -Os optimization flag. I can't turn off the optimization because the binary ends up being bigger than my flash.

I faced strange hard fault errors with some values of my tasks' stack sizes. After some debugging I realized that the top of the stack address of some of my tasks was invalid. Some more debugging and I found the offending part.

Look at this line of the function prvInitialiseNewTask in tasks.c of FreeRTOS' code where the pxTopOfStack variable corrupts:

 {
 pxTopOfStack = pxNewTCB->pxStack + ( ulStackDepth - ( uint32_t ) 1 );
 pxTopOfStack = ( StackType_t * ) ( ( ( portPOINTER_SIZE_TYPE ) pxTopOfStack ) & ( ~( ( portPOINTER_SIZE_TYPE ) portBYTE_ALIGNMENT_MASK ) ) ); /*lint !e923 MISRA exception. Avoiding casts between pointers and integers is not practical. Size differences accounted for using portPOINTER_SIZE_TYPE type. */
 /* Check the alignment of the calculated top of stack is correct. */
 configASSERT( ( ( ( portPOINTER_SIZE_TYPE ) pxTopOfStack & ( portPOINTER_SIZE_TYPE ) portBYTE_ALIGNMENT_MASK ) == 0UL ) );
 }�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?

And the disassembly (with my comments):

; ulStackDepth == r2 == 0x00000200
; r5 == 0x2000085c
 807 [1] pxTopOfStack = pxNewTCB->pxStack + ( ulStackDepth - ( uint32_t ) 1 );
0x80030d4 <+0x0018> 44 4e ldr r6, [pc, #272] ; (0x80031e8 <xTaskCreateStatic+300>) ; r6 == 0x3fffffff
 605 [1] pxNewTCB->pxStack = ( StackType_t * ) puxStackBuffer;
0x80030d6 <+0x001a> 25 63 str r5, [r4, #48] ; 0x30
 807 [2] pxTopOfStack = pxNewTCB->pxStack + ( ulStackDepth - ( uint32_t ) 1 );
0x80030d8 <+0x001c> 92 19 adds r2, r2, r6 ; r2 == 0x400001ff
0x80030da <+0x001e> 92 00 lsls r2, r2, #2 ; r2 == 0x000007fc
0x80030dc <+0x0020> aa 18 adds r2, r5, r2 ; r2 == 0x20001058
 808 [1] pxTopOfStack = ( StackType_t * ) ( ( ( portPOINTER_SIZE_TYPE ) pxTopOfStack ) & ( ~( ( portPOINTER_SIZE_TYPE ) portBYTE_ALIGNMENT_MASK ) ) ); /*lint !e923 MISRA exception. Avoiding casts between pointers and integers is not practical. Size differences accounted for using portPOINTER_SIZE_TYPE type. */
0x80030de <+0x0022> 07 25 movs r5, #7
0x80030e0 <+0x0024> aa 43 bics r2, r5
0x80030e2 <+0x0026> 01 92 str r2, [sp, #4]�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?

I added a cast to uint32_t and then back to a pointer in the first line and now it's fixed:

 {
 pxTopOfStack = ( StackType_t * ) ( ( uint32_t )pxNewTCB->pxStack + ( ulStackDepth - ( uint32_t ) 1 ) );
 pxTopOfStack = ( StackType_t * ) ( ( ( portPOINTER_SIZE_TYPE ) pxTopOfStack ) & ( ~( ( portPOINTER_SIZE_TYPE ) portBYTE_ALIGNMENT_MASK ) ) ); /*lint !e923 MISRA exception. Avoiding casts between pointers and integers is not practical. Size differences accounted for using portPOINTER_SIZE_TYPE type. */
/* Check the alignment of the calculated top of stack is correct. */
configASSERT( ( ( ( portPOINTER_SIZE_TYPE ) pxTopOfStack & ( portPOINTER_SIZE_TYPE ) portBYTE_ALIGNMENT_MASK ) == 0UL ) );
 }
�?�?�?�?�?�?�?

The disassembly:

; ulStackDepth == r2 == 0x00000200
; r5 == 0x2000085c
0x80030d4 <+0x0018> 01 3a subsr2, #1
 807 [1] pxTopOfStack = ( StackType_t * ) ( ( uint32_t )pxNewTCB->pxStack + ( ulStackDepth - ( uint32_t ) 1 ) );
0x80030d6 <+0x001a> aa 18 addsr2, r5, r2 ; r2 == 0x20000a5b
 605 [1] pxNewTCB->pxStack = ( StackType_t * ) puxStackBuffer;
0x80030d8 <+0x001c> 25 63 strr5, [r4, #48]; 0x30
 808 [1] pxTopOfStack = ( StackType_t * ) ( ( ( portPOINTER_SIZE_TYPE ) pxTopOfStack ) & ( ~( ( portPOINTER_SIZE_TYPE ) portBYTE_ALIGNMENT_MASK ) ) ); /*lint !e923 MISRA exception. Avoiding casts between pointers and integers is not practical. Size differences accounted for using portPOINTER_SIZE_TYPE type. */
0x80030da <+0x001e> 07 25 movsr5, #7
0x80030dc <+0x0020> aa 43 bicsr2, r5
0x80030de <+0x0022> 01 92 strr2, [sp, #4]
�?�?�?�?�?�?�?�?�?�?�?�?

So to sum up, pxNewTCB->pxStack is a pointer to uint32_t, casting it to uint32_t helped to fix the bug in the calculation process.

Is this a GCC bug or maybe mixing a pointer and uint32_t in an arithmetic expression is undefined behavior and then this is a FreeRTOS bug?

The funny thing is that after fixing this I'm facing another hard fault in another place. Gone debugging...

#gcc #freertos

[waclawek.jan]

Posted on September 21, 2017 at 21:34

Okay, looked up the sources and docs of FreeRTOS, sorry for the mess, please disregard my previous post - I confused even myself... :|

&sharpdefine portSTACK_TYPE    uint32_t

[...]

typedef portSTACK_TYPE StackType_t;

[...][...]

StackType_t            *pxStack;

Back to the root: the OP probably mistakenly wants to enter the number of bytes.

The assembly generated by gcc presented in opening post correctly calculates the shift as 4x0x200.

(note that I am not the OP :) )

JW

[Barry.Richard]

Posted on September 22, 2017 at 01:40

(note that I am not the OP )

Ah - sorry - I didn't realise I was talking with more than one person.

[After Forever]

Posted on September 22, 2017 at 10:21

I'm so sorry, when creating a task I was passing the size of the buffer in bytes, but FreeRTOS is expecting it in words. Instead of re-reading the documentation and fixing my stupid mistake I 'fixed' FreeRTOS's way of calculating the top of the stack.

Thank you

Barry.Richard

‌ and

Waclawek.Jan

‌, I'm very sorry for wasting your time, although without your helpful comments it would take me much longer to realize my mistake. Thanks!

[David Littell]

Posted on September 22, 2017 at 18:01

Yours was not the stupid mistake.  Requiring stack size to be specified in magical-mystical-BS-nonintuitive 'units' is utterly ridiculous.