One crash trace seen is: * thread #1: tid = 0x18ef, 0x0015ffe2 JavaScriptCore`JSC::ARMv7Assembler::executableOffsetFor(int) + 58, queue = 'com.apple.main-thread, stop reason = EXC_BAD_ACCESS (code=1, address=0x361672c) frame #0: 0x0015ffe2 JavaScriptCore`JSC::ARMv7Assembler::executableOffsetFor(int) + 58 frame #1: 0x0015ffa2 JavaScriptCore`JSC::MacroAssemblerARMv7::executableOffsetFor(int) + 18 frame #2: 0x0015ff80 JavaScriptCore`JSC::AssemblerLabel JSC::LinkBuffer::applyOffset<JSC::AssemblerLabel>(JSC::AssemblerLabel) + 20 frame #3: 0x0015eb94 JavaScriptCore`JSC::LinkBuffer::locationOf(JSC::AbstractMacroAssembler<JSC::ARMv7Assembler>::Label) + 32 frame #4: 0x0018e0ca JavaScriptCore`JSC::DFG::JITFinalizer::finalizeFunction() + 38 frame #5: 0x001c18ba JavaScriptCore`JSC::DFG::Plan::finalizeWithoutNotifyingCallback() + 62 frame #6: 0x00161504 JavaScriptCore`JSC::DFG::compileImpl(JSC::VM&, JSC::CodeBlock*, JSC::DFG::CompilationMode, unsigned int, JSC::Operands<JSC::JSValue, JSC::OperandValueTraits<JSC::JSValue> > const&, WTF::PassRefPtr<JSC::DeferredCompilationCallback>, JSC::DFG::Worklist*) + 932 frame #7: 0x00161116 JavaScriptCore`JSC::DFG::compile(JSC::VM&, JSC::CodeBlock*, JSC::DFG::CompilationMode, unsigned int, JSC::Operands<JSC::JSValue, JSC::OperandValueTraits<JSC::JSValue> > const&, WTF::PassRefPtr<JSC::DeferredCompilationCallback>, JSC::DFG::Worklist*) + 138 frame #8: 0x0027fdbc JavaScriptCore`operationOptimize + 1480 frame #9: 0x44230b79 The issue is that the ARMv7Assembler object is a local in Plan::compileInThreadImpl() and has been freed. ANother more obscure stack trace is: * thread #1: tid = 0x15f8, 0x00488726 JavaScriptCore`WTFCrash + 58 at Assertions.cpp:341, queue = 'com.apple.main-thread, stop reason = EXC_BAD_ACCESS (code=1, address=0xbbadbeef) frame #0: 0x00488726 JavaScriptCore`WTFCrash + 58 at Assertions.cpp:341 frame #1: 0x000ce5e0 JavaScriptCore`WTF::CrashOnOverflow::overflowed() + 8 at CheckedArithmetic.h:80 frame #2: 0x000aa67e JavaScriptCore`WTF::Vector<WTF::Vector<JSC::DFG::OSRExit, 8ul, WTF::CrashOnOverflow>*, 0ul, WTF::CrashOnOverflow>::at(this=0x0125deb4, i=848195) + 54 at Vector.h:584 frame #3: 0x000aa642 JavaScriptCore`WTF::Vector<WTF::Vector<JSC::DFG::OSRExit, 8ul, WTF::CrashOnOverflow>*, 0ul, WTF::CrashOnOverflow>::operator[](this=0x0125deb4, i=848195) + 18 at Vector.h:604 frame #4: 0x000aa5ac JavaScriptCore`WTF::SegmentedVector<JSC::DFG::OSRExit, 8ul, 32ul>::segmentFor(this=0x0125deb0, index=6785564) + 28 at SegmentedVector.h:217 frame #5: 0x000aa574 JavaScriptCore`WTF::SegmentedVector<JSC::DFG::OSRExit, 8ul, 32ul>::at(this=0x0125deb0, index=6785564) + 20 at SegmentedVector.h:128 frame #6: 0x000aa18a JavaScriptCore`WTF::SegmentedVector<JSC::DFG::OSRExit, 8ul, 32ul>::operator[](this=0x0125deb0, index=6785564) + 18 at SegmentedVector.h:138 frame #7: 0x001ba486 JavaScriptCore`compileOSRExit(exec=0x02bff7d8) + 246 at DFGOSRExitCompiler.cpp:61 frame #8: 0x5dec2ca7 frame #9: 0x0034e900 JavaScriptCore`llint_op_call + 218 …
<rdar://problem/15452643>
Created attachment 217341 [details] Patch
Attachment 217341 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source/JavaScriptCore/assembler/MacroAssembler.cpp', u'Source/JavaScriptCore/dfg/DFGJITFinalizer.cpp', u'Source/JavaScriptCore/dfg/DFGJITFinalizer.h']" exit_code: 1 Source/JavaScriptCore/assembler/MacroAssembler.cpp:1: Missing spaces around / [whitespace/operators] [3] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 217341 [details] Patch Attachment 217341 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/28628031
Created attachment 217344 [details] Fixed inadvertent typo
Comment on attachment 217344 [details] Fixed inadvertent typo View in context: https://bugs.webkit.org/attachment.cgi?id=217344&action=review > Source/JavaScriptCore/dfg/DFGJITFinalizer.cpp:44 > + if (m_arityCheck.isSet()) The old code didn't check m_arityCheck.isSet(). Why do we need to check it now? > Source/JavaScriptCore/dfg/DFGJITFinalizer.h:54 > MacroAssembler::Label m_arityCheck; > + MacroAssemblerCodePtr m_withArityCheck; It's not so great to have two data members with equivalent names, one of which works, and the other of which crashes your program. Can we remove the Label version of arityCheck? It appears to be used only in the JITFinalizer constructor function.
Comment on attachment 217344 [details] Fixed inadvertent typo View in context: https://bugs.webkit.org/attachment.cgi?id=217344&action=review > Source/JavaScriptCore/ChangeLog:9 > + Changed JITFinalizer constructor to calculate and save the with-arity-check entry point. > + At that point, the assembler object is still valid. In finalizeFunction(), we use that value Which assembler object are you talking about? Was it locationOf() that was using the invalid assembler? Is there a risk that finalizeCodeWithoutDisassembly() will use the invalid assembler?
(In reply to comment #7) > (From update of attachment 217344 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=217344&action=review > > > Source/JavaScriptCore/ChangeLog:9 > > + Changed JITFinalizer constructor to calculate and save the with-arity-check entry point. > > + At that point, the assembler object is still valid. In finalizeFunction(), we use that value > > Which assembler object are you talking about? Was it locationOf() that was using the invalid assembler? Is there a risk that finalizeCodeWithoutDisassembly() will use the invalid assembler? The assembler object is the MacroAssembler* m_assembler in LinkBuffer. Yes, it was LinkBuffer::locationOf() that was using the freed assembler. finalizeCodeWithoutDisassembly() is making the buffer ready for execution. It doesn't access the assembler. The only reason that locationOf() accesses the assembler is due to branch compaction. We store the delta in the assembler buffer as part of the compaction. I didn't find any other uses of m_assembler after the code has been linked.
Created attachment 217374 [details] Updated patch Changed the JITFinalizer() to take a resolved MacroAssemblerCodePtr instead of a MacroAssembler::Label.
Comment on attachment 217374 [details] Updated patch r=me It looks like, for the FTL JITFinalizer, we'll need to make a similar move away from Labels and toward post-relaxation pointers. Can you file a bug about that?
Committed r159577: <http://trac.webkit.org/changeset/159577>
(In reply to comment #10) > (From update of attachment 217374 [details]) > r=me > > It looks like, for the FTL JITFinalizer, we'll need to make a similar move away from Labels and toward post-relaxation pointers. Can you file a bug about that? Added https://bugs.webkit.org/show_bug.cgi?id=124674 - "FTLJITFinalizer shouldn't keep Labels to the contained generated code"