The size of Queue_t and StaticQueue_t does not match, apparently typedef struct xSTATIC_QUEUE contains an extra pointer. (IDFGH-8303)

[Clor0xD]

Answers checklist.

• I have read the documentation ESP-IDF Programming Guide and the issue is not addressed there.
• I have updated my IDF branch (master or release) to the latest version and checked that the issue is present there.
• I have searched the issue tracker for a similar issue and not found a similar issue.

General issue report

ESP_IDF 4.4.2
The size of Queue_t and StaticQueue_t does not match, apparently typedef struct xSTATIC_QUEUE contains an extra pointer.

"queue.c"

typedef struct QueueDefinition /* The old naming convention is used to prevent breaking kernel aware debuggers. */
{

   //there are 2 pointers here
    int8_t * pcHead;           /*< Points to the beginning of the queue storage area. */
    int8_t * pcWriteTo;        /*< Points to the free next place in the storage area. */

    union
    {
        QueuePointers_t xQueue;     /*< Data required exclusively when this structure is used as a queue. */
        SemaphoreData_t xSemaphore; /*< Data required exclusively when this structure is used as a semaphore. */
    } u;

    List_t xTasksWaitingToSend;             /*< List of tasks that are blocked waiting to post onto this queue.  Stored in priority order. */
    List_t xTasksWaitingToReceive;          /*< List of tasks that are blocked waiting to read from this queue.  Stored in priority order. */

    volatile UBaseType_t uxMessagesWaiting; /*< The number of items currently in the queue. */
    UBaseType_t uxLength;                   /*< The length of the queue defined as the number of items it will hold, not the number of bytes. */
    UBaseType_t uxItemSize;                 /*< The size of each items that the queue will hold. */

    volatile int8_t cRxLock;                /*< Stores the number of items received from the queue (removed from the queue) while the queue was locked.  Set to queueUNLOCKED when the queue is not locked. */
    volatile int8_t cTxLock;                /*< Stores the number of items transmitted to the queue (added to the queue) while the queue was locked.  Set to queueUNLOCKED when the queue is not locked. */

    #if ( ( configSUPPORT_STATIC_ALLOCATION == 1 ) && ( configSUPPORT_DYNAMIC_ALLOCATION == 1 ) )
        uint8_t ucStaticallyAllocated; /*< Set to pdTRUE if the memory used by the queue was statically allocated to ensure no attempt is made to free the memory. */
    #endif

    #if ( configUSE_QUEUE_SETS == 1 )
        struct QueueDefinition * pxQueueSetContainer;
    #endif

    #if ( configUSE_TRACE_FACILITY == 1 )
        UBaseType_t uxQueueNumber;
        uint8_t ucQueueType;
    #endif
#ifdef ESP_PLATFORM
    portMUX_TYPE mux;       //Mutex required due to SMP
#endif // ESP_PLATFORM
} xQUEUE;

/* The old xQUEUE name is maintained above then typedefed to the new Queue_t
 * name below to enable the use of older kernel aware debuggers. */
typedef xQUEUE Queue_t;

"FreeRTOS.h"

typedef struct xSTATIC_QUEUE
{
   //there are 3 pointers here
    void * pvDummy1[ 3 ];

    union
    {
        void * pvDummy2;
        UBaseType_t uxDummy2;
    } u;

    StaticList_t xDummy3[ 2 ];
    UBaseType_t uxDummy4[ 3 ];
    uint8_t ucDummy5[ 2 ];

    #if ( ( configSUPPORT_STATIC_ALLOCATION == 1 ) && ( configSUPPORT_DYNAMIC_ALLOCATION == 1 ) )
        uint8_t ucDummy6;
    #endif

    #if ( configUSE_QUEUE_SETS == 1 )
        void * pvDummy7;
    #endif

    #if ( configUSE_TRACE_FACILITY == 1 )
        UBaseType_t uxDummy8;
        uint8_t ucDummy9;
    #endif
    portMUX_TYPE xDummy10;
} StaticQueue_t;
typedef StaticQueue_t StaticSemaphore_t;

