Not using pxIndex to iterate ready list in trace utility

chinglee-iot commented 4 months ago

Description

pxIndex in List_t should only be used when selecting next task to run. Altering pxIndex of a ready list will cause the scheduler unable to select the right task to run.

Use prvListTasksWithinSingleList as an example.

prvListTasksWithinSingleList is called in uxTaskGetSystemState and pxIndex is used to traverse the ready list in listGET_OWNER_OF_NEXT_ENTRY. This will cause the pxIndex points to a different task after uxTaskGetSystemState is called. Single core FreeRTOS make use of pxIndex to select next task to run. The scheduler will select wrong task since the pxIndex is moved. This also causes problem In FreeRTOS SMP as pxIndex is not moved in SMP and it should always point to xListEnd.

Example code

TaskStatus_t xTaskStatusArray[ 32 ];

void vTestTask( void * pvParam )
{
    /* Break point 1 to observe the pxReadyTaskLists[1]. */    

    uxTaskGetSystemState( xTaskStatusArray,
                          32,
                          NULL );

    /* Break point 2 to observe the pxReadyTaskLists[1] again to see if pxIndex is changed. */

    for(;;);
}

void test_main( void )
{
    uint32_t i;

    /* This is the only running task in single core FreeRTOS due to highest priority. */
    xTaskCreate( vTestTask, "TestTask", configMINIMAL_STACK_SIZE, NULL, 2, NULL );

    for( i = 1; i < 10; i++ )
    {
        xTaskCreate( vTestTask, "TestTask", configMINIMAL_STACK_SIZE, NULL, 1, NULL );
    }

    vTaskStartScheduler();
}

The result shows that print pxReadyTasksLists[1].pxIndex points to different items in the list after calling uxTaskGetSystemState.

Break point 1 to observe pxIndex before calling uxTaskGetSystemState Thread 2 "TestTask" hit Breakpoint 1, vTestTask (pvParam=0x0) at main.c:158 158 uxTaskGetSystemState( xTaskStatusArray, (gdb) print pxReadyTasksLists[1] $1 = {uxNumberOfItems = 9, pxIndex = 0x55555565e418 <pxReadyTasksLists+56>, xListEnd = {xItemValue = 18446744073709551615, pxNext = 0x5555556b6918, pxPrevious = 0x5555556d7ba8}} (gdb) print &pxReadyTasksLists[1].xListEnd $2 = (MiniListItem_t *) 0x55555565e418 <pxReadyTasksLists+56> (gdb) n 162 for(;;);

Break point 2 to observe pxIndex again (gdb) print pxReadyTasksLists[1] $3 = {uxNumberOfItems = 9, pxIndex = 0x5555556b6918, xListEnd = {xItemValue = 18446744073709551615, pxNext = 0x5555556b6918, pxPrevious = 0x5555556d7ba8}} (gdb)

In this PR

Use for loop to traverse a ready list for trace utility instead of listGET_OWNER_OF_NEXT_ENTRY

Test Steps

Running the example above. pxIndex should not be changed after uxTaskGetSystemState is called.

Checklist:

[x] I have tested my changes. No regression in existing tests.
[x] I have modified and/or added unit-tests to cover the code changes in this Pull Request. - https://github.com/FreeRTOS/FreeRTOS/pull/1186

Related Issue

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

sonarcloud[bot] commented 4 months ago

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

codecov[bot] commented 4 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 93.00%. Comparing base (cff947a) to head (9f6f787).

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #1000 +/- ## ========================================== + Coverage 92.97% 93.00% +0.02% ========================================== Files 6 6 Lines 3219 3200 -19 Branches 890 879 -11 ========================================== - Hits 2993 2976 -17 Misses 111 111 + Partials 115 113 -2 ``` | [Flag](https://app.codecov.io/gh/FreeRTOS/FreeRTOS-Kernel/pull/1000/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=FreeRTOS) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/FreeRTOS/FreeRTOS-Kernel/pull/1000/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=FreeRTOS) | `93.00% <100.00%> (+0.02%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=FreeRTOS#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

FreeRTOS / FreeRTOS-Kernel