Closed Bug 900890 Opened 12 years ago Closed 12 years ago

crash in js::ion::LinearScanAllocator::assign

Categories

(Core :: JavaScript Engine, defect)

25 Branch
x86
Windows 7
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla26
Tracking Status
firefox23 --- unaffected
firefox24 --- verified
firefox25 --- verified
firefox26 --- verified

People

(Reporter: scoobidiver, Assigned: nbp)

References

()

Details

(Keywords: crash, regression, reproducible)

Crash Data

Attachments

(2 files)

It started spiking in 25.0a1/20130720 and has been hit mainly by a single user but not only. The regression range might be: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=af4e3ce8c487&tochange=bf73e10f5e54 Signature js::ion::LinearScanAllocator::assign(js::ion::LAllocation) More Reports Search UUID 25c17c97-1134-4372-bc97-e7b922130802 Date Processed 2013-08-02 00:51:03.441835 Uptime 3932 Last Crash 10269 seconds before submission Install Age 37176 since version was first installed. Install Time 2013-08-01 14:01:46 Product Firefox Version 25.0a1 Build ID 20130801030223 Release Channel nightly OS Windows NT OS Version 6.3.9431 Build Architecture x86 Build Architecture Info GenuineIntel family 6 model 37 stepping 2 | 4 Crash Reason EXCEPTION_ACCESS_VIOLATION_READ Crash Address 0x0 App Notes AdapterVendorID: 0x1002, AdapterDeviceID: 0x68c1, AdapterSubsysID: 00000000, AdapterDriverVersion: 13.150.100.1000 D2D! D2D+ DWrite? DWrite+ D3D10 Layers! D3D10 Layers+ Frame Module Signature Source 0 mozjs.dll js::ion::LinearScanAllocator::assign(js::ion::LAllocation) js/src/ion/LinearScan.cpp 1 mozjs.dll js::ion::LinearScanAllocator::spill() js/src/ion/LinearScan.cpp 2 mozjs.dll js::ion::LinearScanAllocator::allocateRegisters() js/src/ion/LinearScan.cpp 3 mozjs.dll js::ion::LinearScanAllocator::go() js/src/ion/LinearScan.cpp 4 mozjs.dll js::ion::GenerateLIR(js::ion::MIRGenerator *) js/src/ion/Ion.cpp 5 mozjs.dll js::ion::CompileBackEnd(js::ion::MIRGenerator *,js::ion::MacroAssembler *) js/src/ion/Ion.cpp 6 mozjs.dll js::ion::IonCompile js/src/ion/Ion.cpp 7 mozjs.dll js::ion::Compile js/src/ion/Ion.cpp 8 mozjs.dll js::ion::CanEnter(JSContext *,js::RunState &) js/src/ion/Ion.cpp 9 mozjs.dll js::RunScript(JSContext *,js::RunState &) js/src/vm/Interpreter.cpp 10 mozjs.dll js::Invoke(JSContext *,JS::CallArgs,js::MaybeConstruct) js/src/vm/Interpreter.cpp 11 mozjs.dll js::CallOrConstructBoundFunction(JSContext *,unsigned int,JS::Value *) js/src/jsfun.cpp More reports at: https://crash-stats.mozilla.com/report/list?product=Firefox&signature=js%3A%3Aion%3A%3ALinearScanAllocator%3A%3Aassign%28js%3A%3Aion%3A%3ALAllocation%29
I am the very user who has submitted these reports. It appears that the Gecko Profiler extension (1.12.5) causes these crashes. This happens randomly on Facebook, often when I clicked for comments from posts. I can reliably hit the crash on this very page. http://www.mtr-shatincentrallink.hk/tc/public-consultation/consultation-meeting.html
(In reply to Xavier Fung from comment #1) > I can reliably hit the crash on this very page. > http://www.mtr-shatincentrallink.hk/tc/public-consultation/consultation- > meeting.html Me too with Gecko Profiler.
Gecko Profiler 1.12.5 was released on Jul 18 and my first crash happened on Jul 20 nightly.
It's a regression from bug 892426.
LSetElementCacheV now requires 7 registers on x86 (1 for the object, 2 for the boxed index, 2 for the boxed value, 2 temps) and with the profiler enabled we only have 6 registers (ebp is not available) so regalloc can't satisfy this..
Flags: needinfo?(nicolas.b.pierron)
(In reply to Jan de Mooij [:jandem] from comment #6) > LSetElementCacheV now requires 7 registers on x86 (1 for the object, 2 for > the boxed index, 2 for the boxed value, 2 temps) and with the profiler > enabled we only have 6 registers (ebp is not available) so regalloc can't > satisfy this.. 2 things: 1/ If we cannot allocate more registers, then we should properly fail the register allocation and not crash. 2/ And fix this issue by either: a. Adding support for stack allocation as argument of our ICs. b. Replacing the setElementIC by a call, if we are profiling.
Flags: needinfo?(nicolas.b.pierron)
nbp can you take this one? Thanks! (In reply to Nicolas B. Pierron [:nbp] from comment #7) > b. Replacing the setElementIC by a call, if we are profiling. This would be unfortunate.. The call will probably be a lot slower than an IC stub so it would make the profiler less reliable.
Assignee: general → nicolas.b.pierron
Status: NEW → ASSIGNED
Based on comment 6, this patch should fake the activation of the profiler in terms of register pressure, and thus should be visible in the test suite.
Attached patch bug900890.patchSplinter Review
Hannes pointed that masm.extractInt32 is reusing the payload register, and thus instead of having a special allocation for unboxing the value on x86 (as needed on x64), we can just use a bogus as a temp value, as long as the temp is dedicated to extractInt32. So I've renamed the temp to make one of them dedicated to handle the int value, and reuse the logic added by Hannes as part as Bug 880471. js1_5/Regress/regress-192414.js was failing if we remove ebp from the list of registers, and pass with this patch applied.
Attachment #787866 - Flags: review?(jdemooij)
Comment on attachment 787866 [details] [diff] [review] bug900890.patch Review of attachment 787866 [details] [diff] [review]: ----------------------------------------------------------------- Nice fix!
Attachment #787866 - Flags: review?(jdemooij) → review+
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
Comment on attachment 787866 [details] [diff] [review] bug900890.patch [Approval Request Comment] Bug caused by (feature/regressing bug #): 892426 User impact if declined: Crash on x86 when using the sampling profiler of Gecko. (not only the addon) Testing completed (on m-c, etc.): yes. Risk to taking this patch (and alternatives if risky): This patch use a function which has been introduced in Bug 880471, which has not yet proved to be faulty since Gecko 24. So I estimate the risk to be minimal. String or IDL/UUID changes made by this patch: N/A
Attachment #787866 - Flags: approval-mozilla-aurora?
(In reply to Scoobidiver from comment #0) > It started spiking in 25.0a1/20130720 and has been hit mainly by a single > user but not only. The regression range might be: > http://hg.mozilla.org/mozilla-central/ > pushloghtml?fromchange=af4e3ce8c487&tochange=bf73e10f5e54 Knowing that this bug is caused by Bug 892426, I am flagging previous versions as affected too.
Comment on attachment 787866 [details] [diff] [review] bug900890.patch [Approval Request Comment] Bug caused by (feature/regressing bug #): 892426 User impact if declined: Crash on x86 when using the sampling profiler of Gecko. (not only the addon) Testing completed (on m-c, etc.): yes. Risk to taking this patch (and alternatives if risky): This patch use a function which has been introduced in Bug 880471, which has not yet proved to be faulty since Gecko 24. So I estimate the risk to be minimal. String or IDL/UUID changes made by this patch: N/A
Attachment #787866 - Flags: approval-mozilla-beta?
Attachment #787866 - Flags: approval-mozilla-beta?
Attachment #787866 - Flags: approval-mozilla-beta+
Attachment #787866 - Flags: approval-mozilla-aurora?
Attachment #787866 - Flags: approval-mozilla-aurora+
I still see post fix crashes on FF 24 beta. Xavier, can you please confirm this is fixed ?
Flags: needinfo?(xavier114fch)
I use Nightly and cannot see any crash on the link posted in comment 1.
Flags: needinfo?(xavier114fch)
Assuming this is verified fixed based on Xavier's comment 19. Paul, if you're seeing a high volume of these signature please provide URLs so we can investigate if a follow-up bug should be reported.
This is safe, there are no crashes on FF > 24.0b5 in the last month.
Depends on: 905993
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: