From d3ae502ba257d8c208581c81b33fb75e2c8a1408 Mon Sep 17 00:00:00 2001 From: Christopher Head Date: Thu, 16 Apr 2020 17:35:25 -0700 Subject: [PATCH] Obey strict aliasing rule in linked lists --- event_groups.c | 10 +++++----- include/list.h | 36 +++++++++++++++++------------------- list.c | 38 +++++++++++++++++++------------------- 3 files changed, 41 insertions(+), 43 deletions(-) diff --git a/event_groups.c b/event_groups.c index 0bf3b9661..c1ba5b30e 100644 --- a/event_groups.c +++ b/event_groups.c @@ -518,8 +518,8 @@ EventBits_t uxReturn; EventBits_t xEventGroupSetBits( EventGroupHandle_t xEventGroup, const EventBits_t uxBitsToSet ) { -ListItem_t *pxListItem, *pxNext; -ListItem_t const *pxListEnd; +MiniListItem_t *pxListItem, *pxNext; +MiniListItem_t const *pxListEnd; List_t const * pxList; EventBits_t uxBitsToClear = 0, uxBitsWaitedFor, uxControlBits; EventGroup_t *pxEventBits = xEventGroup; @@ -545,7 +545,7 @@ BaseType_t xMatchFound = pdFALSE; while( pxListItem != pxListEnd ) { pxNext = listGET_NEXT( pxListItem ); - uxBitsWaitedFor = listGET_LIST_ITEM_VALUE( pxListItem ); + uxBitsWaitedFor = listGET_LIST_ITEM_VALUE( ( ListItem_t * ) pxListItem ); xMatchFound = pdFALSE; /* Split the bits waited for from the control bits. */ @@ -591,7 +591,7 @@ BaseType_t xMatchFound = pdFALSE; eventUNBLOCKED_DUE_TO_BIT_SET bit is set so the task knows that is was unblocked due to its required bits matching, rather than because it timed out. */ - vTaskRemoveFromUnorderedEventList( pxListItem, pxEventBits->uxEventBits | eventUNBLOCKED_DUE_TO_BIT_SET ); + vTaskRemoveFromUnorderedEventList( ( ListItem_t * ) pxListItem, pxEventBits->uxEventBits | eventUNBLOCKED_DUE_TO_BIT_SET ); } /* Move onto the next list item. Note pxListItem->pxNext is not @@ -624,7 +624,7 @@ const List_t *pxTasksWaitingForBits = &( pxEventBits->xTasksWaitingForBits ); /* Unblock the task, returning 0 as the event list is being deleted and cannot therefore have any bits set. */ configASSERT( pxTasksWaitingForBits->xListEnd.pxNext != ( const ListItem_t * ) &( pxTasksWaitingForBits->xListEnd ) ); - vTaskRemoveFromUnorderedEventList( pxTasksWaitingForBits->xListEnd.pxNext, eventUNBLOCKED_DUE_TO_BIT_SET ); + vTaskRemoveFromUnorderedEventList( (ListItem_t *) pxTasksWaitingForBits->xListEnd.pxNext, eventUNBLOCKED_DUE_TO_BIT_SET ); } #if( ( configSUPPORT_DYNAMIC_ALLOCATION == 1 ) && ( configSUPPORT_STATIC_ALLOCATION == 0 ) ) diff --git a/include/list.h b/include/list.h index 0598a935f..fcf87b6ea 100644 --- a/include/list.h +++ b/include/list.h @@ -137,27 +137,25 @@ use of FreeRTOS.*/ * Definition of the only type of object that a list can contain. */ struct xLIST; -struct xLIST_ITEM -{ - listFIRST_LIST_ITEM_INTEGRITY_CHECK_VALUE /*< Set to a known value if configUSE_LIST_DATA_INTEGRITY_CHECK_BYTES is set to 1. */ - configLIST_VOLATILE TickType_t xItemValue; /*< The value being listed. In most cases this is used to sort the list in descending order. */ - struct xLIST_ITEM * configLIST_VOLATILE pxNext; /*< Pointer to the next ListItem_t in the list. */ - struct xLIST_ITEM * configLIST_VOLATILE pxPrevious; /*< Pointer to the previous ListItem_t in the list. */ - void * pvOwner; /*< Pointer to the object (normally a TCB) that contains the list item. There is therefore a two way link between the object containing the list item and the list item itself. */ - struct xLIST * configLIST_VOLATILE pxContainer; /*< Pointer to the list in which this list item is placed (if any). */ - listSECOND_LIST_ITEM_INTEGRITY_CHECK_VALUE /*< Set to a known value if configUSE_LIST_DATA_INTEGRITY_CHECK_BYTES is set to 1. */ -}; -typedef struct xLIST_ITEM ListItem_t; /* For some reason lint wants this as two separate definitions. */ struct xMINI_LIST_ITEM { listFIRST_LIST_ITEM_INTEGRITY_CHECK_VALUE /*< Set to a known value if configUSE_LIST_DATA_INTEGRITY_CHECK_BYTES is set to 1. */ configLIST_VOLATILE TickType_t xItemValue; - struct xLIST_ITEM * configLIST_VOLATILE pxNext; - struct xLIST_ITEM * configLIST_VOLATILE pxPrevious; + struct xMINI_LIST_ITEM * configLIST_VOLATILE pxNext; + struct xMINI_LIST_ITEM * configLIST_VOLATILE pxPrevious; }; typedef struct xMINI_LIST_ITEM MiniListItem_t; +struct xLIST_ITEM +{ + MiniListItem_t xMiniItem; /*< The item value and next and previous pointers. */ + void * pvOwner; /*< Pointer to the object (normally a TCB) that contains the list item. There is therefore a two way link between the object containing the list item and the list item itself. */ + struct xLIST * configLIST_VOLATILE pxContainer; /*< Pointer to the list in which this list item is placed (if any). */ + listSECOND_LIST_ITEM_INTEGRITY_CHECK_VALUE /*< Set to a known value if configUSE_LIST_DATA_INTEGRITY_CHECK_BYTES is set to 1. */ +}; +typedef struct xLIST_ITEM ListItem_t; /* For some reason lint wants this as two separate definitions. */ + /* * Definition of the type of queue used by the scheduler. */ @@ -165,7 +163,7 @@ typedef struct xLIST { listFIRST_LIST_INTEGRITY_CHECK_VALUE /*< Set to a known value if configUSE_LIST_DATA_INTEGRITY_CHECK_BYTES is set to 1. */ volatile UBaseType_t uxNumberOfItems; - ListItem_t * configLIST_VOLATILE pxIndex; /*< Used to walk through the list. Points to the last item returned by a call to listGET_OWNER_OF_NEXT_ENTRY (). */ + MiniListItem_t * configLIST_VOLATILE pxIndex; /*< Used to walk through the list. Points to the last item returned by a call to listGET_OWNER_OF_NEXT_ENTRY (). */ MiniListItem_t xListEnd; /*< List item that contains the maximum possible item value meaning it is always at the end of the list and is therefore used as a marker. */ listSECOND_LIST_INTEGRITY_CHECK_VALUE /*< Set to a known value if configUSE_LIST_DATA_INTEGRITY_CHECK_BYTES is set to 1. */ } List_t; @@ -195,7 +193,7 @@ typedef struct xLIST * \page listSET_LIST_ITEM_VALUE listSET_LIST_ITEM_VALUE * \ingroup LinkedList */ -#define listSET_LIST_ITEM_VALUE( pxListItem, xValue ) ( ( pxListItem )->xItemValue = ( xValue ) ) +#define listSET_LIST_ITEM_VALUE( pxListItem, xValue ) ( ( pxListItem )->xMiniItem.xItemValue = ( xValue ) ) /* * Access macro to retrieve the value of the list item. The value can @@ -205,7 +203,7 @@ typedef struct xLIST * \page listGET_LIST_ITEM_VALUE listGET_LIST_ITEM_VALUE * \ingroup LinkedList */ -#define listGET_LIST_ITEM_VALUE( pxListItem ) ( ( pxListItem )->xItemValue ) +#define listGET_LIST_ITEM_VALUE( pxListItem ) ( ( pxListItem )->xMiniItem.xItemValue ) /* * Access macro to retrieve the value of the list item at the head of a given @@ -238,7 +236,7 @@ typedef struct xLIST * \page listGET_END_MARKER listGET_END_MARKER * \ingroup LinkedList */ -#define listGET_END_MARKER( pxList ) ( ( ListItem_t const * ) ( &( ( pxList )->xListEnd ) ) ) +#define listGET_END_MARKER( pxList ) ( ( MiniListItem_t const * ) ( &( ( pxList )->xListEnd ) ) ) /* * Access macro to determine if a list contains any items. The macro will @@ -284,7 +282,7 @@ List_t * const pxConstList = ( pxList ); \ { \ ( pxConstList )->pxIndex = ( pxConstList )->pxIndex->pxNext; \ } \ - ( pxTCB ) = ( pxConstList )->pxIndex->pvOwner; \ + ( pxTCB ) = ( ( ListItem_t * ) ( pxConstList )->pxIndex )->pvOwner; \ } @@ -304,7 +302,7 @@ List_t * const pxConstList = ( pxList ); \ * \page listGET_OWNER_OF_HEAD_ENTRY listGET_OWNER_OF_HEAD_ENTRY * \ingroup LinkedList */ -#define listGET_OWNER_OF_HEAD_ENTRY( pxList ) ( (&( ( pxList )->xListEnd ))->pxNext->pvOwner ) +#define listGET_OWNER_OF_HEAD_ENTRY( pxList ) ( ( ( ListItem_t * ) (&( ( pxList )->xListEnd ))->pxNext )->pvOwner ) /* * Check to see if a list item is within a list. The list item maintains a diff --git a/list.c b/list.c index 0e0e72d82..fd1e223c3 100644 --- a/list.c +++ b/list.c @@ -39,7 +39,7 @@ void vListInitialise( List_t * const pxList ) /* The list structure contains a list item which is used to mark the end of the list. To initialise the list the list end is inserted as the only list entry. */ - pxList->pxIndex = ( ListItem_t * ) &( pxList->xListEnd ); /*lint !e826 !e740 !e9087 The mini list structure is used as the list end to save RAM. This is checked and valid. */ + pxList->pxIndex = &( pxList->xListEnd ); /*lint !e826 !e740 !e9087 The mini list structure is used as the list end to save RAM. This is checked and valid. */ /* The list end value is the highest possible value in the list to ensure it remains at the end of the list. */ @@ -47,8 +47,8 @@ void vListInitialise( List_t * const pxList ) /* The list end next and previous pointers point to itself so we know when the list is empty. */ - pxList->xListEnd.pxNext = ( ListItem_t * ) &( pxList->xListEnd ); /*lint !e826 !e740 !e9087 The mini list structure is used as the list end to save RAM. This is checked and valid. */ - pxList->xListEnd.pxPrevious = ( ListItem_t * ) &( pxList->xListEnd );/*lint !e826 !e740 !e9087 The mini list structure is used as the list end to save RAM. This is checked and valid. */ + pxList->xListEnd.pxNext = &( pxList->xListEnd ); /*lint !e826 !e740 !e9087 The mini list structure is used as the list end to save RAM. This is checked and valid. */ + pxList->xListEnd.pxPrevious = &( pxList->xListEnd );/*lint !e826 !e740 !e9087 The mini list structure is used as the list end to save RAM. This is checked and valid. */ pxList->uxNumberOfItems = ( UBaseType_t ) 0U; @@ -73,7 +73,7 @@ void vListInitialiseItem( ListItem_t * const pxItem ) void vListInsertEnd( List_t * const pxList, ListItem_t * const pxNewListItem ) { -ListItem_t * const pxIndex = pxList->pxIndex; +MiniListItem_t * const pxIndex = pxList->pxIndex; /* Only effective when configASSERT() is also defined, these tests may catch the list data structures being overwritten in memory. They will not catch @@ -84,14 +84,14 @@ ListItem_t * const pxIndex = pxList->pxIndex; /* Insert a new list item into pxList, but rather than sort the list, makes the new list item the last item to be removed by a call to listGET_OWNER_OF_NEXT_ENTRY(). */ - pxNewListItem->pxNext = pxIndex; - pxNewListItem->pxPrevious = pxIndex->pxPrevious; + pxNewListItem->xMiniItem.pxNext = pxIndex; + pxNewListItem->xMiniItem.pxPrevious = pxIndex->pxPrevious; /* Only used during decision coverage testing. */ mtCOVERAGE_TEST_DELAY(); - pxIndex->pxPrevious->pxNext = pxNewListItem; - pxIndex->pxPrevious = pxNewListItem; + pxIndex->pxPrevious->pxNext = &pxNewListItem->xMiniItem; + pxIndex->pxPrevious = &pxNewListItem->xMiniItem; /* Remember which list the item is in. */ pxNewListItem->pxContainer = pxList; @@ -102,8 +102,8 @@ ListItem_t * const pxIndex = pxList->pxIndex; void vListInsert( List_t * const pxList, ListItem_t * const pxNewListItem ) { -ListItem_t *pxIterator; -const TickType_t xValueOfInsertion = pxNewListItem->xItemValue; +MiniListItem_t *pxIterator; +const TickType_t xValueOfInsertion = pxNewListItem->xMiniItem.xItemValue; /* Only effective when configASSERT() is also defined, these tests may catch the list data structures being overwritten in memory. They will not catch @@ -147,17 +147,17 @@ const TickType_t xValueOfInsertion = pxNewListItem->xItemValue; before vTaskStartScheduler() has been called?). **********************************************************************/ - for( pxIterator = ( ListItem_t * ) &( pxList->xListEnd ); pxIterator->pxNext->xItemValue <= xValueOfInsertion; pxIterator = pxIterator->pxNext ) /*lint !e826 !e740 !e9087 The mini list structure is used as the list end to save RAM. This is checked and valid. *//*lint !e440 The iterator moves to a different value, not xValueOfInsertion. */ + for( pxIterator = &( pxList->xListEnd ); pxIterator->pxNext->xItemValue <= xValueOfInsertion; pxIterator = pxIterator->pxNext ) /*lint !e826 !e740 !e9087 The mini list structure is used as the list end to save RAM. This is checked and valid. *//*lint !e440 The iterator moves to a different value, not xValueOfInsertion. */ { /* There is nothing to do here, just iterating to the wanted insertion position. */ } } - pxNewListItem->pxNext = pxIterator->pxNext; - pxNewListItem->pxNext->pxPrevious = pxNewListItem; - pxNewListItem->pxPrevious = pxIterator; - pxIterator->pxNext = pxNewListItem; + pxNewListItem->xMiniItem.pxNext = pxIterator->pxNext; + pxNewListItem->xMiniItem.pxNext->pxPrevious = &pxNewListItem->xMiniItem; + pxNewListItem->xMiniItem.pxPrevious = pxIterator; + pxIterator->pxNext = &pxNewListItem->xMiniItem; /* Remember which list the item is in. This allows fast removal of the item later. */ @@ -173,16 +173,16 @@ UBaseType_t uxListRemove( ListItem_t * const pxItemToRemove ) item. */ List_t * const pxList = pxItemToRemove->pxContainer; - pxItemToRemove->pxNext->pxPrevious = pxItemToRemove->pxPrevious; - pxItemToRemove->pxPrevious->pxNext = pxItemToRemove->pxNext; + pxItemToRemove->xMiniItem.pxNext->pxPrevious = pxItemToRemove->xMiniItem.pxPrevious; + pxItemToRemove->xMiniItem.pxPrevious->pxNext = pxItemToRemove->xMiniItem.pxNext; /* Only used during decision coverage testing. */ mtCOVERAGE_TEST_DELAY(); /* Make sure the index is left pointing to a valid item. */ - if( pxList->pxIndex == pxItemToRemove ) + if( pxList->pxIndex == &pxItemToRemove->xMiniItem ) { - pxList->pxIndex = pxItemToRemove->pxPrevious; + pxList->pxIndex = pxItemToRemove->xMiniItem.pxPrevious; } else {