search

STMicroelectronics / STM32CubeL0

STM32Cube MCU Full Package for the STM32L0 series - (HAL + LL Drivers, CMSIS Core, CMSIS Device, MW libraries plus a set of Projects running on all boards provided by ST (Nucleo, Evaluation and Discovery Kits))
Other
103 stars 57 forks source link

CRC routines problem with GCC -fstrict-aliasing #2

Closed luciodona closed 5 years ago

luciodona commented 5 years ago

Hi all, I had a problem with STM32CubeL0 about the crc routines.

Set-up

Description

How To Reproduce

  1. Try a small crc calculation with CRC_INPUTDATA_FORMAT_BYTES or CRC_INPUTDATA_FORMAT_HALFWORDS : correct results with "-O0" optimization / bad results with "-O2" optimization.

  2. The file: Drivers/STM32L0xx_HAL_Driver/Src/stm32l0xx_hal_crc.c - CRC_Handle_8 and CRC_Handle_16 routines

Explanation

compiler strict aliasing is enabled by "-O2" optimization. This is causing trouble.

Solution proposal

disable strict aliasing for these functions.

The functions become as follows:

pragma GCC diagnostic push

pragma GCC diagnostic ignored "-Wstrict-aliasing"

static uint32_t attribute((optimize("no-strict-aliasing"))) CRC_Handle_8(CRC_HandleTypeDef hcrc, uint8_t pBuffer[], uint32_t BufferLength) { ...[omissis] } static uint32_t attribute((optimize("no-strict-aliasing"))) CRC_Handle_16(CRC_HandleTypeDef hcrc, uint16_t pBuffer[], uint32_t BufferLength) { ...[omissis] }

pragma GCC diagnostic pop

Comments

Conclusion

Using the corrections as exposed (no-strict-aliasing) the calculations are correct, my board is functioning correctly with "-O0" and with "-O2" compiler flags. NOTE: I applied this patch also on other Cube libraries (example: L4), as the code is virtually the same.

Please, let me know your opinions.

Thanks, best regards

Lucio Dona'

ALABSTM commented 5 years ago

Dear Lucio Dona',

Thank you for pointing out this issue and for the clarity of the description you made. It is now being analyzed. We will provide you with feedback as soon as possible.

With regards

CCASTM commented 5 years ago

adding J.Christophe in the thread @JCBSTM

JCBSTM commented 5 years ago

Dear Lucio Dona’,

I haven’t been able to reproduce the issue you describe. I used the CRC example delivered in the STM32Cube L0 FW package “CRC_bytes_stream_7bit_CRC” and ran it on NUCLEO-L073RZ after having modified the optimization option from -0s (default optimization in SW4STM32 project) to -02.

However, the computed CRC’s were still correct. Please note that my gcc compiler version is 7.3.1.

Is there a way for you to update your gcc version and test again? Could you please provide your generated code as well, so that we can check deeper (that of CRC_Handle_8 or CRC_Handle_16)?

Thank you Best regards Jean-Christophe

luciodona commented 5 years ago

Hi Jean-Christophe, sorry for the delay, but in these days I was abroad. This is a rather "old" problem I had about two-three years ago with a bootloader : it was calculating the checksum of all the flash. Running ok in debug mode, but it was failing in release. I remember having fixed this modifying the checksum routines as I wrote in the first post, and doing various tests for crc correctness.

Then I applied the same modification to all the STM32 libraries I was using (F0, F4, L0, L1, L4), but I did no "deep" checking of the assembly code, neither testing in all the processors. Just all the bootloaders were working correctly. I will schedule some time in the next days to try and reproduce what I wrote ; my suspect is that the problem was related also to the compiler interpretation of the code when dealing with "strict aliasing" and that it has disappeared with the new GCC versions. I apologize in advance in case of "false alarm" with the new compilers ; will be here as soon as possible.

Thanks for your kind support : this direct channel with you is very appreciated.

Best regards Lucio Dona'

ALABSTM commented 5 years ago

ST Internal Reference: 66744

JCBSTM commented 5 years ago

Dear Lucio Dona’,

Were you able to progress at your side on the issue? Otherwise, we can close the issue.

Thanks for your feedback.

Best regards Jean-Christophe

luciodona commented 5 years ago

Hi all, sorry for my delay... Too many tasks.

Well, I took one Nucleo board and tried to reproduce the CRC problem : I did not succeed, but I think I have found something interesting anyway. This is my setup:

1- NUCLEO-L476RG board 2- STM32Cube_FW_L4_V1.4.0 library 3- from this library I took the code contained in 'Projects/STM32L476RG-Nucleo/Examples/CRC/CRC_Example' folder 4- Atollic TrueSTUDIO for STM32 version 9.3.0, build id: 20190212-0734, Linux host (Ubuntu)

