Closed Bug 603077 Opened 14 years ago Closed 14 years ago

crash [@ JSC::X86Assembler::movl_i32m(int, int, JSC::X86Registers::RegisterID) ]

Categories

(Core :: JavaScript Engine, defect)

x86
Windows XP
defect
Not set
critical

Tracking

()

RESOLVED FIXED
Tracking Status
blocking2.0 --- beta7+

People

(Reporter: scoobidiver, Assigned: dmandelin)

References

Details

(Keywords: crash, regression, Whiteboard: fixed-in-tracemonkey)

Crash Data

Attachments

(2 files)

Build: Mozilla/5.0 (Windows NT 6.1; rv:2.0b8pre) Gecko/20101008 Firefox/4.0b8pre It is a new crash signature. Crashes first appeared in b7pre/20100914, then reappeared in b7pre/20100930 and above. Signature JSC::X86Assembler::movl_i32m(int, int, JSC::X86Registers::RegisterID) UUID 7db0b4ee-e3d7-4500-b0a8-af2922101009 Time 2010-10-09 02:30:59.416810 Uptime 594 Last Crash 1037 seconds (17.3 minutes) before submission Install Age 69649 seconds (19.3 hours) since version was first installed. Product Firefox Version 4.0b8pre Build ID 20101008041525 Branch 2.0 OS Windows NT OS Version 5.1.2600 Service Pack 3 CPU x86 CPU Info AuthenticAMD family 15 model 72 stepping 2 Crash Reason EXCEPTION_ACCESS_VIOLATION_WRITE Crash Address 0x41bfbd App Notes AdapterVendorID: 1002, AdapterDeviceID: 5975 Frame Module Signature [Expand] Source 0 mozjs.dll JSC::X86Assembler::movl_i32m js/src/assembler/assembler/X86Assembler.h:1413 1 mozjs.dll js::mjit::Assembler::storeValue<JSC::AbstractMacroAssembler<JSC::X86Assembler>::Address> js/src/methodjit/NunboxAssembler.h:166 2 mozjs.dll js::mjit::FrameState::syncData js/src/methodjit/FrameState-inl.h:512 3 mozjs.dll js::mjit::FrameState::syncAndKill js/src/methodjit/FrameState.cpp:569 4 mozjs.dll js::mjit::Compiler::prepareStubCall js/src/methodjit/Compiler.cpp:1870 5 mozjs.dll js::mjit::Compiler::generateMethod js/src/methodjit/Compiler.cpp:1253 6 mozjs.dll js::mjit::Compiler::Compile js/src/methodjit/Compiler.cpp:149 7 mozjs.dll js::mjit::TryCompile js/src/methodjit/Compiler.cpp:179 8 mozjs.dll js::RunScript js/src/jsinterp.cpp:630 9 mozjs.dll js::Execute js/src/jsinterp.cpp:903 10 mozjs.dll obj_eval js/src/jsobj.cpp:1268 11 @0x76820bf0 12 mozjs.dll EnterMethodJIT js/src/methodjit/MethodJIT.cpp:752 13 mozjs.dll js::mjit::JaegerShot js/src/methodjit/MethodJIT.cpp:780 14 mozjs.dll js::RunScript js/src/jsinterp.cpp:635 15 mozjs.dll js::Execute js/src/jsinterp.cpp:903 16 mozjs.dll JS_EvaluateUCScriptForPrincipals js/src/jsapi.cpp:4763 17 mozjs.dll JS_EvaluateUCScriptForPrincipalsVersion js/src/jsapi.cpp:4740 ... More reports at: http://crash-stats.mozilla.com/report/list?product=Firefox&query_search=signature&query_type=exact&query=&range_value=4&range_unit=weeks&hang_type=any&process_type=any&plugin_field=&plugin_query_type=&plugin_query=&do_query=1&admin=&signature=JSC%3A%3AX86Assembler%3A%3Amovl_i32m%28int%2C%20int%2C%20JSC%3A%3AX86Registers%3A%3ARegisterID%29
blocking2.0: --- → ?
I've gotten this 4-5 times myself over the past few days. Let me know if additional information would be helpful. It *might* be related to TBPL, but I'm not sure of that (it happens at seemingly random times, not associated with particular page loads or interactions).
blocking2.0: ? → beta7+
I checked a minidump, and found this to be an OOM in AssemblerBuffer::grow. It goes into the realloc case, and doesn't check for 0 return value, so it crashes back in oneByteOp.
Assignee: general → dmandelin
Attached patch Patch (deleted) — Splinter Review
See big new comment in AssemblerBuffer.h for a description of the approach. To test handling of OOM, I ran jstests and trace-tests with patches that force the allocations to "fail" 100% of the time, 10% of the time, and 1% of the time and verified no crashes. I also verified that trace-test tests specifically throw OOM errors with the 100% fail patch on.
Attachment #483627 - Flags: review?(dvander)
Comment on attachment 483627 [details] [diff] [review] Patch >+++ b/js/src/methodjit/BaseCompiler.h >@@ -117,16 +117,17 @@ class LinkerHelper : public JSC::LinkBuf > JSC::ExecutablePool *ep = BaseCompiler::GetExecPool(cx, masm.size()); > if (!ep) > return ep; > > m_size = masm.size(); > m_code = executableCopy(masm, ep); > if (!m_code) { > ep->release(); >+ JS_ASSERT(masm.oom()); Does an executableCopy() failure also mean the OOM bit is set in masm? If not, the assert may be overzealous.
Attachment #483627 - Flags: review?(dvander) → review+
(In reply to comment #3) > Created attachment 483627 [details] [diff] [review] > Patch > > See big new comment in AssemblerBuffer.h for a description of the approach. > > To test handling of OOM, I ran jstests and trace-tests with patches that force > the allocations to "fail" 100% of the time, 10% of the time, and 1% of the time > and verified no crashes. I also verified that trace-test tests specifically > throw OOM errors with the 100% fail patch on. Cool! Can your fault injector be saved, #ifdef DEBUG, and enabled via an envar? Gavin Reaney of Picsel had such a thing. See bug 383932. /be
Pushed with bogo-assert removed: http://hg.mozilla.org/tracemonkey/rev/67a041f5f13b I will follow up with the fault injector.
Status: NEW → ASSIGNED
Whiteboard: fixed-in-tracemonkey
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
(In reply to comment #5) > (In reply to comment #3) > > Created attachment 483627 [details] [diff] [review] [details] > > Patch > > > > See big new comment in AssemblerBuffer.h for a description of the approach. > > > > To test handling of OOM, I ran jstests and trace-tests with patches that force > > the allocations to "fail" 100% of the time, 10% of the time, and 1% of the time > > and verified no crashes. I also verified that trace-test tests specifically > > throw OOM errors with the 100% fail patch on. > > Cool! Can your fault injector be saved, #ifdef DEBUG, and enabled via an envar? > Gavin Reaney of Picsel had such a thing. See bug 383932. By the way, my fault injector doesn't take over all of malloc--it's just in the AssemblerBuffer allocator, at the points it calls malloc/realloc. Is it still interesting enough to go in?
(In reply to comment #8) > Is it still interesting enough to go in? NOt sure. If it's easy to rewrite maybe not. Throwing away code only to have another such bug cause someone to reinvent even a small-ish wheel is always a bad feeling, if not always a bad trade. But if we can't keep it #ifdef DEBUG and disabled by envar test (one test only), then it'll bitrot anyway. Your call. /be
Attached patch OOM injector for testing (deleted) — Splinter Review
Attaching the patch. It's pretty trivial, so I think recording it here is sufficient.
Attachment #486764 - Attachment description: Patch → OOM injector for testing
Crash Signature: [@ JSC::X86Assembler::movl_i32m(int, int, JSC::X86Registers::RegisterID) ]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: