Let's fix the tests enough to run with AddressSanitizer and UB Sanitizer and enable those in CI

.github/.cSpellWords.txt

@@ -454,6 +454,7 @@ FPSP
 FRAMERX
 FRMFILTER
 frms
+fsanitize
 FSDMA
 FTSR
 FUDUP

.github/workflows/ci.yml

@@ -38,27 +38,56 @@ jobs:
           echo "::endgroup::"
           echo -e "${{ env.bashPass }} ${{ env.stepName }} ${{ env.bashEnd }}"
 
+      # Separate builds for sanitizers and coverage:
+      # These can currently not be combined without branch coverage dilution.
       - env:
-          stepName: Build Unit Tests
+          stepName: Build Unit Tests (aubsan build)
         name: ${{ env.stepName }}
         run: |
           # ${{ env.stepName }}
           echo -e "::group::${{ env.bashInfo }} ${{ env.stepName }} ${{ env.bashEnd }}"
 
-          cmake -S test/unit-test -B test/unit-test/build/ -G Ninja
-          cmake --build test/unit-test/build/ --target all
+          cmake --fresh -G Ninja -S test/unit-test -B test/unit-test/build/ -DSANITIZE=address,undefined
+          ninja -C test/unit-test/build/
 
           echo "::endgroup::"
           echo -e "${{ env.bashPass }} ${{ env.stepName }} ${{ env.bashEnd }}"
 
       - env:
-          stepName: Run Unit Tests
+          stepName: Run Unit Tests (aubsan build)
         name: ${{ env.stepName }}
         shell: bash
         run: |
           # ${{ env.stepName }}
           echo -e "::group::${{ env.bashInfo }} ${{ env.stepName }} ${{ env.bashEnd }}"
 
+          env ASAN_OPTIONS=detect_odr_violation=0 ctest --test-dir test/unit-test/build/ -E system --output-on-failure
+
+          echo "::endgroup::"
+          echo -e "${{ env.bashPass }} ${{ env.stepName }} ${{ env.bashEnd }}"
+
+      - env:
+          stepName: Build Unit Tests (coverage build)
+        name: ${{ env.stepName }}
+        run: |
+          # ${{ env.stepName }}
+          echo -e "::group::${{ env.bashInfo }} ${{ env.stepName }} ${{ env.bashEnd }}"
+
+          cmake --fresh -G Ninja -S test/unit-test -B test/unit-test/build/ -DSANITIZE=
+          ninja -C test/unit-test/build/
+
+          echo "::endgroup::"
+          echo -e "${{ env.bashPass }} ${{ env.stepName }} ${{ env.bashEnd }}"
+
+      - env:
+          stepName: Run Unit Tests (coverage build)
+        name: ${{ env.stepName }}
+        shell: bash
+        run: |
+          # ${{ env.stepName }}
+          echo -e "::group::${{ env.bashInfo }} ${{ env.stepName }} ${{ env.bashEnd }}"
+
+          find test/unit-test/build/ -name '*.gcda' -delete
           ctest --test-dir test/unit-test/build/ -E system --output-on-failure
 
           echo "::endgroup::"

source/FreeRTOS_DNS.c

