Misra rule 11.3, 11.4 suppression and 4.6 fix

[xuelix]

Description

Test Steps

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.

[paulbartell]

The explanation for Rule 11.3 deviations needs to explain how proper alignment is maintained.

[paulbartell]

Which cast is being referenced here?

[xuelix]

I'll change the wording here. It's a conversion.

[htibosch]

/* MISRA C-2012 Rule 11.3 warns about casting pointer type
* to a different data type. The struct to be casted to is
* defined as a packed struct. The cast won't cause misalignment. */
/* coverity[misra_c_2012_rule_11_3_violation] */

Thank you for these additions, I approve it.

A small comment for those who wonder why we ignore MISRA C-2012 Rule 11.3:

The TCP/IP library receives and sends all sorts of packets: ARP, TCP/IP, UDP/IP, DHCP, DNS etc. These packets are stored in a buffer pucEthernetBuffer which is always aligned in the same manner: a four-byte alignment plus two bytes. The reason for this is that the Ethernet header is 14 (12+2) bytes long. So after the Ethernet header, the words and shorts are properly aligned.

It is convenient (and also efficient) to access a word as a word, as in pxIPHeader->ulDestinationIPAddress.

There is one alignment exception, which is in the ARP header: the sender protocol address, or ucSenderProtocolAddress[4], is declared as an array of 4 bytes. It falls on a 2-byte alignment.

In some situations, in stead of copying shorts and words, byte access will be done like here in FreeRTOS_DNS_Parser.c :

#define vSetField16( pxBase, xType, xField, usValue )                                                        \
    {                                                                                                        \
        ( ( uint8_t * ) ( pxBase ) )[ offsetof( xType, xField ) + 0 ] = ( uint8_t ) ( ( usValue ) >> 8 );    \
        ( ( uint8_t * ) ( pxBase ) )[ offsetof( xType, xField ) + 1 ] = ( uint8_t ) ( ( usValue ) & 0xffU ); \
    }

which works independent from the endianness.

In other situations, where the alignment is unknown, memcpy() is used to access data.

When structs are copied, memcpy() should be used, and not a struct assignment as in xCopy = xOrg. The latter may lead to an alignment exception. The compiler can be mistaken about the alignment of the structs.

I recommend to use the compiler flags -fno-builtin-memcpy and -fno-builtin-memset. In some situations, compilers make the wrong assumptions about alignment, when replacing a 4-byte memcpy() with a 32-byte assignment.

The use of packed structs is necessary, the alignment in the declarations must be guaranteed the same as in the packets on the wire.

The FreeRTOS+TCP library has been tested and used on a wide variety of platforms: little and big endian, and on both 32- and 64-bit platforms.

Hein

[paulbartell]

/* MISRA C-2012 Rule 11.3 warns about casting pointer type
* to a different data type. The struct to be casted to is
* defined as a packed struct. The cast won't cause misalignment. */
/* coverity[misra_c_2012_rule_11_3_violation] */

Thank you for these additions, I approve it.

A small comment for those who wonder why we ignore MISRA C-2012 Rule 11.3:

The TCP/IP library receives and sends all sorts of packets: ARP, TCP/IP, UDP/IP, DHCP, DNS etc. These packets are stored in a buffer pucEthernetBuffer which is always aligned in the same manner: a four-byte alignment plus two bytes. The reason for this is that the Ethernet header is 14 (12+2) bytes long. So after the Ethernet header, the words and shorts are properly aligned.

It is convenient (and also efficient) to access a word as a word, as in pxIPHeader->ulDestinationIPAddress.

There is one alignment exception, which is in the ARP header: the sender protocol address, or ucSenderProtocolAddress[4], is declared as an array of 4 bytes. It falls on a 2-byte alignment.

In some situations, in stead of copying shorts and words, byte access will be done like here in FreeRTOS_DNS_Parser.c :

#define vSetField16( pxBase, xType, xField, usValue )                                                        \
    {                                                                                                        \
        ( ( uint8_t * ) ( pxBase ) )[ offsetof( xType, xField ) + 0 ] = ( uint8_t ) ( ( usValue ) >> 8 );    \
        ( ( uint8_t * ) ( pxBase ) )[ offsetof( xType, xField ) + 1 ] = ( uint8_t ) ( ( usValue ) & 0xffU ); \
    }

which works independent from the endianness.

In other situations, where the alignment is unknown, memcpy() is used to access data.

When structs are copied, memcpy() should be used, and not a struct assignment as in xCopy = xOrg. The latter may lead to an alignment exception. The compiler can be mistaken about the alignment of the structs.

I recommend to use the compiler flags -fno-builtin-memcpy and -fno-builtin-memset. In some situations, compilers make the wrong assumptions about alignment, when replacing a 4-byte memcpy() with a 32-byte assignment.

The use of packed structs is necessary, the alignment in the declarations must be guaranteed the same as in the packets on the wire.

The FreeRTOS+TCP library has been tested and used on a wide variety of platforms: little and big endian, and on both 32- and 64-bit platforms.

Hein

When a struct is declared as packed, that struct and it's members are treated as byte-aligned. This means that a compiler should not make any alignment assumptions when copying or assigning members of the struct.