HAL_Delay function corner case not covered

[VMach.1]

Hi there,

this is the default implementation of HAL_Delay function for stm32h7xx_hal.c (and I assume many other hals too)

__weak void HAL_Delay(uint32_t Delay)
{
  uint32_t tickstart = HAL_GetTick();
  uint32_t wait = Delay;
 
  /* Add a freq to guarantee minimum wait */
  if (wait < HAL_MAX_DELAY)
  {
    wait += (uint32_t)(uwTickFreq);
  }
 
  while ((HAL_GetTick() - tickstart) < wait)
  {
  }
}

The issue here is that it does not cover the overflow corner case.

Let's say that Delay =100 and tickstart = HAL_GetTick() happens to be 0xFFFFFFFE.

This will cause an infinite loop.

If the tick gets incremented by 1ms this overflow happens at leats once every 50 hours.

Where could I add suggestion for code changes? A simple check as this could solve the issue.

__weak void HAL_Delay(uint32_t Delay)
{
  uint32_t tickstart = HAL_GetTick();
  uint32_t wait = Delay;
 
  /* Add a freq to guarantee minimum wait */
  if (wait < HAL_MAX_DELAY)
  {
      wait += (uint32_t)(uwTickFreq);
  }
 
  // Overflow case
  if (tickstart < (tickstart + wait)) 
  {
    while(HAL_GetTick() >= tickstart)
    {
    }
 
    wait = 0xFFFFFFFF - tickstart;
  }
 
  while ((HAL_GetTick() - tickstart) < wait)
  {
  }
}

Thanks

[Tesla DeLorean]

>>The issue here is that it does not cover the overflow corner case. Let's say that Delay =100 and tickstart = HAL_GetTick() happens to be 0xFFFFFFFE. This will cause an infinite loop.

NONSENSE, the unsigned 32-bit math expressed in the loop works just fine across normal and overflow cases.

[VMach.1]

Then i'm missing something.

If

tickstart = 0xFFFFFFFE

​wait = 0x02

Then,​

​(HAL_GetTick() - tickstart) < wait

In different ​HAL_GetTick() cases would be

(​0xFFFFFFFE - 0xFFFFFFFE) < 0x02​ --> no

+tick

(​0xFFFFFFFF - 0xFFFFFFFE) < 0x02​ --> no

+ tick

(​0x0000000 - 0xFFFFFFFE) < 0x02​ --> no

When does this loop exit?​ what im missing?

[VMach.1 · Accepted solution]

Find it!

.] A computation involving unsigned operands can never overflow, because a result that cannot be represented by the resulting unsigned integer type is reduced modulo the number that is one greater than the largest value that can be represented by the resulting type. (ISO/IEC 9899:1999 (E) §6.2.5/9)