@@ -656,7 +656,7 @@
                     if( ulIPAddress != 0UL )
                     {
                         #if ( ipconfigUSE_IPv6 != 0 )
-                            if( ( ppxAddressInfo != NULL ) && ( ( *ppxAddressInfo )->ai_family == FREERTOS_AF_INET6 ) )
+                            if( ( ppxAddressInfo != NULL ) && ( *ppxAddressInfo != NULL ) && ( ( *ppxAddressInfo )->ai_family == FREERTOS_AF_INET6 ) )
                             {
                                 FreeRTOS_printf( ( "prvPrepareLookup: found '%s' in cache: %pip\n",
                                                    pcHostName, ( void * ) ( *ppxAddressInfo )->xPrivateStorage.sockaddr.sin_address.xIP_IPv6.ucBytes ) );

source/FreeRTOS_TCP_IP.c

@@ -627,7 +627,14 @@
                  * active connect(). */
                 if( pxSocket->u.xTCP.ucRepCount < 3U )
                 {
-                    ulDelayMs = ( ( ( uint32_t ) 3000U ) << ( pxSocket->u.xTCP.ucRepCount - 1U ) );
+                    if( pxSocket->u.xTCP.ucRepCount == 0U )
+                    {
+                        ulDelayMs = 0U;
+                    }
+                    else
+                    {
+                        ulDelayMs = ( ( uint32_t ) 3000U ) << ( pxSocket->u.xTCP.ucRepCount - 1U );
+                    }
                 }
                 else
                 {

test/unit-test/CMakeLists.txt

@@ -18,6 +18,12 @@ if( ${PROJECT_SOURCE_DIR} STREQUAL ${PROJECT_BINARY_DIR} )
     message( FATAL_ERROR "In-source build is not allowed. Please build in a separate directory, such as ${PROJECT_SOURCE_DIR}/build." )
 endif()
 
+set(SANITIZE "" CACHE STRING "Comma-separated list of compiler sanitizers to enable; empty string disables.")
+if(NOT ${SANITIZE} STREQUAL "")
+    add_compile_options(-fsanitize=${SANITIZE} -fno-sanitize-recover)
+    add_link_options(-fsanitize=${SANITIZE} -fno-sanitize-recover)
+endif()
+
 # Set global path variables.
 get_filename_component( __MODULE_ROOT_DIR "${CMAKE_CURRENT_LIST_DIR}/../.." ABSOLUTE )
 set( MODULE_ROOT_DIR ${__MODULE_ROOT_DIR} CACHE INTERNAL "FreeRTOS-Plus-TCP repository root." )

test/unit-test/FreeRTOS_ARP/FreeRTOS_ARP_utest.c

@@ -2034,29 +2034,34 @@ void test_eARPGetCacheEntryByMac_OneMatchingEntry( void )
     eARPLookupResult_t eResult;
     MACAddress_t xMACAddress = { 0x22, 0x22, 0x22, 0x22, 0x22, 0x22 };
     int i;
-    struct xNetworkInterface * xInterface;
+    NetworkEndPoint_t xNetworkEndPoint = { 0 };
+    NetworkInterface_t xInterface, * pxInterface = NULL;
+
+    xNetworkEndPoint.pxNetworkInterface = &xInterface;
 
     /* =================================================== */
     /* Make sure one entry matches. */
     for( i = 0; i < ipconfigARP_CACHE_ENTRIES; i++ )
     {
         xARPCache[ i ].ulIPAddress = 0xAABBCCDD;
+        xARPCache[ i ].pxEndPoint = &xNetworkEndPoint;
         memset( xARPCache[ i ].xMACAddress.ucBytes, 0x11, sizeof( xMACAddress.ucBytes ) );
     }
 
     ulEntryToTest = 1;
     memset( xARPCache[ ulEntryToTest ].xMACAddress.ucBytes, 0x22, sizeof( xMACAddress.ucBytes ) );
     xARPCache[ ulEntryToTest ].ulIPAddress = 0xAABBCCEE;
-    eResult = eARPGetCacheEntryByMac( &xMACAddress, &ulIPAddress, &xInterface );
+    eResult = eARPGetCacheEntryByMac( &xMACAddress, &ulIPAddress, &pxInterface );
     TEST_ASSERT_EQUAL( eARPCacheHit, eResult );
     TEST_ASSERT_EQUAL( xARPCache[ ulEntryToTest ].ulIPAddress, ulIPAddress );
+    TEST_ASSERT_EQUAL( &xInterface, pxInterface );
     /* =================================================== */
     eResult = eARPGetCacheEntryByMac( &xMACAddress, &ulIPAddress, NULL );
     TEST_ASSERT_EQUAL( eARPCacheHit, eResult );
     TEST_ASSERT_EQUAL( xARPCache[ ulEntryToTest ].ulIPAddress, ulIPAddress );
     /* =================================================== */
     xARPCache[ ulEntryToTest ].pxEndPoint = NULL;
-    eResult = eARPGetCacheEntryByMac( &xMACAddress, &ulIPAddress, &xInterface );
+    eResult = eARPGetCacheEntryByMac( &xMACAddress, &ulIPAddress, &pxInterface );
     TEST_ASSERT_EQUAL( eARPCacheHit, eResult );
     TEST_ASSERT_EQUAL( xARPCache[ ulEntryToTest ].ulIPAddress, ulIPAddress );
     /* =================================================== */
@@ -2305,6 +2310,7 @@ void test_eARPGetCacheEntry_NoCacheHit( void )
     {
         xARPCache[ i ].ulIPAddress = 0;
         xARPCache[ i ].ucValid = ( uint8_t ) pdTRUE;
+        xARPCache[ i ].pxEndPoint = NULL;
     }
 
     ulSavedGatewayAddress = xNetworkAddressing.ulGatewayAddress;

test/unit-test/FreeRTOS_DHCP/FreeRTOS_DHCP_stubs.c

@@ -12,6 +12,11 @@
 #include "FreeRTOS_IP.h"
 #include "FreeRTOS_IP_Private.h"
 
+#define prvROUND_UP_TO( SIZE, ALIGNMENT )    ( ( ( SIZE ) + ( ALIGNMENT ) -1 ) / ( ALIGNMENT ) *( ALIGNMENT ) )
+
+/* FIXME: Consider instead fixing ipBUFFER_PADDING if it's supposed to be pointer aligned. */
+#define prvALIGNED_BUFFER_PADDING    prvROUND_UP_TO( ipBUFFER_PADDING, sizeof( void * ) )
+
 struct xNetworkEndPoint * pxNetworkEndPoints = NULL;
 
 NetworkInterface_t xInterfaces[ 1 ];
@@ -212,9 +217,9 @@ static NetworkBufferDescriptor_t * GetNetworkBuffer( size_t SizeOfEthBuf,
                                                      long unsigned int xTimeToBlock,
                                                      int callbacks )
 {
-    NetworkBufferDescriptor_t * pxNetworkBuffer = malloc( sizeof( NetworkBufferDescriptor_t ) + ipBUFFER_PADDING ) + ipBUFFER_PADDING;
+    NetworkBufferDescriptor_t * pxNetworkBuffer = malloc( sizeof( NetworkBufferDescriptor_t ) + prvALIGNED_BUFFER_PADDING ) + prvALIGNED_BUFFER_PADDING;
 
-    pxNetworkBuffer->pucEthernetBuffer = malloc( SizeOfEthBuf + ipBUFFER_PADDING ) + ipBUFFER_PADDING;
+    pxNetworkBuffer->pucEthernetBuffer = malloc( SizeOfEthBuf + prvALIGNED_BUFFER_PADDING ) + prvALIGNED_BUFFER_PADDING;
 
     /* Ignore the callback count. */
     ( void ) callbacks;
@@ -230,9 +235,9 @@ static NetworkBufferDescriptor_t * GetNetworkBuffer( size_t SizeOfEthBuf,
 static void ReleaseNetworkBuffer( void )
 {
     /* Free the ethernet buffer. */
-    free( ( ( uint8_t * ) pxGlobalNetworkBuffer[ --GlobalBufferCounter ]->pucEthernetBuffer ) - ipBUFFER_PADDING );
+    free( ( ( uint8_t * ) pxGlobalNetworkBuffer[ --GlobalBufferCounter ]->pucEthernetBuffer ) - prvALIGNED_BUFFER_PADDING );
     /* Free the network buffer. */
-    free( ( ( uint8_t * ) pxGlobalNetworkBuffer[ GlobalBufferCounter ] ) - ipBUFFER_PADDING );
+    free( ( ( uint8_t * ) pxGlobalNetworkBuffer[ GlobalBufferCounter ] ) - prvALIGNED_BUFFER_PADDING );
 }
 
 static void ReleaseUDPBuffer( const void * temp,
@@ -289,19 +294,19 @@ static int32_t FreeRTOS_recvfrom_Generic( const ConstSocket_t xSocket,
     return ulGenericLength;
 }
 
-static int32_t FreeRTOS_recvfrom_Generic_NullBuffer( const ConstSocket_t xSocket,
-                                                     void * pvBuffer,
-                                                     size_t uxBufferLength,
-                                                     BaseType_t xFlags,
-                                                     struct freertos_sockaddr * pxSourceAddress,
-                                                     socklen_t * pxSourceAddressLength,
-                                                     int callbacks )
+static int32_t FreeRTOS_recvfrom_Small_NullBuffer( const ConstSocket_t xSocket,
+                                                   void * pvBuffer,
+                                                   size_t uxBufferLength,
+                                                   BaseType_t xFlags,
+                                                   struct freertos_sockaddr * pxSourceAddress,
+                                                   socklen_t * pxSourceAddressLength,
+                                                   int callbacks )
 {
-    pvBuffer = NULL;
-    return xSizeofUDPBuffer;
+    /* Admittedly, returning a (NULL, 1) slice is contrived, but coverage speaks. */
+    *( ( uint8_t ** ) pvBuffer ) = NULL;
+    return 1;
 }
 
-
 static int32_t FreeRTOS_recvfrom_eWaitingOfferRecvfromLessBytesNoTimeout( const ConstSocket_t xSocket,
                                                                           void * pvBuffer,
                                                                           size_t uxBufferLength,
@@ -336,7 +341,7 @@ static int32_t FreeRTOS_recvfrom_ResetAndIncorrectStateWithSocketAlreadyCreated_
         pxIterator = pxIterator->pxNext;
     }
 
-    if( xFlags == FREERTOS_ZERO_COPY + FREERTOS_MSG_PEEK )
+    if( ( xFlags & FREERTOS_ZERO_COPY ) != 0 )
     {
         *( ( uint8_t ** ) pvBuffer ) = pucUDPBuffer;
     }