From 759e0bf9552fdab59c58d9cdb9f496ba4d852c04 Mon Sep 17 00:00:00 2001 From: Kody Stribrny Date: Tue, 21 Jan 2025 08:29:00 -0800 Subject: [PATCH 01/10] fix: SA violation fixes and simplification for idle task length restrictions This change: * Removes the dependency on strings.h for the prvCreateIdleTask function * Resolves several static analysis violations reported by tools like Parasoft Builds off of - https://github.com/FreeRTOS/FreeRTOS-Kernel/pull/1203 --- tasks.c | 33 ++++++++++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/tasks.c b/tasks.c index d7153f680e..a5aeb60730 100644 --- a/tasks.c +++ b/tasks.c @@ -156,6 +156,23 @@ #define configIDLE_TASK_NAME "IDLE" #endif +#if ( configNUMBER_OF_CORES > 1 ) + /* Reserve space for Core ID and null termination */ + #if ( configMAX_TASK_NAME_LEN < 2U ) + #error Minimum required task name length is 2. Please increase configMAX_TASK_NAME_LEN. + #endif + #define taskRESERVED_TASK_NAME_LENGTH 2U + +#elif ( configNUMBER_OF_CORES > 10 ) + #warning Please increase taskRESERVED_TASK_NAME_LENGTH. 1 character is insufficient to store the core ID. +#else + /* Reserve space for null termination */ + #if ( configMAX_TASK_NAME_LEN < 1U ) + #error Minimum required task name length is 1. Please increase configMAX_TASK_NAME_LEN. + #endif + #define taskRESERVED_TASK_NAME_LENGTH 1U +#endif /* if ( ( configNUMBER_OF_CORES > 1 ) */ + #if ( configUSE_PORT_OPTIMISED_TASK_SELECTION == 0 ) /* If configUSE_PORT_OPTIMISED_TASK_SELECTION is 0 then task selection is @@ -3531,17 +3548,27 @@ static BaseType_t prvCreateIdleTasks( void ) BaseType_t xIdleNameLen; BaseType_t xCopyLen; - configASSERT( ( configIDLE_TASK_NAME != NULL ) && ( configMAX_TASK_NAME_LEN > 3 ) ); - /* The length of the idle task name is limited to the minimum of the length * of configIDLE_TASK_NAME and configMAX_TASK_NAME_LEN - 2, keeping space * for the core ID suffix and the null-terminator. */ xIdleNameLen = strlen( configIDLE_TASK_NAME ); xCopyLen = xIdleNameLen < ( configMAX_TASK_NAME_LEN - 2 ) ? xIdleNameLen : ( configMAX_TASK_NAME_LEN - 2 ); - for( xIdleTaskNameIndex = ( BaseType_t ) 0; xIdleTaskNameIndex < xCopyLen; xIdleTaskNameIndex++ ) + for( xIdleTaskNameIndex = ( BaseType_t ) 0; xIdleTaskNameIndex < ( configMAX_TASK_NAME_LEN - taskRESERVED_TASK_NAME_LENGTH ); xIdleTaskNameIndex++ ) { cIdleName[ xIdleTaskNameIndex ] = configIDLE_TASK_NAME[ xIdleTaskNameIndex ]; + + /* Don't copy all configMAX_TASK_NAME_LEN if the string is shorter than + * configMAX_TASK_NAME_LEN characters just in case the memory after the + * string is not accessible (extremely unlikely). */ + if( cIdleName[ xIdleTaskNameIndex ] == ( char ) 0x00 ) + { + break; + } + else + { + mtCOVERAGE_TEST_MARKER(); + } } /* Ensure null termination. */ From 9462d4abdb73c0eafc480bd5d256897270606938 Mon Sep 17 00:00:00 2001 From: Kody Stribrny Date: Tue, 21 Jan 2025 08:50:32 -0800 Subject: [PATCH 02/10] Delete the strlen call --- tasks.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/tasks.c b/tasks.c index a5aeb60730..d0640c6553 100644 --- a/tasks.c +++ b/tasks.c @@ -3545,14 +3545,6 @@ static BaseType_t prvCreateIdleTasks( void ) char cIdleName[ configMAX_TASK_NAME_LEN ] = { 0 }; TaskFunction_t pxIdleTaskFunction = NULL; BaseType_t xIdleTaskNameIndex; - BaseType_t xIdleNameLen; - BaseType_t xCopyLen; - - /* The length of the idle task name is limited to the minimum of the length - * of configIDLE_TASK_NAME and configMAX_TASK_NAME_LEN - 2, keeping space - * for the core ID suffix and the null-terminator. */ - xIdleNameLen = strlen( configIDLE_TASK_NAME ); - xCopyLen = xIdleNameLen < ( configMAX_TASK_NAME_LEN - 2 ) ? xIdleNameLen : ( configMAX_TASK_NAME_LEN - 2 ); for( xIdleTaskNameIndex = ( BaseType_t ) 0; xIdleTaskNameIndex < ( configMAX_TASK_NAME_LEN - taskRESERVED_TASK_NAME_LENGTH ); xIdleTaskNameIndex++ ) { From b52f825aea628ba310ea57758ae8499c90de83de Mon Sep 17 00:00:00 2001 From: Kody Stribrny Date: Tue, 21 Jan 2025 09:00:57 -0800 Subject: [PATCH 03/10] Further SA fixes --- tasks.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tasks.c b/tasks.c index d0640c6553..97838f248c 100644 --- a/tasks.c +++ b/tasks.c @@ -163,7 +163,7 @@ #endif #define taskRESERVED_TASK_NAME_LENGTH 2U -#elif ( configNUMBER_OF_CORES > 10 ) +#elif ( configNUMBER_OF_CORES > 9 ) #warning Please increase taskRESERVED_TASK_NAME_LENGTH. 1 character is insufficient to store the core ID. #else /* Reserve space for null termination */ @@ -3544,9 +3544,9 @@ static BaseType_t prvCreateIdleTasks( void ) BaseType_t xCoreID; char cIdleName[ configMAX_TASK_NAME_LEN ] = { 0 }; TaskFunction_t pxIdleTaskFunction = NULL; - BaseType_t xIdleTaskNameIndex; + UBaseType_t xIdleTaskNameIndex; - for( xIdleTaskNameIndex = ( BaseType_t ) 0; xIdleTaskNameIndex < ( configMAX_TASK_NAME_LEN - taskRESERVED_TASK_NAME_LENGTH ); xIdleTaskNameIndex++ ) + for( xIdleTaskNameIndex = 0U; xIdleTaskNameIndex < ( configMAX_TASK_NAME_LEN - taskRESERVED_TASK_NAME_LENGTH ); xIdleTaskNameIndex++ ) { cIdleName[ xIdleTaskNameIndex ] = configIDLE_TASK_NAME[ xIdleTaskNameIndex ]; From 795941bacba9f445a052bd0e7ef9ca79986bd6ee Mon Sep 17 00:00:00 2001 From: Kody Stribrny Date: Fri, 24 Jan 2025 09:24:18 -0800 Subject: [PATCH 04/10] Add MISRA suppressions for coverity --- MISRA.md | 19 +++++++++++++++++++ tasks.c | 9 ++++++--- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/MISRA.md b/MISRA.md index 4355ec6781..6b13ecac43 100644 --- a/MISRA.md +++ b/MISRA.md @@ -115,6 +115,25 @@ _Ref 11.5.5_ because data storage buffers are implemented as uint8_t arrays for the ease of sizing, alignment and access. +#### Rule 14.3 + +MISRA C-2012 Rule 14.3: Controlling expressions shall not be invariant. + +_Ref 14.3.1_ + - The `configMAX_TASK_NAME_LEN` and `taskRESERVED_TASK_NAME_LENGTH` values + constant at compile time however can vary depending on build configuration. + This condition takes into account the build configuration of the system. + +#### Rule 18.1 + +MISRA C-2012 Rule 18.1: A pointer resulting from arithmetic on a pointer operand +shall address an element of the same array as that pointer operand. + +_Ref 18.1_ + - The array access is limited to in-bounds values as the null termination + of the IDLE task name results in breaking from the loop. Alternatively, if + the size is smaller than the IDLE task name length, the loop will exit normally. + #### Rule 21.6 MISRA C-2012 Rule 21.6: The Standard Library input/output functions shall not diff --git a/tasks.c b/tasks.c index 97838f248c..2d5d2154e6 100644 --- a/tasks.c +++ b/tasks.c @@ -3546,13 +3546,16 @@ static BaseType_t prvCreateIdleTasks( void ) TaskFunction_t pxIdleTaskFunction = NULL; UBaseType_t xIdleTaskNameIndex; + /* MISRA Ref 18.1 [Configuration dependent invariant] */ + /* More details at: https://github.com/FreeRTOS/FreeRTOS-Kernel/blob/main/MISRA.md#rule-143 */ + /* coverity[misra_c_2012_rule_14_3_violation] */ for( xIdleTaskNameIndex = 0U; xIdleTaskNameIndex < ( configMAX_TASK_NAME_LEN - taskRESERVED_TASK_NAME_LENGTH ); xIdleTaskNameIndex++ ) { cIdleName[ xIdleTaskNameIndex ] = configIDLE_TASK_NAME[ xIdleTaskNameIndex ]; - /* Don't copy all configMAX_TASK_NAME_LEN if the string is shorter than - * configMAX_TASK_NAME_LEN characters just in case the memory after the - * string is not accessible (extremely unlikely). */ + /* MISRA Ref 18.1.1 [Configuration dependent bounds checking] */ + /* More details at: https://github.com/FreeRTOS/FreeRTOS-Kernel/blob/main/MISRA.md#rule-181 */ + /* coverity[misra_c_2012_rule_18_1_violation] */ if( cIdleName[ xIdleTaskNameIndex ] == ( char ) 0x00 ) { break; From 23b0424dddeed4abbe6c8c09f2c4d8c05cf44e55 Mon Sep 17 00:00:00 2001 From: Kody Stribrny Date: Fri, 24 Jan 2025 09:30:14 -0800 Subject: [PATCH 05/10] Fix 14.3 comment --- tasks.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tasks.c b/tasks.c index 2d5d2154e6..5b402b3906 100644 --- a/tasks.c +++ b/tasks.c @@ -3546,7 +3546,7 @@ static BaseType_t prvCreateIdleTasks( void ) TaskFunction_t pxIdleTaskFunction = NULL; UBaseType_t xIdleTaskNameIndex; - /* MISRA Ref 18.1 [Configuration dependent invariant] */ + /* MISRA Ref 14.3 [Configuration dependent invariant] */ /* More details at: https://github.com/FreeRTOS/FreeRTOS-Kernel/blob/main/MISRA.md#rule-143 */ /* coverity[misra_c_2012_rule_14_3_violation] */ for( xIdleTaskNameIndex = 0U; xIdleTaskNameIndex < ( configMAX_TASK_NAME_LEN - taskRESERVED_TASK_NAME_LENGTH ); xIdleTaskNameIndex++ ) From fa0fb6f4d8192789614d72d024f5c07452bd50e3 Mon Sep 17 00:00:00 2001 From: Kody Stribrny Date: Fri, 24 Jan 2025 09:31:38 -0800 Subject: [PATCH 06/10] Fix exception file reference --- MISRA.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MISRA.md b/MISRA.md index 6b13ecac43..a21bd132e5 100644 --- a/MISRA.md +++ b/MISRA.md @@ -119,7 +119,7 @@ _Ref 11.5.5_ MISRA C-2012 Rule 14.3: Controlling expressions shall not be invariant. -_Ref 14.3.1_ +_Ref 14.3_ - The `configMAX_TASK_NAME_LEN` and `taskRESERVED_TASK_NAME_LENGTH` values constant at compile time however can vary depending on build configuration. This condition takes into account the build configuration of the system. From ea2a07694af38a4ea859a1e5c8e71dd34b415750 Mon Sep 17 00:00:00 2001 From: Kody Stribrny Date: Fri, 24 Jan 2025 10:43:33 -0800 Subject: [PATCH 07/10] Correcting suppression line --- tasks.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tasks.c b/tasks.c index 5b402b3906..e5077eab6a 100644 --- a/tasks.c +++ b/tasks.c @@ -3551,11 +3551,11 @@ static BaseType_t prvCreateIdleTasks( void ) /* coverity[misra_c_2012_rule_14_3_violation] */ for( xIdleTaskNameIndex = 0U; xIdleTaskNameIndex < ( configMAX_TASK_NAME_LEN - taskRESERVED_TASK_NAME_LENGTH ); xIdleTaskNameIndex++ ) { - cIdleName[ xIdleTaskNameIndex ] = configIDLE_TASK_NAME[ xIdleTaskNameIndex ]; - /* MISRA Ref 18.1.1 [Configuration dependent bounds checking] */ /* More details at: https://github.com/FreeRTOS/FreeRTOS-Kernel/blob/main/MISRA.md#rule-181 */ /* coverity[misra_c_2012_rule_18_1_violation] */ + cIdleName[ xIdleTaskNameIndex ] = configIDLE_TASK_NAME[ xIdleTaskNameIndex ]; + if( cIdleName[ xIdleTaskNameIndex ] == ( char ) 0x00 ) { break; From 48e41661a8ced82996a8d00e323059c09b1b4c2d Mon Sep 17 00:00:00 2001 From: Kody Stribrny Date: Fri, 24 Jan 2025 10:45:44 -0800 Subject: [PATCH 08/10] Fix suppression numbering --- tasks.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tasks.c b/tasks.c index e5077eab6a..acb7a9b324 100644 --- a/tasks.c +++ b/tasks.c @@ -3546,7 +3546,7 @@ static BaseType_t prvCreateIdleTasks( void ) TaskFunction_t pxIdleTaskFunction = NULL; UBaseType_t xIdleTaskNameIndex; - /* MISRA Ref 14.3 [Configuration dependent invariant] */ + /* MISRA Ref 14.3.1 [Configuration dependent invariant] */ /* More details at: https://github.com/FreeRTOS/FreeRTOS-Kernel/blob/main/MISRA.md#rule-143 */ /* coverity[misra_c_2012_rule_14_3_violation] */ for( xIdleTaskNameIndex = 0U; xIdleTaskNameIndex < ( configMAX_TASK_NAME_LEN - taskRESERVED_TASK_NAME_LENGTH ); xIdleTaskNameIndex++ ) From f9c020eaebfae2b85f59161214d7bf7ceb7af5e0 Mon Sep 17 00:00:00 2001 From: Kody Stribrny <89810515+kstribrnAmzn@users.noreply.github.com> Date: Tue, 28 Jan 2025 07:21:30 -0800 Subject: [PATCH 09/10] Apply suggestions from code review Comment updates Co-authored-by: Gaurav-Aggarwal-AWS <33462878+aggarg@users.noreply.github.com> --- MISRA.md | 7 +++++-- tasks.c | 8 ++++---- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/MISRA.md b/MISRA.md index a21bd132e5..117e9eb488 100644 --- a/MISRA.md +++ b/MISRA.md @@ -120,7 +120,8 @@ _Ref 11.5.5_ MISRA C-2012 Rule 14.3: Controlling expressions shall not be invariant. _Ref 14.3_ - - The `configMAX_TASK_NAME_LEN` and `taskRESERVED_TASK_NAME_LENGTH` values + - The `configMAX_TASK_NAME_LEN` and `taskRESERVED_TASK_NAME_LENGTH` are + evaluated to constants at compile time and may vary based on the build configuration. constant at compile time however can vary depending on build configuration. This condition takes into account the build configuration of the system. @@ -130,7 +131,9 @@ MISRA C-2012 Rule 18.1: A pointer resulting from arithmetic on a pointer operand shall address an element of the same array as that pointer operand. _Ref 18.1_ - - The array access is limited to in-bounds values as the null termination + - Array access remains within bounds since either the null terminator in + the IDLE task name will break the loop, or the loop will break normally + if the array size is smaller than the IDLE task name length. of the IDLE task name results in breaking from the loop. Alternatively, if the size is smaller than the IDLE task name length, the loop will exit normally. diff --git a/tasks.c b/tasks.c index acb7a9b324..518c9e87f1 100644 --- a/tasks.c +++ b/tasks.c @@ -157,7 +157,7 @@ #endif #if ( configNUMBER_OF_CORES > 1 ) - /* Reserve space for Core ID and null termination */ + /* Reserve space for Core ID and null termination. */ #if ( configMAX_TASK_NAME_LEN < 2U ) #error Minimum required task name length is 2. Please increase configMAX_TASK_NAME_LEN. #endif @@ -166,7 +166,7 @@ #elif ( configNUMBER_OF_CORES > 9 ) #warning Please increase taskRESERVED_TASK_NAME_LENGTH. 1 character is insufficient to store the core ID. #else - /* Reserve space for null termination */ + /* Reserve space for null termination. */ #if ( configMAX_TASK_NAME_LEN < 1U ) #error Minimum required task name length is 1. Please increase configMAX_TASK_NAME_LEN. #endif @@ -3547,12 +3547,12 @@ static BaseType_t prvCreateIdleTasks( void ) UBaseType_t xIdleTaskNameIndex; /* MISRA Ref 14.3.1 [Configuration dependent invariant] */ - /* More details at: https://github.com/FreeRTOS/FreeRTOS-Kernel/blob/main/MISRA.md#rule-143 */ + /* More details at: https://github.com/FreeRTOS/FreeRTOS-Kernel/blob/main/MISRA.md#rule-143. */ /* coverity[misra_c_2012_rule_14_3_violation] */ for( xIdleTaskNameIndex = 0U; xIdleTaskNameIndex < ( configMAX_TASK_NAME_LEN - taskRESERVED_TASK_NAME_LENGTH ); xIdleTaskNameIndex++ ) { /* MISRA Ref 18.1.1 [Configuration dependent bounds checking] */ - /* More details at: https://github.com/FreeRTOS/FreeRTOS-Kernel/blob/main/MISRA.md#rule-181 */ + /* More details at: https://github.com/FreeRTOS/FreeRTOS-Kernel/blob/main/MISRA.md#rule-181. */ /* coverity[misra_c_2012_rule_18_1_violation] */ cIdleName[ xIdleTaskNameIndex ] = configIDLE_TASK_NAME[ xIdleTaskNameIndex ]; From 1f59ce71b1552d1b2cfc4016fdb722cca7a196f8 Mon Sep 17 00:00:00 2001 From: Kody Stribrny Date: Tue, 28 Jan 2025 07:42:33 -0800 Subject: [PATCH 10/10] Fix github batch addition --- MISRA.md | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/MISRA.md b/MISRA.md index 117e9eb488..b5941037ff 100644 --- a/MISRA.md +++ b/MISRA.md @@ -121,9 +121,8 @@ MISRA C-2012 Rule 14.3: Controlling expressions shall not be invariant. _Ref 14.3_ - The `configMAX_TASK_NAME_LEN` and `taskRESERVED_TASK_NAME_LENGTH` are - evaluated to constants at compile time and may vary based on the build configuration. - constant at compile time however can vary depending on build configuration. - This condition takes into account the build configuration of the system. + evaluated to constants at compile time and may vary based on the build + configuration. #### Rule 18.1 @@ -134,8 +133,6 @@ _Ref 18.1_ - Array access remains within bounds since either the null terminator in the IDLE task name will break the loop, or the loop will break normally if the array size is smaller than the IDLE task name length. - of the IDLE task name results in breaking from the loop. Alternatively, if - the size is smaller than the IDLE task name length, the loop will exit normally. #### Rule 21.6