Closed Bug 1046016 Opened 10 years ago Closed 10 years ago

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

Categories

(Core :: JavaScript Engine: JIT, defect)

ARM
All
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla34
Tracking Status
firefox33 --- fixed
firefox34 --- fixed

People

(Reporter: dougc, Assigned: dougc)

Details

Attachments

(1 file)

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 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?
Attachment #8464504 - Flags: review?(mrosenberg) → review+
(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
Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
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
Attachment #8464504 - Flags: approval-mozilla-aurora?
Attachment #8464504 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: