Hi,
I'm using Keil RTX5 (5.5.2) over RTOS-2 API. I'm trying to send a simple struct of data (8 bytes) from one thread to another:
typedef struct LogType { uint32_t id; uint32_t timestamp; } LogType_t;
Sending data (passing a pointer to queue) from Thread A:
LogType_t* entry = NULL; entry = (LogType_t*)osMemoryPoolAlloc(logPool, osWaitForever); assert(entry); entry->id = 0xDEADBEEF; entry->timestamp = 0xDEADBEEF; osMessageQueuePut(logQueue, &entry, NULL, osWaitForever);
Receiving data from Thread B:
osStatus_t readLogEntry(const LogType_t* extEntry) { extEntry = NULL; static LogType_t localEntry; memset((void*)&localEntry, 0x0, sizeof(LogType_t)); LogType_t* tempEntry = NULL; PRINTF("\r\n\nBEFORE"); PRINTF("\r\n%16s: %8X", "localEntry addr", &localEntry); PRINTF("\r\n%16s: %8X", "*tempEntry addr", &tempEntry); PRINTF("\r\n%16s: %8X", "tempEntry addr", tempEntry); PRINTF("\r\n%16s: %8X", "*extEntry addr", &extEntry); PRINTF("\r\n%16s: %8X", "extEntry addr", extEntry); /* Wait for log entry data from other threads */ osStatus_t status = osMessageQueueGet(logQueue, (void*)&tempEntry, NULL, osWaitForever); if (status == osOK) { PRINTF("\r\n\nAFTER"); PRINTF("\r\n%16s: %8X", "localEntry addr", &localEntry); PRINTF("\r\n%16s: %8X", "*tempEntry addr", &tempEntry); PRINTF("\r\n%16s: %8X", "tempEntry addr", tempEntry); PRINTF("\r\n%16s: %8X", "*extEntry addr", &extEntry); PRINTF("\r\n%16s: %8X", "extEntry addr", extEntry); PRINTF("\r\n\n%16s: %8X", "ID", tempEntry->id); PRINTF("\r\n%16s: %8X", "Timestamp", tempEntry->timestamp); osMemoryPoolFree(logPool, tempEntry); } return status; }
What I'm getting out of this simplified function is:
BEFORE localEntry addr: 20009E18 // ok *tempEntry addr: 20000DB8 // ok tempEntry addr: 0 // ok at this point *extEntry addr: 20000DBC // ok extEntry addr: 0 // ok at this point AFTER localEntry addr: 20009E18 // still ok *tempEntry addr: 20000DB8 // still ok tempEntry addr: 20002B58 // ok, pointing to my struct in pool *extEntry addr: 20000DBC // still ok extEntry addr: 7FF // NOT OK ID: DEADBEEF // ok Timestamp: DEADBEEF // ok
It seems the local copy of the extEntry pointer is getting overwritten during the call for reason I don't understand. Is this a bug in RTX or the RTOS-2 API or have I done some obvious mistake? I have no similar issues if the size of the struct is max. 4 bytes.
Here are the pool and queue initializations:
osMemoryPoolId_t logPool; osMessageQueueId_t logQueue; const osMemoryPoolAttr_t logMemorypoolAttr = { .name = "log_pool", }; logPool = osMemoryPoolNew(16, sizeof(LogType_t), &logMemorypoolAttr); logQueue = osMessageQueueNew(16, sizeof(LogType_t), NULL);
Environment details:
There are many issues within readLogEntr function.
LogType_t* tempEntry = NULL; ... osMessageQueueGet(logQueue, (void*)&tempEntry, NULL, osWaitForever); ... osMemoryPoolFree(logPool, tempEntry);
osMessageQueue requires the second parameter to be a pointer to buffer for message to get. Take a look at the documentation for osMessageQueueGet.
You are however passing the address of a pointer (4 bytes) which is allocated on stack. Once the function osMessageQueueGet executes it writes 8 bytes to that address and overwrites also 4 bytes of stack which corrupts local variables in function readLogEntr. The stack corruption will not occur when the size of struct is just 4 bytes (as you have noted).
You are also using osMemoryPoolFree on the same pointer which was never allocated with osMemoryPoolAlloc. Makes no sense.
Looking at the code for sending data:
entry = (LogType_t*)osMemoryPoolAlloc(logPool, osWaitForever); entry->id = 0xDEADBEEF; entry->timestamp = 0xDEADBEEF; osMessageQueuePut(logQueue, &entry, NULL, osWaitForever);
There is no need to allocate memory with osMemoryPoolAlloc for the entry. It can be simply a local variable which will get copied by osMessageQueuePut into its message queue.
Thanks Robert!
I forgot to mention that I'm actually trying to use the Zero Copy Mailbox approach to minimize the need for copying data. The actual data is not just 8 bytes. So I'm allocating the data to pool and passing only the pointer to queue. That's the reason why I'm passing the address of the pointer to osMessageQueueGet.
I tried your suggestion of copying a local variable to queue and it works nicely. I'll accept your answer since my description was lacking information.
Then I retried with original code and found the reason for stack corruption. It was actually the queue initialization. I'm only interested about the pointer in queue so element size in queue should be 4, not 8 bytes.
logPool = osMemoryPoolNew(16, sizeof(LogType_t), &logMemorypoolAttr); logQueue = osMessageQueueNew(16, sizeof(LogType_t), NULL); // <-- second parameter should be sizeof(LogType_t*)
On the other hand, I think osMemoryPoolFree makes a lot of sense with Zero Copy Mailbox because the pointer is pointing at address that was allocated by the sending thread.