Potentially uninitialized value in HAL_SD_ConfigWideBusOperation

[apullin]

Describe the set-up Building with gcc 10.2.1 using -fanalyzer option for static analysis.

Describe the bug In HAL_SD_ConfigWideBusOperation, Init.ClockDiv may be used without initialization.

As reported by the analyzer:

myproject/Drivers/STM32L4xx_HAL_Driver/Src/stm32l4xx_hal_sd.c: In function 'HAL_SD_ConfigWideBusOperation':
myproject/Drivers/STM32L4xx_HAL_Driver/Src/stm32l4xx_hal_sd.c:2760:29: warning: 'Init.ClockDiv' may be used uninitialized in this function [-Wmaybe-uninitialized]
 2760 |       if ((sdmmc_clk / (Init.ClockDiv + 2U)) > SD_NORMAL_SPEED_FREQ)
      |                         ~~~~^~~~~~~~~

There appears to only be initialization of Init.ClockDiv inside a chip-specific block here: https://github.com/STMicroelectronics/STM32CubeL4/blob/master/Drivers/STM32L4xx_HAL_Driver/Src/stm32l4xx_hal_sd.c#L2694

How To Reproduce Build STM32L4 HAL to an object library using arm-gcc v10, using the -fanalayzer option.

I would rank the severity of this bug as HIGH, given that the undefined value is used in a division, and thus could result in a divide-by-zero error, or undefined clock behavior in other cases.

[RKOUSTM]

Hi @apullin,

Thank you for reports. According to the Development Toolchains and Compilers section in the Release-Notes, gcc 9.11.1.202004012021 version is compatible with the STM32Cube L4 v1.17.0 version. This gcc version is supported by the STM32CubeIDE v1.5.1toolchain.

Actually, the point you raised is already known and concerns the compatibility of our HAL APIs with the GCC 10.x.y version. This is one of the main points in the tasks queue of our development teams. We cannot share a date regarding its deployment and publication. We will keep you informed.

Thank you for you again for your contribution.

With regards,

[apullin]

This has nothing to do with compiler compatibility.

GCC 10.x includes a static analyzer, which was able to catch this bug. I am sure coverity or C-STAT would also find the same result.

This also violates MISRA C:2004, 9.1 . Note that this is actional by the MISRA Consortium, given that STMicro publicly claims MISRA C compliance[1]. You will likely need to loop in your regulatory arm ASAP.

[1] https://www.st.com/resource/en/user_manual/dm00173145-description-of-stm32l4l4-hal-and-lowlayer-drivers-stmicroelectronics.pdf

[dpgeorge]

I've also hit this problem. It looks like it is a bug in the code, the line:

if ((sdmmc_clk / (Init.ClockDiv + 2U)) > SD_NORMAL_SPEED_FREQ)

should be:

if ((sdmmc_clk / (hsd->Init.ClockDiv + 2U)) > SD_NORMAL_SPEED_FREQ)

[RKOUSTM]

Hi @dpgeorge, @apullin,

Thank you for this report. You are right about this point. The issue has already been fixed internally. The fix now is available in the frame of version v1.17.1 of the STM32CubeL4 published recently on GitHub.

Thank you again for having reported.

With regards,

[RKOUSTM]

ST Internal Reference: 102042.