#3510 Stdlib _modulong() loops endlessly with divisor of zero

[Basil Hussain]

There is a bug in SDCC's non-MCS51-assembly standard library C implementation of the _modulong function, in device/lib/_modulong.c. If the second argument, the divisor, is given a value of zero, the function gets stuck in an infinite loop.

I believe it gets stuck in the first loop:

  while (!MSB_SET(b))
  {
     b <<= 1;
     if (b > a)
     {
        b >>=1;
        break;
     }
     count++;
  }

Because the value of b is zero, its MSb will never be set, nor will left-shifting it ever make it greater in value than a, so the while condition is never false, and neither is the break statement ever reached.

Here is a sample program that triggers the bug:

// Build with: sdcc -mstm8 main.c
// Simulate with: sstm8 -I if=rom[0x5800] main.ihx
// Can also be built and run for other non-MCS51 platform, e.g. Z80.

#define UCSIM_IF_ADDR 0x5800
#define UCSIM_IF (*(volatile unsigned char *)(UCSIM_IF_ADDR))
#define UCSIM_IF_CMD_STOP 's'

static unsigned long do_mod(const unsigned long a, const unsigned long b) {
    return a % b;
}

void main(void) {
    volatile unsigned long foo = do_mod(870005460UL, 0UL);

    UCSIM_IF = UCSIM_IF_CMD_STOP;
}

When run with ucSim it should immediately stop, but instead hangs in an infinite loop. If you manually initiate a stop, you'll see the program is still inside _modulong (as per .map file):

Stop at 0x008072: (105) User stopped
F 0x008072
Simulated 1666642 ticks (8.333e-01 sec)
Host usage: 15.874542 sec, rate=0.052494

Apparently this bug was noticed in the separate implementation of _modulong in the PIC standard library, as discussed in bug #2130, back in 2013, and was fixed, but I guess it was either missed in the platform-agnostic implementation, or there has since been a regression.

The PIC library implementation has the following extra divide-by-zero check, but is otherwise identical:

  if (!b)
    {
      /* Prevent endless loop in case of division by 0. */
      return ~0UL;
    } // if

I propose this addition is added to the platform-agnostic implementation too.

[Basil Hussain]

By the way, although the C standard specifies that the result of a % b when b is zero is undefined, is returning ~0UL (i.e. 4294967295) actually the 'proper' thing to do?

I pose the question because this differs from the result you would get for remainder from the algorithm used in _divulong, if it were to yield such a result. The value it would yield is the original dividend value (i.e. a).

[Philipp Klaus Krause]

Division by zero is undefined behavior in the C standard. From that perspective an endless loop is fine. And I don't really see a better alternative anyway.

[Basil Hussain]

An endless loop is fine? I think "hang the user's program" is a pretty cynical and punitive interpretation of "undefined behaviour".

Besides, such behaviour puts it in contradiction to how _divulong behaves, which is fine with a divisor of zero. Also to how supported CPUs with native division instructions (e.g. STM8, HC08, maybe others I'm not aware of) handle division by zero, where the result is simply indeterminate.

And, if this behaviour is to be categorised as "so what? standard allows for it", why did bug #2130 get a fix?

[Philipp Klaus Krause]

I guess Raphael, who looked into [bugs:#2130] had a different idea about what is reasonable here.
IMO, there is no single good way of handling this. Some users, like you, prefer an indeterminate value; other users prefer a hang (i.e. something that clearly points out there is an issue, while the indeterminate value might result in silent corruption of data for them).

Personally, in such situations, so far I've decided to not put any effort into ensuring a certain behavior. Instead I just implement whatever is most efficient.

EDIT: Fix bug number

[Maarten Brock]

I agree with Philipp here. On a hosted environment (e.g. linux/windows) you would simply get a crashed application due to an uncaught exception. On other non-hosted systems (e.g. ARM Cortex) that can catch the exception you would also hang in a default handler.

[Basil Hussain]

I suppose the question of whether a hanging program is more or less acceptable depends on what means are available for the user to mitigate it. Granted, where you have platform features such as watchdog timers, it is easy. Otherwise, you can't. Do the majority of platforms supported by SDCC have such features?

However, I believe a key thing here is that there is consistency between how a division or modulus operation behaves when _divulong or _modulong (or their char, int, long long siblings) are employed, and when they are not (i.e. native CPU instructions). To your average developer, the difference between (unsigned int)a % (unsigned int)b and (unsigned long)a % (unsigned long)b is wholly transparent. They know not that one arithmetic operation may be native and one may be emulated in software, but they are undoubtedly confused and surprised when they do a divide-by-zero and one simply gives a 'wrong' (but predictable) answer, and one locks up their entire target device.

Is there any merit in looking at changing these software division/modulus routines so that they behave the same as the native equivalent operations (if they exist, naturally) on each target platform? For example, if a native division instruction gives an indeterminate answer, do just that; if it triggers an exception or trap that can be handled by ISR, do that.