I created a very simple new project, named CRCTest, and adapted it adding code from point -3- (CRC_Example) ; did this to create sort of "self-contained" test bench. You can find it attached at the end of this message as CRCTest.tar.gz .

_Well: if you compile this example with '-O3' optimization it works, but... the compiler complains giving you three warnings on the stm32l4xx_hal_crc.c file._

This is the output from the compiler (just for the stm32l4xx_hal_crc.c file): ############################## 16:03:01 Rebuild of configuration Debug for project CRCTest Info: Internal Builder is used for build [OMISSIS] arm-atollic-eabi-gcc -c ../Drivers/STM32L4xx_HAL_Driver/Src/stm32l4xx_hal_crc.c -mthumb -mcpu=cortex-m4 -mfloat-abi=hard -mfpu=fpv4-sp-d16 -std=gnu11 -DSTM32L476xx -I/home/lucio/project/git/gitprj/MTcrctest/CRCTest/src -I/home/lucio/project/git/gitprj/MTcrctest/CRCTest/Drivers/BSP/STM32L4xx_Nucleo -I/home/lucio/project/git/gitprj/MTcrctest/CRCTest/inc -I/home/lucio/project/git/gitprj/MTcrctest/CRCTest/Drivers/CMSIS/Include -I/home/lucio/project/git/gitprj/MTcrctest/CRCTest/Drivers/CMSIS/Device/ST/STM32L4xx/Include -I/home/lucio/project/git/gitprj/MTcrctest/CRCTest/Drivers/STM32L4xx_HAL_Driver/Inc -Os -ffunction-sections -fdata-sections -g -fstack-usage -Wall -specs=nano.specs -o Drivers/STM32L4xx_HAL_Driver/Src/stm32l4xx_hal_crc.o ../Drivers/STM32L4xx_HAL_Driver/Src/stm32l4xx_hal_crc.c: In function 'CRC_Handle_8': ../Drivers/STM32L4xx_HAL_Driver/Src/stm32l4xx_hal_crc.c:487:8: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing] (uint16_t volatile) (&hcrc->Instance->DR) = ((uint32_t)pBuffer[4i]<<8) | (uint32_t)pBuffer[4i+1]; ^ ../Drivers/STM32L4xx_HAL_Driver/Src/stm32l4xx_hal_crc.c:491:8: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing] (uint16_t volatile) (&hcrc->Instance->DR) = ((uint32_t)pBuffer[4i]<<8) | (uint32_t)pBuffer[4i+1]; ^ ../Drivers/STM32L4xx_HAL_Driver/Src/stm32l4xx_hal_crc.c: In function 'CRC_Handle_16': ../Drivers/STM32L4xx_HAL_Driver/Src/stm32l4xx_hal_crc.c:526:8: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing] (uint16_t volatile) (&hcrc->Instance->DR) = pBuffer[2*i]; ^ [OMISSIS] Generate build reports... Print size information text data bss dec hex filename 4820 20 1096 5936 1730 CRCTest.elf Print size information done Generate listing file Output sent to: CRCTest.list Generate listing file done Generate build reports done

16:03:03 Build Finished (took 2s.260ms) ##############################

Note the "warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]" .

Conclusion:

the code works, but personally I do not like having any warning in my code. It's like walking on the dark side of the moon : you don't know ;-)

As I wrote in my first post, with a previous version of GCC I had warnings and ALSO wrong results ; with this version of the build environment I have just the warnings and the code works, but I corrected it anyway to make warnings go away. Just look for "//LD" pattern in stm32l4xx_hal_crc.c .

That's all. Just my two cents...

Thank you all for your support ; just let me know if you need other tests on my side.

Best regards, Lucio Dona'

TEST PROJECT FOR NUCLEO-L476RG BOARD:

CRCTest.tar.gz

JCBSTM commented 5 years ago

Dear Lucio Dona’,

Yes, definitively, I agree with you, we want to avoid warnings in our code. The warnings you are listing were reported a few months ago thru other channels and have been corrected.

You may want to use the latest STM32CubeFW L4 package which is the V1.14.0 and is available on the net since last April (the correction is actually available since V1.12.0 version).

Best regards Jean-Christophe

luciodona commented 5 years ago

Hi Jean-Christophe, I downloaded the new L4 package and will use it on my next firmware updates. We can close the issue.

Thank you for your assistance, best regards

Lucio

ALABSTM commented 5 years ago

Dear Lucio and Jean-Christophe,

Thank you for this exchange. This issue is worth being labeled "good first issue" to serve as an example. It can be closed now it has been solved.

With regards

ALABSTM commented 3 years ago

ST Internal Reference: 45893

ALABSTM commented 3 years ago

NOTE: Migration to new firmware version (v1.14.0) solved the issue.