[negativekelvin]

Why do you want them to be the same size?

[Clor0xD]

@negativekelvin
So this is the same descriptor Queue_t in the rtos pipeline. Just for static memory allocation, it had to be moved to the public header and obfuscate. Here is an official comment from freertos with guarantees of size and alignment.

/*
 * In line with software engineering best practice, especially when supplying a
 * library that is likely to change in future versions, FreeRTOS implements a
 * strict data hiding policy.  This means the Queue structure used internally by
 * FreeRTOS is not accessible to application code.  However, if the application
 * writer wants to statically allocate the memory required to create a queue
 * then the size of the queue object needs to be know.  The StaticQueue_t
 * structure below is provided for this purpose.  Its sizes and alignment
 * requirements are guaranteed to match those of the genuine structure, no
 * matter which architecture is being used, and no matter how the values in
 * FreeRTOSConfig.h are set.  Its contents are somewhat obfuscated in the hope
 * users will recognise that it would be unwise to make direct use of the
 * structure members.
 */

probably such differences can lead to unpredictable behavior of user code if the programmer relies on guarantees.

I understand that in most cases now static allocation just consumes 4 bytes more memory per structure. More is not less so it works. However, a bug is a bug.

I apologize for my poor knowledge of English

[negativekelvin]

This exists in upstream freertos so maybe better to raise the issue there?
https://github.com/FreeRTOS/FreeRTOS-Kernel/blob/ff88fc8b6ce9f0c53b06dc46238e08a99ca2c55a/include/FreeRTOS.h#L1322

[Clor0xD]

The size was broken by modifying the file "queue.c" precisely in the ESP-IDF fork. In the vanilla version, everything just fits together. I was looking at the differences and came across this error. Before publishing, I checked the ESP-IDF code and vanilla FREERTOS code. The esp-idf version does not matter, there is a bug in both versions 4 and 5.
@negativekelvin ESP-IDF FreeRTOS is noticeably different from Vanilla FreeRTOS so the kernel files have been modified.

[negativekelvin]

Vanilla version still has 3 pointer vs 2 pointer issue. IDF added a mux to both structs.

https://github.com/FreeRTOS/FreeRTOS-Kernel/blob/ff88fc8b6ce9f0c53b06dc46238e08a99ca2c55a/queue.c#L102

[Clor0xD]

@negativekelvin
I figured out more deeply in each field of the structure. Still, I was in a hurry, even though I was sure. In my checks, the size did not match because I missed the macro definition when compiling the test code. Although it's probably worth adding the #ifdef section to FreeRTOS.h as it was done in queue.c

queue.c 143-145

#ifdef ESP_PLATFORM
    portMUX_TYPE mux;       //Mutex required due to SMP
#endif // ESP_PLATFORM

FreeRTOS.h 1298-1300

    #endif
    portMUX_TYPE xDummy10;
} StaticQueue_t;

Thank you very much for the time spent on the proceedings.

[Alvin1Zhang]

Thanks for reporting and sharing the updates, fix on release/4.4 is available at 06cad10. Feel free to reopen.

[xentec]

Backporting this to v5.0 broke the following code:

static struct {
	TaskHandle_t task;
	SemaphoreHandle_t sem;
	StaticSemaphore_t _sem;
	bool enabled;

	const char *something;
} state;

void init(const char* something) {
	state.something = something;
	state.sem = xSemaphoreCreateMutexStatic(&state._sem);
	// state.something is overwritten here
}

After init(...) is done, state.something becomes something else instead of the passed argument.
Reverting fae2522 on release/v5.0 fixes it in this case.

[xentec]

Disregard that comment as the issue only happens with FreeRTOS kernel from Amazon.
Sorry for false alarm.