llvm dev - Apr 2017 - 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

On 21 March 2017 at 18:32, Peter Jakubek via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
> 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. :)

> 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

On Tue, Mar 21, 2017 at 07:32:50PM +0100, Peter Jakubek via llvm-dev
wrote:
> 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)
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

On 5 April 2017 at 12:40, Joerg Sonnenberger via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
> 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.
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


On 4/5/2017 3:50 AM, Renato Golin wrote:
> On 21 March 2017 at 18:32, Peter Jakubek via llvm-dev
> <llvm-dev at lists.llvm.org> wrote:
>> 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. :)
>
>
>> 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
-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The
Linux Foundation