[Warning] dereferencing type-punned pointer will break strict-aliasing rules [stm32f0xx_hal_crc.c]

[valentin]

Posted on October 30, 2016 at 22:27

This is what I get when compiling an empty created CubeMX project (CubeMX 4.17.0, STM32F0 lib v1.6.0) with the CRC module enabled.

File: stm32f0xx_hal_crc.c, affected lines: 470 -> *(uint16_t*) (&hcrc->Instance->DR) = ((uint16_t)pBuffer[4*i]<<8) | (uint16_t)pBuffer[4*i+1]; 474 -> *(uint16_t*) (&hcrc->Instance->DR) = ((uint16_t)pBuffer[4*i]<<8) | (uint16_t)pBuffer[4*i+1]; 506 -> *(uint16_t*) (&hcrc->Instance->DR) = pBuffer[2*i]; According to some googling, those pointers are accessing the memory of DR while interpreting it as uint16_t while DR originally is defined as uint32_t. This breaks the aliasing rules and should be avoided. See:

http://stackoverflow.com/questions/3246228/dereferencing-type-punned-pointer-will-break-strict-aliasing-rules

and:

http://stackoverflow.com/questions/8824622/fix-for-dereferencing-type-punned-pointer-will-break-strict-aliasing

So changing the *(uint16_t*) to *(uint32_t*) fixes the warnings (as &hcrc->Instance->DR is of type uint32_t, not uint16_t). See definition in stm32f042x6.h: typedef struct {   __IO uint32_t DR;          /*!< CRC Data register,                           Address offset: 0x00 */   __IO uint8_t  IDR;         /*!< CRC Independent data register,               Address offset: 0x04 */   uint8_t       RESERVED0;   /*!< Reserved,                                                    0x05 */   uint16_t      RESERVED1;   /*!< Reserved,                                                    0x06 */   __IO uint32_t CR;          /*!< CRC Control register,                        Address offset: 0x08 */   uint32_t      RESERVED2;   /*!< Reserved,                                                    0x0C */   __IO uint32_t INIT;        /*!< Initial CRC value register,                  Address offset: 0x10 */   __IO uint32_t RESERVED3;   /*!< Reserved,                                                    0x14 */ } CRC_TypeDef; If the lines have to use uint16_t, though (for whatever reason) a union has been suggested. So, ST please fix this as I don't really like the possibility of undefined behaviour and therefore hard to fix bugs in my projects. thanks! #cubemx-crc-strict-aliasing-warn

[Tesla DeLorean]

Posted on October 31, 2016 at 06:26

Ok, but it wants to do a 16-bit write, it results in some *defined* behaviour of the hardware.

In some modes you only want the CRC to advance 2 bytes, advancing 4 changes the math and the result. A union might be workable, but not all compilers permit transparent naming.

[valentin]

Posted on October 31, 2016 at 20:48

Hmm, let me see whether I get that correctly.

Are you saying that by aliasing the pointer as *(uint16_t*) only the two least significant bytes of the DR register are being changed while the two MSB are left untouched? If so, I don't see why a union shouldn't work. OR the type-punning could be done using chars instead. Chars are allowed to do it. Maybe an array of uint8_t [2] which I think equals chars somewhere down the line. But, if the two MSB of the DR register don't matter I don't see why we shouldn't be able to use *(uint32_t*) instead. I have to admit that I haven't looked into the inner workings of the CRC module yet, though ...A union might be workable, but not all compilers permit transparent naming. Even less would they be required to interpret non-standard code in the desired way, would they? Anyway, there must be a way to do this in compliance with ANSI-C. Other suggestions are welcome.

[Tesla DeLorean]

Posted on October 31, 2016 at 21:10

The bus at a peripheral level can differentiate between an 8-bit, 16-bit and 32-bit write. It is not that a write on the low-order 16-bit causes a 32-bit write with the high-order 16-bits being zero.

I don't understand why a pointer cast, to a smaller granularity data item is an issue. It doesn't cause an alignment issue. I don't even see this as being an undefined or unexpected behaviour, it is pretty explicit.

typedef union _CRCDR {
uint8_t dru8;
uint16_t dru16;
uint32_t dru32; 
} CRCDR;
void write16(uint16_t data)
{
CRCDR *p = (void *)&CRC->DR; // or some cast you prefer to indicate you have awareness
p->dru16 = data;
}

An unname union would solve the problem transparently, but like I said there are compilers that don't like that. ST doesn't seem to like name unions in their structures. Of all the functional correctness of code in the HAL, I think this is the least of the issues.

[valentin]

Posted on November 01, 2016 at 22:15

Okay, I'll trust you on that one and leave it the way it is :)

It only bugs me that I won't get rid of the compiler warnings then ... and I don't really just want to suppress them.

[Andrei Chichak]

Posted on October 06, 2017 at 23:17

Okay, this hasn't been patched in the last year, and now it's my issue.

I'm starting a new project using a '767 and stm32f7xx_hal_crc.c version FW_F7 V1.8.0 still triggers this warning.

When working with safety critical software it is common to make all warnings in to errors. The compiler is telling you there is an issue, it's not fatal, but you should deal with the issue.

Since the CRC code seems to really want to do 8 and 16-bit operations on 32-bit data items without using an intermediate value, GCC complains (rightly) that this can cause the generation of undefined code. You're breaking a rule, so the code probably won't be what you're expecting, WARNING!

Blaming the HAL really isn't appropriate since the SPL doesn't have code for new processors.

Can someone at ST take a look at this please?

Andrei

[Tesla DeLorean]

Posted on October 07, 2017 at 00:46

>>When working with safety critical software it is common to make all warnings in to errors. 

In many cases the use of third-party libraries isn't permitted either, at the very least everything would require a code walk and meet the same stringent standards applied to the rest of the code base.

[Andrei Chichak]

Posted on October 07, 2017 at 01:11

SOUP!