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)
Tracking
()
RESOLVED
FIXED
mozilla34
People
(Reporter: dougc, Assigned: dougc)
Details
Attachments
(1 file)
854 bytes,
patch
|
mjrosenb
:
review+
Sylvestre
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•10 years ago
|
||
Attachment #8464504 -
Flags: review?(mrosenberg)
Comment 2•10 years ago
|
||
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+
Assignee | ||
Comment 3•10 years ago
|
||
(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
Comment 4•10 years ago
|
||
Keywords: checkin-needed
Comment 5•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
Assignee | ||
Comment 6•10 years ago
|
||
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?
Updated•10 years ago
|
status-firefox33:
--- → affected
status-firefox34:
--- → fixed
Updated•10 years ago
|
Attachment #8464504 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 7•10 years ago
|
||
You need to log in
before you can comment on or make changes to this bug.
Description
•