1046016 - IonMonkey (ARM): incorrect detection of the number of FP registers is causing SIGILL crashes.

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Douglas Crosher [:dougc]]

The logic for the detection of the number of ARM double precision (DP) registers supported by the VFP implementation is returning 32 registers even when VFPv3 is not supported.

Without the VFPv3 capability flag set, only 16 DP registers are supported.

When the VFPv3 flags is set, there may be 16 or 32 DP registers. When there are only 16 DP registers the VFPv3D16 flag is set, and this flags is only set when VFPv3 is available.

The current code in incorrectly checking the VFPv3D16 flag without firstly checking for the VFPv3 flag, and is checking the NEON flag which is not relevant.

With recent changes from bug 991153, invalid code is generated for device that do not support 32 DP registers which causes a crash with SIGILL.

Comment 1

[Douglas Crosher [:dougc]]

Attachment: Correct the detection of the number of floating point registers.

Comment 2

[Marty Rosenberg [:mjrosenb]]

Comment on attachment 8464504 [details] [diff] [review]
Correct the detection of the number of floating point registers.

Review of attachment 8464504 [details] [diff] [review]:
-----------------------------------------------------------------

It looks like this will only affect devices without VFPv3 support?

Comment 3

[Douglas Crosher [:dougc]]

(In reply to Marty Rosenberg [:mjrosenb] from comment #2)
> Comment on attachment 8464504 [details] [diff] [review]
> Correct the detection of the number of floating point registers.
> 
> Review of attachment 8464504 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> It looks like this will only affect devices without VFPv3 support?

Yes, that looks right. Neon support requires 32 DP.

Try run: https://tbpl.mozilla.org/?tree=Try&rev=941a681dab9b

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f56f74a7e562

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/f56f74a7e562

Comment 6

[Douglas Crosher [:dougc]]

Comment on attachment 8464504 [details] [diff] [review]
Correct the detection of the number of floating point registers.

Approval Request Comment
[Feature/regressing bug #]: Long standing issue, exposed by bug 991153 which landed on Aurora 33. The patched function is not used before this.

[User impact if declined]: Crashes, or bad code generation, on ARM devices without vfp3.

[Describe test coverage new/current, TBPL]: Local testing. TBPL try run.

[Risks and why]: Low risk. If the logic is wrong then the compiler will just use 16 rather than 32 DP registers, a performance matter.

[String/UUID change made/needed]: n/a

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/0b662794f7dc