From 9c7d1b7a28563b0174539990edc90aff621d4074 Mon Sep 17 00:00:00 2001 From: Ubuntu Date: Tue, 24 Oct 2023 03:51:01 +0000 Subject: [PATCH 1/6] Suppress MISRA C rule 11.3 in MISRA.md * FreeRTOS supports static allocation. Suppress this rule to allow conversion of static structure type to internal structure type. --- MISRA.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/MISRA.md b/MISRA.md index e7ebf77ea6a..f1cce2be430 100644 --- a/MISRA.md +++ b/MISRA.md @@ -63,6 +63,10 @@ Copy below content to `misra.conf` to run Coverity on FreeRTOS-Kernel. deviation: "Rule 8.7", reason: "API functions are not used by the library outside of the files they are defined; however, they must be externally visible in order to be used by an application." }, + { + deviation: "Rule 11.3", + reason: "Allow kernel object static allocation for StaticEventGroup_t, StaticQueue_t, StaticStreamBuffer_t, StaticTimer_t and StaticTask_t." + }, { deviation: "Rule 11.5", reason: "Allow casts from `void *`. List owner, pvOwner, is stored as `void *` and are cast to various types for use in functions." From d078cd687baec901a1d220f07f020bcf4090ddab Mon Sep 17 00:00:00 2001 From: Ubuntu Date: Sat, 28 Oct 2023 05:57:54 +0000 Subject: [PATCH 2/6] Use comment suppression instead of config suppression --- MISRA.md | 16 +++++++++++----- event_groups.c | 11 ++++++++++- queue.c | 8 +++++++- stream_buffer.c | 13 +++++++++++-- tasks.c | 8 +++++++- timers.c | 8 +++++++- 6 files changed, 53 insertions(+), 11 deletions(-) diff --git a/MISRA.md b/MISRA.md index f1cce2be430..a7277b330e3 100644 --- a/MISRA.md +++ b/MISRA.md @@ -31,6 +31,16 @@ _Ref 8.4.1_ a declaration in header file is not useful as the assembly code will still need to declare it separately. + +#### Rule 11.3 + +_Ref 11.3.1_ + +- MISRA C:2012 Rule 11.3: A cast shall not be performed between a pointer to object + type and a pointer to a different object type. + The rule requires not to cast a pointer to object into a pointer to a different object to prevent undefined behavior due to incorrectly aligned. To support static memory allocation, FreeRTOS creates static type kernel objects which are aliases for kernel object type with prefix "Static" for data hiding purpose. A static kernel object type is guaranteed to have the same size and alignment with kernel object, which is checked by configASSERT. Static kernel object types include StaticEventGroup_t, StaticQueue_t, StaticStreamBuffer_t, StaticTimer_t and StaticTask_t. + + ### MISRA configuration Copy below content to `misra.conf` to run Coverity on FreeRTOS-Kernel. @@ -63,14 +73,10 @@ Copy below content to `misra.conf` to run Coverity on FreeRTOS-Kernel. deviation: "Rule 8.7", reason: "API functions are not used by the library outside of the files they are defined; however, they must be externally visible in order to be used by an application." }, - { - deviation: "Rule 11.3", - reason: "Allow kernel object static allocation for StaticEventGroup_t, StaticQueue_t, StaticStreamBuffer_t, StaticTimer_t and StaticTask_t." - }, { deviation: "Rule 11.5", reason: "Allow casts from `void *`. List owner, pvOwner, is stored as `void *` and are cast to various types for use in functions." } ] } -``` \ No newline at end of file +``` diff --git a/event_groups.c b/event_groups.c index 556637b4cda..e6fc7e68cc0 100644 --- a/event_groups.c +++ b/event_groups.c @@ -98,7 +98,10 @@ static BaseType_t prvTestWaitCondition( const EventBits_t uxCurrentEventBits, #endif /* configASSERT_DEFINED */ /* The user has provided a statically allocated event group - use it. */ - pxEventBits = ( EventGroup_t * ) pxEventGroupBuffer; /*lint !e740 !e9087 EventGroup_t and StaticEventGroup_t are deliberately aliased for data hiding purposes and guaranteed to have the same size and alignment requirement - checked by configASSERT(). */ + /* MISRA Ref 11.3.1 [Misaligned access] */ + /* More details at: https://github.com/FreeRTOS/FreeRTOS-Kernel/blob/main/MISRA.md#rule-113 */ + /* coverity[misra_c_2012_rule_11_3_violation] */ + pxEventBits = ( EventGroup_t * ) pxEventGroupBuffer; if( pxEventBits != NULL ) { @@ -710,6 +713,9 @@ void vEventGroupDelete( EventGroupHandle_t xEventGroup ) /* Check if the event group was statically allocated. */ if( pxEventBits->ucStaticallyAllocated == ( uint8_t ) pdTRUE ) { + /* MISRA Ref 11.3.1 [Misaligned access] */ + /* More details at: https://github.com/FreeRTOS/FreeRTOS-Kernel/blob/main/MISRA.md#rule-113 */ + /* coverity[misra_c_2012_rule_11_3_violation] */ *ppxEventGroupBuffer = ( StaticEventGroup_t * ) pxEventBits; xReturn = pdTRUE; } @@ -721,6 +727,9 @@ void vEventGroupDelete( EventGroupHandle_t xEventGroup ) #else /* configSUPPORT_DYNAMIC_ALLOCATION */ { /* Event group must have been statically allocated. */ + /* MISRA Ref 11.3.1 [Misaligned access] */ + /* More details at: https://github.com/FreeRTOS/FreeRTOS-Kernel/blob/main/MISRA.md#rule-113 */ + /* coverity[misra_c_2012_rule_11_3_violation] */ *ppxEventGroupBuffer = ( StaticEventGroup_t * ) pxEventBits; xReturn = pdTRUE; } diff --git a/queue.c b/queue.c index 3bc959721ed..1324a7eaa85 100644 --- a/queue.c +++ b/queue.c @@ -408,7 +408,10 @@ BaseType_t xQueueGenericReset( QueueHandle_t xQueue, /* The address of a statically allocated queue was passed in, use it. * The address of a statically allocated storage area was also passed in * but is already set. */ - pxNewQueue = ( Queue_t * ) pxStaticQueue; /*lint !e740 !e9087 Unusual cast is ok as the structures are designed to have the same alignment, and the size is checked by an assert. */ + /* MISRA Ref 11.3.1 [Misaligned access] */ + /* More details at: https://github.com/FreeRTOS/FreeRTOS-Kernel/blob/main/MISRA.md#rule-113 */ + /* coverity[misra_c_2012_rule_11_3_violation] */ + pxNewQueue = ( Queue_t * ) pxStaticQueue; #if ( configSUPPORT_DYNAMIC_ALLOCATION == 1 ) { @@ -459,6 +462,9 @@ BaseType_t xQueueGenericReset( QueueHandle_t xQueue, *ppucQueueStorage = ( uint8_t * ) pxQueue->pcHead; } +/* MISRA Ref 11.3.1 [Misaligned access] */ +/* More details at: https://github.com/FreeRTOS/FreeRTOS-Kernel/blob/main/MISRA.md#rule-113 */ +/* coverity[misra_c_2012_rule_11_3_violation] */ *ppxStaticQueue = ( StaticQueue_t * ) pxQueue; xReturn = pdTRUE; } diff --git a/stream_buffer.c b/stream_buffer.c index 32aa8e4f2b3..32b56fa8905 100644 --- a/stream_buffer.c +++ b/stream_buffer.c @@ -406,7 +406,10 @@ static void prvInitialiseNewStreamBuffer( StreamBuffer_t * const pxStreamBuffer, StreamBufferCallbackFunction_t pxSendCompletedCallback, StreamBufferCallbackFunction_t pxReceiveCompletedCallback ) { - StreamBuffer_t * const pxStreamBuffer = ( StreamBuffer_t * ) pxStaticStreamBuffer; /*lint !e740 !e9087 Safe cast as StaticStreamBuffer_t is opaque Streambuffer_t. */ + /* MISRA Ref 11.3.1 [Misaligned access] */ + /* More details at: https://github.com/FreeRTOS/FreeRTOS-Kernel/blob/main/MISRA.md#rule-113 */ + /* coverity[misra_c_2012_rule_11_3_violation] */ + StreamBuffer_t * const pxStreamBuffer = ( StreamBuffer_t * ) pxStaticStreamBuffer; StreamBufferHandle_t xReturn; uint8_t ucFlags; @@ -466,7 +469,10 @@ static void prvInitialiseNewStreamBuffer( StreamBuffer_t * const pxStreamBuffer, traceSTREAM_BUFFER_CREATE( pxStreamBuffer, xIsMessageBuffer ); - xReturn = ( StreamBufferHandle_t ) pxStaticStreamBuffer; /*lint !e9087 Data hiding requires cast to opaque type. */ + /* MISRA Ref 11.3.1 [Misaligned access] */ + /* More details at: https://github.com/FreeRTOS/FreeRTOS-Kernel/blob/main/MISRA.md#rule-113 */ + /* coverity[misra_c_2012_rule_11_3_violation] */ + xReturn = ( StreamBufferHandle_t ) pxStaticStreamBuffer; } else { @@ -498,6 +504,9 @@ static void prvInitialiseNewStreamBuffer( StreamBuffer_t * const pxStreamBuffer, if( ( pxStreamBuffer->ucFlags & sbFLAGS_IS_STATICALLY_ALLOCATED ) != ( uint8_t ) 0 ) { *ppucStreamBufferStorageArea = pxStreamBuffer->pucBuffer; + /* MISRA Ref 11.3.1 [Misaligned access] */ + /* More details at: https://github.com/FreeRTOS/FreeRTOS-Kernel/blob/main/MISRA.md#rule-113 */ + /* coverity[misra_c_2012_rule_11_3_violation] */ *ppxStaticStreamBuffer = ( StaticStreamBuffer_t * ) pxStreamBuffer; xReturn = pdTRUE; } diff --git a/tasks.c b/tasks.c index d74a4ccac88..a17c3bffc58 100644 --- a/tasks.c +++ b/tasks.c @@ -1271,7 +1271,10 @@ static void prvAddNewTaskToReadyList( TCB_t * pxNewTCB ) PRIVILEGED_FUNCTION; { /* The memory used for the task's TCB and stack are passed into this * function - use them. */ - pxNewTCB = ( TCB_t * ) pxTaskBuffer; /*lint !e740 !e9087 Unusual cast is ok as the structures are designed to have the same alignment, and the size is checked by an assert. */ + /* MISRA Ref 11.3.1 [Misaligned access] */ + /* More details at: https://github.com/FreeRTOS/FreeRTOS-Kernel/blob/main/MISRA.md#rule-113 */ + /* coverity[misra_c_2012_rule_11_3_violation] */ + pxNewTCB = ( TCB_t * ) pxTaskBuffer; ( void ) memset( ( void * ) pxNewTCB, 0x00, sizeof( TCB_t ) ); pxNewTCB->pxStack = ( StackType_t * ) puxStackBuffer; @@ -4319,6 +4322,9 @@ char * pcTaskGetName( TaskHandle_t xTaskToQuery ) /*lint !e971 Unqualified char if( pxTCB->ucStaticallyAllocated == tskSTATICALLY_ALLOCATED_STACK_AND_TCB ) { *ppuxStackBuffer = pxTCB->pxStack; + /* MISRA Ref 11.3.1 [Misaligned access] */ + /* More details at: https://github.com/FreeRTOS/FreeRTOS-Kernel/blob/main/MISRA.md#rule-113 */ + /* coverity[misra_c_2012_rule_11_3_violation] */ *ppxTaskBuffer = ( StaticTask_t * ) pxTCB; xReturn = pdTRUE; } diff --git a/timers.c b/timers.c index bff14ea7ecb..d9287c02c6d 100644 --- a/timers.c +++ b/timers.c @@ -394,7 +394,10 @@ /* A pointer to a StaticTimer_t structure MUST be provided, use it. */ configASSERT( pxTimerBuffer ); - pxNewTimer = ( Timer_t * ) pxTimerBuffer; /*lint !e740 !e9087 StaticTimer_t is a pointer to a Timer_t, so guaranteed to be aligned and sized correctly (checked by an assert()), so this is safe. */ + /* MISRA Ref 11.3.1 [Misaligned access] */ + /* More details at: https://github.com/FreeRTOS/FreeRTOS-Kernel/blob/main/MISRA.md#rule-113 */ + /* coverity[misra_c_2012_rule_11_3_violation] */ + pxNewTimer = ( Timer_t * ) pxTimerBuffer; if( pxNewTimer != NULL ) { @@ -664,6 +667,9 @@ if( ( pxTimer->ucStatus & tmrSTATUS_IS_STATICALLY_ALLOCATED ) != 0 ) { + /* MISRA Ref 11.3.1 [Misaligned access] */ + /* More details at: https://github.com/FreeRTOS/FreeRTOS-Kernel/blob/main/MISRA.md#rule-113 */ + /* coverity[misra_c_2012_rule_11_3_violation] */ *ppxTimerBuffer = ( StaticTimer_t * ) pxTimer; xReturn = pdTRUE; } From d5024ea972ffdc253d2ae444b5b9018084a1f528 Mon Sep 17 00:00:00 2001 From: Ubuntu Date: Sat, 28 Oct 2023 10:24:43 +0000 Subject: [PATCH 3/6] Fix format --- MISRA.md | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/MISRA.md b/MISRA.md index a7277b330e3..e2316d85411 100644 --- a/MISRA.md +++ b/MISRA.md @@ -38,7 +38,14 @@ _Ref 11.3.1_ - MISRA C:2012 Rule 11.3: A cast shall not be performed between a pointer to object type and a pointer to a different object type. - The rule requires not to cast a pointer to object into a pointer to a different object to prevent undefined behavior due to incorrectly aligned. To support static memory allocation, FreeRTOS creates static type kernel objects which are aliases for kernel object type with prefix "Static" for data hiding purpose. A static kernel object type is guaranteed to have the same size and alignment with kernel object, which is checked by configASSERT. Static kernel object types include StaticEventGroup_t, StaticQueue_t, StaticStreamBuffer_t, StaticTimer_t and StaticTask_t. + The rule requires not to cast a pointer to object into a pointer to a + different object to prevent undefined behavior due to incorrectly aligned. + To support static memory allocation, FreeRTOS creates static type kernel + objects which are aliases for kernel object type with prefix "Static" for + data hiding purpose. A static kernel object type is guaranteed to have the + same size and alignment with kernel object, which is checked by configASSERT. + Static kernel object types include StaticEventGroup_t, StaticQueue_t, + StaticStreamBuffer_t, StaticTimer_t and StaticTask_t. ### MISRA configuration From 073e2ce8a371a0d85991014464cb46699d02cd35 Mon Sep 17 00:00:00 2001 From: Ubuntu Date: Mon, 30 Oct 2023 07:21:17 +0000 Subject: [PATCH 4/6] Fix queue.c format --- queue.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/queue.c b/queue.c index 1324a7eaa85..a9a65496d18 100644 --- a/queue.c +++ b/queue.c @@ -462,9 +462,9 @@ BaseType_t xQueueGenericReset( QueueHandle_t xQueue, *ppucQueueStorage = ( uint8_t * ) pxQueue->pcHead; } -/* MISRA Ref 11.3.1 [Misaligned access] */ -/* More details at: https://github.com/FreeRTOS/FreeRTOS-Kernel/blob/main/MISRA.md#rule-113 */ -/* coverity[misra_c_2012_rule_11_3_violation] */ + /* MISRA Ref 11.3.1 [Misaligned access] */ + /* More details at: https://github.com/FreeRTOS/FreeRTOS-Kernel/blob/main/MISRA.md#rule-113 */ + /* coverity[misra_c_2012_rule_11_3_violation] */ *ppxStaticQueue = ( StaticQueue_t * ) pxQueue; xReturn = pdTRUE; } From 8c2503c289a60a049d8d0055b51bda9280383453 Mon Sep 17 00:00:00 2001 From: Gaurav Aggarwal Date: Tue, 28 Nov 2023 12:14:08 +0000 Subject: [PATCH 5/6] Code review suggestions Signed-off-by: Gaurav Aggarwal --- MISRA.md | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/MISRA.md b/MISRA.md index e2316d85411..90df7b3e00d 100644 --- a/MISRA.md +++ b/MISRA.md @@ -36,16 +36,18 @@ _Ref 8.4.1_ _Ref 11.3.1_ -- MISRA C:2012 Rule 11.3: A cast shall not be performed between a pointer to object - type and a pointer to a different object type. - The rule requires not to cast a pointer to object into a pointer to a - different object to prevent undefined behavior due to incorrectly aligned. - To support static memory allocation, FreeRTOS creates static type kernel - objects which are aliases for kernel object type with prefix "Static" for - data hiding purpose. A static kernel object type is guaranteed to have the - same size and alignment with kernel object, which is checked by configASSERT. - Static kernel object types include StaticEventGroup_t, StaticQueue_t, - StaticStreamBuffer_t, StaticTimer_t and StaticTask_t. +- MISRA C:2012 Rule 11.3: A cast shall not be performed between a pointer to + object type and a pointer to a different object type. + This rule prohibits casting a pointer to object into a pointer to a + different object because it may result in an incorrectly aligned pointer, + leading to undefined behavior. Even if the casting produces a correctly + aligned pointer, the behavior may be still undefined if the pointer is + used to access an object. FreeRTOS deliberately creates external aliases + for all the kernel object types (StaticEventGroup_t, StaticQueue_t, + StaticStreamBuffer_t, StaticTimer_t and StaticTask_t) for data hiding + purposes. The internal object types and the corresponding external + aliases are guarnteed to have the same size and alignment which is + checked using configASSERT. ### MISRA configuration From 09ac5281c7e1f9f5e89b9fe6561084e091f66e20 Mon Sep 17 00:00:00 2001 From: Gaurav Aggarwal Date: Tue, 28 Nov 2023 12:20:46 +0000 Subject: [PATCH 6/6] Fix spelling Signed-off-by: Gaurav Aggarwal --- MISRA.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MISRA.md b/MISRA.md index 90df7b3e00d..b6a5ee197f3 100644 --- a/MISRA.md +++ b/MISRA.md @@ -46,7 +46,7 @@ _Ref 11.3.1_ for all the kernel object types (StaticEventGroup_t, StaticQueue_t, StaticStreamBuffer_t, StaticTimer_t and StaticTask_t) for data hiding purposes. The internal object types and the corresponding external - aliases are guarnteed to have the same size and alignment which is + aliases are guaranteed to have the same size and alignment which is checked using configASSERT.