Bug in CRC32 calculation

[josesimoes]

Hi,

On this line:
https://github.com/STMicroelectronics/stm32f7xx_hal_driver/blob/8a83a680a5c0c678df3897ae41b430444abb3cb7/Src/stm32f7xx_hal_crc.c#L453

and on this one too:
https://github.com/STMicroelectronics/stm32f7xx_hal_driver/blob/8a83a680a5c0c678df3897ae41b430444abb3cb7/Src/stm32f7xx_hal_crc.c#L467

it's missing a cast to (uint8_t) when loading the DR register, otherwise it won't compute the correct CRC32.

[ALABSTM]

Hi @josesimoes,

Thank you for your report. May I ask you to provide us with the test case that allowed you to obtain an incorrect CRC32?

Thank you,

[josesimoes]

@ALABSTM I haven't added any test case… just found out about it when using the code on my app.
The issue is not a specific to a particular calculation is just wrong for any calculation on which there is a single byte left in the buffer (after feeding the register with 32bits data chunks). On another perspective, if one is feeding the calculator with a byte array with an odd length.

[ALABSTM]

Hi @josesimoes,

Do you confirm you obtain a wrong CRC32 even though member InputDataFormat in the handler structure is set to CRC_INPUTDATA_FORMAT_BYTES? Thank you in advance for your answer.

With regards,

[josesimoes]

That is correct. On our project the CRC32 only applies to byte arrays and we set the InputDataFormat to CRC_INPUTDATA_FORMAT_BYTES when configuring the CRC peripheral. And that is never changed afterwards.

[ALABSTM]

Hi @josesimoes,

Thank you for your answer. In order to be sure I correctly understood your request, you suggest the following implementation?

- *(__IO uint8_t *)(__IO void *)(&hcrc->Instance->DR) = pBuffer[4U * i];
+ *(__IO uint8_t *)(__IO void *)(&hcrc->Instance->DR) = (uint8_t) pBuffer[4U * i];

May I also ask you whether you obtained a correct CRC once you added the cast? Thank you for your answers once more.

With regards,

[josesimoes]

Basically yes. On both lines 453 and 467. That cast is all that's needed to fix this. Actually inline with the other casts there on the other data sizes cases.

[ALABSTM]

Hi @josesimoes,

Please excuse my insistence. Do you confirm you updated the two lines above by adding the (uint8_t)cast to the pBuffer[4U * i] array element and had the issue fixed in consequence (i.e. the correct CRC has been obtained)?

I ask the question because the pBuffer parameter is already defined to be a pointer on uint8_t data. Thank you for your comprehension.

With regards,

[josesimoes]

Hi again,

That is correct. I agree that this sounds weird but it is what it is... I guess that's probably because of the way the compiler is handling that. The cast makes sure that the proper data and size are feed to the CRC32 register.

All that code feeding the registers looks over engineered and there are notes about that.
If it's of interest: I'm using GCC 8-2019-q3-update.

[ALABSTM]

Hi @josesimoes,

Shall I understand you tested the fix you are proposing and it solved the issue?

With regards,

[josesimoes]

Hi @ALABSTM yes, of course. Otherwise I wouldn't be reporting this and offering the fix I've used.
The code with the above fix is being used in production (for almost two month now) in a project I'm with the core team: https://github.com/nanoframework
(BTW, it's a framework allowing to code for embedded systems using .NET C#)

[ALABSTM]

Hi @josesimoes,

Thank you for the confirmation. Your request has been forwarded to our development teams. I will be back to you as soon as they provide me with their feedback.

Thank you once again for having reported this point. Thank you also for having introduced the "nanoframework" initiative I have not knew about before reading your message. The project looks promising and really interesting. I wish you the best of lucks.

With regards,

[ALABSTM]

Hi @josesimoes,

I hope you are fine.

Our development teams could not reproduce the issue you are describing. They used a STM32L5 Discovery board and the STM32Cube ID3 v1.4.0 (with the GCC compiler version 7_2018_q2_update). The CRC computation of a 5-byte long stream returned the expected result.

Their recommendation is to migrate to the last compiler version. Please allow me to close this issue now. Thank you again for your contribution and patience.

With regards,

[josesimoes]

Hi @ALABSTM doing fine thanks.

I have to say that I'm a bit puzzled with your reply... I specifically mentioned that this was happening on an F7 target and that I was seeing this with GCC 8-2019-q3-update. Now you tell me that your development team tried to reproduce this on a L5 target with GCC 7_2018_q2_update... I don't get it...

We'll stick with our local fix then.

[ALABSTM]

Hi @josesimoes,

"Mea culpa" for the inconsistent details in my last comment. With some colleague, we ran the CRC_Bytes_Stream_7bit_CRC example:

• on a STM32F7696-EVAL board using EWARM v8.32.4.
• on a STM32F767ZI-Nucleo board using Cube IDE v1.6.0 (integrating GCC 9.11.1.202004012021).

The CRC calculations passed in both cases.

May I ask you to send us a modified version of the above mentioned example allowing to reproduce the issue?

With regards,

[ALABSTM]

Hi @josesimoes,

Please allow me to close this issue as no reply has been received since a couple of months.

With regards,