compiler-rt, v4.0: arm\udivsi3.S broken for division by zero

Hello,

I think the current implementation for the call "bl __aeabi_idiv0" in builtins\arm\udivsi3.S is broken.
At least for the case that __aeabi_idiv0 returns. (the provided implementation does)

Since LR is not preserved, the following JMP(lr) results in an endless loop.

Or is this an intentional change of the behavior?

The file contains another implementation enabled by __ARM_ARCH_EXT_IDIV__. This uses "b" instead of "bl".
(This works as in previous versions)

Cheers,

Peter

I think the current implementation for the call "bl __aeabi_idiv0" in
builtins\arm\udivsi3.S is broken.
At least for the case that __aeabi_idiv0 returns. (the provided
implementation does)

Since LR is not preserved, the following JMP(lr) results in an endless loop.

Or is this an intentional change of the behavior?

Hi Peter,

That is most certainly a bug. Weiming's patch was supposed to only
introduce Thumb1 code, not transform div0 into a busy loop. :slight_smile:

The file contains another implementation enabled by __ARM_ARCH_EXT_IDIV__.
This uses "b" instead of "bl".
(This works as in previous versions)

The comment on the patch makes that clear:

bl __aeabi_idiv0 // due to relocation limit, can't use b.

I'm not sure why the bottom one is fine and the top one isn't, wrt.
range, but we may have to add a trampoline here.

Weiming, Saleem, comments?

cheers,
--renato

IMO it shouldn't. But it is kind of difficult to provide a good
implementation in general as it will depend on system-specific
interfaces, i.e. to raise the signal.

Joerg

I think that was the reasoning behind the decision to return. Let
systems that care about it dictate their own behaviour and default to
the softest reaction possible.

--renato

Yes, it's a bug.

Please review https://reviews.llvm.org/D31716

I recommend the other incarnation of the call to divby0 at line 256 is also
modified to use bl instead of b for the same reason.

Or just remove one of the two to avoid confusion in the future?

Cheers,

Peter

Using "b " is correct as it's a tail call.

However, for Thumb1, it can't use "b" due to +/- 2 KB range. "BL label" has a much wider range of +/- 4MB.

The reason we have two blocks of "divby0": conditional branch in Thumb1 has even shorter range -252 to +258, so it can't jump to the block around line 258.

So we cloned the "divby0" block. I think we can fold the 2nd "divby0" into line 39 to make it clear.

Thanks,

Weiming